From 9510f666c0aeefbc7bb70a9be5fa2e8217a5e707 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Tue, 3 Mar 2026 04:22:10 -0800 Subject: [PATCH 1/4] mcp|feat: Emit run_failed event when complete_run receives failed status Route event emission by status in completeRun: when status is 'failed', emit a run_failed event (with optional reason field) instead of run_completed. For 'completed' and 'needs_manual_review', behavior is unchanged. Add reason parameter to the MCP tool input schema and test coverage for all three paths. --- packages/mcp/src/server.ts | 4 ++- .../src/tools/__tests__/complete-run.test.ts | 27 +++++++++++++++++-- packages/mcp/src/tools/complete-run.ts | 21 +++++++++++---- 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/packages/mcp/src/server.ts b/packages/mcp/src/server.ts index 6342e5f7..e226ae14 100644 --- a/packages/mcp/src/server.ts +++ b/packages/mcp/src/server.ts @@ -91,10 +91,12 @@ export function createServer(): McpServer { server.registerTool( 'complete_run', { - description: 'Complete a run: emit run_completed event and stamp completedAt on run-index.json.', + description: + 'Complete a run: emit run_completed (or run_failed when status is failed) event and stamp completedAt on run-index.json.', inputSchema: { runDir: z.string(), status: z.enum(['completed', 'failed', 'needs_manual_review']), + reason: z.string().optional(), }, }, async (args) => { diff --git a/packages/mcp/src/tools/__tests__/complete-run.test.ts b/packages/mcp/src/tools/__tests__/complete-run.test.ts index 02ffdd35..1bdb6059 100644 --- a/packages/mcp/src/tools/__tests__/complete-run.test.ts +++ b/packages/mcp/src/tools/__tests__/complete-run.test.ts @@ -54,7 +54,7 @@ describe('completeRun', () => { expect(result.error).toContain('must be one of'); }); - it('accepts failed status', async () => { + it('emits run_failed event when status is failed', async () => { const runDir = await createRunDir(); const result = await completeRun({ runDir, status: 'failed' }); @@ -62,7 +62,30 @@ describe('completeRun', () => { const content = await readFile(join(runDir, 'run-log.jsonl'), 'utf8'); const event: unknown = JSON.parse(content.trim()); - expect(event).toMatchObject({ status: 'failed' }); + expect(event).toMatchObject({ event: 'run_failed', status: 'failed' }); + }); + + it('includes reason in run_failed event', async () => { + const runDir = await createRunDir(); + const result = await completeRun({ runDir, status: 'failed', reason: 'TypeScript compilation errors' }); + + expect(result.success).toBe(true); + + const content = await readFile(join(runDir, 'run-log.jsonl'), 'utf8'); + const event: unknown = JSON.parse(content.trim()); + expect(event).toMatchObject({ event: 'run_failed', status: 'failed', reason: 'TypeScript compilation errors' }); + }); + + it('ignores reason when status is not failed', async () => { + const runDir = await createRunDir(); + const result = await completeRun({ runDir, status: 'completed', reason: 'should be ignored' }); + + expect(result.success).toBe(true); + + const content = await readFile(join(runDir, 'run-log.jsonl'), 'utf8'); + const event: unknown = JSON.parse(content.trim()); + expect(event).toMatchObject({ event: 'run_completed', status: 'completed' }); + expect(event).not.toHaveProperty('reason'); }); it('accepts needs_manual_review status', async () => { diff --git a/packages/mcp/src/tools/complete-run.ts b/packages/mcp/src/tools/complete-run.ts index 828040a0..b40ead8d 100644 --- a/packages/mcp/src/tools/complete-run.ts +++ b/packages/mcp/src/tools/complete-run.ts @@ -13,6 +13,7 @@ export type CompletionStatus = z.infer; export interface CompleteRunInput { runDir: string; status: string; + reason?: string | undefined; } export interface CompleteRunResult { @@ -21,11 +22,16 @@ export interface CompleteRunResult { } /** - * Complete a run: emit a `run_completed` event and stamp `completedAt` on the - * run-index.json header for fast discovery without reading the JSONL log. + * Complete a run: emit a `run_completed` or `run_failed` event (based on the + * status) and stamp `completedAt` on the run-index.json header for fast + * discovery without reading the JSONL log. + * + * When `status` is `'failed'`, a `run_failed` event is emitted instead of + * `run_completed`. The optional `reason` field is included in the `run_failed` + * event; it is ignored for other statuses. */ export async function completeRun(input: CompleteRunInput): Promise { - const { runDir, status } = input; + const { runDir, status, reason } = input; const statusResult = completionStatusSchema.safeParse(status); if (!statusResult.success) { @@ -40,10 +46,15 @@ export async function completeRun(input: CompleteRunInput): Promise Date: Tue, 3 Mar 2026 04:25:36 -0800 Subject: [PATCH 2/4] mcp|tests: Strengthen complete-run test assertions Assert absence of reason property when run_failed is emitted without a reason, and verify event type for needs_manual_review status. --- packages/mcp/src/tools/__tests__/complete-run.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/mcp/src/tools/__tests__/complete-run.test.ts b/packages/mcp/src/tools/__tests__/complete-run.test.ts index 1bdb6059..50372229 100644 --- a/packages/mcp/src/tools/__tests__/complete-run.test.ts +++ b/packages/mcp/src/tools/__tests__/complete-run.test.ts @@ -63,6 +63,7 @@ describe('completeRun', () => { const content = await readFile(join(runDir, 'run-log.jsonl'), 'utf8'); const event: unknown = JSON.parse(content.trim()); expect(event).toMatchObject({ event: 'run_failed', status: 'failed' }); + expect(event).not.toHaveProperty('reason'); }); it('includes reason in run_failed event', async () => { @@ -96,7 +97,7 @@ describe('completeRun', () => { const content = await readFile(join(runDir, 'run-log.jsonl'), 'utf8'); const event: unknown = JSON.parse(content.trim()); - expect(event).toMatchObject({ status: 'needs_manual_review' }); + expect(event).toMatchObject({ event: 'run_completed', status: 'needs_manual_review' }); }); it('rejects when run-index.json is missing', async () => { From fcdf37e34d6a137de210ad02868d32b5ae258a32 Mon Sep 17 00:00:00 2001 From: William Thorsen Date: Tue, 3 Mar 2026 04:31:40 -0800 Subject: [PATCH 3/4] mcp|refactor: Simplify conditional reason property in run_failed event Replace the conditional spread pattern with a direct property assignment. JSON.stringify omits undefined values, and the Zod schema strips unknown keys during parse, so the behavior is identical. --- packages/mcp/src/tools/complete-run.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mcp/src/tools/complete-run.ts b/packages/mcp/src/tools/complete-run.ts index b40ead8d..38b2a9e1 100644 --- a/packages/mcp/src/tools/complete-run.ts +++ b/packages/mcp/src/tools/complete-run.ts @@ -49,7 +49,7 @@ export async function completeRun(input: CompleteRunInput): Promise Date: Tue, 3 Mar 2026 04:34:56 -0800 Subject: [PATCH 4/4] agents|docs: Update complete_run documentation to reflect run_failed event The orchestrate skill documentation described complete_run as always emitting run_completed. Updated to document that failed status now emits run_failed, and that the optional reason parameter is available for failures. --- packages/agents/content/skills/orchestrate/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/agents/content/skills/orchestrate/SKILL.md b/packages/agents/content/skills/orchestrate/SKILL.md index 7330034e..a6f266ee 100644 --- a/packages/agents/content/skills/orchestrate/SKILL.md +++ b/packages/agents/content/skills/orchestrate/SKILL.md @@ -448,7 +448,7 @@ Include: After writing the artifact, call `register_artifact` for the run-summary artifact. Present the same summary to the user in the conversation. The conversational output should match the artifact content — do not abbreviate or omit sections. -Call MCP tool `complete_run` with `{ runDir: {run-dir}, status: "completed" | "failed" | "needs_manual_review" }`. This emits a `run_completed` event and stamps `completedAt` on the run-index.json header. The `status` field within the event carries the actual outcome (`completed`, `failed`, or `needs_manual_review`). +Call MCP tool `complete_run` with `{ runDir: {run-dir}, status: "completed" | "failed" | "needs_manual_review", reason?: string }`. When `status` is `"failed"`, this emits a `run_failed` event (the optional `reason` field is included if provided); otherwise it emits a `run_completed` event. Either way, `completedAt` is stamped on the run-index.json header. ## Phase 6: Wrap-up (prompted, conditional)