Files

T

Tianyi Cui 9ebc38badf 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.

2026-07-04 11:24:44 +08:00

5.1 KiB

Raw Blame History

RFC: Prune dead core-spine surface — `SurfaceManager.invalidate()`, the loop-internal exports, `ToolExecutionResult.callId`

Status: proposed

Problem

Three pieces of public spine surface share one defect class: their only possible role is to be ignored, or their trigger is unreachable.

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.
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: 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).
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 (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 (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's full-rebuild-after-wholesale-replacement sentence per 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 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?

A future consumer that swaps a session's log in place would want a reset primitive — it re-adds invalidate with itself. A replacement-loop author might want to reuse the inbox or the driver — the architecture already answers that a replacement loop is a different bundle. An isolated result-logging listener might want self-contained correlation on the result — the execution object is in scope at every listener, and a field that exists only to be ignored is worse than absent: it invites exactly the orphaned-pairing bug the loop comment warns about.

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 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.

5.1 KiB Raw Blame History

RFC: Prune dead core-spine surface — SurfaceManager.invalidate(), the loop-internal exports, ToolExecutionResult.callId