Second review round (ds-review-bot v4p/v5/v6): the exit cleanup claim that
collectors unlink their spill files on dispose was wrong — completed spill
files are retained (seal() only closes, discardSpill() is the only unlink),
so the exit removal only ever applies to directories that never spilled.
Reword the JSDoc, README, and Agent Note accordingly, drop the redundant
readdirSync precheck (rmdirSync ENOTEMPTY is caught anyway), and recount the
v8-ignore window. The spawn.spec default-spill test now records the default
per-process directory it created and removes it in afterAll, so a completed
spill file is never left behind by the unit suite.
The Agent Note now links the 2026-07-17 local-spill-startup-cleanup note it
builds on, and the privateSpillDir JSDoc documents the empty-dir exit
removal alongside the directory's creation contract.
Review (ds-review-bot) found the exit-time deletion of per-process spill
roots conflicts with the documented retention decision: spill artifacts are
model-visible locators that resumed or forked sessions may still reference
(2026-07-17-local-spill-startup-cleanup), and an exception thrown from an
'exit' listener can change the process exit code.
- dsh-spill-local: revert the exit handler entirely; its default root is
already owned by the package's 30-day startup sweep.
- dsh-subprocess-local/spawn: remove the per-process spill dir at a
JavaScript-observable exit only while it is EMPTY (collectors unlink
their spill files on dispose), best-effort with a named swallow so a
Windows-held handle never changes the exit code; dirs holding spill
files keep them for external cleanup.
- Document the empty-dir exit removal in the package README and the Agent
Note, and re-record both bilingual pairs.
The exit listener runs after the coverage dump, so its body can never be
measured by the unit coverage lane; mark it v8-ignore with the reason, as
with other process-exit-only code paths.
Spec files that create /tmp/dsh-* directories via mkdtemp now track and
delete them in afterEach/afterAll; module-scope fixture dirs (executor
spill dirs) are removed in afterAll. The file list came from the
observed-residue inventory on the self-hosted CI host: only specs whose
dirs actually accumulated were leak sources (issue #3134), superseding
the kept-but-unmerged CI sweep branch per the #3233 review decision.
Product per-process spill roots (dsh-subprocess-local spawn,
dsh-spill-local store) register a process-exit handler that removes the
memoized dir, so processes that used the spawn/spill path clean up on
normal exit. A SIGKILLed process cannot run in-process teardown; the
machine-side timer remains the backstop for that path.
Agent Note: .agents/notes/implemented/process/2026-08-28-test-temp-dir-self-cleanup.md
Review found the shared observation defeated the very fence it fed:
it carries the original PID-to-start-time pairing forward, so a
recycled PID still matches it and takes a signal meant for the process
that exited. Capturing it outside the per-member try also let one
failed read abort a whole teardown round, breaking the synchronous
host-exit contract, and an empty round paid a read for no members.
signalProcess now reads ProcessInspector.isAlive immediately before
delivering, from the narrowest per-identity source each platform
offers; signalMembers and waitForMembers return before capturing when
a round has no members. snapshot() keeps serving the readiness poll,
whose per-poll table read stays at one.
Windows enumerates Toolhelp32 lazily on the first tree question, so a
snapshot asked only for liveness — the 25 ms teardown poll — performs
no table walk at all.
MacProcessInspector answered the descendant tree and every member's
liveness with its own `/bin/ps` fork, so one readiness poll cost N+1
full table reads for N tracked descendants. With execFileSync on that
path and a 50 ms poll interval, any command spawning two or more
children saturated the host event loop until it exited.
ProcessInspector.snapshot() now returns one ProcessSnapshot that
answers tree, session, and alive from a single observation, and
signalProcess takes the caller's observation so its PID-reuse fence
does not re-read the table per member.
Routes koffi allocations through a branded NativePtr helper (koffi's TS
types are any), binds the creationTime callback instead of passing the
unbound method, and braces the no-op signal assertions.
createProcessInspector now returns a WindowsProcessInspector on win32
instead of throwing: Toolhelp32 tree enumeration with GetProcessTimes
start identities, the shell pid as a pseudo foreground group, taskkill
tree signalling, and inspector-verified Windows teardown (node-pty
signal kills throw on Windows, and externally taskkilled shells may
never fire its exit notification, so the handle settles \done\ from the
verified absence). subprocess-local and pty-local suites now run on
Windows with platform gates; the koffi-backed inspector joins the
windows-only coverage exclusions on Linux and is fully covered by the
windows-native lane.
Also flips vitest.config so subprocess-local and pty-local sources are
coverage-required on win32, and adapts the spawn/terminal suites to run
natively there (node-translated shell commands, injected POSIX group
paths, taskkill signal semantics).
Machine-produced by `pnpm run rescope-vendor --apply` plus the regeneration it
prints: `pnpm install` for the lockfile, `pnpm run gen-third-party-notices`,
`verify-translation-pairing --write` for the touched bilingual pairs,
`gen-doc-graphs`, and one typert snapshot whose ids embed character offsets.
`pnpm run rescope-vendor --check` verifies the result.
Renames nine vendored packages (cordis, cosmokit, schemastery and the six
@cordisjs plugins) and every reference that resolves them: manifest names and
dependency keys, module specifiers including declare-module merges, cordis.yml
plugin names, tsconfig paths, every Markdown fence, and `docs/` prose.
Directory names, upstream versions, and dependency ranges are unchanged, so
vendor/README.md still reads as an upstream snapshot; its manifest table gains
an upstream-name column so THIRD_PARTY_NOTICES keeps MIT attribution pointed
at each fork's origin.
The tutorial tier follows the rename end to end: its yaml fences named plugins
the Loader can no longer resolve, its `ts ignore-check` fences disagreed with
the compiled fences beside them, and its prose quoted both. The contracts that
told readers to keep upstream names — the root convention and the vendoring
cookbook's tree comment and manifest invariant — now say to rescope instead.
Two rules read `@deepseek-ai/` as "another workspace plugin": the client bundle
purity gate now names the vendored libraries a browser bundle inlines, and the
files where a bare `cordis` is an agent-preset id keep that product data.
A recycled shell pid could donate an unrelated process's children to the
terminal session's cleanup signalling: post-exit rescans queried
processTree/processSession by numeric pid alone. The handle now captures
the spawned shell's start identity at construction and adopts newly
scanned members only while the root pid still carries it; already-adopted
members keep their own identities, which every signal already rechecks.
Regressions cover a recycled root donating an imposter child and a shell
whose identity was never observable; the terminal fakes now model the
root row the real /proc and ps scans include.
Also from the review round: tool-pty's dependency list is re-sorted, and
the LSP renderer documents the deliberate drive-letter reading of
ambiguous file: URIs (display-only blast radius).
The linear replay carried each commit's own lineage, so this checkpoint
restores the master-owned surfaces the conflicted regions clobbered and
migrates branch-owned code to master's post-rebase APIs:
- rebuild subprocess-local spawn.ts on master's tree-exit-observer
machinery, keeping the branch's win32 childEnv key semantics and the
Linux zombie-quiescence probe; the zombie test reaps its survivor
directly since a confirmed-absent verdict is a permanent
no-more-signals boundary
- migrate pty-local test stubs to the Inbox-model Agent interface,
Session.create, runnerFailureRules, and the new turn/start payload
- implement the seam's resolveExecutable/spawnTerminal abstracts in the
new pwsh-local and tool-fs-search test fakes
- restore code-runtime, atomic-write, pwsh-local, and app-boot to
master's exact content (the net-zero code-runtime churn is pruned
from this history) and drop rename-detection graft debris
- re-apply the PR's architecture rows and execution-world paragraph,
re-record bilingual pairings, regenerate catalogs, and reconcile the
lockfile
- pty-local: a pre-write inspection rejection no longer releases a canceled
send while its foreground SIGINT is in flight; the interrupt path retains
the slot and its post-signal tail resumes polling. Regression pins the
failure-shaped cancellation and a close-during-write release.
- pty-local: SEND_ACTIVE names which provider operation is draining; README
states the never-settling-provider outcome (slot retained, close recovers).
- subprocess-local: resolveExecutable rejects relative paths containing
separators instead of expanding them per PATH entry with a misleading
not-found error; seam JSDoc pins the rule for every provider.
- subprocess-local: LocalTerminalHandle documents why the seam's
in-flight-join promise holds without operation tracking.
- lsp-local: the oversized-source diagnostic reports the observed byte lower
bound; README documents that processId: null trades away server-side
orphan cleanup after a hard-killed harness.
- pty/subprocess seams: cross-reference the twin five-member signal unions.
- pty-local: TODO markers for send-state consolidation and initialize-race
relocation.
Remove the one-consumer bounded-read primitive and shared terminal lifecycle controller, make terminal cleanup one awaited provider operation, and reuse one Code Runtime contract suite. Keep only reproduced cancellation and policy fixes; defer unproven replacement, prompt-attribution, and streaming-frame concerns to scoped markers.
Chinesezjc