mirror of
https://github.com/getpaseo/paseo.git
synced 2026-10-25 18:52:39 +00:00
fix(cli): wait for daemon feature flags
Provider and pairing gates now wait for the daemon's server_info message instead of reading the cache immediately after connect. This removes a startup race where slower CLI cold starts could treat a supported daemon as missing the feature.
This commit is contained in:
@@ -33,10 +33,9 @@ export async function runPairCommand(options: PairOptions): Promise<void> {
|
||||
if (host) {
|
||||
const client = await tryConnectToDaemon({ host, timeout: 1500 });
|
||||
if (client) {
|
||||
const supportsDaemonStatusRpc =
|
||||
client.getLastServerInfoMessage()?.features?.daemonStatusRpc === true;
|
||||
if (supportsDaemonStatusRpc) {
|
||||
try {
|
||||
try {
|
||||
const serverInfo = await client.waitForServerInfo(PAIRING_DAEMON_RPC_TIMEOUT_MS);
|
||||
if (serverInfo.features?.daemonStatusRpc === true) {
|
||||
const offer = await client.getDaemonPairingOffer({
|
||||
timeout: PAIRING_DAEMON_RPC_TIMEOUT_MS,
|
||||
});
|
||||
@@ -46,11 +45,11 @@ export async function runPairCommand(options: PairOptions): Promise<void> {
|
||||
options,
|
||||
);
|
||||
return;
|
||||
} catch {
|
||||
// COMPAT(daemon-rpc-rollout): fall back to CLI-side pairing generation while
|
||||
// old daemons lack daemonStatusRpc. Remove once the daemon floor is past
|
||||
// v0.1.76; pairing should come from daemon.get_pairing_offer.
|
||||
}
|
||||
} catch {
|
||||
// COMPAT(daemon-rpc-rollout): fall back to CLI-side pairing generation while
|
||||
// old daemons lack daemonStatusRpc. Remove once the daemon floor is past
|
||||
// v0.1.76; pairing should come from daemon.get_pairing_offer.
|
||||
}
|
||||
await client.close().catch(() => {});
|
||||
}
|
||||
|
||||
@@ -27,7 +27,7 @@ interface RecordingClientInput {
|
||||
|
||||
function createClient(input: RecordingClientInput) {
|
||||
return {
|
||||
getLastServerInfoMessage: () => ({
|
||||
waitForServerInfo: async () => ({
|
||||
status: "server_info",
|
||||
serverId: "test-daemon",
|
||||
features: input.features ?? { paseoAgentCatalog: true },
|
||||
@@ -619,7 +619,7 @@ describe("provider add", () => {
|
||||
},
|
||||
write: () => {},
|
||||
connectDaemon: async () => ({
|
||||
getLastServerInfoMessage: () => ({
|
||||
waitForServerInfo: async () => ({
|
||||
status: "server_info",
|
||||
serverId: "test-daemon",
|
||||
features: {},
|
||||
|
||||
@@ -48,7 +48,7 @@ interface ProviderConfiguredItem {
|
||||
|
||||
interface ProviderAddClient extends Pick<
|
||||
DaemonClient,
|
||||
| "getLastServerInfoMessage"
|
||||
| "waitForServerInfo"
|
||||
| "getPaseoAgentCatalog"
|
||||
| "setPaseoAgentProvider"
|
||||
| "startPaseoAgentOAuth"
|
||||
@@ -462,7 +462,7 @@ export async function runAddCommand(
|
||||
const deps = { ...defaultDependencies, ...dependencies };
|
||||
const client = await deps.connectDaemon({ host: options.host });
|
||||
try {
|
||||
requirePaseoAgentCatalogFeature(client);
|
||||
await requirePaseoAgentCatalogFeature(client);
|
||||
const catalogResult = await client.getPaseoAgentCatalog();
|
||||
if (catalogResult.error) {
|
||||
throw {
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { ServerInfoStatusPayload } from "@getpaseo/protocol/messages";
|
||||
|
||||
import { requirePaseoAgentCatalogFeature } from "./feature.js";
|
||||
|
||||
function createServerInfo(features: ServerInfoStatusPayload["features"]): ServerInfoStatusPayload {
|
||||
return {
|
||||
status: "server_info",
|
||||
serverId: "test-daemon",
|
||||
features,
|
||||
};
|
||||
}
|
||||
|
||||
describe("provider feature gate", () => {
|
||||
it("waits for delayed server_info before allowing catalog commands", async () => {
|
||||
let resolveServerInfo: ((serverInfo: ServerInfoStatusPayload) => void) | null = null;
|
||||
|
||||
const allowed = requirePaseoAgentCatalogFeature({
|
||||
waitForServerInfo: async () =>
|
||||
new Promise<ServerInfoStatusPayload>((resolve) => {
|
||||
resolveServerInfo = resolve;
|
||||
}),
|
||||
});
|
||||
|
||||
expect(resolveServerInfo).not.toBeNull();
|
||||
resolveServerInfo?.(createServerInfo({ paseoAgentCatalog: true }));
|
||||
|
||||
await expect(allowed).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it("surfaces the server_info wait timeout", async () => {
|
||||
const timeoutError = new Error("Timed out waiting for server_info status message (5ms)");
|
||||
|
||||
await expect(
|
||||
requirePaseoAgentCatalogFeature({
|
||||
waitForServerInfo: async () => {
|
||||
throw timeoutError;
|
||||
},
|
||||
}),
|
||||
).rejects.toThrow("Timed out waiting for server_info status message (5ms)");
|
||||
});
|
||||
|
||||
it("requires the catalog feature when server_info arrives without it", async () => {
|
||||
await expect(
|
||||
requirePaseoAgentCatalogFeature({
|
||||
waitForServerInfo: async () => createServerInfo({ paseoAgentCatalog: false }),
|
||||
}),
|
||||
).rejects.toMatchObject({
|
||||
code: "HOST_UPDATE_REQUIRED",
|
||||
message: "Update the Paseo daemon to use this command.",
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1,13 +1,13 @@
|
||||
import type { DaemonClient } from "@getpaseo/client/internal/daemon-client";
|
||||
import type { CommandError } from "../../output/index.js";
|
||||
|
||||
export interface PaseoAgentCatalogFeatureClient extends Pick<
|
||||
DaemonClient,
|
||||
"getLastServerInfoMessage"
|
||||
> {}
|
||||
export interface PaseoAgentCatalogFeatureClient extends Pick<DaemonClient, "waitForServerInfo"> {}
|
||||
|
||||
export function requirePaseoAgentCatalogFeature(client: PaseoAgentCatalogFeatureClient): void {
|
||||
if (client.getLastServerInfoMessage()?.features?.paseoAgentCatalog === true) {
|
||||
export async function requirePaseoAgentCatalogFeature(
|
||||
client: PaseoAgentCatalogFeatureClient,
|
||||
): Promise<void> {
|
||||
const serverInfo = await client.waitForServerInfo();
|
||||
if (serverInfo.features?.paseoAgentCatalog === true) {
|
||||
return;
|
||||
}
|
||||
throw {
|
||||
|
||||
@@ -16,7 +16,7 @@ describe("provider ls", () => {
|
||||
it("renders an empty configured-provider table with headers", async () => {
|
||||
const result = await runLsCommand({ host: "localhost:7777" }, {} as never, {
|
||||
connectDaemon: async () => ({
|
||||
getLastServerInfoMessage: createServerInfo,
|
||||
waitForServerInfo: async () => createServerInfo(),
|
||||
getPaseoAgentCatalog: async () => ({
|
||||
requestId: "catalog-1",
|
||||
catalog: [],
|
||||
@@ -42,7 +42,7 @@ describe("provider ls", () => {
|
||||
connectDaemon: async (options) => {
|
||||
expect(options.host).toBe("localhost:7777");
|
||||
return {
|
||||
getLastServerInfoMessage: createServerInfo,
|
||||
waitForServerInfo: async () => createServerInfo(),
|
||||
getPaseoAgentCatalog: async () => ({
|
||||
requestId: "catalog-1",
|
||||
catalog: [
|
||||
@@ -142,7 +142,7 @@ describe("provider rm", () => {
|
||||
connectDaemon: async (options) => {
|
||||
expect(options.host).toBe("localhost:7777");
|
||||
return {
|
||||
getLastServerInfoMessage: createServerInfo,
|
||||
waitForServerInfo: async () => createServerInfo(),
|
||||
removePaseoAgentProvider: async (name: string) => {
|
||||
removedNames.push(name);
|
||||
return {
|
||||
|
||||
@@ -25,7 +25,7 @@ export interface ProviderListItem {
|
||||
|
||||
interface ProviderLsClient extends Pick<
|
||||
DaemonClient,
|
||||
"getLastServerInfoMessage" | "getPaseoAgentCatalog" | "getPaseoAgentProviders" | "close"
|
||||
"waitForServerInfo" | "getPaseoAgentCatalog" | "getPaseoAgentProviders" | "close"
|
||||
> {}
|
||||
|
||||
export interface ProviderLsDependencies {
|
||||
@@ -80,7 +80,7 @@ export async function runLsCommand(
|
||||
const client = await deps.connectDaemon({ host: options.host });
|
||||
|
||||
try {
|
||||
requirePaseoAgentCatalogFeature(client);
|
||||
await requirePaseoAgentCatalogFeature(client);
|
||||
const catalogResult = await client.getPaseoAgentCatalog();
|
||||
if (catalogResult.error) {
|
||||
throw {
|
||||
|
||||
@@ -21,7 +21,7 @@ interface ProviderRemoveItem {
|
||||
|
||||
interface ProviderRmClient extends Pick<
|
||||
DaemonClient,
|
||||
"getLastServerInfoMessage" | "removePaseoAgentProvider" | "close"
|
||||
"waitForServerInfo" | "removePaseoAgentProvider" | "close"
|
||||
> {}
|
||||
|
||||
export interface ProviderRmDependencies {
|
||||
@@ -49,7 +49,7 @@ export async function runRmCommand(
|
||||
const deps = { ...defaultDependencies, ...dependencies };
|
||||
const client = await deps.connectDaemon({ host: options.host });
|
||||
try {
|
||||
requirePaseoAgentCatalogFeature(client);
|
||||
await requirePaseoAgentCatalogFeature(client);
|
||||
const result = await client.removePaseoAgentProvider(name);
|
||||
if (!result.success) {
|
||||
throw {
|
||||
|
||||
@@ -42,6 +42,20 @@ interface ProviderListRow {
|
||||
models: string;
|
||||
}
|
||||
|
||||
interface CliResult {
|
||||
exitCode: number;
|
||||
stdout: string;
|
||||
stderr: string;
|
||||
}
|
||||
|
||||
function assertExitCode(result: CliResult, expected: number, message: string): void {
|
||||
assert.strictEqual(result.exitCode, expected, `${message}\nstderr:\n${result.stderr}`);
|
||||
}
|
||||
|
||||
function assertNonZeroExitCode(result: CliResult, message: string): void {
|
||||
assert.notStrictEqual(result.exitCode, 0, `${message}\nstderr:\n${result.stderr}`);
|
||||
}
|
||||
|
||||
const EXPECTED_CLAUDE_MODELS = [
|
||||
{
|
||||
id: "claude-fable-5",
|
||||
@@ -137,7 +151,7 @@ function parseProviderListJson(stdout: string): ProviderListRow[] {
|
||||
|
||||
async function getProviderRows(): Promise<ProviderListRow[]> {
|
||||
const result = await ctx.paseo(["provider", "ls", "--json"]);
|
||||
assert.strictEqual(result.exitCode, 0, "provider ls --json should exit 0");
|
||||
assertExitCode(result, 0, "provider ls --json should exit 0");
|
||||
return parseProviderListJson(result.stdout);
|
||||
}
|
||||
|
||||
@@ -181,7 +195,7 @@ try {
|
||||
{
|
||||
console.log("Test 1: provider --help shows subcommands");
|
||||
const result = await ctx.paseo(["provider", "--help"]);
|
||||
assert.strictEqual(result.exitCode, 0, "provider --help should exit 0");
|
||||
assertExitCode(result, 0, "provider --help should exit 0");
|
||||
assert(result.stdout.includes("ls"), "help should mention ls");
|
||||
assert(result.stdout.includes("add"), "help should mention add");
|
||||
assert(result.stdout.includes("rm"), "help should mention rm");
|
||||
@@ -193,12 +207,12 @@ try {
|
||||
{
|
||||
console.log("Test 2: provider ls on a fresh daemon shows no configured providers");
|
||||
const result = await ctx.paseo(["provider", "ls"]);
|
||||
assert.strictEqual(result.exitCode, 0, "provider ls should exit 0");
|
||||
assertExitCode(result, 0, "provider ls should exit 0");
|
||||
assertProviderTableHeader(result.stdout);
|
||||
assert(!result.stdout.includes("OpenRouter"), "fresh output should have no provider rows");
|
||||
|
||||
const jsonResult = await ctx.paseo(["provider", "ls", "--json"]);
|
||||
assert.strictEqual(jsonResult.exitCode, 0, "provider ls --json should exit 0");
|
||||
assertExitCode(jsonResult, 0, "provider ls --json should exit 0");
|
||||
assert.deepStrictEqual(parseProviderListJson(jsonResult.stdout), []);
|
||||
console.log("✓ provider ls on a fresh daemon shows no configured providers\n");
|
||||
}
|
||||
@@ -209,7 +223,7 @@ try {
|
||||
const result = await ctx.paseo(["provider", "add", "openrouter", "--api-key-stdin"], {
|
||||
stdin: "dummy-openrouter-key\n",
|
||||
});
|
||||
assert.strictEqual(result.exitCode, 0, `provider add should exit 0\n${result.stderr}`);
|
||||
assertExitCode(result, 0, "provider add should exit 0");
|
||||
assert(result.stdout.includes("openrouter"), "add output should include the instance name");
|
||||
assert(result.stdout.includes("OpenRouter"), "add output should include the catalog label");
|
||||
assert(result.stdout.includes("Connected"), "add output should show connected auth state");
|
||||
@@ -234,7 +248,7 @@ try {
|
||||
const result = await ctx.paseo(["provider", "add", "openrouter", "--api-key-stdin"], {
|
||||
stdin: "dummy-openrouter-key-2\n",
|
||||
});
|
||||
assert.strictEqual(result.exitCode, 0, `provider add should exit 0\n${result.stderr}`);
|
||||
assertExitCode(result, 0, "provider add should exit 0");
|
||||
|
||||
const rows = await getProviderRows();
|
||||
assert.strictEqual(rows.length, 1, "re-running add should not create another instance");
|
||||
@@ -249,7 +263,7 @@ try {
|
||||
const result = await ctx.paseo(["provider", "add", "nonsense-id", "--api-key-stdin"], {
|
||||
stdin: "dummy-key\n",
|
||||
});
|
||||
assert.notStrictEqual(result.exitCode, 0, "provider add should fail for unknown ids");
|
||||
assertNonZeroExitCode(result, "provider add should fail for unknown ids");
|
||||
const output = result.stdout + result.stderr;
|
||||
assert(output.includes("nonsense-id"), "error should mention the requested id");
|
||||
assert(output.includes("Known provider ids"), "error should mention known provider ids");
|
||||
@@ -262,14 +276,14 @@ try {
|
||||
{
|
||||
console.log("Test 6: provider rm removes a configured provider");
|
||||
const result = await ctx.paseo(["provider", "rm", "openrouter"]);
|
||||
assert.strictEqual(result.exitCode, 0, `provider rm should exit 0\n${result.stderr}`);
|
||||
assertExitCode(result, 0, "provider rm should exit 0");
|
||||
assert(result.stdout.includes("openrouter"), "rm output should include the instance name");
|
||||
assert(result.stdout.includes("yes"), "rm output should report removal");
|
||||
|
||||
const rows = await getProviderRows();
|
||||
assert.deepStrictEqual(rows, [], "provider ls should be empty after removing openrouter");
|
||||
const table = await ctx.paseo(["provider", "ls"]);
|
||||
assert.strictEqual(table.exitCode, 0, "provider ls should stay successful after removal");
|
||||
assertExitCode(table, 0, "provider ls should stay successful after removal");
|
||||
assertProviderTableHeader(table.stdout);
|
||||
console.log("✓ provider rm removes a configured provider\n");
|
||||
}
|
||||
@@ -280,7 +294,7 @@ try {
|
||||
const result = await ctx.paseo(["provider", "add", "kimi", "--api-key-stdin"], {
|
||||
stdin: "dummy-kimi-key\n",
|
||||
});
|
||||
assert.strictEqual(result.exitCode, 0, `provider add kimi should exit 0\n${result.stderr}`);
|
||||
assertExitCode(result, 0, "provider add kimi should exit 0");
|
||||
assert(result.stdout.includes("kimi"), "add output should include the instance name");
|
||||
assert(result.stdout.includes("Kimi Coding Plan"), "add output should include the label");
|
||||
|
||||
@@ -356,7 +370,7 @@ try {
|
||||
{
|
||||
console.log("Test 11: provider models unknown fails with error");
|
||||
const result = await ctx.paseo(["provider", "models", "unknown"]);
|
||||
assert.notStrictEqual(result.exitCode, 0, "should fail for unknown provider");
|
||||
assertNonZeroExitCode(result, "should fail for unknown provider");
|
||||
const output = result.stdout + result.stderr;
|
||||
assert(
|
||||
output.toLowerCase().includes("unknown") || output.toLowerCase().includes("provider"),
|
||||
@@ -388,7 +402,7 @@ try {
|
||||
"claude model IDs should be captured from --json output",
|
||||
);
|
||||
const result = await ctx.paseo(["provider", "models", "claude", "--quiet"]);
|
||||
assert.strictEqual(result.exitCode, 0, "should exit 0");
|
||||
assertExitCode(result, 0, "provider models claude --quiet should exit 0");
|
||||
const lines = result.stdout.trim().split("\n").filter(Boolean);
|
||||
assert.strictEqual(
|
||||
lines.length,
|
||||
|
||||
@@ -75,6 +75,30 @@ function createMockTransport() {
|
||||
return {
|
||||
transport,
|
||||
sent,
|
||||
sendServerInfo: (input: { serverId?: string; features?: Record<string, boolean> } = {}) => {
|
||||
onMessage(
|
||||
JSON.stringify({
|
||||
type: "session",
|
||||
message: {
|
||||
type: "status",
|
||||
payload: {
|
||||
status: "server_info",
|
||||
serverId: input.serverId ?? `srv_test_${serverInfoOrdinal++}`,
|
||||
hostname: null,
|
||||
version: null,
|
||||
...(input.features ? { features: input.features } : {}),
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
},
|
||||
triggerOpenWithoutServerInfo: (options?: { preserveSent?: boolean }) => {
|
||||
onOpen();
|
||||
if (!options?.preserveSent) {
|
||||
// Ignore HELLO handshake payloads in assertions.
|
||||
sent.length = 0;
|
||||
}
|
||||
},
|
||||
triggerOpen: (options?: { preserveSent?: boolean }) => {
|
||||
onOpen();
|
||||
if (!options?.preserveSent) {
|
||||
@@ -191,6 +215,57 @@ test("advertises consumer-provided browser automation capabilities", async () =>
|
||||
expect(hello.capabilities[CLIENT_CAPS.desktopBrowserAutomation]).toBe(true);
|
||||
});
|
||||
|
||||
test("waitForServerInfo resolves when server_info arrives after the socket opens", async () => {
|
||||
vi.useFakeTimers();
|
||||
const mock = createMockTransport();
|
||||
const client = new DaemonClient({
|
||||
url: "ws://test",
|
||||
clientId: "server_info_wait_unit_test",
|
||||
transportFactory: () => mock.transport,
|
||||
reconnect: { enabled: false },
|
||||
});
|
||||
clients.push(client);
|
||||
|
||||
const serverInfoPromise = client.waitForServerInfo(100);
|
||||
const connectPromise = client.connect();
|
||||
mock.triggerOpenWithoutServerInfo();
|
||||
setTimeout(() => {
|
||||
mock.sendServerInfo({
|
||||
serverId: "srv_delayed_server_info",
|
||||
features: { paseoAgentCatalog: true },
|
||||
});
|
||||
}, 25);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(25);
|
||||
|
||||
await expect(serverInfoPromise).resolves.toMatchObject({
|
||||
status: "server_info",
|
||||
serverId: "srv_delayed_server_info",
|
||||
features: { paseoAgentCatalog: true },
|
||||
});
|
||||
await connectPromise;
|
||||
});
|
||||
|
||||
test("waitForServerInfo rejects clearly when server_info never arrives", async () => {
|
||||
vi.useFakeTimers();
|
||||
const mock = createMockTransport();
|
||||
const client = new DaemonClient({
|
||||
url: "ws://test",
|
||||
clientId: "server_info_timeout_unit_test",
|
||||
transportFactory: () => mock.transport,
|
||||
reconnect: { enabled: false },
|
||||
});
|
||||
clients.push(client);
|
||||
|
||||
const serverInfoPromise = client.waitForServerInfo(50);
|
||||
const rejection = expect(serverInfoPromise).rejects.toThrow(
|
||||
"Timed out waiting for server_info status message (50ms)",
|
||||
);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(50);
|
||||
await rejection;
|
||||
});
|
||||
|
||||
const noopLogger: Logger = {
|
||||
debug: () => {},
|
||||
info: () => {},
|
||||
|
||||
@@ -840,6 +840,7 @@ const DEFAULT_RECONNECT_BASE_DELAY_MS = 1500;
|
||||
const DEFAULT_RECONNECT_MAX_DELAY_MS = 30000;
|
||||
const DEFAULT_SESSION_RPC_TIMEOUT_MS = 60_000;
|
||||
const DEFAULT_CONNECT_TIMEOUT_MS = 15_000;
|
||||
const DEFAULT_SERVER_INFO_TIMEOUT_MS = 10_000;
|
||||
const DEFAULT_LIVENESS_TIMEOUT_MS = 5000;
|
||||
const LIVENESS_HEARTBEAT_INTERVAL_MS = 10_000;
|
||||
const LIVENESS_HEARTBEAT_TIMEOUT_MS = 15_000;
|
||||
@@ -4741,6 +4742,29 @@ export class DaemonClient {
|
||||
return this.lastServerInfoMessage;
|
||||
}
|
||||
|
||||
async waitForServerInfo(
|
||||
timeoutMs = DEFAULT_SERVER_INFO_TIMEOUT_MS,
|
||||
): Promise<ServerInfoStatusPayload> {
|
||||
if (this.lastServerInfoMessage) {
|
||||
return this.lastServerInfoMessage;
|
||||
}
|
||||
|
||||
const { promise } = this.waitForWithCancel<ServerInfoStatusPayload>(
|
||||
(msg) => {
|
||||
if (msg.type !== "status") {
|
||||
return null;
|
||||
}
|
||||
return parseServerInfoStatusPayload(msg.payload);
|
||||
},
|
||||
timeoutMs,
|
||||
{
|
||||
timeoutMessage: `Timed out waiting for server_info status message (${timeoutMs}ms)`,
|
||||
},
|
||||
);
|
||||
|
||||
return promise;
|
||||
}
|
||||
|
||||
private resolveTransportUrlForAttempt(): string {
|
||||
return this.config.url;
|
||||
}
|
||||
@@ -5258,10 +5282,12 @@ export class DaemonClient {
|
||||
private waitForWithCancel<T>(
|
||||
predicate: (msg: SessionOutboundMessage) => T | null,
|
||||
timeout = 30000,
|
||||
_options?: { skipQueue?: boolean },
|
||||
options?: { skipQueue?: boolean; timeoutMessage?: string },
|
||||
): WaitHandle<T> {
|
||||
// Capture stack trace at call site, not inside setTimeout
|
||||
const timeoutError = new Error(`Timeout waiting for message (${timeout}ms)`);
|
||||
const timeoutError = new Error(
|
||||
options?.timeoutMessage ?? `Timeout waiting for message (${timeout}ms)`,
|
||||
);
|
||||
|
||||
let waiter: Waiter<T> | null = null;
|
||||
let settled = false;
|
||||
|
||||
Reference in New Issue
Block a user