fix(hub): send every refusal reason as a list agents can read

apps/hub/src/artifacts/routes.ts

@@ -33,7 +33,7 @@ import {
   ProjectExport,
 } from "@corbits-builder/schemas/project";
 import { actorOf } from "../projects/authorization.ts";
-import { ProjectError } from "../projects/errors.ts";
+import { ProjectError, typeIssues } from "../projects/errors.ts";
 import { readJson } from "../lib/json.ts";
 import { mailRunAsCaller } from "../mail/caller-mail.ts";
 import type { createArtifactService } from "./service.ts";
@@ -691,7 +691,7 @@ export function mountArtifactRoutes(
       );
       if (sections instanceof type.errors)
         throw new ProjectError(400, "invalid_export_sections", {
-          issues: sections.summary,
+          issues: typeIssues(sections, "sections"),
         });
       const { filename, archive } = await service.exportArchive(
         actorOf(c),

apps/hub/src/artifacts/service.ts

@@ -96,7 +96,11 @@ import type { AgentActivityTracker } from "./activity.ts";
 import type { Actor } from "../lib/actor.ts";
 import { parseJson } from "../lib/json.ts";
 import { authorizeProject } from "../projects/authorization.ts";
-import { ProjectError, projectResource } from "../projects/errors.ts";
+import {
+  ProjectError,
+  projectResource,
+  typeIssues,
+} from "../projects/errors.ts";
 import { readAppRepository } from "../projects/app-repository.ts";
 import {
   spawnedWorkflowRole,
@@ -918,7 +922,7 @@ export function createArtifactService(
       const input = DesignTweaks(raw);
       if (input instanceof type.errors)
         throw new ProjectError(400, "invalid_design_tweaks", {
-          issues: input.summary,
+          issues: typeIssues(input),
         });
       await requireProject(actor, projectId, "manage");
       const tokens: Record<string, string> = input.tokens;
@@ -950,7 +954,7 @@ export function createArtifactService(
         const bundle = DesignBundle(parseJson(stored.content));
         if (bundle instanceof type.errors)
           throw new ProjectError(409, "invalid_design", {
-            issues: bundle.summary,
+            issues: typeIssues(bundle),
           });
         const files = {
           ...bundle.files,
@@ -1086,7 +1090,7 @@ export function createArtifactService(
       const material = ProjectExportMaterial(input);
       if (material instanceof type.errors)
         throw new ProjectError(400, "invalid_material", {
-          issues: material.summary,
+          issues: typeIssues(material),
         });
       const allowed = await requireProject(actor, projectId, "manage");
       if (!(await allowed(artifactResource(MATERIAL_KIND), "write")))
@@ -1157,7 +1161,7 @@ export function createArtifactService(
       const input = ArtifactRead(raw);
       if (input instanceof type.errors)
         throw new ProjectError(400, "invalid_artifact_read", {
-          issues: input.summary,
+          issues: typeIssues(input),
         });
       await requireProject(actor, projectId, "read");
       const pinned = input.artifactId?.match(PINNED_REF);
@@ -1252,27 +1256,35 @@ export function createArtifactService(
       const input = ArtifactKindWrite(raw);
       if (input instanceof type.errors)
         throw new ProjectError(400, "invalid_artifact_write", {
-          issues: input.summary,
+          issues: typeIssues(input),
         });
       const bundled =
         input.kind === "design" ||
         input.kind === "design_option" ||
         input.kind === "app";
       if (bundled && input.openQuestions)
         throw new ProjectError(400, "invalid_artifact_write", {
-          issues: "openQuestions belong to documents, not a " + input.kind,
+          issues: [
+            {
+              path: "openQuestions",
+              message: `goes on documents; write this ${input.kind} without it`,
+            },
+          ],
         });
       if (input.approaches) {
         if (input.kind !== "approaches")
           throw new ProjectError(400, "invalid_artifact_write", {
-            issues:
-              "approaches belong to the approaches document, not a " +
-              input.kind,
+            issues: [
+              {
+                path: "approaches",
+                message: `goes on the approaches document; write this ${input.kind} without it`,
+              },
+            ],
           });
         const issue = approachesIssue(input.approaches);
         if (issue)
           throw new ProjectError(400, "invalid_artifact_write", {
-            issues: issue,
+            issues: [issue],
           });
       }
       if (bundled) {
@@ -1281,7 +1293,7 @@ export function createArtifactService(
         );
         if (bundle instanceof type.errors)
           throw new ProjectError(400, "invalid_" + input.kind, {
-            issues: bundle.summary,
+            issues: typeIssues(bundle, "content"),
           });
       }
       if (input.kind === "app") {
@@ -1295,7 +1307,7 @@ export function createArtifactService(
         const document = DocumentContent(input.content);
         if (document instanceof type.errors)
           throw new ProjectError(400, "invalid_artifact_write", {
-            issues: document.summary,
+            issues: typeIssues(document, "content"),
           });
       }
       const allowed = await requireProject(actor, projectId, "read");
@@ -1699,7 +1711,7 @@ export function createArtifactService(
       const input = DesignConceptRecord(raw);
       if (input instanceof type.errors)
         throw new ProjectError(400, "invalid_design_concept", {
-          issues: input.summary,
+          issues: typeIssues(input),
         });
       await requireProject(actor, projectId, "manage");
       const [run] = await rawDB
@@ -1779,7 +1791,7 @@ export function createArtifactService(
       const input = ArtifactApprovalRecord(raw);
       if (input instanceof type.errors)
         throw new ProjectError(400, "invalid_artifact_approval", {
-          issues: input.summary,
+          issues: typeIssues(input),
         });
       await requireProject(actor, projectId, "manage");
       const covers = APPROVAL_COVERS[input.tool]!;
@@ -1886,7 +1898,7 @@ export function createArtifactService(
       const input = DesignChoice(raw);
       if (input instanceof type.errors)
         throw new ProjectError(400, "invalid_design_choice", {
-          issues: input.summary,
+          issues: typeIssues(input),
         });
       await requireProject(actor, projectId, "manage");
       const [chooser] = await rawDB

apps/hub/src/projects/errors.ts

@@ -1,13 +1,40 @@
+import type { ArkError, ArkErrors } from "arktype";
 import { ProjectId } from "@corbits-builder/schemas/project";
+
+// Agents see a rejection's reasons only in this list form.
+type Issues = readonly { path: string; message: string }[];
+
 export class ProjectError extends Error {
   constructor(
     readonly status: 400 | 403 | 404 | 409 | 413 | 503,
     readonly code: string,
-    readonly details: Record<string, unknown> = {},
+    readonly details: { issues?: Issues; [detail: string]: unknown } = {},
     options?: ErrorOptions,
   ) {
     super(code, options);
   }
 }
 export const projectResource = (id: string) =>
   `sb-project:${ProjectId.assert(id)}`;
+
+function issuePath(issue: ArkError, root: string | undefined): string {
+  if (root === undefined) return issue.propString;
+  if (!issue.propString || issue.propString.startsWith("["))
+    return root + issue.propString;
+  return root + "." + issue.propString;
+}
+
+// arktype's own problem text quotes the submitted value; reasons name the rule only.
+const ruleText: Partial<Record<ArkError["code"], string>> = {
+  union: "is not one of the allowed values",
+  predicate: "is not allowed here",
+};
+
+export const typeIssues = (errors: ArkErrors, root?: string): Issues =>
+  [...errors].slice(0, 10).map((issue) => ({
+    path: issuePath(issue, root),
+    message: (ruleText[issue.code] ?? "must be " + issue.expected).slice(
+      0,
+      200,
+    ),
+  }));

apps/hub/src/projects/routes.ts

@@ -30,11 +30,11 @@ import {
 import { readProjectImport } from "@corbits-builder/schemas/project-import";
 import {
   createProjectService,
-  ProjectError,
   type CreateTenant,
   type ProjectCaller,
 } from "./service.ts";
 import { actorOf, resolveProjectTenant } from "./authorization.ts";
+import { ProjectError, typeIssues } from "./errors.ts";
 import {
   createProjectNextStep,
   mountProjectNextStepRoutes,
@@ -271,7 +271,7 @@ export function createProjectRoutes(
         const read = readProjectImport(input);
         if (read instanceof type.errors)
           throw new ProjectError(400, "invalid_project_import", {
-            issues: read.summary,
+            issues: typeIssues(read),
           });
         const { file, omitted } = read;
         const materials = file.materials ?? [];

apps/hub/src/projects/service.ts

@@ -43,7 +43,7 @@ import {
 import { workflowRoleSql } from "../workflows/spawn-role.ts";
 
 export { ProjectError, projectResource } from "./errors.ts";
-import { ProjectError, projectResource } from "./errors.ts";
+import { ProjectError, projectResource, typeIssues } from "./errors.ts";
 import { digest } from "../lib/digest.ts";
 import type { Actor } from "../lib/actor.ts";
 
@@ -417,10 +417,7 @@ export function createProjectService(db: DB["db"]) {
       throw new ProjectError(400, "invalid_project_mutation");
     if (mutation instanceof type.errors)
       throw new ProjectError(400, "invalid_project_mutation", {
-        issues: [...mutation].slice(0, 10).map((issue) => ({
-          path: issue.propString,
-          message: issue.problem.slice(0, 200),
-        })),
+        issues: typeIssues(mutation),
       });
     if (
       mutation.command === "version_project" &&

packages/schemas/src/approaches.ts

@@ -29,12 +29,17 @@ export type ApproachesMeta = typeof ApproachesMeta.infer;
 export function approachesIssue({
   approaches,
   recommended,
-}: ApproachesMeta): string | undefined {
+}: ApproachesMeta): { path: string; message: string } | undefined {
   const ids = approaches.map((approach) => approach.id);
-  const repeated = duplicate(ids);
-  if (repeated !== undefined)
-    return "approach id " + repeated + " is used twice";
+  if (duplicate(ids) !== undefined)
+    return {
+      path: "approaches.approaches",
+      message: "give each approach a unique id",
+    };
   if (recommended !== undefined && !ids.includes(recommended))
-    return "recommended " + recommended + " is not one of the approaches";
+    return {
+      path: "approaches.recommended",
+      message: "name one of the approach ids",
+    };
   return undefined;
 }

test/e2e/artifact-write-issues.test.ts

@@ -0,0 +1,119 @@
+import { expect, test } from "bun:test";
+import { randomBytes, randomUUID } from "node:crypto";
+import { mkdtemp, rm } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import postgres from "postgres";
+import { createHubRuntime } from "@corbits-builder/hub/app";
+import { migrateDatabase } from "@corbits-builder/hub/migrate";
+
+function required(name: string): string {
+  const value = process.env[name];
+  if (!value) throw new Error(`Missing required e2e environment: ${name}`);
+  return value;
+}
+
+const approach = (id: string) => ({
+  id,
+  name: `Approach ${id}`,
+  summary: "A way to build it.",
+  bestFor: "Trying it out.",
+  pros: ["Quick"],
+  cons: ["Limited"],
+  effort: "low",
+  risks: [],
+});
+
+// Agents read a rejection's reasons only as a list of {path, message}, so a rejected document
+// write must name the field to change in that form.
+test("a rejected document write names the field in its issues", async () => {
+  const database = {
+    host: required("E2E_DB_HOST"),
+    port: Number(required("E2E_DB_PORT")),
+    username: required("E2E_DB_USER"),
+    password: required("E2E_DB_PASSWORD"),
+  };
+  const dbName = `sb_write_${randomUUID().replaceAll("-", "")}`;
+  const admin = postgres({
+    ...database,
+    database: "postgres",
+    max: 1,
+    onnotice: () => undefined,
+  });
+  await admin.unsafe(`create database ${dbName}`);
+  const directory = await mkdtemp(join(tmpdir(), "sb-artifact-write-"));
+  const origin = "http://127.0.0.1:1";
+  const environment: Record<string, string> = {
+    NODE_ENV: "test",
+    HOST: "127.0.0.1",
+    // Never bound: requests go straight to the app.
+    PORT: "1",
+    DB_HOST: database.host,
+    DB_PORT: String(database.port),
+    DB_USER: database.username,
+    DB_PASSWORD: database.password,
+    DB_NAME: dbName,
+    DB_SSL: "false",
+    BETTER_AUTH_BASE_URL: origin,
+    BETTER_AUTH_SECRET: randomBytes(32).toString("hex"),
+    CREDENTIAL_ENCRYPTION_KEY: randomBytes(32).toString("hex"),
+    PRINCIPAL_KEY_ENCRYPTION_KEY: randomBytes(32).toString("hex"),
+    HUB_DATA_DIR: join(directory, "hub"),
+  };
+  let runtime: Awaited<ReturnType<typeof createHubRuntime>> | undefined;
+  try {
+    await migrateDatabase(environment);
+    runtime = await createHubRuntime(environment);
+    const call = (path: string, body: unknown, cookie?: string) =>
+      runtime!.app.fetch(
+        new Request(`${origin}${path}`, {
+          method: "POST",
+          headers: {
+            origin,
+            "content-type": "application/json",
+            ...(cookie ? { cookie } : {}),
+          },
+          body: JSON.stringify(body),
+        }),
+      );
+    const signUp = await call("/api/auth/sign-up/email", {
+      email: `write-${dbName}@example.invalid`,
+      password: randomBytes(16).toString("hex"),
+      name: "Artifact write e2e",
+    });
+    expect(signUp.status).toBe(200);
+    const cookie = signUp.headers
+      .getSetCookie()
+      .map((value) => value.split(";")[0])
+      .join("; ");
+    const created = await call(
+      "/api/tenants",
+      { name: "Artifact write e2e", slug: dbName.replaceAll("_", "-") },
+      cookie,
+    );
+    expect(created.status).toBe(201);
+    const tenantId = ((await created.json()) as { id: string }).id;
+    // The write is checked before the project is looked up, so no project (and no coordinator
+    // run) is needed for the rejection.
+    const write = await call(
+      `/api/tenants/${tenantId}/projects/p-${randomUUID().replaceAll("-", "")}/artifacts`,
+      {
+        kind: "problem_brief",
+        title: "Brief",
+        content: "# Brief\n\nThe problem.",
+        approaches: { approaches: [approach("a"), approach("b")] },
+      },
+      cookie,
+    );
+    expect(write.status).toBe(400);
+    expect(await write.json()).toEqual({
+      code: "invalid_artifact_write",
+      issues: [{ path: "approaches", message: expect.any(String) }],
+    });
+  } finally {
+    await runtime?.close();
+    await admin.unsafe(`drop database if exists ${dbName} with (force)`);
+    await admin.end();
+    await rm(directory, { recursive: true, force: true });
+  }
+}, 120_000);