fix(cli): name the record a 404 could not find

.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.

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 ?? "<id>"}.`,
+    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 <contract>" 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) =>

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", () => {

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<string> = 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<string, unknown>)[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" };

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, {

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");
     });
   });
 });

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: