The core-spine RFC's acceptance criterion still asserted all three surfaces 'appear only in this RFC', contradicting the corrected scope (runLoop/Inbox stay as package-internal symbols; only the public re-exports go). Also add the deepseek README image-skip row and the compact-basic [image]-placeholder row to the image RFC's removal set.
4.8 KiB
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 theSessionconstructor,_surfaceis created lazily on first access, and the log reference is never reassigned afterward, so no constructedSurfaceManagerever observes a wholesale replacement. Sole caller: its own unit test. A rollback primitive protecting a scenario the implementation cannot produce.- The
runLoop,Inbox, andInboxMessageexports (packages/core/agent-loop/src/index.ts).runLoophas 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/InboxMessagelikewise 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 isAgent.send()/Agent.steer()— and the architecture dependency rule: nothing programs againstdsh-agent-loop; a replacement loop is a different bundle built ondsh-agent, not a consumer of this package's internals.ReactLoopAgentstays exported (cross-package tests construct it by package name). ToolExecutionResult.callId(packages/core/tools/src/index.ts; the inputToolExecution.callIdstays). Zero readers. The loop deliberately ignores it and documents it as a footgun — the correlation id must be the loop's owncall.id, because atools/executewaterfall 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'sdata.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 (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 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.
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 resultcallIdappear only in this RFC;runLoop/Inbox/InboxMessageremain 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
callIdon a result.
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.