Make confirm a real approval everywhere it lands, and stop two silent losses

[sroussey]

#929 has since been squash-merged, so this now targets main directly. The branch was rebuilt as its own commits on top of current main — the eight commits it previously carried from #929's branch are already in main as that squash, and were the whole of the merge conflict. Three findings against #929's work, each verified here before it was fixed, plus one from the review round below.

1. confirm was a public kind no shipped connector implemented

What was wrong. HumanInteractionKind and HumanInputTask's config enum both gained confirm, but neither connector in examples/cli handled it. HumanInteractionHost's switch fell through to HumanElicitPanel, which builds editable fields from every property (prepareSchemaFormFields = getAllFields) and resolves { action: "accept", content: merged } on submit; Esc gives cancel, and there was no decline path at all. The web console was worse: RunEventHumanConnector does emit kind and data on the run event, but reduceRunEvent kept neither, so HumanPrompt built empty text boxes from the confirm's contentSchema, never rendered the values, and offered only Send→accept / Cancel.

What that allowed. A graph running HumanInputTask({ kind: "confirm", ... }) on a TTY showed "Input required" with the description of the action as prefilled editable fields; pressing Enter through them returned accept, and HumanInputTask.execute spreads response.content onto its output ports — so the operator's edits to the description of the action became part of what the graph did with it. On the console the same confirm showed the message, two empty boxes, and no way to refuse. roundtrip.confirm.decline exists to catch exactly "accept without asking", and neither connector was registered with the suite, so it never ran against either.

What changed. humanPromptModel (examples/cli/src/ui/model/humanPrompt.ts) decides the shape once for both renderers — it lives in the directory whose existing guard test fails if anything there imports a renderer, so the Ink panel and the browser card cannot disagree. A confirm is an approval: its values are read, not edited; it offers approve and decline distinctly from cancel; and it carries no content back. HumanInteractionHost gained a HumanConfirmPanel (y/Enter approve, n decline, Esc cancel), reduceRunEvent now keeps kind and data, and the console's HumanPrompt renders the model.

Both connectors are now registered with runHumanConnectorConformance, answered through that model: the scripted answer is given when the drawn surface offers it, and otherwise the person is left with the surface's primary action. That is what makes the registration non-vacuous — a confirm drawn as a form has no decline to press, so the assertion fails rather than passing on an answer no rendering could have produced.

Also pinned, per the review's open question about whether a confirm may return content at all: it may not. IHumanResponse.content says so, a new roundtrip.confirm.noContent assertion pushes a scripted content alongside an accepted confirm and requires it not to come back, and MockHumanConnector holds the contract as the reference implementation. McpElicitationConnector already did.

2. trimHistoryForModel silently dropped a leading system message

packages/ai/src/task/ChatHistory.ts derived cut points from role === "user" indices only and sliced from one of them, so anything ahead of the first user message went with the trim. ChatRole includes "system" and this package builds such lists itself (AiChatWithKbTask). A host keeping its system prompt as history[0] lost its instructions and guardrails, with no error, the first time a conversation crossed 120k chars — and every turn after. A leading run of system messages is now held out of the turn scan and prepended to the result; its size still counts against the budget, so the turns cut to fit around it.

3. The MCP approval card could be forged by a newline

withConfirmDetails JSON-escaped non-strings and interpolated strings verbatim into Label: value lines joined by \n. A confirm's requestedSchema is deliberately empty, so that string is the whole approval card, and HumanInputTask.contentData is an input port — reachable from a model over MCP. One newline in a value wrote a second labelled line indistinguishable from a real one (Reaches: (nothing beyond running a model) under a true Reaches: naming an attacker host), or padded the card with blanks until the real detail was out of view. Line breaks in an interpolated value or label are now escaped, so the card's line structure comes from the code.

4. The reference connector did not hold the contract this PR documents (review round)

Raised by a Copilot review on the diff above and confirmed against the code. MockHumanConnector.shape() stripped content for a confirm only, while item 1 documents IHumanResponse.content as present only for an accepted elicit. A scripted decline/cancel therefore handed data straight back, and nothing caught it — roundtrip.decline and roundtrip.cancel both script content: undefined, so the gap was invisible to the suite. That matters because this connector is the yardstick adapters are measured against: a loose reference cannot fail an adapter that leaks a refused form onto HumanInputTask's output ports, which is the same failure item 1 exists to close.

shape() now keeps content only when request.kind === "elicit" && response.action === "accept". The other half of the review's claim — accept on a non-elicit kind — was already safe by a different route: notify/display return before shape() on the fast-resolve path. The tightened condition states that directly rather than depending on that early return. Nothing depended on the looser behaviour: the two content-bearing assertions (multiTurnFollowUp, concurrentIsolation) both use elicit + accept.

The coverage is a unit test on the reference connector rather than a new conformance assertion, deliberately: neither CLI surface can decline an elicit at all — both already declare roundtrip.decline as an expected failure — so a suite-level assertion would have added two more expected failures rather than testing anything new.

Verified

Run in a worktree after bun install and bun run use-source, and re-run in full after the rebuild onto main:

• bunx vitest run packages/ai/src/task/__tests__/ChatHistory.test.ts — the two new system-message tests failed on the unfixed code (expected [ 'user' ] to deeply equal [ 'system', 'user' ]), pass after. 12 passed.
• bunx vitest run packages/test/src/test/human/ — 7 files, 109 passed, 3 expected fail, 4 skipped. The three MCP forgery tests failed on the unfixed code (4 detail lines where 2 were expected; 40 blank lines where 1 was expected). The three expected failures are unchanged from before item 4, so no adapter regressed against the tightened reference.
• The two new conformance registrations were run against the unfixed connectors first: 7 failed, including roundtrip.confirm.decline for both, plus roundtrip.confirm.details and both notify/display fast-resolve assertions for the console connector (the reducer dropped kind and data).
• Item 4's two new cases were run against the unfixed connector: 2 failed | 17 passed; 19 passed with the fix.
• bunx vitest run --project cli — 40 files, 294 passed, 1 skipped.
• bun run build:types — 43/43 successful. bun run lint — exit 0. bun run format-check — clean, 2478 files. Working tree clean after install (no lockfile drift).

Not run here: the full bun run test and the full bun run build — both are long, and nothing outside the sections above imports what changed. The integration and end-to-end tiers were not run (they want databases and live keys).

Deliberately left unfixed

• Neither CLI surface can decline an elicit. The Ink form offers submit and Esc; the console offers Send and Cancel. Both connectors declare roundtrip.decline as an expected failure with a comment saying why, rather than growing a third exit on a text form: unlike a confirm, an elicit's caller wanted a value and gets none either way. Recorded rather than papered over.
• RunEventHumanConnector resolves an aborted send as cancel rather than rejecting. That is deliberate and documented on the class — a rejection would surface as a task failure rather than the cancellation it is — but abort.beforeSend states the opposite, so it is declared an expected failure too. Turning it into a capability flag would widen the contract API beyond these findings.
• No length bound on a confirm value. The review mentioned it alongside the newline fix; the line-structure forgery is what this addresses.

New surface

@workglow/cli gains a ./human subpath (the two connectors, the prompt model, the console reducer) so the conformance suite can reach them without loading the command tree behind lib.ts, and @workglow/test devDepends on @workglow/cli — the same direction as every other arrow into that package. Worth a maintainer's eye: this is a new public export and a new dependency edge, a design call rather than a bug fix.

🤖 Generated with Claude Code

https://claude.ai/code/session_01T8DU24G9GWuRUJncc5TJFZ

[Copilot]

🟡 Changes recommended

MockHumanConnector still allows content to survive on non-accepted or non-elicit responses, which contradicts the newly documented IHumanResponse.content contract and can mask adapter bugs in the conformance suite.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR tightens the “human interaction” contract across CLI + web console + MCP by making kind: "confirm" behave as a true approval (read-only details + approve/decline/cancel, and no returned content), while also fixing two silent data-loss/forgery issues in adjacent surfaces (chat history trimming and confirm-card rendering).

Changes:

• Introduces a shared, renderer-agnostic humanPromptModel and wires both Ink + web console UIs to render/answer confirm as an approval (not an editable form), with conformance coverage.
• Fixes trimHistoryForModel to preserve leading system messages when trimming by budget.
• Prevents newline-based “approval card” forgery by escaping line breaks in MCP confirm-card details (labels and values).

File summaries

File	Description
packages/util/src/human/HumanConnector.ts	Documents the tightened IHumanResponse.content contract (only accepted elicit may carry content).
packages/test/src/test/human/RunEventHumanConnector.conformance.test.ts	Adds conformance coverage for the web console connector (via reduced run-event view state).
packages/test/src/test/human/McpElicitationConnector.confirmCard.test.ts	Adds regression tests for newline/blank-line forgery in MCP confirm cards.
packages/test/src/test/human/InkHumanConnector.conformance.test.ts	Adds conformance coverage for the Ink connector.
packages/test/src/test/human/cliHumanSurface.ts	Test harness that answers through humanPromptModel (surface-constrained scripting).
packages/test/src/contract/human-connector/types.ts	Extends assertion id set for the new confirm “no content” rule.
packages/test/src/contract/human-connector/MockHumanConnector.ts	Updates reference connector behavior for confirm responses (but currently not fully aligned with the new content contract).
packages/test/src/contract/human-connector/assertions/roundtrip.ts	Adds roundtrip.confirm.noContent assertion.
packages/test/package.json	Adds @workglow/cli devDependency so contract tests can import connector implementations.
packages/mcp/src/tasks/McpElicitationConnector.ts	Escapes line breaks in confirm-card labels/values to prevent forged lines.
packages/ai/src/task/ChatHistory.ts	Preserves a leading system prefix when trimming to a character budget.
packages/ai/src/task/tests/ChatHistory.test.ts	Adds tests covering preservation of leading system messages across trimming.
examples/cli/src/web/client/views/HumanPrompt.tsx	Renders via humanPromptModel to distinguish approvals vs forms; adds decline action support for confirm.
examples/cli/src/web/client/state.ts	Ensures run-event reduction retains kind and data for human requests (needed for confirm details).
examples/cli/src/ui/model/humanPrompt.ts	New shared prompt-shaping model (approval/form/acknowledge) + one-line escaping for details.
examples/cli/src/ui/model/humanPrompt.test.ts	Unit tests for the shared prompt model.
examples/cli/src/ui/HumanInteractionHost.tsx	Adds HumanConfirmPanel with approve/decline/cancel key handling.
examples/cli/src/human.ts	New @workglow/cli/human subpath entrypoint exporting connectors + shared model + reducer utilities.
examples/cli/package.json	Adds ./human export and build step for the new entrypoint.
bun.lock	Updates workspace lockfile for the new @workglow/cli dependency edge.

Review details

• Files reviewed: 19/20 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.