From d35fd77bd7d45f1960b09af7887a08be838ed3d3 Mon Sep 17 00:00:00 2001 From: Dylan Boudro <121908331+starmorph@users.noreply.github.com> Date: Tue, 27 Oct 2026 10:16:44 -0700 Subject: [PATCH] Return safe workflow errors with run correlation --- docs/0_QUICKSTART.md | 19 ++++++++---- docs/1_EXTENSION.md | 12 +++++++- src/api.ts | 5 ++-- src/ask.ts | 4 +-- test/api.test.ts | 29 +++++++++++++++++-- test/ask.test.ts | 69 ++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 126 insertions(+), 12 deletions(-) create mode 100644 test/ask.test.ts diff --git a/docs/0_QUICKSTART.md b/docs/0_QUICKSTART.md index 6a25ec0..75599b1 100644 --- a/docs/0_QUICKSTART.md +++ b/docs/0_QUICKSTART.md @@ -110,13 +110,19 @@ curl "$API_URL/v1/questions/trn-abc123def456.Kx9mZ7..." \ -H "Authorization: Bearer $API_TOKEN" ``` -While the workflow is active, the status is `running`. When it finishes, -`reply.kind` is: +While the workflow is active, the status is `running`. A completed workflow +returns `status: "completed"`, with `reply.kind` set to: - `answer`: a structured answer with commit-pinned, validated evidence. - `clarification`: a focused question to answer before the agent continues. - `failure`: a user-safe explanation without internal errors or secrets. +If the parent workflow fails or is canceled, polling returns HTTP `200` with +`status: "failed"`, the generic error `The investigation could not be completed.`, +and a `trace_id` identifying the workflow run. The CLI displays that safe +message and trace ID and exits with status `1`. Give the trace ID to an +operator to find the raw diagnostic in the Render Dashboard. + The full request, polling, callback, and idempotency contracts are in [Extend the question API](1_EXTENSION.md#extend-the-question-api). @@ -215,9 +221,9 @@ All services emit one JSON object per line. The most useful events are: | `deliver_failed` | Workflow | Callback retries were exhausted | | `snapshot_refresh` | Cron | Snapshot build result, repository count, and duration | -Every reply includes the workflow run's trace ID. Use it to find the task run -in the Dashboard. Alert on consecutive `snapshot_refresh` events with -`"ok": false`; a failed refresh leaves the previous snapshot available. +Every reply and workflow-level failure response includes a trace ID. Use it +to find the task run in the Dashboard. Alert on consecutive `snapshot_refresh` +events with `"ok": false`; a failed refresh leaves the previous snapshot available. ### After a change @@ -230,6 +236,9 @@ in the Dashboard. Alert on consecutive `snapshot_refresh` events with ### Troubleshooting +- **Workflow run failed or was canceled:** use the polling response's + `trace_id` (shown by the CLI as `Trace ...`) to open that run in the Render + Dashboard. Public responses omit the underlying task diagnostic. - **No snapshot / snapshot not ready:** trigger the snapshot cron and inspect its `snapshot_refresh` event. Common causes are missing Git credentials, the wrong workspace ID, or a failed `verify.sh` check. diff --git a/docs/1_EXTENSION.md b/docs/1_EXTENSION.md index 3d3419d..d9009be 100644 --- a/docs/1_EXTENSION.md +++ b/docs/1_EXTENSION.md @@ -238,7 +238,17 @@ type Reply = For a clarification, show `result.question` to the asker and send their answer as a new question with `follow_up_to` set to this ID. Failure reasons are safe to display. A workflow-level `failed` status means the parent task -itself failed or was canceled. +itself failed or was canceled. This polling response still uses HTTP `200`: + +```json +{"id":"","status":"failed","error":"The investigation could not be completed.","trace_id":""} +``` + +The generic `error` is safe to display. Give an operator the `trace_id` to +find the verified workflow run in the Render Dashboard, where the raw task +diagnostic remains. The CLI displays the safe error and trace ID and exits +with status `1`. A completed workflow whose `reply.kind` is `failure` still +returns `status: "completed"` with its safe failure reply. Question IDs are `.`, signed with `API_TOKEN`. Rotating the token invalidates outstanding IDs. The signature also prevents this API from diff --git a/src/api.ts b/src/api.ts index 3804a14..7a50baf 100644 --- a/src/api.ts +++ b/src/api.ts @@ -138,8 +138,9 @@ export function createApp(deps: AppDeps) { if (result) { return c.json({ id, status: "completed", trace_id: result.trace_id, reply: result.reply }); } - // failed or canceled - return c.json({ id, status: "failed", error: run.error ?? "task failed" }); + // readRun verified the signed ID before its public run-ID prefix is used for correlation. + const traceId = id.slice(0, id.lastIndexOf(".")); + return c.json({ id, status: "failed", error: "The investigation could not be completed.", trace_id: traceId }); }); return app; diff --git a/src/ask.ts b/src/ask.ts index 9d2c780..9e664d6 100644 --- a/src/ask.ts +++ b/src/ask.ts @@ -10,7 +10,7 @@ example to answer a clarification. --json prints the raw reply.`; const POLL_MS = 5_000; -type Poll = { status: "running" } | { status: "completed"; reply: Reply } | { status: "failed"; error: string }; +type Poll = { status: "running" } | { status: "completed"; reply: Reply } | { status: "failed"; error: string; trace_id: string }; const { values, positionals } = parseArgs({ allowPositionals: true, @@ -53,7 +53,7 @@ while ((poll = await api(`/v1/questions/${encodeURIComponent(id)}`)).statu process.stderr.write("\n"); if (poll.status === "failed") { - console.error(`The workflow run failed: ${poll.error}`); + console.error(`The workflow run failed: ${poll.error} Trace ${poll.trace_id}.`); process.exit(1); } if (values.json) { diff --git a/test/api.test.ts b/test/api.test.ts index 54b2ed4..5b42e71 100644 --- a/test/api.test.ts +++ b/test/api.test.ts @@ -144,8 +144,10 @@ describe("HTTP API", () => { [{ status: "running", results: [] }, { id, status: "running" }], [{ status: "paused", results: [] }, { id, status: "running" }], [{ status: "succeeded", results: [{ trace_id: "t1", reply }] }, { id, status: "completed", trace_id: "t1", reply }], - [{ status: "failed", results: [], error: "timeout" }, { id, status: "failed", error: "timeout" }], - [{ status: "canceled", results: [] }, { id, status: "failed", error: "task failed" }], + [{ status: "completed", results: [{ trace_id: "t1", reply }] }, { id, status: "completed", trace_id: "t1", reply }], + [{ status: "stopped", results: [] }, { id, status: "failed", error: "The investigation could not be completed.", trace_id: "run-1" }], + [{ status: "failed", results: [], error: "timeout" }, { id, status: "failed", error: "The investigation could not be completed.", trace_id: "run-1" }], + [{ status: "canceled", results: [] }, { id, status: "failed", error: "The investigation could not be completed.", trace_id: "run-1" }], ]; for (const [run, expected] of cases) { const res = await withRun(run).get(id); @@ -179,6 +181,29 @@ describe("HTTP API", () => { expect(logs).toContainEqual(expect.objectContaining({ level: "error", event: "request_failed", method: "POST", error: "boom" })); expect(logs).toContainEqual(expect.objectContaining({ level: "error", event: "request_failed", method: "GET" })); }); + + it.each(["failed", "canceled"])("keeps %s workflow diagnostics private and returns the verified run correlation", async (status) => { + const marker = "SYNTHETIC_TASK_DIAGNOSTIC_NOT_FOR_CALLERS"; + const runId = "run-safe-failure"; + const { get, logs } = harness({ getTaskRun: async () => ({ + status, error: marker, input: [{ internal: marker }], results: [{ trace_id: "untrusted-result-trace", internal: marker }], + }) }); + const res = await get(qid(runId)); + expect(res.status).toBe(200); + const body = await res.text(); + expect(body).not.toContain(marker); + expect(JSON.parse(body)).toEqual({ + id: qid(runId), status: "failed", error: "The investigation could not be completed.", trace_id: runId, + }); + expect(JSON.stringify(logs)).not.toContain(marker); + }); + + it.each(["succeeded", "completed"])("preserves the %s response when no result was recorded", async (status) => { + const id = qid("run-empty-result"); + const res = await withRun({ status, results: [] }).get(id); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ id, status: "completed" }); + }); }); describe("signCallback / verifyCallback", () => { diff --git a/test/ask.test.ts b/test/ask.test.ts new file mode 100644 index 0000000..737a943 --- /dev/null +++ b/test/ask.test.ts @@ -0,0 +1,69 @@ +import { execFile } from "node:child_process"; +import { once } from "node:events"; +import { fileURLToPath } from "node:url"; +import { serve } from "@hono/node-server"; +import { describe, expect, it } from "vitest"; +import { createApp, type TaskRun } from "../src/api.ts"; +import { createLog } from "../src/log.ts"; + +const API_TOKEN = "synthetic-cli-api-token"; +const RUN_ID = "run-cli-safe-failure"; +const MARKER = "SYNTHETIC_TASK_DIAGNOSTIC_NOT_FOR_CALLERS"; + +function ask(apiUrl: string) { + return new Promise<{ code: number | null; stdout: string; stderr: string }>((resolve) => { + execFile(process.execPath, ["--import", "tsx", fileURLToPath(new URL("../src/ask.ts", import.meta.url)), "What failed?"], { + cwd: fileURLToPath(new URL("../", import.meta.url)), + env: { PATH: process.env.PATH, API_URL: apiUrl, API_TOKEN }, + encoding: "utf8", + timeout: 5_000, + killSignal: "SIGKILL", + maxBuffer: 64 * 1024, + }, (error, stdout, stderr) => resolve({ code: typeof error?.code === "number" ? error.code : error ? null : 0, stdout, stderr })); + }); +} + +async function askWithRun(run: TaskRun) { + const lookups: string[] = []; + // Keep this structural fixture compatible with the separate question-ID signing secret. + const deps = { + apiToken: API_TOKEN, + questionIdSecret: "synthetic-question-id-secret".repeat(2), + startQuestion: async () => ({ id: RUN_ID }), + getTaskRun: async (id: string) => { + lookups.push(id); + return run; + }, + isLocalDev: true, + log: createLog({}, () => {}), + }; + const server = serve({ fetch: createApp(deps).fetch, hostname: "127.0.0.1", port: 0 }); + try { + await once(server, "listening"); + const address = server.address(); + if (!address || typeof address === "string") throw new Error("fixture server has no TCP address"); + return { ...await ask(`http://127.0.0.1:${address.port}`), lookups }; + } finally { + await new Promise((resolve, reject) => server.close((error) => error ? reject(error) : resolve())); + } +} + +describe("ask CLI workflow failures", { timeout: 10_000 }, () => { + it.each(["failed", "canceled"])("shows a safe %s response and correlation from the real API", async (status) => { + const result = await askWithRun({ status, results: [], error: MARKER }); + expect(result.code).toBe(1); + expect(result.stdout + result.stderr).not.toContain(MARKER); + expect(result.stderr).toContain("The investigation could not be completed."); + expect(result.stderr).toContain(`Trace ${RUN_ID}.`); + expect(result.lookups).toEqual([RUN_ID]); + }); + + it("preserves the safe failure reply of a successfully completed workflow", async () => { + const reply = { kind: "failure", traceId: "completed-investigation-trace", reason: "the investigation hit its time limit" }; + const result = await askWithRun({ status: "succeeded", results: [{ trace_id: reply.traceId, reply }], error: MARKER }); + expect(result.code).toBe(1); + expect(result.stdout + result.stderr).not.toContain(MARKER); + expect(result.stdout).toContain(`I couldn't complete this investigation: ${reply.reason}. Trace ${reply.traceId}.`); + expect(result.lookups).toEqual([RUN_ID]); + }); +});