The prior fix handled a throwing session/event listener on the turn/start
append, but the SAME push-before-notify hazard remained on the three
FINALIZER appends. Session.append pushes the event before notifying, so a
throwing listener on a finalizer event left the event logged but aborted
the rest of finalization — stranding the turn open.
- failTurn(): set `reason` BEFORE appending the `error` event, and contain
a throwing session/event listener on it (the event is already logged
either way). Otherwise reason stayed unset, agent/error was skipped, and
the caller's closeTurn(false) never ran → open turn.
- closeStep(): the try/catch wrapped only the agent/step-end EMIT, not the
step/end APPEND. A throwing session/event listener on step/end escaped —
fatal when closeStep runs from the outer catch during finalization
(turn/start + step/end but no turn/end). Now both the append and the emit
are contained and surface as a turn error via failTurn.
- closeTurn(): contain a throwing session/event listener on the turn/end
append (it would propagate to the runLoop backstop from closeTurn(false),
or skip the turn-end emit from closeTurn(true)). turn/end is logged
either way, so the turn stays balanced.
Regressions: a throwing session/event listener on the error event, on
step/end during finalization (driven by a throwing agent/step-start), and
on turn/end — each leaves a balanced turn and the loop survives.
Session.append pushes the event BEFORE notifying session/event listeners,
so a throwing listener leaves the event in the log while the line after
the append (a boolean flag) never runs. Both turn-balance decisions were
gated on such flags, so a throwing listener could strand an open turn or
skip a durability checkpoint.
- loop.ts: the outer catch decided "turn/end owed" from `turnStarted`.
A throwing listener on the turn/start append left turn/start logged but
the flag false → catch rethrew and skipped turn/end → permanently open
turn (violating ADR 0017). Now decided from the log (this turn's
turn/start present), so the turn is always balanced; only a genuine
pre-push failure (non-serializable trigger — turn/start never logged) is
rethrown to the runLoop backstop. Removed the now-dead `turnStarted`.
- agent.ts inject(): the idle one-shot-turn flush was gated on a
`turnRecorded` flag set after append('turn/end'); a throwing turn/end
listener skipped the flush, losing the balanced in-memory injection turn
on crash. Now the flush decision is read from the log, the synthetic
turn/end append contains a throwing listener (turn stays balanced), and
a failing idle flush is reported via agent/error (step 0 convention) AND
the logger — mirroring the loop's post-turn/end flush path — with a
throwing agent/error listener contained.
Rewrote the test that encoded the old (buggy) "turn/start listener throw
is rethrown, no turn/end" semantics to assert the balanced-turn contract,
and added regressions for the throwing-turn/end-listener flush and the
agent/error report. Updated Agent.inject JSDoc.
Every session event now lives inside a turn (between turn/start and its
turn/end). The loop records queued user/message events AFTER turn/start;
an idle agent.inject() wraps its context/message in a one-shot injection
turn. This makes the turn the single durability/replay boundary so a
persistence backend can treat anything after the last turn/end as a
crash tail without dropping legitimate between-turn context.
A failure once the turn is already closed (rejecting session/flush, a
throwing agent/turn-end listener) has no in-turn position for a session
error event, so it is reported via agent/error + logger only; the turn
stays balanced. failTurn appends an error event only while the turn is
open.
The dsh-invariants plugin enforces turn-enclosure via a default case:
every non-boundary event type — including plugin-added merge-extensible
keys — must sit inside an open turn or it throws.
Documented in ADR 0017 + architecture.md.
closeStep() previously caught and silently swallowed a throw from
agent/step-end emit. In the normal no-tool/no-steering path, this
caused runTurn to reach closeTurn(true) with reason still
{kind:completed}, so the session recorded a completed turn with
zero error events — even though a plugin had failed at a loop
boundary. This violates the contract that a throwing plugin is
contained as a turn error, not a silent success.
Now the catch calls failTurn(toError(error)), which appends the
single error event and sets reason={kind:error,…}. failTurn is
idempotent (errorReported guard), so existing error paths that
call failTurn after closeStep are unaffected.
Add regression test: a throwing agent/step-end listener during a
successful step now produces exactly one error event, a turn/end
with reason error, balanced boundaries (step/end before turn/end),
and a surviving loop.
The loop passed the authoritative call.id into ctx.tools.execute() but then
appended tool/result using result.callId — the value a tools/execute waterfall
listener returns — with no check. A listener returning a mismatched id silently
recorded the result under the wrong call. callId is the model-transcript
correlation id: deriveMessages() turns it into the tool-result block's
toolCallId, which must pair with the assistant tool-call block; a wrong id
orphans that pairing in the next model request.
Append tool/result with callId: call.id (the loop's authoritative id). A
listener-internal id, if ever worth keeping, belongs in a separate diagnostic
field — never overloaded onto callId.
Test: a tools/execute listener returns a wrong callId; assert the logged
tool/result.callId equals call.id AND deriveMessages() yields a tool-result
block whose toolCallId equals call.id (not the wrong returned id). Verified the
test fails on the pre-fix result.callId behavior.
After turn/start was appended, nothing guaranteed a matching turn/end: a throw
from a boundary emit (agent/turn-start, agent/step-start, the normal-path
agent/turn-end) escaped runTurn, and the outer runLoop backstop logged an
error but never appended turn/end — leaving an unbalanced turn that replay,
telemetry, and the invariants plugin all assume is impossible.
runTurn is restructured around idempotent finalizers that satisfy the four
traps a naive finally would hit:
- closeStep()/closeTurn(emit) are guarded (stepOpen/turnEnded) so they run at
most once; the agent/step-end and agent/error emits are contained so a
throwing listener can't strand the turn open.
- failTurn() records the single error event + reason and emits agent/error
exactly once (errorReported guard) — no double-logging when the outer catch
also runs (e.g. a step error followed by a throwing turn-end listener).
- the catch closes an open step BEFORE turn/end (invariants reject turn/end
while a step is open), and rethrows ONLY pre-turn throws (turnStarted false),
where no turn/end is owed, so the backstop still nets them.
- disposal precedence: reason stays disposed only when disposed AND no error
was reported; otherwise the error reason wins.
Tests (with the invariants plugin loaded as a balance oracle): throwing
turn-start (one error, one turn/end, no step), throwing step-start (step/end
before turn/end), throwing agent/error on a step-error path (balanced, loop
survives), disposal mid-turn (reason disposed, no error event), a pre-turn
turn/start-append throw (rethrown to the backstop, no turn/end owed), and a
step error + throwing turn-end listener (error logged exactly once). Verified
all six fail against a simulated finalizer bypass. dsh-invariants added as an
agent-loop devDependency (test-only oracle; no package cycle).
Every loop boundary appends the session event before emitting the Cordis
event (ADR 0003's append-before-emit rule) — except step/start, which was
inverted. A listener on agent/step-start that inspected session.events could
not see the step it was just told had started.
- Swap the two lines so session.append('step/start') precedes the emit.
- Fix the two stale pseudo-code copies (the runLoop JSDoc STEP-loop block and
docs/architecture.md) so neither shows step-start emitted before the append.
- Regression test: a step-start listener observes the matching step/start
event already at the tail of session.events. Verified the test fails on the
pre-fix (emit-first) order.
Introduce HarnessError in dsh-llm (the leaf package): a stable machine-routable
code distinct from the message, cause chaining, name from the subclass, plus
isHarnessError. LlmError, ToolArgsError, and InvariantError now extend it.
Tool failures carry the structure end-to-end: ToolExecutionResult gains
error: { name, code } (populated from a thrown HarnessError), and the loop
forwards it onto the tool/result session event (which gained the same optional
field) for retry/sandbox plugins and replay. The loop's toError wraps non-Error
throws in a HarnessError(code: UNKNOWN, cause) instead of a bare Error.
Landed last and in isolation so it's a pure upgrade over the plain Error+code
the earlier PRs used — independently revertible. Graduates RFC 005 pt 2 ->
ADR 0015; RFC 005 now fully implemented.
The first real LlmAdapter implementations, shipped as a deliberate pair:
same models and wire protocol, completely different internals, so the
StreamChunk protocol is verified across independent implementations.
- dsh-llm-deepseek: hand-rolled fetch + SSE parser + chunk-translation
state machine against the official chat-completions format (thinking
mode via top-level thinking/reasoning_effort; the empty-string
reasoning_content first chunk; usage attached to the finish chunk or
trailing; reasoning_content passback on tool-call turns; disjoint
cache-token accounting).
- dsh-llm-pi-ai: the same endpoint through @earendil-works/pi-ai,
mapping its event vocabulary (parsed tool arguments, in-stream error
events, folded reasoning tokens) onto the same chunks.
The agent loop now honors the in-band error path: an adapter that ends
its stream with finish {kind:error|aborted} (the only option for
adapters that can't throw mid-stream, like pi-ai) is translated into a
step error, so the turn ends error/aborted with a logged error event
instead of a normal completed assistant message. This makes the
StreamChunk error contract real for both adapters; docs/architecture.md
and the StreamChunk doc are updated accordingly.
New yarn test:e2e (vitest.e2e.config.ts, *.e2e.ts) runs key-gated
real-API matrices for both adapters across V4 Flash/Pro and all
thinking/effort levels; it self-skips without DEEPSEEK_API_KEY. Unit
suites run against local node:http mock SSE servers at 100% per-file
coverage.
Nominal string types via a unique-symbol brand (zero runtime cost):
an AgentId can no longer be passed where a CallId is expected. Each
core package brands the IDs it owns — CallId in dsh-llm (tool-call
correlation across blocks, chunks, session events, and execution
results), SessionId in dsh-session, AgentId in dsh-agent. Construction
goes through same-named factory functions; public string-in APIs
(sessions.create, agentLoop.create) keep accepting plain strings and
brand internally. Policy note in the brand module: brand IDs that
cross package boundaries, not every string.
Flat config with two layers. Correctness (type-checked): the headline
rules for this codebase are no-floating-promises / no-misused-promises
(a lost promise in the agent loop is our primary bug class),
switch-exhaustiveness-check (we switch over merge-extensible unions
everywhere), no-unnecessary-condition, require-await, and
no-explicit-any. Style (@stylistic): 2-space, no semicolons, single
quotes, trailing commas, max-len 140 — the existing house style, now
enforced instead of drifting between agents. vendor/ is excluded
(vendored source keeps upstream style); tests relax the rules that
fight test ergonomics (non-null assertions after expects, async mock
signatures, non-Error throws).
Code adjusted to pass: registry disposers wrap ctx.effect's
promise-returning disposer behind a sync () => void (our public API),
BlockAssembler gains an invariant-checking mustGet instead of non-null
assertions, lastTurnNumber uses findLast, waterfall tails return
Promise.resolve instead of async-without-await arrows, and the two
deliberate suppressions (non-exhaustive derivation switch, unbound
execute pass-through) carry justification comments.
yarn lint / yarn lint:fix added.
tsconfig.base.json adds noUncheckedIndexedAccess,
exactOptionalPropertyTypes, noImplicitOverride,
noFallthroughCasesInSwitch, noUnusedLocals, and noUnusedParameters on
top of strict. Vendored packages opt out of the new flags locally
(their tsconfigs are ours to regenerate; their source is not), keeping
upstream-sync friendliness.
Our code fixed accordingly: index accesses acknowledge undefined
(assembler flush cursors, lastTurnNumber); optional properties are
omitted instead of set-to-undefined (GenerateResult.usage,
ToolDefinition.strict, GenerateOptions.system/tools, error payloads
via an errorData helper); Session.onAppend is explicitly
`(…) => void | undefined`; tests and examples updated for unused
parameters and indexed access.
Docs: per-folder README.md for packages/ (family overview + one per
package: service, events, API, extension points, TODOs), examples/,
and examples/echo-agent/; folder-level AGENTS.md (+ CLAUDE.md
symlinks) for packages/ and vendor/; module-level doc comments in
every packages/*/src file; richer JSDoc on all exported API
(event side effects, disposal contracts, error behavior). Root
AGENTS.md gains a "Type Safety and Documentation" policy section:
the codebase aims to be very type-safe and well documented; type
gymnastics are acceptable in core packages when they improve
plugin-author DX; verbose docs are fine as long as they stay strictly
in sync with the code.
Type safety: removed the upstream-inherited "noImplicitAny": false
from tsconfig.base.json — packages/* now compile under full strict
mode; vendor/loader and vendor/include set it locally (vendor/cordis
already did). Eliminated every `: any` / `as any` from packages and
examples (catch clauses use unknown + a CodedError narrowing type;
event data access uses discriminated-union narrowing).
Typed tool schemas: new @deepseek-ai/dsh-tools schema DSL —
SchemaSpec with per-property `required: true` booleans, type-level
InferArgs<S>, a runtime SchemaSpec → JSON Schema converter, and
defineTool() so first-party tools get typed execute(args) with zero
casts (raw JSON Schema still accepted for MCP interop; chosen over
schemastery because it targets JSON Schema generation directly).
echo-tool and all test tools migrated; +7 tests.
High (loop pipeline): agent/step-result now runs before the
assistant/message append so the session log records what tool dispatch
actually uses; abort is honored between tool calls, not just
mid-stream; steering drains at step start, pending steering overrides
a negative turn-continuation decision (/goal pattern), and leftover
steering is re-enqueued as queued messages so it is never stranded;
exceptions from turn-continuation listeners and session/flush are
contained to the turn (error event + agent/error) instead of killing
the driver loop.
Medium: disposal emits agent/status('disposed') and mid-turn disposal
records reason 'disposed'; duplicate LLM adapter registration throws
(all-or-nothing); SessionEvent is a real discriminated union (casts
removed); model-less agents fail with a clear actionable error unless
agent/request supplies a model.
Low: agent/queued and agent/steering carry the resolved MessageSource;
streamBlocks() yields strictly in stream order and flushes delta-only
blocks (matches generate()); BlockAssembler freezes blocks on
block-end and ignores stragglers from malformed streams; turn
numbering is a counter seeded from the log (fork-safe); LoopAgent's
stop disposer is infallible (a throwing status listener cannot skip
registry cleanup); AgentLoop.create uses a generator effect so stop
and unregister are independent disposables; SessionStore wires
onAppend inside its effect.
21 regression tests added (review-fixes.spec.ts), organized by
finding. Docs updated: loop pseudocode (status emissions, ordering,
error containment, steering guarantees) and waterfall composition
caveat in docs/architecture.md; AGENTS.md notes that excessive tests
are welcome.
Tianyi Cui