Просмотр исходного кода

test: narrow process-exit spill cleanup to empty dirs; drop spill-store deletion

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.
Chinesezjc 1 месяц назад
Родитель
Сommit
29e6669e3c

+ 2 - 2
.agents/notes/implemented/process/2026-08-28-test-temp-dir-self-cleanup.i18n.yaml

@@ -2,5 +2,5 @@
 # side as of the last confirmed-consistent state. Both languages carry equal authority;
 # after editing either side, bring the other along and re-record with:
 #   pnpm run verify-translation-pairing --write .agents/notes/implemented/process/2026-08-28-test-temp-dir-self-cleanup.md
-2026-08-28-test-temp-dir-self-cleanup.md: e8e0c5e2db716ff9a63ef55ae1af07a66eaf28a0
-2026-08-28-test-temp-dir-self-cleanup.zh.md: cdc0dfa2b6a00fc11703743c51326238bf644fb1
+2026-08-28-test-temp-dir-self-cleanup.md: a9190878628d1c7bc7be87ff6c90297256a5a32f
+2026-08-28-test-temp-dir-self-cleanup.zh.md: 1b7b80d3e6eeaa66b2ea8e49722bc7e7d88433da

+ 3 - 3
.agents/notes/implemented/process/2026-08-28-test-temp-dir-self-cleanup.md

@@ -15,7 +15,7 @@ Retrofit removal of every `dsh-*` temp dir a spec file creates, at the owning te
 - Spec files that created dirs without removing any now track each created root in a module-level list and delete the list in `afterEach`/`afterAll` (`rm`/`rmSync` with `recursive: true, force: true`), the convention already used across the session packages. Root-creating helpers (`tmp()`, `tempDir()`, `fakeLauncher()`, harness functions) register the root at creation, so every caller is covered at one point.
 - Module-scope fixture dirs shared by a whole file (executor spill dirs) are removed in `afterAll` after the last test.
 - The file list came from the observed-residue inventory on the CI host (a template histogram of current `/tmp/dsh-*` dirs): only spec files whose dirs actually appeared were leak sources. Files that already remove their dirs (agent-team, tool-subagent, list-children, hooks coverage cases) were confirmed clean on the normal-exit path and left unchanged.
-- Product per-process spill roots (`privateSpillDir` in `dsh-subprocess-local/spawn`, `privateRoot` in `dsh-spill-local/store`) register a `process.once('exit')` handler that removes the memoized directory, so every process that used the spawn/spill path cleans up on normal exit.
+- Product cleanup is limited to the per-process spill directory of `dsh-subprocess-local/spawn` (`privateSpillDir`): it is removed at a JavaScript-observable process exit while it is EMPTY — collectors unlink their spill files on dispose, so a normal exit leaves at most empty residue, and a directory still holding spill files keeps them (their content may outlive the process). The removal is best-effort (a Windows-held handle must not change the exit code). `dsh-spill-local`'s default root is deliberately NOT exit-deleted: it is covered by the package's own 30-day startup sweep, and the retention decision in the 2026-07-17 local-spill-startup-cleanup note forbids deleting fresh spill artifacts that resumed or forked sessions may still reference.
 
 ## Verification
 
@@ -34,6 +34,6 @@ Not chosen: the files that leak each create roots through their own small helper
 
 ## Consequences
 
-- Bought: on normal completion — including failed tests — a spec's `dsh-*` dirs are removed at teardown, and per-process spill roots are removed when their process exits normally.
+- Bought: on normal completion — including failed tests — a spec's `dsh-*` dirs are removed at teardown; the empty per-process spill directory of `dsh-subprocess-local` is removed at a JavaScript-observable process exit.
 - Cost: a process killed with SIGKILL (a cancelled run, a timeout kill) cannot run any in-process teardown; its in-flight residue remains. The machine-side timer stays as the backstop for that path.
-- Cost: dirs created by a spawned child are covered only when the test knows their paths; product-owned per-process roots are covered by the exit handler in the process that created them.
+- Cost: dirs created by a spawned child are covered only when the test knows their paths; product-owned spill dirs holding spill files, and `dsh-spill-local`'s default root, keep their files per the existing retention policy and are cleaned by that package's own sweep.

+ 3 - 3
.agents/notes/implemented/process/2026-08-28-test-temp-dir-self-cleanup.zh.md

@@ -15,7 +15,7 @@ Status: implemented
 - 创建目录但从不删除的 spec 文件,现在把每个创建的 root 记入模块级列表,并在 `afterEach`/`afterAll` 里删除(`rm`/`rmSync` 带 `recursive: true, force: true`)——与 session 包既有的 `roots.splice(0)` 约定一致。创建 root 的 helper(`tmp()`、`tempDir()`、`fakeLauncher()`、harness 函数)在创建处登记,一个点覆盖全部调用方。
 - 整文件共享的模块级 fixture 目录(executor spill 目录)在最后一个测试之后的 `afterAll` 里删除。
 - 目标文件清单来自 CI 主机上的残留实测清单(当前 `/tmp/dsh-*` 目录的模板直方图):只有目录确实出现在残留里的 spec 文件才是泄漏源。已有删除逻辑的文件(agent-team、tool-subagent、list-children、hooks coverage cases)确认在正常结束路径上本来干净,不改。
-- 产品侧每进程 spill root(`dsh-subprocess-local/spawn` 的 `privateSpillDir`、`dsh-spill-local/store` 的 `privateRoot`)注册 `process.once('exit')` handler,在进程正常退出时删除记忆化的目录——凡走过 spawn/spill 路径的进程都会在正常结束时清理。
+- 产品侧清理限定在 `dsh-subprocess-local/spawn` 的每进程 spill 目录(`privateSpillDir`):在 JavaScript 可观察的进程退出时仅当目录**为空**才删除——collector 在 dispose 时删除自己的 spill 文件,因此正常退出最多留下空目录残留;仍持有 spill 文件的目录会保留(其内容可能比进程更长命)。删除是 best-effort(Windows 上被占用的句柄不得改变退出码)。`dsh-spill-local` 的默认 root **刻意不做**退出删除:该 root 由包自带的 30 天启动 sweep 覆盖,且 2026-07-17 local-spill-startup-cleanup note 的保留策略禁止删除 resume/fork 会话仍可能引用的新 spill 产物。
 
 ## Verification
 
@@ -34,6 +34,6 @@ Sweep 步骤与 timer 只删已存在的残留;本地运行仍会累积,机
 
 ## Consequences
 
-- 收益:正常结束(含测试失败)时,spec 的 `dsh-*` 目录在 teardown 删除;每进程 spill root 在其进程正常退出时删除。
+- 收益:正常结束(含测试失败)时,spec 的 `dsh-*` 目录在 teardown 删除;`dsh-subprocess-local` 的空每进程 spill 目录在 JavaScript 可观察的进程退出时删除。
 - 代价:被 SIGKILL 的进程(run 被取消、超时被杀)无法运行任何进程内 teardown,飞行中的残留仍在——机器侧 timer 继续兜底该路径。
-- 代价:子进程创建的目录只有在测试知道其路径时才被覆盖;产品自有每进程 root 由创建它的进程的 exit handler 覆盖。
+- 代价:子进程创建的目录只有在测试知道其路径时才被覆盖;产品自有、仍持有 spill 文件的目录与 `dsh-spill-local` 的默认 root 按既有保留策略保留文件,由该包自身的 sweep 清理。

+ 1 - 9
packages/spill/spill-local/src/store.ts

@@ -7,7 +7,7 @@
  */
 
 import { createHash, randomBytes } from 'node:crypto'
-import { mkdtempSync, rmSync } from 'node:fs'
+import { mkdtempSync } from 'node:fs'
 import { mkdir, open } from 'node:fs/promises'
 import { join } from 'node:path'
 import { tmpdir } from 'node:os'
@@ -38,14 +38,6 @@ export function privateRoot(): string {
   return defaultRoot
 }
 
-// The spill root is private to this process, so it is removed when the process
-// ends normally; a process killed with SIGKILL cannot run this, and its
-// residue is left to OS temp hygiene.
-/* v8 ignore next 3 -- exit listeners run after the coverage dump; removal is verified by the CI /tmp residue measurement. */
-process.once('exit', () => {
-  if (defaultRoot !== undefined) rmSync(defaultRoot, { recursive: true, force: true })
-})
-
 // Spill keeps its empty-name policy local so storage backends stay decoupled.
 /* jscpd:ignore-start */
 /**

+ 2 - 2
packages/subprocess/subprocess-local/README.i18n.yaml

@@ -2,5 +2,5 @@
 # side as of the last confirmed-consistent state. Both languages carry equal authority;
 # after editing either side, bring the other along and re-record with:
 #   pnpm run verify-translation-pairing --write packages/subprocess/subprocess-local/README.md
-README.md: dd9edbc99578f5411fad993cf88f93429dc9cab2
-README.zh.md: 7bb43f7bbce5ef6b128edc9ef2104d35a5887845
+README.md: 28b61eeec972b3bcd80c91824eaa972d4d4e469b
+README.zh.md: fce2bf42fdfe8a751b104eb01cad824dfe41cd37

+ 2 - 2
packages/subprocess/subprocess-local/README.md

@@ -50,7 +50,7 @@ Collect mode keeps the last `maxBytes` of a stream in memory — errors and fina
 
 ### Shutdown behavior
 
-Normal disposal terminates every running tree and terminal and awaits their exit. During a JavaScript-observable host exit — direct `process.exit()`, default uncaught exceptions, default unhandled rejections — a synchronous finalization force-terminates everything still owned (SIGKILL to the group, `taskkill /T /F` on Windows) without creating promises or timers. Unhandled `SIGTERM`/`SIGINT`/`SIGHUP`, `SIGKILL`, fatal OOM, native crashes, and power loss need an external supervisor.
+Normal disposal terminates every running tree and terminal and awaits their exit. During a JavaScript-observable host exit — direct `process.exit()`, default uncaught exceptions, default unhandled rejections — a synchronous finalization force-terminates everything still owned (SIGKILL to the group, `taskkill /T /F` on Windows) without creating promises or timers. The same exit removes the private per-process spill directory when it is empty; a directory still holding spill files keeps them (their content outlives the process) until an external cleanup. Unhandled `SIGTERM`/`SIGINT`/`SIGHUP`, `SIGKILL`, fatal OOM, native crashes, and power loss need an external supervisor.
 
 ### What can go wrong
 
@@ -127,7 +127,7 @@ These limits define when the provider is a poor fit or needs special operational
 - **A daemonized terminal descendant can still escape the observable boundary** — on macOS, a child that reparents before any foreground-inspection snapshot is no longer discoverable from the PTY root; on Linux, a `setsid` child leaves both the tree and the owned terminal session; the provider adds no continuous process-table monitor.
 - **In-process cleanup requires a JavaScript-observable exit** — direct `process.exit()`, default uncaught exceptions, and default unhandled rejections emit Node's synchronous `exit` event; an unhandled `SIGTERM`, `SIGINT`, or `SIGHUP`, `SIGKILL`, fatal OOM, `process.abort()`, native crashes, and power loss require an external supervisor, container init, or equivalent OS owner.
 - **The credential scrub is a name heuristic** — `*KEY*`/`*PASSWORD*`/`*SECRET*`/`*TOKEN*` only; differently named secrets (for example `*PASSPHRASE*`) pass through, and a whitelist for over-scrubbed variables is noted future work.
-- **Completed spill files are not deleted** — bounded full-output recovery files (and the private per-process spill directory) accumulate under the OS tmpdir until something external cleans them.
+- **Completed spill files are not deleted** — bounded full-output recovery files accumulate under the OS tmpdir until something external cleans them; the private per-process spill directory is removed at a JavaScript-observable exit once it is empty.
 
 <a id="dev-note"></a>
 ### Dev Note

+ 2 - 2
packages/subprocess/subprocess-local/README.zh.md

@@ -50,7 +50,7 @@ kind: "package-reference"
 
 ### 关闭行为
 
-正常 dispose 会终止每棵仍在运行的进程树与终端并等待其退出。在 JavaScript 可观察的宿主退出期间——直接 `process.exit()`、默认未捕获异常、默认未处理 rejection——同步最终清理会强制终止所有仍归本包所有的对象(对进程组发送 SIGKILL,Windows 上运行 `taskkill /T /F`),且不创建任何 Promise 或定时器。未处理的 `SIGTERM`/`SIGINT`/`SIGHUP`、`SIGKILL`、fatal OOM、native crash 与断电则需要外部 supervisor。
+正常 dispose 会终止每棵仍在运行的进程树与终端并等待其退出。在 JavaScript 可观察的宿主退出期间——直接 `process.exit()`、默认未捕获异常、默认未处理 rejection——同步最终清理会强制终止所有仍归本包所有的对象(对进程组发送 SIGKILL,Windows 上运行 `taskkill /T /F`),且不创建任何 Promise 或定时器。同一退出阶段会删除每进程私有 spill 目录(仅当其为空时);仍持有 spill 文件的目录会保留这些文件(其内容比进程更长命),直到外部机制清理。未处理的 `SIGTERM`/`SIGINT`/`SIGHUP`、`SIGKILL`、fatal OOM、native crash 与断电则需要外部 supervisor。
 
 ### 可能出错的地方
 
@@ -127,7 +127,7 @@ spill 文件以 `0600` 权限、`O_EXCL` 与随机名称在 `0700` 每进程目
 - **守护化的终端后代仍可能逃出可观察边界**——在 macOS 上,子进程如果在任何前台检查快照之前重新设定父进程,将无法再从 PTY 根进程发现;在 Linux 上,调用 `setsid` 的子进程会同时离开进程树与自有终端会话;本提供方不新增持续进程表监视器。
 - **进程内清理要求退出阶段仍能执行 JavaScript**——直接 `process.exit()`、默认未捕获异常和默认未处理 rejection 会发出 Node 同步 `exit` 事件;未处理的 `SIGTERM`、`SIGINT` 或 `SIGHUP`、`SIGKILL`、fatal OOM、`process.abort()`、native crash 与断电,都需要外部 supervisor、容器 init 或等价的 OS 所有者负责。
 - **凭据清除依赖名称启发式规则**——只匹配 `*KEY*`/`*PASSWORD*`/`*SECRET*`/`*TOKEN*`;名称不同的 secret(例如 `*PASSPHRASE*`)会继续传递,对误删变量引入白名单属于已记录的后续工作。
-- **不会删除已完成的 spill 文件**——有界的完整输出恢复文件(以及每进程私有 spill 目录)会在 OS tmpdir 下累积,直到外部机制进行清理。
+- **不会删除已完成的 spill 文件**——有界的完整输出恢复文件会在 OS tmpdir 下累积,直到外部机制进行清理;每进程私有 spill 目录在 JavaScript 可观察的退出阶段一旦为空即被删除。
 
 <a id="dev-note"></a>
 ### 开发备注

+ 11 - 6
packages/subprocess/subprocess-local/src/spawn.ts

@@ -10,7 +10,7 @@
 import { type ChildProcess, spawn, spawnSync } from 'node:child_process'
 import type { Readable } from 'node:stream'
 import { randomBytes } from 'node:crypto'
-import { closeSync, mkdtempSync, openSync, rmSync, unlinkSync, writeSync } from 'node:fs'
+import { closeSync, mkdtempSync, openSync, readdirSync, rmdirSync, unlinkSync, writeSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { setTimeout as sleepMs } from 'node:timers/promises'
@@ -91,12 +91,17 @@ function privateSpillDir(): string {
   return defaultSpillDir
 }
 
-// The spill directory is private to this process, so it is removed when the
-// process ends normally; a process killed with SIGKILL cannot run this, and
-// its residue is left to OS temp hygiene.
-/* v8 ignore next 3 -- exit listeners run after the coverage dump; removal is verified by the CI /tmp residue measurement. */
+// The per-process spill directory is removed at process exit only while it is
+// EMPTY: collectors unlink their spill files on dispose, so a normal exit
+// leaves at most empty residue. A directory still holding spill files keeps
+// them (their content outlives the process), and a SIGKILLed process cannot
+// run this at all; both are left to OS temp hygiene.
+/* v8 ignore next 3 -- exit listeners run after the coverage dump; empty-only removal is verified by the CI /tmp residue measurement. */
 process.once('exit', () => {
-  if (defaultSpillDir !== undefined) rmSync(defaultSpillDir, { recursive: true, force: true })
+  if (defaultSpillDir === undefined) return
+  try {
+    if (readdirSync(defaultSpillDir).length === 0) rmdirSync(defaultSpillDir)
+  } catch { /* best-effort: a Windows-held handle must not change the exit code. */ }
 })
 
 /**