feat(harness): the harness creates the agent, then the chat starts [SAP-2981] - #746
Conversation
…old [SAP-2981] 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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019fEqzA8eEEdQsmEKzLNnuf
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019fEqzA8eEEdQsmEKzLNnuf
…iled request [SAP-2981]
Measured on a real server: the create dialog rendered
`POST /api/agents/scaffold → 409: {"error":"probes already has an agent called
hello-world."}` — a log line with a JSON body inside it. ApiError already
carries `.reason`, the server's own sentence; every user-facing catch now goes
through one helper that prefers it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019fEqzA8eEEdQsmEKzLNnuf
…TA [SAP-2981] Specs for the create flow in mock mode, asserting the ORDER (one append-only log holding both halves of a create) rather than two counters that each say a thing happened and neither says which came first. Every guard was mutation-tested: creating the session first, swallowing the refusal, dropping the name rule, sending the old scaffold prompt, and putting the rail's create back on a session all fail the suite. Driving the empty-project CTA for the first time found it inert: the row carries `pointer-events: none` from when it was a label, and once "Create the first agent here" moved in, that rule swallowed every click on it. `elementFromPoint` over the middle of the button returned the enclosing `.workspace-group`. The spec beside it asserted only `toBeEnabled()`, which a control nobody can click passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019fEqzA8eEEdQsmEKzLNnuf
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019fEqzA8eEEdQsmEKzLNnuf
…he create route [SAP-2981] CodeQL flagged the new route: `childPath`'s dirname comparison is not a barrier it recognizes, so every fs call below it read as path injection, and the inline cleanup made the handler itself an unbounded filesystem sink. The join now re-asserts containment at the sink with `resolveWithinRoot` — the analyzer-recognized shape, and the one `server/canvas.ts` already uses — and the failed-scaffold cleanup moved into its own function. A create is the most expensive request this server serves (a template copy, npm install, git init), so it also gets the same rate limiter shape the attachment upload has. Said plainly: stubbing the resolveWithinRoot barrier leaves the suite green. Nothing childPath returns can fail it, so it is defence in depth and a static- analysis barrier, not a second reachable rule. The cleanup's own mutation does fail, as it must. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019fEqzA8eEEdQsmEKzLNnuf
The bare-project door names the session that was in that folder when the dialog opened, and a dialog can stay open longer than a pty lives. Binding the new agent to an exited session would leave it with nothing to talk to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019fEqzA8eEEdQsmEKzLNnuf
The dialog took Return for the whole form, so a focused Cancel took it too: pressing Return on Cancel closed the dialog AND created the agent. Measured both ways — with the guard removed the spec's "nothing created" assertion fails. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019fEqzA8eEEdQsmEKzLNnuf
Measured: four blocks make it the tallest dialog in the app (658px), and a 620px-tall window pushed its header — title and close button — 19px above the viewport, on a dialog that cannot be scrolled to. The frame is capped and the body scrolls; header and actions stay put at every height measured (950 / 700 / 620). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019fEqzA8eEEdQsmEKzLNnuf
Review — PR #746 (
|
…fault root, no dead branch [SAP-2981] Four findings from the automated review, each fixed and each proven. **The cleanup could delete an agent it did not create.** `lstat` is a message, not a lock: two POSTs with the same body both passed it, one scaffolded, and the other's DIR_NOT_EMPTY cleanup recursively deleted the winner's freshly installed project while the winner's caller had been told it exists. The destination is now CLAIMED with a plain non-recursive `mkdir` — the filesystem decides who gets the name — and the recursive delete is entitled to run only because this request created the directory. The new spec fires both requests at once and reads the winner's file back; it fails against the old ordering. **The template gallery's starters were refused on a fresh install.** The SPA puts a new project under `AppState.defaultProjectRoot` (`<launchDir>/projects` under Electron), which the host never persists — so it was in none of the lists the create route accepts, and the flow could not bootstrap: `recentDirs` learns a root only once a session exists there, and creation now runs first. The host's default is part of `listProjectDirs` now. That was invisible because the mock implemented the name and duplicate guards but not the root barrier — a mock missing a guard is a suite that certifies the bug. It has the barrier now, built from the same list; removing the default root from it fails `templates.spec.ts`, which is the fresh install reproduced. **A dead branch with a test certifying it.** `useTemplatePrompt`'s starter half became unreachable when starters moved onto the endpoint. Deleted, along with the test that asserted the exact arguments of a handoff nothing performs. **A trailing dot** is refused: Windows turns `foo.` into `foo`, so the path the caller is told it got would not be the directory on disk. And the dialog's `triggerRef` is gone rather than undocumented — every control that opens it unmounts on use, so the ref would name a detached node. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019fEqzA8eEEdQsmEKzLNnuf
Follow-up review — PR #746 (delta since
|
…at [SAP-2981]
Round two of the review, and it is right: the atomic claim fixed the delete but
introduced a new dead end at the same click. `<launchDir>/projects` is the
desktop host's default parent for new projects and NOTHING creates it — the
scaffold's own recursive mkdir did, until the non-recursive claim started
running ahead of it. A fresh install's first template was refused at its own
suggested destination, with a new sentence: "that folder no longer exists".
The claim now makes the parent first — recursive, and only on the directory the
rail's list already vetted — so the claim itself stays a single non-recursive
mkdir on the agent's own name, which is what makes it exclusive.
The spec that pinned the wrong shape ("refuses when the project directory has
been deleted under it") was the bug written down as a rule; it is now "creates
the project directory when it does not exist yet". Both mutations fail as they
must: drop the parent mkdir and the fresh-install spec fails; make the claim
recursive again and the concurrent-create spec fails.
Verified on a real server: with `projectRoot` pointed at a path that did not
exist, the create made it and scaffolded into it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019fEqzA8eEEdQsmEKzLNnuf
Primary change type
Problem and motivation
project-create-agent-*→handleScaffoldSessioncreated a pty session and then injected an English sentence into the terminal asking the coding agent to please call an MCP tool. The error copy admitted it: "Ask the coding agent to call sapiom_dev_agents_scaffold." There was no server-side scaffold endpoint — the top CTA, the project+, the empty-project CTA and the Templates gallery all ended in prompt injection, in three near-identical copies of the same sentence.So a failed create surfaced as a confused model rather than an error, and "did it work?" could only be answered by reading a terminal.
Captured on the real 76-agent install, before this change. Clicking Create an agent in probes:
…and what came back:
The button was not merely indirect; on any project that already holds agents it could not work at all, because the scaffold was aimed at the project folder.
Summary and scope
The harness creates the agent.
POST /api/agents/scaffold—{ root, name, template? }→{ path, name, template, dependenciesInstalled }. It calls the samescaffoldroutine the MCP tool calls, and refuses on its own findings, mirroringPOST /api/agents/move(the closest existing precedent for a harness-owned filesystem mutation):nameis one plain segment (childPath)../evil,a/b,"",.hiddenreachingfstemplateis a plain segmentresolveTemplatejoins it onto the bundled templates dirrootmatched against the directories the rail can show — and the list's entry is what gets written intolstatfs.rmon a scaffold that throwsonScaffoldedrescans and broadcasts before the response, so the agent is in the rail before the caller can open a session on it.The flow.
+opens a dialog: name, starter, and the project stated, not chosen — you clicked that row.A refusal lands in the dialog, under the field that produces it, and nothing starts:
Then the agent is in the rail and the session opens on it, with the optional first instruction — which says the scaffold is done, so the agent never re-scaffolds a folder that already exists:
The whole flow end to end (real server, real 76-agent tree):
https://raw.githubusercontent.com/sapiom/sapiom-js/pr-media/media/746/create-flow.webm
Three prompt constructions collapsed to one. The project
+, the empty-project CTA, the bare-project affordance and the gallery's bundled starters all go through the endpoint (E4.6 — two creation paths for one operation is how they drift). The composer keeps its prompt, per the design's carve-out: no project, no name, and a folder that does not exist yet, so there is no row to create in and nothing for a dialog to state. Cloning a gallery template still goes through the coding agent — forking a published agent over the network is a different operation with a different failure mode, and the harness has no route for it.One name rule, two guards.
src/shared/agent-name.tsis imported by both the dialog and the route, because a name the field accepts and the server rejects reads as a broken app.Two defects found by measuring, fixed here:
.workspace-row-empty { pointer-events: none }predates the row carrying a button; once Create the first agent here moved in, that rule swallowed every click.elementFromPointover the middle of the button returned the enclosing.workspace-group. The spec beside it asserted onlytoBeEnabled()— which a control nobody can click passes.POST /api/agents/scaffold → 409: {"error":"…"}.ApiError.reasonis the server's own sentence; oneerrorMessage()helper now prefers it.Also: the rail scrolls a newly focused agent into view (
block: "nearest"). Measured — creating inprobesleft the rail parked onsocial-content, three screens from the row the create had just added.Found after opening this PR, by continuing to measure (each proven both ways):
childPath'sdirname(...) === rootcomparison as no barrier and flagged everyfscall below it. The join now re-asserts containment at the sink withresolveWithinRoot— the analyzer-recognized shape, already used byserver/canvas.ts— the failed-scaffold cleanup moved out of the handler, and the route took the same rate limiter shape the attachment upload has. Said plainly: stubbing that second barrier leaves the suite green. NothingchildPathreturns can fail it, so it is defence in depth and a static-analysis barrier, not a second reachable rule. The cleanup's own mutation does fail.Both hosts. The templates directory is resolved through one helper that applies the asar translation (
scaffoldcopies withcpSync, which cannotopendirinsideapp.asar) — the demo seed's hand-written copy of that regex is now a call to it. The best-effortnpm installshells out tonpm, which the desktop host already puts onprocess.env.PATHbeforestartServer()for exactly this reason (harness-desktop/src/main/runtime-shims.ts); if it is ever missing the scaffold still succeeds and the Canvas degrades to its existing "run npm install" hint.Review round 1 — four findings, all real, all fixed
lstatis a message, not a lock: two POSTs with the same body both passed it, one scaffolded, and the loser'sDIR_NOT_EMPTYcleanup recursively deleted the winner's freshly installed project while the winner's caller had been told it exists. The destination is claimed with a plain non-recursivemkdirnow — the filesystem decides who gets the name — and the recursive delete is entitled to run only because this request created the directory. New spec fires both requests at once and reads the winner's file back; it fails against the old ordering. Confirmed on the real server:409 probes already contains raced.+200, with the winner'sAGENTS.md/index.ts/node_modulesintact.AppState.defaultProjectRoot(<launchDir>/projectsunder Electron), which the host never persists — so it was in none of the lists the route accepts, and the flow could not bootstrap (recentDirslearns a root only once a session exists there, and creation now runs first). The host's default is part oflistProjectDirs. It was invisible because the mock had the name and duplicate guards but not the root barrier; it has it now, from the same list — removing the default root from that list failstemplates.spec.ts, which is the fresh install reproduced.useTemplatePrompt's starter half became unreachable when starters moved onto the endpoint. Deleted, with the test that asserted the exact arguments of a handoff nothing performs.foo.intofoo, so the path returned would not be the directory on disk), and the dialog's never-passedtriggerRefis gone rather than left undocumented — every control that opens it unmounts on use.Review round 2 — one finding, and it was right
The atomic claim fixed the delete and introduced a new dead end at the same click:
<launchDir>/projectsis the desktop host's default parent for new projects and nothing creates it — the scaffold's own recursive mkdir did, until the non-recursive claim started running ahead of it. A fresh install's first template was still refused at its own suggested destination, with a new sentence.The claim makes the parent first — recursive, and only on the directory the rail's list already vetted — so the claim itself stays a single non-recursive
mkdiron the agent's own name, which is what makes it exclusive. The spec that pinned the wrong shape ("refuses when the project directory has been deleted under it") was the bug written down as a rule; it is now "creates the project directory when it does not exist yet". Both mutations fail as they must: drop the parent mkdir and the fresh-install spec fails; make the claim recursive again and the concurrent-create spec fails. Verified on a real server withprojectRootpointed at a path that did not exist — the create made it and scaffolded into it.Out of scope: the gallery clone path, the composer's prompt, and any change to the map or the graph (SAP-2983 owns those files).
Related work
Related issue or discussion: SAP-2981 · design:
plans/studio-project-experience/design.md§ E4 · follows #740 (SAP-2982) and #742 (SAP-2980).Validation
Driven against a real server —
/Users/gwitwer/sapiom/agents(76 agents / 9 projects), own port, own--state-root. Every refusal was posted directly to the endpoint, dialog bypassed:Filesystem verified after every refusal:
probesunchanged at 16 entries, noevilanywhere. Happy path: 2.3 s includingnpm install; the agent was inGET /api/workflows(77) before the response returned.Ordering, measured in the browser (poll every 50 ms): the rail row appeared at 1.3–4.1 s, the first session at 2.1–4.6 s — the agent is in the rail ~0.6 s before any session exists, on every run.
Mutation-tested, every guard. Server (
scaffold.test.ts): neutering the name rule, the template guard, the root barrier, the registry duplicate check, the disklstat, or the failed-scaffold cleanup each fails the suite. E2E (create-agent.spec.ts): creating the session before the scaffold, swallowing the refusal, dropping the name rule, sending the old scaffold prompt, or putting the rail's create back on a session each fails the suite.One assertion did not fail its own mutation and was rewritten. "Rescans before it answers" survived
await→void, because a synchronous stub records "scanned" first either way. The stub now awaits a real tick before recording; the mutation then fails, as it must.Tests and documentation
src/server/scaffold.test.ts(13, real filesystem, route posted directly),src/shared/agent-name.test.ts(3),web/e2e/create-agent.spec.ts(6).rail-grammar.spec.ts's create test andtemplates.spec.ts's starter test asserted the old mechanism and now assert the new contract — both by order, not by counting. The REST surface block insrc/shared/types.tsgains the scaffold route (and the move route, which was already missing).Compatibility and release impact
POST /api/agents/scaffold; the project+and the gallery's bundled starters no longer start a session before creating. A starter now lands under a folder Studio shows as a project — a destination outside one is refused, deliberately, with a reason..changeset/create-agent-flow.md.Security
will follow the
Security Policy for
private reporting.
AI assistance
Claude Code (Opus 5) wrote the endpoint, the dialog, the specs and this description, and drove the verification above — a real harness on its own port and state root, Playwright against the real 76-agent tree, direct
curlat every refusal, and a mutation for every guard. Findings are quoted from what actually ran; the one assertion that survived its own mutation is named above.Checklist
CONTRIBUTING.md, and this contribution follows the direct-PR or issue-first policy.any N/A checks above.