From 26fd89039bdf690f734ff898187286702460ec1a Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 1 Nov 2026 12:04:21 -0800 Subject: [PATCH 1/2] fix(hub): tell agents which field a rejected document write got wrong Refs CL-11925 --- apps/hub/src/artifacts/service.ts | 30 +++++-- apps/hub/src/projects/errors.ts | 24 +++++ apps/hub/src/projects/service.ts | 7 +- packages/schemas/src/approaches.ts | 15 ++-- test/e2e/artifact-write-issues.test.ts | 119 +++++++++++++++++++++++++ 5 files changed, 176 insertions(+), 19 deletions(-) create mode 100644 test/e2e/artifact-write-issues.test.ts diff --git a/apps/hub/src/artifacts/service.ts b/apps/hub/src/artifacts/service.ts index 2d1ccecc..77524202 100644 --- a/apps/hub/src/artifacts/service.ts +++ b/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, @@ -1252,7 +1256,7 @@ 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" || @@ -1260,19 +1264,27 @@ export function createArtifactService( 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"); diff --git a/apps/hub/src/projects/errors.ts b/apps/hub/src/projects/errors.ts index 4421b399..c4db78a0 100644 --- a/apps/hub/src/projects/errors.ts +++ b/apps/hub/src/projects/errors.ts @@ -1,3 +1,4 @@ +import type { ArkError, ArkErrors } from "arktype"; import { ProjectId } from "@corbits-builder/schemas/project"; export class ProjectError extends Error { constructor( @@ -11,3 +12,26 @@ export class ProjectError extends Error { } 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> = { + union: "is not one of the allowed values", + predicate: "is not allowed here", +}; + +// Agents see a rejection's reasons only in this list form. +export const typeIssues = (errors: ArkErrors, root?: string) => + [...errors].slice(0, 10).map((issue) => ({ + path: issuePath(issue, root), + message: (ruleText[issue.code] ?? "must be " + issue.expected).slice( + 0, + 200, + ), + })); diff --git a/apps/hub/src/projects/service.ts b/apps/hub/src/projects/service.ts index 86f4c129..7552f91d 100644 --- a/apps/hub/src/projects/service.ts +++ b/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" && diff --git a/packages/schemas/src/approaches.ts b/packages/schemas/src/approaches.ts index b70ebff5..15e28dae 100644 --- a/packages/schemas/src/approaches.ts +++ b/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; } diff --git a/test/e2e/artifact-write-issues.test.ts b/test/e2e/artifact-write-issues.test.ts new file mode 100644 index 00000000..d46d9499 --- /dev/null +++ b/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 = { + 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> | 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); From bd6ba59b960633696bde4a5461f8d94c37f1259e Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 1 Nov 2026 12:06:43 -0800 Subject: [PATCH 2/2] fix(hub): send every refusal reason as a list agents can read Refs CL-11927 --- apps/hub/src/artifacts/routes.ts | 4 ++-- apps/hub/src/artifacts/service.ts | 14 +++++++------- apps/hub/src/projects/errors.ts | 9 ++++++--- apps/hub/src/projects/routes.ts | 4 ++-- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/apps/hub/src/artifacts/routes.ts b/apps/hub/src/artifacts/routes.ts index 16d87e26..1158c2d2 100644 --- a/apps/hub/src/artifacts/routes.ts +++ b/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), diff --git a/apps/hub/src/artifacts/service.ts b/apps/hub/src/artifacts/service.ts index 77524202..5766e3d1 100644 --- a/apps/hub/src/artifacts/service.ts +++ b/apps/hub/src/artifacts/service.ts @@ -922,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 = input.tokens; @@ -954,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, @@ -1090,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"))) @@ -1161,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); @@ -1711,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 @@ -1791,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]!; @@ -1898,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 diff --git a/apps/hub/src/projects/errors.ts b/apps/hub/src/projects/errors.ts index c4db78a0..d06fc0ac 100644 --- a/apps/hub/src/projects/errors.ts +++ b/apps/hub/src/projects/errors.ts @@ -1,10 +1,14 @@ 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 = {}, + readonly details: { issues?: Issues; [detail: string]: unknown } = {}, options?: ErrorOptions, ) { super(code, options); @@ -26,8 +30,7 @@ const ruleText: Partial> = { predicate: "is not allowed here", }; -// Agents see a rejection's reasons only in this list form. -export const typeIssues = (errors: ArkErrors, root?: string) => +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( diff --git a/apps/hub/src/projects/routes.ts b/apps/hub/src/projects/routes.ts index 4eea6f6f..a26089ab 100644 --- a/apps/hub/src/projects/routes.ts +++ b/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 ?? [];