-
Notifications
You must be signed in to change notification settings - Fork 1
feat(cli): add NDJSON invocation lifecycle events #96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
0b2d924
0c8a310
9d64f47
2835d9a
6ece1b9
42424fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| import { Stdout } from "../stdout" | ||
| import { SCHEMA_VERSION } from "./run.errors" | ||
|
|
||
| export type RunInvocationPhase = "validation" | "stdin" | "bootstrap" | "session" | ||
|
|
||
| export function createRunInvocation(enabled: boolean) { | ||
| const id = enabled ? crypto.randomUUID() : undefined | ||
| const started = Date.now() | ||
| let phase: RunInvocationPhase = "validation" | ||
| let sessionID: string | undefined | ||
| let failed = false | ||
| let completed = false | ||
| let writes = Promise.resolve() | ||
|
|
||
| function emit(type: string, data: Record<string, unknown> = {}) { | ||
| if (!id) return | ||
| writes = writes.then(() => | ||
| Stdout.json({ | ||
| type, | ||
| timestamp: Date.now(), | ||
| schemaVersion: SCHEMA_VERSION, | ||
| invocationID: id, | ||
| ...data, | ||
| }), | ||
| ) | ||
| } | ||
|
|
||
| emit("invocation_start") | ||
|
|
||
| function error(_error: unknown, code = `INVOCATION_${phase.toUpperCase()}_FAILED`) { | ||
| if (!id || failed || completed) return | ||
| failed = true | ||
| if (sessionID) return | ||
| emit("invocation_error", { | ||
| phase, | ||
| code, | ||
| message: `Invocation failed during ${phase}`, | ||
| }) | ||
| } | ||
|
|
||
| function complete() { | ||
| if (!id || completed) return | ||
| completed = true | ||
| emit("invocation_complete", { | ||
| status: failed ? "error" : "completed", | ||
| durationMs: Date.now() - started, | ||
| ...(sessionID ? { sessionID } : {}), | ||
| }) | ||
| } | ||
|
|
||
| async function abort(cause: unknown, code?: string) { | ||
| error(cause, code) | ||
| complete() | ||
| await writes | ||
| } | ||
|
|
||
| return { | ||
| id, | ||
| phase(next: RunInvocationPhase) { | ||
| phase = next | ||
| }, | ||
| link(session: string) { | ||
| sessionID = session | ||
| }, | ||
| error, | ||
| abort, | ||
| async guard<T>(task: () => T | Promise<T>) { | ||
| try { | ||
| return await task() | ||
| } catch (cause) { | ||
| await abort(cause) | ||
| throw cause | ||
| } | ||
| }, | ||
| async run<T>(task: Promise<T> | (() => T | Promise<T>)) { | ||
| try { | ||
| return await (typeof task === "function" ? task() : task) | ||
| } catch (cause) { | ||
| error(cause) | ||
| throw cause | ||
| } finally { | ||
| await writes | ||
| complete() | ||
| await writes | ||
| } | ||
| }, | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -38,6 +38,7 @@ import { BashTool } from "../../tool/bash" | |||||
| import { TodoWriteTool } from "../../tool/todo" | ||||||
| import { Locale } from "../../util/locale" | ||||||
| import { Stdout } from "../stdout" | ||||||
| import { createRunInvocation } from "./run.invocation" | ||||||
|
|
||||||
| type ToolProps<T extends Tool.Info> = { | ||||||
| input: Tool.InferParameters<T> | ||||||
|
|
@@ -375,19 +376,26 @@ export const RunCommand = cmd({ | |||||
| }) | ||||||
| }, | ||||||
| handler: async (args) => { | ||||||
| const invocation = createRunInvocation(args.format === "json") | ||||||
|
|
||||||
| async function fail(message: string, code: string) { | ||||||
| UI.error(message) | ||||||
| await invocation.abort(message, code) | ||||||
| process.exit(1) | ||||||
| } | ||||||
|
|
||||||
| let message = [...args.message, ...(args["--"] || [])] | ||||||
| .map((arg) => (arg.includes(" ") ? `"${arg.replace(/"/g, '\\"')}"` : arg)) | ||||||
| .join(" ") | ||||||
|
|
||||||
| const directory = (() => { | ||||||
| const directory = await (async () => { | ||||||
| if (!args.dir) return undefined | ||||||
| if (args.attach) return args.dir | ||||||
| try { | ||||||
| process.chdir(args.dir) | ||||||
| return process.cwd() | ||||||
| } catch { | ||||||
| UI.error("Failed to change directory to " + args.dir) | ||||||
| process.exit(1) | ||||||
| await fail("Failed to change directory to " + args.dir, "INVOCATION_INVALID_DIRECTORY") | ||||||
| } | ||||||
| })() | ||||||
|
|
||||||
|
|
@@ -397,12 +405,13 @@ export const RunCommand = cmd({ | |||||
|
|
||||||
| for (const filePath of list) { | ||||||
| const resolvedPath = path.resolve(process.cwd(), filePath) | ||||||
| if (!(await Filesystem.exists(resolvedPath))) { | ||||||
| UI.error(`File not found: ${filePath}`) | ||||||
| process.exit(1) | ||||||
| if (!(await invocation.guard(() => Filesystem.exists(resolvedPath)))) { | ||||||
| await fail(`File not found: ${filePath}`, "INVOCATION_FILE_NOT_FOUND") | ||||||
| } | ||||||
|
|
||||||
| const mime = (await Filesystem.isDir(resolvedPath)) ? "application/x-directory" : "text/plain" | ||||||
| const mime = (await invocation.guard(() => Filesystem.isDir(resolvedPath))) | ||||||
| ? "application/x-directory" | ||||||
| : "text/plain" | ||||||
|
|
||||||
| files.push({ | ||||||
| type: "file", | ||||||
|
|
@@ -413,16 +422,18 @@ export const RunCommand = cmd({ | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| if (!process.stdin.isTTY) message += "\n" + (await Bun.stdin.text()) | ||||||
| if (!process.stdin.isTTY) { | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 Pre-run throw skips invocation_complete event. 🤖 Fix with your agentWhy this mattersThe handler body between }
}
if (!process.stdin.isTTY) {
invocation.phase("stdin")
message += "\n" + (await Bun.stdin.text())
invocation.phase("validation")
} |
||||||
| invocation.phase("stdin") | ||||||
| message += "\n" + (await invocation.guard(() => Bun.stdin.text())) | ||||||
| invocation.phase("validation") | ||||||
| } | ||||||
|
|
||||||
| if (message.trim().length === 0 && !args.command) { | ||||||
| UI.error("You must provide a message or a command") | ||||||
| process.exit(1) | ||||||
| await fail("You must provide a message or a command", "INVOCATION_EMPTY_INPUT") | ||||||
| } | ||||||
|
|
||||||
| if (args.fork && !args.continue && !args.session) { | ||||||
| UI.error("--fork requires --continue or --session") | ||||||
| process.exit(1) | ||||||
| await fail("--fork requires --continue or --session", "INVOCATION_INVALID_ARGUMENTS") | ||||||
| } | ||||||
|
|
||||||
| const rules: PermissionNext.Ruleset = [ | ||||||
|
|
@@ -508,7 +519,14 @@ export const RunCommand = cmd({ | |||||
|
|
||||||
| function emit(type: string, data: Record<string, unknown>) { | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Local emit() duplicates NDJSON writer.
Suggested change
🤖 Fix with your agentWhy this mattersThe local function emit(type: string, data: Record<string, unknown>) {
if (args.format === "json") {
Stdout.write(
JSON.stringify({ type, timestamp: Date.now(), invocationID: invocation.id, sessionID, ...data }) + EOL,
)
return true
}
return false |
||||||
| if (args.format === "json") { | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Two emit() helpers diverge on schemaVersion handling. 🤖 Fix with your agentWhy this mattersThere are now two sibling function emit(type: string, data: Record<string, unknown>) {
if (args.format === "json") {
process.stdout.write(
JSON.stringify({ type, timestamp: Date.now(), invocationID: Invocation.id, sessionID, ...data }) + EOL,
)
return true
}
return false |
||||||
| Stdout.write(JSON.stringify({ type, timestamp: Date.now(), sessionID, ...data }) + EOL) | ||||||
| Stdout.json({ | ||||||
| type, | ||||||
| timestamp: Date.now(), | ||||||
| schemaVersion: SCHEMA_VERSION, | ||||||
| invocationID: invocation.id, | ||||||
| sessionID, | ||||||
| ...data, | ||||||
| }) | ||||||
| return true | ||||||
| } | ||||||
| return false | ||||||
|
|
@@ -678,6 +696,7 @@ export const RunCommand = cmd({ | |||||
| // spuriously-green job. process.exitCode (not process.exit) lets the | ||||||
| // loop drain to session.status idle and emit session_complete first. | ||||||
| process.exitCode = 1 | ||||||
| invocation.error(props.error) | ||||||
| if (!sessionErrorEmitted) { | ||||||
| sessionErrorEmitted = true | ||||||
| const classified = classifySessionError(props.error) | ||||||
|
|
@@ -811,11 +830,12 @@ export const RunCommand = cmd({ | |||||
| })() | ||||||
| const agent = agentInfo.name | ||||||
|
|
||||||
| invocation.phase("session") | ||||||
| const sessionID = await session(sdk) | ||||||
| if (!sessionID) { | ||||||
| UI.error("Session not found") | ||||||
| process.exit(1) | ||||||
| await fail("Session not found", "INVOCATION_SESSION_CREATE_FAILED") | ||||||
| } | ||||||
| invocation.link(sessionID) | ||||||
| await share(sdk, sessionID) | ||||||
|
|
||||||
| emit("session_start", { | ||||||
|
|
@@ -876,6 +896,7 @@ export const RunCommand = cmd({ | |||||
| }) | ||||||
| console.error(e) | ||||||
| process.exitCode = 1 | ||||||
| invocation.error(e) | ||||||
| }) | ||||||
|
|
||||||
| if (args.command) { | ||||||
|
|
@@ -926,17 +947,18 @@ export const RunCommand = cmd({ | |||||
| }) | ||||||
| console.error(e) | ||||||
| process.exitCode = 1 | ||||||
| invocation.error(e) | ||||||
| }, | ||||||
| ), | ||||||
| ]) | ||||||
| } | ||||||
|
|
||||||
| invocation.phase("bootstrap") | ||||||
| if (args.attach) { | ||||||
| const sdk = createAictrlClient({ baseUrl: args.attach, directory }) | ||||||
| return await execute(sdk) | ||||||
| return await invocation.run(() => execute(createAictrlClient({ baseUrl: args.attach, directory }))) | ||||||
| } | ||||||
|
|
||||||
| await bootstrap(process.cwd(), async () => { | ||||||
| const execution = bootstrap(process.cwd(), async () => { | ||||||
| const sdk = { | ||||||
| session: { | ||||||
| async list() { | ||||||
|
|
@@ -1016,5 +1038,6 @@ export const RunCommand = cmd({ | |||||
| } | ||||||
| await execute(sdk) | ||||||
| }) | ||||||
| await invocation.run(execution) | ||||||
| }, | ||||||
| }) | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Error code set undocumented in EVENTS.md.
🤖 Fix with your agent
Why this matters
EVENTS.md describes
invocation_error.codeonly as a "stable machine-readable error category" with a single example (INVOCATION_FILE_NOT_FOUND). But run.ts passes a fixed set of codes (INVOCATION_INVALID_DIRECTORY,INVOCATION_FILE_NOT_FOUND,INVOCATION_EMPTY_INPUT,INVOCATION_INVALID_ARGUMENTS,INVOCATION_SESSION_CREATE_FAILED) and run.invocation.ts:31 synthesizes a defaultINVOCATION_${phase.toUpperCase()}_FAILED(e.g.INVOCATION_BOOTSTRAP_FAILED,INVOCATION_SESSION_FAILED,INVOCATION_STDIN_FAILED) for any uncaught error. None of these are enumerated in the doc and the default-code pattern is entirely undocumented, so consumers pinning to "stable" codes cannot discover the full set or rely on the fallback shape.