From aaa9e46065f99970668f1b8906567459d75e9e49 Mon Sep 17 00:00:00 2001 From: David Witwer Date: Sun, 30 Aug 2026 09:26:01 -0700 Subject: [PATCH 01/11] =?UTF-8?q?feat(harness):=20the=20harness=20creates?= =?UTF-8?q?=20the=20agent=20=E2=80=94=20POST=20/api/agents/scaffold=20[SAP?= =?UTF-8?q?-2981]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every create door in the Studio ended in an English sentence injected into a terminal asking the coding agent to call the scaffold MCP tool, so a failed create surfaced as a confused model and "did it work?" was answered by reading a terminal. This is the missing server-side create. It runs the same `scaffold` routine the MCP tool runs and refuses on its own findings, mirroring POST /api/agents/move: one plain segment for the name, a plain segment for the template (`resolveTemplate` joins it onto the bundled templates dir), a root matched against the same directory list the move route drops into, and an lstat of the destination. A failed scaffold removes the directory it created — a half-created agent is worse than a refusal. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_019fEqzA8eEEdQsmEKzLNnuf --- .../harness/src/core/agent-core-templates.ts | 22 ++ packages/harness/src/core/example-seed.ts | 15 +- packages/harness/src/server/index.ts | 53 +++ packages/harness/src/server/scaffold.test.ts | 346 ++++++++++++++++++ packages/harness/src/server/scaffold.ts | 288 +++++++++++++++ packages/harness/src/shared/types.ts | 2 + 6 files changed, 712 insertions(+), 14 deletions(-) create mode 100644 packages/harness/src/core/agent-core-templates.ts create mode 100644 packages/harness/src/server/scaffold.test.ts create mode 100644 packages/harness/src/server/scaffold.ts diff --git a/packages/harness/src/core/agent-core-templates.ts b/packages/harness/src/core/agent-core-templates.ts new file mode 100644 index 000000000..2f344e510 --- /dev/null +++ b/packages/harness/src/core/agent-core-templates.ts @@ -0,0 +1,22 @@ +/** + * Where `@sapiom/agent-core`'s bundled starter templates live on disk. + * + * One resolver, because there are now two callers that scaffold a real project + * — the demo seed (`core/example-seed.ts`) and `POST /api/agents/scaffold` — + * and both need the same two corrections. `scaffold()` takes `templatesDir` + * explicitly when its caller is ESM (there is no `__dirname` to resolve the + * bundled `templates/` from), and the packaged app needs the asar translation: + * `scaffold` COPIES the template with `cpSync`, which cannot `opendir` inside + * `app.asar` (ENOTDIR) no matter what Electron patches. + */ +import { createRequire } from "node:module"; +import * as path from "node:path"; + +import { unpackedPath } from "./asar-path.js"; + +const nodeRequire = createRequire(import.meta.url); + +export function agentCoreTemplatesDir(): string { + const entry = nodeRequire.resolve("@sapiom/agent-core"); + return unpackedPath(path.resolve(path.dirname(entry), "..", "..", "templates")); +} diff --git a/packages/harness/src/core/example-seed.ts b/packages/harness/src/core/example-seed.ts index 56b77405a..787a2bc25 100644 --- a/packages/harness/src/core/example-seed.ts +++ b/packages/harness/src/core/example-seed.ts @@ -36,7 +36,6 @@ import { execFileSync } from "node:child_process"; import { existsSync } from "node:fs"; import * as fs from "node:fs/promises"; -import { createRequire } from "node:module"; import * as path from "node:path"; import { @@ -47,23 +46,11 @@ import { type ResolvedVersions, } from "@sapiom/agent-core"; +import { agentCoreTemplatesDir } from "./agent-core-templates.js"; import { TEMPLATE_HTML, renderCanvasDocument } from "./canvas-template.js"; -const nodeRequire = createRequire(import.meta.url); - export const SAMPLE_PROJECT_NAME = "order-triage"; -/** Locate @sapiom/agent-core's bundled templates dir (no `__dirname` in ESM). */ -function agentCoreTemplatesDir(): string { - const entry = nodeRequire.resolve("@sapiom/agent-core"); - const dir = path.resolve(path.dirname(entry), "..", "..", "templates"); - // Embedded in Electron, require.resolve reports the app.asar (virtual) path; - // scaffold()'s cpSync can't opendir inside the asar archive (ENOTDIR), so - // point at the unpacked twin. No-op under the CLI (real filesystem path). - // The desktop host must asarUnpack node_modules (it unpacks all of them). - return dir.replace(/([\\/])app\.asar([\\/])/, "$1app.asar.unpacked$2"); -} - function tryGit(cwd: string, args: string[]): boolean { try { execFileSync("git", args, { cwd, stdio: "ignore", windowsHide: true }); diff --git a/packages/harness/src/server/index.ts b/packages/harness/src/server/index.ts index d21fa404e..6873c41c2 100644 --- a/packages/harness/src/server/index.ts +++ b/packages/harness/src/server/index.ts @@ -15,6 +15,7 @@ import { readFileSync } from "node:fs"; import { dirname, join, resolve, sep } from "node:path"; import { fileURLToPath } from "node:url"; import express, { type Express } from "express"; +import { scaffold } from "@sapiom/agent-core"; import { WebSocketServer } from "ws"; import open from "open"; @@ -87,6 +88,7 @@ import { removeGeneratedSessionDir, sweepGeneratedDirs, } from "../core/inject/retention.js"; +import { agentCoreTemplatesDir } from "../core/agent-core-templates.js"; import { CanvasWatcherManager } from "../core/canvas-watcher.js"; import { WorkspaceWatcherManager } from "../core/workspace-watcher.js"; import { InstallWatcherManager } from "../core/install-watcher.js"; @@ -150,6 +152,7 @@ import { moveTargetDirs, remapSessions, } from "./agent-move.js"; +import { createAgentScaffoldRouter } from "./scaffold.js"; import { createMacrosRouter } from "./macros.js"; import { createFsRouter } from "./fs.js"; import { createRunsRouter } from "./runs.js"; @@ -319,6 +322,7 @@ type WorkflowScanReason = | "session-create" | "workspace-change" | "agent-linked" + | "agent-created" | "agent-moved" | "graph-refresh" | "requested"; @@ -1731,6 +1735,55 @@ export const startServer = async ( }, }), ); + // SAP-2981: the harness CREATES the agent. Every create door used to end in + // an English sentence injected into a terminal asking the coding agent to + // call the scaffold MCP tool, so a failed create surfaced as a confused model + // and "did it work?" was answered by reading a terminal. The route runs the + // same `scaffold` routine that tool runs, and its guards live in the module: + // one plain segment for the name, a plain segment for the template (which + // `resolveTemplate` JOINS onto the bundled templates dir), and a root matched + // against the SAME directory list the move route drops into — so "a folder + // the rail can show" and "a folder the studio will create a project in" stay + // one answer. + app.use( + createAgentScaffoldRouter({ + listProjectDirs: async () => { + const stored = await loadSettings(statePaths.settings); + return moveTargetDirs( + [ + ...stored.recentDirs, + ...(stored.projectRoot ? [stored.projectRoot] : []), + ...sessionManager.list().map((session) => session.cwd), + ], + workflowsCache.map((w) => w.path), + ); + }, + resolveAgent: (agentPath) => + workflowsCache.find((w) => resolve(w.path) === agentPath) ?? null, + scaffoldAgent: async ({ targetDir, template }) => { + // `installDependencies: true` for the same reason the MCP tool passes + // it: the Canvas bundles the project on its first, unprompted render + // and resolves `@sapiom/agent`/`zod` from the project's own + // node_modules, so a never-installed agent opens on a "Could not + // resolve …" error. Best-effort inside agent-core — a failed install + // still returns a created project. + const result = await scaffold({ + targetDir, + template, + templatesDir: agentCoreTemplatesDir(), + installDependencies: true, + }); + return { dependenciesInstalled: result.dependenciesInstalled }; + }, + // Rescan the PROJECT root, not the agent directory: the registry has to + // learn the new agent under the project the rail draws it in, and the + // scan broadcasts `workflows.changed` so the row is there before the + // dialog's caller opens a session on it. + onScaffolded: async (agentDir) => { + await scanWorkflowsAndBroadcast(dirname(agentDir), "agent-created"); + }, + }), + ); app.use( createWorkflowsRouter(enrichedWorkflowRegistry), createFsRouter(), diff --git a/packages/harness/src/server/scaffold.test.ts b/packages/harness/src/server/scaffold.test.ts new file mode 100644 index 000000000..bee77c0be --- /dev/null +++ b/packages/harness/src/server/scaffold.test.ts @@ -0,0 +1,346 @@ +/** + * The scaffold endpoint, on a real filesystem (SAP-2981). + * + * Every test here goes STRAIGHT AT THE ROUTE — no dialog, no client. The whole + * point of the endpoint is that it refuses on its own findings: a name field + * that disables its own submit button proves nothing about a macro, a curl, or + * a future keyboard path that reaches the route another way. So the refusals + * are posted directly, and the filesystem is inspected afterwards, because + * "refused" and "refused without leaving a half-created agent behind" are + * different claims and only the second one is worth having. + * + * The scaffold routine itself is injected: what is under test is the guard set + * and the ordering, not `@sapiom/agent-core`'s template copy. The one test that + * cares about disk state stubs a scaffold that creates the directory and then + * throws, which is exactly the failure the cleanup exists for. + */ +import { describe, expect, it, beforeEach, afterEach } from "vitest"; +import express from "express"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; + +import { + createAgentScaffoldRouter, + refuseAgentName, + refuseScaffoldOnDisk, + type AgentScaffoldResponse, +} from "./scaffold.js"; + +let tmp: string; + +beforeEach(async () => { + // realpath: macOS hands out `/var/folders/…`, a symlink to `/private/var/…`, + // and the route compares the request's resolved root against the list's. + tmp = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), "agent-scaffold-"))); +}); + +afterEach(async () => { + await fs.rm(tmp, { recursive: true, force: true }); +}); + +interface ScaffoldCall { + targetDir: string; + template: string; +} + +/** A live express app over the router, so the route is exercised as a route. */ +async function serve( + options: { + agents?: string[]; + projectDirs?: string[]; + scaffoldAgent?: (call: ScaffoldCall) => Promise<{ dependenciesInstalled: boolean }>; + onScaffolded?: (dir: string) => Promise; + } = {}, +): Promise<{ + calls: ScaffoldCall[]; + scaffolded: string[]; + post: (body: unknown) => Promise<{ status: number; body: any }>; + close: () => Promise; +}> { + const calls: ScaffoldCall[] = []; + const scaffolded: string[] = []; + const agents = options.agents ?? []; + const app = express(); + app.use(express.json()); + app.use( + createAgentScaffoldRouter({ + listProjectDirs: () => options.projectDirs ?? [tmp], + resolveAgent: (agentPath) => + agents.includes(agentPath) + ? { name: path.basename(agentPath), path: agentPath } + : null, + scaffoldAgent: async (call) => { + calls.push(call); + if (options.scaffoldAgent) return await options.scaffoldAgent(call); + // The default stand-in does what the real one does first: make the + // directory. A guard that only looks like it fired because nothing was + // ever created is not a guard. + await fs.mkdir(call.targetDir, { recursive: true }); + await fs.writeFile( + path.join(call.targetDir, "sapiom.json"), + JSON.stringify({ name: path.basename(call.targetDir) }), + ); + return { dependenciesInstalled: false }; + }, + onScaffolded: async (dir) => { + scaffolded.push(dir); + await options.onScaffolded?.(dir); + }, + }), + ); + const server = app.listen(0); + await new Promise((resolve) => server.once("listening", resolve)); + const address = server.address(); + const port = typeof address === "object" && address !== null ? address.port : 0; + return { + calls, + scaffolded, + post: async (body) => { + const res = await fetch(`http://127.0.0.1:${port}/api/agents/scaffold`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + }); + return { status: res.status, body: (await res.json()) as any }; + }, + close: () => new Promise((resolve) => server.close(() => resolve())), + }; +} + +const exists = async (p: string): Promise => + await fs + .lstat(p) + .then(() => true) + .catch(() => false); + +describe("refuseAgentName", () => { + it("accepts an ordinary agent folder name", () => { + expect(refuseAgentName("order-triage")).toBeNull(); + expect(refuseAgentName("Order_Triage2")).toBeNull(); + }); + + it("refuses the shapes that would escape the project", () => { + // Each of these is a directory the caller must not be able to name, and + // each carries its own sentence because the user reads it verbatim. + expect(refuseAgentName("../evil")).toMatch(/one folder name/); + expect(refuseAgentName("a/b")).toMatch(/one folder name/); + expect(refuseAgentName("..")).toMatch(/dot/); + expect(refuseAgentName(".hidden")).toMatch(/dot/); + expect(refuseAgentName("")).toMatch(/Give the agent a name/); + expect(refuseAgentName(" ")).toMatch(/Give the agent a name/); + expect(refuseAgentName(" leading")).toMatch(/space/); + expect(refuseAgentName(42)).toMatch(/Give the agent a name/); + expect(refuseAgentName("x".repeat(65))).toMatch(/too long/); + }); + + it("refuses a NUL, which reaches fs as a throw rather than a refusal", () => { + expect(refuseAgentName("ok\u0000name")).toMatch(/isn't a folder name/); + }); +}); + +describe("refuseScaffoldOnDisk", () => { + it("passes an absent destination and refuses anything already there", async () => { + expect(await refuseScaffoldOnDisk(path.join(tmp, "fresh"), "proj")).toBeNull(); + await fs.mkdir(path.join(tmp, "taken")); + expect(await refuseScaffoldOnDisk(path.join(tmp, "taken"), "proj")).toMatch( + /already contains taken/, + ); + }); + + it("refuses a DANGLING symlink — lstat, not stat", async () => { + // `stat` follows the link, finds nothing, and would report the destination + // as free; the scaffold would then write through a link the user placed. + await fs.symlink(path.join(tmp, "nowhere"), path.join(tmp, "link")); + expect(await refuseScaffoldOnDisk(path.join(tmp, "link"), "proj")).toMatch( + /already contains link/, + ); + }); +}); + +describe("POST /api/agents/scaffold", () => { + it("creates the agent inside the project and reports its path", async () => { + const srv = await serve(); + try { + const res = await srv.post({ root: tmp, name: "billing-bot" }); + expect(res.status).toBe(200); + const body = res.body as AgentScaffoldResponse; + expect(body.path).toBe(path.join(tmp, "billing-bot")); + expect(body.name).toBe("billing-bot"); + expect(body.template).toBe("default"); + expect(srv.calls).toEqual([ + { targetDir: path.join(tmp, "billing-bot"), template: "default" }, + ]); + expect(await exists(path.join(tmp, "billing-bot", "sapiom.json"))).toBe(true); + } finally { + await srv.close(); + } + }); + + it("rescans BEFORE it answers, so the rail has the agent before the caller acts", async () => { + const order: string[] = []; + const srv = await serve({ + onScaffolded: async (dir) => { + // A REAL tick before the push, deliberately: a synchronous stub records + // "scanned" first even when the route forgets to await, so the + // assertion would survive the very mutation it exists to catch. + await new Promise((resolve) => setTimeout(resolve, 10)); + order.push(`scanned:${path.basename(dir)}`); + }, + }); + try { + const res = await srv.post({ root: tmp, name: "late" }); + order.push(`answered:${res.status}`); + expect(order).toEqual(["scanned:late", "answered:200"]); + } finally { + await srv.close(); + } + }); + + it("passes the chosen template through", async () => { + const srv = await serve(); + try { + const res = await srv.post({ root: tmp, name: "paused", template: "coding-pause" }); + expect(res.status).toBe(200); + expect(srv.calls[0].template).toBe("coding-pause"); + } finally { + await srv.close(); + } + }); + + it("refuses a name that is not one folder segment — and never calls scaffold", async () => { + const srv = await serve(); + try { + for (const name of ["../evil", "a/b", "", "..", ".hidden"]) { + const res = await srv.post({ root: tmp, name }); + expect(res.status).toBe(400); + expect(typeof res.body.error).toBe("string"); + } + // The escape the name guard exists for: nothing landed outside the root. + expect(await exists(path.join(path.dirname(tmp), "evil"))).toBe(false); + expect(srv.calls).toEqual([]); + } finally { + await srv.close(); + } + }); + + it("refuses a template that is not a plain segment", async () => { + // `resolveTemplate` JOINS this onto the bundled templates dir, so an + // unguarded value names any directory on the machine as the thing to copy. + const srv = await serve(); + try { + for (const template of ["../../../etc", "a/b", "", 7]) { + const res = await srv.post({ root: tmp, name: "agent", template }); + expect(res.status).toBe(400); + } + expect(srv.calls).toEqual([]); + } finally { + await srv.close(); + } + }); + + it("refuses a root the studio does not show as a project", async () => { + // The barrier is the LIST, not the string: an absolute, traversal-free, + // perfectly real directory is still refused when the rail can't show it. + const outside = path.join(tmp, "outside"); + await fs.mkdir(outside); + const srv = await serve({ projectDirs: [path.join(tmp, "known")] }); + try { + const res = await srv.post({ root: outside, name: "sneaky" }); + expect(res.status).toBe(409); + expect(res.body.error).toMatch(/doesn't show that folder as a project/); + expect(await exists(path.join(outside, "sneaky"))).toBe(false); + expect(srv.calls).toEqual([]); + } finally { + await srv.close(); + } + }); + + it("refuses a relative or traversing root", async () => { + const srv = await serve(); + try { + for (const root of ["relative/path", `${tmp}/../${path.basename(tmp)}`, 5, null]) { + const res = await srv.post({ root, name: "agent" }); + expect(res.status).toBe(400); + } + expect(srv.calls).toEqual([]); + } finally { + await srv.close(); + } + }); + + it("refuses a duplicate name — and leaves the existing agent untouched", async () => { + const existing = path.join(tmp, "twin"); + await fs.mkdir(existing); + await fs.writeFile(path.join(existing, "index.ts"), "// the original\n"); + const srv = await serve({ agents: [existing] }); + try { + const res = await srv.post({ root: tmp, name: "twin" }); + expect(res.status).toBe(409); + expect(res.body.error).toMatch(/already has an agent called twin/); + expect(srv.calls).toEqual([]); + expect(await fs.readFile(path.join(existing, "index.ts"), "utf8")).toBe( + "// the original\n", + ); + } finally { + await srv.close(); + } + }); + + it("refuses a plain directory in the way, which the registry cannot see", async () => { + // The registry knows nothing about a folder with no agent in it, so only a + // real `lstat` answers this one — and scaffolding into it would fail deep + // inside agent-core instead of here, with a directory the user then owns. + await fs.mkdir(path.join(tmp, "notes")); + const srv = await serve(); + try { + const res = await srv.post({ root: tmp, name: "notes" }); + expect(res.status).toBe(409); + expect(res.body.error).toMatch(/already contains notes/); + expect(srv.calls).toEqual([]); + } finally { + await srv.close(); + } + }); + + it("leaves NOTHING on disk when the scaffold fails part-way", async () => { + // The real `scaffold` makes the directory and then copies into it, so a + // template failure mid-copy leaves a folder the user never made — and the + // retry then dies on "already contains", which is the worse of the two + // failures. + const srv = await serve({ + scaffoldAgent: async ({ targetDir }) => { + await fs.mkdir(targetDir, { recursive: true }); + await fs.writeFile(path.join(targetDir, "half"), "written"); + throw new Error("template copy blew up"); + }, + }); + try { + const res = await srv.post({ root: tmp, name: "doomed" }); + expect(res.status).toBe(500); + expect(res.body.error).toMatch(/template copy blew up/); + expect(await exists(path.join(tmp, "doomed"))).toBe(false); + // Nothing was announced either: the rail must not be told about an agent + // that does not exist. + expect(srv.scaffolded).toEqual([]); + } finally { + await srv.close(); + } + }); + + it("creates in a nested project directory the rail shows, using the LIST's spelling", async () => { + // The move route's rule, applied here: the directory that gets written into + // is the one from the list, so a request may not smuggle a different + // spelling of it past the match. + const nested = path.join(tmp, "systems", "payments"); + await fs.mkdir(nested, { recursive: true }); + const srv = await serve({ projectDirs: [nested] }); + try { + const res = await srv.post({ root: `${nested}/`, name: "refunds" }); + expect(res.status).toBe(200); + expect((res.body as AgentScaffoldResponse).path).toBe(path.join(nested, "refunds")); + } finally { + await srv.close(); + } + }); +}); diff --git a/packages/harness/src/server/scaffold.ts b/packages/harness/src/server/scaffold.ts new file mode 100644 index 000000000..c149b8440 --- /dev/null +++ b/packages/harness/src/server/scaffold.ts @@ -0,0 +1,288 @@ +/** + * `POST /api/agents/scaffold` — the harness creates the agent (SAP-2981; + * design.md § E4). + * + * THE HARNESS CREATES THE AGENT. Every create door in the Studio used to end in + * an English sentence injected into a terminal — "call the + * sapiom_dev_agents_scaffold tool with {…}" — and the error copy admitted it + * ("Ask the coding agent to call sapiom_dev_agents_scaffold"). A creation + * mechanism that is a prompt has no outcome the app can read: a failed scaffold + * surfaces as a confused model rather than an error, and "did it work?" is + * answered by reading a terminal. This route is the outcome, so the dialog can + * report a refusal and the rail can show the agent before any chat starts. + * + * It runs the SAME routine the MCP tool runs (`scaffold` from + * `@sapiom/agent-core`, injected), so the two creation paths cannot drift into + * producing different projects. + * + * REFUSES ON ITS OWN FINDINGS, like `POST /api/agents/move` beside it — the + * closest existing precedent for a harness-owned filesystem mutation, and the + * shape this route copies deliberately: + * + * - `name` must be ONE plain directory segment (`childPath`), so `../evil`, + * `a/b`, "" and `.` are refused here rather than only being disabled in a + * dialog. The co-located test posts them directly, dialog bypassed. + * - `template` must be a plain segment too. `resolveTemplate` joins it onto + * the bundled templates dir, so an unguarded `../../..` would name any + * directory on the machine as the thing to copy. + * - `root` is matched against the directories the RAIL CAN SHOW and the + * match from THAT LIST is what the scaffold writes into — the request's + * spelling is discarded. A folder the studio has never been pointed at is + * not a folder this route creates projects in, so the endpoint cannot be + * turned into an arbitrary-path mkdir by a caller that never opened the + * rail. No request string reaches `fs`, which is a barrier a static + * analyzer can see and a reordered `if` cannot undo. + * - the destination is STAT'd. A name already taken in that project is a + * refusal, whether the thing sitting there is a registered agent or a + * plain directory the registry knows nothing about — only a real `lstat` + * answers the second one. + * + * AND IT LEAVES NOTHING BEHIND. `scaffold` makes the directory before it copies + * into it, so a template failure mid-copy would otherwise leave a half-created + * agent on disk — worse than a refusal, because the retry then fails on "name + * already exists" and the user has to clean up a folder they never made. A + * failed scaffold removes the directory this route created. + * + * Mounted under the same `/api` boot-token middleware as the rest of the REST + * surface (server/index.ts). + */ +import * as fs from "node:fs/promises"; +import * as path from "node:path"; +import { Router, type Router as ExpressRouter } from "express"; + +import { childPath, hasTraversalSegment } from "../core/path-safety.js"; + +/** `POST /api/agents/scaffold` response. */ +export interface AgentScaffoldResponse { + ok: true; + /** Absolute path of the new agent's directory — server-authored, never the + * caller's string. */ + path: string; + name: string; + template: string; + /** Whether the best-effort `npm install` succeeded. False is not a failure: + * the Canvas degrades to its "run npm install" hint (see + * `@sapiom/agent-core`'s install-deps). */ + dependenciesInstalled: boolean; +} + +export interface AgentScaffoldDeps { + /** + * Every directory an agent may be created IN: the project roots the studio + * knows about, plus the branching directories the Project axis renders + * between a root and an agent. Same list `agent-move.ts` moves into + * (`moveTargetDirs`), and for the same reason — "a directory the rail can + * show" and "a directory this route will write into" must be one list, or + * creation and drag disagree about what a project is. + */ + listProjectDirs: () => string[] | Promise; + /** + * The registered agent at this absolute path, or null. Consulted before the + * `lstat` purely so the refusal can NAME what is in the way ("an agent + * called x") instead of describing a directory. + */ + resolveAgent: (agentPath: string) => { name: string; path: string } | null; + /** + * Creates the project. Injected so the co-located test can exercise every + * guard without npm, git, or the registry on the far side of them — and so + * the route stays the thing under test rather than `@sapiom/agent-core`. + */ + scaffoldAgent: (opts: { + targetDir: string; + template: string; + }) => Promise<{ dependenciesInstalled: boolean }>; + /** + * Applied AFTER the project is on disk, BEFORE the response. The + * integrator's job: rescan the project root so the registry holds the new + * agent and `workflows.changed` is broadcast. It runs before the response + * because that ordering IS the criterion — the agent is in the rail before + * the caller can open a session on it, so "did it work?" is never a question + * the user answers by reading a terminal. + */ + onScaffolded: (agentDir: string) => Promise; +} + +/** + * A template name that is safe to hand to `resolveTemplate`, which joins it + * onto the bundled templates directory. Plain segment, no separators, no dots: + * a bundled template is a directory name in a package we ship, and nothing + * legitimate needs more than this alphabet. + */ +const TEMPLATE_NAME = /^[a-z0-9][a-z0-9-]*$/i; + +/** + * Two paths naming one directory — trailing separators and (on Windows) case + * are spelling, not identity. Same rule `agent-move.ts` and `studio-rail.ts` + * apply to their roots, and for the same reason: the client stores whichever + * form the user typed while the server stores whatever it resolved. + */ +function samePath(a: string, b: string): boolean { + const norm = (p: string): string => { + const resolved = path.resolve(p); + return process.platform === "win32" ? resolved.toLowerCase() : resolved; + }; + return norm(a) === norm(b); +} + +/** + * Characters that cannot appear in a directory name on every platform the + * Studio runs on, plus the C0 controls. A NUL in particular reaches `fs` as a + * thrown `ERR_INVALID_ARG_VALUE` rather than a refusal, so it is answered here + * where the caller gets a sentence instead of a 500. + */ +// eslint-disable-next-line no-control-regex +const FORBIDDEN_IN_NAME = /[\u0000-\u001f<>:"|?*]/; + +/** + * Why this name cannot be a new agent's folder, or null when it can. + * + * The message is shown verbatim in the dialog, so it says what to do rather + * than what a regex thinks. The rule is `childPath`'s — one plain segment + * under the project root, enforced again at the join below — plus a length cap + * and a leading-dot refusal: a dotted directory is hidden from the agent scan, + * so `.notes` would scaffold successfully and then never appear in the rail, + * which is the "did it work?" failure this whole endpoint exists to end. + */ +export function refuseAgentName(name: unknown): string | null { + if (typeof name !== "string" || name.trim() === "") + return "Give the agent a name."; + if (name.trim() !== name) + return "An agent name can't start or end with a space."; + if (name.length > 64) + return "That name is too long — keep it under 64 characters."; + if (/[/\\]/.test(name)) + return "An agent name is one folder name — it can't contain / or \\."; + if (name.startsWith(".")) + return "An agent name can't start with a dot — a dotted folder is hidden from the rail."; + if (FORBIDDEN_IN_NAME.test(name)) return `'${name}' isn't a folder name.`; + return null; +} + +/** + * The route's OWN refusal, from the filesystem rather than from the registry — + * a message when the directory must not be created, null when it may. + * + * `lstat`, not `stat`: a dangling symlink sitting at the destination is still + * something the user put there, and `scaffold` would happily create through it. + */ +export async function refuseScaffoldOnDisk( + target: string, + projectLabel: string, +): Promise { + const name = path.basename(target); + try { + await fs.lstat(target); + return `${projectLabel} already contains ${name}.`; + } catch { + // Absent — the only acceptable state for a destination. + } + return null; +} + +/** + * POST /api/agents/scaffold { root, name, template? } -> AgentScaffoldResponse + * + * 400 — a malformed body, a name that is not one folder segment, a template + * that is not a plain segment, a root that is not an absolute path. + * 409 — a refusal, with the reason in `error` so the dialog shows it verbatim: + * a root the studio doesn't show as a project, or a name already taken there. + * 500 — the scaffold itself failed; the directory it may have created is + * removed first, so the retry meets the same clean state the first attempt did. + */ +export function createAgentScaffoldRouter( + deps: AgentScaffoldDeps, +): ExpressRouter { + const router = Router(); + + router.post("/api/agents/scaffold", async (req, res, next) => { + const body = (req.body ?? {}) as { + root?: unknown; + name?: unknown; + template?: unknown; + }; + const { root, name } = body; + const template = body.template ?? "default"; + + if (typeof root !== "string" || !path.isAbsolute(root) || hasTraversalSegment(root)) { + res.status(400).json({ error: "root must be an absolute path" }); + return; + } + const nameRefusal = refuseAgentName(name); + if (nameRefusal != null) { + res.status(400).json({ error: nameRefusal }); + return; + } + if (typeof template !== "string" || !TEMPLATE_NAME.test(template)) { + res.status(400).json({ error: `Unknown template '${String(template)}'.` }); + return; + } + + try { + // THE DESTINATION BARRIER, identical in shape to the move route's: the + // requested root is matched against the directories the rail can show, + // and the DIRECTORY FROM THAT LIST is what the scaffold writes into. + const requested = path.resolve(root); + const projectDir = (await deps.listProjectDirs()).find( + (dir) => + typeof dir === "string" && dir.trim() !== "" && samePath(dir, requested), + ); + if (projectDir == null) { + res.status(409).json({ + error: `Can't create an agent in ${requested} — Studio doesn't show that folder as a project.`, + }); + return; + } + const projectLabel = path.basename(path.resolve(projectDir)) || projectDir; + // `path.resolve` on the LIST's entry, not the request's, and `childPath` + // re-derives the join it already blessed above: the guard that produces + // the path is the guard that proved it, so no later edit can separate + // them. + const target = childPath(path.resolve(projectDir), name as string); + if (target == null) { + res.status(400).json({ error: `'${String(name)}' isn't a folder name.` }); + return; + } + + const existing = deps.resolveAgent(target); + if (existing != null) { + res.status(409).json({ + error: `${projectLabel} already has an agent called ${existing.name}.`, + }); + return; + } + const diskRefusal = await refuseScaffoldOnDisk(target, projectLabel); + if (diskRefusal != null) { + res.status(409).json({ error: diskRefusal }); + return; + } + + let result: { dependenciesInstalled: boolean }; + try { + result = await deps.scaffoldAgent({ targetDir: target, template }); + } catch (err) { + // NOTHING HALF-CREATED. Everything above proved the destination was + // absent, so whatever is there now is this attempt's own wreckage. + await fs.rm(target, { recursive: true, force: true }).catch(() => {}); + res.status(500).json({ + error: (err as Error).message || `Couldn't create ${String(name)}.`, + }); + return; + } + + // Before the response, deliberately: the agent is in the rail by the time + // the caller can act on the result. + await deps.onScaffolded(target); + res.json({ + ok: true, + path: target, + name: path.basename(target), + template, + dependenciesInstalled: result.dependenciesInstalled, + } satisfies AgentScaffoldResponse); + } catch (err) { + next(err); + } + }); + + return router; +} diff --git a/packages/harness/src/shared/types.ts b/packages/harness/src/shared/types.ts index cb2a503a5..54a2b208f 100644 --- a/packages/harness/src/shared/types.ts +++ b/packages/harness/src/shared/types.ts @@ -987,6 +987,8 @@ export interface SessionRecord { // POST /api/sessions/:id/input InjectInputRequest → { ok: true } // POST /api/sessions/:id/attachments AttachFileRequest → AttachFileResponse (materialize only) // PATCH /api/sessions/:id/workflow BindWorkflowRequest → HarnessSession +// POST /api/agents/scaffold { root, name, template? } → AgentScaffoldResponse (the harness creates the agent) +// POST /api/agents/move { from, to } → AgentMoveResponse (rename an agent's directory) // GET /api/workflows → WorkflowInfo[] // POST /api/workflows/connect { path } → WorkflowInfo // POST /api/workflows/scan { root } → WorkflowInfo[] From 4ab50d08606fd47524d678d8097a89ff1bb6c7ea Mon Sep 17 00:00:00 2001 From: David Witwer Date: Sun, 30 Aug 2026 09:34:59 -0700 Subject: [PATCH 02/11] feat(harness): a create dialog that creates, then chats [SAP-2981] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The project + opens a dialog: a name, a starter, and the project it lands in stated rather than chosen — you clicked that row, that is the answer. Submit scaffolds through POST /api/agents/scaffold, the agent joins the rail, and only then does a session open on it with the optional first instruction. Collapses the three copies of the injected scaffold sentence to one: the composer keeps a prompt (no project, no name, a folder that does not exist yet), while the project +, the empty-project CTA, the bare-project affordance and the bundled starters in the template gallery all go through the endpoint. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_019fEqzA8eEEdQsmEKzLNnuf --- packages/harness/src/server/scaffold.test.ts | 33 +-- packages/harness/src/server/scaffold.ts | 49 +--- .../harness/src/shared/agent-name.test.ts | 35 +++ packages/harness/src/shared/agent-name.ts | 52 ++++ packages/harness/src/shared/types.ts | 18 ++ packages/harness/vitest.config.ts | 1 + packages/harness/web/src/App.tsx | 212 ++++++++++++-- .../web/src/components/CreateAgentDialog.tsx | 275 ++++++++++++++++++ .../web/src/components/WorkflowsRail.tsx | 55 ++-- packages/harness/web/src/lib/api.ts | 79 +++++ packages/harness/web/src/lib/templates.ts | 47 ++- .../harness/web/src/lib/use-harness-state.ts | 38 +++ packages/harness/web/src/styles.css | 106 +++++++ packages/harness/web/tsconfig.json | 1 + packages/harness/web/vite.config.ts | 3 + 15 files changed, 859 insertions(+), 145 deletions(-) create mode 100644 packages/harness/src/shared/agent-name.test.ts create mode 100644 packages/harness/src/shared/agent-name.ts create mode 100644 packages/harness/web/src/components/CreateAgentDialog.tsx diff --git a/packages/harness/src/server/scaffold.test.ts b/packages/harness/src/server/scaffold.test.ts index bee77c0be..545f08ef7 100644 --- a/packages/harness/src/server/scaffold.test.ts +++ b/packages/harness/src/server/scaffold.test.ts @@ -20,12 +20,8 @@ import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; -import { - createAgentScaffoldRouter, - refuseAgentName, - refuseScaffoldOnDisk, - type AgentScaffoldResponse, -} from "./scaffold.js"; +import type { AgentScaffoldResponse } from "../shared/types.js"; +import { createAgentScaffoldRouter, refuseScaffoldOnDisk } from "./scaffold.js"; let tmp: string; @@ -114,31 +110,6 @@ const exists = async (p: string): Promise => .then(() => true) .catch(() => false); -describe("refuseAgentName", () => { - it("accepts an ordinary agent folder name", () => { - expect(refuseAgentName("order-triage")).toBeNull(); - expect(refuseAgentName("Order_Triage2")).toBeNull(); - }); - - it("refuses the shapes that would escape the project", () => { - // Each of these is a directory the caller must not be able to name, and - // each carries its own sentence because the user reads it verbatim. - expect(refuseAgentName("../evil")).toMatch(/one folder name/); - expect(refuseAgentName("a/b")).toMatch(/one folder name/); - expect(refuseAgentName("..")).toMatch(/dot/); - expect(refuseAgentName(".hidden")).toMatch(/dot/); - expect(refuseAgentName("")).toMatch(/Give the agent a name/); - expect(refuseAgentName(" ")).toMatch(/Give the agent a name/); - expect(refuseAgentName(" leading")).toMatch(/space/); - expect(refuseAgentName(42)).toMatch(/Give the agent a name/); - expect(refuseAgentName("x".repeat(65))).toMatch(/too long/); - }); - - it("refuses a NUL, which reaches fs as a throw rather than a refusal", () => { - expect(refuseAgentName("ok\u0000name")).toMatch(/isn't a folder name/); - }); -}); - describe("refuseScaffoldOnDisk", () => { it("passes an absent destination and refuses anything already there", async () => { expect(await refuseScaffoldOnDisk(path.join(tmp, "fresh"), "proj")).toBeNull(); diff --git a/packages/harness/src/server/scaffold.ts b/packages/harness/src/server/scaffold.ts index c149b8440..1e8fb0b9e 100644 --- a/packages/harness/src/server/scaffold.ts +++ b/packages/harness/src/server/scaffold.ts @@ -50,21 +50,10 @@ import * as fs from "node:fs/promises"; import * as path from "node:path"; import { Router, type Router as ExpressRouter } from "express"; +import { refuseAgentName } from "../shared/agent-name.js"; +import type { AgentScaffoldResponse } from "../shared/types.js"; import { childPath, hasTraversalSegment } from "../core/path-safety.js"; -/** `POST /api/agents/scaffold` response. */ -export interface AgentScaffoldResponse { - ok: true; - /** Absolute path of the new agent's directory — server-authored, never the - * caller's string. */ - path: string; - name: string; - template: string; - /** Whether the best-effort `npm install` succeeded. False is not a failure: - * the Canvas degrades to its "run npm install" hint (see - * `@sapiom/agent-core`'s install-deps). */ - dependenciesInstalled: boolean; -} export interface AgentScaffoldDeps { /** @@ -124,40 +113,6 @@ function samePath(a: string, b: string): boolean { return norm(a) === norm(b); } -/** - * Characters that cannot appear in a directory name on every platform the - * Studio runs on, plus the C0 controls. A NUL in particular reaches `fs` as a - * thrown `ERR_INVALID_ARG_VALUE` rather than a refusal, so it is answered here - * where the caller gets a sentence instead of a 500. - */ -// eslint-disable-next-line no-control-regex -const FORBIDDEN_IN_NAME = /[\u0000-\u001f<>:"|?*]/; - -/** - * Why this name cannot be a new agent's folder, or null when it can. - * - * The message is shown verbatim in the dialog, so it says what to do rather - * than what a regex thinks. The rule is `childPath`'s — one plain segment - * under the project root, enforced again at the join below — plus a length cap - * and a leading-dot refusal: a dotted directory is hidden from the agent scan, - * so `.notes` would scaffold successfully and then never appear in the rail, - * which is the "did it work?" failure this whole endpoint exists to end. - */ -export function refuseAgentName(name: unknown): string | null { - if (typeof name !== "string" || name.trim() === "") - return "Give the agent a name."; - if (name.trim() !== name) - return "An agent name can't start or end with a space."; - if (name.length > 64) - return "That name is too long — keep it under 64 characters."; - if (/[/\\]/.test(name)) - return "An agent name is one folder name — it can't contain / or \\."; - if (name.startsWith(".")) - return "An agent name can't start with a dot — a dotted folder is hidden from the rail."; - if (FORBIDDEN_IN_NAME.test(name)) return `'${name}' isn't a folder name.`; - return null; -} - /** * The route's OWN refusal, from the filesystem rather than from the registry — * a message when the directory must not be created, null when it may. diff --git a/packages/harness/src/shared/agent-name.test.ts b/packages/harness/src/shared/agent-name.test.ts new file mode 100644 index 000000000..8a187bb76 --- /dev/null +++ b/packages/harness/src/shared/agent-name.test.ts @@ -0,0 +1,35 @@ +/** + * The one agent-name rule (SAP-2981). + * + * It is tested here rather than beside either consumer because both the dialog + * and `POST /api/agents/scaffold` depend on it saying the SAME thing: a name + * the field accepts and the route refuses reads as a broken app. + */ +import { describe, expect, it } from "vitest"; + +import { refuseAgentName } from "./agent-name.js"; + +describe("refuseAgentName", () => { + it("accepts an ordinary agent folder name", () => { + expect(refuseAgentName("order-triage")).toBeNull(); + expect(refuseAgentName("Order_Triage2")).toBeNull(); + }); + + it("refuses the shapes that would escape the project", () => { + // Each of these is a directory the caller must not be able to name, and + // each carries its own sentence because the user reads it verbatim. + expect(refuseAgentName("../evil")).toMatch(/one folder name/); + expect(refuseAgentName("a/b")).toMatch(/one folder name/); + expect(refuseAgentName("..")).toMatch(/dot/); + expect(refuseAgentName(".hidden")).toMatch(/dot/); + expect(refuseAgentName("")).toMatch(/Give the agent a name/); + expect(refuseAgentName(" ")).toMatch(/Give the agent a name/); + expect(refuseAgentName(" leading")).toMatch(/space/); + expect(refuseAgentName(42)).toMatch(/Give the agent a name/); + expect(refuseAgentName("x".repeat(65))).toMatch(/too long/); + }); + + it("refuses a NUL, which reaches fs as a throw rather than a refusal", () => { + expect(refuseAgentName("ok\u0000name")).toMatch(/isn't a folder name/); + }); +}); diff --git a/packages/harness/src/shared/agent-name.ts b/packages/harness/src/shared/agent-name.ts new file mode 100644 index 000000000..c81a9a76c --- /dev/null +++ b/packages/harness/src/shared/agent-name.ts @@ -0,0 +1,52 @@ +/** + * What a new agent may be called — ONE rule, shared by the dialog and the + * route (SAP-2981). + * + * The dialog validates as you type so the refusal arrives before the click, and + * `POST /api/agents/scaffold` refuses again on its own findings because a + * disabled button is not a permission system. Those are two guards, and two + * guards written twice become two different rules: a name the field accepts and + * the server rejects reads as a broken app, and a name the server would accept + * but the field greys out reads as an arbitrary one. So the rule lives here and + * both sides import it. + * + * It is deliberately a folder-name rule, not a naming-convention one. The agent + * gets a directory inside the project, and everything refused below is refused + * because of what it would do to that directory — not because of house style. + */ + +/** + * Characters that cannot appear in a directory name on every platform the + * Studio runs on, plus the C0 controls. A NUL in particular reaches `fs` as a + * thrown `ERR_INVALID_ARG_VALUE` rather than a refusal, so it is answered here + * where the caller gets a sentence instead of a 500. + */ +// eslint-disable-next-line no-control-regex +const FORBIDDEN_IN_NAME = /[\u0000-\u001f<>:"|?*]/; + +/** Longest agent folder name accepted — a bound, not a style rule. */ +const MAX_NAME_LENGTH = 64; + +/** + * Why this name cannot be a new agent's folder, or null when it can. + * + * The message is shown verbatim, so each sentence says what to do rather than + * what a regex thinks. The leading-dot refusal is the one that is not about + * path escape: a dotted directory is skipped by the agent scan, so `.notes` + * would scaffold successfully and then never appear in the rail — the exact + * "did it work?" failure the create endpoint exists to end. + */ +export function refuseAgentName(name: unknown): string | null { + if (typeof name !== "string" || name.trim() === "") + return "Give the agent a name."; + if (name.trim() !== name) + return "An agent name can't start or end with a space."; + if (name.length > MAX_NAME_LENGTH) + return `That name is too long — keep it under ${MAX_NAME_LENGTH} characters.`; + if (/[/\\]/.test(name)) + return "An agent name is one folder name — it can't contain / or \\."; + if (name.startsWith(".")) + return "An agent name can't start with a dot — a dotted folder is hidden from the rail."; + if (FORBIDDEN_IN_NAME.test(name)) return `'${name}' isn't a folder name.`; + return null; +} diff --git a/packages/harness/src/shared/types.ts b/packages/harness/src/shared/types.ts index 54a2b208f..5affa9e0e 100644 --- a/packages/harness/src/shared/types.ts +++ b/packages/harness/src/shared/types.ts @@ -1007,6 +1007,24 @@ export interface SessionRecord { // POST /api/track UiTrackRequest → { ok: true } (UI-interaction analytics) // POST /ingest (hook payloads; bearer = ingest token) +/** + * `POST /api/agents/scaffold` response — the agent the harness just created. + * + * `path` is SERVER-AUTHORED: the project directory came from the list of + * folders the rail can show and the name from a validated single segment, so + * nothing here is the caller's string reflected back. The SPA focuses and binds + * on this path rather than on the one it asked for. + */ +export interface AgentScaffoldResponse { + ok: true; + path: string; + name: string; + template: string; + /** Whether the best-effort `npm install` succeeded. False is not a failure — + * the Canvas degrades to its "run npm install" hint. */ + dependenciesInstalled: boolean; +} + /** The app's active UI theme, as tracked client-side (web/src/lib/theme.ts). */ export type UiTheme = "light" | "dark"; diff --git a/packages/harness/vitest.config.ts b/packages/harness/vitest.config.ts index 1edffc99b..49d2a28cc 100644 --- a/packages/harness/vitest.config.ts +++ b/packages/harness/vitest.config.ts @@ -18,6 +18,7 @@ export default defineConfig({ // truth. Mirrors the alias in web/vite.config.ts. "@shared/types": fileURLToPath(new URL("src/shared/types.ts", import.meta.url)), "@shared/system-graph": fileURLToPath(new URL("src/shared/system-graph.ts", import.meta.url)), + "@shared/agent-name": fileURLToPath(new URL("src/shared/agent-name.ts", import.meta.url)), }, }, test: { diff --git a/packages/harness/web/src/App.tsx b/packages/harness/web/src/App.tsx index 4ccf24ef5..4409b4ec2 100644 --- a/packages/harness/web/src/App.tsx +++ b/packages/harness/web/src/App.tsx @@ -77,6 +77,7 @@ import { Toast } from "./components/Toast"; import { TooltipLayer } from "./components/TooltipLayer"; import { NewSessionComposer } from "./components/NewSessionComposer"; import { HelpOverlay } from "./components/HelpOverlay"; +import { CreateAgentDialog } from "./components/CreateAgentDialog"; import { OverviewModal } from "./components/OverviewModal"; import { WorkflowsRail } from "./components/WorkflowsRail"; import { WorkspaceGraphView } from "./components/WorkspaceGraphView"; @@ -90,7 +91,7 @@ import { resolveProjectRoot, slugifyIdea, } from "./lib/project-dir"; -import { basenameOf, isWithinDir, samePath } from "./lib/paths"; +import { basenameOf, isWithinDir, parentOf, samePath } from "./lib/paths"; import { canvasSourceFor, canvasSubject, @@ -124,7 +125,8 @@ import { editorLabel, editorUrl, resolveEditor } from "./lib/editors"; import { CloneAgentConfirm } from "./components/CloneAgentConfirm"; import { cloneDefinitionPrompt, - starterScaffoldInstruction, + composerScaffoldPrompt, + firstInstructionPrompt, useTemplatePrompt, type GalleryTemplate, type StudioTemplate, @@ -286,6 +288,19 @@ export const App = (): JSX.Element => { label: string; } | null>(null); const startingProjectRootsRef = useRef(new Set()); + /** + * The project a create-agent dialog is open for (SAP-2981), or null. + * + * It holds the SUBJECT, not a form: the row that was clicked answers "where", + * and the dialog only asks what that row cannot. `sessionId` is set by the + * bare-project door, where a live session in the folder is already the + * session the new agent should bind to. + */ + const [creatingAgent, setCreatingAgent] = useState<{ + root: string; + label: string; + sessionId?: string; + } | null>(null); /** * Leave map altitude — unless the thing being opened lives INSIDE the * selected project. @@ -1249,6 +1264,14 @@ export const App = (): JSX.Element => { * do not: that folder is the new project's root by construction, and * resolving it upward would drop the new agent into its parent project. */ + /** + * The provider a create-initiated session boots with — the same stored + * preference the rail used to read before it dispatched. It moved here with + * the create itself; the rail no longer starts sessions. + */ + const preferredHarness = (): HarnessKind => + loadUiPrefs().preferredHarness === "codex" ? "codex" : "claude-code"; + const sessionCwdForAgent = (agentPath: string): string => projectRootForAgent(agentPath, knownProjectRoots()); @@ -1302,20 +1325,25 @@ export const App = (): JSX.Element => { await createSessionAt(cwd, agentHarness); }; + /** + * The composer's scaffold prompt — the ONE door left where the coding agent + * creates the project (SAP-2981). + * + * Everywhere the project is already known, the harness creates the agent + * itself (`handleCreateAgentInProject` below). The composer is the home + * screen: no project, no name, just an idea, and the folder it invents does + * not exist yet — so there is nothing here to state and no row to create in. + * The prompt is honest about being a prompt, and its failure message names + * the tool the user would have to ask for by hand. + */ const sendScaffoldPrompt = ( session: HarnessSession, cwd: string, idea?: string, ): void => { - const base = - `Scaffold a new Sapiom agent project in this directory: ${starterScaffoldInstruction(cwd, "default")}, ` + - "then run npm install, read AGENTS.md, and use the sapiom-agent-authoring skill to"; - const trimmedIdea = idea?.trim(); sendPromptWhenReady( session.id, - trimmedIdea - ? `${base} build this:\n\n${trimmedIdea}` - : `${base} define the first agent.`, + composerScaffoldPrompt(cwd, idea), "Couldn't send the scaffold prompt. Ask the coding agent to call sapiom_dev_agents_scaffold.", ); }; @@ -1339,6 +1367,73 @@ export const App = (): JSX.Element => { sendScaffoldPrompt(session, cwd, idea); }; + /** + * THE IN-PROJECT CREATE (SAP-2981; design.md § E4). + * + * The `+` on a project row used to start a session and inject an English + * sentence asking the coding agent to please call the scaffold MCP tool. The + * harness does it now: the dialog collects a name and a starter, the endpoint + * creates the directory and rescans it, and only THEN does a session open. + * + * The order is the feature. Creation completes before the chat starts, so a + * failure is a sentence in the dialog rather than a confused model, and the + * agent is a row in the rail before anything can ask "did it work?". + * + * The project is not asked for — it is the row that was clicked. + */ + const handleCreateAgentInProject = (root: string, label: string): void => { + setCreatingAgent({ root, label }); + }; + + const createAgentInProject = async (input: { + name: string; + template: string; + instruction: string; + }): Promise => { + const request = creatingAgent; + if (!request) return; + // Throws on refusal, and the dialog shows the server's own sentence. It + // resolves only once the agent is on disk AND in the registry. + const created = await harness.scaffoldAgent( + request.root, + input.name, + input.template, + ); + setCreatingAgent(null); + // The rail already has it (the server rescanned before answering); this is + // the selection following the thing the user just made. + setSelectedProject(null); + setFocusedAgentPath(created.path); + + // EVERYTHING BELOW IS THE CHAT, and the agent already exists. A session + // that fails to start is a session failure, reported as one — it must + // never read as "the agent wasn't created", because it was. + try { + const existing = request.sessionId + ? (state.sessions.find((s) => s.id === request.sessionId) ?? null) + : null; + const session = + existing ?? + (await createSessionAt(request.root, preferredHarness())); + await harness.bindWorkflow(session.id, created.path); + harness.setActiveSessionId(session.id); + setFocusedAgentPath(created.path); + if (input.instruction) { + sendPromptWhenReady( + session.id, + firstInstructionPrompt(created.path, input.instruction), + "Couldn't send your first instruction — the agent is created; type it into the terminal.", + ); + } + } catch (err) { + harness.showToast( + `${created.name} was created, but its session didn't start. ${ + (err as Error).message ?? "" + }`.trim(), + ); + } + }; + // The workbench tab + starts a fresh coding-agent process beside the active // session. Folder, provider, and optional agent binding carry over; prompt, // transcript, resume identity, and rehydration deliberately do not. @@ -1428,20 +1523,38 @@ export const App = (): JSX.Element => { })(); }; - // Bare-scaffold folder affordance: a live session sits in a folder - // with no agent yet. Ask that session to scaffold its first agent in place. + /** + * Bare-project affordance: a live session sits in a folder with no agent yet. + * + * It used to inject the scaffold prompt into that session — the third copy of + * the same English sentence. It is the same create as any other project now, + * aimed at the session's own folder, and the session it already has is the + * one the new agent binds to rather than a second pty beside it. + */ const handleScaffoldInSession = (sessionId: string): void => { - const cwd = - state.sessions.find((session) => session.id === sessionId)?.cwd ?? "."; - sendPromptWhenReady( + const session = state.sessions.find((s) => s.id === sessionId); + if (!session) return; + setCreatingAgent({ + root: session.cwd, + label: basenameOf(session.cwd) || session.cwd, sessionId, - `Scaffold a new Sapiom agent project in this directory: ${starterScaffoldInstruction(cwd, "default")}, then run npm install, read AGENTS.md, and use the sapiom-agent-authoring skill to define the first agent.`, - "Couldn't send the scaffold prompt. Ask the coding agent to call sapiom_dev_agents_scaffold.", - ); + }); }; - // Templates journey v0: "Use template" starts a session in the destination - // folder and hands the agent the real operation. + /** + * "Use template" — one journey, two operations, and only one of them is a + * prompt (SAP-2981, E4.6). + * + * A STARTER is the same local scaffold the project `+` does, so it goes + * through the same endpoint: the folder is created before the session opens, + * and a refusal is an error the dialog shows. Two creation paths for one + * operation is exactly how they drift. + * + * A GALLERY template is a different operation — it forks a published agent + * into a repo the user owns, over the network, with an auth failure mode — + * and the harness has no route for that. It stays the coding agent's job, and + * says so. + */ const handleUseTemplate = async ( cwd: string, template: StudioTemplate, @@ -1450,21 +1563,43 @@ export const App = (): JSX.Element => { | "template_gallery" | "template_detail" = "template_gallery", ): Promise => { - const session = await createSessionAt(cwd, "claude-code"); // Product metric — "templates used". Fires at the choke point every // template surface funnels through; `agent.created` fires later when the // clone produces a real sapiom.json, so built ≥ templates holds. - trackProduct("agent.template_cloned", { - template_slug: template.id, - template_id: template.id, - surface, - }); + const trackUse = (): void => { + trackProduct("agent.template_cloned", { + template_slug: template.id, + template_id: template.id, + surface, + }); + }; + if (template.kind === "starter") { + // `cwd` is the folder the destination picker settled on: its parent is + // the project, its basename the agent's name. The endpoint refuses both + // on its own findings, so a rejection here is the server's sentence and + // the dialog shows it verbatim. + const parent = parentOf(cwd); + if (!parent) + throw new Error(`Can't create an agent at ${cwd} — pick a folder inside a project.`); + const created = await harness.scaffoldAgent( + parent, + basenameOf(cwd), + template.id, + ); + trackUse(); + setTemplatesOpen(false); + setFocusedAgentPath(created.path); + const session = await createSessionAt(parent, "claude-code"); + await harness.bindWorkflow(session.id, created.path); + setFocusedAgentPath(created.path); + return; + } + const session = await createSessionAt(cwd, "claude-code"); + trackUse(); sendPromptWhenReady( session.id, useTemplatePrompt(template, cwd), - template.kind === "gallery" - ? "Couldn't send the clone prompt. Ask the coding agent to run sapiom_dev_agents_clone." - : "Couldn't send the starter prompt. Ask the coding agent to call sapiom_dev_agents_scaffold.", + "Couldn't send the clone prompt. Ask the coding agent to run sapiom_dev_agents_clone.", ); }; @@ -2008,7 +2143,7 @@ export const App = (): JSX.Element => { listDir={harness.listDir} onCreateSession={handleCreateSession} listHarnesses={harness.listHarnesses} - onScaffoldSession={handleScaffoldSession} + onCreateAgent={handleCreateAgentInProject} onScaffoldInSession={handleScaffoldInSession} onBrowseTemplates={() => { setSelectedProject(null); @@ -2693,6 +2828,25 @@ export const App = (): JSX.Element => { /> )} + {/* The create-agent dialog (SAP-2981). Mounted here, beside the other + cards-on-top, because the create has to outlive the rail popover that + opened it — the menu unmounts on click, and a dialog rendered inside + it would go with it. */} + {creatingAgent && ( + setCreatingAgent(null)} + onCreate={createAgentInProject} + onBrowseTemplates={() => { + setCreatingAgent(null); + setSelectedProject(null); + setTemplatesOpen(true); + setOverviewOpen(false); + }} + /> + )} + {/* The one-time explainer. It owns its own visibility (first run, the account menu's "How Studio is organised"), so the shell only has to give it a place to mount beside the other card-on-top. */} diff --git a/packages/harness/web/src/components/CreateAgentDialog.tsx b/packages/harness/web/src/components/CreateAgentDialog.tsx new file mode 100644 index 000000000..cf9055116 --- /dev/null +++ b/packages/harness/web/src/components/CreateAgentDialog.tsx @@ -0,0 +1,275 @@ +/** + * Create an agent in a project you already picked (SAP-2981). + * + * The project is STATED, NOT CHOSEN. You clicked that row's menu — that is the + * answer to "where", and re-asking it with a folder picker would be the same + * subject confusion the rail's `+`/`×` pair had: a control that acts on one + * noun while inviting you to pick another. So the destination is a sentence at + * the top of the dialog, spelled out to its absolute path because a rail label + * can be a widened or shared name and this creates a real directory. + * + * WHAT IT ASKS: a name, a starter, and — optionally — the first thing to build. + * Nothing else, because nothing else is decidable here: the folder is known, + * and the harness resolves dependency versions itself. + * + * CREATION COMPLETES BEFORE THE CHAT STARTS. `onCreate` resolves only once the + * server has scaffolded the agent and rescanned it into the registry, so a + * refusal lands in THIS dialog as a sentence (the field keeps what you typed, + * ready to fix) rather than as a coding agent that was asked to do a filesystem + * operation in English and got confused. The dialog stays up and busy while it + * runs — the whole point is that the outcome is reported. + * + * The name is validated as you type against the SAME rule the endpoint refuses + * with (`@shared/agent-name`). Two guards, one rule: a name the field accepts + * and the server rejects reads as a broken app. + */ + +import { useEffect, useMemo, useRef, useState } from "react"; +import type { JSX, RefObject } from "react"; + +import { refuseAgentName } from "@shared/agent-name"; + +import type { StarterTemplate } from "../lib/templates"; +import { STARTER_TEMPLATES } from "../lib/templates"; +import { trackingAttrs } from "../lib/analytics/tracking-attrs"; +import { useDismissable } from "../lib/use-dismissable"; +import { Icon } from "./Icon"; + +export function CreateAgentDialog({ + projectLabel, + projectRoot, + templates = STARTER_TEMPLATES, + onCancel, + onCreate, + onBrowseTemplates, + triggerRef, +}: { + /** The project's rail label — what the user actually read on the row. */ + projectLabel: string; + /** The absolute folder the agent is created in, spelled out. */ + projectRoot: string; + /** Bundled starters. The gallery is a separate journey (see the footnote + * below `onBrowseTemplates`), so these are the ones this dialog offers. */ + templates?: readonly StarterTemplate[]; + onCancel: () => void; + /** Rejects with the server's own sentence, which is shown in place of the + * hint. Resolves only once the agent exists. */ + onCreate: (input: { + name: string; + template: string; + instruction: string; + }) => Promise; + /** Leaves for the template gallery — the clone journey this dialog does not + * own. Omitted, the link is not rendered. */ + onBrowseTemplates?: () => void; + /** The control that opened this — Escape returns focus to it. */ + triggerRef?: RefObject; +}): JSX.Element { + const [name, setName] = useState(""); + const [template, setTemplate] = useState(templates[0]?.id ?? "default"); + const [instruction, setInstruction] = useState(""); + const [busy, setBusy] = useState(false); + const [error, setError] = useState(null); + const panelRef = useRef(null); + const nameRef = useRef(null); + // Never dismissable mid-flight: the agent is being written to disk, and + // pulling the dialog would leave the user with no report of how it went. + useDismissable(!busy, { onDismiss: onCancel, containerRef: panelRef, triggerRef }); + + useEffect(() => { + nameRef.current?.focus(); + }, []); + + // Only after something has been typed: an empty field on open is not a + // mistake the user has made yet, and greeting them with "Give the agent a + // name" is a scold, not a hint. + const nameRefusal = useMemo( + () => (name === "" ? null : refuseAgentName(name)), + [name], + ); + const submittable = name.trim() !== "" && nameRefusal == null && !busy; + + const submit = async (): Promise => { + if (!submittable) return; + setBusy(true); + setError(null); + try { + await onCreate({ name, template, instruction: instruction.trim() }); + } catch (err) { + // The server's sentence, verbatim — it knows things this dialog cannot + // (a folder already sitting there, a project the rail stopped showing). + setError((err as Error).message || `Couldn't create ${name}.`); + setBusy(false); + nameRef.current?.focus(); + } + }; + + return ( +
+
{ + // Return submits from the single-line field; the textarea keeps + // Return for newlines and takes ⌘/Ctrl+Return instead. + if (event.key !== "Enter") return; + const inTextarea = + (event.target as HTMLElement).tagName === "TEXTAREA"; + if (inTextarea && !(event.metaKey || event.ctrlKey)) return; + event.preventDefault(); + void submit(); + }} + > +
+ Create an agent + +
+ +
+ {/* The destination, stated. `title` carries the full path for a root + long enough to ellipsize. */} +

+ In {projectLabel} + {projectRoot} +

+ +
+ + { + setName(event.target.value); + setError(null); + }} + /> + {nameRefusal ? ( +

+ {nameRefusal} +

+ ) : ( +

+ It becomes a folder in {projectLabel}. +

+ )} +
+ +
+ Template +
+ {templates.map((starter) => ( + + ))} +
+ {onBrowseTemplates && ( + /* The gallery is a CLONE, not a scaffold: it forks a published + template into a repo you own and needs an account. It is a + different operation with a different failure mode, so it keeps + its own journey rather than hiding behind this radio list. */ +

+ {" "} + to start from a published agent instead. +

+ )} +
+ +
+ +