Responding to ds-review-bot round 2 on #662:
- LANG_ALIASES is a Map: an assistant-authored fence label like
constructor or __proto__ now misses (plain render) instead of resolving
an inherited object property and crashing shiki mid-conversation. Test
sweeps the inherited-key labels.
- The singleton is pre-warmed in a deferred task at plugin boot (the
~120-175ms engine+grammar construction long task moves off the first
finalized fence's render); the lazy path remains the correctness
fallback, and unref keeps non-browser imports from pinning the loop.
Agent Note updated (both languages).
Responding to ds-review-bot round 2 on #661:
- logWork is bounded: past maxParallelSubCalls pending shaped-append tasks
the ordered commit lane holds (Promise.race drains one), so a slow spill
backend backpressures the run instead of accumulating unbounded pending
I/O and retained results. Tasks self-remove on settlement; run
settlement still drains every task inside the open turn. New spill test
drives three oversized reads against a hung backend at cap 1 and proves
the third dispatch cannot start until a save drains.
- shapeDispatchLog's catch uses errorMessage() (total), so a thrown value
with a throwing toString cannot escape the containment and lose the
settle event.
- CodeDispatchLog.content documented as the RENDERED result projection
(native tool/result vocabulary), not what the program received — the
program gets the structured value; doc pair + type-equiv re-synced.
- New RECORDED tui-agent snapshot scenario code-mode-dispatch-spill: the
real Loader-visible composition (worker runtime + spill-local + policy)
drives an oversized bash sub-call end-to-end; replay proves the durable
dispatch copy is bounded to preview + locator while the program value
stays whole (the outer result carries just the line count).
Agent Note updated (both languages).
Three review rounds each found a new silent-loss path in the same
wrapper state (dispose racing an in-flight flush, overlapping hints
displacing the retained promise, the provider's fixed 30s flush timeout
rejecting while the processor still drains). Every path exists only
because forwarding the seam's turn-boundary hint to forceFlush() made
this backend the process's second flusher against undocumented SDK
internals from the upstream experimental tree.
The backend now implements no flush(): the batch processor is the only
flusher, its scheduledDelayMillis (already deployment-tunable through
the processor passthrough) governs export cadence, and shutdown()'s
drain is complete by construction. The two race-pin tests collapse into
one dispose-during-in-flight-batch drain pin; the seam's optional
flush() contract now tells implementers they own the concurrent-flush/
shutdown interaction. Removal rationale and the reinstatement trigger
(a stated turn-boundary latency requirement scheduledDelayMillis cannot
meet — and then via the processor's own forceFlush(), never the
provider's timeout-wrapped one) are recorded in the revival Agent Note,
both languages.
Resolutions: regenerate docs/capability-seams.md (the only conflict);
master widened the bilingual pairing scope to package READMEs, so this
merge adds the Chinese counterparts and i18n records for the three
telemetry READMEs and patches packages/README.zh.md with the telemetry
group row.
(A1) terminate()/dispose()/service teardown keyed on direct-child settlement
could leak a TERM-trapping descendant that outlived the leader (Codex
reproduced it with a disowned trap-SIGTERM helper). kill()/terminate() now
gate on tree liveness instead of outcome settlement; the SIGKILL escalation
timer survives settle (unref'd, re-probing the tree); dispose's tier
quiescence is whole-tree exit via a bounded waitForExit; the service's live
set releases handles only when their tree is gone, and its teardown awaits
tree exit. Three new suites pin the survivor scenarios end to end
(terminate, dispose, service teardown).
(A2) the escalation branch is now real tested behavior — its ignore is gone;
the one remaining signalTree guard ignore states why it is unreachable
through the handle verbs.
(A3) docs contradictions fixed: the impl README's stale POSIX-only bullet
now states the contained best-effort Windows tree story; the lsp-local
README no longer claims taskkill failures stay visible (containment + the
tree-liveness wait is the actual contract); the architecture tables (en+zh)
list all three consumer families.
(B1) OutputCollector keeps a byte-exact tail across uneven chunk boundaries
(trim the head chunk instead of dropping it whole) — the LSP diagnostic-tail
contract; pinned by a cross-chunk test.
(B2) the subagent-acp coverage ignore is narrowed to exactly the
never-settling success arm.
A repo-wide Not Invented Here audit (ten parallel domain surveys covering
every package group, scripts/, native/, vendor/ edges, python/, test
infrastructure, and CI) asked of each hand-rolled surface whether a
maintained external package or Node builtin deletes it with a net win.
Policy: new implemented process note records that a dependency which
genuinely deletes owned code is a preferred simplification (bar: net
deletion, health, boundary fit, settled seams stay); root AGENTS.md
carries the one-line rule and dsh-find-simplifications now surveys for
hand-rolled-where-a-dependency-exists candidates.
Findings, all bilingual from birth:
- proposed/simplification: eventsource-parser for llm-deepseek SSE,
node:timers/promises for three hand-rolled sleeps, turndown (or
minimal 'entities') for tool-web HTML->markdown, gate-script
consolidation onto mdast/parseArgs/globSync
- proposed/testing: execa + parseArgs + loadEnvFile + vi.waitFor for
hand-rolled test subprocess plumbing
- proposed/process: pnpm/action-setup for symmetric CI caching
- proposed/feature: evaluate landstrip before building a Windows
sandbox launcher
- rejected/simplification: ~30 swap verdicts recorded (vscode-jsonrpc,
p-retry, Ajv, write-file-atomic, msw, hono, better-sqlite3, wireit,
landstrip-for-linux, YAML consolidation, ...) so the survey is not
re-litigated from scratch
Also drops the stale prompt/ entry from the AGENTS.md layout map
(workspace instructions live in packages/context/workspace-context).
Adds the zh counterpart, aligns the seam note's zh side with its updated
interface bullet and follow-up sentence, records both pairs, and ratchets
the translation-pairing manifest.
Conflict resolution: drainDispatches composes both sides — the reworked
single-lane driver's drive() (ordered stages, mid-flight commit drain)
followed by this branch's logWork drain, so every shaped settle append
still lands inside the open run_code turn.
Conflict resolution: session.ts keeps this branch's paired-start callTime
(the start event exists here; a settle-only window stays null — the PR2
fix's semantics compose). The README's codeDispatches section is restored
to the running→settled lifecycle this branch implements, keeping PR2's
null-callTime nuance for starts outside the window.
Review direction (tianyicui, PR #660): in a stacked PR, change all other
process-running places to use the new service.
- lsp-local: LspConnection spawns through ctx.subprocess (piped protocol
streams + a no-spill collected stderr tail); its private process-tree
helpers (POSIX group signalling, Windows taskkill, liveness polling) are
deleted in favor of the seam's handle verbs, and its buildChildEnv now
rides scrubbedParentEnv (LSP children also stop inheriting stale DSH_*).
The plugin injects 'subprocess'; compositions/tests mount
dsh-subprocess-local.
- subagent-acp: the ACP child spawns through the seam (piped ndjson streams,
inherited stderr); spawn failure surfaces through done-rejection into the
same startup race; disposal is handle.dispose with the plugin's configured
graces. dsh-subagent-subprocess is DELETED — its dispose ladder and scrub
are the seam's, and the isolated-config-dir helper had no consumer.
- mcp-client, pty-local, sdk-helper: adopt scrubbedParentEnv as the one
scrub definition (their spawns stay put by ownership: the MCP SDK and
node-pty own those calls; the SDK wizard runs outside any composition).
- Coverage: per-file 100% over every touched src file, with each v8 ignore
carrying a platform or contract reason; new suites cover stdio
dispositions, the dispose ladder tiers, injected-win32 tree semantics,
waitForExit, settled-kill/terminate no-ops, and spawn-failure disposal.
- Docs: consumer-migration Agent Note (en; zh follows in this PR), seam note
updated in place, subprocess.md rewritten for the reshaped vocabulary
(type-equiv re-registered), READMEs and SERVICE_ROLES updated, taskkill
added to knip ignoreBinaries.
Responding to ds-review-bot round 2 on #658 (three critical findings, one
warning — all rooted in the pump/commit split racing ordered stages):
- ONE driver lane now owns every ordered stage: the start append, prepare
(pre-execute/guards), and the head-of-line commit (post-execute, context
deferral, settle append). start() is awaited before the next entry can
start, so concurrent submissions can no longer run pre-execute pipelines
concurrently; only the around-dispatch/body stage overlaps, matching the
native loop's fillPool sequencing.
- An exclusive call's barrier now holds through its COMMIT: later starts
wait for the exclusive pipeline (post-execute included) to finish, the
native exclusive-group semantics.
- drainDispatches() awaits the driver run itself, so a commit already
mid-flight when the program returns is drained before run_code closes
the turn — the settle event and deferred contexts land inside it.
- maxParallelSubCalls is resolved and validated at construction (positive
integer), so direct construction can no longer wedge the pool with 0.
New tests: overlapping-submission ordered-prepare, barrier-through-commit,
drain-mid-commit, cap rejection. 96 keyless snapshots replay unchanged;
Agent Note updated (both languages).
Responding to ds-review-bot round 2 on #653: the tool/code-dispatch event
is appended at settlement, so using its time as callTime fabricated a
zero-duration call for duration-aware consumers — it is now null (start
unknown) per the ToolResultNode contract, pinned in the session spec. The
README's codeDispatches section described the PR3 running→settled
lifecycle a stack ahead of this tree; it now documents the settled-only
index this PR ships (the running shape lands with the start event in
#658, which already merges cleanly over this).
Responding to ds-review-bot round 2 on #648: the env seam's accepted
values, native default, process-wide scope, loud-failure behavior, and
temporary status now live in apps/cli/README.md next to the Web/headless
surface it configures, not only in the cordis.yml comment.
CI coverage flagged MarkdownText's pre route: the array-element probe
(raw[0]) and the mixed-content fallbacks were unreachable — the markdown
pipeline hands pre one code element whose children are one string (or
none, for an empty fence). Simplify to the string check, annotate the
isValidElement guard as representation-change armor, and pin both live
arms: the empty fence keeps the stock <pre>, a language-less fence renders
the plain CodeBlock arm.
The websettings merge (#644) localized the sidebar foot to 设置; the
lifecycle-chrome hero golden pinned the old English label. Keyless
DSH_SNAPSHOT=refresh rewrite; full lane green twice after.
CI's full-tree coverage flagged three untaken paths this PR introduced:
- shapeDispatchLog's catch (a throwing tools/code-dispatch-log listener
must be contained — the settle event logs the unshaped content);
- the spill listener's flatten-decline arm (non-text sub-result content
passes through unchanged);
- the generated scope-key extractor row for tools/code-dispatch-log
(registered in the scope invariant matrix like the other tools events).
Responding to ds-review-bot on #664:
- SubSpanLane gains a 'timing' discriminant (measured | running | unknown).
A settle-only replay entry (callTime null, start outside the window) was
previously indistinguishable from a measured 0 ms span; it now renders
hollow with a 'duration unknown' hover title, and durationMs stays null
for anything unmeasured. Pairs with the client-runtime fix that stopped
fabricating callTime = settle time (826c3696a on the live-parallel PR).
- The built-client Code Mode fixture snapshot now switches to the
Trajectory and Waterfall tabs and pins the assembled rendering: three
Sub cells with real +0.8s durations and three measured lanes with their
hover titles — product-visible coverage through the real bundle graph,
not just package-level jsdom fixtures.
Agent Note (both languages) updated for the timing contract; pairing
re-recorded.
Both ds-review-bot findings were real:
- markLlmAdapterFailure's carried-facts cross-check read error.code
directly; a foreign Error with a valid own failure payload but a
throwing code accessor would replace the original adapter error with
the accessor exception, breaking the error-identity guarantee. The
read now goes through foreignErrorCode(), which contains the trap and
falls back to the normalized snapshot (test: hostile code accessor
beside a valid failure payload -> original identity kept, UNKNOWN
facts).
- live-interactions' afterEach caught scaffold.close() into undefined,
silently disabling ReplayHandle.assertConsumed() — the fixture-drift
tripwire — and hiding cleanup defects. Teardown now runs every step,
collects failures, and rethrows (AggregateError when several).
Codex re-review (A): AGENTS.md layout tree, the architecture capability
tables (en+zh), the seam Agent Note's group-name sentence (en+zh), and the
SERVICE_ROLES entry still carried process/-era labels; regenerated the doc
graphs and config catalog from the corrected inputs.
Review direction (tianyicui, PR #660): make the interface closer to Node's
API so the other process-running places can adopt it. The spec gains
per-stream stdio dispositions — 'pipe' (raw Readable/Writable for protocol
streams), 'inherit' (diagnostics to the parent), and collect mode ({maxBytes,
spill?} — the old bounded tail-keep shape, now with spill optional for
diagnostic tails). SubprocessOutcome carries exit facts only; collected
output stays readable through handle.collected after settlement (spill fds
are sealed at the settle boundary). The handle grows Node-style kill(signal)
(single signal, tree-scoped, no-op after settlement), terminate() (the
SIGTERM→grace→SIGKILL escalation, also driven by the spec signal),
waitForExit() (tree liveness, not just the direct child), and dispose()
(the cooperative stdin-EOF→SIGTERM→SIGKILL ladder from subagent-subprocess,
graces caller-supplied). Tree semantics are platform-correct: POSIX detached
groups with direct-child fallback; Windows taskkill /T with an injectable
runner. scrubbedParentEnv/SENSITIVE_ENV_PATTERN move to the seam as the one
shared scrub definition.
bash-local maps its config onto collect modes and batch stdin and reads
results through the collected readers; its kill() maps to terminate() so
task_kill keeps escalation semantics.
Responding to ds-review-bot on #664 (root cause lives here): when a history
window carries a tool/code-dispatch settle without its paired start, the
runtime fabricated callTime = settle time, so downstream duration views
presented a measured 0 ms. Match the native tool-result contract instead —
callTime: null = unknown — and pin it; the trajectory cell already renders
null as the em dash, and the waterfall gains explicit unknown handling in
its own PR.
