diff --git a/apps/web/src/app.css b/apps/web/src/app.css index 301c227c8..9252f4811 100644 --- a/apps/web/src/app.css +++ b/apps/web/src/app.css @@ -3722,6 +3722,13 @@ tr.insights-row-clickable:hover { font-weight: 600; text-decoration: underline; } +.chat-package-rejection-icon { + width: 0.9em; + height: 0.9em; + margin-right: 0.35rem; + vertical-align: -0.1em; + color: var(--destructive); +} /* Sidebar sections: Workbenches above Chats, each with its own label. */ .shell-panel-section-label { diff --git a/apps/web/src/chat/deployable-package.test.ts b/apps/web/src/chat/deployable-package.test.ts index 72dc1b0cf..cc526b71a 100644 --- a/apps/web/src/chat/deployable-package.test.ts +++ b/apps/web/src/chat/deployable-package.test.ts @@ -3,6 +3,7 @@ import { describe, expect, test } from "bun:test"; import { deployablePackageFromBody, isFiveFieldCron, + isPackageRejection, resolveMessagePackage, } from "./deployable-package"; @@ -15,14 +16,15 @@ describe("deployablePackageFromBody", () => { `Here's the agent.\n\npackage.json\n\`\`\`\n${PACKAGE_JSON}\n\`\`\`\n\n` + `definition.json\n\`\`\`\n${DEFINITION_JSON}\n\`\`\`\n\nPress Deploy.`; const result = deployablePackageFromBody(body); - expect(result?.pkg.name).toBe("Echo"); + expect(isPackageRejection(result?.outcome ?? null)).toBe(false); + expect(result?.outcome && "name" in result.outcome ? result.outcome.name : null).toBe("Echo"); expect(result?.strippedBody).toBe("Here's the agent.\n\nPress Deploy."); }); test("matches an info-string label instead of a preceding line", () => { const body = `\`\`\`json package.json\n${PACKAGE_JSON}\n\`\`\`\n\`\`\`definition.json\n${DEFINITION_JSON}\n\`\`\``; const result = deployablePackageFromBody(body); - expect(result?.pkg.name).toBe("Echo"); + expect(result?.outcome && "name" in result.outcome ? result.outcome.name : null).toBe("Echo"); }); test("reads a comment label with a path prefix on the fence's first line", () => { @@ -30,7 +32,7 @@ describe("deployablePackageFromBody", () => { `Here is the package:\n\n\`\`\`json\n// Scribe/definition.json\n${DEFINITION_JSON}\n\`\`\`\n\n` + `\`\`\`json\n# Scribe/package.json\n${PACKAGE_JSON}\n\`\`\`\n\nPress Deploy.`; const result = deployablePackageFromBody(body); - expect(result?.pkg.name).toBe("Echo"); + expect(result?.outcome && "name" in result.outcome ? result.outcome.name : null).toBe("Echo"); expect(result?.strippedBody).toBe("Here is the package:\n\nPress Deploy."); }); @@ -38,7 +40,8 @@ describe("deployablePackageFromBody", () => { const body = `\`\`\`json\npackage.json\n${PACKAGE_JSON}\n\`\`\`\n\n` + `\`\`\`json\ndefinition.json\n${DEFINITION_JSON}\n\`\`\``; - expect(deployablePackageFromBody(body)?.pkg.name).toBe("Echo"); + const result = deployablePackageFromBody(body); + expect(result?.outcome && "name" in result.outcome ? result.outcome.name : null).toBe("Echo"); }); test("is null when only one of the two files is present", () => { @@ -46,11 +49,46 @@ describe("deployablePackageFromBody", () => { expect(deployablePackageFromBody(body)).toBeNull(); }); - test("is null when the definition block fails validation", () => { + test("rejects with a named field when the definition block fails validation", () => { const body = `package.json\n\`\`\`\n${PACKAGE_JSON}\n\`\`\`\n` + - `definition.json\n\`\`\`\n{"name": ""}\n\`\`\``; - expect(deployablePackageFromBody(body)).toBeNull(); + `definition.json\n\`\`\`\n{"systemPrompt": ""}\n\`\`\``; + const result = deployablePackageFromBody(body); + expect(isPackageRejection(result?.outcome ?? null)).toBe(true); + expect(result?.outcome && "reason" in result.outcome ? result.outcome.reason : null).toBe( + "definition.json is missing systemPrompt", + ); + }); + + test("rejects with a plain-words reason when definition.json isn't valid JSON", () => { + const body = + `package.json\n\`\`\`\n${PACKAGE_JSON}\n\`\`\`\n` + + `definition.json\n\`\`\`\nnot json\n\`\`\``; + const result = deployablePackageFromBody(body); + expect(result?.outcome && "reason" in result.outcome ? result.outcome.reason : null).toBe( + "definition.json is not valid JSON", + ); + }); + + test("takes name from package.json when definition.json omits it", () => { + const body = + `package.json\n\`\`\`\n${PACKAGE_JSON}\n\`\`\`\n` + + `definition.json\n\`\`\`\n{"systemPrompt": "Echo back what you hear."}\n\`\`\``; + const result = deployablePackageFromBody(body); + expect(result?.outcome && "name" in result.outcome ? result.outcome.name : null).toBe("echo"); + }); + + test("ignores an unknown extra field in definition.json", () => { + const body = + `package.json\n\`\`\`\n${PACKAGE_JSON}\n\`\`\`\n` + + `definition.json\n\`\`\`\n${JSON.stringify({ + name: "Echo", + systemPrompt: "Echo back what you hear.", + type: "workflow", + })}\n\`\`\``; + const result = deployablePackageFromBody(body); + expect(isPackageRejection(result?.outcome ?? null)).toBe(false); + expect(result?.outcome && "name" in result.outcome ? result.outcome.name : null).toBe("Echo"); }); }); @@ -71,7 +109,7 @@ describe("resolveMessagePackage", () => { test("falls back to the body when there are no attachments", () => { const body = `package.json\n\`\`\`\n${PACKAGE_JSON}\n\`\`\`\ndefinition.json\n\`\`\`\n${DEFINITION_JSON}\n\`\`\``; const { pkg, renderedBody } = resolveMessagePackage([], body); - expect(pkg?.name).toBe("Echo"); + expect(pkg && "name" in pkg ? pkg.name : null).toBe("Echo"); expect(renderedBody).toBe(""); }); @@ -88,7 +126,25 @@ describe("resolveMessagePackage", () => { ], body, ); - expect(pkg?.name).toBe("Attached"); + expect(pkg && "name" in pkg ? pkg.name : null).toBe("Attached"); expect(renderedBody).toBe(body); }); + + test("surfaces a rejection from attachments naming the missing field", () => { + const { pkg } = resolveMessagePackage( + [ + { name: "package.json", contentType: "application/json", text: PACKAGE_JSON }, + { + name: "definition.json", + contentType: "application/json", + text: `{"name": "Attached"}`, + }, + ], + "unused body", + ); + expect(isPackageRejection(pkg)).toBe(true); + expect(pkg && "reason" in pkg ? pkg.reason : null).toBe( + "definition.json is missing systemPrompt", + ); + }); }); diff --git a/apps/web/src/chat/deployable-package.ts b/apps/web/src/chat/deployable-package.ts index 8d0f5e8da..99073884c 100644 --- a/apps/web/src/chat/deployable-package.ts +++ b/apps/web/src/chat/deployable-package.ts @@ -12,8 +12,12 @@ import { reportError } from "@corbits/error-sink"; import type { MailAttachment } from "./threads-api"; -const AgentDefinition = type({ - name: "string", +const PackageManifest = type({ + "name?": "string", +}); + +const AgentDefinitionShape = type({ + "name?": "string", systemPrompt: "string", "description?": "string", "schedule?": "string", @@ -26,30 +30,97 @@ export function isFiveFieldCron(schedule: string): boolean { return schedule.trim().split(/\s+/).length === 5; } -export type DeployablePackage = typeof AgentDefinition.infer; +export interface DeployablePackage { + readonly name: string; + readonly systemPrompt: string; + readonly description?: string; + readonly schedule?: string; +} export const PACKAGE_MANIFEST_NAME = "package.json"; export const AGENT_DEFINITION_NAME = "definition.json"; -/** The agent package a message's attachments describe, or null when they - * are just files. Untrusted input: parsed, never cast. */ -export function deployablePackage( - attachments: readonly MailAttachment[], -): DeployablePackage | null { - if (!attachments.some((attachment) => attachment.name === PACKAGE_MANIFEST_NAME)) return null; - const definition = attachments.find((attachment) => attachment.name === AGENT_DEFINITION_NAME); - if (definition === undefined) return null; - let body: unknown; +/** + * What a message's attachments (or fenced blocks) resolve to: a parsed + * package, a named rejection when the message attempted a package but + * got a field wrong, or `null` when the message isn't a package attempt + * at all. + */ +export type PackageOutcome = DeployablePackage | { readonly reason: string } | null; + +export function isPackageRejection( + outcome: PackageOutcome, +): outcome is { readonly reason: string } { + return outcome !== null && "reason" in outcome; +} + +/** The plain-words reason a definition.json's arktype error names, e.g. + * "definition.json is missing systemPrompt" or "definition.json's + * schedule must be a string". Field name comes from the error's path. */ +function definitionRejectionReason(errors: type.errors): string { + const first = errors[0]; + const field = first === undefined ? "value" : String(first.path.at(-1) ?? "value"); + if (first?.code === "required") return `definition.json is missing ${field}`; + return `definition.json's ${field} ${first?.problem ?? "is invalid"}`; +} + +/** Parses the two contract files' text into a package outcome. Returns + * `null` only when called on files that were never actually found — the + * caller decides that; this always parses given both texts. */ +function parsePackageFiles( + manifestText: string, + definitionText: string, +): DeployablePackage | { readonly reason: string } { + let manifestBody: unknown; + try { + manifestBody = JSON.parse(manifestText); + } catch (cause) { + reportError(cause, { operation: "chat_deployable_package_manifest_parse" }); + return { reason: "package.json is not valid JSON" }; + } + const manifest = PackageManifest(manifestBody); + if (manifest instanceof type.errors) { + return { reason: `package.json's ${String(manifest[0]?.path.at(-1) ?? "value")} is invalid` }; + } + + let definitionBody: unknown; try { - body = JSON.parse(definition.text); + definitionBody = JSON.parse(definitionText); } catch (cause) { - reportError(cause, { operation: "chat_deployable_package_parse" }); - return null; + reportError(cause, { operation: "chat_deployable_package_definition_parse" }); + return { reason: "definition.json is not valid JSON" }; + } + const definition = AgentDefinitionShape(definitionBody); + if (definition instanceof type.errors) { + return { reason: definitionRejectionReason(definition) }; + } + + const name = definition.name?.trim() || manifest.name?.trim(); + if (name === undefined || name === "") { + return { reason: "definition.json is missing name" }; } - const parsed = AgentDefinition(body); - if (parsed instanceof type.errors) return null; - if (parsed.name.trim() === "" || parsed.systemPrompt.trim() === "") return null; - return parsed; + if (definition.systemPrompt.trim() === "") { + return { reason: "definition.json is missing systemPrompt" }; + } + + return { + name, + systemPrompt: definition.systemPrompt, + ...(definition.description !== undefined ? { description: definition.description } : {}), + ...(definition.schedule !== undefined ? { schedule: definition.schedule } : {}), + }; +} + +/** The agent package a message's attachments describe: a parsed package, + * a rejection naming what's wrong, or `null` when the attachments aren't + * a package attempt at all (no package.json or no definition.json). + * Untrusted input: parsed, never cast. */ +export function deployablePackage(attachments: readonly MailAttachment[]): PackageOutcome { + const manifest = attachments.find((attachment) => attachment.name === PACKAGE_MANIFEST_NAME); + if (manifest === undefined) return null; + const definition = attachments.find((attachment) => attachment.name === AGENT_DEFINITION_NAME); + if (definition === undefined) return null; + return parsePackageFiles(manifest.text, definition.text); } /** Strips wrapping backticks/colon and returns the canonical file name the @@ -143,25 +214,19 @@ function findNamedFencedBlocks(lines: readonly string[]): Map a.start - b.start); const kept: string[] = []; @@ -175,20 +240,20 @@ export function deployablePackageFromBody( .join("\n") .replace(/\n{3,}/g, "\n\n") .trim(); - return { pkg: parsed, strippedBody }; + return { outcome, strippedBody }; } -/** The package a message describes and the body to render for it — +/** The package outcome a message describes and the body to render for it — * attachments win when present (the intended path once `mail_send` grows * attachments support), otherwise the body's fenced blocks are read and * stripped from the rendered text. */ export function resolveMessagePackage( attachments: readonly MailAttachment[], body: string, -): { readonly pkg: DeployablePackage | null; readonly renderedBody: string } { +): { readonly pkg: PackageOutcome; readonly renderedBody: string } { const fromAttachments = deployablePackage(attachments); if (fromAttachments !== null) return { pkg: fromAttachments, renderedBody: body }; const fromBody = deployablePackageFromBody(body); - if (fromBody !== null) return { pkg: fromBody.pkg, renderedBody: fromBody.strippedBody }; + if (fromBody !== null) return { pkg: fromBody.outcome, renderedBody: fromBody.strippedBody }; return { pkg: null, renderedBody: body }; } diff --git a/apps/web/src/chat/message-attachments.tsx b/apps/web/src/chat/message-attachments.tsx index 29ec6c3e0..0dfcf903a 100644 --- a/apps/web/src/chat/message-attachments.tsx +++ b/apps/web/src/chat/message-attachments.tsx @@ -5,10 +5,17 @@ import { Button } from "@corbits/react-ui"; import { cronSentence } from "@corbits/workflows/client"; +import { WarningCircle } from "@/lib/icons"; + import { useDeployAgentMutation } from "../agents-api"; import { chatPath } from "../chat-path"; import { Link } from "../navigation"; -import { isFiveFieldCron, type DeployablePackage } from "./deployable-package"; +import { + isFiveFieldCron, + isPackageRejection, + type DeployablePackage, + type PackageOutcome, +} from "./deployable-package"; import type { MailAttachment } from "./threads-api"; function errorText(cause: unknown): string { @@ -24,9 +31,10 @@ export function MessageAttachments({ readonly attachments: readonly MailAttachment[]; /** Resolved by the caller via `resolveMessagePackage` — attachments and * body-carried fenced blocks both land here. */ - readonly pkg: DeployablePackage | null; + readonly pkg: PackageOutcome; }) { if (pkg === null && attachments.length === 0) return null; + if (isPackageRejection(pkg)) return ; if (pkg === null) { return (