diff --git a/apps/hub/src/artifacts/routes.ts b/apps/hub/src/artifacts/routes.ts index 16d87e26..e8fd456d 100644 --- a/apps/hub/src/artifacts/routes.ts +++ b/apps/hub/src/artifacts/routes.ts @@ -691,7 +691,7 @@ export function mountArtifactRoutes( ); if (sections instanceof type.errors) throw new ProjectError(400, "invalid_export_sections", { - issues: sections.summary, + issues: 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 2d1ccecc..517c44d7 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, @@ -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: input, }); await requireProject(actor, projectId, "manage"); const tokens: Record = 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: 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: 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: input, }); await requireProject(actor, projectId, "read"); const pinned = input.artifactId?.match(PINNED_REF); @@ -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: 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"); @@ -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: 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: 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: input, }); await requireProject(actor, projectId, "manage"); const [chooser] = await rawDB diff --git a/apps/hub/src/projects/errors.test.ts b/apps/hub/src/projects/errors.test.ts new file mode 100644 index 00000000..0d9ba326 --- /dev/null +++ b/apps/hub/src/projects/errors.test.ts @@ -0,0 +1,207 @@ +import { expect, test } from "bun:test"; +import { type } from "arktype"; +import { PostgresJsDatabase } from "drizzle-orm/postgres-js"; +import { Hono } from "hono"; +import type { TenantEnv } from "@intx/hub-api"; +import { + ArtifactKindWrite, + DesignBundle, + DesignTweaks, +} from "@corbits-builder/schemas/artifact"; +import { ProjectExportMaterial } from "@corbits-builder/schemas/project"; +import { mountArtifactRoutes } from "../artifacts/routes.ts"; +import { createArtifactService } from "../artifacts/service.ts"; +import { createArtifactTools } from "../artifacts/tools.ts"; +import { ProjectError, typeIssues } from "./errors.ts"; + +const deployment = { + source: { + kind: "asset" as const, + assetId: "asset", + package: { format: "tarball" as const }, + }, + pin: "@corbits-builder/writer@1.0.0", + entry: "entry", + sourceOfferingIds: ["offering"], + defaultSourceOfferingId: "offering", +}; + +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: [], +}); + +test("a rejected write_artifact call reaches the agent with {path, message} issues", async () => { + const service = createArtifactService( + Object.create(PostgresJsDatabase.prototype), + {} as never, + ); + const app = new Hono(); + app.onError((error, c) => { + if (error instanceof ProjectError) + return c.json({ code: error.code, ...error.details }, error.status); + throw error; + }); + app.use("*", async (c, next) => { + c.set("tenant", { id: "tenant" } as never); + c.set("principal", { id: "principal" } as never); + await next(); + }); + const projects = new Hono(); + mountArtifactRoutes(projects, service); + app.route("/api/tenants/:tenantId/projects", projects); + app.get("/api/workflow-project-scope", (c) => + c.json({ + tenantId: "tenant", + projectId: "p-00000000000000000000000000000000", + domain: "example.invalid", + role: "writer", + workflows: { + writer: deployment, + reviewer: deployment, + coder: deployment, + designer: deployment, + }, + openNetwork: false, + build: { + assetDirectory: "/a", + browserCli: "/b", + childPath: "/c", + dependencyRoots: ["/d"], + ripgrepPath: "/r", + runtimeVersion: "1", + }, + }), + ); + const tools = createArtifactTools("writer")({ + capabilities: { + resolve: () => ({ + resolve: async () => ({ + kind: "http", + fetch: (path: string, init?: RequestInit) => + app.fetch(new Request("http://hub.local" + path, init)), + }), + }), + }, + } as never); + const write = async (args: Record) => { + const result = await tools.run( + { + id: "call", + name: "write_artifact", + arguments: { title: "Doc", content: "# Doc\n\nBody.", ...args }, + }, + new AbortController().signal, + ); + expect(result.isError).toBe(true); + return JSON.parse(String(result.content)); + }; + + expect( + await write({ + kind: "problem_brief", + approaches: { approaches: [approach("a"), approach("b")] }, + }), + ).toEqual({ + code: "invalid_artifact_write", + status: 400, + issues: [ + { + path: "approaches", + message: + "goes on the approaches document; write this problem_brief without it", + }, + ], + }); + expect( + await write({ + kind: "approaches", + approaches: { approaches: [approach("a"), approach("a")] }, + }), + ).toEqual({ + code: "invalid_artifact_write", + status: 400, + issues: [ + { + path: "approaches.approaches", + message: "give each approach a unique id", + }, + ], + }); + expect( + await write({ + kind: "approaches", + approaches: { + approaches: [approach("a"), approach("b")], + recommended: "c", + }, + }), + ).toEqual({ + code: "invalid_artifact_write", + status: 400, + issues: [ + { + path: "approaches.recommended", + message: "name one of the approach ids", + }, + ], + }); +}); + +test("typeIssues names the rule for narrow checks, unions and extra keys without quoting submitted values", () => { + const secret = "sk-live-secret-value-not-for-issues"; + const bundle = DesignBundle({ files: { "notes.txt": secret } }); + const tweaks = DesignTweaks({ tokens: {} }); + const write = ArtifactKindWrite({ + kind: secret, + title: "Title", + content: "Body", + extra: secret, + }); + const material = ProjectExportMaterial({ + title: "file", + content: secret.repeat(8000), + }); + if ( + !(bundle instanceof type.errors) || + !(tweaks instanceof type.errors) || + !(write instanceof type.errors) || + !(material instanceof type.errors) + ) + throw new Error("expected validation errors"); + + expect(typeIssues(bundle, "content")).toEqual([ + { + path: "content.files", + message: "must be a bundle with DESIGN.md and designs/index.html", + }, + ]); + expect(typeIssues(tweaks)).toEqual([ + { path: "tokens", message: "must be at least one token" }, + ]); + expect(typeIssues(write)).toEqual([ + { + path: "kind", + message: + 'must be "app", "approaches", "build_plan", "constraints", "design", "design_option", "problem_brief" or "product_requirements"', + }, + { path: "extra", message: "is not allowed here" }, + ]); + expect(typeIssues(material)).toEqual([ + { path: "content", message: "is not one of the allowed values" }, + ]); + expect( + JSON.stringify([ + typeIssues(bundle, "content"), + typeIssues(tweaks), + typeIssues(write), + typeIssues(material), + ]), + ).not.toContain(secret); +}); diff --git a/apps/hub/src/projects/errors.ts b/apps/hub/src/projects/errors.ts index 4421b399..d4e202e8 100644 --- a/apps/hub/src/projects/errors.ts +++ b/apps/hub/src/projects/errors.ts @@ -1,13 +1,48 @@ +import { type, type ArkError, type 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 { + readonly details: Record; constructor( readonly status: 400 | 403 | 404 | 409 | 413 | 503, readonly code: string, - readonly details: Record = {}, + { + issues, + ...details + }: { issues?: Issues | ArkErrors; [detail: string]: unknown } = {}, options?: ErrorOptions, ) { super(code, options); + const listed = issues instanceof type.errors ? typeIssues(issues) : issues; + this.details = + listed === undefined ? details : { ...details, issues: listed }; } } 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 problem text and union expected text quote the submitted value; reasons name the rule only. +function issueMessage(issue: ArkError): string { + if (issue.code === "union") return "is not one of the allowed values"; + if (issue.expected === "removed") return "is not allowed here"; + return ("must be " + issue.expected).slice(0, 200); +} + +export const typeIssues = (errors: ArkErrors, root?: string): Issues => + [...errors] + .flatMap((issue) => issue.flat) + .slice(0, 10) + .map((issue) => ({ + path: issuePath(issue, root), + message: issueMessage(issue), + })); diff --git a/apps/hub/src/projects/routes.ts b/apps/hub/src/projects/routes.ts index 4eea6f6f..4c2793e8 100644 --- a/apps/hub/src/projects/routes.ts +++ b/apps/hub/src/projects/routes.ts @@ -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: read, }); const { file, omitted } = read; const materials = file.materials ?? []; diff --git a/apps/hub/src/projects/service.ts b/apps/hub/src/projects/service.ts index 86f4c129..db899212 100644 --- a/apps/hub/src/projects/service.ts +++ b/apps/hub/src/projects/service.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: 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; }