Skip to content

Commit ee1e10f

Browse files
committed
fix: catch "does not exist" errors in GET /runs/:runId/progress handler
The PlaybookRegistryService.getPlaybook() throws "does not exist" when a playbook ID isn't found, but the route handler only caught "not found" errors — causing unhandled exceptions to fall through to the generic 500 handler. Added "does not exist" to the 404 error check.
1 parent 12d87a2 commit ee1e10f

File tree

2 files changed

+177
-1
lines changed

2 files changed

+177
-1
lines changed

packages/sidecar/src/server/routes/runs.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,10 @@ export function createRunRoutes(runtime: SidecarRuntime): Hono {
9292
return context.json(progress);
9393
} catch (error) {
9494
if (error instanceof Error) {
95-
if (error.message.includes("not found")) {
95+
if (
96+
error.message.includes("not found") ||
97+
error.message.includes("does not exist")
98+
) {
9699
return context.json({ error: error.message }, 404);
97100
}
98101
if (error.message.includes("not associated with a playbook")) {
Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
import { describe, expect, it, vi } from "vitest";
2+
import { Hono } from "hono";
3+
import { createRunRoutes } from "../../packages/sidecar/src/server/routes/runs.ts";
4+
import type { SidecarRuntime } from "../../packages/sidecar/src/server/context.ts";
5+
6+
const MOCK_PROGRESS = {
7+
runId: "run-abc12345",
8+
playbookId: "launch-pack",
9+
playbookTitle: "Launch Pack",
10+
currentPhase: "Research",
11+
runStatus: "running",
12+
phases: [
13+
{
14+
name: "Research",
15+
description: "Identify best-fit launch surfaces.",
16+
status: "current",
17+
specialistId: "distribution",
18+
expectedArtifacts: ["community shortlist", "launch timing plan"],
19+
matchedArtifacts: [],
20+
missingArtifacts: ["community shortlist", "launch timing plan"],
21+
},
22+
{
23+
name: "Draft",
24+
description: "Write Product Hunt copy.",
25+
status: "upcoming",
26+
specialistId: "distribution",
27+
expectedArtifacts: ["Product Hunt copy"],
28+
matchedArtifacts: [],
29+
missingArtifacts: ["Product Hunt copy"],
30+
},
31+
],
32+
};
33+
34+
function createMockRuntime(overrides: Record<string, unknown> = {}) {
35+
const runtime = {
36+
opengoatPaths: { homeDir: "/tmp/test" },
37+
runService: {
38+
listRuns: vi.fn().mockResolvedValue({ runs: [], total: 0, limit: 50, offset: 0 }),
39+
getRun: vi.fn().mockResolvedValue({
40+
runId: "run-abc12345",
41+
projectId: "proj-1",
42+
objectiveId: "obj-1",
43+
playbookId: "launch-pack",
44+
title: "Launch Pack",
45+
status: "running",
46+
phase: "Research",
47+
phaseSummary: "Identify best-fit launch surfaces.",
48+
startedFrom: "action",
49+
agentId: "goat",
50+
createdAt: "2025-01-01T00:00:00.000Z",
51+
updatedAt: "2025-01-01T00:00:00.000Z",
52+
}),
53+
createRun: vi.fn(),
54+
updateRunStatus: vi.fn(),
55+
advancePhase: vi.fn(),
56+
},
57+
playbookExecutionService: {
58+
getRunProgress: vi.fn().mockResolvedValue(MOCK_PROGRESS),
59+
},
60+
...overrides,
61+
} as unknown as SidecarRuntime;
62+
63+
return runtime;
64+
}
65+
66+
function buildApp(runtime: SidecarRuntime) {
67+
const app = new Hono();
68+
app.route("/runs", createRunRoutes(runtime));
69+
app.onError((error, c) => c.json({ error: "Internal Server Error", detail: error.message }, 500));
70+
return app;
71+
}
72+
73+
describe("GET /runs/:runId/progress", () => {
74+
it("returns 200 with phase breakdown for valid run", async () => {
75+
const runtime = createMockRuntime();
76+
const app = buildApp(runtime);
77+
78+
const res = await app.request("/runs/run-abc12345/progress");
79+
expect(res.status).toBe(200);
80+
81+
const data = await res.json();
82+
expect(data).toEqual(MOCK_PROGRESS);
83+
});
84+
85+
it("response includes currentPhase, runStatus, playbookTitle, and phases", async () => {
86+
const runtime = createMockRuntime();
87+
const app = buildApp(runtime);
88+
89+
const res = await app.request("/runs/run-abc12345/progress");
90+
const data = await res.json() as typeof MOCK_PROGRESS;
91+
92+
expect(data.currentPhase).toBe("Research");
93+
expect(data.runStatus).toBe("running");
94+
expect(data.playbookTitle).toBe("Launch Pack");
95+
expect(data.playbookId).toBe("launch-pack");
96+
expect(data.phases).toHaveLength(2);
97+
});
98+
99+
it("phase objects include status, expectedArtifacts, matchedArtifacts, missingArtifacts", async () => {
100+
const runtime = createMockRuntime();
101+
const app = buildApp(runtime);
102+
103+
const res = await app.request("/runs/run-abc12345/progress");
104+
const data = await res.json() as typeof MOCK_PROGRESS;
105+
106+
const phase = data.phases[0];
107+
expect(phase.name).toBe("Research");
108+
expect(phase.status).toBe("current");
109+
expect(phase.expectedArtifacts).toEqual(["community shortlist", "launch timing plan"]);
110+
expect(phase.matchedArtifacts).toEqual([]);
111+
expect(phase.missingArtifacts).toEqual(["community shortlist", "launch timing plan"]);
112+
});
113+
114+
it("returns 404 for non-existent run", async () => {
115+
const runtime = createMockRuntime({
116+
playbookExecutionService: {
117+
getRunProgress: vi.fn().mockRejectedValue(new Error("Run not found")),
118+
},
119+
});
120+
const app = buildApp(runtime);
121+
122+
const res = await app.request("/runs/run-missing/progress");
123+
expect(res.status).toBe(404);
124+
125+
const data = await res.json() as { error: string };
126+
expect(data.error).toContain("not found");
127+
});
128+
129+
it("returns 400 for run not associated with a playbook", async () => {
130+
const runtime = createMockRuntime({
131+
playbookExecutionService: {
132+
getRunProgress: vi
133+
.fn()
134+
.mockRejectedValue(new Error("Run is not associated with a playbook")),
135+
},
136+
});
137+
const app = buildApp(runtime);
138+
139+
const res = await app.request("/runs/run-no-playbook/progress");
140+
expect(res.status).toBe(400);
141+
142+
const data = await res.json() as { error: string };
143+
expect(data.error).toContain("not associated with a playbook");
144+
});
145+
146+
it("returns 404 when playbook does not exist in registry", async () => {
147+
const runtime = createMockRuntime({
148+
playbookExecutionService: {
149+
getRunProgress: vi
150+
.fn()
151+
.mockRejectedValue(new Error('Playbook "unknown-id" does not exist.')),
152+
},
153+
});
154+
const app = buildApp(runtime);
155+
156+
const res = await app.request("/runs/run-bad-playbook/progress");
157+
expect(res.status).toBe(404);
158+
159+
const data = await res.json() as { error: string };
160+
expect(data.error).toContain("does not exist");
161+
});
162+
163+
it("does not interfere with GET /:runId route", async () => {
164+
const runtime = createMockRuntime();
165+
const app = buildApp(runtime);
166+
167+
const res = await app.request("/runs/run-abc12345");
168+
expect(res.status).toBe(200);
169+
170+
const data = await res.json() as { runId: string };
171+
expect(data.runId).toBe("run-abc12345");
172+
});
173+
});

0 commit comments

Comments
 (0)