The continuation-override comment said "step-end/continuation listeners". With no
agent/step-end emit, the surviving step-boundary listener is the durable step/end
SESSION event, so spell it "step/end session-event/continuation listeners" to avoid
implying a removed agent/* mirror. Comment-only; no behavior change.
Second-round Codex review of the PR-A taxonomy change found four issues, all
verified against the code:
- The /goal regression guard asserted only that the steered content reached
requests[1], which passes even with the hasSteering override (loop.ts) disabled:
leftover steering is re-enqueued as a next-turn queued message and also lands in
requests[1], one turn later. The guard now asserts the same-turn shape — ONE
turn, TWO steps, a steering/message recorded before step 2 — which is the
mechanism the override drives. Proven to fail red with the override disabled.
- The event-domain-semantics RFC's consequence list still described the pre-fix
behavior (step marked open AFTER step/start, so no step/end owed). It now states
the shipped behavior: the loop marks the step open BEFORE the append, so a
throwing step/start listener gets a balancing step/end via closeStep().
- architecture.md's loop pseudocode said only continuation listeners force
continuation; step/end session-event listeners (the /goal pattern) do too.
- The agent/turn-end JSDoc listed a `rejected` TurnEndReason that does not exist on
this branch (it belongs to the later interception work). Removed it and
regenerated the cordis catalog; `interrupted` (a real variant) stays.
Codex review of PR-A found three blockers:
- A throwing step/start session-event listener left an unbalanced log
(turn/start → step/start → turn/end with no step/end), which the invariants
oracle rejects — masked because that rejection was itself contained as a
throwing turn/end listener. Fix the root cause in the loop: mark the step open
BEFORE appending step/start (Session.append pushes before notifying), so the
outer catch's closeStep() appends the balancing step/end. The test now asserts
the balanced outcome (stepEnd:1, step/end before turn/end); proven load-bearing
(revert the reorder → the test goes red with stepEnd:0).
- Reintroduce the /goal-pattern guard deleted in the prior commit, migrated to a
step/end session-event listener (the surviving step-boundary hook point), with
a no-tools first step so it exercises the hasSteering continuation override.
- Update packages/core/agent/README.md: step boundaries are no longer agent/*
emits.
Pin the three-domain rule (session = durable fact log, agent = live runtime
surface, tools = registry/exec): a durable replayable fact is a SessionEvent; a
live interception or transient/live-object signal is an agent/tools Cordis
event. A boundary that is both is mirrored as an agent/* emit ONLY where a live
consumer needs the Agent handle.
Apply it to the boundary twins: drop agent/step-start and agent/step-end (no
production consumer needs the live Agent at a step boundary — consumers read the
durable step/start/step/end session events). Keep agent/turn-start/turn-end (the
stdio UI labels output by agent.id). Tests that observed step boundaries via the
removed emits now observe the durable session events; the pinned behavior is
unchanged.
Conservative subset of the proposed "remove boundary mirror events"
simplification; foundation for the Hooks subsystem's canonical event surface.
The registry's validateArgs rejects a bad `status` enum before execute runs, so
the in-body re-check (`status !== 'pending' && …` → throw) was unreachable dead
code — line 70 was uncovered, failing the per-file 100% coverage gate. Narrow
the registry-guaranteed value with `status as TodoItem['status']` instead of
re-validating it, mirroring tool-bash (which only checks what the DSL can't
express). The malformed-status test still passes — it exercises the registry's
rejection, the actual path. Coverage back to 100%.
Codex confirmation review: the trim-the-stored-content fix had no test that
would fail if it regressed (existing assertions use already-trimmed todos).
Add a focused test asserting " plan the work " appends content "plan the
work". Verified it fails red against the pre-fix code.
toTodoList dedupes and length-checks on the trimmed content but stored the raw
item.content, so a todo with leading/trailing whitespace was deduped by its
trimmed form yet persisted untrimmed — the stored value and the uniqueness key
could differ. Store the trimmed content so the persisted list matches what was
validated.
Add @deepseek-ai/dsh-tool-todo (a new packages/todo/ group): a model-facing
todo_write(todos: [{content, status}]) tool with whole-list-replace semantics.
Each call appends the full list as a todo/write event to the calling agent's
session log; the current list is the most recent such event (last-write-wins).
Single-owner — a non-agent caller is rejected. Beyond the schema's
type/required/enum checks, execute rejects empty/duplicate content and more than
one in_progress task, narrowing the loosely-typed args into a real TodoItem[].
Both UIs render off the existing session/event: the stdio UI prints a glyphed
checklist; the ACP bridge maps the list to a `plan` sessionUpdate (todosToPlan
synthesizes the priority ACP requires; status maps 1:1). Wired into the
coding-agent, acp-agent, and snapshot example configs with a system-prompt nudge.
Tests: unit (schema, validation, append/replace, no-agent rejection, presentCall,
HMR-safety, Loader export-shape guard), full-loop integration through the agent
loop, the ACP todosToPlan mapping + stream-update arm, the stdio render arm, and
a session/load replay that re-emits the plan. New-group TS wiring added to
tsconfig.base/json/build. RFC + a doc-inventory sweep (architecture, packages
README, AGENTS layout, cookbook group list, example READMEs) ship with it.
The todo-plan ACP snapshot scenario is recorded separately (needs an API key).
Codex Phase 1 review: the event JSDoc described Phase 2 consumers (the
todo_write tool, stdio printing, ACP plan mapping) as current state, and put an
@mode tag on a SessionEventMap member. @mode is for first-class Cordis
`interface Events` entries the catalog generator reads — this event rides the
existing session/event emit and has no catalog row, so the tag was wrong.
Trim the JSDoc to the event's own contract (snapshot data shape,
last-write-wins, not-a-surface-event) and drop @mode; phrase TodoItem in terms
of its own purpose rather than a not-yet-present tool.
Add the TodoItem type and a todo/write SessionEventMap variant carrying the
whole todo list as a snapshot (last-write-wins on replay). It is NOT a
SurfaceEventType: it produces no LLM message and never reaches
deriveMessages(), so it carries no surfaceOp and stays off the surface — it is
durable, replayable UI state that rides the existing session/event emit.
Tests cover the snapshot-clone-on-append contract, last-write-wins, the
not-on-surface guarantee, and a seeded replay round-trip. Docs: session.md
gains the TodoItem type-equiv block + the event member; core.md's variant count
goes to twelve; the type-equiv manifest gains TodoItem.
The per-file 100% coverage gate flagged surface.ts line 46 — the
branch where a surface-eligible event type carries no surfaceOp marker
(isSurfaceEvent returns false). Exercise both guards directly: the
type-only eligibility check, the positive narrowing path, a
non-eligible type, and the markerless-but-eligible branch.
P1: both merge parents shipped SCHEMA_VERSION=3 for different layouts (surface
columns vs seed_length), so an on-disk 3 was ambiguous and wrongly accepted.
Bump to 4 (merged layout) so the version check rejects both sibling v3s.
P2: a surface-eligible event with no surfaceOp lands in the log but vanishes
from deriveMessages() (surface is the sole derivation path). The typed append
overload enforces the marker only when the type arg is a literal; it collapses
to optional when widened to the union (a caller iterating raw events). Guard at
runtime in both append() and the seed constructor — no backward-compat for
surface-less logs. Shared seed fixtures carry surfaceOp explicitly and the
appendLog helper forwards it verbatim (no synthesized default). Exports
isSurfaceEligibleType. Regression tests for all three, each verified to fail
on the unfixed code.
Gates: typecheck, test (1115), snapshot (14), doc-sync, lint, build, hygiene green.
Reconciles the session-surface work (surfaceOp/sourceEventSeqs provenance as
the sole derivation path) with master's worktree-subagent series (fork-seed
boundary + out-of-process subagent backends).
Semantic reconciliations beyond the textual auto-merge:
- SQLite SCHEMA_VERSION: both sides bumped 2->3. Merged to a single v3 carrying
BOTH column families — master's seed_length on `sessions` and surface's
source_event_seqs/surface_op on `events`. writeRow + both INSERT sites bind
the full set; the schema doc lists all three added columns as the v2->v3 gap.
- agent-loop runStep request: master's `sessionId: session.id` and surface's
per-append surfaceOp/sourceEventSeqs coexist (different regions).
- Fork seed + surface: a fork seeds the child from the parent's LIVE events,
which now carry surfaceOp, so the child's surface rebuilds correctly. Verified
end-to-end — the subagent-fork replay recalls the inherited "SAFFRON" codeword
through the seeded prefix.
- Subagent snapshot fixtures (recorded pre-surface) re-enriched via KEYLESS
deterministic replay: only surfaceOp/sourceEventSeqs added onto existing
recorded lines (matched by seq), no recorded value changed. Not re-recorded
against the live API.
Gates: typecheck, test (1112), test:snapshot (14), doc-sync, lint, build,
hygiene all green.
A fork subagent seeds its child session with a prefix of the parent's log, and
that seed becomes the child's persisted log — so a fork child's .jsonl begins
with the PARENT's events, including the parent's assistant/chunk events. The
snapshot replay harness derived a child's script from its whole log, which would
replay the parent's recorded responses as the child's model calls. Spawn-only
scenarios never hit it, but a fork snapshot would mis-route silently.
Record the seed boundary and skip the inherited prefix at replay:
- SessionHeader gains an optional `seedLength` (how many leading events were
inherited via a seed), threaded through CreateSessionOptions/CreateAgentOptions
meta and stamped by the fork backend (= seeded-prefix length; absent for spawn).
It is EXPLICIT, never inferred from seed.length: a resume seeds the whole stored
log, so the resume path passes the persisted boundary back.
- Both persistence backends round-trip it: JSONL header line, SQLite seed_length
column. The SQLite table change bumps SCHEMA_VERSION 2->3; per the pre-release
stance the backend rejects an older user_version on open with NO migration.
- llm-replay's parseSessionHeader reads seedLength and loadSessionScripts derives
a child script from events AFTER the boundary. seedLength is 0 for spawn, so
spawn replay is byte-for-byte unchanged.
Closes the routing-correctness gap the per-session snapshot replay RFC under-
stated; a recorded fork scenario remains a future addition but now derives
correctly. RFC: docs/rfc/implemented/testing/2026-06-22-fork-child-replay-seed-boundary.md.
Regression coverage: a fork child fixture whose seeded prefix carries a parent
chunk (derived script must exclude it, proven red without the slice); a seedLength
persistence round-trip through the shared coordinator contract (both backends);
the fork backend stamping it; resume preserving it from the persisted header.
Two findings on the ACP backend:
Blocking: cancel() only sent session/cancel, so a child that ignores the notify
or wedges the prompt left result hung forever — the model-facing tool awaits
result before its finally disposes, so the parent cancellation hung and the
child stayed alive, violating the SubagentRun.cancel() contract (result settles
aborted). The result path now races the ACP drive against a cancelSettled
promise that requestCancel resolves, so result settles aborted the instant a
cancel is requested, regardless of the child. dispose() still kills+reaps the
process. New MOCK_IGNORE_CANCEL mock mode (receives cancel, never resolves the
prompt, never exits) drives a regression proven to hang without the race.
Nit: the drive-path catch was an empty broad catch that discarded the error
(AGENTS.md forbids). Because cancellation is now handled by the race arm, a
rejection reaching the catch is always a genuine child-level error — bind it,
flatten to error, and surface the original via a new AcpRunSpec.onError sink
that the provider wires to ctx.logger.warn, so a real fault is preserved.
addEventListener('abort') does not fire for a signal already aborted before the
listener is added, so a parent step cancelled before the subagent tool ran
would never reach the child — the tool leaned on each provider re-checking
request.signal itself, leaving the bridge's own claim incomplete for any
provider that relies on run.cancel(). Re-check exec.signal.aborted right after
registering and cancel explicitly. Regression test uses a spy provider that
only reacts to cancel() (never inspects the signal); proven to hang without the
fix (result never settles) and settle aborted with it.
Adds the @deepseek-ai/dsh-compact interface package: the abstract
CompactService (ctx.compact) with compactIfNeeded / compactRegion, the
compact/* session-event types via SessionEventMap declaration merging, and the
capability-seam RFC. Wires the package into the three root tsconfigs and the
cordis catalog. A backend implementation lands separately.
Master's #36 moved declaration output to lib/types (and types/exports/files
point there). The merge applied that to all pre-existing packages, but the
subagent backends introduced on this stack (subagent-inprocess, subagent-spawn,
subagent-fork) still used the old lib/ layout. Bring them onto the new
convention and add them to the single typecheck tsconfig.json references.
The shared run driver lived inside dsh-subagent-spawn, so the spawn package
carried fork-aware seeding logic and dsh-subagent-fork depended backward on
dsh-subagent-spawn — the two in-process backends were not independent.
Move the driver (startInProcessRun, depthOf, SubagentDepthError,
InProcessRunOptions) into a new pure-library package
@deepseek-ai/dsh-subagent-inprocess that registers nothing. spawn and fork now
both depend only on that driver and neither knows about the other; spawn no
longer re-exports it and fork no longer imports from spawn.
Also wire BOTH backends in examples/coding-agent/cordis.yml (config-only): load
dsh-subagent-spawn + dsh-subagent-fork + two dsh-tool-subagent instances with
distinct toolNames (subagent → spawn, subagent_fork → fork), demonstrating that
exposing multiple transports needs no code change.
Two round-3 findings:
(A) The EOF-quiesce window reused the 3000ms SIGTERM grace, the SAME value as
dsh-bash-local's own SIGTERM->SIGKILL grace. The child acp-agent's EOF teardown
disposes its loop, which stops child-owned bash -- and a SIGTERM-trapping bash
grandchild can hold that for up to ~3s before its own SIGKILL, then the child
still owes a final flush. With both graces equal, the parent's SIGTERM fired
exactly as the child reached its own SIGKILL+flush, cutting it off. Split the
EOF grace into its own knob (disposeEofGraceMs, default 6000ms) that exceeds a
single signal-grace of nested-teardown headroom. The child is an arbitrary ACP
agent, so the value is a standalone generous default, NOT derived from any
child's internals. Tier-1 test now uses a flush that outlasts the SIGTERM grace
but fits the EOF grace, so it lands only because the EOF tier honors its own
wider window (proven RED when tier 1 reuses the small SIGTERM grace).
(B) The middle-tier (SIGTERM) test only asserted dispose returned in time, so
an EOF->SIGKILL ladder with the rung removed would still pass. The mock's
MOCK_IGNORE_EOF mode now installs a SIGTERM handler that touches an observable
marker before exiting; SIGKILL is uncatchable, so removing the SIGTERM rung
leaves the marker absent (proven RED). The test asserts the marker exists.
dispose() ended stdin and sent SIGTERM in the same tick, so the child's
EOF-driven quiesce had no window to run. The real acp-agent has no SIGTERM
handler in a normal session — it flushes persistence and stops child-owned
work via the server bridge's connection-close path (conn.closed → per-agent
dispose → final session/flush), driven by stdin EOF, NOT by a signal. A prompt
response can resolve from a turn/end before that post-turn flush lands, so the
child still owes durable work when dispose runs; a same-tick default SIGTERM
terminated it mid-flush, orphaning child-owned bash and dropping the flush.
dispose now waits for the child's natural exit after stdin EOF first, then
escalates SIGTERM (grace), then SIGKILL — a three-tier ladder. Add an
`exitsWithin` helper for the bounded waits.
Regression coverage: a new mock mode (MOCK_FLUSH_ON_EOF) flushes a marker
asynchronously on EOF then self-exits; the tier-1 test asserts the marker
lands (proven RED on the same-tick-SIGTERM ordering — child killed mid-flush).
MOCK_IGNORE_EOF covers the middle tier (ignores EOF, dies on default SIGTERM);
the existing MOCK_TRAP_SIGTERM test covers the SIGKILL tier.
Two lifecycle findings from the review:
- A (blocker): dispose() could hang forever. It only sent SIGTERM and awaited
exit, with no escalation — a child that traps SIGTERM (or our acp-agent if it
doesn't quiesce on stdin EOF) would wedge dispose, stranding tool-subagent's
finally cleanup and orphaning child-owned work (e.g. bash subprocesses). dispose
now: ends stdin (graceful ACP close so the child can flush + exit), SIGTERM,
then escalates to SIGKILL if it doesn't exit within a grace period
(DEFAULT_DISPOSE_GRACE_MS, injectable via spec.disposeGraceMs), awaiting the
certain exit. Mirrors the bash executor's bounded teardown. Regression test
drives a SIGTERM-trapping mock subprocess and asserts dispose returns promptly
— proven to hang (red) without the escalation.
- B: an already-aborted request still spawned the configured binary. startAcpRun
now returns an inert already-aborted run BEFORE spawning, so a pre-cancelled
request launches nothing. Test points the command at `touch <sentinel>` and
asserts the sentinel never appears.
The dispose regression test exposed (via systematic-debugging) that the child
must signal trap-armed readiness before the test cancels — a bare timeout raced
the trap install and the default SIGTERM handler killed the child, making the
guard a no-op. The mock now touches its ready file once the trap is in place and
the test waits on that condition. The `cancelled` flag moved onto a holder object
so TS control-flow doesn't narrow the catch-time read to always-false.
The first OUT-OF-PROCESS subagent backend, proving the seam generalizes past the
in-process backends. @deepseek-ai/dsh-subagent-acp runs each child agent in a
spawned subprocess, driven over the Agent Client Protocol as the CLIENT — the
direction-inverted twin of the dsh-acp server bridge. Point the configured
command at the acp-agent example and the harness talks to its own process.
- Fresh process per run: start spawns, runs one ACP session (initialize →
newSession → prompt), dispose kills the subprocess and awaits its exit.
- Minimal client stub: advertises no fs/terminal; accumulates agent_message_chunk
text as the result output; auto-answers session/request_permission by a
configured policy (reject default / allow). No start-time capabilities (an
out-of-process child can't enforce the parent's depth/tool-filter); ignores
request.parent; injects only `subagents`.
- StopReason mapping (end_turn→completed, cancelled→aborted, …); result resolves
error/aborted on a child failure, never rejects (seam contract).
- Security: credential-shaped ambient env vars are scrubbed; the child's own key
is forwarded only via explicit config.env. A spawn-level error (ENOENT) is
captured and raced against the ACP drive so a bad command settles error rather
than crashing the parent.
Testing designed at every tier: keyless integration drives a scripted mock ACP
server subprocess (cancellation incl. the pre-newSession race and a
torn-pipe-after-cancel, permission auto-answer, non-message updates, spawn
failure, HMR, export shape) at 100% coverage; a with-key e2e drives the REAL
acp-agent example process (PONG + real file write, verified on disk) — the
harness driving itself. Snapshot coverage of an ACP child is deferred as
TODO(acp-subagent-replay) (each child is its own process with its own replay).
Stayed on @agentclientprotocol/sdk 0.25.1: the proposed 0.28.x bump only
deprecates the stable ClientSideConnection/AgentSideConnection API this layer
uses (33 sites incl. the server bridge), turning no-deprecated red across code
this PR shouldn't rewrite — that fluent-API migration is its own follow-up. The
backend needs nothing 0.28.x adds.
This completes the subagent seam stack (PR1 interface → PR2 in-process → PR2.5
snapshot infra → PR3 ACP); the seam RFC moves to implemented/, amended.
Reconcile the session-surface feature with master's package reorg and
simplifications:
- Adopt master's folded usage (assistant/message.usage; standalone `usage`
event dropped) and re-attach surface metadata (surfaceOp/sourceEventSeqs).
- Add surface opts to master's new max-tokens assistant/message append.
- Port surface columns onto the coordinator-refactored SQLite backend at its
new path; drop the dead v1->v2 migration (bump-and-reject, no migration per
pre-release policy).
- Move the session-surface RFC into implemented/architecture/ and refresh its
stale body (no migration, SESSION_FORMAT_VERSION=0, renamed package paths).
- Update the core-data-structures catalog SessionEvent blocks for the two new
surface fields; regenerate the cordis catalog.
- Re-harvest ACP snapshot fixtures (keyless replay) to carry surface metadata.
The createdAt+recordedId child sort comment over-claimed "tie-safe". Codex
flagged that a same-millisecond sibling tie would be broken by random session
id, which does not recover first-call order. In the current synchronous cut that
tie is unreachable — the subagent tool awaits one child's result and disposes it
before the parent starts the next, so siblings' createdAt values are strictly
ordered and match first-call order. Restate the comment to that real invariant
(at both the replay sort and the harvest sort), note that the id tiebreak only
makes a degenerate collision deterministic, and flag the concurrent-subagent cut
that would need a real first-call ordinal with XXX(concurrent-subagents). The RFC
records the same limitation. Comment/doc only — no behavior change.
The snapshot tier was built single-session: dsh-llm-replay served calls from
one global positional cursor, and the harness harvested one session log. A
subagent runs as a second agent with its own session, so a parent→child
scenario could neither replay deterministically nor harvest the child's log.
This resolves the TODO(subagent-snapshots) deferral from the subagent RFC.
- Stamp the calling session id onto the model request: GenerateOptions.sessionId
(typed Branded<'SessionId'> to avoid the dsh-llm↔dsh-session cycle), set by the
agent loop from agent.session.id. Adapters ignore it; an llm/stream listener
routes by it.
- Key replay per session: dsh-llm-replay loads the parent log plus one per child
(childFiles / $DSH_SNAPSHOT_CHILD_FILES), derives a script per recorded session,
and binds each live (freshly-random) session to a recorded script by first-call
order — parent first (earliest createdAt, first to stream). Keys by WHO calls,
so it survives a future concurrent/backgrounded subagent; a global cursor would
not. An unrecorded extra session fails loud.
- Harvest every log: the harness collects all .jsonl across cwd buckets, ordered
primary-first (top-level, then children by createdAt), and RunResult exposes the
plural sessionLogs. The spec writes each back on record (session.jsonl +
session.<n>.jsonl) and diffs each against its fixture on replay.
- Wire the subagent seam + spawn + fork + tool into the acp-agent example (both
cordis configs) and add two nested scenarios recorded against the real API:
subagent-spawn (parent + 1 child) and subagent-multi (parent + 2 children, 3
sessions). Both replay keyless in the default gate.
A new RFC documents the design (docs/rfc/implemented/testing/). Single-session
replay is unchanged (a call with no sessionId is one anonymous primary session).
TODO follow-up: a dedicated branded-ids package could own the SessionId brand and
dissolve the cross-package cycle note; out of scope for this testing PR.
A request signal aborted BEFORE the run starts never fires an `abort` event
(`addEventListener` only fires on the transition), so the backend-level bridge
missed it and ran the child to `completed`. The driver now checks
`request.signal?.aborted` at the top of the result path and settles `aborted`
without running the child. Regression test proven red on the pre-fix code.
Also refresh two stale RFC prose blocks the round-1 fix left behind: the
subagent RFC's Problem statement (cited the removed `TODO(sub-agents)` markers
and claimed nothing existed yet) and the unify-id RFC's fork/spawn risk bullet
(described the seam as "explicitly deferred" via `AgentLoop.create`'s old TODO),
now pointing at the realized seam.
