feat!: replace the sub-harness system with an Agent Client Protocol client - #89
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Owner
Author
Review findings closed outAll 13 correctness findings and all 11 claim findings from the adversarial review are now fixed in this branch. Final pass (
The one claim nobody had verifiedThe docs promised ACP output reaches It holds, and there's now a test so it keeps holding. Verification2,097 tests (130 in Every fix in this branch has a regression test that fails against the pre-fix code; I checked each by reverting the fix and re-running. The liveness suite hangs outright without its fix, which is the original symptom. Still outstandingThe five …and the same for |
…ocol client
Coding-agent steps are now built on the Agent Client Protocol (ACP), a
JSON-RPC 2.0 standard in which Noetic is the Client and the coding agent is
the Agent. This replaces the five per-vendor sub-harness packages with one
`@noetic-tools/acp` package and one generic `step.acpAgent`.
The old system was a Noetic-invented contract wrapping a different vendor SDK
per agent, normalising everything into a nine-variant stream union. Adding an
agent meant editing core and the published JSON Schema, because `SubHarnessKind`
was a closed enum baked into both. Permissions, plans, session modes, slash
commands, MCP passthrough, multimodal prompts, and terminals had no
representation and landed in `raw` or were dropped. And a coding agent read
files and ran commands through its own SDK, so none of it passed through
`ctx.fs` / `ctx.shell`.
ACP inverts the control flow: the agent asks the client to read files, write
files, and run terminals, and asks permission before running a tool. Serving
those requests means a sub-agent's file and shell access now flows through
Noetic's own adapters — the same sandboxing, virtual filesystem, and audit path
as a first-party step.
- `packages/types/src/types/acp.ts` holds the contract (`AcpAgent` →
`AcpAgentConnection` → `AcpSession`, plus `AcpTransport`, `AcpClientHost`,
and the permission types), re-exporting the protocol types verbatim from
`@zed-industries/agent-client-protocol` so the wire surface cannot drift.
- `@noetic-tools/acp` owns the protocol client, the client-side `fs/*`,
`terminal/*`, and permission handlers, the agent presets, and the transports.
Its main entry is runtime-neutral; the Node stdio transport is behind
`./stdio`. Core never imports it — one sentrux boundary each way replaces the
eighteen the sub-harness packages needed.
- Permissions resolve in three tiers — step policy, then steering (as a veto,
since `beforeToolCall` also returns allow when no hook exists), then an async
handler — falling back to a default of deny.
- The JSON node is `{ kind: "acp-agent", agent: "<registry key>" }`, an open
registry: adding an agent touches neither core nor the published schema.
The 74 tests in `packages/acp` drive the real wire protocol in both directions
through an in-process `AgentSideConnection`, covering capability negotiation,
all eight session-update variants, permission round-trips, the `fs/*` and
`terminal/*` callbacks, cancellation, and every stop reason. The previous
adapters were only ever exercised against fake runners that touched no
protocol. No real agent binary is spawned in CI; `examples/acp-e2e.ts` does
that behind `ACP_LIVE_AGENT=1`.
BREAKING CHANGE: `step.claudeCode`, `step.codex`, `step.opencode`, and
`step.pi` are removed, along with the `claude-code` / `codex` / `opencode` /
`pi` step kinds and JSON workflow node kinds. Use `step.acpAgent({ agent })`
and the `acp-agent` node kind instead. The packages `@noetic-tools/sub-harness`
and `@noetic-tools/sub-harness-{claude-code,codex,opencode,pi}` are removed in
favour of `@noetic-tools/acp`. The `SubHarness*` types are replaced by the
`Acp*` contract, and `HydrationContext.subHarnesses` by
`HydrationContext.acpAgents` (keyed by a free-form `agentId` string rather than
a `SubHarnessKind`). Session teardown is `'close' | 'keep'` instead of
`'stop' | 'detach' | 'destroy'`, and the `UNKNOWN_SUB_HARNESS_REFERENCE`
hydration error is now `UNKNOWN_ACP_AGENT_REFERENCE`.
Signed-off-by: Matt Apperson <me@mattapperson.com>
An `AcpAgentConnection` owns a live agent — usually a child process whose stdio keeps the host's event loop alive. `executeAcpAgent` only closed it on a failed `session/prompt`; a failure in `session/new`, `session/load`, `session/set_mode`, or `session/set_model` propagated straight out and left the agent running. The step did not merely leak: the process never exited. Found by running `examples/acp-e2e.ts` against a real agent, where `session/new` failed and the example hung indefinitely instead of reporting the error. Both paths now tear a freshly opened connection down before rethrowing. A reused connection is still left intact for a later step to retry against. The example's live path now also clears the `CLAUDECODE` marker: Claude Code refuses to launch nested inside another Claude Code session, and spawning it as an ACP subprocess is exactly the supported usage. Signed-off-by: Matt Apperson <me@mattapperson.com>
…ession is reused Sharing one ACP session across several steps — the documented "investigate read-only, then fix with edits" pattern — was broken three ways. All three were silent. 1. Per-step configuration was ignored. The client host carries the permission policy, steering hook, async handler, and event sink, but it was built once when the connection opened and never rebound. Every step after the first was answered with the opening step's policy. `openAcpConnection` compounded this by spreading the host into a new object, so even a rebind could not reach the client. The host is now passed by reference the whole way down, the client reads it at call time instead of snapshotting, and the runtime rebinds it before every turn. 2. Output was misattributed. Because the update sink was part of that frozen host, a reused session streamed its notifications into the first step's already-finalized event bridge, under the first step's id. 3. Kept sessions were never disposed. Nothing emptied the harness's session store, so `onComplete: 'keep'` meant "keep forever" — with the stdio transport that leaves a child process running whose stdio keeps the event loop alive, so the host never exits. Reuse is now scoped to a root run: the harness closes whatever it still holds when the run finishes. Two connection-level settings cannot vary per step, because ACP negotiates client capabilities once during `initialize` and a connection speaks to one agent. Requesting different ones from a step joining an existing session now raises `ACP_SESSION_CAPABILITY_CONFLICT` / `ACP_SESSION_AGENT_CONFLICT` instead of quietly handing back a session that is not what the step asked for. Regression tests cover all of it at both levels: the interpreter's rebinding and run-scoped disposal in core, and the client's live read of the host in `packages/acp` — the latter is what the spread-copy defeated, and it fails against the previous code. BREAKING CHANGE: `AcpClientHost.permissions`, `steerPermission`, `onPermissionRequest`, and `onSessionUpdate` are no longer `readonly` — the runtime rebinds them per turn, and any implementation that copies the host rather than holding it by reference will silently freeze per-step configuration. `AcpLiveSession` gains required `host` and `agentId` fields. `session.onComplete: 'keep'` is now scoped to the root run rather than the lifetime of the harness. Signed-off-by: Matt Apperson <me@mattapperson.com>
…one outlive its run An ACP connection owns a live agent — usually a child process whose stdio keeps the event loop alive — so how long one is kept should be something a step says, not something the runtime infers. `session.onComplete` inferred it twice over: a fresh session defaulted to `'close'` and a reused one to `'keep'`, so naming a reuse key silently extended the lifetime and closing it early meant opting back out. `session.keepAlive` replaces it and states the scope directly: 'step' (default) closed when the step finishes — nothing is maintained 'run' kept for the rest of the root run, then collected 'harness' kept until the caller runs `harness.closeAcpSessions()` `reuse` now requires `'run'` or `'harness'`. A connection closed at the end of its step has nothing left to share, so a reuse key without a scope raises `ACP_REUSE_WITHOUT_KEEPALIVE` rather than quietly upgrading the lifetime on the step's behalf — the step asked for two things and named one. `'harness'` is new: it makes the long-lived case reachable at all. A harness serving a conversation can keep one warm coding agent across many `execute()` calls, which the previous run-scoped collection made impossible. Ownership transfers with it — nothing closes a `'harness'` session for you, and `closeAcpSessions()` (now public and idempotent) is how you give it back. Verified end to end for all three scopes: the default opens a connection per step and holds none; `'run'` shares one across steps in a run and is collected after it; `'harness'` survives runs and is released only on disposal. Signed-off-by: Matt Apperson <me@mattapperson.com>
… a step
`step.acpAgent` puts a coding agent where the step-tree author decided it goes.
Three surfaces were missing for anyone who wants the decision made elsewhere.
**A tool the model can call.** `acpAgentTool()` wraps an agent as a `Tool`, so a
`callModel` step can delegate by calling it — `{ prompt }` in, `{ text }` out,
named `delegate_to_<agentId>` by default. It runs the same `step.acpAgent`
through `harness.run` on the calling tool's context, so the delegated turn lands
in the same item log, usage totals, and event stream as any other step rather
than becoming a side channel. Giving it a `session.reuse` key lets the model
hold a conversation with the agent across calls instead of starting cold.
**Permission requests routed to a person.** A declarative policy can only answer
what it was told in advance. `askUserForPermission()` publishes the request on
an external channel and parks for an answer, using the request/decision shape
`@noetic-tools/chat-sdk` already established for tool approvals: a queue for
requests (each belongs to one reviewer) and a topic for decisions (broadcast,
filtered by `requestId`). The prompt carries what a reviewer needs — agent,
step, thread, tool title and kind, raw input, and the agent's own options, which
a UI should present rather than inventing its own. An unanswered prompt denies
on timeout: waiting must never become approval.
Because reaching a person means reaching a channel, `onPermissionRequest` now
takes `(request, ctx, info)`. The runtime binds the context and the asking
agent/step before handing the handler to the protocol client, which keeps
`Context` out of the client entirely.
**A live-session surface.** `listAcpSessions()`, `getAcpSession()`, and
`cancelAcpSession()` let a UI show running sub-agents — with their mode and
slash commands — and interrupt one. Deliberately read-and-interrupt only: turns
are driven by steps, so a turn started here would bypass the item log, usage
accounting, and the event bridge. Follow-ups go through a step sharing the
`session.reuse` key. The raw session map is now marked internal.
Writing the end-to-end proof caught a real bug in the permission handler: it
published the request and only then parked on the decision topic, but topic
delivery reaches only subscribers parked at send time — so a reviewer who
answered immediately answered into the void, and the agent waited out the full
timeout and was denied something a human had allowed. It now parks first. The
regression test uses a topic-faithful stub that drops undelivered sends, which
is what the more forgiving stub could not catch.
Signed-off-by: Matt Apperson <me@mattapperson.com>
…act a false sandboxing claim
I claimed in four places that routing a sub-agent's file and shell access
through Noetic's adapters gave it "the same sandboxing, virtual filesystem, and
audit path as a first-party step". That was false, and I verified it was false
by running it: with the local adapters an agent read a file well outside the
workspace and ran an arbitrary shell command.
Worse, it did so with `permissions: { default: 'deny' }` — the strictest policy
the API offers. A permission policy answers `session/request_permission`, which
covers the agent's own TOOL CALLS. `fs/read_text_file`, `fs/write_text_file`,
and `terminal/*` are client methods the agent invokes on us directly. An agent
that simply never asks was never gated by any policy, however strict. A reader
of those docs would reasonably have believed otherwise.
`createLocalFsAdapter()` is a bare passthrough to `node:fs/promises`, so there
was no constraint anywhere in the path.
ACP places boundary enforcement on the client, so the fix belongs here rather
than in an adapter a user might or might not supply. `fs/*` paths are now
confined to the session working directory by default, rejecting — before the
adapter is touched — paths outside the roots, `..` traversal out of them,
sibling directories that merely share a root's name prefix (`/workspace-secrets`
is not inside `/workspace`), and relative paths, which the specification forbids
on the wire in the first place. `clientCapabilities.additionalDirectories`
widens the boundary; `allowAnyPath` removes it. `terminal/create` confines its
starting cwd the same way.
The docs, spec, README, and CLAUDE.md now say what actually holds, including
what confinement does NOT stop: symlinks inside the workspace pointing out of
it (the check is lexical and deliberately does not touch the filesystem), and
anything a terminal command does after it starts, since a shell can `cd`
anywhere — `clientCapabilities: { terminal: false }` is the only hard boundary
for shell access. It narrows what a cooperative-but-careless agent reaches; it
is not a sandbox against a hostile one.
Three of the new over-the-wire tests fail against the unconfined client.
BREAKING CHANGE: an ACP agent can no longer read or write outside the session
working directory by default. A host that relied on the previous unconfined
behaviour must set `clientCapabilities.additionalDirectories` or
`allowAnyPath: true`.
Signed-off-by: Matt Apperson <me@mattapperson.com>
… touches I claimed an "audit path" alongside the sandboxing claim, retracted the word in e8f4dfa, and never checked whether the capability existed. It did not. The client served `fs/*` and `terminal/*` and emitted nothing, so the only trace of what a sub-agent did was whatever it chose to narrate in its own `tool_call` updates — its account of itself, not an observation of it. Every client-side call now reports an `AcpClientActivity` to the host, which the runtime emits as an `acp_client_activity` framework event: the method, the path or command line, and whether it was served. Refusals are recorded too, and are the more useful half — a refusal is the moment an agent reached for something it was not allowed to have, which a log of successes alone would hide entirely. Scope is stated rather than implied: this covers the client boundary. Work the agent does inside its own process, and whatever a terminal command does once running, are not visible here. Signed-off-by: Matt Apperson <me@mattapperson.com>
…ore correct An adversarial review found four defects that share a theme: the runtime assumed the happy path and had no answer for an agent, or a peer run, behaving otherwise. **A dead agent hung the step forever.** The protocol library breaks its read loop on end-of-stream without rejecting the responses still pending, and `openAcpConnection` never read the `signal` it was handed. So a missing binary, a crash on startup, or an OOM mid-turn left `initialize` and `prompt` unsettled with no error and no timeout — and aborting the context did not help, because the promise it would race against never settled either. The `AcpConnectError` for a failed handshake was unreachable in exactly the case it was written for. Every request now races a transport watcher that turns end-of-stream, and the abort signal, into a rejection. A deliberate `close()` rejects in-flight work too, since once the transport is down nothing can ever answer it. **Anything kept without a `reuse` key leaked permanently.** Registration only happened when the step named a key, so `keepAlive: 'run'` or `'harness'` without one was held by nobody — not swept at the end of the run, and out of reach of `closeAcpSessions()`. That is a child process whose stdio keeps the host alive forever: the exact failure the previous two lifecycle commits were written to fix, surviving in the axis their tests never covered. **One run's completion closed a concurrent run's session mid-turn.** The sweep took every entry the harness held, with no notion of ownership, and the store was keyed only by the user's reuse string — so two unrelated runs also shared one agent's conversation silently. Run-scoped sessions are now keyed and swept per root run; `'harness'` scope stays global, which is its purpose. **Two parallel steps sharing a key opened two connections.** Check, await connect, register left a window where both missed and the second registration orphaned the first — unreachable, never closed. The lookup and the registration of the in-flight promise now happen in one synchronous turn; a second caller awaits the first's connection instead of racing it. `acpAgentTool` made this materially easier to hit, since a model can emit parallel tool calls. The store moved into `AcpSessionStore`, which owns keying, ownership, and teardown rather than leaving them spread across the interpreter and harness. All five new regression tests fail against the previous code, and the liveness suite hangs outright without the fix — the symptom itself. BREAKING CHANGE: `AcpSessionInfo.key` is now an opaque handle rather than the step's `session.reuse` value, which moved to the new `reuseKey` field. `getAcpSession`/`cancelAcpSession` accept either. Signed-off-by: Matt Apperson <me@mattapperson.com>
… and fix terminal truncation Four more findings from the adversarial review, three of which made a shipped claim untrue. **The prompt-capability gate was dead code.** `assertPromptContentSupported` was defined, exported, and covered by four tests — and called from nothing on a runtime path. The spec and docs promised an `AcpCapabilityError` "before anything reaches the wire"; an image sent to an agent that never advertised image support went on the wire and failed opaquely inside the agent. The session now enforces it before the turn, with the agent's advertised capabilities threaded in. **A denied tool told the agent the whole turn was cancelled.** When no offered option matched the decision, the client answered `cancelled` — which the specification defines as *the prompt turn was cancelled*. A conforming agent aborts the turn and answers `stopReason: 'cancelled'`, which the step converts to a thrown error: one denied tool killed the entire step. A deny now falls back to either reject flavour, and a genuinely unusable option set raises a Noetic-side error instead of lying on the wire. **Output truncation destroyed the output.** Trimming dropped whole chunks, so a single chunk larger than `outputByteLimit` left nothing at all — a `npm test` producing megabytes read back empty with only `truncated: true` as a hint. ACP asks the client to truncate from the beginning; it now keeps the tail, backing off to a UTF-8 boundary rather than splitting a character. **`terminal/kill` reported a clean exit.** The killed status was inferred from `exec` rejecting, but the shipped local adapter resolves on a signal-driven abort — only a timeout rejects — so the signal branch was dead against every real adapter and an agent could not tell a killed command from a normal one. The registry tracks the kill itself. The test that "proved" otherwise passed only because the shell double threw on abort, which no production adapter does; the double now matches the real contract, which is what makes the test mean anything. Also: `finalize` no longer leaves an unterminated response when a turn throws — a UI driven off `getFullStream()` saw `response.created` and waited forever for a completion that never came. `StepMeta.acpStopReason` records why a turn stopped, so a caller can finally distinguish a truncated turn from a complete one; both return normally, and the docs claimed this was already recorded. A terminal can no longer be created during or after teardown. All five new tests fail against the previous code. BREAKING CHANGE: `selectPermissionOption` returns `undefined` rather than a `cancelled` outcome when no offered option matches; callers must handle it. `terminal/kill` now reports `SIGTERM` rather than `SIGKILL`, matching the signal actually sent. Signed-off-by: Matt Apperson <me@mattapperson.com>
…packages deprecated Two gaps: the ACP package exported 53 symbols of which 40 appeared in no prose, and nothing told a reader on the old sub-harness packages that they had been replaced. **The public surface is now 30 symbols, every one tagged `@public` and listed in an API reference.** The 23 removed were incidental exports — item builders, the turn accumulator, the terminal registry, the permission engine, the capability and path assertions. They are implementation detail of the client, and a smaller surface documented completely is worth more than a larger one documented partly. Tests already imported them by path, so nothing moved. `assertPromptContentSupported` in particular should never have been public: it is now called on the real path, so exporting it only invited callers to duplicate a check the session already performs. **A migration guide** covers why the shape changed, a field-by-field mapping of every renamed option, the two semantic changes that are not renames (`onComplete` → the opt-in `keepAlive`, `permissionMode` → a permission policy), and what the move buys. It states plainly that a permission policy does not govern `fs/*` or `terminal/*`, since someone translating `permissionMode` would otherwise assume it does. It also explains why opencode and pi have no preset, and shows the one-liner that reaches them anyway. The npm packages themselves are not deprecated by this commit — that is a registry action against published artefacts, and it needs a deliberate decision rather than a side effect of a docs change. Signed-off-by: Matt Apperson <me@mattapperson.com>
…eview found false **Two concurrent turns on one session corrupted both.** ACP notifications carry no turn id, and `activeTurn` was a single field, so a second prompt overwrote it and every update fanned into the newest accumulator — the first turn lost its tail and the second gained it. Silent, with no error and nothing either caller could detect. Reachable whenever two steps share a `session.reuse` key under `inParallel`, and easier still now that a model can emit parallel `acpAgentTool` calls. Turns are now serialised rather than rejected: an ACP session is one conversation, so sequencing is what it means anyway, and both callers get a correct answer. A failed turn does not poison the ones queued behind it. **Capability comparison raised false conflicts.** Comparing raw JSON made two spellings of the same policy conflict — a different key order, or an omitted capability against the default it resolves to — telling an author to make two identical policies identical. Compared field by field after normalisation now. Claims the review found untrue, corrected rather than deleted where the honest version is still worth stating: - Both headline examples used `harness.execute()`, which returns `void`. The first thing a reader copied printed `undefined`. Now `harness.run(...)`. - "No protocol library enters core's dependency graph" was false as written. The *runtime import graph* is genuinely clean — the emitted `types` dist has no import of the protocol package, only erased type re-exports — but the npm tree does contain it, since `types` declares it a real dependency. Both halves are now stated, including in the sentrux `reason`, which a source-import check cannot prove. - The `./stdio` subpath does not keep the module out of a browser bundle: the dynamic `import()` uses a static specifier, so bundlers resolve it. The runtime property is real and still worth having; the bundling claim was not. - Withdrawing `writeTextFile` does not make a step read-only — a shell redirect through `terminal/create` writes just as well. - The `user_message_chunk` comment claimed the replayed history "is already in the item log". True for a session this run started; false for a `session.load` of one it never ran, where that history is genuinely not captured. Also: a drift test now compares the hand-maintained Zod copies of `ToolKind` and `McpServer` against the upstream package, so an added variant fails a test instead of becoming silently unrepresentable in a workflow document. And `AcpSessionStore` moved from `harness/` to `runtime/` — the interpreter imports it, and sentrux correctly rejected that layer edge. Both concurrency tests fail against the previous code. Signed-off-by: Matt Apperson <me@mattapperson.com>
The last of the adversarial review, plus the one documented claim neither reviewer could confirm. **Terminal teardown.** Releasing terminals used to snapshot the list with no guard while the connection released them *before* closing the transport, so an agent could keep issuing `terminal/create` throughout teardown and orphan a child process — precisely what closing a connection is supposed to prevent. The transport now closes first and the registry refuses work once disposed; three tests cover the guard, the kill-on-release, and the connection-level path. **Connection-scoped settings now fail consistently.** A step joining an existing session with a different `cwd` or `mcpServers` had them silently dropped, while a mismatched `clientCapabilities` threw — so a step naming different MCP servers simply got none of them, with no diagnostic. All three raise now. **Event ordering.** `finalize` synthesized tool-call items *before* closing the assistant message, so output item 0 finished after items 1..n — interleaved output to any consumer tracking `outputIndex`. **A negative `limit` on `fs/read_text_file`** became a negative slice bound, quietly meaning "all but the last N lines". Out of schema either way, but a plausible-looking answer to a malformed request is worse than an empty one. **The stream claims are now verified, not assumed.** The docs promised ACP output reaches `getTextStream()`, `getReasoningStream()`, and `getItemStream()`; neither reviewer could confirm it end to end. Driven through a real harness, all three carry it. The "a turn always emits its output" wording was also true only on the success path until the previous commit added the error bracket — it now says which reason each path completes with, since the failure case is the one that matters to a UI. Both the ordering and bracket tests fail against the previous code. Signed-off-by: Matt Apperson <me@mattapperson.com>
…tion live Two of the three loose ends from the PR, both closed by actually running the thing rather than reasoning about it. **opencode and pi now have presets, verified against the real binaries.** Both were previously reachable only through `customAcpAgent` on the theory that their ACP support was unconfirmed. It is now confirmed: I ran an `initialize` handshake against each and both answered protocol version 1. - `opencode()` runs the official `opencode acp` command — native ACP, `agentInfo.name: "OpenCode"`, advertising http/sse MCP and image/embedded content. - `pi()` goes through the community `pi-acp` adapter (the same bridge Zed's registry points at), since pi has no native ACP mode yet. `agentInfo.name: "pi-acp"`, advertising loadSession and image. The docs say plainly that this preset depends on a third-party adapter, so a failure there is not a Noetic bug. **Model-driven delegation is verified against a real model.** `examples/acp-e2e.ts` gains Path I: a live Claude Sonnet 4.5 (via OpenRouter) with `acpAgentTool` in its tool set, delegating to a real spawned claude-code-acp agent. The model called the tool, the agent answered `DELEGATED_OK`, and the model relayed it — the one integration the loopback tests structurally cannot cover, since it needs a real model deciding to call the tool. Gated on both `ACP_LIVE_AGENT=1` and `OPENROUTER_API_KEY`. Docs, README, spec, migration guide, and the agent-builder skill now list all six presets and drop the "no preset / unconfirmed" language for opencode and pi. Signed-off-by: Matt Apperson <me@mattapperson.com>
mattapperson
force-pushed
the
mattapperson/acp
branch
from
August 14, 2026 13:59
1bd38ea to
ecafa47
Compare
|
🎉 This PR is included in version @noetic-tools/types-v3.2.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version @noetic-tools/context-v1.2.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version @noetic-tools/acp-v1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version @noetic-tools/core-v5.4.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version @noetic-tools/eval-v1.0.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
mattapperson
added a commit
that referenced
this pull request
Aug 19, 2026
main moved 15 commits ahead while this branch was in review, including two breaking changes: the sub-harness system was replaced wholesale by an ACP client (#89), and the public API was renamed (#68). Eleven of the files this branch touches also changed there, so this is a real integration rather than a fast-forward. Resolutions: - `.sentrux/rules.toml` — main deleted the sub-harness section outright. Took main's file and re-applied only the agent-plugins layer + boundaries, with the rationale reworded against the ACP client, which is now the package that establishes "no protocol library or vendor SDK in core's graph". - `release-packages.yml` — the five sub-harness jobs are gone. The chain is now types → context → acp → core → openui → agent-plugins → eval, which keeps this package's `needs: openui` and eval's dependency on it intact. - `ci.yml` — kept both sides: main's new root-examples typecheck and this branch's agent-plugins typecheck/test steps. - Root `package.json`, `bun.lock`, `tsconfig.kiira.json` — took main's package set and re-added agent-plugins. The lockfile was regenerated rather than hand-merged. - `CLAUDE.md`, `specs/00-overview.md` — main rewrote both dependency graphs. Re-added agent-plugins alongside acp, which is where it belongs: both depend only on types and are composed in by a host. - Docs `meta.json` — every context-layer page was renamed by #68 (`static-content` → `instructions`, `file-reference` → `filesystem`, and so on). Took the new list and re-inserted the agent-plugins page. Two API renames from #68 reached this branch's own documentation, both caught by the docs typechecker rather than by review: `AgentHarnessOpts.context` is now `contextLayers`, and `llm` is now `callModelDefaults`. Fixed in the docs page and in the agent-builder skill. The package's own source needed no changes — every type it imports from `@noetic-tools/types` survived the rename. Verified on the merged tree: lint clean, every package typechecks, the whole workspace suite passes with zero failures, doc snippets typecheck, the built package imports under plain Node, and `sentrux check` now reports **all rules pass** — main resolved the three violations this branch had been carrying as pre-existing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces the sub-harness system (5 packages, one vendor SDK per agent) with a client for the Agent Client Protocol. Noetic is the Client; the coding agent is the Agent.
Workspace goes 15 packages → 11, and ~23 sentrux boundary rules → 3.
Why
The old system was a Noetic-invented contract wrapping a different SDK per agent, normalised into a 9-variant stream union. Three consequences:
Step.kind, the interpreter, and the published JSON Schema.ctx.fs/ctx.shell, so there was no interception point at all.ACP covers all of it, and because the agent asks the client to read files and run terminals, the client becomes the place a boundary can exist.
What's here
One step, open agent set.
step.acpAgent({ agent })replaces four builders;agentis a free-form adapter, so a new agent needs a registry entry, not a framework change.Three ways to drive an agent — a step, a tool a model can call (
acpAgentTool), or a human answering permission prompts over channels (askUserForPermission). PluslistAcpSessions/cancelAcpSessionso a UI can watch and steer live sub-agents.Filesystem confinement, on by default.
fs/*is confined to the session cwd, rejecting outside paths,..traversal, name-prefix siblings, and relative paths.additionalDirectorieswidens;allowAnyPathopts out.An audit trail. Every
fs/*andterminal/*call an agent makes is emitted asacp_client_activity, allowed and refused — an observed record, not the agent's account of itself.Explicit session lifetime.
session.keepAlive: 'step' | 'run' | 'harness'replaces the inferredonComplete. Areusekey without a scope is an error, not a silent lifetime extension.Verification
packages/acpthat drive the real JSON-RPC wire in both directions via an in-processAgentSideConnection.@zed-industries/claude-code-acpover stdio (ACP_LIVE_AGENT=1 bun examples/acp-e2e.ts).check:exports,gen:schemadrift gate, sentrux — all clean.Reviewed adversarially, and it found real things
Three review agents audited this for correctness, protocol conformance, and overclaiming. Everything below was found and fixed in this PR:
permissions: { default: 'deny' }did not stop it, becausefs/*andterminal/*are client methods the agent calls directly, not tool calls it asks about. Fixed by making it true (confinement + audit) and rewriting the claim to say exactly what holds.reusekey leaked permanently; one run's completion closed a concurrent run's agent mid-turn; parallel steps sharing a key opened two connections and orphaned one.cancelled, which tells a conforming agent the whole turn was cancelled, killing the step over one refusal.terminal/killreported a clean exit because the killed status was inferred from a rejection the real adapter never produces.assertPromptContentSupportedhad four tests and zero call sites while the docs promised the guarantee.Several doc claims were corrected rather than deleted: the dependency-graph claim (runtime import graph is clean, the npm tree is not), the
./stdiobrowser claim, and both headline examples, which usedharness.execute()and printedundefined.Breaking changes
step.claudeCode/codex/opencode/piand their JSON node kinds are gone — usestep.acpAgent({ agent })and{ "kind": "acp-agent", "agent": "..." }. The five@noetic-tools/sub-harness*packages are replaced by@noetic-tools/acp.SubHarness*types →Acp*;HydrationContext.subHarnesses→acpAgents. Session teardown iskeepAliverather thanonComplete. Agents can no longer read outside the session cwd by default.A migration guide maps every renamed option and calls out the two changes that aren't renames.
Follow-ups not in this PR
@noetic-tools/sub-harness*npm packages still neednpm deprecate— the local npm session is unauthenticated.customAcpAgent({ command, args })reaches them.check-docscount claim.