Two independent multipliers were pushing bare `pnpm run lint` past node's
default heap:
1. parserOptions.project expanded to every package tsconfig plus the root
one — each matched config built its OWN ts.Program, and the dev paths
map pulls sibling package SOURCES (plus the vendored Cordis stack) into
every such program, so resident memory grew as sum-of-closures, not
repo size: ~4.6 GB peak for 425 repo files. projectService shares one
tsserver-style graph: ~2.0 GB peak, ~28 s → ~14 s wall.
2. `eslint .` traversed .claude/ harness-local state — stale worktree
checkouts there carry tens of thousands of additional .ts files
(whole-repo copies), roughly tripling the work again even under the
project service. Other checkouts are not this one's sources; ignore
them like node_modules. (#169 carries the identical ignore line inside
its chain; the hunks dedupe on its next rebase.)
Type-aware rules verified live under the service: a floating-promise
probe still trips no-floating-promises.
Review finding, valid — the previous SDK fix covered only the
non-default branch: in the default mode under Code Mode the wire filter
dropped exit_plan_mode but the registry-rendered tools:sdk section
still advertised its binding, offering default-mode agents a call that
can only error and breaking the byte-identical claim (a no-dsh-mode
deployment's registry never saw the tool, so its SDK never listed it).
The SDK re-render extracts to one helper both branches share: the
non-default branch passes the mode's visibility rule, the default
branch hides exactly the exit binding. The pinning test now compares
the default-mode SDK byte-for-byte against a bare deployment without
dsh-mode — the strongest form of the invariant the RFC states.
A review finding claimed plan mode under the registry's 'both'
presentation leaves non-allowlisted native schemas on the wire. The
claim misreads the filter predicate — 'keep visible plus run_code' IS
'drop everything else' — and the SDK re-render shares the same visible()
predicate, which is precisely the remedy the finding requests. This
test refutes it empirically on unchanged code (first run green) and
stays as the regression pin: plan + both = [exit_plan_mode, read,
run_code] on the wire, read documented and write absent in the SDK.
Review follow-up on the residual the previous commit accepted — and the
acceptance was wrong, because the fix is clean: in Code Mode the SDK
section IS the soft surface (the wire carries only run_code), section
text resolves in assemble's base, and renderToolsSdk is an exported
pure renderer. The outermost wrapper therefore re-renders tools:sdk
from the same visibility predicate the wire filter applies (allowlist,
exit-IFF-plan, minus run_code mirroring the registry's own exclusion):
a plan-mode program is documented exactly the callable bindings — read
and the exit, never the denied write. The default mode leaves the
section untouched (absence of policy), both pinned by tests.
The soft layer's promise — the model is never encouraged toward a tool
the gate denies — now holds in Code Mode too; the only remaining
prompt-honesty residual is a prepend-after-load assemble listener,
where the gate still covers execution.
Review follow-up: the blanket 10s default correctly targets worker-thread
cold-start races (starting, first-script-line, async child-registration
messages — genuinely CPU-bound under CI contention), but four waits assert
something different — that the HOST reacted PROMPTLY to an event that
already happened (a settled result, an observed worker death). Those had no
cold-start left to wait on, so the generous default just widened the window
a real regression could hide in.
Verified by injecting a 6s delay into the settle-reap's abort call: the
un-overridden helper's test still passed in ~6s. The same mutation now fails
in ~1s with the explicit 1000ms override restored on all four sites (the
abort-on-settle test's own assertion, the two worker-death cancel/dispose
reap checks, and the dispose-ack-race check). The other 12 waits keep the
10s default — they run BEFORE a result is awaited, waiting on the worker to
actually start rather than on a host reaction. Doc comment corrected to
describe the split instead of claiming every wait is a cold-start race.
Review finding, valid: under the registry's Code Mode the assembly's
only wire tool is run_code, which the plan allowlist filtered out —
leaving the model with NO tools at all, the exit review included. The
composition exists today (the acp-agent example ships a code-mode
overlay), so plan mode bricked it outright.
run_code is a transport, not a capability: every bridged sub-call is
serialized back through ToolRegistry.execute() carrying the same agent,
so tools/pre-execute judges each capability individually — exactly like
native calls. Both layers now exempt it by name: the filter keeps it
visible (tests pin plan-mode Code Mode assembly = ['run_code']) and the
gate passes the wrapper while the same run's write sub-call still
denies with the plan-mode reason.
Documented residual, same class as the prepend-after-load one: the SDK
section renders from the registry's store, so a plan-mode program may
be offered bindings whose dispatch the gate then denies — nothing runs
that a native call could not.
Review finding at the seam of two surfaces this branch added to the
same stdin: with an ask_user_question (or plan-review) prompt active,
the line handler dispatched every line as the answer first, so
'/mode plan' typed mid-question was recorded as free-text feedback —
model-visible in the tool result — and the mode never changed. Command
handling now runs before answer dispatch: the command executes, the
question stays pending and still owns the next non-command line. A
literal '/mode…' free-text answer is the trade-off deliberately spent —
a swallowed command that becomes review feedback costs far more than
that contrived answer shape.
Review finding with a real in-repo instance: the structured runtime's
per-spawn final-assembly wrapper (prepend, post-next) re-injects
structured_output OUTSIDE the mode filter, so a structured child in
plan mode would see a tool the gate then denies — the soft policy and
the hard gate telling different stories. The suggested fix (make the
mode filter outermost) cannot beat that instance: prepend unshifts, so
the per-spawn listener always registers later and wraps outer.
Two-part resolution instead. Semantically, structured_output enters the
shipped plan allowlist — it is a child's pure result channel, the same
ask/report class as ask_user_question and exit_plan_mode, so the
filter, the re-injection, and the gate now agree wherever a structured
child runs in plan mode. Mechanically, the filter registers with
prepend anyway: it now wraps outside every append-registered listener
regardless of load order (regression test pins a pre-registered
post-next mutator being filtered), narrowing the documented cosmetic
residual to prepend-after-load listeners only, where the gate still
covers execution. Severity note: no execution breach existed — the gate
held throughout; this closes the prompt-honesty gap.
The spec's 16 vi.waitFor sites used the 1s default timeout to wait for
worker-thread startup and child registration — CPU-bound work that blows
past 1s on a contended runner. The CI coverage lane (4 vitest workers plus
suites that spawn real subprocesses) hit this 3 times across 4 recent PR
runs, each a different subset of the cancellation/worker-death tests, each
green on rerun.
Every wait now goes through a shared helper with a 10s bound, and the file
sets a 30s test timeout to make room for it. The one deliberately tight
wait keeps its 800ms bound through the helper's override — it proves the
host (not the wedged worker's later loop turn) delivered the cancel, so a
generous bound would erase what it tests. No behavior under test changed.
spec.onError is a caller-supplied callback boundary, but the flattening
catch invoked it unguarded: a throwing sink rejected the whole async result,
breaking the seam's "result never rejects on a child-level failure"
contract (and docs/defensive-patterns.md's contain-callback-exceptions
rule). The sink's own throw is now swallowed — named as such — while the
original child failure still settles as stopReason 'error'. Regression test
drives a spawn failure through a throwing sink and asserts result resolves.
Same defect as the codex backend's, fixed there on PR #240; this is the
symmetric fix for the already-merged ACP backend.
Review hardening (the finding's ordering premise did not hold — see the
PR thread — but its failure-path kernel did): onBoundary cleared the
pending intent BEFORE appending the mode/set, so a backend rejecting
that one write lost the switch forever while the picker kept showing it
optimistically. The intent is now cleared only after the append lands;
a failed flush stays parked and the next healthy boundary converges the
log with the picker. The containment test extends to pin the re-park
and the retry.
The bridge's re-notify keeps deriving from the logged event's value —
now documented in place: the service holds ONE coalesced pending slot
(every flush reads the latest selection, so a stale flush cannot
exist), and for any other writer the logged value is the truth the
picker should track, in log order.
The three wedged-child scenarios busy-spin their worker for 1.5s while the
host waits for the start RPC; under a loaded 2-core CI runner (this branch
adds several parallel suites) the RPC lands after vi.waitFor default 1s,
failing the coverage lane three runs in a row at the same three sites. An
explicit 10s waitFor timeout (well inside each test own 15s budget) makes
the assertions load-tolerant without weakening them. Belongs upstream with
dsh-workflow-workerthread; carried here because it gates this PR.
Tianyi Cui
Hypatia May
Yichen Jiang
kingwl
pku-xht
Ziya