Merge pull request #76 from corbitsdev/cl-11952-scope-coordinator-and-writer-write_artifact-schemas-to-the

fix(artifacts): scope coordinator and writer write_artifact schemas to their kinds

Author: TheGreatAxios

apps/hub/src/artifacts/tools.test.ts

@@ -0,0 +1,239 @@
+import { expect, test } from "bun:test";
+import type { RuntimeCapabilities } from "@intx/types/runtime-capabilities";
+import type { WorkflowRole } from "@corbits-builder/schemas/installed-workflows";
+import { createArtifactTools } from "./tools.ts";
+
+const deployment = {
+  source: {
+    kind: "asset" as const,
+    assetId: "asset",
+    package: { format: "tarball" as const },
+  },
+  entry: "entry",
+  pin: "@corbits-builder/writer-native@1.0.0",
+  sourceOfferingIds: ["offering"],
+  defaultSourceOfferingId: "offering",
+};
+
+const sampleApproaches = {
+  approaches: [
+    {
+      id: "simple",
+      name: "Simple",
+      summary: "Client-side forecast view.",
+      bestFor: "Fast launch",
+      pros: ["Fast to build"],
+      cons: ["No server cache"],
+      effort: "low" as const,
+      risks: ["Rate limits"],
+    },
+    {
+      id: "cached",
+      name: "Cached",
+      summary: "Server-cached forecast view.",
+      bestFor: "Higher traffic",
+      pros: ["Fewer upstream calls"],
+      cons: ["More moving parts"],
+      effort: "medium" as const,
+      risks: ["Stale cache"],
+    },
+  ],
+  recommended: "simple",
+};
+
+function harness(role: WorkflowRole) {
+  const written: unknown[] = [];
+  const fetch = async (path: string, init: RequestInit) => {
+    if (path === "/api/workflow-project-scope")
+      return Response.json({
+        tenantId: "tenant",
+        projectId: "sb-project1",
+        domain: "example.invalid",
+        role,
+        workflows: {
+          writer: deployment,
+          reviewer: deployment,
+          coder: deployment,
+          designer: deployment,
+        },
+        openNetwork: false,
+        build: {
+          assetDirectory: "/tmp",
+          browserCli: "/tmp/browser",
+          childPath: "/tmp/child.ts",
+          dependencyRoots: ["/tmp"],
+          ripgrepPath: "/tmp/rg",
+          runtimeVersion: "1.0.0",
+        },
+      });
+    if (path.endsWith("/artifacts") && init.method === "POST") {
+      const body = JSON.parse(String(init.body)) as { kind: string };
+      written.push(body);
+      return Response.json({
+        artifact: {
+          artifactId: "art-" + body.kind,
+          kind: body.kind,
+          version: written.length,
+          sha256: "a".repeat(64),
+        },
+      });
+    }
+    return Response.json({ code: "unexpected_route" }, { status: 404 });
+  };
+  const capabilities = {
+    resolve: () => ({ resolve: async () => ({ kind: "http", fetch }) }),
+  } as unknown as RuntimeCapabilities;
+  const instance = createArtifactTools(role)({ capabilities } as never);
+  return {
+    definitions: instance.definitions,
+    written,
+    write: (args: Record<string, unknown>) =>
+      instance.run(
+        { id: "call", name: "write_artifact", arguments: args },
+        new AbortController().signal,
+      ),
+  };
+}
+
+test("coordinator write_artifact exposes only problem_brief without approaches", async () => {
+  const coordinator = harness("coordinator");
+  const definition = coordinator.definitions.find(
+    (item) => item.name === "write_artifact",
+  );
+  expect(definition).toBeDefined();
+  expect(definition?.description).not.toContain("approaches");
+  const properties = (
+    definition?.inputSchema as
+      | { properties?: Record<string, unknown> }
+      | undefined
+  )?.properties;
+  expect(properties?.["kind"]).toEqual({ const: "problem_brief" });
+  expect(properties?.["approaches"]).toBeUndefined();
+
+  const withApproaches = await coordinator.write({
+    kind: "problem_brief",
+    title: "Problem Brief",
+    content: "# Problem Brief\n\nWeather at a glance.",
+    approaches: sampleApproaches,
+  });
+  expect(withApproaches.isError).toBe(true);
+  expect(withApproaches.content).toContain("approaches");
+
+  const wrongKind = await coordinator.write({
+    kind: "constraints",
+    title: "Constraints",
+    content: "# Constraints",
+  });
+  expect(wrongKind.isError).toBe(true);
+  expect(wrongKind.content).toBe(
+    "brief_only: you write the problem brief; deploy_writer has a writer write the constraints",
+  );
+
+  const valid = await coordinator.write({
+    kind: "problem_brief",
+    title: "Problem Brief",
+    content: "# Problem Brief\n\nWeather at a glance.",
+  });
+  expect(valid.isError).toBeUndefined();
+  expect(coordinator.written).toHaveLength(1);
+});
+
+test("every role's artifact tool definitions declare top-level type: object", () => {
+  const roles: WorkflowRole[] = [
+    "coordinator",
+    "writer",
+    "reviewer",
+    "coder",
+    "designer",
+  ];
+  for (const role of roles) {
+    for (const definition of harness(role).definitions) {
+      expect(
+        (definition.inputSchema as { type?: unknown } | undefined)?.type,
+      ).toBe("object");
+    }
+  }
+});
+
+test("writer write_artifact exposes only writer kinds and validates approaches", async () => {
+  const writer = harness("writer");
+  const definition = writer.definitions.find(
+    (item) => item.name === "write_artifact",
+  );
+  expect(
+    (definition?.inputSchema as { type?: unknown } | undefined)?.type,
+  ).toBe("object");
+  const branches = (
+    definition?.inputSchema as
+      | {
+          anyOf?: {
+            additionalProperties?: boolean;
+            properties?: {
+              kind?: { const?: string };
+              approaches?: unknown;
+            };
+          }[];
+        }
+      | undefined
+  )?.anyOf;
+  const branchFor = (kind: string) =>
+    branches?.find((branch) => branch.properties?.kind?.const === kind);
+  expect(branches).toHaveLength(4);
+  const approaches = branchFor("approaches");
+  expect(approaches?.additionalProperties).toBe(false);
+  expect(Object.keys(approaches?.properties ?? {}).sort()).toEqual([
+    "approaches",
+    "content",
+    "kind",
+    "openQuestions",
+    "title",
+  ]);
+  for (const kind of ["constraints", "product_requirements", "build_plan"]) {
+    const branch = branchFor(kind);
+    expect(branch?.additionalProperties).toBe(false);
+    expect(Object.keys(branch?.properties ?? {}).sort()).toEqual([
+      "content",
+      "kind",
+      "openQuestions",
+      "title",
+    ]);
+  }
+  expect(branchFor("problem_brief")).toBeUndefined();
+
+  const briefAttempt = await writer.write({
+    kind: "problem_brief",
+    title: "Problem Brief",
+    content: "# Problem Brief",
+  });
+  expect(briefAttempt.isError).toBe(true);
+  expect(briefAttempt.content).toStartWith("invalid_arguments:");
+
+  for (const kind of ["constraints", "product_requirements", "build_plan"]) {
+    const rejected = await writer.write({
+      kind,
+      title: "Doc",
+      content: "# Doc",
+      approaches: sampleApproaches,
+    });
+    expect(rejected.isError).toBe(true);
+    expect(rejected.content).toStartWith("invalid_arguments:");
+    expect(rejected.content).toContain("approaches");
+  }
+  expect(writer.written).toHaveLength(0);
+
+  const validApproaches = await writer.write({
+    kind: "approaches",
+    title: "Approaches",
+    content: "# Approaches",
+    approaches: sampleApproaches,
+  });
+  expect(validApproaches.isError).toBeUndefined();
+  expect(writer.written).toEqual([
+    {
+      kind: "approaches",
+      title: "Approaches",
+      content: "# Approaches",
+      approaches: sampleApproaches,
+    },
+  ]);
+});

apps/hub/src/artifacts/tools.ts

@@ -8,10 +8,11 @@ import { HUB_CREDENTIAL_HANDLE } from "@intx/types/credential-delivery";
 import type { RuntimeCapabilities } from "@intx/types/runtime-capabilities";
 import {
   ArtifactContent,
-  DocumentWrite,
+  BriefWrite,
   ArtifactList,
   ArtifactRead,
   ArtifactReceipt,
+  WriterDocumentWrite,
 } from "@corbits-builder/schemas/artifact";
 import type { WorkflowRole } from "@corbits-builder/schemas/installed-workflows";
 import { unpackDesign } from "./design-tools.ts";
@@ -45,11 +46,20 @@ const readDefinition = {
     "See which project documents exist, or read one.\nWith no arguments it lists: each document's artifactId, kind, title, current version, sha256, approvedVersion, the version a person approved, and openQuestionCount, the questions its current version leaves open for the person; inFlight, the writer, reviewer, coder and designer runs still open, each with its status, deployed until its first assignment arrives and running after, and each working (mid-turn) or waiting (parked for your next mail), with progress once it has acted: its turns so far, its latest tool calls as tool and target, the files it has written, and lastActivityAt; failed, the latest runs that stopped on an error, with when and why, which deliver nothing more; awaitingApproval, the kinds of filed documents whose current version waits on the person's approval; and preferenceGuidance, how the person has said they want to work.\nWith a kind or an artifactId it returns that document's content at its current version, or at the given version, with the openQuestions that version leaves for the person; a long document comes in numbered parts, and part reads the next one.",
   inputSchema: { ...ToolRead.toJsonSchema() },
 };
-const writeDefinition = {
+const briefWriteDefinition = {
+  name: "write_artifact",
+  description:
+    "Write the next version of the problem brief. The hub assigns its id, version and sha256 and returns them. Writing a version is not approval.\nopenQuestions lists each question in the document's Open questions section that the person must answer: a short id, the question as they would read it, whyItBlocks, and suggestedAnswers, your recommendation first. A version written without openQuestions leaves none open.",
+  inputSchema: { ...BriefWrite.toJsonSchema() },
+};
+const writerWriteDefinition = {
   name: "write_artifact",
   description:
     "Write the next version of the project document of one kind. The hub assigns its id, version and sha256 and returns them. Writing a version is not approval.\nopenQuestions lists each question in the document's Open questions section that the person must answer: a short id, the question as they would read it, whyItBlocks, and suggestedAnswers, your recommendation first. A version written without openQuestions leaves none open.\nOn the approaches document, approaches carries the approaches it compares, for the person to see side by side: each with a short id, its name, a one-line summary, bestFor, its pros, cons and risks as short phrases, and its effort; recommended names the id of the one you recommend.",
-  inputSchema: { ...DocumentWrite.toJsonSchema() },
+  inputSchema: {
+    type: "object" as const,
+    ...WriterDocumentWrite.toJsonSchema(),
+  },
 };
 
 export function failure(callId: string, error: unknown) {
@@ -67,17 +77,23 @@ export function failure(callId: string, error: unknown) {
   return projectScopeFailure(callId, error);
 }
 
+const DEFINITIONS_BY_ROLE: Record<
+  WorkflowRole,
+  readonly (typeof readDefinition)[]
+> = {
+  coordinator: [readDefinition, briefWriteDefinition],
+  writer: [writerWriteDefinition],
+  reviewer: [readDefinition],
+  coder: [readDefinition],
+  designer: [readDefinition],
+};
+
 export function createArtifactTools(
   role: WorkflowRole,
 ): AnnotatedToolFactory<ArtifactToolEnv> {
   // A writer's or reviewer's task carries every document it needs in full; a read tool sends
   // writers looking for documents that do not exist.
-  const definitions =
-    role === "coordinator"
-      ? [readDefinition, writeDefinition]
-      : role === "writer"
-        ? [writeDefinition]
-        : [readDefinition];
+  const definitions = DEFINITIONS_BY_ROLE[role];
   return defineTool<ArtifactToolEnv>({
     id: "@corbits-builder/artifacts/tools",
     credentials: [{ handle: HUB_CREDENTIAL_HANDLE }],
@@ -120,19 +136,28 @@ export function createArtifactTools(
                 signal,
               });
             if (call.name === "write_artifact") {
-              const input = DocumentWrite(call.arguments);
-              if (input instanceof type.errors)
+              // The coordinator's writers own every kind but the brief, so the documents the person
+              // decides on come from them.
+              const rawKind = (call.arguments as { kind?: unknown } | undefined)
+                ?.kind;
+              if (
+                role === "coordinator" &&
+                typeof rawKind === "string" &&
+                rawKind !== "problem_brief"
+              )
                 return {
                   callId: call.id,
-                  content: "invalid_arguments: " + input.summary,
+                  content: `brief_only: you write the problem brief; deploy_writer has a writer write the ${rawKind}`,
                   isError: true,
                 };
-              // The coordinator's writers own every kind but the brief, so the documents the person
-              // decides on come from them.
-              if (role === "coordinator" && input.kind !== "problem_brief")
+              const input =
+                role === "coordinator"
+                  ? BriefWrite(call.arguments)
+                  : WriterDocumentWrite(call.arguments);
+              if (input instanceof type.errors)
                 return {
                   callId: call.id,
-                  content: `brief_only: you write the problem brief; deploy_writer has a writer write the ${input.kind}`,
+                  content: "invalid_arguments: " + input.summary,
                   isError: true,
                 };
               const body = JSON.stringify(input);

apps/hub/src/projects/errors.test.ts

@@ -103,22 +103,6 @@ test("a rejected write_artifact call reaches the agent with {path, message} issu
     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",