docs(rfc): revise the batch for the hooks-stack base; add three post-stack RFCs
The branch now bases on the hooks stack (PR #138's head), so every 'in-flight' reference to stack content became a current-state fact. Audited all nine RFCs + three supplements claim-by-claim against the merged tree (none invalidated; several strengthened): - prune-dead-core-spine-surface: describe the landed tools/pre-execute → dispatch → tools/post-execute pipeline — listeners return Decisions, the registry builds every result and snapshots it to protect callId, and a second (mutation-guard) test pins the field; drop the resolved wait-for-interception hedge; scope the additionalContext ferry out. - prune-producerless-vocabulary-variants: the ui-stdio fixture migrated off the continuation trigger (llm-replay is the sole writer now); the stack's own additions (rejected, prompt/blocked, hook/invoked+result) all arrived with producers — the admission policy demonstrated live. - prune-unimplemented-subagent-vocabulary: enrichment landed as lastAssistantMessage only (agentType was dropped in the stack's own review — the same judgment this RFC extends); the seam RFC now names tools/pre-execute deny, which exists, sharpening the re-add path. - drop-inert-request-knobs / drop-image-content-block / trim-acp-bridge-unreachable-surface: current-state rewordings (shipped bridges set no request fields; only compact-basic has explicit image arms; 13 hook goldens also pin agentInfo). - generic-long-running-tool-runtime census: second production seam consumer (hook-protocol runHook: resolve+run, stdin/env, foreground only — background machinery stays single-consumer); scrub-duplication blast radius. - discover-package-inventory: identical 54-entry tsconfig reference sets; the comparesLog scenario knob (fixture-derivable, like recorded). - unify-agent-and-session-id: third divergence site (in-process subagent children mint two UUIDs), the hooks bridge id-lookups, and ui-stdio's labelBySession map as a consumer that deletes under unification. New RFCs from the post-stack survey: - remove-agent-steering-mirror: the last mirror-of-durable event; zero production listeners; both retention RFCs deferred its fate, and the 'no durable twin' rationale is contradicted by the adjacent append. - tighten-hook-protocol-contract: producer-less 'native' dialect, parsed-and-discarded suppressOutput, and hook/result semantics (truncation + decision-string) defined twice in the bridges instead of the lib that owns the event. - single-source-acp-replay-config: cordis.yml/cordis.snapshot.yml differ by exactly one plugin entry, with no gate on the forced symmetry.
This commit is contained in:
Tianyi Cui
@@ -9,12 +9,13 @@ The agent factory carries TWO ids for what is, in every live consumer, one thing
|
||||
- `agentId` — the `AgentRegistry` handle (the actor identity; the registry rejects a duplicate).
|
||||
- `sessionId` — the event-sourced session / persisted-log identity (`session.header.id`).
|
||||
|
||||
`CreateAgentOptions` takes both separately; `ResumeAgentOptions` takes an `agentId` plus a `resumeSessionId`. They diverge in exactly two places:
|
||||
`CreateAgentOptions` takes both separately; `ResumeAgentOptions` takes an `agentId` plus a `resumeSessionId`. They diverge in exactly three places:
|
||||
|
||||
- **Config-driven create** (`AgentLoop.create`): a stable `agentId` (e.g. `"echo"`) with a fresh per-run `sessionId` (`${id}-session-<uuid>`).
|
||||
- **Resume**: a caller-supplied `agentId` (e.g. `"main"`) on a persisted `resumeSessionId`.
|
||||
- **In-process subagent children**: the backend mints the child's `agentId` and `sessionId` as two independent UUIDs (`packages/subagent/subagent-inprocess/src/index.ts`) that nothing distinguishes — `parentSession` records lineage independently.
|
||||
|
||||
Everywhere a live consumer actually looks an agent up — the **ACP bridge, the only production path** — the two are already unified: `agentId === sessionId === <uuid>`. Concretely, both bridge factory call sites brand `AgentId(sessionId)` directly, and the bridge's reverse lookup keys on the `Agent` object itself — there is no id translation anywhere in the bridge to migrate.
|
||||
Where a live consumer looks an agent up, no lookup needs an id translation: the ACP bridge — the primary production path — already unifies the two (`agentId === sessionId === <uuid>`; both factory call sites brand `AgentId(sessionId)` directly, and its reverse lookup keys on the `Agent` object itself), and the CC hooks bridge resolves subagent children directly by the `agentId` its lifecycle event carries. The one production population whose two ids actually DIVERGE is the in-process subagent children — the same cosmetic separation as the config path, and the same one-field simplification under unification. One consumer already pays the two-id tax: ui-stdio keeps a `labelBySession` map (seeded from the registry, maintained by `agent/created`/`agent/disposed` listeners) solely to translate `session.header.id` back to an agent id for its turn labels — machinery that deletes outright when the ids unify. And the CC hooks bridge stamps `session_id: agent.session.header.id` into every hook payload, so under unification a subagent hook's `session_id` and `agent_id` become the same string — one less identity for a hook author to reconcile.
|
||||
|
||||
The separation is **latent generality no consumer exercises**: nothing reads a *stable* `agentId` back across runs (each process starts fresh, and persistence keys off the session id, never the agent id). The config path's "stable agentId, fresh sessionId" buys nothing concrete — it is cosmetic. And the `agentId !== sessionId` case is precisely what opens the bash owner-token alias hole: the bash completion-notice routes by `session.header.id`, but the registry enforces uniqueness only on `agentId`, so a programmatic caller registering two agents with different agent ids but the SAME session id can mis-route a notice (see [agent lifecycle and ownership seams](../../implemented/architecture/2026-06-18-agent-lifecycle-and-ownership-seams.md) § Seam precondition). The current code documents this as a precondition rather than guaranteeing it.
|
||||
|
||||
@@ -40,7 +41,7 @@ That was the review's first suggestion. It would couple the generic registry to
|
||||
|
||||
## Risks
|
||||
|
||||
This touches public factory interfaces (`CreateAgentOptions`, `ResumeAgentOptions`, `AgentFactory`) and the config-agent id scheme, so it is a deliberate cross-package change, not a local patch — it ships as its own PR (converged with Codex), stacked on the bash owner-token work that surfaced the precondition.
|
||||
This touches public factory interfaces (`CreateAgentOptions`, `ResumeAgentOptions`, `AgentFactory`) and the config-agent id scheme, so it is a deliberate cross-package change, not a local patch — it ships as its own PR (converged with Codex); the bash owner-token precondition it closes is documented in [agent lifecycle and ownership seams](../../implemented/architecture/2026-06-18-agent-lifecycle-and-ownership-seams.md).
|
||||
|
||||
The genuine risks of collapsing the two ids into one (the case AGAINST this proposal — to be weighed honestly before implementing):
|
||||
|
||||
|
||||
@@ -8,7 +8,7 @@ Status: proposed
|
||||
|
||||
## Proposal
|
||||
|
||||
Remove `ImageBlock`, its `ContentBlockMap` entry, and the explicit skip/estimate branches in the deepseek serializer, the pi-ai converter, the ACP codec's outbound mapping, and compact-basic — the default arms those switches already carry for plugin-added block types absorb the cases. Update the vocabulary line in [architecture.md](../../../architecture.md), the block list in `packages/llm/llm/README.md`, the deepseek README's image-skip row, the pi-ai README's images-not-representable row, the compact-basic README's image-estimation and `[image]`-placeholder rows, the pastes in [core.md](../../../core-data-structures/core.md) and [llm-streaming.md](../../../core-data-structures/llm-streaming.md), and the type-equiv manifest; amend the [content-block vocabulary RFC](../../implemented/architecture/2026-06-11-content-block-vocabulary.md)'s block list and multimodal-home consequence per [implemented/AGENTS.md](../../implemented/AGENTS.md); drop or retarget the tests that construct image blocks to exercise the removed branches. The ACP codec's inbound rejection of image PROMPT content is unaffected — that guard is about protocol content a client can send regardless of our vocabulary, and it stays.
|
||||
Remove `ImageBlock`, its `ContentBlockMap` entry, the explicit `image` estimate/placeholder arms in compact-basic, and the image-naming comments in the deepseek serializer's, pi-ai converter's, and ACP codec's default arms — those default arms already absorb the case the way they absorb any unknown block type. Update the vocabulary line in [architecture.md](../../../architecture.md), the block list in `packages/llm/llm/README.md`, the deepseek README's image-skip row, the pi-ai README's images-not-representable row, the compact-basic README's image-estimation and `[image]`-placeholder rows, the pastes in [core.md](../../../core-data-structures/core.md) and [llm-streaming.md](../../../core-data-structures/llm-streaming.md), and the type-equiv manifest; amend the [content-block vocabulary RFC](../../implemented/architecture/2026-06-11-content-block-vocabulary.md)'s block list and multimodal-home consequence per [implemented/AGENTS.md](../../implemented/AGENTS.md); drop or retarget the tests that construct image blocks to exercise the removed branches. The ACP codec's inbound rejection of image PROMPT content is unaffected — that guard is about protocol content a client can send regardless of our vocabulary, and it stays.
|
||||
|
||||
## Why not keep it?
|
||||
|
||||
|
||||
@@ -30,4 +30,4 @@ This RFC deliberately does NOT touch `temperature`, `stop`, or `maxTokens`: thos
|
||||
|
||||
## Risks
|
||||
|
||||
A hooks/config plugin arriving via the interception seams may want to set request fields — it will reach for `temperature`/`stop` (kept, working), not a field adapters reject. If chat-prefix completion or strict mode become product features, the re-add lands with the adapter/endpoint work, where the contract can say what actually happens rather than "everyone throws".
|
||||
The shipped hook bridges set no request fields at all, and a request-mutating plugin (an `agent/request` waterfall listener) would reach for `temperature`/`stop` (kept, working), not a field adapters reject. If chat-prefix completion or strict mode become product features, the re-add lands with the adapter/endpoint work, where the contract can say what actually happens rather than "everyone throws".
|
||||
|
||||
@@ -8,13 +8,13 @@ Three pieces of public spine surface share one defect class: their only possible
|
||||
|
||||
1. **`SurfaceManager.invalidate()`** (`packages/core/session/src/surface.ts`). Its documented trigger — "the log has been replaced wholesale (e.g. after Session seed)" — is structurally unreachable: seeding happens inside the `Session` constructor, `_surface` is created lazily on first access, and the log reference is never reassigned afterward, so no constructed `SurfaceManager` ever observes a wholesale replacement. Sole caller: its own unit test. A rollback primitive protecting a scenario the implementation cannot produce.
|
||||
2. **The `runLoop`, `Inbox`, and `InboxMessage` exports** (`packages/core/agent-loop/src/index.ts`). `runLoop` has no importer outside the package — the only callers are the package's own internals (the agent constructs its loop with it), so the public re-export has zero consumers; `Inbox`/`InboxMessage` likewise reach outside code only through the package's own inbox spec (switchable to the source module). The exports contradict the package's own docs — the inbox module doc says the public surface is `Agent.send()`/`Agent.steer()` — and the [architecture dependency rule](../../../architecture.md): nothing programs against `dsh-agent-loop`; a replacement loop is a different bundle built on `dsh-agent`, not a consumer of this package's internals. `ReactLoopAgent` stays exported (cross-package tests construct it by package name).
|
||||
3. **`ToolExecutionResult.callId`** (`packages/core/tools/src/index.ts`; the *input* `ToolExecution.callId` stays). Zero readers. The loop deliberately ignores it and documents it as a footgun — the correlation id must be the loop's own `call.id`, because a `tools/execute` waterfall listener returning a mismatched id would otherwise orphan the call↔result pairing — and a regression test exists solely to prove the field is ignored. So every waterfall short-circuiter must fabricate a field whose only power is to be a bug if trusted; the ACP bridge correlates via the session event's `data.callId`, never via the execution result.
|
||||
3. **`ToolExecutionResult.callId`** (`packages/core/tools/src/index.ts`; the *input* `ToolExecution.callId` stays). Zero readers — and no listener can even construct a result: `tools/pre-execute`/`tools/post-execute` listeners return Decisions, the registry builds every result itself and always sets `callId` to the input `exec.callId`, and the post-execute dispatch snapshots the outcome before the waterfall precisely so a listener mutating the shared result reference cannot corrupt the id. The loop independently ignores `result.callId` in favor of its own `call.id`, and two regression tests exist solely to prove the field cannot matter (the loop's ignores-result-callId test and the registry's mutation guard). A field that is by construction a copy of its input, defended by snapshot machinery, and pinned by tests proving it is ignored is pure liability surface; the ACP bridge correlates via the session event's `data.callId`, never via the execution result.
|
||||
|
||||
## Proposal
|
||||
|
||||
Delete the method and its test; delete the three export lines and their `packages/core/agent-loop/README.md` rows, pointing the inbox spec at the source module; drop the result field from the type, the registry's construction sites, and `toolErrorResult`, along with the loop's ignore-comment and the proves-ignored regression test — the hazard they guard disappears with the field. Update the `ToolExecutionResult` paste in [tools.md](../../../core-data-structures/tools.md) (and its `scripts/type-equiv.manifest.json` row) and the result-shape row in `packages/core/tools/README.md`; for the `invalidate()` removal, amend the [session-surface RFC](../../implemented/architecture/2026-06-18-session-surface.md)'s full-rebuild-after-wholesale-replacement sentence per [implemented/AGENTS.md](../../implemented/AGENTS.md).
|
||||
Delete the method and its test; delete the three export lines and their `packages/core/agent-loop/README.md` rows, pointing the inbox spec at the source module; drop the result field from the type, the registry's construction sites (the deny result, the dispatch result, `toolErrorResult`, and the post-execute snapshot's `callId` leg), the loop's ignore-comment, the proves-ignored regression test, and the mutation guard's `callId` assertions — the hazard they all pin disappears with the field, while the result's `additionalContext` ferry (a consumed post-execute channel) stays untouched. Update the `ToolExecutionResult` paste in [tools.md](../../../core-data-structures/tools.md) (and its `scripts/type-equiv.manifest.json` row) and the result-shape row in `packages/core/tools/README.md`; for the `invalidate()` removal, amend the [session-surface RFC](../../implemented/architecture/2026-06-18-session-surface.md)'s full-rebuild-after-wholesale-replacement sentence per [implemented/AGENTS.md](../../implemented/AGENTS.md).
|
||||
|
||||
Sequencing: the in-flight surface-cache work (tool-pairing balance caching) neither uses nor touches `invalidate`, so that removal lands after or alongside it mechanically. The `callId` removal waits for the in-flight interception-seams work that splits `tools/execute` into pre/post phases and currently carries the field verbatim — the argument transfers unchanged (post-execute listeners receive the execution object alongside the result), so the removal targets whichever seam shape is on master when implemented.
|
||||
Sequencing: the in-flight surface-cache work (tool-pairing balance caching) neither uses nor touches `invalidate`, so that removal lands after or alongside it mechanically. The execute pipeline is `tools/pre-execute` → dispatch → `tools/post-execute`, and post-execute listeners receive the execution object alongside the result — nothing needs the result's own id.
|
||||
|
||||
## Why not keep them?
|
||||
|
||||
@@ -23,8 +23,8 @@ A future consumer that swaps a session's log in place would want a reset primiti
|
||||
## Acceptance criteria
|
||||
|
||||
- `invalidate()` and the result `callId` appear only in this RFC; `runLoop`/`Inbox`/`InboxMessage` remain package-internal only — no re-export from the package index and no outside-package importer; the agent-loop README lists only the consumed public surface; the inbox spec imports the source module.
|
||||
- The tools/execute contract tests pass with the shrunk result type; no waterfall test fabricates a `callId` on a result.
|
||||
- The pre-/post-execute pipeline contract tests pass with the shrunk result type; the mutation-guard and proves-ignored tests shed their `callId` legs with the hazard they pin.
|
||||
|
||||
## Risks
|
||||
|
||||
All three are compile-visible removals with no runtime behavior change on any shipped path. The `callId` change lands on whatever execute-seam shape is current, as noted under sequencing.
|
||||
All three are compile-visible removals with no runtime behavior change on any shipped path.
|
||||
|
||||
+4
-4
@@ -8,11 +8,11 @@ The merge-extensible vocabulary maps are designed to grow by declaration merging
|
||||
|
||||
- **`CacheHint` and the three `cache?: CacheHint` fields** on `TextBlock`/`ToolResultBlock`/`ImageBlock` (`packages/llm/llm/src/types.ts`). Nothing constructs a block with `cache:` anywhere — src, tests, and doc pastes all come up empty — and neither adapter reads `.cache`: DeepSeek prompt caching is automatic, so the adapters map `prompt_cache_hit_tokens` OUT of responses without ever sending a hint IN. This is Anthropic-style `cache_control` surface with no provider that can honor it.
|
||||
- **`MessageSourceMap.agent`** (`{ kind: 'agent'; agentId: string }`, same file). Zero constructors, tests included. Its intended producer shipped without it: the subagent backends send the parent's prompt to the child with no `source`, so it logs as `{ kind: 'user' }`, and the generic envelope renderer interpolates `source.kind` without ever routing on it. The variant is pasted into [core.md](../../../core-data-structures/core.md).
|
||||
- **`TurnTriggerMap.continuation`** (`packages/core/session/src/types.ts`). The loop structurally cannot emit it — continuation happens *within* a turn as further steps, never as a new turn — and it constructs only `message` and `injection` triggers. The only writers are two hand-built test fixtures that need an arbitrary non-message trigger (`packages/support/llm-replay/tests/llm-replay.spec.ts`, `packages/support/ui-stdio/tests/ui-stdio.spec.ts`); the only production trigger reader, the ACP bridge, filters on `kind === 'message'`. The variant is pasted into [session.md](../../../core-data-structures/session.md).
|
||||
- **`TurnTriggerMap.continuation`** (`packages/core/session/src/types.ts`). The loop structurally cannot emit it — continuation happens *within* a turn as further steps, never as a new turn — and it constructs only `message` and `injection` triggers. The only writer is one hand-built test fixture that needs an arbitrary non-message trigger (`packages/support/llm-replay/tests/llm-replay.spec.ts`); the only production trigger reader, the ACP bridge, filters on `kind === 'message'`. The variant is pasted into [session.md](../../../core-data-structures/session.md).
|
||||
|
||||
## Proposal
|
||||
|
||||
Delete `CacheHint` with its three `cache?` fields, the `agent` message-source variant, and the `continuation` turn-trigger variant. Switch the two test fixtures to `injection` triggers (any non-`message` trigger serves their purpose). Update the type-equiv pastes in [core.md](../../../core-data-structures/core.md) and [session.md](../../../core-data-structures/session.md) (and `scripts/type-equiv.manifest.json` where block identity shifts) in the same change, and amend the [content-block vocabulary RFC](../../implemented/architecture/2026-06-11-content-block-vocabulary.md)'s consequence line naming cache hints as having a home, per [implemented/AGENTS.md](../../implemented/AGENTS.md).
|
||||
Delete `CacheHint` with its three `cache?` fields, the `agent` message-source variant, and the `continuation` turn-trigger variant. Switch the llm-replay fixture to an `injection` trigger (any non-`message` trigger serves its purpose). Update the type-equiv pastes in [core.md](../../../core-data-structures/core.md) and [session.md](../../../core-data-structures/session.md) (and `scripts/type-equiv.manifest.json` where block identity shifts) in the same change, and amend the [content-block vocabulary RFC](../../implemented/architecture/2026-06-11-content-block-vocabulary.md)'s consequence line naming cache hints as having a home, per [implemented/AGENTS.md](../../implemented/AGENTS.md).
|
||||
|
||||
Each variant returns the day it gains a real producer, exactly as the maps are designed to grow: a caching feature re-adds `cache` together with the adapter that transmits it; subagent attribution re-adds `agent` together with the backend that stamps it and a consumer that routes on it; an auto-continue feature that genuinely starts new turns re-adds `continuation` with the plugin that emits it.
|
||||
|
||||
@@ -24,8 +24,8 @@ The [content-block vocabulary RFC](../../implemented/architecture/2026-06-11-con
|
||||
|
||||
- `rg` for `CacheHint`, the `agent` message-source spelling, and the `continuation` trigger spelling returns only this RFC.
|
||||
- The core-data-structures pastes and the type-equiv manifest are in sync (`pnpm run doc-sync` green).
|
||||
- The two fixtures assert the same replay behavior with `injection` triggers; the suite is green.
|
||||
- The fixture asserts the same replay behavior with an `injection` trigger; the suite is green.
|
||||
|
||||
## Risks
|
||||
|
||||
None operational — nothing can construct these values today. The in-flight event-taxonomy work reworks the transient `agent/*` mirror events, not the durable vocabulary declarations, so there is no collision. If the [image-block RFC](2026-07-04-drop-image-content-block.md) ships first, one of the three `cache?` fields leaves with it; the two proposals are independent and compose in either order.
|
||||
None operational — nothing can construct these values today. The event-taxonomy rework that removed the transient `agent/*` mirrors left the durable vocabulary declarations untouched, and the vocabulary the loop and the hook bridges DID add — the `rejected` turn-end reason, the `prompt/blocked` session event, `hook/invoked`/`hook/result` — all arrived together with their producers: live demonstrations of the admission policy this RFC applies retroactively. If the [image-block RFC](2026-07-04-drop-image-content-block.md) ships first, one of the three `cache?` fields leaves with it; the two proposals are independent and compose in either order.
|
||||
|
||||
+2
-2
@@ -21,7 +21,7 @@ This is the seam-vocabulary echo of [prune dead methods from the persistence sea
|
||||
|
||||
## Why not keep it?
|
||||
|
||||
The two-kinds-of-capability design is the seam RFC's headline, and re-adding `outputSchema` later touches several files. But the design survives with `depthLimit` as its live example and the RFCs as its record, and the seam RFC itself concedes the shipped `toolFilter` shape is wrong (real enforcement needs a `tools/execute` veto, not schema filtering) — re-adding against a real implementing provider will pin a better contract than the current speculative one.
|
||||
The two-kinds-of-capability design is the seam RFC's headline, and re-adding `outputSchema` later touches several files. But the design survives with `depthLimit` as its live example and the RFCs as its record, and the seam RFC itself concedes the shipped `toolFilter` shape is wrong (real enforcement needs a `tools/pre-execute` deny in the child's context, not schema filtering) — that deny primitive now exists on the interception seams, so re-adding against a real implementing provider will pin a better contract than the current speculative one.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
@@ -30,4 +30,4 @@ The two-kinds-of-capability design is the seam RFC's headline, and re-adding `ou
|
||||
|
||||
## Risks
|
||||
|
||||
The in-flight hooks stack enriches subagent lifecycle event payloads (agent type, last assistant message) — adjacent files, no field overlap; coordinate landing order mechanically. Worth recording while here: nothing production sets `maxDepth` today (`tool-subagent` exposes no knob for it), so in-process recursion is uncapped — wiring the depth machinery this RFC keeps is a small feature gap, and an argument for keeping it, not for cutting it.
|
||||
The subagent lifecycle events carry `lastAssistantMessage` on the end payload (the [subagent-observe-enrich RFC](../../implemented/feature/2026-06-30-subagent-observe-enrich.md)) — that enrichment lives in the service module, not the seam vocabulary this RFC shrinks, and the same review that shipped it dropped an `agentType` sibling for lacking a consumer: the judgment this RFC extends. The CC hooks bridge, the first outside consumer of those lifecycle events, reads only the event payloads and touches none of the surface removed here; the observe-enrich RFC's deferred control-flow redesign names implementing `resume` as its own future work — exactly the re-add trigger this RFC's pattern anticipates. Worth recording while here: nothing production sets `maxDepth` today (`tool-subagent` exposes no knob for it), so in-process recursion is uncapped — wiring the depth machinery this RFC keeps is a small feature gap, and an argument for keeping it, not for cutting it.
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
# RFC: Remove the `agent/steering` mirror emit
|
||||
|
||||
Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
`agent/steering` is the last remaining transient mirror of a durable session event. The loop's steering drain appends the durable `steering/message { turn, content, source }` and, on the very next line, emits `agent/steering(agent, turn, content, source)` — the identical fact as a fire-and-forget event (`packages/core/agent-loop/src/loop.ts`, `drainSteering`). It has zero production listeners: the only subscriber anywhere is a loop regression test asserting the emit carries `source` — the same fact the durable event already records one line above.
|
||||
|
||||
Both mirror-removal RFCs retained it while explicitly deferring the decision this RFC now makes. The [boundary-mirror removal](../../implemented/simplification/2026-06-20-remove-agent-boundary-mirror-events.md) kept it as "a live control signal, not a boundary"; the [stream-chunk removal](../../implemented/simplification/2026-07-02-remove-stream-chunk-mirror.md) kept it as "a live control signal with no durable twin, retained (its fate is a separate future decision)". The second rationale does not survive the code: the durable twin is `steering/message`, appended immediately before the emit with the same payload. The mirrored-vs-live-only line the taxonomy actually draws puts it on the mirror side: `agent/queued` is genuinely live-only (it fires at enqueue time, before any durable event exists, and already carries a `steering: boolean` flag — cancelled queued work never enters the log), while `agent/steering` fires at the exact moment its durable twin lands, carrying nothing the log does not.
|
||||
|
||||
Steering itself is busier than ever — the hook bridges' turn-continuation decisions inject their reasons through `inbox.steer()`, landing as durable `steering/message` events that the hook-matrix goldens pin — and every one of those consumers observes the durable event. Nothing observes the mirror.
|
||||
|
||||
## Proposal
|
||||
|
||||
Remove the `agent/steering` declaration from `packages/core/agent/src/types.ts` (and its mention in the live-events JSDoc list there), the emit in `drainSteering` (whose `ctx` parameter becomes unused and goes too), the row in `packages/core/agent/README.md`, and the emit line in the loop-pseudocode blocks (`packages/core/agent-loop/src/loop.ts` module doc and [architecture.md](../../../architecture.md)); run `pnpm run gen-cordis-catalog`. Retarget the one regression test at the durable `steering/message` event — the source-preservation fact it pins lives on the log. The implementing PR amends the two retaining RFCs' scope lines per [implemented/AGENTS.md](../../implemented/AGENTS.md): the boundary RFC's retained-list entry and the stream-chunk RFC's "no durable twin" clause.
|
||||
|
||||
## Why not keep it?
|
||||
|
||||
"It is a control signal, not a boundary" — but the taxonomy's operative distinction is mirrored-vs-live-only, not control-vs-boundary, and this event mirrors. A consumer that wants enqueue-time notification has `agent/queued` (with its steering flag); a consumer that wants drain-time notification is by definition asking for the moment `steering/message` is appended, which `session/event` delivers with the same payload plus durability. The rejected [retire-mid-turn-steering RFC](../../rejected/simplification/2026-06-20-retire-mid-turn-steering.md) defended the steering *capability* — `steer()`, the durable event, continuation forcing — all of which this removal keeps untouched.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- No `agent/steering` spelling outside this RFC and the two amended RFCs; the catalog is regenerated and fresh.
|
||||
- The retargeted test pins source preservation on `steering/message`; the suite is green.
|
||||
|
||||
## Risks
|
||||
|
||||
None known: zero production listeners exist to migrate, and both live-notification needs (enqueue, drain) have surviving homes (`agent/queued`, `session/event`).
|
||||
@@ -0,0 +1,29 @@
|
||||
# RFC: Tighten the hook-protocol contract — the `native` dialect, `suppressOutput`, and lib-owned `hook/result` semantics
|
||||
|
||||
Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
Three pieces of the freshly-landed `dsh-hook-protocol` contract miss the discipline the hooks stack itself applied elsewhere (its review dropped a `subagent/end` `agentType` field for lacking a consumer — the same test these fail):
|
||||
|
||||
1. **`HookDialect`'s `'native'` variant** (`packages/hooks/hook-protocol/src/types.ts`) has zero producers — the bridges stamp `'claude'` and `'codex'`; the only `'native'` constructor anywhere is the lib's own unit test. The field's own JSDoc defines `dialect` as "the bridge that ran it", and native is not a bridge: the [interception-seams RFC](../../implemented/feature/2026-06-30-interception-seams.md) records that native hooks are not a package and that "a native plugin can already use the typed Decisions" without the durable hook log, and the flagship native-plugin worked example asserts exactly that (no `hook/*` events at all).
|
||||
2. **`HookOutput.suppressOutput`** (same file) is parsed by the codec and discarded on every path: no bridge branch, no merge fold, no warn, no deferred-list row — uniquely among its parsed-but-unhonored siblings, each of which carries a stated deferral (`updatedInput` → a logged warn plus the [pre-tool-input-rewrite proposal](../feature/2026-06-30-pre-tool-input-rewrite.md); `systemMessage` → a logged warn plus a README deferred row; `continue`/`stopReason` → a `TODO(hook-continue-false)` anchor plus the `'stop'` decision record). Structurally there is nothing to suppress: hook stdout never enters any transcript (context flows only via `additionalContext`; the log records only `decision`/`stderrSummary`), so a hook author setting `suppressOutput: true` gets silent nothing with no warn.
|
||||
3. **The `hook/result` semantics live in the bridges, twice, not in the lib that owns the event.** `summarize()` — the 500-character stderr truncation rule — is byte-identical in `packages/hooks/hooks-claude/src/index.ts` and `packages/hooks/hooks-codex/src/index.ts`, and so is the decision-string rule `output.decision ?? (output.continue === false ? 'stop' : 'pass')`; yet `dsh-hook-protocol` declares `hook/result`, documents `stderrSummary` as "truncated" without owning the truncation, and documents the decision values without owning the mapping. If one bridge drifts (a different cap, a different fallback), the shared durable event's semantics fork silently.
|
||||
|
||||
## Proposal
|
||||
|
||||
Narrow `HookDialect` to `'claude' | 'codex'` and fix its JSDoc; retarget the lib's one `'native'` test. Drop `suppressOutput` from `HookOutput`, the codec's parse lines, its codec-test assertions, and the parsed-superset lists in the lib README and [hook-protocol-lib RFC](../../implemented/feature/2026-06-30-hook-protocol-lib.md) (amended per [implemented/AGENTS.md](../../implemented/AGENTS.md)). Move the `hook/result` semantics into the lib: `appendHookResult` (or a helper it exposes) derives `stderrSummary` and the decision string from the `HookOutput` + exit outcome, and both bridges delete their private copies. Rider: un-export `BLOCKING_EXIT_CODE` (zero importers; even the codec tests spell the literal `2`).
|
||||
|
||||
## Why not keep them?
|
||||
|
||||
The hook-protocol-lib RFC deliberately records "parses the full CC superset", and `'native'`/`suppressOutput` are days old — the strongest counterargument is that this re-litigates fresh decisions. But parsing a field whose value can never influence anything is not protocol faithfulness, it is a reader trap; and a dialect variant that the design's own thesis says will never be stamped is vocabulary without an interpreter — the exact bar the stack's own review enforced when it dropped `agentType`. Both return trivially with their first real producer (a transcript surface that has hook stdout to suppress; a native-provenance feature that logs hook events). On item 3, the lib RFC chose per-bridge explicitness over a parameterized engine — but that choice governed payload construction and Decision mapping; the semantics of the SHARED durable event are precisely the "primitives where duplication would actually be dangerous" that the same RFC assigns to the lib.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- `HookDialect` is two-valued; `rg "'native'"` in the hooks packages returns only this RFC's amended references.
|
||||
- `suppressOutput` appears nowhere in source, tests, or parsed-field doc lists.
|
||||
- One definition each of the truncation rule and the decision-string rule, in `dsh-hook-protocol`, exercised by both bridges' suites; the hook-matrix snapshot goldens are byte-identical.
|
||||
|
||||
## Risks
|
||||
|
||||
All three changes are invisible on the wire and in the goldens (`dialect` values emitted today are `claude`/`codex`; `suppressOutput` influences nothing; the folded semantics are the same rules). The cost is touching a week-old package — cheap now, per the pre-release stance, and cheaper than letting two copies of a durable event's semantics age apart.
|
||||
@@ -6,7 +6,7 @@ Status: proposed
|
||||
|
||||
Two pieces of `dsh-acp` surface are unreachable from any shipped configuration:
|
||||
|
||||
1. **`AcpConfig.agentName` / `agentVersion`** (`packages/ui/acp/src/index.ts`). The shipped app package hands the bridge only `{ model, systemPrompt }` (`packages/ui/acp-agent/src/index.ts`), so no leaf `cordis.yml` — the only production config surface — can set the knobs at all; they are settable solely by direct-mounting the bridge, which only a unit test does. Every snapshot golden pins the schema defaults (`deepseek-harness-acp` / `0.0.1`). The pair also carries a live `TODO(double-default)`: the literals exist twice (schema `.default(...)` plus `??` fallbacks), with the TODO asking to pick one home.
|
||||
1. **`AcpConfig.agentName` / `agentVersion`** (`packages/ui/acp/src/index.ts`). The shipped app package hands the bridge only `{ model, systemPrompt }` (`packages/ui/acp-agent/src/index.ts`), so no leaf `cordis.yml` — the only production config surface — can set the knobs at all; they are settable solely by direct-mounting the bridge, which only a unit test does. Every snapshot golden — the hook-matrix scenarios included — pins the schema defaults (`deepseek-harness-acp` / `0.0.1`). The pair also carries a live `TODO(double-default)`: the literals exist twice (schema `.default(...)` plus `??` fallbacks), with the TODO asking to pick one home.
|
||||
2. **The `toolKindFor` name heuristic** (same file) special-cases `bash*`/`read*`/`write`/`edit*` tool names in the generic-fallback path. Since the [render-intent union](../../implemented/architecture/2026-07-02-tool-render-intent-union.md), every first-party tool those arms match ships its own `presentCall` carrying its kind, and the presenter-less production tools (`subagent`, `subagent_fork`) fall through to `other` anyway. The arms are production-reachable only when a tool's `presentCall` THROWS (the containment fallback) — and the bridge's own module doc states the design rule the heuristic violates: "the bridge never special-cases tool names".
|
||||
|
||||
## Proposal
|
||||
|
||||
Reference in New Issue
Block a user