Deep audit of dsh 0.1.0-rc.7: one critical event-loop stall, two agent-loop correctness defects, and a process-fatal transport gap

[Jiaaqiliu]

I ran a full-repo audit of deepseek-harness at 99f6f02 (0.1.0-rc.7), building from source on Node 24 and taking pnpm run build + pnpm run test green as the baseline (13507 passed / 816 files). Everything below is therefore a defect that the current suite does not cover.

I have reproduced and patch-validated the first item. The rest are reported with the mechanism traced in source; I have flagged my confidence honestly on each.

Reporting here because CONTRIBUTING.md states external PRs are not accepted and Issues are disabled. Happy to hand over patches in whatever form is useful.

---

1. terminal-bash scrollback is quadratic — every PTY chunk blocks the event loop (critical)

packages/terminal/terminal-bash/src/session.ts:26

utf8Tail() re-scans the entire retained buffer on every append:

function utf8Tail(text: string, maxBytes: number): { text: string; truncated: boolean } {
  if (Buffer.byteLength(text) <= maxBytes) return { text, truncated: false }
  const chars = Array.from(text)              // one array element per code point of the WHOLE buffer
  let bytes = 0
  let start = chars.length
  while (start > 0) {
    const next = Buffer.byteLength(chars[start - 1] as string)   // per-character Buffer call, ~maxBytes iterations
    ...
  }
  return { text: chars.slice(start).join(''), truncated: true }
}

BoundedTextBuffer.append() (:49) calls it unconditionally, and appendOutput() (:416) feeds two such buffers on every onTerminalData event: scrollback (scrollbackMaxBytes = 4 MiB) and the per-send viewport (maxReadBytes = 256 KiB, no line cap). Once each buffer sits at its cap, the cost is paid in full per chunk.

The maxLines pre-trim only helps newline-dense output. With long lines lines.length stays 1 and the full cost lands.

I benchmarked verbatim copies of utf8Tail/BoundedTextBuffer at shipped defaults, feeding 8 KiB chunks after the buffers pass their caps (Node 24, M-series):

output shape	per 8 KiB chunk	throughput ceiling
long lines (minified JS, base64, jq -c)	128 ms	0.061 MiB/s
newline-dense	6.4 ms	1.2 MiB/s

That is synchronous main-loop time. While it runs, nothing else in the harness progresses: no LLM streaming, no timers, no other tool call, no abort handling. pollIntervalMs, idleSilenceMs, and timeoutMs are all timer-driven and cannot fire.

Trigger: any terminal_send whose command emits a few MB without newlines — cat a minified bundle, base64 /dev/urandom | head -c 100000000, jq -c . big.json. A 5 MB file stalls the harness for over a minute.

Fix — keep the bound in bytes and cut on a UTF-8 lead byte instead of walking characters:

function utf8Tail(text: string, maxBytes: number): { text: string; truncated: boolean } {
  const bytes = Buffer.from(text, 'utf8')
  if (bytes.length <= maxBytes) return { text, truncated: false }
  let start = bytes.length - maxBytes
  while (start < bytes.length && ((bytes[start] as number) & 0xc0) === 0x80) start += 1
  return { text: bytes.toString('utf8', start), truncated: true }
}

Validation: differential-tested against the original across 32,800 cases (ASCII, CJK, 4-byte emoji, mixed 1/2/3/4-byte, lengths 0–200 × caps 0–40) — 0 divergences. packages/terminal/terminal-bash suite: 69/69 passing, run twice. Measured 27× faster on the long-line path (144.6 ms → 5.3 ms combined per chunk).

This is still O(n) per append because the whole string is re-encoded. If you want the real fix, BoundedTextBuffer should retain Buffer chunks with a running byte total and trim from the head, making append O(chunk). I kept the patch above minimal so it drops in without touching the class contract.

---

2. Sticky max-tokens ends the turn after tools run, so the model never sees the results (high)

packages/core/agent-loop/src/agent.ts:290, :295, :299

turnEnds serves two purposes at once: the durable turn/end reason, and the "nothing is owed, stop" flag.

const stepEnd = await this.step(decision.assembly)
if (turnEnds === null || turnEnds.kind !== 'max-tokens') turnEnds = stepEnd   // sticky
...
if (turnEnds && this.inbox.nextStep.length === 0) { await this.dispatch.serial('agent/turn-stopping', ...) }
if (turnEnds && this.inbox.nextStep.length === 0) break

step() returns null exactly when tools ran and did not conclude the turn — i.e. another model request is owed (:399). The sticky guard refuses to overwrite max-tokens, so that null is discarded and the two conditions read turnEnds as truthy and break.

Sequence:

1. Step 1 finishes max-tokens (:391 returns before any tool dispatch) → turnEnds = {kind:'max-tokens'}.
2. Something sits in next-step, so the turn continues. Shipped producers: a blocking Stop hook calling agent.steer(...) from agent/turn-stopping (packages/hooks/hooks-claude-code/src/index.ts:270, same in hooks-codex), or agent.inject() from user-approval/src/index.ts:230.
3. Step 2 emits tool calls. They execute, {concluded:false} → step() returns null.
4. Sticky guard keeps max-tokens; nextStep is empty → break.
5. turn/end {kind:'max-tokens'}. The tools ran with their side effects and no model request ever consumed the results. Headless one-shot terminates with no final answer.

loop.spec.ts:983 covers max-tokens → completed step and :1054 covers "does not dispatch tool calls from a max-tokens-truncated step". Neither covers max-tokens → tool-calling step.

Fix — separate "work owed" from "reason to record":

const stepEnd = await this.step(decision.assembly)
const owed = stepEnd === null
if (turnEnds === null || turnEnds.kind !== 'max-tokens') turnEnds = stepEnd
...
if (!owed && turnEnds && this.inbox.nextStep.length === 0) { /* turn-stopping */ }
if (!owed && turnEnds && this.inbox.nextStep.length === 0) break

---

3. Auto-compaction deletes the runtime-context snapshot from the very request it is compacting for (high)

packages/core/agent-loop/src/agent.ts:233, with packages/compaction/compaction-basic/src/index.ts:147 and packages/core/agent-loop/src/runtime-context.ts:50

preStep() computes the runtime-context candidate before the waterfall runs, then closes over it:

const context = this.runtimeContext.project(joinContextSections(sections), sections)
const decision = await this.dispatch.waterfall('agent/pre-step', {...},
  () => Promise.resolve({ kind: 'enter',
    messages: context === undefined ? claimed : [...claimed, context] }))   // stale capture

project() returns undefined when the rendered snapshot equals the retained one. BasicCompactionEngine registers an agent/pre-step listener that compacts before next(). A landed compaction appends a user/message with surfaceOp: {op:'replace'} shadowing the seqs, which drives RuntimeContextProjection to set retained = null. But context was bound to undefined already, so the default enter adds nothing. Listener order is irrelevant — the value is bound before the chain starts.

Result: with dsh-base (compaction-basic mounted auto: true), on the turn where compaction lands, the request carries no runtime-context message at all — the model is not told the sandbox mode or the approval policy. Restored only at step N+1, which in a headless one-shot run never happens.

loop.spec.ts:413 guards this class but performs the replacement between turns, so the in-waterfall ordering is untested.

Fix: re-project inside the default arm instead of capturing, so it re-reads retained after the waterfall.

---

4. SDK JSON-RPC transport never listens for output errors — stdout EPIPE kills the harness (critical)

packages/sdk/protocol/src/transport.ts:76-82, :260

start() attaches 'data'/'error'/'end' to this.input only. Nothing is ever attached to this.output, and write() is a bare this.output.write(...) with no callback. packages/sdk/server/src/index.ts:55 wires config.output ?? process.stdout straight in, while HarnessSdkJsonRpcServer subscribes four event streams that call notify() continuously.

A write failure therefore surfaces only as an 'error' on process.stdout with zero listeners → throw er; // Unhandled 'error' event. installFailLoud registers unhandledRejection only, never uncaughtException.

Trigger: the SDK client dies. runner.ts:51 gets stdin EOF and starts await ctx.fiber.dispose() (async: persistence flush, agent teardown, child shutdown). During that window the still-live subscriptions write to a broken pipe and the runtime dies mid-dispose — truncating the durable JSONL session log and orphaning spawned children.

Two signals this is a real gap: the ACP bridge writes through Writable.toWeb(process.stdout) (packages/acp/acp/src/index.ts:445), which does contain EPIPE; and the SDK server's own test has to add output.on('error', ...) (tests/plugin-apply.spec.ts:100) to keep the test process alive.

Fix: attach this.output.on('error', ...) in start() (removed in close()), routing through failPending() so pending requests reject instead of the process dying.

Related, same file (:148, :260): request() wraps the send in try/catch, but Writable.write() does not throw on a broken stream — it returns false and reports asynchronously. The catch arm is unreachable for real transport failures, so the pending entry is never settled. Combined with HarnessClient swallowing stdin errors (packages/sdk/client/src/client.ts:223) and requestTimeoutMs having no default, await harness.run(...) can hang forever with no diagnostic. Pass a write callback and reject the matching pending entry.

---

5. SqliteSessionPersistence turns a recoverable schema mismatch into a process exit (high)

packages/session/session-persistence-sqlite/src/index.ts:133

this.ready = this.openDb(config.path, journalMode)   // no rejection handler

Every backend hook does await this.ready, but nothing awaits it at construction: there is no [Service.init], and the only same-turn consumer iterates ctx.sessions.list(), empty on cold boot. openDb awaits mkdir before openDatabase can throw, so the rejection settles a full event-loop turn later with no handler attached.

The sibling backend does exactly the right thing and says why — packages/storage/storage-sqlite/src/index.ts:72:

this.ready = openDatabase(config.path, journalMode)
// Mark the rejection handled: every primitive re-awaits `ready`, so an
// open failure still surfaces to each caller; this guard only prevents an
// unhandled-rejection crash when the failure precedes the first use.
this.ready.catch(() => {})

The session backend omits that one line. Result: a SCHEMA_VERSION mismatch after an upgrade/downgrade becomes proc.exit(1) at an arbitrary point after boot, instead of each persistence call rejecting with the intended diagnostic.

Fix: add this.ready.catch(() => {}) after line 133.

---

Smaller items, same audit

• packages/subprocess/subprocess-local/src/spawn.ts:207 — readFrom decodes each byte range independently with no StringDecoder and returns a raw byte nextOffset, so a multi-byte character split across a poll boundary becomes three U+FFFD. Hits the background-job read path (bash-local/src/index.ts:290) deterministically for non-ASCII output. The foreground path is safe (single readFrom(0)).
• spawn.ts:397 / index.ts:153 — every settled subprocess starts a permanent 15 ms poll; on Linux, once a backgrounded descendant survives (cmd &), each tick does a full /proc readdir plus a readFileSync per numeric entry, forever. The ref'd timer also blocks natural process exit.
• spawn.ts:89 — spill files and the mkdtemp spill dir are never removed. With maxOutputBytes 64 KB / maxSpillBytes 64 MiB, a working session can leave GBs under $TMPDIR, invisible to the harness.
• spawn.ts:362 — collectStream attaches no 'error' listener to child.stdout/stderr, while the symmetric stdin case four lines below is handled with a comment. An unhandled 'error' on these takes the whole process down.
• packages/llm/llm-deepseek/src/sse.ts:39 — a cleanly-closed truncated stream throws STREAM_CLOSED, which is absent from DEFAULT_RETRYABLE_CODES; the same truncation delivered as a socket reset becomes TRANSPORT and is retried. The sibling adapter takes the opposite view explicitly (llm-pi-ai/src/stream.ts:52: "this is a transport truncation, not a model-level error"). Retrying is provably safe here — the loop re-sends a byte-identical body and commits no assistant/message for the failed attempt.
• packages/llm/llm-deepseek/src/translate.ts:152 — tool-call deltas are keyed on the raw wire call.index with no validation. Against an OpenAI-compatible endpoint that omits index, every parallel tool call collapses into one block with concatenated argument strings. baseURL is user-configurable, and AGENTS.md:115 lists "model/tool JSON" and "wire boundaries" as places validation is required.
• packages/mcp/mcp-client/src/tools.ts:152 — do { ... } while (cursor) trusts an external MCP server's nextCursor with no repeat detection and no page cap, inside a chain that dispose() awaits. A constant cursor wedges plugin disposal permanently.
• packages/client/connection/src/rpc-host.ts:108 — configured maxRequestBodyBytes is threaded into the /api route but never into HostConnectionService, so every generic connection.rpc.handle(...) channel silently keeps the 160 MiB default.

---

Full reproduction details, the differential-test harness, and patches for each are available if the team wants them. Nothing here needs a key to reproduce — the benchmark in item 1 is standalone, and items 2–5 are traceable in source.