From 64c031baeebaedd19796e67a210fbc5d01923891 Mon Sep 17 00:00:00 2001 From: Goran Gajic Date: Thu, 17 Sep 2026 17:12:03 +0200 Subject: [PATCH] fix(cli): name the record a 404 could not find, not the endpoint A missing trigger read as "could not find trigger.get", which sounds like the endpoint is gone rather than the record. Triggers, issues, flows, agent sessions and files now answer with what was asked for and its id, and a request with no such id says it matched nothing. --- .changeset/name-the-missing-record.md | 5 ++ src/core/messages/authErrors.ts | 8 +++- src/core/publicApi/notFoundSubject.test.ts | 46 +++++++++++++++++-- src/core/publicApi/notFoundSubject.ts | 23 ++++++++++ .../platform/callPublicApi.notFound.test.ts | 10 ++++ src/shell/platform/describeNotFound.test.ts | 25 ++++++++-- src/shell/platform/describeNotFound.ts | 2 + 7 files changed, 109 insertions(+), 10 deletions(-) create mode 100644 .changeset/name-the-missing-record.md diff --git a/.changeset/name-the-missing-record.md b/.changeset/name-the-missing-record.md new file mode 100644 index 000000000..8a1f08ffc --- /dev/null +++ b/.changeset/name-the-missing-record.md @@ -0,0 +1,5 @@ +--- +"@qawolf/cli": patch +--- + +A 404 on a trigger, issue, flow, agent session or file now names that record and the id it was asked for — `QA Wolf has no trigger trg-1 (HTTP 404).` It used to name the endpoint instead, which read as though the endpoint itself were gone. A request with no such id to name says it matched nothing, rather than that it could not be found. diff --git a/src/core/messages/authErrors.ts b/src/core/messages/authErrors.ts index c27aea40d..34e5f5a8d 100644 --- a/src/core/messages/authErrors.ts +++ b/src/core/messages/authErrors.ts @@ -47,8 +47,14 @@ export const authErrorMessages = { */ runIdMayBeRunnerLocal: (runId: string | undefined) => `A run id printed by qawolf runner run belongs to that runner rather than to the platform, so this command cannot resolve it. Read that run with qawolf runner events run-status --run ${runId ?? ""}.`, + notFound404Record: (noun: string, id: string) => + `QA Wolf has no ${noun} ${id} (HTTP 404).`, + // Says the request matched nothing rather than that the endpoint is + // missing, which is how "could not find " read. notFound404: (noun: string | undefined) => - `QA Wolf API could not find ${noun ?? "what the request named"} (HTTP 404).`, + noun === undefined + ? "QA Wolf found nothing matching the request (HTTP 404)." + : `QA Wolf found nothing matching the ${noun} request (HTTP 404).`, failedWithStatus: (status: number, noun: string | undefined) => `QA Wolf API${noun ? ` ${noun}` : ""} request failed (HTTP ${status}).`, networkUnreachable: (baseUrl: string, noun: string | undefined) => diff --git a/src/core/publicApi/notFoundSubject.test.ts b/src/core/publicApi/notFoundSubject.test.ts index 78daf62aa..3705462c3 100644 --- a/src/core/publicApi/notFoundSubject.test.ts +++ b/src/core/publicApi/notFoundSubject.test.ts @@ -36,12 +36,50 @@ describe("notFoundSubject", () => { ).toEqual({ kind: "environment" }); }); - // The bug this replaces: a trigger or an issue that does not exist was - // reported as an environment problem. - it("leaves a request that names no environment unattributed", () => { - expect(notFoundSubject("trigger.get", { triggerId: "t-1" })).toEqual({ + it.each([ + ["trigger.get", { triggerId: "trg-1" }, "trigger", "trg-1"], + ["issue.get", { issueId: "iss-1" }, "issue", "iss-1"], + ["flow.update", { flowId: "flw-1" }, "flow", "flw-1"], + ["agent.get", { sessionId: "ses-1" }, "session", "ses-1"], + [ + "file.requestDownload", + { filePath: "logs/out.txt" }, + "file", + "logs/out.txt", + ], + ] as const)( + "names the record %s resolves", + (contractName, input, noun, id) => { + expect(notFoundSubject(contractName, input)).toEqual({ + id, + kind: "record", + noun, + }); + }, + ); + + // The record is what was not found; the environment it was looked up in is + // beside the point, and blaming it is the bug this family of fixes is about. + it("names the record even when the request also scoped an environment", () => { + expect( + notFoundSubject("trigger.update", { + environmentId: "env-1", + triggerId: "trg-1", + }), + ).toEqual({ id: "trg-1", kind: "record", noun: "trigger" }); + }); + + // An id the route takes as a parameter is not the thing being resolved. + it("ignores the workspace and the ids a route only takes as arguments", () => { + expect(notFoundSubject("tag.list", { workspaceId: "wsp-1" })).toEqual({ kind: "other", }); + expect( + notFoundSubject("trigger.create", { + environmentId: "env-1", + timezoneId: "UTC", + }), + ).toEqual({ kind: "environment" }); }); it("survives an input that is not an object", () => { diff --git a/src/core/publicApi/notFoundSubject.ts b/src/core/publicApi/notFoundSubject.ts index 6457864d9..6f4059c7b 100644 --- a/src/core/publicApi/notFoundSubject.ts +++ b/src/core/publicApi/notFoundSubject.ts @@ -6,6 +6,7 @@ export type NotFoundSubject = | { kind: "runner"; runnerId: string | undefined } | { kind: "run"; runId: string | undefined } | { kind: "environment" } + | { kind: "record"; noun: string; id: string } | { kind: "other" }; // `launch` starts a runner and `list` names none, so neither can 404 over a @@ -24,6 +25,22 @@ const runRoutesThatResolveOneRun: ReadonlySet = new Set([ "run.stop", ]); +/** + * The input fields that name the record a route resolves, and what to call it. + * + * An allowlist rather than every key ending in `Id`, because several routes + * carry an id that is a parameter rather than the subject: `trigger.create` + * takes a `timezoneId`, `run.create` an `aiTaskId`, and almost everything + * carries the `workspaceId` the client injects. + */ +const recordFieldNouns: readonly (readonly [string, string])[] = [ + ["sessionId", "session"], + ["triggerId", "trigger"], + ["issueId", "issue"], + ["flowId", "flow"], + ["filePath", "file"], +]; + function field(input: unknown, name: string): string | undefined { if (typeof input !== "object" || input === null) return undefined; const value = (input as Record)[name]; @@ -48,6 +65,12 @@ export function notFoundSubject( if (runRoutesThatResolveOneRun.has(contractName)) { return { kind: "run", runId: field(input, "runId") }; } + // Before the environment rule: a route that resolves a record can also be + // scoped to an environment, and it is the record that was not found. + for (const [name, noun] of recordFieldNouns) { + const id = field(input, name); + if (id !== undefined) return { id, kind: "record", noun }; + } if (field(input, "environmentId") !== undefined) return { kind: "environment" }; return { kind: "other" }; diff --git a/src/shell/platform/callPublicApi.notFound.test.ts b/src/shell/platform/callPublicApi.notFound.test.ts index 34779b5fa..0999a03f2 100644 --- a/src/shell/platform/callPublicApi.notFound.test.ts +++ b/src/shell/platform/callPublicApi.notFound.test.ts @@ -54,6 +54,16 @@ describe("a public API 404", () => { expect(result.errorBody).toContain("qawolf runner run"); }); + it("reads a trigger lookup as a trigger this team does not hold", async () => { + const result = await client().callPublicApi(publicContractsV1.trigger.get, { + triggerId: "trg-1", + }); + + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toBe("QA Wolf has no trigger trg-1 (HTTP 404)."); + }); + // The one case the old wording was true of, and the only one that keeps it. it("keeps pointing an environment-scoped route at --env", async () => { const result = await client().callPublicApi(publicContractsV1.run.create, { diff --git a/src/shell/platform/describeNotFound.test.ts b/src/shell/platform/describeNotFound.test.ts index fd9d329e5..a877e561e 100644 --- a/src/shell/platform/describeNotFound.test.ts +++ b/src/shell/platform/describeNotFound.test.ts @@ -145,18 +145,33 @@ describe("describeNotFound", () => { }); }); - describe("anything else", () => { - it("names what was asked for without blaming an environment", () => { + describe("a record the platform does not hold", () => { + it("names the record and the id, not the endpoint", () => { const described = describeNotFound( - { kind: "other" }, + { id: "trg-1", kind: "record", noun: "trigger" }, "trigger.get", bareNotFound, ); + expect(described.error).toBe("QA Wolf has no trigger trg-1 (HTTP 404)."); + expect(described.exitCode).toBe(exitCodes.notFound); + expect("errorBody" in described).toBe(false); + }); + }); + + describe("a request with nothing to name", () => { + // "could not find tag.list" read as though the endpoint were gone. + it("says the request matched nothing", () => { + const described = describeNotFound( + { kind: "other" }, + "tag.list", + bareNotFound, + ); + expect(described.error).toBe( - "QA Wolf API could not find trigger.get (HTTP 404).", + "QA Wolf found nothing matching the tag.list request (HTTP 404).", ); - expect("errorBody" in described).toBe(false); + expect(described.error).not.toContain("environment"); }); }); }); diff --git a/src/shell/platform/describeNotFound.ts b/src/shell/platform/describeNotFound.ts index 2dc7c7a0b..7745311af 100644 --- a/src/shell/platform/describeNotFound.ts +++ b/src/shell/platform/describeNotFound.ts @@ -62,6 +62,8 @@ export function describeNotFound( guess: m.runIdMayBeRunnerLocal(subject.runId), statement: m.notFound404Run(subject.runId), }); + case "record": + return say({ statement: m.notFound404Record(subject.noun, subject.id) }); case "other": return say({ statement: m.notFound404(noun) }); // A caller that named no subject is one of the environment-scoped reads: