feat(S-102): Test-drive and implement --timeout flag, config layering, and default in
This commit is contained in:
@@ -21,7 +21,7 @@ export interface CliDeps {
|
|||||||
prompt: string,
|
prompt: string,
|
||||||
config?: { client?: ClientName; defaultClient?: ClientName }
|
config?: { client?: ClientName; defaultClient?: ClientName }
|
||||||
) => ClientName | null;
|
) => ClientName | null;
|
||||||
resolveConfig?: () => { paths: Partial<Record<ClientName, string>>; defaultClient?: ClientName };
|
resolveConfig?: () => { paths: Partial<Record<ClientName, string>>; defaultClient?: ClientName; timeout?: number };
|
||||||
stdoutWrite?: (chunk: string) => void;
|
stdoutWrite?: (chunk: string) => void;
|
||||||
stderrWrite?: (chunk: string) => void;
|
stderrWrite?: (chunk: string) => void;
|
||||||
}
|
}
|
||||||
@@ -54,7 +54,7 @@ export async function main(
|
|||||||
rawArgs[0]?.includes("cli.ts") ? rawArgs.slice(1) : rawArgs;
|
rawArgs[0]?.includes("cli.ts") ? rawArgs.slice(1) : rawArgs;
|
||||||
|
|
||||||
const args = minimist(parseArgs, {
|
const args = minimist(parseArgs, {
|
||||||
string: ["client", "prompt"],
|
string: ["client", "prompt", "timeout"],
|
||||||
boolean: ["json", "text", "help", "debug"],
|
boolean: ["json", "text", "help", "debug"],
|
||||||
alias: { h: "help" },
|
alias: { h: "help" },
|
||||||
});
|
});
|
||||||
@@ -114,8 +114,12 @@ export async function main(
|
|||||||
return 1;
|
return 1;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const config = resolveConfig();
|
||||||
|
const timeoutMs =
|
||||||
|
typeof args.timeout === "string" ? Number(args.timeout) : config.timeout;
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const result = await executePrompt(client, prompt);
|
const result = await executePrompt(client, prompt, { timeoutMs });
|
||||||
if (jsonMode) {
|
if (jsonMode) {
|
||||||
console.log(JSON.stringify(result, null, 2));
|
console.log(JSON.stringify(result, null, 2));
|
||||||
} else {
|
} else {
|
||||||
@@ -167,8 +171,11 @@ export async function main(
|
|||||||
return 1;
|
return 1;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const timeoutMs =
|
||||||
|
typeof args.timeout === "string" ? Number(args.timeout) : config.timeout;
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const result = await executePrompt(client, prompt);
|
const result = await executePrompt(client, prompt, { timeoutMs });
|
||||||
if (jsonMode) {
|
if (jsonMode) {
|
||||||
console.log(JSON.stringify(result, null, 2));
|
console.log(JSON.stringify(result, null, 2));
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ import { CLIENT_NAMES, isWindows } from "./constants.js";
|
|||||||
export interface ResolvedConfig {
|
export interface ResolvedConfig {
|
||||||
paths: Partial<Record<ClientName, string>>;
|
paths: Partial<Record<ClientName, string>>;
|
||||||
defaultClient?: ClientName;
|
defaultClient?: ClientName;
|
||||||
|
timeout?: number;
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface ResolveConfigOptions {
|
export interface ResolveConfigOptions {
|
||||||
@@ -89,5 +90,22 @@ export function resolveConfig(
|
|||||||
) {
|
) {
|
||||||
result.defaultClient = defaultClient as ClientName;
|
result.defaultClient = defaultClient as ClientName;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const flagTimeout =
|
||||||
|
typeof flags.timeout === "string" ? Number(flags.timeout) : undefined;
|
||||||
|
const envTimeout =
|
||||||
|
typeof env.AI_CLI_TIMEOUT === "string"
|
||||||
|
? Number(env.AI_CLI_TIMEOUT)
|
||||||
|
: undefined;
|
||||||
|
const fileTimeout =
|
||||||
|
typeof fileConfig.timeout === "number" ? fileConfig.timeout : undefined;
|
||||||
|
|
||||||
|
const resolvedTimeout =
|
||||||
|
(Number.isFinite(flagTimeout) ? flagTimeout : undefined) ??
|
||||||
|
(Number.isFinite(envTimeout) ? envTimeout : undefined) ??
|
||||||
|
(Number.isFinite(fileTimeout) ? fileTimeout : undefined) ??
|
||||||
|
600_000;
|
||||||
|
|
||||||
|
result.timeout = resolvedTimeout;
|
||||||
return result;
|
return result;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -28,12 +28,14 @@ export async function executePrompt(
|
|||||||
stdout: "",
|
stdout: "",
|
||||||
stderr: "",
|
stderr: "",
|
||||||
exitCode: -1,
|
exitCode: -1,
|
||||||
|
client,
|
||||||
|
durationMs: 0,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
const spawnImpl = options.spawn ?? defaultSpawn;
|
const spawnImpl = options.spawn ?? defaultSpawn;
|
||||||
const existsSyncImpl = options.existsSync ?? defaultExistsSync;
|
const existsSyncImpl = options.existsSync ?? defaultExistsSync;
|
||||||
const timeoutMs = options.timeoutMs ?? 300_000;
|
const timeoutMs = options.timeoutMs ?? 600_000;
|
||||||
|
|
||||||
const command = options.clientPath ?? client;
|
const command = options.clientPath ?? client;
|
||||||
if (options.clientPath && !existsSyncImpl(options.clientPath)) {
|
if (options.clientPath && !existsSyncImpl(options.clientPath)) {
|
||||||
@@ -46,6 +48,8 @@ export async function executePrompt(
|
|||||||
stdout: "",
|
stdout: "",
|
||||||
stderr: "",
|
stderr: "",
|
||||||
exitCode: -1,
|
exitCode: -1,
|
||||||
|
client,
|
||||||
|
durationMs: 0,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
const args = argBuilder(prompt);
|
const args = argBuilder(prompt);
|
||||||
@@ -55,6 +59,7 @@ export async function executePrompt(
|
|||||||
let timedOut = false;
|
let timedOut = false;
|
||||||
let stdout = "";
|
let stdout = "";
|
||||||
let stderr = "";
|
let stderr = "";
|
||||||
|
const startMs = Date.now();
|
||||||
|
|
||||||
const child = spawnImpl(command, args, {
|
const child = spawnImpl(command, args, {
|
||||||
shell: false,
|
shell: false,
|
||||||
@@ -80,8 +85,16 @@ export async function executePrompt(
|
|||||||
if (settled) return;
|
if (settled) return;
|
||||||
settled = true;
|
settled = true;
|
||||||
clearTimeout(timeout);
|
clearTimeout(timeout);
|
||||||
if (err) reject(err);
|
const durationMs = Date.now() - startMs;
|
||||||
else resolve(result!);
|
if (err) {
|
||||||
|
if (err instanceof ExecError) {
|
||||||
|
err.result.client = client;
|
||||||
|
err.result.durationMs = durationMs;
|
||||||
|
}
|
||||||
|
reject(err);
|
||||||
|
} else {
|
||||||
|
resolve({ ...result!, client, durationMs });
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
child.on("error", (err: NodeJS.ErrnoException) => {
|
child.on("error", (err: NodeJS.ErrnoException) => {
|
||||||
@@ -89,7 +102,7 @@ export async function executePrompt(
|
|||||||
settle(new ClientNotFoundError(client));
|
settle(new ClientNotFoundError(client));
|
||||||
} else {
|
} else {
|
||||||
settle(
|
settle(
|
||||||
new ExecError(err.message, { stdout, stderr, exitCode: -1 })
|
new ExecError(err.message, { stdout, stderr, exitCode: -1, client, durationMs: 0 })
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
@@ -101,10 +114,12 @@ export async function executePrompt(
|
|||||||
stdout,
|
stdout,
|
||||||
stderr,
|
stderr,
|
||||||
exitCode: -1,
|
exitCode: -1,
|
||||||
|
client,
|
||||||
|
durationMs: 0,
|
||||||
})
|
})
|
||||||
);
|
);
|
||||||
} else {
|
} else {
|
||||||
settle(undefined, { stdout, stderr, exitCode: code ?? -1 });
|
settle(undefined, { stdout, stderr, exitCode: code ?? -1, client, durationMs: 0 });
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -39,6 +39,8 @@ describe("main", () => {
|
|||||||
stdout: "output",
|
stdout: "output",
|
||||||
stderr: "",
|
stderr: "",
|
||||||
exitCode: 0,
|
exitCode: 0,
|
||||||
|
client: "codex",
|
||||||
|
durationMs: 42,
|
||||||
};
|
};
|
||||||
|
|
||||||
it("returns 0 for --help and prints usage", async () => {
|
it("returns 0 for --help and prints usage", async () => {
|
||||||
@@ -152,6 +154,8 @@ describe("main", () => {
|
|||||||
stdout: "hello-out",
|
stdout: "hello-out",
|
||||||
stderr: "hello-err",
|
stderr: "hello-err",
|
||||||
exitCode: 0,
|
exitCode: 0,
|
||||||
|
client: "codex",
|
||||||
|
durationMs: 42,
|
||||||
}),
|
}),
|
||||||
stdoutWrite: (chunk) => stdoutChunks.push(chunk),
|
stdoutWrite: (chunk) => stdoutChunks.push(chunk),
|
||||||
stderrWrite: (chunk) => stderrChunks.push(chunk),
|
stderrWrite: (chunk) => stderrChunks.push(chunk),
|
||||||
@@ -259,7 +263,7 @@ describe("main", () => {
|
|||||||
return mockResult;
|
return mockResult;
|
||||||
},
|
},
|
||||||
resolveClient: () => "claude",
|
resolveClient: () => "claude",
|
||||||
resolveConfig: () => ({ paths: {} }),
|
resolveConfig: () => ({ paths: {}, timeout: 600_000 }),
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
assert.strictEqual(code, 0);
|
assert.strictEqual(code, 0);
|
||||||
@@ -286,7 +290,7 @@ describe("main", () => {
|
|||||||
return mockResult;
|
return mockResult;
|
||||||
},
|
},
|
||||||
resolveClient: () => "codex",
|
resolveClient: () => "codex",
|
||||||
resolveConfig: () => ({ paths: {} }),
|
resolveConfig: () => ({ paths: {}, timeout: 600_000 }),
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
assert.strictEqual(code, 0);
|
assert.strictEqual(code, 0);
|
||||||
@@ -310,7 +314,7 @@ describe("main", () => {
|
|||||||
return mockResult;
|
return mockResult;
|
||||||
},
|
},
|
||||||
resolveClient: (_p, cfg) => cfg?.client ?? null,
|
resolveClient: (_p, cfg) => cfg?.client ?? null,
|
||||||
resolveConfig: () => ({ paths: {}, defaultClient: "claude" }),
|
resolveConfig: () => ({ paths: {}, defaultClient: "claude", timeout: 600_000 }),
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
assert.strictEqual(code, 0);
|
assert.strictEqual(code, 0);
|
||||||
@@ -329,7 +333,7 @@ describe("main", () => {
|
|||||||
detectClients: () => mockClients,
|
detectClients: () => mockClients,
|
||||||
executePrompt: async () => mockResult,
|
executePrompt: async () => mockResult,
|
||||||
resolveClient: () => null,
|
resolveClient: () => null,
|
||||||
resolveConfig: () => ({ paths: {} }),
|
resolveConfig: () => ({ paths: {}, timeout: 600_000 }),
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
assert.strictEqual(code, 1);
|
assert.strictEqual(code, 1);
|
||||||
@@ -351,7 +355,7 @@ describe("main", () => {
|
|||||||
throw new ClientNotFoundError("claude");
|
throw new ClientNotFoundError("claude");
|
||||||
},
|
},
|
||||||
resolveClient: () => "claude",
|
resolveClient: () => "claude",
|
||||||
resolveConfig: () => ({ paths: {} }),
|
resolveConfig: () => ({ paths: {}, timeout: 600_000 }),
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
assert.strictEqual(code, 1);
|
assert.strictEqual(code, 1);
|
||||||
@@ -388,9 +392,11 @@ describe("main", () => {
|
|||||||
stdout: "done",
|
stdout: "done",
|
||||||
stderr: "",
|
stderr: "",
|
||||||
exitCode: 0,
|
exitCode: 0,
|
||||||
|
client: "codex",
|
||||||
|
durationMs: 42,
|
||||||
}),
|
}),
|
||||||
resolveClient: () => "codex",
|
resolveClient: () => "codex",
|
||||||
resolveConfig: () => ({ paths: {} }),
|
resolveConfig: () => ({ paths: {}, timeout: 600_000 }),
|
||||||
stdoutWrite: (chunk) => stdoutChunks.push(chunk),
|
stdoutWrite: (chunk) => stdoutChunks.push(chunk),
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -135,4 +135,71 @@ describe("resolveConfig", () => {
|
|||||||
});
|
});
|
||||||
assert.strictEqual(config.defaultClient, undefined);
|
assert.strictEqual(config.defaultClient, undefined);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("returns default timeout of 600000 when no sources are present", () => {
|
||||||
|
const config = resolveConfig({
|
||||||
|
existsSync: () => false,
|
||||||
|
whichSync: () => undefined,
|
||||||
|
});
|
||||||
|
assert.strictEqual(config.timeout, 600_000);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("loads timeout from file config", () => {
|
||||||
|
const config = resolveConfig({
|
||||||
|
existsSync: () => true,
|
||||||
|
readFileSync: () => JSON.stringify({ timeout: 120_000 }),
|
||||||
|
whichSync: () => undefined,
|
||||||
|
});
|
||||||
|
assert.strictEqual(config.timeout, 120_000);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("overrides file timeout with env var", () => {
|
||||||
|
const config = resolveConfig({
|
||||||
|
env: { AI_CLI_TIMEOUT: "240000" },
|
||||||
|
existsSync: () => true,
|
||||||
|
readFileSync: () => JSON.stringify({ timeout: 120_000 }),
|
||||||
|
whichSync: () => undefined,
|
||||||
|
});
|
||||||
|
assert.strictEqual(config.timeout, 240_000);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("overrides env timeout with CLI flag", () => {
|
||||||
|
const config = resolveConfig({
|
||||||
|
flags: { timeout: "480000" },
|
||||||
|
env: { AI_CLI_TIMEOUT: "240000" },
|
||||||
|
existsSync: () => true,
|
||||||
|
readFileSync: () => JSON.stringify({ timeout: 120_000 }),
|
||||||
|
whichSync: () => undefined,
|
||||||
|
});
|
||||||
|
assert.strictEqual(config.timeout, 480_000);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("respects full priority ordering for timeout: flag > env > file > default", () => {
|
||||||
|
const config = resolveConfig({
|
||||||
|
flags: { timeout: "480000" },
|
||||||
|
env: { AI_CLI_TIMEOUT: "240000" },
|
||||||
|
existsSync: () => true,
|
||||||
|
readFileSync: () => JSON.stringify({ timeout: 120_000 }),
|
||||||
|
whichSync: () => undefined,
|
||||||
|
});
|
||||||
|
assert.strictEqual(config.timeout, 480_000);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("ignores invalid timeout from env var and falls back to default", () => {
|
||||||
|
const config = resolveConfig({
|
||||||
|
env: { AI_CLI_TIMEOUT: "not-a-number" },
|
||||||
|
existsSync: () => false,
|
||||||
|
whichSync: () => undefined,
|
||||||
|
});
|
||||||
|
assert.strictEqual(config.timeout, 600_000);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("ignores invalid timeout from file and falls back to default", () => {
|
||||||
|
const config = resolveConfig({
|
||||||
|
existsSync: () => true,
|
||||||
|
readFileSync: () => JSON.stringify({ timeout: "not-a-number" }),
|
||||||
|
whichSync: () => undefined,
|
||||||
|
});
|
||||||
|
assert.strictEqual(config.timeout, 600_000);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -203,4 +203,58 @@ describe("executePrompt", () => {
|
|||||||
err instanceof ExecError && err.message.includes("Unknown client")
|
err instanceof ExecError && err.message.includes("Unknown client")
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("includes client and durationMs in result", async () => {
|
||||||
|
const scenarios = new Map<string, MockScenario>([
|
||||||
|
["codex exec --yolo hello", { stdout: "ok", exitCode: 0 }],
|
||||||
|
]);
|
||||||
|
const result = await executePrompt("codex", "hello", {
|
||||||
|
spawn: mockSpawn(scenarios),
|
||||||
|
existsSync: () => true,
|
||||||
|
});
|
||||||
|
assert.strictEqual(result.client, "codex");
|
||||||
|
assert.strictEqual(typeof result.durationMs, "number");
|
||||||
|
assert.ok(result.durationMs >= 0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects with ExecError containing custom timeout value", async () => {
|
||||||
|
const scenarios = new Map<string, MockScenario>([
|
||||||
|
["codex exec --yolo slow", { hang: true }],
|
||||||
|
]);
|
||||||
|
await assert.rejects(
|
||||||
|
executePrompt("codex", "slow", {
|
||||||
|
spawn: mockSpawn(scenarios),
|
||||||
|
existsSync: () => true,
|
||||||
|
timeoutMs: 50,
|
||||||
|
}),
|
||||||
|
(err: unknown) =>
|
||||||
|
err instanceof ExecError &&
|
||||||
|
err.message === "Execution timed out after 50ms" &&
|
||||||
|
err.result.exitCode === -1 &&
|
||||||
|
err.result.client === "codex" &&
|
||||||
|
typeof err.result.durationMs === "number"
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("uses default timeout of 600000 when timeoutMs is not provided", async () => {
|
||||||
|
const delays: number[] = [];
|
||||||
|
const origSetTimeout = global.setTimeout;
|
||||||
|
(global as any).setTimeout = function(callback: any, delay: number) {
|
||||||
|
delays.push(delay);
|
||||||
|
return origSetTimeout(callback, delay);
|
||||||
|
};
|
||||||
|
|
||||||
|
const scenarios = new Map<string, MockScenario>([
|
||||||
|
["codex exec --yolo hello", { stdout: "ok", exitCode: 0 }],
|
||||||
|
]);
|
||||||
|
try {
|
||||||
|
await executePrompt("codex", "hello", {
|
||||||
|
spawn: mockSpawn(scenarios),
|
||||||
|
existsSync: () => true,
|
||||||
|
});
|
||||||
|
assert.strictEqual(delays[0], 600_000);
|
||||||
|
} finally {
|
||||||
|
global.setTimeout = origSetTimeout;
|
||||||
|
}
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user