From b2d7a3ad3a1471082c23183589fe8564523f8bdf Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Mon, 28 Sep 2026 23:34:38 -0700 Subject: [PATCH 01/20] docs(plan): map runner hygiene tasks and evidence gates --- docs/plans/2026-09-28-runner-hygiene.md | 32 +++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 docs/plans/2026-09-28-runner-hygiene.md diff --git a/docs/plans/2026-09-28-runner-hygiene.md b/docs/plans/2026-09-28-runner-hygiene.md new file mode 100644 index 00000000..46d5b251 --- /dev/null +++ b/docs/plans/2026-09-28-runner-hygiene.md @@ -0,0 +1,32 @@ +# Runner hygiene execution plan + +## Status + +Research drafted against `develop` commit `e2f9dcae0554ff63921df618a819fd5e6afe80d2` on `test/runner-hygiene`. Only Task 10 research and this plan are authorized in this pass. No implementation, backlog removal, push, PR or merge has occurred. Native Windows/Linux proof and complete manual lifecycle certification remain open. Default sibling handling is list-only on every platform. + +## Contract and dependencies + +The [v2 scope](2026-09-28-remediation-program-v2.md#v5-testrunner-hygiene-suites-that-clean-up-after-themselves) inherits [archived Branch 9](../archive/2026-09-28-superpowers-plan-branch-9-follow-ups.md) Tasks 5, 7 and 10–13, under B9-R1–R8. V1 is integrated at the baseline. V4/V6 must coordinate before changing environment-helper consumers. Worktree ownership is limited to this branch; shared manifests remain the integration owner's responsibility. + +The [cleanup design](2026-09-28-test-temp-folder-cleanup-design.md) selects B9-R5's explicit list-only fallback. Task 11 must not interpret a dead PID, empty process group, empty registry or empty handle scan as proof of abandonment. Task 12 must keep the interrupted root even after its known child exits on list-only platforms. This conditions the archived example's removal assertion; it does not relax B9-R5. + +## File, test and dependency map + +| Unit | Exact owned files or proposed files | Validation and acceptance | Depends on | +|---|---|---|---| +| Task 10 research | This plan; `docs/plans/2026-09-28-test-temp-folder-cleanup-design.md`; ignored scratch report/probes | Real macOS orphan observation, official docs, explicit unmeasured platforms; Markdown, links, docs citations/layout | Baseline and brief | +| Task 5 About render | New `tests/kit/about-install-edit-render.test.mjs`; read `src/lib/dashboard/client/about.mjs`, `src/lib/install-edits.mjs`, `src/commands/status/sections/natives.mjs` | Real About renderer, escaped single Ruflo pin line, no AgentDB line/no-edit line; wording mutation fails; existing `about-install-edits.test.mjs`, `about-agentdb-join.test.mjs` | Research handoff; fresh file claims | +| Task 7 concurrent writers | `scripts/real-state-tripwire.mjs`, `tests/kit/real-state-tripwire.test.mjs` | Absent-to-present `.claude-flow`, two proven-config files are concurrent locally, fail in strict mode; config.json still fails; `run-tests-runner.test.mjs`, `home-sandbox-tripwire.test.mjs` | Reverify installed supported Ruflo sources; no fabricated version | +| Task 11 owner record and safe paths | New `scripts/run-roots.mjs`, `tests/kit/run-roots.test.mjs`; `scripts/run-tests.mjs`, `tests/kit/run-tests-runner.test.mjs`, `AGENTS.md` | Red then green path/owner/schema/symlink/UID/host/invalid data tests; listing does not change exit; interrupted roots never removed by defaults; `real-state-tripwire.test.mjs`, `temp-dir-helper.test.mjs` | Task 10 decisions accepted; preserve synchronous spawn/signal behavior | +| Task 12 focus and exit proof | `scripts/run-tests.mjs`, `tests/kit/run-tests-runner.test.mjs`, `AGENTS.md` | Focus pass=0, leftover=4, missing args=2; killed runner's idle child and concurrently live runner retained; root still retained after child death on list-only platforms; exact-PID cleanup finally | Task 11; controller updates external brief template | +| Windows smoke lifetime diagnosis | Proposed `tests/kit/status-zero-spawn.test.mjs`; new disposable fixture only if needed | Handshake shows whether fork outlives parent; compare explicit close wait; preserve assertion and cleanup errors; Windows Node 24 regression required | Controller authorizes implementation; native Windows CI, not fixture simulation | +| LQ-1 environment premise | Read `scripts/run-tests.mjs`, `tests/kit/helpers/home-sandbox.mjs`; if proven defect, those files plus `tests/kit/run-tests-runner.test.mjs`, `tests/kit/spawn-env-guard.test.mjs` | Current runner deletes FORCE_COLOR only; demonstrate sentinel AQE variables across each actual child boundary before any patch; retain state isolation | Task 12; reconcile scope text with source | +| LQ-4 Chrome environment | `tests/ui/helpers/launch-chrome.mjs`; new `tests/kit/launch-chrome-env.test.mjs`; environment helper only with exact claim | Preserve required display/path/platform variables and private Chrome temp; exclude user state; mocked launch failure and close cleanup; native UI smoke | Task 12; coordinate helper users; installed Playwright available | +| Task 13 reviewed inventory | Ignored source/list/report in controller-approved report folder, no tracked cleanup program | Literal absolute paths, prefix attribution, exclusion counts, independent review of same snapshot; no removal | Last focused run; controller provides report destination/reviewer | +| Branch handoff | Update this plan and design status; archive via `scripts/docs-relocate.mjs` in completion PR with index rows | Focused gates, type/lint/build and hermetic full gate appropriate to eventual implementation; exact commit/evidence receipt | All implementation units complete; integration approval | + +## Execution boundaries + +For now, use `node scripts/run-tests.mjs exec -- --test ` with disposable home/state roots. After Task 12, use `node scripts/run-tests.mjs focus `. Do not use pnpm in a worktree with symlinked dependencies. Run focused failure-path checks before wider gates; do not repeat green gates without a new concern. Each code unit needs its own failing/passing evidence and conventional commit after authorization. Before editing/staging `AGENTS.md`, verify there is no injected drift. + +Task 13 excludes recently modified entries, live-handle matches, unattributed prefixes, product-created `ak-sync-preview-npm-*`, and valid owner roots. Age is a manual-review filter only. Task 13 does not implement deletion. No unit claims interrupted-run backlog reclamation until a platform can prove every descendant gone. From 0027ad26526f386729b8980ab93faf3fda3fbc34 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Mon, 28 Sep 2026 23:34:47 -0700 Subject: [PATCH 02/20] docs(research): how test temp folders leak and how a run root is proven abandoned --- ...6-09-28-test-temp-folder-cleanup-design.md | 127 ++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 docs/plans/2026-09-28-test-temp-folder-cleanup-design.md diff --git a/docs/plans/2026-09-28-test-temp-folder-cleanup-design.md b/docs/plans/2026-09-28-test-temp-folder-cleanup-design.md new file mode 100644 index 00000000..018c4956 --- /dev/null +++ b/docs/plans/2026-09-28-test-temp-folder-cleanup-design.md @@ -0,0 +1,127 @@ +# Test temp folder cleanup design + +## Status and decision + +Research snapshot: 2026-09-28, `e2f9dcae0554ff63921df618a819fd5e6afe80d2`, macOS Darwin 27.0.0, Node 26.4.0; Node 22.22.3 used for CLI checks. Proposed behavior is **list-only for abandoned sibling roots on macOS, Linux and Windows**. No candidate establishes complete descendant liveness. This uses B9-R5's explicit fallback, preserves B9-R1–R8, and introduces no native sweeper or deletion authority. + +The [execution plan](2026-09-28-runner-hygiene.md) maps subsequent work. Raw commands, JSON results, full lexical census, counts and literal experiment paths are retained in ignored `.superpowers/sdd/2026-09-28-runner-hygiene/`. No production/test code changed. This design is reviewable with known evidence gaps; it is not a claim that every creator lifecycle or platform has been certified. + +## Verified runner and creator behavior + +At `scripts/run-tests.mjs:57-85`, the runner creates a unique suite root, redirects all three temp variables, runs synchronous commands, reports leftovers and removes its root after commands finish, including failure. SIGKILL cannot reach this cleanup. The source has no owner record or sibling collector. It strips FORCE_COLOR only (`scripts/run-tests.mjs:68`); LQ-1's premise that this layer strips AQE_EMBEDDER variables is refuted at this revision. Downstream boundaries still need sentinel tests. + +The static census scans `.js`, `.mjs` and `.cjs` under tests for calls to mkdtemp/mkdtempSync/tempDir/sandboxHome/sandboxProject/usePrivateTmpdir/redirectToolState, retaining file, line and source. It finds 634 candidate call sites across 277 files: 117 tempDir calls, 83 module-level sandbox calls, 434 manually managed or unresolved sites. This is a lexical census, not a control-flow proof: aliases, generated fixture strings and wrapper calls require review. The complete `creator-census.json` is the review queue; unresolved sites are never classified safe merely because the file contains an after hook. + +| Lifecycle class | Source evidence | Failure boundary | +|---|---|---| +| Cleanup registered before caller assertions | `tests/kit/helpers/temp-dir.mjs:16-20` creates then registers t.after or file after; `tests/kit/run-tests-runner.test.mjs:15-16` registers immediately | Assertion failure is covered after registration; process kill, allocation-to-registration failure and removal error remain | +| Manual cleanup after successful operations | `tests/kit/status-zero-spawn.test.mjs:86-90` removes its ledger after exec/read | Failed exec or parsing skips ledger removal; the parent run still diagnoses it | +| Module-level creator with file hook | `tests/kit/evidence.test.mjs:9` and `:231`; `tests/kit/refresh.test.mjs:17-19` | Abrupt exit misses hooks; in evidence, an early import/assertion may occur before hook registration | +| Exit hook in plain scripts | `tests/kit/helpers/private-tmpdir.cjs:11-16`; dashboard/statusline callers | Normal exit runs synchronous removal; SIGKILL never does; registration is after allocation | +| Caller-owned tools state | `tests/kit/helpers/home-sandbox.mjs:108-130` | redirectToolState returns restore; caller must arrange finally/hook before assertions; helper itself registers none | +| Caller-owned home/project | `tests/kit/helpers/home-sandbox.mjs:141-157` and `:303-306` | Creation does not register cleanup; consumers determine lifetime | +| Child temp base | `tests/kit/helpers/home-sandbox.mjs:83-97` | spawnEnv uses home/tmp and permits extra overrides; containment requires caller's home and final TMP values to be inside run root | +| Non-Node child | `tests/ui/helpers/launch-chrome.mjs:20-33` | Launch failure and browser.close remove private root; killed test or omitted close bypasses cleanup; Chrome is outside a Node-only registry | +| Hardcoded /tmp strings | `tests/kit/dashboard-live-source.test.mjs:7-21` | Path parsing and assertions only; these lines create no directory or file | + +A full per-site manual classification of the 434 unresolved entries remains a research limitation. It is not needed to establish the conservative list-only decision, but must be completed before claiming exhaustive failure-path coverage or changing helper lifecycle policy. + +## The fourteen reported post-runner leaks + +The [archived premise table](../archive/2026-09-28-superpowers-plan-branch-9-follow-ups.md#premise-verification-done-by-the-planner-tasks-carry-the-evidence-forward) records fourteen newer folders by prefix. These are historical observations, not fourteen reproduced failures today. + +| Historical entries | Attributed creator and cleanup | What can be concluded | +|---|---|---| +| ak-evidence-home ×2 | `tests/kit/evidence.test.mjs:9`, file after at `:231` | Creation precedes imports/assertions and late hook. Early failure or killed process can leak; exact historical cause unknown | +| ak-ruflo-components-evidence-location-home ×2 | `tests/kit/ruflo-components-evidence-location.test.mjs:6`, after at `:18` | Early import/assertion before hook or interruption possible; exact cause unknown | +| ak-refresh-home ×3; ak-refresh-proj ×3 | `tests/kit/refresh.test.mjs:17-19` | Hook is early, so ordinary later assertion failure should clean. Kill, failure before registration or failed removal remain hypotheses | +| ak-status-live-home ×3 | `tests/kit/status-live.test.mjs:17`, after at `:267` | Late registration leaves early initialization failure window; interruption/removal failure possible | +| ak-live-checks-home ×1 | `tests/kit/live-checks.test.mjs:21`, after at `:688` | Late registration has the same exposure; no historical exit trace identifies cause | + +A basename and mtime cannot tell whether an assertion, import, kill or cleanup error caused a specific leak. Reconstructing that requires corresponding process/test logs. Do not rewrite this attribution as proof that every ordinary failed test leaks. + +## Concurrent runs and race windows + +Each mkdtemp root is unique, but sibling worktrees share the temp parent. An owner file is proposed to be written atomically via a temporary file and rename immediately after root creation. Until a valid record exists, keep the root (B9-R3). Old runners also have no owner and remain untouched. Malformed records, wrong host/user/platform, unreadable entries and unknown schema all mean keep. + +An owner may finish during inspection, a PID may be reused, a descendant may start after a snapshot, and a root could change between validation and deletion. Owner records and start identities do not close these races. Current selection performs no sibling removal, so racing observations cannot authorize it. A future collector needs complete process containment plus a stable filesystem identity/revalidation protocol; a path name and PID are insufficient. + +## Abandonment candidates and proof limits + +| Candidate | macOS | Linux | Windows | Signal and cost assessment | +|---|---|---|---|---| +| A: process group / parent descent | Reject: detached descendants escape recorded group | Same source counterexample; no native execution here | Reject: snapshots can lose exited intermediate parents; PID reuse complicates descent | detached changes session/group and requires signal forwarding; no equivalence proof | +| B: parent identity plus handle scan / descent | Reject: measured live orphan has no root handle | Same logical gap; native /proc and lsof not measured | ParentProcessId/CreationDate cannot recover every missing intermediate ancestor | Avoiding signal changes is possible; fast probes still cannot prove absence | +| C: every Node process imports PID registrar | Reject: non-Node children; env replacement; startup registration race | Same coverage gap; no native execution | Same coverage gap; no native execution | NODE_OPTIONS affects children and can be removed; no transparent behavior proof | +| Selected: list-only | Never returns abandoned | Never returns abandoned | Never returns abandoned | No process launch/signal change; no expensive liveness probe required for removal | + +`tests/kit/process-tree.test.mjs:52`, `tests/kit/exec-kill-tree.test.mjs:53` and `tests/kit/mcp-tool-call.test.mjs:21` deliberately use detached children. POSIX detached children create a new group/session; unref and stdio determine parent waiting behavior. Thus even an empty original group is insufficient. [Node child process documentation](https://nodejs.org/api/child_process.html#optionsdetached). + +Windows ParentProcessId may refer to a dead or reused parent. CreationDate helps disambiguate identity, but a snapshot cannot reconstruct an already vanished chain of intermediate processes. This is a design inference from the documented fields, not a native Windows experiment. [Microsoft Win32_Process](https://learn.microsoft.com/en-us/windows/win32/cimwin32prov/win32-process). + +B9-R6's startedAt milliseconds and 2-second reuse tolerance can distinguish a later process, but never establishes that the original process's children exited. Clock precision, permission errors and unparseable identity must resolve to unknown/keep. A newly started owner record should bind observed process start identity, not simply assume its file-write timestamp is the process start. + +## Real macOS orphan experiment and timings + +Scratch `probe.mjs` created an isolated temp parent, home, project and tmp. It launched the existing runner using `exec --repo -- `. The child wrote PID/cwd/TMPDIR to a handshake file then idled for 60 seconds. The controller killed only the exact runner PID, waited for its exit, inspected the known child and finally sent SIGTERM to that child. No process search was used to select kill targets. + +Trimmed evidence: + +```text +runner PID 57521: SIGKILL +child PID 57522: alive=true + PID PPID PGID COMMAND +57522 1 57490 node /sleep.mjs +child TMPDIR: /tmp/ak-suite-tHEZEe +child cwd: /project +root exists=true +lsof -nP +D : exit 1, stdout empty, stderr empty +cleanup ps -p 57522: header only; child gone +``` + +The child cwd was the disposable project, outside its suite root. It had no open root file. The root was retained for review. This proves candidate B unsound for this case and disproves parent-death-only collection. The script's finally targets only its exact owned PIDs; its bounded timer is secondary protection. + +Five sequential samples on this host (no cross-platform claim): + +| Probe/workload | Measured range | Median | +|---|---|---| +| process.kill(childPid, 0) | 0.00075–0.008 ms | 0.00108 ms | +| ps -o lstart= -p childPid | 1.625–2.054 ms | 1.742 ms | +| lsof -nP +D empty suite root | 144.769–150.222 ms | 145.634 ms | +| Plain node --test inert.test.mjs | 64.153–66.150 ms | 64.621 ms | +| Guarded exec of same one-file test | 91.135–100.622 ms | 96.753 ms | + +Observed median guarded overhead was 32.132 ms with 22 empty sandbox tripwire roots. This is not a measurement of a populated real home, a large backlog or the future collector. `/proc` and PowerShell CIM cost are **unmeasured** because there is no native Linux/Windows runtime in this task. A list-only inventory still needs bounded I/O and later performance validation against many roots; no production latency claim is made. + +## Windows locks and CI failure + +The controller supplied CI run **36520154869**, Windows Node 24: `status-zero-spawn.test.mjs` failed in inSandbox rmSync(project), line 55, EPERM; the leftover was ak-spawn-guard-smoke-proj. This failure is attributed evidence from the brief, not a freshly downloaded CI log. + +Verified source: the smoke script forks at `tests/kit/status-zero-spawn.test.mjs:82` then calls process.exit(0) at `:83`, explicitly avoiding waiting for the grandchild. execFileSync waits for that immediate child; it does not establish that the grandchild released its project cwd. A surviving grandchild causing the observed EPERM is a plausible hypothesis, not proven root cause. The fork target itself exits immediately, making scheduling relevant. + +Proposed regression: in a copied disposable fixture, hold the fork on a bounded handshake; record exact child/grandchild PID and spawn/exit/close timestamps; attempt cleanup while held; compare an explicit wait-for-close variant. Capture Windows error code/path and remaining entries. Preserve both assertion and cleanup errors rather than letting finally mask the first. The ignored `windows-probe-plan.md` specifies native Windows Node 24/26 diagnostics, one timed CIM snapshot and exact-PID cleanup. No workflow was edited or run. + +Recursive rm is not atomic; an error can leave a partially removed tree. On a removal error, report retained/partially removed and never claim an intact preserved root. Windows cwd/open-handle behavior depends on handle sharing; not every open handle universally blocks deletion. Native locking behavior remains unmeasured here. The proof must precede any future removal attempt. + +Node documents recursive rm retries for EBUSY, EMFILE, ENFILE, ENOTEMPTY and EPERM with linear backoff; maxRetries defaults to 0 and retryDelay to 100 ms. These options are ignored without recursive mode. Retries cannot establish ownership or abandonment. [Node fs.rmSync](https://nodejs.org/api/fs.html#fsrmsyncpath-options). + +## Focused runs on Node 22 and 26 + +Both installed binaries were executed in the sandbox with an invalid node.config.json and one inert test. Plain `node --test inert.test.mjs` passed on 22.22.3 and 26.4.0, demonstrating that neither loaded the config by default. Adding `--experimental-default-config-file` failed with exit 9 and invalid-content diagnostics on both. Explicit `--experimental-config-file` and `--import` are also opt-in; external NODE_OPTIONS can inject imports, but a repository file cannot silently establish that environment. + +`--test-global-setup=./missing.mjs` is rejected as a bad option (exit 9) by installed 22.22.3; 26.4.0 recognizes it and fails resolving the intentionally missing module (exit 7). Thus the inherited wording must not imply global setup exists on Node 22.22.3. B9-R7's conclusion stands: plain node --test is unguarded; use the explicit wrapper. [Node 22.22.3 CLI](https://nodejs.org/download/release/v22.22.3/docs/api/cli.html), [Node 26.4.0 CLI](https://nodejs.org/download/release/v26.4.0/docs/api/cli.html). + +## Backlog and classification boundary + +A read-only direct-child listing of the real temp parent observed **34,013** current-user, non-symlink entries beginning ak-, grouped into 127 suffix-normalized prefixes. These raw counts include this research's own new root and concurrent activity; they are not removal candidates. The full per-prefix snapshot is `backlog-counts.json`, generated by retained `census.py`. Leading counts: ak-usage 2,520; ak-adapter-conformance-cli 2,301; ak-quota 2,152; ak-live-service 2,150; ak-adapter-consent 2,124; ak-intel-history 2,106; ak-adapter-grants 1,836; ak-stamp 1,525; ak-conformance-tiers-grants 1,512; ak-usage-solo 1,400; ak-host-cli 855; ak-host-project 855. + +Task 13 must take a fresh snapshot after the last focused run. Direct children only, real absolute parent, current owner, no symlinks; exclude valid owner roots, product ak-sync-preview-npm roots, all paths found by one lsof snapshot, anything changed in 24 hours, and unattributed prefixes. A: before 2026-09-27 12:01 local runner landing; B: later attributable leaks; C: legacy ownerless suite roots. Keep all exclusion counts and script source, independently rederive counts, and submit literal paths for maintainer judgment. A missing lsof result due to error is incomplete evidence, not proof that nothing is in use. Idle orphans can evade lsof, so this remains a manual review list with no deletion authority. + +## Decisions for Tasks 11–13 + +1. Implement one proveAbandoned interface with **abandoned=false, reason=cannot prove complete descendant exit** by default on macOS/Linux/Windows. Unit fixtures may exercise collector plumbing but cannot enable a production deletion path or constitute native platform proof. +2. Proposed owner fields: schema=1, random runId, absolute canonical root/temp parent, pid, startedAt milliseconds, hostname, platform, uid (null only when unavailable), proofMode=list-only. Validate schema, finite positive PID/timestamp, host/user identity, path and file type. Atomic record writing improves attribution only. +3. Retain B9-R4 path rules for own-root removal; refuse real home/filesystem-root temp bases before allocation, check absolute canonical direct parent, exact suite basename, lstat/non-symlink, POSIX owner. Unknown/error means refuse. List sibling roots without deleting them; do not change command/tripwire/leftover exit precedence (B9-R2). +4. Preserve synchronous runner and Ctrl-C/tool-kill behavior. Owner files are ignored in own leftovers. Interrupted runners remove nothing; completed runs retain current own-root semantics under B9-R1. Own-root cleanup is not a descendant-exit proof; the Windows lifetime regression must address unfinished children explicitly. +5. Task 12's list-only exit proof keeps a killed runner's root while its child lives **and after that child exits**. Do not execute the archived example's unconditional run-3 removal on a list-only platform. This follows B9-R5; controller was notified before dependent implementation. +6. B9-R1–R8 need no safety-rule relaxation. R7 has a factual clarification: global setup is unavailable in installed 22.22.3; config/import still require explicit opt-in. Task 13 remains report-only. Unresolved manual census and native Windows diagnostics stay visible; no claim of exhaustive leak causes or automatic backlog cleanup is justified. From 55f444b6c7e14e223559bf4d95e4e46cfdfb0d2a Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Mon, 28 Sep 2026 23:45:20 -0700 Subject: [PATCH 03/20] docs(research): complete test creator lifecycle census --- docs/plans/2026-09-28-runner-hygiene.md | 2 +- ...6-09-28-test-temp-folder-cleanup-design.md | 326 +++++++++++++++++- 2 files changed, 323 insertions(+), 5 deletions(-) diff --git a/docs/plans/2026-09-28-runner-hygiene.md b/docs/plans/2026-09-28-runner-hygiene.md index 46d5b251..30731ae0 100644 --- a/docs/plans/2026-09-28-runner-hygiene.md +++ b/docs/plans/2026-09-28-runner-hygiene.md @@ -2,7 +2,7 @@ ## Status -Research drafted against `develop` commit `e2f9dcae0554ff63921df618a819fd5e6afe80d2` on `test/runner-hygiene`. Only Task 10 research and this plan are authorized in this pass. No implementation, backlog removal, push, PR or merge has occurred. Native Windows/Linux proof and complete manual lifecycle certification remain open. Default sibling handling is list-only on every platform. +Research drafted against `develop` commit `e2f9dcae0554ff63921df618a819fd5e6afe80d2` on `test/runner-hygiene`. Only Task 10 research and this plan are authorized in this pass. No implementation, backlog removal, push, PR or merge has occurred. The creator census is complete: 647 sites classified, zero unresolved current lifecycles. Native Windows/Linux behavioral proof remains open. Default sibling handling is list-only on every platform. ## Contract and dependencies diff --git a/docs/plans/2026-09-28-test-temp-folder-cleanup-design.md b/docs/plans/2026-09-28-test-temp-folder-cleanup-design.md index 018c4956..3f3c656e 100644 --- a/docs/plans/2026-09-28-test-temp-folder-cleanup-design.md +++ b/docs/plans/2026-09-28-test-temp-folder-cleanup-design.md @@ -4,13 +4,13 @@ Research snapshot: 2026-09-28, `e2f9dcae0554ff63921df618a819fd5e6afe80d2`, macOS Darwin 27.0.0, Node 26.4.0; Node 22.22.3 used for CLI checks. Proposed behavior is **list-only for abandoned sibling roots on macOS, Linux and Windows**. No candidate establishes complete descendant liveness. This uses B9-R5's explicit fallback, preserves B9-R1–R8, and introduces no native sweeper or deletion authority. -The [execution plan](2026-09-28-runner-hygiene.md) maps subsequent work. Raw commands, JSON results, full lexical census, counts and literal experiment paths are retained in ignored `.superpowers/sdd/2026-09-28-runner-hygiene/`. No production/test code changed. This design is reviewable with known evidence gaps; it is not a claim that every creator lifecycle or platform has been certified. +The [execution plan](2026-09-28-runner-hygiene.md) maps subsequent work. Raw commands, JSON results, full lexical census, counts and literal experiment paths are retained in ignored `.superpowers/sdd/2026-09-28-runner-hygiene/`. No production/test code changed. Current creator lifecycles are fully classified below; native Windows/Linux behavior remains explicitly unmeasured. ## Verified runner and creator behavior At `scripts/run-tests.mjs:57-85`, the runner creates a unique suite root, redirects all three temp variables, runs synchronous commands, reports leftovers and removes its root after commands finish, including failure. SIGKILL cannot reach this cleanup. The source has no owner record or sibling collector. It strips FORCE_COLOR only (`scripts/run-tests.mjs:68`); LQ-1's premise that this layer strips AQE_EMBEDDER variables is refuted at this revision. Downstream boundaries still need sentinel tests. -The static census scans `.js`, `.mjs` and `.cjs` under tests for calls to mkdtemp/mkdtempSync/tempDir/sandboxHome/sandboxProject/usePrivateTmpdir/redirectToolState, retaining file, line and source. It finds 634 candidate call sites across 277 files: 117 tempDir calls, 83 module-level sandbox calls, 434 manually managed or unresolved sites. This is a lexical census, not a control-flow proof: aliases, generated fixture strings and wrapper calls require review. The complete `creator-census.json` is the review queue; unresolved sites are never classified safe merely because the file contains an after hook. +The completed AST and source census below classifies 647 creator sites across 279 files. The original 634-hit lexical inventory is superseded; two comment hits were excluded, three inline allocations and seven aliased calls were recovered, five sandboxConfigBase calls were added, and two executable child templates were retained separately. | Lifecycle class | Source evidence | Failure boundary | |---|---|---| @@ -24,7 +24,15 @@ The static census scans `.js`, `.mjs` and `.cjs` under tests for calls to mkdtem | Non-Node child | `tests/ui/helpers/launch-chrome.mjs:20-33` | Launch failure and browser.close remove private root; killed test or omitted close bypasses cleanup; Chrome is outside a Node-only registry | | Hardcoded /tmp strings | `tests/kit/dashboard-live-source.test.mjs:7-21` | Path parsing and assertions only; these lines create no directory or file | -A full per-site manual classification of the 434 unresolved entries remains a research limitation. It is not needed to establish the conservative list-only decision, but must be completed before claiming exhaustive failure-path coverage or changing helper lifecycle policy. +Every site now has a current cleanup mechanism and evidence reference. This does not establish historical leak causality, successful native cleanup, or freedom from setup-before-registration gaps. The success-path classes identify assertion/error leak exposure; the parent-hook classes distinguish allocations already covered by enclosing cleanup. + +## Child temp routing and paths outside the run root + +The retained `temp-routing-sites.txt` records explicit temp-variable sites. `spawnEnv` places child temp under the supplied home; both it and redirectToolState remain beneath the outer suite when the home/base was created there. Nested runner refusal uses a temp base deliberately inside a disposable project (`tests/kit/run-tests-runner.test.mjs:119`); harvest redirects all three variables into its disposable repository (`tests/kit/agentdb-retirement.test.mjs:221-225`). Neither is a real repository escape. Chrome pins all four platform variables at `tests/ui/helpers/launch-chrome.mjs:22`. + +The source does contain child environments that lose outer-root containment: the live AQE version probe at `tests/live/aqe-stop-hook-conformance.test.mjs:40` passes only PATH and NO_COLOR, so it does not inherit the runner's temp variables. Shell command-discovery probes at `tests/live/aqe-stop-hook-conformance.test.mjs:24` and `tests/live/aqe-codex-guidance-conformance.test.mjs:43` likewise use PATH-only environments. These are source-confirmed routing gaps, not measured folder leaks. Later live fixtures explicitly set TMPDIR but do not establish Windows TEMP/TMP containment. The proof-key guard test sets only TMPDIR (`tests/kit/aqe-live-proof-key-guard.test.mjs:24`); its expected early refusal does not make that a portable temp-isolation contract. + +Literal Windows temp paths in ruflo-memory-location tests are injected path-classification inputs, not spawned child environments. dashboard-live-source's /tmp values are also parsing inputs. No cleanup design may assume every subprocess preserves the root simply because the top runner sets it. ## The fourteen reported post-runner leaks @@ -117,6 +125,316 @@ A read-only direct-child listing of the real temp parent observed **34,013** cur Task 13 must take a fresh snapshot after the last focused run. Direct children only, real absolute parent, current owner, no symlinks; exclude valid owner roots, product ak-sync-preview-npm roots, all paths found by one lsof snapshot, anything changed in 24 hours, and unattributed prefixes. A: before 2026-09-27 12:01 local runner landing; B: later attributable leaks; C: legacy ownerless suite roots. Keep all exclusion counts and script source, independently rederive counts, and submit literal paths for maintainer judgment. A missing lsof result due to error is incomplete evidence, not proof that nothing is in use. Idle orphans can evade lsof, so this remains a manual review list with no deletion authority. +## Completed creator census + +The final census classifies **647 creator sites in 279 files, with zero unclassified current lifecycle sites**: 645 AST calls plus two executable child-template sites. It adds seven `makeTempDir` aliases, five sandboxConfigBase calls and three inline allocator definitions, and removes two comment-only lexical hits. Local factory invocations are represented by their allocator definition and caller contract, rather than counted as additional allocations. The retained AST/parser and manual override scripts reproduce the census without executing tests or tools. + +This is a source lifecycle classification, not a guarantee that cleanup runs after SIGKILL or that recursive removal succeeds. Each returned fixture is traced to caller cleanup; mixed callers stay mixed. Assertions before hook registration were checked in the allocating scope, excluding callbacks that run later. `sandboxHome` and `sandboxProject` register **no** cleanup themselves; they must not inherit tempDir's safe-return contract. Allocation, setup and multiple cleanup operations can still throw before protection or skip a later cleanup. + +| Code | Sites | Current lifecycle | +|---|---:|---| +| H | 130 | Shared helper registers cleanup before return; see temp-dir:19 or home-sandbox:176. | +| R | 168 | Local test/file hook registration; cleanup line shown. No preceding direct assert/assertSandboxed call in the allocating scope was found. | +| M | 87 | Module allocation with file hook; early imports/assertions can precede registration. | +| F | 113 | Removal in finally; setup before entering try remains exposed. | +| S | 68 | Direct cleanup reached only on normal execution, often after assertions. | +| CF | 20 | Factory returns root; callers remove in finally; pre-return setup remains exposed. | +| CH | 1 | Factory returns root; caller registers a hook; pre-registration setup remains exposed. | +| CM | 5 | Factory callers have mixed success-only/finally/hook lifecycles; exact examples in JSON. | +| O | 2 | Helper transfers ownership without registering cleanup; callers classified separately. | +| CS | 15 | Factory returns root; callers remove only on normal execution. | +| E | 6 | Process exit handler; normal exit only. | +| XS | 1 | Executable child template uses success-path cleanup. | +| XL | 1 | Executable child template deliberately leaks to exercise runner detection. | +| PE | 10 | Allocation covered transitively by enclosing private temp exit handler. | +| P | 15 | Allocation covered transitively by parent folder cleanup hook. | +| G | 2 | Root added to collection consumed by already registered/file cleanup hook. | +| C | 3 | Returns a cleanup method; construction failure handling and caller obligations described in JSON. | + +Source index below uses `allocation line:code→cleanup/caller evidence line` within the named file. H and E also use the shared helper references in the legend. Full cleanup expressions, all evidence locations, caller notes and scope boundaries are in ignored `creator-lifecycle-final.json`; reproducible scripts are `ast-census.mjs`, `lifecycle.mjs`, `classify.py` and `render-census.py`. No site is deemed safe simply because its file contains an unrelated hook. + +| Source file | Every creator site and lifecycle evidence | +|---|---| +| `tests/dashboard.test.cjs` | 19:E→helper; 70:PE→19 | +| `tests/kit/about-install-edits.test.mjs` | 11:M→12 | +| `tests/kit/about-security.test.mjs` | 10:M→11 | +| `tests/kit/adapter-admission.test.mjs` | 253:H→helper; 274:H→helper; 303:H→helper; 506:F→540 | +| `tests/kit/adapter-aqe-provider.test.mjs` | 89:CM→115,206,252; 247:H→helper; 350:H→helper; 498:F→514 | +| `tests/kit/adapter-conformance.test.mjs` | 127:S→240; 333:F→345; 356:F→369 | +| `tests/kit/adapter-execution.test.mjs` | 481:F→499 | +| `tests/kit/adapter-grants.test.mjs` | 16:H→helper; 169:H→helper | +| `tests/kit/adapter-hook-runner.test.mjs` | 197:F→208; 304:CS→305,313,320 | +| `tests/kit/adapter-integrity.test.mjs` | 48:F→61; 66:F→76; 81:F→96; 109:F→129; 134:F→152; 157:F→173; 158:F→174; 179:F→187 | +| `tests/kit/adapter-registries.test.mjs` | 74:H→helper | +| `tests/kit/adapter-sources.test.mjs` | 150:F→159; 171:F→182; 187:F→194; 199:F→208; 349:F→361; 367:F→376; 382:F→389; 395:F→402; 439:F→453; 458:F→470; 475:F→487; 492:F→505; 512:F→528 | +| `tests/kit/agent-browser-runtime.test.mjs` | 23:CS→23,88,126 | +| `tests/kit/agentdb-retirement.test.mjs` | 26:M→27; 40:M→41; 218:R→225 | +| `tests/kit/ak-launcher-evidence.test.mjs` | 17:H→helper | +| `tests/kit/aqe-embedding-probe.test.mjs` | 10:R→11 | +| `tests/kit/aqe-embedding-projection.test.mjs` | 14:R→15 | +| `tests/kit/aqe-embedding-transport.test.mjs` | 93:R→94; 109:R→110 | +| `tests/kit/aqe-guidance.test.mjs` | 38:R→39; 50:R→51; 71:H→helper | +| `tests/kit/aqe-lifecycle-migration.test.mjs` | 18:R→19 | +| `tests/kit/aqe-live-proof-key-guard.test.mjs` | 15:H→helper | +| `tests/kit/aqe-project-pin.test.mjs` | 24:H→helper; 283:H→helper; 349:H→helper | +| `tests/kit/aqe-readiness.test.mjs` | 11:F→17 | +| `tests/kit/aqe-store-holders.test.mjs` | 28:H→helper; 39:H→helper; 47:H→helper; 57:H→helper; 78:H→helper; 93:H→helper; 131:H→helper; 149:H→helper; 157:H→helper; 167:H→helper; 183:H→helper; 196:H→helper | +| `tests/kit/aqe-store-merge-fixture.test.mjs` | 30:H→helper; 49:H→helper; 71:H→helper | +| `tests/kit/aqe-store-merge-preview.test.mjs` | 44:H→helper | +| `tests/kit/aqe-store-merge.test.mjs` | 174:H→helper | +| `tests/kit/blocks-drift-parity.test.mjs` | 19:M→20; 30:M→31,36 | +| `tests/kit/blocks-dual-mode.test.mjs` | 33:S→53; 58:S→74; 79:S→96 | +| `tests/kit/blocks.test.mjs` | 99:S→106; 110:S→135; 139:S→146; 178:S→185; 189:S→199 | +| `tests/kit/brain-held-refresh-sync.test.mjs` | 21:M→36; 31:M→36 | +| `tests/kit/brain-held-refresh.test.mjs` | 13:M→14 | +| `tests/kit/claude-env-projection.test.mjs` | 13:R→15; 14:R→15 | +| `tests/kit/claude-window-ledger.test.mjs` | 14:H→helper | +| `tests/kit/clean-machine-setup.test.mjs` | 14:S→46; 50:S→76 | +| `tests/kit/cli-help.test.mjs` | 13:M→14 | +| `tests/kit/cli-json-honesty.test.mjs` | 16:M→18; 17:M→18 | +| `tests/kit/codex-context-command.test.mjs` | 7:M→67 | +| `tests/kit/codex-context.test.mjs` | 11:R→12 | +| `tests/kit/codex-mcp-convergence.test.mjs` | 11:M→12; 23:M→24 | +| `tests/kit/codex-mcp.test.mjs` | 14:CF→18,38,46; 94:F→126; 131:F→145; 150:F→164; 169:F→178; 183:F→196; 201:F→219; 224:F→256; 261:F→297; 302:F→327; 332:F→354; 360:F→398 | +| `tests/kit/codex-plugins.test.mjs` | 10:M→269; 214:F→231 | +| `tests/kit/codex-state.test.mjs` | 13:H→helper | +| `tests/kit/codex-statusline.test.mjs` | 13:G→14,103 | +| `tests/kit/codex-usage-diagnostic.test.mjs` | 13:R→14 | +| `tests/kit/conformance-tiers.test.mjs` | 24:H→helper; 260:H→helper; 298:H→helper; 317:H→helper; 355:H→helper; 389:H→helper; 673:H→helper | +| `tests/kit/context-audit.test.mjs` | 151:F→178 | +| `tests/kit/daemon-sweep-evidence.test.mjs` | 33:H→helper; 49:F→54 | +| `tests/kit/daemons-status.test.mjs` | 11:R→12 | +| `tests/kit/dashboard-context-hooks.test.mjs` | 203:F→262; 267:F→304 | +| `tests/kit/dashboard-hermetic-defaults.test.mjs` | 15:M→16; 56:P→14,16,56 | +| `tests/kit/dashboard-intel-integration.test.mjs` | 129:H→helper | +| `tests/kit/dashboard-project-identity.test.mjs` | 16:R→17 | +| `tests/kit/dashboard-status-cost.test.mjs` | 27:F→75; 29:F→76 | +| `tests/kit/dashboard-status-inprocess.test.mjs` | 36:CF→48,101,104; 38:CF→48,101,104 | +| `tests/kit/deja-vu-lifecycle.test.mjs` | 17:H→helper | +| `tests/kit/deja-vu-teardown-verify.test.mjs` | 15:M→503 | +| `tests/kit/deja-vu.test.mjs` | 160:F→197; 202:F→237; 242:F→277; 282:F→339; 344:F→359 | +| `tests/kit/dispatch-surface.test.mjs` | 79:H→helper | +| `tests/kit/disposable-memory-project.test.mjs` | 40:R→41; 130:R→131 | +| `tests/kit/drift-freshness.test.mjs` | 16:M→17; 26:M→27 | +| `tests/kit/dry-run-nudge.test.mjs` | 26:CF→43,64,65; 28:CF→43,64,65; 37:CF→43,64,65 | +| `tests/kit/evidence.test.mjs` | 9:M→231 | +| `tests/kit/exec-kill-tree.test.mjs` | 23:R→24; 81:R→82 | +| `tests/kit/exec.test.mjs` | 93:F→111; 147:F→185; 190:F→215; 222:F→234 | +| `tests/kit/execution-runner.test.mjs` | 470:H→helper | +| `tests/kit/external-lifecycle.test.mjs` | 31:M→445; 172:F→188; 193:F→209; 214:F→230; 235:F→251; 258:F→271; 276:F→288; 293:F→306; 313:F→328; 335:F→356; 369:F→440; 370:F→441 | +| `tests/kit/file-identity-bigint.test.mjs` | 151:H→helper | +| `tests/kit/footprint-collectors.test.mjs` | 50:R→51 | +| `tests/kit/footprint-executable-paths.test.mjs` | 9:R→10 | +| `tests/kit/footprint-known-files.test.mjs` | 12:F→19 | +| `tests/kit/footprint-observation-forest.test.mjs` | 11:R→12 | +| `tests/kit/footprint-performance.test.mjs` | 20:R→21 | +| `tests/kit/footprint-projects.test.mjs` | 44:R→45 | +| `tests/kit/footprint-snapshot-v2.test.mjs` | 12:R→13 | +| `tests/kit/footprint-stack.test.mjs` | 47:R→48 | +| `tests/kit/guidance-targets.test.mjs` | 34:S→40; 74:S→82; 88:S→95; 100:S→109; 101:S→110; 239:S→259; 265:S→279; 283:S→296 | +| `tests/kit/heal-natives.test.mjs` | 30:H→helper; 34:H→helper; 52:H→helper; 114:H→helper; 139:H→helper; 150:H→helper; 166:H→helper; 214:H→helper; 254:H→helper; 277:H→helper; 297:H→helper | +| `tests/kit/helper-stamp.test.mjs` | 85:H→helper | +| `tests/kit/helpers/aqe-store-merge-fixture.mjs` | 14:H→helper | +| `tests/kit/helpers/aqe-store-merge-harness.mjs` | 119:H→helper | +| `tests/kit/helpers/codex-rollout.mjs` | 130:H→helper | +| `tests/kit/helpers/home-sandbox.mjs` | 110:C→124,125,129; 140:O→139,158,303; 169:R→181; 304:O→139,158,303 | +| `tests/kit/helpers/private-tmpdir.cjs` | 12:E→16 | +| `tests/kit/helpers/project-isolation.mjs` | 117:R→122 | +| `tests/kit/helpers/temp-dir.mjs` | 17:H→18,19,20 | +| `tests/kit/home-sandbox-tripwire.test.mjs` | 19:R→20; 27:XS→49,50,63 | +| `tests/kit/hook-audit-hosts.test.mjs` | 21:CF→138,139,164 | +| `tests/kit/hook-audit.test.mjs` | 20:CF→61,62,90 | +| `tests/kit/hook-auto-memory-retirement.test.mjs` | 123:CF→176,177,192 | +| `tests/kit/hook-legacy-retirement.test.mjs` | 84:CF→125,126,157 | +| `tests/kit/hook-remediation-cli.test.mjs` | 27:F→83; 88:F→141 | +| `tests/kit/hook-remediation.test.mjs` | 30:CF→68,69,98; 104:F→164; 169:F→204; 240:F→274; 279:F→305; 432:F→456; 481:F→506; 531:F→537; 542:F→553 | +| `tests/kit/hook-upstream.test.mjs` | 17:F→25 | +| `tests/kit/host-adapters-cli.test.mjs` | 77:H→helper; 754:H→helper | +| `tests/kit/host-alignment.test.mjs` | 7:M→8; 9:M→10; 11:M→12 | +| `tests/kit/host-cli-migration.test.mjs` | 13:H→helper; 14:H→helper | +| `tests/kit/host-dry-run.test.mjs` | 38:R→39; 41:R→42 | +| `tests/kit/host-executable.test.mjs` | 9:H→helper | +| `tests/kit/host-health-connected.test.mjs` | 202:R→203 | +| `tests/kit/host-health-evidence.test.mjs` | 9:R→10; 29:R→30; 39:R→40 | +| `tests/kit/host-readiness-local.test.mjs` | 9:R→12 | +| `tests/kit/host-setup-evidence.test.mjs` | 28:H→helper; 44:F→52 | +| `tests/kit/hosts.test.mjs` | 152:H→helper | +| `tests/kit/install-edits.test.mjs` | 21:R→22 | +| `tests/kit/integration-command-facts.test.mjs` | 9:M→10 | +| `tests/kit/intel-history.test.mjs` | 21:H→helper | +| `tests/kit/intelligence-picker-groups.test.mjs` | 51:R→52; 67:R→68 | +| `tests/kit/intelligence-table-groups.test.mjs` | 10:R→11 | +| `tests/kit/intelligence-watch.test.mjs` | 8:H→helper | +| `tests/kit/language-coverage.test.mjs` | 10:R→10 | +| `tests/kit/live-check-evidence.test.mjs` | 15:M→260; 25:M→260 | +| `tests/kit/live-checks.test.mjs` | 21:M→688; 30:M→688; 401:F→415; 421:R→422; 440:F→449; 468:R→469; 701:R→702; 733:R→734 | +| `tests/kit/live-core.test.mjs` | 149:H→helper; 178:H→helper; 202:H→helper; 226:H→helper; 248:H→helper; 263:H→helper; 274:H→helper; 275:H→helper; 288:H→helper; 299:H→helper; 309:H→helper | +| `tests/kit/live-folder-correlator.test.mjs` | 14:R→15 | +| `tests/kit/live-process-sessions.test.mjs` | 250:F→293; 299:F→314 | +| `tests/kit/live-qe-contract.test.mjs` | 40:H→helper | +| `tests/kit/live-service.test.mjs` | 9:H→helper | +| `tests/kit/live-tailer.test.mjs` | 9:H→helper; 150:H→helper | +| `tests/kit/live-transcript.test.mjs` | 15:H→helper | +| `tests/kit/maintenance-action-service.test.mjs` | 25:R→26 | +| `tests/kit/maintenance-cli.test.mjs` | 15:H→helper | +| `tests/kit/maintenance-dashboard-api.test.mjs` | 600:R→601 | +| `tests/kit/maintenance-dashboard-e2e.test.mjs` | 177:R→179 | +| `tests/kit/maintenance-discovery-checkpoint.test.mjs` | 14:R→15 | +| `tests/kit/maintenance-discovery-configuration.test.mjs` | 21:R→22 | +| `tests/kit/maintenance-discovery-coverage.test.mjs` | 15:R→16 | +| `tests/kit/maintenance-discovery-orchestrator.test.mjs` | 23:R→24; 649:R→650 | +| `tests/kit/maintenance-discovery-preview.test.mjs` | 17:R→18 | +| `tests/kit/maintenance-git-project-patch.test.mjs` | 26:R→27; 41:R→42; 178:R→179 | +| `tests/kit/maintenance-host-alignment.test.mjs` | 6:M→7; 8:M→9 | +| `tests/kit/maintenance-interruption-audit.test.mjs` | 21:R→22 | +| `tests/kit/maintenance-management-activity.test.mjs` | 15:H→helper; 149:H→helper | +| `tests/kit/maintenance-management-procedures.test.mjs` | 21:R→22 | +| `tests/kit/maintenance-management-service.test.mjs` | 42:R→43 | +| `tests/kit/maintenance-native-findings.test.mjs` | 24:R→25 | +| `tests/kit/maintenance-one-action.test.mjs` | 20:R→21 | +| `tests/kit/maintenance-owned-providers.test.mjs` | 25:R→26 | +| `tests/kit/maintenance-persistence-support.test.mjs` | 22:R→23 | +| `tests/kit/maintenance-project-kind.test.mjs` | 13:R→14 | +| `tests/kit/maintenance-read-model.test.mjs` | 87:R→88; 104:R→105; 144:R→145; 187:R→188; 205:R→206; 236:R→237; 251:R→252; 269:R→270 | +| `tests/kit/maintenance-recovery.test.mjs` | 20:R→21 | +| `tests/kit/maintenance-transaction.test.mjs` | 19:R→20 | +| `tests/kit/mcp-scopes.test.mjs` | 21:R→26 | +| `tests/kit/mcp-tool-call.test.mjs` | 64:R→65 | +| `tests/kit/memory-maintenance.test.mjs` | 17:R→18 | +| `tests/kit/memory-probe-cleanup.test.mjs` | 16:M→17,186; 59:R→60 | +| `tests/kit/model-dashboard-read-model.test.mjs` | 553:F→604; 609:F→645 | +| `tests/kit/model-inventory-store.test.mjs` | 27:CS→38,60,73 | +| `tests/kit/natives-probe.test.mjs` | 16:CF→31,40,52 | +| `tests/kit/natives-runtime.test.mjs` | 23:H→helper; 26:CM→39,47,57; 74:S→82 | +| `tests/kit/natives.test.mjs` | 13:CS→29,41,47; 33:S→35; 53:S→60; 68:S→79; 88:S→95; 99:S→106; 110:S→113; 117:CF→128,130,143 | +| `tests/kit/node-runtime.test.mjs` | 23:H→helper | +| `tests/kit/npx.test.mjs` | 33:CS→40,47,54; 109:S→116 | +| `tests/kit/nudge.test.mjs` | 15:H→helper | +| `tests/kit/opencode-agents-stale-reason.test.mjs` | 25:M→39; 35:M→39 | +| `tests/kit/opencode-aqe-embedding.test.mjs` | 9:R→10 | +| `tests/kit/opencode-ruflo-gateway.test.mjs` | 8:CF→9,237,238 | +| `tests/kit/opencode-state-hermeticity.test.mjs` | 27:F→32; 43:R→44 | +| `tests/kit/opencode-stock-ruflo-gateway.test.mjs` | 33:CH→280,289,297 | +| `tests/kit/opencode-version-drift.test.mjs` | 14:M→56 | +| `tests/kit/opencode.test.mjs` | 20:M→21; 23:P→20,21,23 | +| `tests/kit/output-progress.test.mjs` | 119:H→helper | +| `tests/kit/owned-env-backup-prune.test.mjs` | 13:M→14 | +| `tests/kit/owned-env-projection.test.mjs` | 11:R→12; 131:R→132 | +| `tests/kit/paths-global-root-evidence.test.mjs` | 37:H→helper; 156:H→helper; 159:F→198 | +| `tests/kit/paths-global-root.test.mjs` | 94:F→106; 111:F→117 | +| `tests/kit/project-census.test.mjs` | 27:H→helper | +| `tests/kit/project-guidance.test.mjs` | 20:R→21 | +| `tests/kit/project-isolation.test.mjs` | 27:R→28 | +| `tests/kit/project-memory-status.test.mjs` | 11:R→12; 37:R→38; 83:R→92; 118:R→119; 139:R→140; 156:R→157; 170:R→171; 195:R→196; 211:R→212; 235:R→236; 328:R→329 | +| `tests/kit/project-memory.test.mjs` | 11:M→51,65,79; 82:R→83; 98:R→99; 107:R→108; 120:R→121; 133:R→134; 143:R→144; 160:R→168; 190:R→191; 201:R→202; 222:R→223; 269:R→270; 283:R→284 | +| `tests/kit/project-sources-imports.test.mjs` | 21:R→22 | +| `tests/kit/prompts-mainline-boundary.test.mjs` | 14:F→22 | +| `tests/kit/provider-cli.test.mjs` | 51:CS→86,103,109; 62:CS→86,103,109; 193:CF→297,335,336; 215:CF→297,335,336 | +| `tests/kit/provider-credentials.test.mjs` | 24:M→25; 33:P→24,25,33; 40:S→42 | +| `tests/kit/provider-refresh-cli.test.mjs` | 28:CS→48,84,89; 43:CS→48,84,89 | +| `tests/kit/provider-teardown-preservation.test.mjs` | 9:R→11 | +| `tests/kit/providers-drift-parity.test.mjs` | 22:M→113; 62:R→63; 96:R→97 | +| `tests/kit/providers-external.test.mjs` | 22:G→16,18,23 | +| `tests/kit/providers.test.mjs` | 170:CS→175,193,210; 178:S→180; 371:S→378; 437:F→466; 475:S→483 | +| `tests/kit/qeCourt.test.mjs` | 214:S→216; 220:S→226; 230:F→253; 257:F→279; 283:R→284 | +| `tests/kit/quota-codex-presence.test.mjs` | 18:M→25 | +| `tests/kit/quota.test.mjs` | 17:H→helper | +| `tests/kit/real-state-tripwire.test.mjs` | 16:R→17 | +| `tests/kit/reference-command.test.mjs` | 11:M→64; 19:F→60 | +| `tests/kit/refresh.test.mjs` | 17:M→19; 18:M→19 | +| `tests/kit/reverse-bridge.test.mjs` | 10:H→helper; 67:H→helper | +| `tests/kit/routing-config.test.mjs` | 147:S→176; 180:S→189; 193:S→211 | +| `tests/kit/routing-projection.test.mjs` | 13:CS→17,49,50; 21:CS→17,49,50 | +| `tests/kit/routing-retirement-convergence.test.mjs` | 29:M→30; 109:R→110,124; 128:R→129,154; 165:R→166,185 | +| `tests/kit/ruflo-components-apply.test.mjs` | 132:R→133; 164:R→165; 189:R→190; 211:R→212; 234:R→235; 245:R→246; 262:R→263; 276:R→277 | +| `tests/kit/ruflo-components-catalogue.test.mjs` | 81:R→82 | +| `tests/kit/ruflo-components-convergence.test.mjs` | 12:M→13; 42:R→43; 197:R→198; 213:R→214 | +| `tests/kit/ruflo-components-env.test.mjs` | 15:R→16 | +| `tests/kit/ruflo-components-evidence-location.test.mjs` | 6:M→18 | +| `tests/kit/ruflo-components-evidence.test.mjs` | 67:R→68; 188:R→189 | +| `tests/kit/ruflo-components-git-exclude.test.mjs` | 19:H→helper; 33:R→34; 100:R→101 | +| `tests/kit/ruflo-components-hosts.test.mjs` | 19:R→20; 45:R→46; 142:R→143; 161:F→170 | +| `tests/kit/ruflo-components-snapshot.test.mjs` | 194:F→210 | +| `tests/kit/ruflo-daemon-config.test.mjs` | 19:R→20 | +| `tests/kit/ruflo-mcp-launcher.test.mjs` | 21:R→22; 83:R→84 | +| `tests/kit/ruflo-memory-location.test.mjs` | 19:R→20; 50:R→51 | +| `tests/kit/ruflo-memory-root-pin.test.mjs` | 24:R→25 | +| `tests/kit/ruflo-memory.test.mjs` | 11:F→31; 39:R→40 | +| `tests/kit/run-tests-runner.test.mjs` | 15:R→16; 108:XL→16,110 | +| `tests/kit/ruvector.test.mjs` | 18:M→214; 25:M→214 | +| `tests/kit/ruvnet-brain-plugin.test.mjs` | 10:R→11 | +| `tests/kit/ruvnet-brain.test.mjs` | 87:H→helper; 118:H→helper; 181:H→helper; 196:H→helper; 213:H→helper; 222:H→helper; 238:H→helper; 257:H→helper | +| `tests/kit/rvf.test.mjs` | 22:CS→49,56,64 | +| `tests/kit/scaffold.test.mjs` | 19:CF→34,42,52; 25:CF→34,42,52; 85:F→91; 143:F→157 | +| `tests/kit/security-status.test.mjs` | 12:M→13 | +| `tests/kit/settings-config.test.mjs` | 12:S→19; 29:S→34; 38:S→45; 49:S→57; 61:S→67; 71:S→77; 81:S→99; 103:S→119; 130:S→141; 146:S→153; 178:S→188; 192:S→197; 201:S→209; 213:S→224; 228:S→234 | +| `tests/kit/setup-command.test.mjs` | 19:M→104,775; 122:S→136; 191:S→197; 208:S→220; 225:S→239; 474:F→484; 489:F→506; 512:F→541; 546:F→588; 593:S→601; 606:S→622; 607:P→775; 627:P→775; 783:S→798; 803:S→811 | +| `tests/kit/setup-host-flags.test.mjs` | 103:S→107 | +| `tests/kit/setup-memory-probe.test.mjs` | 26:R→27; 83:R→84 | +| `tests/kit/spawn-env-guard.test.mjs` | 176:R→177 | +| `tests/kit/sqlite.test.mjs` | 18:S→27; 31:S→42; 46:S→60 | +| `tests/kit/status-agent-browser.test.mjs` | 13:R→17 | +| `tests/kit/status-aqe-drift.test.mjs` | 17:M→18; 58:R→59; 71:R→72; 83:R→84; 103:H→helper; 127:R→128; 145:R→146; 160:R→161; 171:R→172 | +| `tests/kit/status-command.test.mjs` | 19:M→1451; 29:M→386,536,671; 340:F→387 | +| `tests/kit/status-golden.test.mjs` | 21:M→22; 29:M→30 | +| `tests/kit/status-live.test.mjs` | 17:M→267; 31:M→144,267; 125:F→144 | +| `tests/kit/status-manual-fixes.test.mjs` | 17:M→28; 27:M→28 | +| `tests/kit/status-repair-contract.test.mjs` | 17:M→18; 30:M→31,146 | +| `tests/kit/status-setup-hints.test.mjs` | 14:M→38,62; 22:M→62; 23:P→14,23,62 | +| `tests/kit/status-version-drift-refresh.test.mjs` | 27:M→40; 37:M→40; 47:P→40,47 | +| `tests/kit/status-viability.test.mjs` | 24:M→347; 35:M→347 | +| `tests/kit/status-zero-spawn.test.mjs` | 41:F→54; 43:F→55 | +| `tests/kit/statusline-config-dir-parity.test.mjs` | 42:H→helper | +| `tests/kit/statusline-version.test.mjs` | 20:M→212; 62:P→62,212 | +| `tests/kit/statusline.test.mjs` | 22:M→23; 44:H→helper | +| `tests/kit/sync-command.test.mjs` | 20:M→35,1320; 31:M→1320 | +| `tests/kit/sync-daemon-repair.test.mjs` | 16:M→17; 27:CF→46,61,76 | +| `tests/kit/sync-dry-run-preview.test.mjs` | 20:M→42; 33:M→42; 183:P→40,42,339; 184:P→40,42,339; 254:P→40,42,339; 325:P→40,42,339; 339:P→40,42,339 | +| `tests/kit/sync-host-repair.test.mjs` | 5:M→6 | +| `tests/kit/sync-needs-your-action.test.mjs` | 25:M→26; 35:M→36 | +| `tests/kit/sync-self-freshness.test.mjs` | 10:M→11 | +| `tests/kit/sync-skip-versions.test.mjs` | 22:M→43; 34:M→43; 107:P→41,43,107 | +| `tests/kit/system-command.test.mjs` | 21:H→helper; 22:H→helper | +| `tests/kit/system-summary.test.mjs` | 644:H→helper; 666:H→helper | +| `tests/kit/telemetry-cli.test.mjs` | 15:R→16; 19:H→helper | +| `tests/kit/telemetry-source-bounds.test.mjs` | 19:R→20 | +| `tests/kit/temp-dir-helper.test.mjs` | 10:H→helper | +| `tests/kit/uninstall-command.test.mjs` | 18:M→493; 170:S→187; 192:S→203; 449:CS→448,452,474; 458:S→474; 501:S→519; 507:S→519 | +| `tests/kit/upstream-watch-fixtures.mjs` | 31:F→48 | +| `tests/kit/upstream-watch-ledger-branch.test.mjs` | 128:F→167 | +| `tests/kit/upstream-watch-registry.test.mjs` | 30:F→38 | +| `tests/kit/upstream-watch-script.test.mjs` | 878:F→904 | +| `tests/kit/usage-audit-211.test.mjs` | 15:R→16 | +| `tests/kit/usage-claude-dedup.test.mjs` | 193:H→helper | +| `tests/kit/usage-cli.test.mjs` | 16:H→helper | +| `tests/kit/usage-codex-large-rollout.test.mjs` | 22:H→helper | +| `tests/kit/usage-deps-contract.test.mjs` | 42:H→helper | +| `tests/kit/usage-git-projects.test.mjs` | 37:R→38 | +| `tests/kit/usage-index-claude-window.test.mjs` | 31:CM→70,81,138 | +| `tests/kit/usage-index-opencode.test.mjs` | 15:CM→118,130,268 | +| `tests/kit/usage-index-v6.test.mjs` | 87:H→helper | +| `tests/kit/usage-index.test.mjs` | 33:H→helper; 66:H→helper; 929:H→helper; 970:H→helper | +| `tests/kit/usage-local-pricing.test.mjs` | 20:CF→126,133,173 | +| `tests/kit/usage-opencode.test.mjs` | 14:CM→95,137,325 | +| `tests/kit/usage-openrouter.test.mjs` | 13:CS→137,192,211 | +| `tests/kit/usage-project-groups.test.mjs` | 81:R→82 | +| `tests/kit/usage-truncation.test.mjs` | 43:H→helper | +| `tests/kit/verify-memory-routes.test.mjs` | 91:M→92; 96:M→97; 106:H→helper; 157:H→helper; 217:R→219; 218:P→217,218,219 | +| `tests/kit/version-lookup-record.test.mjs` | 22:M→41 | +| `tests/kit/versions.test.mjs` | 112:F→126 | +| `tests/kit/working-context.test.mjs` | 9:R→10 | +| `tests/live/aqe-codex-guidance-conformance.test.mjs` | 84:R→85 | +| `tests/live/aqe-external-provider-transport.test.mjs` | 258:R→280 | +| `tests/live/aqe-stop-hook-conformance.test.mjs` | 44:R→45 | +| `tests/live/codex-context-contract.test.mjs` | 16:R→17 | +| `tests/live/disposable-memory-project.mjs` | 49:C→41,43,57 | +| `tests/live/ruflo-memory-routing.test.mjs` | 26:M→28 | +| `tests/statusline-brain.test.cjs` | 18:E→helper; 41:PE→18 | +| `tests/statusline-segments.test.cjs` | 19:E→helper; 64:PE→19; 346:PE→19; 380:PE→19 | +| `tests/statusline-window-ledger.test.cjs` | 21:E→helper; 45:F→54; 176:PE→21; 186:PE→21 | +| `tests/ui/dashboard-ui.mjs` | 68:E→helper; 162:PE→68; 1102:PE→68; 1103:PE→68 | +| `tests/ui/helpers/launch-chrome.mjs` | 20:C→21,29,31 | + ## Decisions for Tasks 11–13 1. Implement one proveAbandoned interface with **abandoned=false, reason=cannot prove complete descendant exit** by default on macOS/Linux/Windows. Unit fixtures may exercise collector plumbing but cannot enable a production deletion path or constitute native platform proof. @@ -124,4 +442,4 @@ Task 13 must take a fresh snapshot after the last focused run. Direct children o 3. Retain B9-R4 path rules for own-root removal; refuse real home/filesystem-root temp bases before allocation, check absolute canonical direct parent, exact suite basename, lstat/non-symlink, POSIX owner. Unknown/error means refuse. List sibling roots without deleting them; do not change command/tripwire/leftover exit precedence (B9-R2). 4. Preserve synchronous runner and Ctrl-C/tool-kill behavior. Owner files are ignored in own leftovers. Interrupted runners remove nothing; completed runs retain current own-root semantics under B9-R1. Own-root cleanup is not a descendant-exit proof; the Windows lifetime regression must address unfinished children explicitly. 5. Task 12's list-only exit proof keeps a killed runner's root while its child lives **and after that child exits**. Do not execute the archived example's unconditional run-3 removal on a list-only platform. This follows B9-R5; controller was notified before dependent implementation. -6. B9-R1–R8 need no safety-rule relaxation. R7 has a factual clarification: global setup is unavailable in installed 22.22.3; config/import still require explicit opt-in. Task 13 remains report-only. Unresolved manual census and native Windows diagnostics stay visible; no claim of exhaustive leak causes or automatic backlog cleanup is justified. +6. B9-R1–R8 need no safety-rule relaxation. R7 has a factual clarification: global setup is unavailable in installed 22.22.3; config/import still require explicit opt-in. Task 13 remains report-only. Native Windows diagnostics and historical-cause limitations stay visible; no claim of exhaustive leak causes or automatic backlog cleanup is justified. From 17833652f10314f3594b8023400b00241d43ba1c Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Mon, 28 Sep 2026 23:50:46 -0700 Subject: [PATCH 04/20] docs(research): account for parent cleanup in credential census --- docs/plans/2026-09-28-test-temp-folder-cleanup-design.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/plans/2026-09-28-test-temp-folder-cleanup-design.md b/docs/plans/2026-09-28-test-temp-folder-cleanup-design.md index 3f3c656e..58e12eee 100644 --- a/docs/plans/2026-09-28-test-temp-folder-cleanup-design.md +++ b/docs/plans/2026-09-28-test-temp-folder-cleanup-design.md @@ -1,6 +1,6 @@ # Test temp folder cleanup design -## Status and decision +## Status Research snapshot: 2026-09-28, `e2f9dcae0554ff63921df618a819fd5e6afe80d2`, macOS Darwin 27.0.0, Node 26.4.0; Node 22.22.3 used for CLI checks. Proposed behavior is **list-only for abandoned sibling roots on macOS, Linux and Windows**. No candidate establishes complete descendant liveness. This uses B9-R5's explicit fallback, preserves B9-R1–R8, and introduces no native sweeper or deletion authority. @@ -137,7 +137,7 @@ This is a source lifecycle classification, not a guarantee that cleanup runs aft | R | 168 | Local test/file hook registration; cleanup line shown. No preceding direct assert/assertSandboxed call in the allocating scope was found. | | M | 87 | Module allocation with file hook; early imports/assertions can precede registration. | | F | 113 | Removal in finally; setup before entering try remains exposed. | -| S | 68 | Direct cleanup reached only on normal execution, often after assertions. | +| S | 67 | Direct cleanup reached only on normal execution, often after assertions. | | CF | 20 | Factory returns root; callers remove in finally; pre-return setup remains exposed. | | CH | 1 | Factory returns root; caller registers a hook; pre-registration setup remains exposed. | | CM | 5 | Factory callers have mixed success-only/finally/hook lifecycles; exact examples in JSON. | @@ -147,7 +147,7 @@ This is a source lifecycle classification, not a guarantee that cleanup runs aft | XS | 1 | Executable child template uses success-path cleanup. | | XL | 1 | Executable child template deliberately leaks to exercise runner detection. | | PE | 10 | Allocation covered transitively by enclosing private temp exit handler. | -| P | 15 | Allocation covered transitively by parent folder cleanup hook. | +| P | 16 | Allocation covered transitively by parent folder cleanup hook. | | G | 2 | Root added to collection consumed by already registered/file cleanup hook. | | C | 3 | Returns a cleanup method; construction failure handling and caller obligations described in JSON. | @@ -330,7 +330,7 @@ Source index below uses `allocation line:code→cleanup/caller evidence line` wi | `tests/kit/project-sources-imports.test.mjs` | 21:R→22 | | `tests/kit/prompts-mainline-boundary.test.mjs` | 14:F→22 | | `tests/kit/provider-cli.test.mjs` | 51:CS→86,103,109; 62:CS→86,103,109; 193:CF→297,335,336; 215:CF→297,335,336 | -| `tests/kit/provider-credentials.test.mjs` | 24:M→25; 33:P→24,25,33; 40:S→42 | +| `tests/kit/provider-credentials.test.mjs` | 24:M→25; 33:P→24,25,33; 40:P→24,25,42 (local S→42) | | `tests/kit/provider-refresh-cli.test.mjs` | 28:CS→48,84,89; 43:CS→48,84,89 | | `tests/kit/provider-teardown-preservation.test.mjs` | 9:R→11 | | `tests/kit/providers-drift-parity.test.mjs` | 22:M→113; 62:R→63; 96:R→97 | From 3cb5a1e26c96e3465fd86f0e4fdf3f83a3dcc5ca Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Mon, 28 Sep 2026 23:55:28 -0700 Subject: [PATCH 05/20] test(about): render the Ruflo install-edit line on the About card --- tests/kit/about-install-edit-render.test.mjs | 47 ++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 tests/kit/about-install-edit-render.test.mjs diff --git a/tests/kit/about-install-edit-render.test.mjs b/tests/kit/about-install-edit-render.test.mjs new file mode 100644 index 00000000..3af9b5c6 --- /dev/null +++ b/tests/kit/about-install-edit-render.test.mjs @@ -0,0 +1,47 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import vm from 'node:vm'; +import { RANK, esc } from '../../src/lib/dashboard/groups.mjs'; +import { RUFLO_PIN_NOTE } from '../../src/lib/install-edits.mjs'; +import { installEditRows } from '../../src/commands/status/sections/natives.mjs'; + +const ABOUT_SOURCE = fs.readFileSync(new URL('../../src/lib/dashboard/client/about.mjs', import.meta.url), 'utf8'); +const context = vm.createContext({ RANK, esc, sourceHostIcon: () => '', aboutHostChip: () => null }); +vm.runInContext(ABOUT_SOURCE.replace(/^import .*;$/gm, '').replace(/\bexport /g, ''), context); + +function renderCard(id, rows) { + context.__entry = { id, name: id, category: 'engine-memory', tagline: '', paragraph: '', icon: { ref: 'R' } }; + context.__rows = rows; + return vm.runInContext('aboutCard(__entry, { rows: __rows })', context); +} + +test('About renders one escaped Ruflo install-edit line from the real natives row', () => { + const rufloRoot = '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/test/ruflo'; + const rows = installEditRows([{ + state: 'applied', + file: `${rufloRoot}/node_modules/@claude-flow/cli/package.json`, + section: 'optionalDependencies', + name: 'better-sqlite3', + from: '', + to: '^12.10.0', + }], { rufloRoot }); + assert.equal(rows.length, 1); + assert.match(rows[0].message, /^ak applied Ruflo's native SQLite pin \(ruvnet\/ruflo#2219\)/); + + const rufloCard = renderCard('ruflo', rows); + const editLines = rufloCard.match(/
[^<]*<\/div>/g) || []; + assert.deepEqual(editLines, [`
${esc(rows[0].message)}
`]); + assert.ok(editLines[0].startsWith('
ak applied Ruflo's native SQLite pin (ruvnet/ruflo#2219)')); + assert.doesNotMatch(rufloCard, //); + assert.doesNotMatch(renderCard('agentdb', rows), /
/); + assert.doesNotMatch(renderCard('ruflo', []), /
/); +}); + +test('About edit-line matcher accepts the status row wording contract', () => { + const editLineSource = ABOUT_SOURCE.split('function aboutEditLine(')[1]?.split('function aboutCard(')[0]; + assert.ok(editLineSource, 'the shipped About edit-line function exists'); + const pattern = editLineSource.match(/\/\^([^/]+)\/\.test\(String\(er\.message/); + assert.ok(pattern, 'the shipped About edit-line matcher exists'); + assert.match(RUFLO_PIN_NOTE, new RegExp(`^${pattern[1]}`)); +}); From 705bd5ff51c431a36e7b2b1e22a5c699c4315fa9 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Tue, 29 Sep 2026 00:04:49 -0700 Subject: [PATCH 06/20] test(tripwire): list the live Ruflo session's .claude-flow folder and proven-config files as concurrent writers --- scripts/real-state-tripwire.mjs | 3 ++- tests/kit/real-state-tripwire.test.mjs | 37 ++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/scripts/real-state-tripwire.mjs b/scripts/real-state-tripwire.mjs index e74198d2..09b23794 100644 --- a/scripts/real-state-tripwire.mjs +++ b/scripts/real-state-tripwire.mjs @@ -34,7 +34,8 @@ export const CONCURRENT_WRITERS = [ { kind: 'state', pattern: /^statusline-debug\.log$/, writer: 'statusline debug log (src/templates/statusline-footer.cjs:2-15)' }, { kind: 'repo', pattern: /^\.swarm(\/|$)/, writer: 'Ruflo hooks and daemon of a live session' }, { kind: 'repo', pattern: /^\.agentic-qe\/(?!llm-config\.json)/, writer: 'AQE hooks of a live session' }, - { kind: 'repo', pattern: /^\.claude-flow\/(?!config\.json$)/, writer: 'Ruflo hooks and statusline caches of a live session' }, + { kind: 'repo', pattern: /^\.claude-flow(?:\/(?!config\.json$)|$)/, writer: 'Ruflo hooks and statusline caches of a live session' }, + { kind: 'repo', pattern: /^\.claude\/(?:proven-config\.json|\.proven-config-version)$/, writer: 'Ruflo proven-config adoption on CLI startup (@claude-flow/cli 3.48.0 dist/src/config/proven-config-refresh.js:24,30,94,120-125; dist/src/index.js:173-175)' }, { kind: 'user-file', pattern: /^\.claude\.json$/, writer: 'Claude Code session state in ~/.claude.json (https://code.claude.com/docs/en/settings)' }, ]; diff --git a/tests/kit/real-state-tripwire.test.mjs b/tests/kit/real-state-tripwire.test.mjs index ef911ec8..2a90383c 100644 --- a/tests/kit/real-state-tripwire.test.mjs +++ b/tests/kit/real-state-tripwire.test.mjs @@ -144,6 +144,43 @@ test('developer mode moves live-session writers to "concurrent"; strict mode fai assert.match(formatReport(dev), /concurrent writers \(not failing\)/); }); +test('Ruflo-created .claude-flow root and proven-config files are concurrent only in developer mode', (t) => { + const home = tmp(t, 'ak-trip-ruflo'); + const repo = path.join(home, 'repo'); + fs.mkdirSync(path.join(repo, '.claude'), { recursive: true }); + const roots = realStateRoots({ platform: process.platform, homedir: home, repoRoot: repo, env: {} }); + const before = snapshotRoots(roots); + fs.mkdirSync(path.join(repo, '.claude-flow')); + fs.writeFileSync(path.join(repo, '.claude-flow', 'session.json'), '{}'); + fs.writeFileSync(path.join(repo, '.claude', 'proven-config.json'), '{}'); + fs.writeFileSync(path.join(repo, '.claude', '.proven-config-version'), 'v1'); + fs.writeFileSync(path.join(repo, '.claude-flow', 'config.json'), '{}'); + const after = snapshotRoots(roots); + const dev = compareSnapshots(before, after, { strict: false }); + assert.deepEqual(dev.concurrent.map((c) => c.rel).sort(), [ + '.claude-flow/', '.claude-flow/session.json', '.claude/.proven-config-version', '.claude/proven-config.json', + ].sort()); + assert.deepEqual(dev.failing.map((c) => c.rel), ['.claude-flow/config.json']); + const strict = compareSnapshots(before, after, { strict: true }); + assert.deepEqual(strict.concurrent, []); + assert.deepEqual(strict.failing.map((c) => c.rel).sort(), [ + '.claude-flow/', '.claude-flow/config.json', '.claude-flow/session.json', + '.claude/.proven-config-version', '.claude/proven-config.json', + ].sort()); +}); + +test('removing the .claude-flow root is concurrent only in developer mode', (t) => { + const home = tmp(t, 'ak-trip-ruflo-remove'); + const repo = path.join(home, 'repo'); + fs.mkdirSync(path.join(repo, '.claude-flow'), { recursive: true }); + const roots = realStateRoots({ platform: process.platform, homedir: home, repoRoot: repo, env: {} }); + const before = snapshotRoots(roots); + fs.rmSync(path.join(repo, '.claude-flow'), { recursive: true }); + const after = snapshotRoots(roots); + assert.deepEqual(compareSnapshots(before, after, { strict: false }).concurrent.map((c) => c.rel), ['.claude-flow/']); + assert.deepEqual(compareSnapshots(before, after, { strict: true }).failing.map((c) => c.rel), ['.claude-flow/']); +}); + test('CI and AK_TRIPWIRE_STRICT make the comparison strict', () => { assert.equal(isStrict({ CI: 'true' }), true); assert.equal(isStrict({ CI: '1' }), true); From dd663f6bec25a0e5478629f70f2ed7ff50a019f8 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Tue, 29 Sep 2026 00:30:16 -0700 Subject: [PATCH 07/20] fix(test-runner): guard owner roots and keep sibling cleanup list-only --- AGENTS.md | 11 +- docs/plans/2026-09-28-runner-hygiene.md | 2 +- scripts/run-roots.mjs | 170 +++++++++++++++++ scripts/run-tests.mjs | 31 +++- tests/kit/run-roots.test.mjs | 235 ++++++++++++++++++++++++ tests/kit/run-tests-runner.test.mjs | 79 +++++++- 6 files changed, 518 insertions(+), 10 deletions(-) create mode 100644 scripts/run-roots.mjs create mode 100644 tests/kit/run-roots.test.mjs diff --git a/AGENTS.md b/AGENTS.md index c0d3f693..cf08dacf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -332,8 +332,15 @@ them, and `sandboxHome()` and `redirectToolState()` do the same for in-process c Code's own `~/.claude.json`) are listed as "concurrent writers" and do not fail a local run; CI (or `AK_TRIPWIRE_STRICT=1`) fails on them too. Every command also runs with `TMPDIR`/`TEMP`/`TMP` pointed at a fresh `ak-suite-*` folder: anything left in it afterwards fails the run and is -listed, and the runner refuses to start when that folder sits inside a git repository (point -`TMPDIR` elsewhere). The runner also drops `FORCE_COLOR` (Claude Code shells set it), because +listed (excluding its private atomic `.ak-suite-owner.json` and Node compile cache). The runner +refuses home/filesystem-root temp bases before allocation and refuses roots inside a git +repository (point `TMPDIR` elsewhere). A completed run removes only its own validated direct, +canonical, nonsymlink, current-owner root. It then lists sibling suite roots: missing, invalid, +foreign or uncertain owner metadata means keep. Sibling handling is list-only on macOS, Linux +and Windows because no installed probe proves all descendants have exited; even a dead owner +is insufficient. Interrupted runs remove and collect nothing. Listing/removal errors preserve +the suite's exit code; removal errors may leave a partially removed own root. The runner also +drops `FORCE_COLOR` (Claude Code shells set it), because tests read plain text from pipes. Tests make temporary folders with `tempDir()` from `tests/kit/helpers/temp-dir.mjs`, and spawned children get their environment from `spawnEnv()` in `tests/kit/helpers/home-sandbox.mjs`. UI tests launch Chrome with `launchChrome()` from diff --git a/docs/plans/2026-09-28-runner-hygiene.md b/docs/plans/2026-09-28-runner-hygiene.md index 30731ae0..cbd8900d 100644 --- a/docs/plans/2026-09-28-runner-hygiene.md +++ b/docs/plans/2026-09-28-runner-hygiene.md @@ -2,7 +2,7 @@ ## Status -Research drafted against `develop` commit `e2f9dcae0554ff63921df618a819fd5e6afe80d2` on `test/runner-hygiene`. Only Task 10 research and this plan are authorized in this pass. No implementation, backlog removal, push, PR or merge has occurred. The creator census is complete: 647 sites classified, zero unresolved current lifecycles. Native Windows/Linux behavioral proof remains open. Default sibling handling is list-only on every platform. +Research drafted against `develop` commit `e2f9dcae0554ff63921df618a819fd5e6afe80d2` on `test/runner-hygiene`. Tasks 10/5/7 were independently accepted. Task 11 now implements private owner records, guarded own-root removal and list-only sibling handling; focused refusal/race/error tests and helper coverage are recorded in the ignored Task 11 report. No backlog removal, push, PR or merge has occurred. Task 12 focus implementation has not started. The creator census is complete: 647 sites classified, zero unresolved current lifecycles. Native Windows/Linux behavioral proof remains open. Default sibling handling is list-only on every platform. ## Contract and dependencies diff --git a/scripts/run-roots.mjs b/scripts/run-roots.mjs new file mode 100644 index 00000000..2bcbd358 --- /dev/null +++ b/scripts/run-roots.mjs @@ -0,0 +1,170 @@ +// Builtin-only attribution and conservative run-root handling. Native probes +// deliberately cannot authorize sibling deletion on any supported platform. +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { randomUUID } from 'node:crypto'; + +export const RUN_ROOT_NAME = /^ak-suite-[A-Za-z0-9]{6}$/; +export const OWNER_FILE = '.ak-suite-owner.json'; +export const IGNORED_IN_ROOT = new Set(['node-compile-cache', OWNER_FILE]); +const MAX_OWNER_BYTES = 8192; +const currentUid = () => process.getuid?.() ?? null; + +/** Attribution timestamp only: startedAt is NOT an observed OS process start. + * @param {{pid?:number, now?:number, hostname?:string, uid?:number|null, platform?:string}} [options] + */ +export function ownerRecord({ pid = process.pid, now = Date.now(), hostname = os.hostname(), + uid = currentUid(), platform = process.platform } = {}) { + return { schema: 1, runId: randomUUID(), pid, startedAt: now, hostname, uid, platform, proofMode: 'list-only' }; +} + +/** Private, exclusive staging file followed by atomic publication. */ +export function writeOwner(root, record) { + const canonical = fs.realpathSync(root); + if (canonical !== root || !fs.lstatSync(root).isDirectory()) throw Error('noncanonical run root'); + const bound = { ...record, root, tmpdir: path.dirname(root) }; + if (!validRecord(bound, root)) throw Error('invalid owner record'); + const staging = path.join(root, `${OWNER_FILE}.tmp`); + fs.writeFileSync(staging, JSON.stringify(bound), { flag: 'wx', mode: 0o600 }); + fs.renameSync(staging, path.join(root, OWNER_FILE)); +} + +function validRecord(r, root) { + return r !== null && typeof r === 'object' && !Array.isArray(r) + && r.schema === 1 && typeof r.runId === 'string' && /^[a-f0-9-]{36}$/.test(r.runId) + && Number.isSafeInteger(r.pid) && r.pid > 0 + && Number.isSafeInteger(r.startedAt) && r.startedAt > 0 + && r.hostname === os.hostname() && r.uid === currentUid() && r.platform === process.platform + && r.proofMode === 'list-only' && r.root === root && r.tmpdir === path.dirname(root); +} + +/** Bounded, no-follow metadata read; missing, changed or invalid means unknown. */ +export function readOwner(root) { + let fd; + let record = null; + try { + const file = path.join(root, OWNER_FILE); + const before = fs.lstatSync(file); + if (!before.isFile() || before.isSymbolicLink() || before.nlink !== 1 + || before.size > MAX_OWNER_BYTES || (currentUid() !== null && before.uid !== currentUid())) { + throw Error('unsafe owner file'); + } + // Bitwise flags treat an unavailable platform constant as zero. + fd = fs.openSync(file, fs.constants.O_RDONLY | fs.constants.O_NOFOLLOW | fs.constants.O_NONBLOCK); + const opened = fs.fstatSync(fd); + if (!opened.isFile() || !sameIdentity(before, opened) || opened.size > MAX_OWNER_BYTES) { + throw Error('owner file changed at open'); + } + const bytes = Buffer.alloc(MAX_OWNER_BYTES + 1); + const count = fs.readSync(fd, bytes, 0, bytes.length, 0); + if (count > MAX_OWNER_BYTES || !sameIdentity(opened, fs.lstatSync(file))) throw Error('owner file changed at read'); + const parsed = JSON.parse(bytes.subarray(0, count).toString('utf8')); + if (validRecord(parsed, root)) record = parsed; + } catch { /* Unknown metadata never grants ownership. */ } + finally { if (fd !== undefined) fs.closeSync(fd); } + return record; +} + +/** Pure path check also accepts Windows paths in cross-platform unit fixtures. */ +export function unsafeTempBase(tmpdir, homedir) { + const windows = path.win32.isAbsolute(tmpdir) && !path.posix.isAbsolute(tmpdir); + const flavor = windows ? path.win32 : path.posix; + const normalize = (p) => windows ? flavor.resolve(p).toLowerCase() : flavor.resolve(p); + const tmp = normalize(tmpdir); + if (tmp === normalize(flavor.parse(tmp).root)) return 'filesystem root'; + if (tmp === normalize(homedir)) return 'home directory'; + return null; +} + +/** @param {string} dir + * @param {{tmpdir:string, homedir:string, uid?:number|null, requireOwner?:boolean}} options + * @returns {{ok:boolean, reason?:string}} + */ +export function removableRunRoot(dir, { tmpdir, homedir, uid = currentUid(), requireOwner = true }) { + try { + if (!path.isAbsolute(dir) || path.resolve(dir) !== dir || !path.isAbsolute(tmpdir) + || fs.realpathSync(tmpdir) !== tmpdir) return { ok: false, reason: 'noncanonical absolute path required' }; + const unsafe = unsafeTempBase(tmpdir, fs.realpathSync(homedir)); + if (unsafe) return { ok: false, reason: `unsafe temp base: ${unsafe}` }; + if (!RUN_ROOT_NAME.test(path.basename(dir)) || path.dirname(dir) !== tmpdir) { + return { ok: false, reason: 'not an exact direct run-root child' }; + } + const stat = fs.lstatSync(dir); + if (!stat.isDirectory() || stat.isSymbolicLink() || fs.realpathSync(dir) !== dir) { + return { ok: false, reason: 'not a canonical nonsymlink directory' }; + } + if (uid !== currentUid() || (uid !== null && stat.uid !== uid)) return { ok: false, reason: 'foreign filesystem owner' }; + if (requireOwner && !readOwner(dir)) return { ok: false, reason: 'no valid owner record (missing, malformed or foreign)' }; + return { ok: true }; + } catch { return { ok: false, reason: 'path inspection failed' }; } +} + +/** Probes are injected code, never derived from metadata or CLI input. + * completeExit must prove ALL users/descendants gone, not a snapshot/handle scan. + * @typedef {{alive:(pid:number)=>boolean|null, startedAfter:(pid:number,ms:number)=>boolean|null, + * completeExit:(root:string,owner:object)=>boolean|null, listOnly?:boolean}} Probes + * @param {string} root + * @param {ReturnType} owner + * @param {Probes} probes + */ +export function proveAbandoned(root, owner, probes) { + try { + if (!validRecord(owner, root)) return { abandoned: false, reason: 'invalid owner metadata' }; + if (probes.listOnly) return { abandoned: false, reason: 'cannot prove complete descendant exit (list-only)' }; + const alive = probes.alive(owner.pid); + if (alive !== false && (alive !== true || probes.startedAfter(owner.pid, owner.startedAt) !== true)) { + return { abandoned: false, reason: 'owner alive or identity uncertain' }; + } + if (probes.completeExit(root, owner) !== true) return { abandoned: false, reason: 'descendant exit uncertain or live user' }; + return { abandoned: true, reason: 'injected complete exit proof' }; + } catch { return { abandoned: false, reason: 'probe failed; exit uncertain' }; } +} + +/** @returns {Probes} No process scans: none could authorize removal. */ +export function defaultProbes(_platform = process.platform) { + return { listOnly: true, alive: () => null, startedAfter: () => null, completeExit: () => null }; +} + +function sameIdentity(a, b) { return a.dev === b.dev && a.ino === b.ino && a.ctimeMs === b.ctimeMs; } + +/** Revalidate after injected probes; recursive rm can still fail partway through. + * The fixture seam is not an installed platform containment implementation. + * @param {{tmpdir:string, selfRoot?:string, homedir:string, uid?:number|null, probes?:Probes, + * log?:(s:string)=>void, remove?:(root:string)=>void}} options + */ +export function collectAbandonedRoots({ tmpdir, selfRoot, homedir, uid = currentUid(), + probes = defaultProbes(), log = console.error, remove = (root) => fs.rmSync(root, { recursive: true }) }) { + /** @type {{removed:string[], kept:Array<{path:string,reason:string}>}} */ + const result = { removed: [], kept: [] }; + const report = (message) => { try { log(message); } catch { /* Reporting cannot alter cleanup outcomes. */ } }; + const keep = (root, reason) => { result.kept.push({ path: root, reason }); report(`kept run root ${root}: ${reason}`); }; + let names; + try { names = fs.readdirSync(tmpdir); } + catch { keep(tmpdir, 'could not list run roots'); return result; } + for (const name of names) { + if (!RUN_ROOT_NAME.test(name)) continue; + const root = path.join(tmpdir, name); + if (root === selfRoot) continue; + const options = { tmpdir, homedir, uid, requireOwner: true }; + const safe = removableRunRoot(root, options); + if (!safe.ok) { keep(root, safe.reason); continue; } + try { + const identity = fs.lstatSync(root); + const owner = readOwner(root); + if (!owner) { keep(root, 'owner changed during inspection'); continue; } + const proof = proveAbandoned(root, owner, probes); + if (!proof.abandoned) { keep(root, proof.reason); continue; } + const boundary = removableRunRoot(root, options); + if (!boundary.ok || !sameIdentity(identity, fs.lstatSync(root)) + || JSON.stringify(owner) !== JSON.stringify(readOwner(root))) { + keep(root, 'root or owner changed before removal'); continue; + } + try { remove(root); } + catch (error) { keep(root, `removal failed; root may be partially removed: ${error.message}`); continue; } + result.removed.push(root); + report(`removed abandoned run root ${root}`); + } catch { keep(root, 'inspection changed or failed; removal not attempted'); } + } + return result; +} diff --git a/scripts/run-tests.mjs b/scripts/run-tests.mjs index d579b881..3d826b7a 100644 --- a/scripts/run-tests.mjs +++ b/scripts/run-tests.mjs @@ -9,6 +9,7 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; +import { ownerRecord, writeOwner, unsafeTempBase, removableRunRoot, collectAbandonedRoots, IGNORED_IN_ROOT } from './run-roots.mjs'; import { realStateRoots, snapshotRoots, compareSnapshots, isStrict, formatReport } from './real-state-tripwire.mjs'; const REPO = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); @@ -54,10 +55,27 @@ export function runGuarded(commands, { } = {}) { // Every command runs with this run's own templated temp root: leftovers are then // attributable to the run, and they fail it. - const tempRoot = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'ak-suite-'))); + const tmpdir = fs.realpathSync(os.tmpdir()); + const unsafe = unsafeTempBase(tmpdir, fs.realpathSync(homedir)); + if (unsafe) { log(`unsafe temp base ${tmpdir}: ${unsafe}`); return 2; } + const tempRoot = fs.mkdtempSync(path.join(tmpdir, 'ak-suite-')); + try { writeOwner(tempRoot, ownerRecord()); } + catch (error) { log(`could not record run owner; kept run root ${tempRoot}: ${error.message}`); return 2; } + const identity = fs.lstatSync(tempRoot); + const removeOwnRoot = () => { + const safe = removableRunRoot(tempRoot, { tmpdir, homedir, requireOwner: false }); + if (!safe.ok) { log(`kept own run root ${tempRoot}: ${safe.reason}`); return; } + try { + const current = fs.lstatSync(tempRoot); + if (current.dev !== identity.dev || current.ino !== identity.ino || current.birthtimeMs !== identity.birthtimeMs) { + log(`kept own run root ${tempRoot}: directory identity changed`); return; + } + fs.rmSync(tempRoot, { recursive: true, force: true, maxRetries: 3 }); + } catch (error) { log(`own run root removal failed; may be partially removed ${tempRoot}: ${error.message}`); } + }; const enclosing = enclosingRepository(tempRoot); if (enclosing) { - fs.rmSync(tempRoot, { recursive: true, force: true }); + removeOwnRoot(); log(`the suite temp root ${tempRoot} is inside the git repository ${enclosing}; tests that probe "outside a ` + 'git repository" would write into it. Point TMPDIR outside any repository.'); return 2; @@ -74,11 +92,16 @@ export function runGuarded(commands, { let code = 0; for (const args of commands) { const r = spawnSync(process.execPath, args, { cwd: repoRoot, env: childEnv, stdio: 'inherit' }); + if (r.signal) { log(`interrupted run; kept run root ${tempRoot}: ${r.signal}`); return 1; } if (r.error) { log(`could not run node ${args.join(' ')}: ${r.error.message}`); code = 1; break; } if (r.status !== 0) { code = r.status ?? 1; break; } } - const leftovers = fs.readdirSync(tempRoot).filter((name) => name !== 'node-compile-cache'); - fs.rmSync(tempRoot, { recursive: true, force: true, maxRetries: 3 }); + let leftovers = []; + try { leftovers = fs.readdirSync(tempRoot).filter((name) => !IGNORED_IN_ROOT.has(name)); } + catch (error) { log(`could not list own run root ${tempRoot}: ${error.message}`); } + removeOwnRoot(); + try { collectAbandonedRoots({ tmpdir, selfRoot: tempRoot, homedir, log }); } + catch (error) { log(`could not list sibling run roots: ${error.message}`); } if (leftovers.length) log(`temp folders left behind by the run (${leftovers.length}):\n ${leftovers.join('\n ')}`); const result = compareSnapshots(before, snapshotRoots(roots), { strict: isStrict(env) }); const report = formatReport(result); diff --git a/tests/kit/run-roots.test.mjs b/tests/kit/run-roots.test.mjs new file mode 100644 index 00000000..d52a80e2 --- /dev/null +++ b/tests/kit/run-roots.test.mjs @@ -0,0 +1,235 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { tempDir } from './helpers/temp-dir.mjs'; +import { ownerRecord, writeOwner, readOwner, unsafeTempBase, removableRunRoot, + proveAbandoned, collectAbandonedRoots, defaultProbes, OWNER_FILE } from '../../scripts/run-roots.mjs'; + +const uid = process.getuid?.() ?? null; +const complete = { alive: () => false, startedAfter: () => false, completeExit: () => true }; +function fixture(t) { + const tmpdir = tempDir('ak-root-fixture', t); + const root = fs.mkdtempSync(path.join(tmpdir, 'ak-suite-')); + const homedir = path.join(tmpdir, 'home'); + fs.mkdirSync(homedir); + writeOwner(root, ownerRecord()); + return { root, tmpdir, homedir, uid }; +} +function collect(f, probes = complete, extra = {}) { + return collectAbandonedRoots({ ...f, probes, log: () => {}, ...extra }); +} + +test('private atomic owner metadata round trips and does not leave staging data', (t) => { + const f = fixture(t); + const record = readOwner(f.root); + assert.equal(record.pid, process.pid); + assert.equal(record.root, f.root); + assert.equal(record.tmpdir, f.tmpdir); + assert.equal(record.proofMode, 'list-only'); + assert.deepEqual(fs.readdirSync(f.root), [OWNER_FILE]); + if (process.platform !== 'win32') assert.equal(fs.statSync(path.join(f.root, OWNER_FILE)).mode & 0o777, 0o600); +}); + +test('native defaults never prove abandonment, including dead owners and reused PIDs', (t) => { + const f = fixture(t); + for (const platform of ['darwin', 'linux', 'win32', 'other']) { + assert.equal(proveAbandoned(f.root, readOwner(f.root), defaultProbes(platform)).abandoned, false); + assert.deepEqual(collect(f, defaultProbes(platform)).removed, []); + } + for (const probes of [ { ...complete, alive: () => true }, { ...complete, alive: () => null }, + { ...complete, completeExit: () => null }, { ...complete, completeExit: () => false }, + { ...complete, alive: () => true, startedAfter: () => null }, + { ...complete, alive: () => { throw Error('uncertain'); } } ]) { + assert.equal(collect(f, probes).kept.length, 1); + assert.ok(fs.existsSync(f.root)); + } +}); + +test('only explicit complete fixture proof permits deletion; reused PID needs independent descendant proof', (t) => { + const f = fixture(t); + fs.writeFileSync(path.join(f.root, 'payload'), 'fixture'); + assert.deepEqual(collect(f, { ...complete, alive: () => true, startedAfter: () => true }).removed, [f.root]); + assert.equal(fs.existsSync(f.root), false); +}); + +test('missing, malformed, oversized, foreign and path-mismatched owners stay listed', (t) => { + const f = fixture(t); + const record = readOwner(f.root); + const file = path.join(f.root, OWNER_FILE); + const values = ['{', 'x'.repeat(8193), 'null', '[]', JSON.stringify({ ...record, schema: 2 }), + ...[{ pid: 0 }, { startedAt: -1 }, { hostname: 'foreign' }, { uid: 123456789 }, + { platform: 'foreign' }, { root: f.tmpdir }, { tmpdir: f.root }, { proofMode: 'delete' }, + { runId: '' }].map((patch) => JSON.stringify({ ...record, ...patch }))]; + fs.unlinkSync(file); + assert.equal(collect(f).kept.length, 1); + for (const value of values) { + fs.writeFileSync(file, value); + assert.equal(readOwner(f.root), null, value.slice(0, 100)); + assert.equal(collect(f).kept.length, 1); + assert.ok(fs.existsSync(f.root)); + } +}); + +test('symlink owner files and directory owners are never read', (t) => { + const f = fixture(t); + const file = path.join(f.root, OWNER_FILE); + fs.renameSync(file, path.join(f.tmpdir, 'record')); + fs.symlinkSync(path.join(f.tmpdir, 'record'), file); + assert.equal(readOwner(f.root), null); + assert.equal(collect(f).kept.length, 1); + fs.unlinkSync(file); fs.mkdirSync(file); + assert.equal(readOwner(f.root), null); +}); + +test('unsafe home and filesystem-root temp parents are refused for POSIX and Windows', () => { + for (const [tmp, home] of [['/', '/home/me'], ['/home/me', '/home/me'], ['C:\\', 'C:\\Users\\me'], + ['C:\\Users\\ME', 'c:\\users\\me'], ['\\\\server\\share\\', 'C:\\Users\\me']]) { + assert.ok(unsafeTempBase(tmp, home)); + } + assert.equal(unsafeTempBase('/tmp', '/home/me'), null); + assert.equal(unsafeTempBase('C:\\Temp', 'C:\\Users\\me'), null); +}); + +test('root guards reject path escapes, aliases, non-direct children, wrong owners and symlinks', (t) => { + const f = fixture(t); + assert.equal(removableRunRoot(f.root, { ...f, requireOwner: true }).ok, true); + for (const dir of ['relative', f.tmpdir, path.join(f.root, 'ak-suite-AAAAAA'), + `${f.tmpdir}/x/../${path.basename(f.root)}`]) { + assert.equal(removableRunRoot(dir, f).ok, false); + } + assert.equal(removableRunRoot(f.root, { ...f, tmpdir: f.homedir }).ok, false); + assert.equal(removableRunRoot(f.root, { ...f, homedir: f.tmpdir }).ok, false); + if (uid !== null) assert.equal(removableRunRoot(f.root, { ...f, uid: uid + 1 }).ok, false); + const alias = path.join(f.tmpdir, 'ak-suite-AAAAAA'); + fs.symlinkSync(f.root, alias, 'junction'); + fs.mkdirSync(path.join(f.tmpdir, 'ak-suite-not-exact')); + assert.equal(removableRunRoot(alias, f).ok, false); + const r = collect(f, complete, { selfRoot: f.root }); + assert.deepEqual(r.removed, []); + assert.equal(r.kept.length, 1); + assert.ok(fs.existsSync(f.root)); +}); + +test('revalidation refuses root replacement during the proof', (t) => { + const f = fixture(t); + const r = collect(f, { ...complete, completeExit: () => { + fs.renameSync(f.root, `${f.root}-saved`); + fs.mkdirSync(f.root); writeOwner(f.root, ownerRecord()); + return true; + } }); + assert.deepEqual(r.removed, []); + assert.match(r.kept[0].reason, /changed/); + assert.ok(fs.existsSync(f.root)); +}); + +test('removal errors disclose possible partial deletion and listing errors are reported', (t) => { + const f = fixture(t); + fs.writeFileSync(path.join(f.root, 'payload'), 'fixture'); + const r = collect(f, complete, { remove: (root) => { + fs.unlinkSync(path.join(root, 'payload')); throw Error('EBUSY'); + } }); + assert.deepEqual(r.removed, []); + assert.match(r.kept[0].reason, /partially removed.*EBUSY/); + assert.equal(fs.existsSync(path.join(f.root, 'payload')), false); + assert.equal(collect({ ...f, tmpdir: path.join(f.tmpdir, 'absent') }).kept.length, 1); +}); + +test('default probe functions explicitly report unknown without supplying authority', () => { + const probes = defaultProbes(); + assert.equal(probes.alive(process.pid), null); + assert.equal(probes.startedAfter(process.pid, Date.now()), null); + assert.equal(probes.completeExit('/unused', {}), null); +}); + +test('owner publication rejects invalid attribution and refuses existing staging links', (t) => { + const f = fixture(t); + assert.throws(() => writeOwner(`${f.root}/.`, ownerRecord()), /noncanonical/); + assert.throws(() => writeOwner(f.root, ownerRecord({ pid: 0 })), /invalid/); + const target = path.join(f.tmpdir, 'target'); + fs.writeFileSync(target, 'untouched'); + fs.symlinkSync(target, path.join(f.root, `${OWNER_FILE}.tmp`)); + assert.throws(() => writeOwner(f.root, ownerRecord()), /EEXIST/); + assert.equal(fs.readFileSync(target, 'utf8'), 'untouched'); +}); + +test('owner reader refuses oversized or replaced files at its descriptor boundary', (t) => { + const f = fixture(t); + const realFstat = fs.fstatSync; + const realRead = fs.readSync; + for (const patch of [{ size: 8193 }, { ino: -1 }, { isFile: () => false }]) { + const mock = t.mock.method(fs, 'fstatSync', (...args) => Object.assign(realFstat(...args), patch)); + assert.equal(readOwner(f.root), null); + mock.mock.restore(); + } + const mock = t.mock.method(fs, 'readSync', (...args) => { realRead(...args); return 8193; }); + assert.equal(readOwner(f.root), null); + mock.mock.restore(); + const file = path.join(f.root, OWNER_FILE); + const hardlink = path.join(f.tmpdir, 'hardlink'); + fs.linkSync(file, hardlink); + assert.equal(readOwner(f.root), null); +}); + +test('inspection races retain roots before any removal attempt', (t) => { + const f = fixture(t); + const original = fs.lstatSync; + let calls = 0; + const mock = t.mock.method(fs, 'lstatSync', (...args) => { + if (args[0] === f.root && ++calls === 2) throw Error('inspection denied'); + return original(...args); + }); + const r = collect(f); + assert.deepEqual(r.removed, []); + assert.match(r.kept[0].reason, /inspection.*failed/); + mock.mock.restore(); + const ownerFile = path.join(f.root, OWNER_FILE); + let reads = 0; + const mock2 = t.mock.method(fs, 'lstatSync', (...args) => { + if (args[0] === ownerFile && ++reads === 3) throw Error('owner vanished'); + return original(...args); + }); + assert.match(collect(f).kept[0].reason, /owner changed/); + mock2.mock.restore(); + assert.ok(fs.existsSync(f.root)); +}); + +test('default collection keeps valid roots without injected probes', (t) => { + const f = fixture(t); + const messages = []; + const r = collectAbandonedRoots({ ...f, log: (s) => messages.push(s) }); + assert.deepEqual(r.removed, []); + assert.match(messages[0], /list-only/); +}); + +test('missing roots and noncanonical parents fail closed', (t) => { + const f = fixture(t); + fs.rmSync(f.root, { recursive: true }); + assert.equal(removableRunRoot(f.root, f).ok, false); + assert.equal(removableRunRoot(f.root, { ...f, tmpdir: 'relative' }).ok, false); +}); + +test('platforms without numeric uid retain attributable roots by default', (t) => { + const original = Object.getOwnPropertyDescriptor(process, 'getuid'); + Object.defineProperty(process, 'getuid', { value: undefined, configurable: true }); + t.after(() => { if (original) Object.defineProperty(process, 'getuid', original); }); + const f = fixture(t); + assert.equal(readOwner(f.root).uid, null); + assert.deepEqual(collectAbandonedRoots({ ...f, uid: null, log: () => {} }).removed, []); +}); + +test('logging failure cannot turn a completed removal into an intact-preservation claim', (t) => { + const f = fixture(t); + assert.doesNotThrow(() => { + const result = collectAbandonedRoots({ ...f, probes: complete, log: () => { throw Error('closed pipe'); } }); + assert.deepEqual(result.removed, [f.root]); + assert.deepEqual(result.kept, []); + }); +}); + +test('direct abandonment proof refuses malformed metadata even with complete injected probes', (t) => { + const f = fixture(t); + for (const record of [null, {}, { ...readOwner(f.root), pid: -1 }]) { + assert.equal(proveAbandoned(f.root, record, complete).abandoned, false); + } +}); diff --git a/tests/kit/run-tests-runner.test.mjs b/tests/kit/run-tests-runner.test.mjs index fb74f826..c2d4a9b0 100644 --- a/tests/kit/run-tests-runner.test.mjs +++ b/tests/kit/run-tests-runner.test.mjs @@ -2,18 +2,17 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import fs from 'node:fs'; -import os from 'node:os'; import path from 'node:path'; import { spawnSync } from 'node:child_process'; import { fileURLToPath } from 'node:url'; +import { tempDir } from './helpers/temp-dir.mjs'; import { spawnEnv } from './helpers/home-sandbox.mjs'; const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..'); const RUNNER = path.join(ROOT, 'scripts', 'run-tests.mjs'); function sandbox(t) { - const home = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'ak-runner-'))); - t.after(() => fs.rmSync(home, { recursive: true, force: true })); + const home = tempDir('ak-runner', t); const repo = path.join(home, 'repo'); fs.mkdirSync(path.join(repo, '.git'), { recursive: true }); const env = spawnEnv(home, { APPDATA: path.join(home, 'AppData', 'Roaming'), CI: 'true' }); @@ -130,3 +129,77 @@ test('the suite runs without the shell FORCE_COLOR', (t) => { const r = spawnSync(process.execPath, [RUNNER, 'exec', '--repo', repo, '--', probe], { env: { ...env, FORCE_COLOR: '3' }, encoding: 'utf8' }); assert.equal(r.status, 0, r.stdout + r.stderr); }); + +test('home temp base is refused before creating a run root', (t) => { + const { home, repo, env } = sandbox(t); + const before = fs.readdirSync(home); + const r = spawnSync(process.execPath, [RUNNER, 'exec', '--repo', repo, '--', '-e', ''], { + env: { ...env, TMPDIR: home, TMP: home, TEMP: home }, encoding: 'utf8', + }); + assert.equal(r.status, 2, r.stderr); + assert.match(r.stderr, /unsafe temp base/); + assert.deepEqual(fs.readdirSync(home), before); +}); + +test('completed runners retain sibling roots and ignore their own owner metadata', async (t) => { + const { ownerRecord, writeOwner } = await import('../../scripts/run-roots.mjs'); + const { home, repo, env } = sandbox(t); + const parent = env.TMPDIR; + const sibling = fs.mkdtempSync(path.join(parent, 'ak-suite-')); + writeOwner(sibling, ownerRecord({ pid: process.pid })); + fs.writeFileSync(path.join(sibling, 'sentinel'), 'preserve'); + for (const exit of [0, 7]) { + const r = spawnSync(process.execPath, [RUNNER, 'exec', '--repo', repo, '--', '-e', `process.exit(${exit})`], { env, encoding: 'utf8' }); + assert.equal(r.status, exit, r.stderr); + assert.match(r.stderr, /kept run root.*cannot prove complete descendant exit/); + assert.doesNotMatch(r.stderr, /temp folders left behind/); + assert.equal(fs.readFileSync(path.join(sibling, 'sentinel'), 'utf8'), 'preserve'); + assert.deepEqual(fs.readdirSync(parent), [path.basename(sibling)]); + } + assert.ok(home); +}); + +test('a reaped owner does not authorize sibling deletion after a completed run', async (t) => { + const { ownerRecord, writeOwner } = await import('../../scripts/run-roots.mjs'); + const { repo, env } = sandbox(t); + const startedAt = Date.now(); + const child = spawnSync(process.execPath, ['-e', ''], { env }); + assert.equal(child.status, 0); + const sibling = fs.mkdtempSync(path.join(env.TMPDIR, 'ak-suite-')); + writeOwner(sibling, ownerRecord({ pid: child.pid, now: startedAt })); + const r = spawnSync(process.execPath, [RUNNER, 'exec', '--repo', repo, '--', '-e', ''], { env, encoding: 'utf8' }); + assert.equal(r.status, 0, r.stderr); + assert.match(r.stderr, /cannot prove complete descendant exit/); + assert.ok(fs.existsSync(sibling)); +}); + +test('a signalled command retains its run root and performs no sibling collection', (t) => { + const { repo, env } = sandbox(t); + const sibling = fs.mkdtempSync(path.join(env.TMPDIR, 'ak-suite-')); + const r = spawnSync(process.execPath, [RUNNER, 'exec', '--repo', repo, '--', '-e', + "process.kill(process.pid, 'SIGTERM')"], { env, encoding: 'utf8' }); + assert.notEqual(r.status, 0); + assert.match(r.stderr, /interrupted run/); + assert.doesNotMatch(r.stderr, /^kept run root/m); + assert.equal(fs.readdirSync(env.TMPDIR).length, 2); + assert.ok(fs.existsSync(sibling)); +}); + +test('own-root listing and removal failures preserve the suite exit code', (t) => { + const { home, repo, env } = sandbox(t); + const probe = stub(home, 'cleanup-errors.mjs', `import fs from 'node:fs'; + import { runGuarded } from ${JSON.stringify(new URL('../../scripts/run-tests.mjs', import.meta.url).href)}; + const readdir = fs.readdirSync; + fs.readdirSync = (p, ...args) => { + if (String(p).includes('ak-suite-') && !String(p).includes('/repo')) throw Error('listing denied'); + return readdir(p, ...args); + }; + fs.rmSync = () => { throw Error('EBUSY'); }; + process.exitCode = runGuarded([['-e', 'process.exit(' + process.argv[3] + ')']], { repoRoot: process.argv[2] });`); + for (const code of [0, 7]) { + const r = spawnSync(process.execPath, [probe, repo, String(code)], { env, encoding: 'utf8' }); + assert.equal(r.status, code, r.stderr); + assert.match(r.stderr, /could not list own run root/); + assert.match(r.stderr, /may be partially removed/); + } +}); From 3365429e099d9e4ea65ac87ca7970623995651c5 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Tue, 29 Sep 2026 00:37:04 -0700 Subject: [PATCH 08/20] fix(test-runner): fail hygiene on own-root cleanup errors --- AGENTS.md | 6 ++- scripts/run-tests.mjs | 23 +++++++---- tests/kit/run-tests-runner.test.mjs | 62 ++++++++++++++++++++++++----- 3 files changed, 72 insertions(+), 19 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index cf08dacf..4d98fc98 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -338,8 +338,10 @@ repository (point `TMPDIR` elsewhere). A completed run removes only its own vali canonical, nonsymlink, current-owner root. It then lists sibling suite roots: missing, invalid, foreign or uncertain owner metadata means keep. Sibling handling is list-only on macOS, Linux and Windows because no installed probe proves all descendants have exited; even a dead owner -is insufficient. Interrupted runs remove and collect nothing. Listing/removal errors preserve -the suite's exit code; removal errors may leave a partially removed own root. The runner also +is insufficient. Interrupted runs remove and collect nothing. Sibling listing/collection errors +do not change the suite's exit code. Own-root inspection failure retains the root; inspection, +removal or safety-refusal failure returns hygiene exit 4 unless a command or tripwire failure +already takes precedence. Removal errors may leave a partially removed own root. The runner also drops `FORCE_COLOR` (Claude Code shells set it), because tests read plain text from pipes. Tests make temporary folders with `tempDir()` from `tests/kit/helpers/temp-dir.mjs`, and spawned children get their environment from `spawnEnv()` in diff --git a/scripts/run-tests.mjs b/scripts/run-tests.mjs index 3d826b7a..8c61e7c4 100644 --- a/scripts/run-tests.mjs +++ b/scripts/run-tests.mjs @@ -48,7 +48,7 @@ export function commandsFor(mode, env = process.env) { * @param {{ env?: NodeJS.ProcessEnv, repoRoot?: string, platform?: string, homedir?: string, log?: (s: string) => void }} [o] * @returns {number} exit code: 2 when the suite temp root sits inside a git repository, * else the first failing command's, else 3 on a real-state change, else 4 on leftover - * temp folders, else 0 + * temp folders or failed own-root inspection/cleanup, else 0 */ export function runGuarded(commands, { env = process.env, repoRoot = REPO, platform = process.platform, homedir = os.homedir(), log = console.error, @@ -64,14 +64,18 @@ export function runGuarded(commands, { const identity = fs.lstatSync(tempRoot); const removeOwnRoot = () => { const safe = removableRunRoot(tempRoot, { tmpdir, homedir, requireOwner: false }); - if (!safe.ok) { log(`kept own run root ${tempRoot}: ${safe.reason}`); return; } + if (!safe.ok) { log(`kept own run root ${tempRoot}: ${safe.reason}`); return false; } try { const current = fs.lstatSync(tempRoot); if (current.dev !== identity.dev || current.ino !== identity.ino || current.birthtimeMs !== identity.birthtimeMs) { - log(`kept own run root ${tempRoot}: directory identity changed`); return; + log(`kept own run root ${tempRoot}: directory identity changed`); return false; } fs.rmSync(tempRoot, { recursive: true, force: true, maxRetries: 3 }); - } catch (error) { log(`own run root removal failed; may be partially removed ${tempRoot}: ${error.message}`); } + return true; + } catch (error) { + log(`own run root removal failed; may be partially removed ${tempRoot}: ${error.message}`); + return false; + } }; const enclosing = enclosingRepository(tempRoot); if (enclosing) { @@ -97,16 +101,21 @@ export function runGuarded(commands, { if (r.status !== 0) { code = r.status ?? 1; break; } } let leftovers = []; + let ownHygieneFailed = false; try { leftovers = fs.readdirSync(tempRoot).filter((name) => !IGNORED_IN_ROOT.has(name)); } - catch (error) { log(`could not list own run root ${tempRoot}: ${error.message}`); } - removeOwnRoot(); + catch (error) { + log(`could not list own run root; kept ${tempRoot}: ${error.message}`); + ownHygieneFailed = true; + } + // Unknown contents must remain available for inspection, never count as clean. + if (!ownHygieneFailed && !removeOwnRoot()) ownHygieneFailed = true; try { collectAbandonedRoots({ tmpdir, selfRoot: tempRoot, homedir, log }); } catch (error) { log(`could not list sibling run roots: ${error.message}`); } if (leftovers.length) log(`temp folders left behind by the run (${leftovers.length}):\n ${leftovers.join('\n ')}`); const result = compareSnapshots(before, snapshotRoots(roots), { strict: isStrict(env) }); const report = formatReport(result); if (report) log(report); - return code || (result.failing.length ? 3 : 0) || (leftovers.length ? 4 : 0); + return code || (result.failing.length ? 3 : 0) || (leftovers.length || ownHygieneFailed ? 4 : 0); } /** The nearest folder at or above `dir` that holds a `.git` entry, or null. */ diff --git a/tests/kit/run-tests-runner.test.mjs b/tests/kit/run-tests-runner.test.mjs index c2d4a9b0..4ddd259b 100644 --- a/tests/kit/run-tests-runner.test.mjs +++ b/tests/kit/run-tests-runner.test.mjs @@ -185,21 +185,63 @@ test('a signalled command retains its run root and performs no sibling collectio assert.ok(fs.existsSync(sibling)); }); -test('own-root listing and removal failures preserve the suite exit code', (t) => { - const { home, repo, env } = sandbox(t); - const probe = stub(home, 'cleanup-errors.mjs', `import fs from 'node:fs'; +// Inject failures only at this subprocess's disposable temp boundary. +function cleanupProbe(home) { + return stub(home, 'cleanup-errors.mjs', `import fs from 'node:fs'; + import os from 'node:os'; import path from 'node:path'; import { runGuarded } from ${JSON.stringify(new URL('../../scripts/run-tests.mjs', import.meta.url).href)}; - const readdir = fs.readdirSync; + const [repo, mode, code, tripwire] = process.argv.slice(2); + const parent = fs.realpathSync(os.tmpdir()); + const own = (p) => path.dirname(String(p)) === parent && /^ak-suite-[A-Za-z0-9]{6}$/.test(path.basename(String(p))); + const readdir = fs.readdirSync, remove = fs.rmSync, lstat = fs.lstatSync; fs.readdirSync = (p, ...args) => { - if (String(p).includes('ak-suite-') && !String(p).includes('/repo')) throw Error('listing denied'); + if ((mode === 'inspection' && own(p)) || (mode === 'sibling' && p === parent)) throw Error('EACCES'); return readdir(p, ...args); }; - fs.rmSync = () => { throw Error('EBUSY'); }; - process.exitCode = runGuarded([['-e', 'process.exit(' + process.argv[3] + ')']], { repoRoot: process.argv[2] });`); + fs.rmSync = (p, ...args) => { + if (mode === 'removal' && own(p)) throw Error('EBUSY'); + return remove(p, ...args); + }; + let ownStats = 0; + fs.lstatSync = (p, ...args) => { + const stat = lstat(p, ...args); + if (own(p)) { + ownStats++; + if (mode === 'refusal' && ownStats >= 3) stat.isDirectory = () => false; + if (mode === 'identity' && ownStats >= 4) stat.birthtimeMs += 1; + } + return stat; + }; + const child = "const fs=require('fs'),path=require('path'),os=require('os');" + + (mode === 'inspection' ? "fs.writeFileSync(path.join(os.tmpdir(),'leftover'),'retain me');" : '') + + (tripwire === 'yes' ? "fs.mkdirSync(path.join(process.env.XDG_CONFIG_HOME,'agentic-kit'),{recursive:true});fs.writeFileSync(path.join(process.env.XDG_CONFIG_HOME,'agentic-kit','kit.json'),'{}');" : '') + + 'process.exit(' + code + ')'; + process.exitCode = runGuarded([['-e', child]], { repoRoot: repo });`); +} + +for (const mode of ['inspection', 'removal', 'refusal', 'identity']) { + test(`own-root ${mode} failure fails hygiene and retains command/tripwire precedence`, (t) => { + for (const [commandCode, tripwire, expected] of [[0, 'no', 4], [7, 'no', 7], [0, 'yes', 3]]) { + const { home, repo, env } = sandbox(t); + const r = spawnSync(process.execPath, [cleanupProbe(home), repo, mode, String(commandCode), tripwire], { env, encoding: 'utf8' }); + assert.equal(r.status, expected, r.stderr); + const roots = fs.readdirSync(env.TMPDIR).filter((name) => /^ak-suite-[A-Za-z0-9]{6}$/.test(name)); + assert.equal(roots.length, 1, r.stderr); + if (mode === 'inspection') { + assert.match(r.stderr, /could not list own run root/); + assert.equal(fs.readFileSync(path.join(env.TMPDIR, roots[0], 'leftover'), 'utf8'), 'retain me'); + } else if (mode === 'removal') assert.match(r.stderr, /may be partially removed/); + else assert.match(r.stderr, /kept own run root/); + } + }); +} + +test('sibling listing failure stays nonfatal and does not mask command failure', (t) => { for (const code of [0, 7]) { - const r = spawnSync(process.execPath, [probe, repo, String(code)], { env, encoding: 'utf8' }); + const { home, repo, env } = sandbox(t); + const r = spawnSync(process.execPath, [cleanupProbe(home), repo, 'sibling', String(code), 'no'], { env, encoding: 'utf8' }); assert.equal(r.status, code, r.stderr); - assert.match(r.stderr, /could not list own run root/); - assert.match(r.stderr, /may be partially removed/); + assert.match(r.stderr, /could not list run roots/); + assert.deepEqual(fs.readdirSync(env.TMPDIR), []); } }); From d3b09a85daf92d5ffcfaa6698bddce9b880c3e21 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Tue, 29 Sep 2026 00:50:11 -0700 Subject: [PATCH 09/20] feat(test-runner): guard focused runs and prove interrupted retention --- AGENTS.md | 4 +- docs/plans/2026-09-28-runner-hygiene.md | 4 +- scripts/run-tests.mjs | 18 +++- tests/kit/run-tests-interruption.test.mjs | 115 ++++++++++++++++++++++ tests/kit/run-tests-runner.test.mjs | 34 +++++++ 5 files changed, 171 insertions(+), 4 deletions(-) create mode 100644 tests/kit/run-tests-interruption.test.mjs diff --git a/AGENTS.md b/AGENTS.md index 4d98fc98..120a8109 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -307,7 +307,7 @@ compression, or neural-routing targets are not measured agentic-kit guarantees. pnpm test # One focused suite -node --test tests/kit/dispatch-surface.test.mjs +node scripts/run-tests.mjs focus tests/kit/dispatch-surface.test.mjs # Browser verification pnpm run test:ui @@ -320,6 +320,8 @@ pnpm run lint:md pnpm run build ``` +A plain `node --test` run lacks the wrapper's real-state tripwire and temp-root checks. + `pnpm test` and `pnpm run test:ui` run through `scripts/run-tests.mjs`, which fingerprints `~/.config/agentic-kit`, `~/.local/state/agentic-kit` (or `%APPDATA%`/`%LOCALAPPDATA%` on Windows), `~/.claude/CLAUDE.md`, `~/.claude/settings.json`, `~/.claude.json`, diff --git a/docs/plans/2026-09-28-runner-hygiene.md b/docs/plans/2026-09-28-runner-hygiene.md index cbd8900d..ed775d02 100644 --- a/docs/plans/2026-09-28-runner-hygiene.md +++ b/docs/plans/2026-09-28-runner-hygiene.md @@ -2,7 +2,7 @@ ## Status -Research drafted against `develop` commit `e2f9dcae0554ff63921df618a819fd5e6afe80d2` on `test/runner-hygiene`. Tasks 10/5/7 were independently accepted. Task 11 now implements private owner records, guarded own-root removal and list-only sibling handling; focused refusal/race/error tests and helper coverage are recorded in the ignored Task 11 report. No backlog removal, push, PR or merge has occurred. Task 12 focus implementation has not started. The creator census is complete: 647 sites classified, zero unresolved current lifecycles. Native Windows/Linux behavioral proof remains open. Default sibling handling is list-only on every platform. +Research drafted against `develop` commit `e2f9dcae0554ff63921df618a819fd5e6afe80d2` on `test/runner-hygiene`. Tasks 10/5/7 and Task 11 were independently accepted. Task 12 adds guarded `focus ` and a bounded POSIX interrupted-run proof: a completed focus run retains an interrupted root while its child lives, after that child exits, and while another runner is live. Default sibling handling remains list-only on every platform; native Windows/Linux behavior is unmeasured. No backlog removal, push, PR or merge has occurred. The creator census is complete: 647 sites classified, zero unresolved current lifecycles. ## Contract and dependencies @@ -27,6 +27,6 @@ The [cleanup design](2026-09-28-test-temp-folder-cleanup-design.md) selects B9-R ## Execution boundaries -For now, use `node scripts/run-tests.mjs exec -- --test ` with disposable home/state roots. After Task 12, use `node scripts/run-tests.mjs focus `. Do not use pnpm in a worktree with symlinked dependencies. Run focused failure-path checks before wider gates; do not repeat green gates without a new concern. Each code unit needs its own failing/passing evidence and conventional commit after authorization. Before editing/staging `AGENTS.md`, verify there is no injected drift. +Use `node scripts/run-tests.mjs focus ` with disposable home/state roots for focused tests. Do not use pnpm in a worktree with symlinked dependencies. Run focused failure-path checks before wider gates; do not repeat green gates without a new concern. Each code unit needs its own failing/passing evidence and conventional commit after authorization. Before editing/staging `AGENTS.md`, verify there is no injected drift. Task 13 excludes recently modified entries, live-handle matches, unattributed prefixes, product-created `ak-sync-preview-npm-*`, and valid owner roots. Age is a manual-review filter only. Task 13 does not implement deletion. No unit claims interrupted-run backlog reclamation until a platform can prove every descendant gone. diff --git a/scripts/run-tests.mjs b/scripts/run-tests.mjs index 8c61e7c4..6ef90a01 100644 --- a/scripts/run-tests.mjs +++ b/scripts/run-tests.mjs @@ -71,6 +71,8 @@ export function runGuarded(commands, { log(`kept own run root ${tempRoot}: directory identity changed`); return false; } fs.rmSync(tempRoot, { recursive: true, force: true, maxRetries: 3 }); + try { log(`removed own run root ${tempRoot}`); } + catch { /* Reporting cannot change a completed removal into a failure. */ } return true; } catch (error) { log(`own run root removal failed; may be partially removed ${tempRoot}: ${error.message}`); @@ -132,6 +134,15 @@ function enclosingRepository(dir) { function main(argv) { const [mode] = argv; if (mode === 'unit' || mode === 'ui') return runGuarded(commandsFor(mode)); + if (mode === 'focus') { + const files = argv.slice(1); + // Reject Node options disguised as filenames before constructing an argv vector. + if (!files.length || files.some((file) => !file || file.startsWith('-') || !isTestFile(file))) { + console.error('usage: run-tests.mjs focus (each file must exist)'); + return 2; + } + return runGuarded([['--test', ...files]]); + } if (mode === 'exec') { const sep = argv.indexOf('--'); const repoAt = argv.indexOf('--repo'); @@ -139,10 +150,15 @@ function main(argv) { const repoRoot = repoAt >= 0 && repoAt < sep ? path.resolve(argv[repoAt + 1]) : REPO; return runGuarded([argv.slice(sep + 1)], { repoRoot }); } - console.error('usage: run-tests.mjs unit|ui|exec'); + console.error('usage: run-tests.mjs unit|ui|exec|focus'); return 2; } +function isTestFile(file) { + try { return fs.statSync(path.resolve(REPO, file)).isFile(); } + catch { return false; } +} + // Compare real paths (drive-letter case differs on Windows): a missed match would // make `pnpm test` exit 0 having run nothing. const isMain = () => { diff --git a/tests/kit/run-tests-interruption.test.mjs b/tests/kit/run-tests-interruption.test.mjs new file mode 100644 index 00000000..4fedd3ac --- /dev/null +++ b/tests/kit/run-tests-interruption.test.mjs @@ -0,0 +1,115 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { spawn, spawnSync } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; +import { tempDir } from './helpers/temp-dir.mjs'; +import { spawnEnv } from './helpers/home-sandbox.mjs'; + +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..'); +const RUNNER = path.join(ROOT, 'scripts', 'run-tests.mjs'); +const ownerFile = '.ak-suite-owner.json'; + +async function until(check, description, timeout = 5000) { + const deadline = Date.now() + timeout; + while (Date.now() < deadline) { + const result = check(); + if (result) return result; + await new Promise((resolve) => setTimeout(resolve, 25)); + } + throw Error(`timed out waiting for ${description}`); +} + +function roots(parent) { + return fs.readdirSync(parent).filter((name) => /^ak-suite-[A-Za-z0-9]{6}$/.test(name)).sort(); +} + +function runnerFor(repo, script, handshake, stop, done, env, log) { + const fd = fs.openSync(log, 'w'); + try { + return spawn(process.execPath, [RUNNER, 'exec', '--repo', repo, '--', script, handshake, stop, done], { + env, stdio: ['ignore', fd, fd], + }); + } finally { fs.closeSync(fd); } +} + +test('interrupted and live sibling roots stay listed, including after the orphan exits', { + skip: process.platform === 'win32' && 'POSIX runner termination probe; Windows retains siblings by list-only policy', + timeout: 20000, +}, async (t) => { + const home = tempDir('ak-interrupt', t); + const repo = path.join(home, 'repo'); + fs.mkdirSync(path.join(repo, '.git'), { recursive: true }); + const env = spawnEnv(home, { APPDATA: path.join(home, 'AppData', 'Roaming'), CI: 'true' }); + delete env.NODE_TEST_CONTEXT; + const parent = env.TMPDIR; + const baseline = fs.readdirSync(parent); + const script = path.join(home, 'hold.mjs'); + fs.writeFileSync(script, `import fs from 'node:fs'; + const [handshake, stop, done] = process.argv.slice(2); + fs.writeFileSync(handshake, JSON.stringify({ pid: process.pid, cwd: process.cwd(), tmpdir: process.env.TMPDIR })); + const timer = setInterval(() => { + if (fs.existsSync(stop)) { fs.writeFileSync(done, 'stopped'); clearInterval(timer); process.exit(0); } + }, 25); + setTimeout(() => process.exit(8), 12000);`); + const clean = path.join(home, 'clean.test.mjs'); + fs.writeFileSync(clean, "import { test } from 'node:test'; test('clean', () => {});"); + const first = ['first-handshake', 'first-stop', 'first-done'].map((name) => path.join(home, name)); + const live = ['live-handshake', 'live-stop', 'live-done'].map((name) => path.join(home, name)); + let runner1; + let runner4; + let interruptedRoot; + try { + runner1 = runnerFor(repo, script, ...first, env, path.join(home, 'run1.log')); + const firstData = await until(() => fs.existsSync(first[0]) && JSON.parse(fs.readFileSync(first[0], 'utf8')), 'first child handshake'); + const root1 = await until(() => roots(parent).map((name) => path.join(parent, name)).find((root) => { + try { return JSON.parse(fs.readFileSync(path.join(root, ownerFile), 'utf8')).pid === runner1.pid; } + catch { return false; } + }), 'first owner record'); + interruptedRoot = root1; + assert.equal(firstData.cwd, repo); + assert.equal(firstData.tmpdir, root1); + assert.ok(firstData.pid > 0); + assert.equal(runner1.kill('SIGTERM'), true); + await until(() => runner1.exitCode !== null || runner1.signalCode !== null, 'owned runner termination'); + assert.ok(fs.existsSync(root1)); + assert.equal(fs.existsSync(first[2]), false, 'orphan has not stopped'); + + const runFocus = () => spawnSync(process.execPath, [RUNNER, 'focus', clean], { env, encoding: 'utf8' }); + const second = runFocus(); + assert.equal(second.status, 0, second.stdout + second.stderr); + assert.match(second.stderr, /kept run root.*cannot prove complete descendant exit \(list-only\)/); + assert.deepEqual(roots(parent), [path.basename(root1)]); + + runner4 = runnerFor(repo, script, ...live, env, path.join(home, 'run4.log')); + const liveData = await until(() => fs.existsSync(live[0]) && JSON.parse(fs.readFileSync(live[0], 'utf8')), 'live child handshake'); + const root4 = await until(() => roots(parent).map((name) => path.join(parent, name)).find((root) => { + try { return JSON.parse(fs.readFileSync(path.join(root, ownerFile), 'utf8')).pid === runner4.pid; } + catch { return false; } + }), 'live owner record'); + assert.equal(liveData.tmpdir, root4); + fs.writeFileSync(first[1], 'stop'); + await until(() => fs.existsSync(first[2]), 'orphan private stop acknowledgement'); + const third = runFocus(); + assert.equal(third.status, 0, third.stdout + third.stderr); + assert.match(third.stderr, /kept run root.*cannot prove complete descendant exit \(list-only\)/); + assert.deepEqual(roots(parent), [path.basename(root1), path.basename(root4)].sort()); + assert.ok(fs.existsSync(root1), 'known stopped child does not authorize sibling removal'); + assert.ok(fs.existsSync(root4), 'concurrently live sibling remains'); + } finally { + // Stop only children created by this fixture, through their private channels. + fs.writeFileSync(first[1], 'stop'); + fs.writeFileSync(live[1], 'stop'); + if (runner1 && runner1.exitCode === null && runner1.signalCode === null) runner1.kill('SIGTERM'); + if (runner4 && runner4.exitCode === null && runner4.signalCode === null) { + await until(() => runner4.exitCode !== null || runner4.signalCode !== null, 'live runner completion', 14000); + } + if (fs.existsSync(first[0])) await until(() => fs.existsSync(first[2]), 'first child exit', 14000); + if (fs.existsSync(live[0])) await until(() => fs.existsSync(live[2]), 'live child exit', 14000); + } + // Only this test's disposable fixture root is removed after its child stops. + assert.deepEqual(roots(parent), [path.basename(interruptedRoot)]); + fs.rmSync(interruptedRoot, { recursive: true }); + assert.deepEqual(fs.readdirSync(parent), baseline); +}); diff --git a/tests/kit/run-tests-runner.test.mjs b/tests/kit/run-tests-runner.test.mjs index 4ddd259b..242c414c 100644 --- a/tests/kit/run-tests-runner.test.mjs +++ b/tests/kit/run-tests-runner.test.mjs @@ -17,11 +17,45 @@ function sandbox(t) { fs.mkdirSync(path.join(repo, '.git'), { recursive: true }); const env = spawnEnv(home, { APPDATA: path.join(home, 'AppData', 'Roaming'), CI: 'true' }); delete env.AK_TRIPWIRE_STRICT; + // Nested CLI fixtures must run as independent test processes. + delete env.NODE_TEST_CONTEXT; return { home, repo, env }; } const stub = (dir, name, body) => { const f = path.join(dir, name); fs.writeFileSync(f, body); return f; }; +test('focus runs a literal clean test file through the guarded root', (t) => { + const { home, env } = sandbox(t); + const file = stub(home, 'clean.test.mjs', "import { test } from 'node:test'; test('clean', () => {});"); + const before = fs.readdirSync(env.TMPDIR); + const r = spawnSync(process.execPath, [RUNNER, 'focus', file], { env, encoding: 'utf8' }); + assert.equal(r.status, 0, r.stdout + r.stderr); + assert.match(r.stderr, /real-state tripwire: watching/); + assert.match(r.stderr, /removed own run root /); + assert.deepEqual(fs.readdirSync(env.TMPDIR), before); +}); + +test('focus reports a leaked temp folder with hygiene exit code', (t) => { + const { home, env } = sandbox(t); + const file = stub(home, 'leaky.test.mjs', `import { test } from 'node:test'; + import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; + test('leak', () => { fs.mkdtempSync(path.join(os.tmpdir(), 'focus-leak-')); });`); + const r = spawnSync(process.execPath, [RUNNER, 'focus', file], { env, encoding: 'utf8' }); + assert.equal(r.status, 4, r.stdout + r.stderr); + assert.match(r.stderr, /temp folders left behind.*focus-leak-/s); +}); + +test('focus rejects missing files and option-shaped filenames before creating roots', (t) => { + const { env } = sandbox(t); + const before = fs.readdirSync(env.TMPDIR); + for (const args of [[], ['--test-reporter=dot'], ['does-not-exist.test.mjs']]) { + const r = spawnSync(process.execPath, [RUNNER, 'focus', ...args], { env, encoding: 'utf8' }); + assert.equal(r.status, 2, r.stdout + r.stderr); + assert.match(r.stderr, /usage: run-tests\.mjs focus/); + assert.deepEqual(fs.readdirSync(env.TMPDIR), before); + } +}); + test('a command that writes real state fails the run and the path is named', (t) => { const { home, repo, env } = sandbox(t); const leak = stub(home, 'leak.mjs', `import fs from 'node:fs'; import path from 'node:path'; From 0cc5c11df5366c7048a349591088afc4ce7eb243 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Tue, 29 Sep 2026 01:02:58 -0700 Subject: [PATCH 10/20] test(test-runner): observe orphan exit before retention check --- tests/kit/run-tests-interruption.test.mjs | 28 +++++++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/tests/kit/run-tests-interruption.test.mjs b/tests/kit/run-tests-interruption.test.mjs index 4fedd3ac..7ef5d60e 100644 --- a/tests/kit/run-tests-interruption.test.mjs +++ b/tests/kit/run-tests-interruption.test.mjs @@ -25,6 +25,11 @@ function roots(parent) { return fs.readdirSync(parent).filter((name) => /^ak-suite-[A-Za-z0-9]{6}$/.test(name)).sort(); } +function childGone(pid) { + try { process.kill(pid, 0); return false; } + catch (error) { return error.code === 'ESRCH'; } +} + function runnerFor(repo, script, handshake, stop, done, env, log) { const fd = fs.openSync(log, 'w'); try { @@ -50,7 +55,10 @@ test('interrupted and live sibling roots stay listed, including after the orphan const [handshake, stop, done] = process.argv.slice(2); fs.writeFileSync(handshake, JSON.stringify({ pid: process.pid, cwd: process.cwd(), tmpdir: process.env.TMPDIR })); const timer = setInterval(() => { - if (fs.existsSync(stop)) { fs.writeFileSync(done, 'stopped'); clearInterval(timer); process.exit(0); } + if (!fs.existsSync(stop)) return; + const request = fs.readFileSync(stop, 'utf8'); + if (!fs.existsSync(done)) fs.writeFileSync(done, 'stopped'); + if (request === 'exit') { clearInterval(timer); process.exit(0); } }, 25); setTimeout(() => process.exit(8), 12000);`); const clean = path.join(home, 'clean.test.mjs'); @@ -91,6 +99,10 @@ test('interrupted and live sibling roots stay listed, including after the orphan assert.equal(liveData.tmpdir, root4); fs.writeFileSync(first[1], 'stop'); await until(() => fs.existsSync(first[2]), 'orphan private stop acknowledgement'); + assert.equal(childGone(firstData.pid), false, 'acknowledgement precedes child exit'); + fs.writeFileSync(first[1], 'exit'); + await until(() => childGone(firstData.pid), 'orphan PID absent after private exit request'); + assert.equal(childGone(liveData.pid), false, 'concurrent child remains alive'); const third = runFocus(); assert.equal(third.status, 0, third.stdout + third.stderr); assert.match(third.stderr, /kept run root.*cannot prove complete descendant exit \(list-only\)/); @@ -99,14 +111,20 @@ test('interrupted and live sibling roots stay listed, including after the orphan assert.ok(fs.existsSync(root4), 'concurrently live sibling remains'); } finally { // Stop only children created by this fixture, through their private channels. - fs.writeFileSync(first[1], 'stop'); - fs.writeFileSync(live[1], 'stop'); + fs.writeFileSync(first[1], 'exit'); + fs.writeFileSync(live[1], 'exit'); if (runner1 && runner1.exitCode === null && runner1.signalCode === null) runner1.kill('SIGTERM'); if (runner4 && runner4.exitCode === null && runner4.signalCode === null) { await until(() => runner4.exitCode !== null || runner4.signalCode !== null, 'live runner completion', 14000); } - if (fs.existsSync(first[0])) await until(() => fs.existsSync(first[2]), 'first child exit', 14000); - if (fs.existsSync(live[0])) await until(() => fs.existsSync(live[2]), 'live child exit', 14000); + if (fs.existsSync(first[0])) { + const { pid } = JSON.parse(fs.readFileSync(first[0], 'utf8')); + await until(() => childGone(pid), 'first child exit', 14000); + } + if (fs.existsSync(live[0])) { + const { pid } = JSON.parse(fs.readFileSync(live[0], 'utf8')); + await until(() => childGone(pid), 'live child exit', 14000); + } } // Only this test's disposable fixture root is removed after its child stops. assert.deepEqual(roots(parent), [path.basename(interruptedRoot)]); From 9b5b3a4394da3a078c9860163ef0379ce565cf61 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Tue, 29 Sep 2026 01:19:33 -0700 Subject: [PATCH 11/20] test(runner): preserve tool selector propagation --- docs/plans/2026-09-28-runner-hygiene.md | 2 ++ tests/kit/run-tests-runner.test.mjs | 15 +++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/docs/plans/2026-09-28-runner-hygiene.md b/docs/plans/2026-09-28-runner-hygiene.md index ed775d02..11ff6943 100644 --- a/docs/plans/2026-09-28-runner-hygiene.md +++ b/docs/plans/2026-09-28-runner-hygiene.md @@ -4,6 +4,8 @@ Research drafted against `develop` commit `e2f9dcae0554ff63921df618a819fd5e6afe80d2` on `test/runner-hygiene`. Tasks 10/5/7 and Task 11 were independently accepted. Task 12 adds guarded `focus ` and a bounded POSIX interrupted-run proof: a completed focus run retains an interrupted root while its child lives, after that child exits, and while another runner is live. Default sibling handling remains list-only on every platform; native Windows/Linux behavior is unmeasured. No backlog removal, push, PR or merge has occurred. The creator census is complete: 647 sites classified, zero unresolved current lifecycles. +LQ1 source inspection refutes the claimed missing selector propagation: the runner copies its input environment and removes only `FORCE_COLOR`. A guarded-child sentinel regression is green on the original implementation and fails when that copy is removed. LQ4 Chrome environment narrowing is in progress. + ## Contract and dependencies The [v2 scope](2026-09-28-remediation-program-v2.md#v5-testrunner-hygiene-suites-that-clean-up-after-themselves) inherits [archived Branch 9](../archive/2026-09-28-superpowers-plan-branch-9-follow-ups.md) Tasks 5, 7 and 10–13, under B9-R1–R8. V1 is integrated at the baseline. V4/V6 must coordinate before changing environment-helper consumers. Worktree ownership is limited to this branch; shared manifests remain the integration owner's responsibility. diff --git a/tests/kit/run-tests-runner.test.mjs b/tests/kit/run-tests-runner.test.mjs index 242c414c..a36978df 100644 --- a/tests/kit/run-tests-runner.test.mjs +++ b/tests/kit/run-tests-runner.test.mjs @@ -164,6 +164,21 @@ test('the suite runs without the shell FORCE_COLOR', (t) => { assert.equal(r.status, 0, r.stdout + r.stderr); }); +test('guarded runner preserves tool selectors while scrubbing FORCE_COLOR', (t) => { + const { home, repo, env } = sandbox(t); + const probe = stub(home, 'selectors.mjs', ` + import assert from 'node:assert/strict'; + assert.equal(process.env.AQE_EMBEDDER_PROVIDER, 'sentinel-provider'); + assert.equal(process.env.AQE_EMBEDDER_MODEL, 'sentinel-model'); + assert.equal(process.env.FORCE_COLOR, undefined); + `); + const r = spawnSync(process.execPath, [RUNNER, 'exec', '--repo', repo, '--', probe], { + env: { ...env, AQE_EMBEDDER_PROVIDER: 'sentinel-provider', AQE_EMBEDDER_MODEL: 'sentinel-model', FORCE_COLOR: '3' }, + encoding: 'utf8', + }); + assert.equal(r.status, 0, r.stdout + r.stderr); +}); + test('home temp base is refused before creating a run root', (t) => { const { home, repo, env } = sandbox(t); const before = fs.readdirSync(home); From a50223fa3b72c7112fb61adda3a13ac5bf654566 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Tue, 29 Sep 2026 01:24:10 -0700 Subject: [PATCH 12/20] fix(ui): isolate Chrome launch environment --- docs/plans/2026-09-28-runner-hygiene.md | 2 +- tests/kit/ui-chrome-launch.test.mjs | 41 +++++++++++++++++++++++++ tests/ui/helpers/launch-chrome.mjs | 32 +++++++++++++++++-- 3 files changed, 72 insertions(+), 3 deletions(-) diff --git a/docs/plans/2026-09-28-runner-hygiene.md b/docs/plans/2026-09-28-runner-hygiene.md index 11ff6943..e4700a04 100644 --- a/docs/plans/2026-09-28-runner-hygiene.md +++ b/docs/plans/2026-09-28-runner-hygiene.md @@ -4,7 +4,7 @@ Research drafted against `develop` commit `e2f9dcae0554ff63921df618a819fd5e6afe80d2` on `test/runner-hygiene`. Tasks 10/5/7 and Task 11 were independently accepted. Task 12 adds guarded `focus ` and a bounded POSIX interrupted-run proof: a completed focus run retains an interrupted root while its child lives, after that child exits, and while another runner is live. Default sibling handling remains list-only on every platform; native Windows/Linux behavior is unmeasured. No backlog removal, push, PR or merge has occurred. The creator census is complete: 647 sites classified, zero unresolved current lifecycles. -LQ1 source inspection refutes the claimed missing selector propagation: the runner copies its input environment and removes only `FORCE_COLOR`. A guarded-child sentinel regression is green on the original implementation and fails when that copy is removed. LQ4 Chrome environment narrowing is in progress. +LQ1 source inspection refutes the claimed missing selector propagation: the runner copies its input environment and removes only `FORCE_COLOR`. A guarded-child sentinel regression is green on the original implementation and fails when that copy is removed. LQ4 now uses an explicit Chrome launch environment with a private home and temp root; the local macOS system Chrome passed the full guarded UI suite (495 dashboard checks, 15 Node UI tests). Guarded unit, TypeScript and targeted ESLint gates passed. Native Windows and Linux Chrome behavior remains unmeasured. ## Contract and dependencies diff --git a/tests/kit/ui-chrome-launch.test.mjs b/tests/kit/ui-chrome-launch.test.mjs index e279710a..e0e43aa7 100644 --- a/tests/kit/ui-chrome-launch.test.mjs +++ b/tests/kit/ui-chrome-launch.test.mjs @@ -61,3 +61,44 @@ test('launchChrome removes its temp folder when Chrome fails to start', async (t await assert.rejects(launchChrome(), /no chrome/); assert.equal(fs.existsSync(dir), false); }); + +test('launchChrome excludes credentials and caller env while retaining launch options', async (t) => { + const { chromium } = await import('playwright'); + const { launchChrome } = await import('../ui/helpers/launch-chrome.mjs'); + const injected = ['AK_CHROME_SECRET_SENTINEL', 'AQE_EMBEDDER_PROVIDER', 'CODEX_HOME', 'CLAUDE_CONFIG_DIR']; + const original = Object.fromEntries(injected.map((key) => [key, process.env[key]])); + for (const key of injected) process.env[key] = `sentinel-${key}`; + t.after(() => { + for (const key of injected) { + if (original[key] === undefined) delete process.env[key]; + else process.env[key] = original[key]; + } + }); + let seen; + t.mock.method(chromium, 'launch', async (options) => { seen = options; return { close: async () => {} }; }); + const browser = await launchChrome({ headless: false, args: ['--disable-gpu'], env: { AK_CHROME_SECRET_SENTINEL: 'override' } }); + try { + assert.equal(seen.headless, false); + assert.deepEqual(seen.args, ['--disable-gpu']); + assert.equal(seen.env.AK_CHROME_SECRET_SENTINEL, undefined); + for (const key of injected) { + assert.equal(seen.env[key], undefined, `${key} should not reach Chrome`); + } + for (const key of ['HOME', 'USERPROFILE', 'XDG_CONFIG_HOME', 'XDG_CACHE_HOME', 'APPDATA', 'LOCALAPPDATA']) { + assert.ok(seen.env[key].startsWith(seen.env.TMPDIR), `${key} should be private`); + } + } finally { await browser.close(); } +}); + +test('Chrome environment selects Windows names without duplicate case variants', async () => { + const { chromeEnv } = await import('../ui/helpers/launch-chrome.mjs'); + const env = chromeEnv({ Path: 'first', PATH: 'second', display: ':8', SystemRoot: 'C:\\Windows', + temp: 'real-temp', TOKEN: 'secret' }, 'private-temp', 'win32'); + assert.equal(env.Path, 'second'); + assert.equal(env.SystemRoot, 'C:\\Windows'); + assert.equal(env.TEMP, 'private-temp'); + assert.equal(env.TMP, 'private-temp'); + assert.equal(Object.keys(env).filter((key) => key.toUpperCase() === 'PATH').length, 1); + assert.equal(Object.keys(env).filter((key) => key.toUpperCase() === 'TEMP').length, 1); + assert.equal(env.TOKEN, undefined); +}); diff --git a/tests/ui/helpers/launch-chrome.mjs b/tests/ui/helpers/launch-chrome.mjs index bb38d9bb..25f485f8 100644 --- a/tests/ui/helpers/launch-chrome.mjs +++ b/tests/ui/helpers/launch-chrome.mjs @@ -12,6 +12,35 @@ import os from 'node:os'; import path from 'node:path'; import { chromium } from 'playwright'; +// Chrome needs the executable search path, display connection and a few Windows +// process basics. Its home and temp state belong to this launch, not the caller. +const CHROME_KEYS = ['PATH', 'DISPLAY', 'WAYLAND_DISPLAY', 'XAUTHORITY', 'XDG_RUNTIME_DIR', + 'DBUS_SESSION_BUS_ADDRESS', 'SYSTEMROOT', 'WINDIR', 'COMSPEC', 'PATHEXT']; +const WINDOWS_NAMES = { PATH: 'Path', SYSTEMROOT: 'SystemRoot', WINDIR: 'windir', COMSPEC: 'ComSpec', PATHEXT: 'PATHEXT' }; + +/** @param {NodeJS.ProcessEnv} source @param {string} dir @param {string} [platform] */ +export function chromeEnv(source, dir, platform = process.platform) { + const windows = platform === 'win32'; + const env = {}; + for (const key of CHROME_KEYS) { + let value = source[key]; + if (windows) { + const matches = Object.keys(source).filter((name) => name.toUpperCase() === key); + const chosen = matches.includes(key) ? key : matches.sort()[0]; + value = chosen === undefined ? undefined : source[chosen]; + } + if (value !== undefined) env[windows ? (WINDOWS_NAMES[key] ?? key) : key] = value; + } + return { + ...env, + HOME: dir, USERPROFILE: dir, + XDG_CONFIG_HOME: path.join(dir, 'config'), XDG_CACHE_HOME: path.join(dir, 'cache'), + XDG_DATA_HOME: path.join(dir, 'data'), APPDATA: path.join(dir, 'appdata'), + LOCALAPPDATA: path.join(dir, 'localappdata'), + TMPDIR: dir, TEMP: dir, TMP: dir, MAC_CHROMIUM_TMPDIR: dir, + }; +} + /** * @param {import('playwright').LaunchOptions} [options] merged over { channel: 'chrome', headless: true } * @returns {Promise} a browser whose close() also removes Chrome's temp folder @@ -19,12 +48,11 @@ import { chromium } from 'playwright'; export async function launchChrome(options = {}) { const dir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'ak-ui-chrome-'))); const remove = () => fs.rmSync(dir, { recursive: true, force: true, maxRetries: 3 }); - const temp = { TMPDIR: dir, TEMP: dir, TMP: dir, MAC_CHROMIUM_TMPDIR: dir }; let browser; try { browser = await chromium.launch({ channel: 'chrome', headless: true, ...options, - env: { ...process.env, ...temp }, // spawn-env: inherits (the browser needs the display and PATH; only its temp dir moves) + env: chromeEnv(process.env, dir), }); } catch (error) { remove(); throw error; } const close = browser.close.bind(browser); From 5d322f8e37b4ee01c2e72dd680937e621e9d9be3 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Tue, 29 Sep 2026 01:33:13 -0700 Subject: [PATCH 13/20] test(status): await owned spawn guard child before cleanup --- docs/plans/2026-09-28-runner-hygiene.md | 2 + tests/kit/status-zero-spawn.test.mjs | 113 +++++++++++++++++++----- 2 files changed, 92 insertions(+), 23 deletions(-) diff --git a/docs/plans/2026-09-28-runner-hygiene.md b/docs/plans/2026-09-28-runner-hygiene.md index e4700a04..cd3cc2e4 100644 --- a/docs/plans/2026-09-28-runner-hygiene.md +++ b/docs/plans/2026-09-28-runner-hygiene.md @@ -6,6 +6,8 @@ Research drafted against `develop` commit `e2f9dcae0554ff63921df618a819fd5e6afe8 LQ1 source inspection refutes the claimed missing selector propagation: the runner copies its input environment and removes only `FORCE_COLOR`. A guarded-child sentinel regression is green on the original implementation and fails when that copy is removed. LQ4 now uses an explicit Chrome launch environment with a private home and temp root; the local macOS system Chrome passed the full guarded UI suite (495 dashboard checks, 15 Node UI tests). Guarded unit, TypeScript and targeted ESLint gates passed. Native Windows and Linux Chrome behavior remains unmeasured. +The Windows smoke lifetime follow-up has a deterministic handshake regression: the previous guarded parent exited while its forked child waited in the project directory. The smoke now waits for that exact child's `close` or `error`, checks its status, and releases it before sandbox removal. The guarded focused suite, typecheck and targeted ESLint pass on macOS. This establishes the lifecycle gap locally; native Windows CI remains the required platform proof for the reported `EPERM`. + ## Contract and dependencies The [v2 scope](2026-09-28-remediation-program-v2.md#v5-testrunner-hygiene-suites-that-clean-up-after-themselves) inherits [archived Branch 9](../archive/2026-09-28-superpowers-plan-branch-9-follow-ups.md) Tasks 5, 7 and 10–13, under B9-R1–R8. V1 is integrated at the baseline. V4/V6 must coordinate before changing environment-helper consumers. Worktree ownership is limited to this branch; shared manifests remain the integration owner's responsibility. diff --git a/tests/kit/status-zero-spawn.test.mjs b/tests/kit/status-zero-spawn.test.mjs index a14ea7e2..28892bfa 100644 --- a/tests/kit/status-zero-spawn.test.mjs +++ b/tests/kit/status-zero-spawn.test.mjs @@ -17,7 +17,7 @@ // `refresh: true` (always probes again, even warm). import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { execFileSync } from 'node:child_process'; +import { execFileSync, spawn } from 'node:child_process'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; @@ -37,7 +37,7 @@ function readLedger(file) { /** Runs `fn` inside a disposable sandboxed HOME/project, with `extraEnv` * merged into the child's environment. Cleans up unconditionally. */ -function inSandbox(prefix, extraEnv, fn) { +async function inSandbox(prefix, extraEnv, fn) { const home = fs.mkdtempSync(path.join(os.tmpdir(), `${prefix}-home-`)); fs.mkdirSync(path.join(home, '.config'), { recursive: true }); const project = sandboxProject(prefix); @@ -48,16 +48,20 @@ function inSandbox(prefix, extraEnv, fn) { PATH: path.join(home, 'no-such-bin'), ...extraEnv, }); - try { - return fn({ home, project, env }); - } finally { - fs.rmSync(home, { recursive: true, force: true }); - fs.rmSync(project, { recursive: true, force: true }); + let result; + let failure; + try { result = await fn({ home, project, env }); } + catch (error) { failure = error; } + for (const root of [home, project]) { + try { fs.rmSync(root, { recursive: true, force: true }); } + catch (error) { failure = failure ? new AggregateError([failure, error], 'sandbox assertion and cleanup failed') : error; } } + if (failure) throw failure; + return result; } -test('spawn-guard is a no-op when AK_SPAWN_LEDGER_FILE is unset', () => { - inSandbox('ak-spawn-guard-noop', {}, ({ project, env }) => { +test('spawn-guard is a no-op when AK_SPAWN_LEDGER_FILE is unset', async () => { + await inSandbox('ak-spawn-guard-noop', {}, ({ project, env }) => { // A vacuous "no ledger file exists" check would pass even if the guard // patched child_process regardless of the env var — assert the function // ITSELF is untouched (native name `spawn`, not our wrapper's `patched`). @@ -68,25 +72,89 @@ test('spawn-guard is a no-op when AK_SPAWN_LEDGER_FILE is unset', () => { }); }); -test('spawn-guard records a spawn made inside the guarded child, by every wrapped form', () => { - inSandbox('ak-spawn-guard-smoke', {}, ({ project, env: baseEnv }) => { - const ledgerFile = path.join(os.tmpdir(), `ak-spawn-guard-smoke-${process.pid}.ndjson`); +test('spawn-guard records every wrapped form and waits for its owned fork', async () => { + await inSandbox('ak-spawn-guard-smoke', {}, async ({ project, env: baseEnv }) => { + const ledgerFile = path.join(project, 'spawn-ledger.ndjson'); + const readyFile = path.join(project, 'fork-ready'); + const releaseFile = path.join(project, 'fork-release'); + const doneFile = path.join(project, 'fork-done'); + const pidFile = path.join(project, 'fork-pid'); const env = { ...baseEnv, AK_SPAWN_LEDGER_FILE: ledgerFile }; - const forkTarget = path.join(project, 'fork-target.mjs'); - fs.writeFileSync(forkTarget, 'process.exit(0);\n'); + const forkTarget = path.join(project, 'fork-target.cjs'); + fs.writeFileSync(forkTarget, [ + "const fs = require('node:fs');", + `fs.writeFileSync(${JSON.stringify(readyFile)}, 'ready');`, + `const release = ${JSON.stringify(releaseFile)};`, + 'const timer = setInterval(() => {', + ' if (!fs.existsSync(release)) return;', + ` fs.writeFileSync(${JSON.stringify(doneFile)}, 'done');`, + ' clearInterval(timer);', + ' process.exit(0);', + '}, 10);', + ].join('\n')); const script = [ + 'async function main() {', "const { spawnSync, execFileSync: ef, execSync: es, fork } = require('node:child_process');", "spawnSync(process.execPath, ['-e', '0']);", "ef(process.execPath, ['-e', '0']);", 'try { es(\'true\'); } catch {}', // shell builtin: exercised even with PATH broken - `fork(${JSON.stringify(forkTarget)}, [], { stdio: 'ignore' });`, - 'process.exit(0);', // do not wait on the forked grandchild's IPC channel + `const child = fork(${JSON.stringify(forkTarget)}, [], { stdio: 'ignore' });`, + `require('node:fs').writeFileSync(${JSON.stringify(pidFile)}, String(child.pid));`, + 'await new Promise((resolve, reject) => {', + ' child.once("error", reject);', + ' child.once("close", (code, signal) => code === 0 && !signal', + ' ? resolve() : reject(new Error(`fork failed: code=${code}, signal=${signal}`)));', + '});', + '}', + 'main().catch((error) => { console.error(error); process.exitCode = 1; });', ].join(' '); - execFileSync(process.execPath, [ + const guarded = spawn(process.execPath, [ `--import=${SPAWN_GUARD_URL}`, '-e', script, - ], { cwd: project, env, encoding: 'utf8' }); + ], { cwd: project, env, stdio: ['ignore', 'ignore', 'pipe'] }); + let stderr = ''; + guarded.stderr.on('data', (chunk) => { stderr += chunk; }); + let closed = false; + const parentClose = new Promise((resolve) => guarded.once('close', (code, signal) => { + closed = true; + resolve({ code, signal }); + })); + let failure; + try { + const deadline = Date.now() + 5_000; + while (!fs.existsSync(readyFile) && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 10)); + } + assert.ok(fs.existsSync(readyFile), `fork did not become ready: ${stderr}`); + assert.strictEqual(closed, false, 'guarded parent exited while its owned fork was still running'); + } catch (error) { failure = error; } + try { + fs.writeFileSync(releaseFile, 'release'); + const deadline = Date.now() + 5_000; + while (!fs.existsSync(doneFile) && Date.now() < deadline) await new Promise((resolve) => setTimeout(resolve, 10)); + if (!fs.existsSync(doneFile) && fs.existsSync(pidFile)) { + const pid = Number(fs.readFileSync(pidFile, 'utf8')); + if (Number.isInteger(pid) && pid > 0) { try { process.kill(pid); } catch { /* already exited */ } } + } + if (!fs.existsSync(doneFile) && !closed) guarded.kill(); + let timeout; + try { + await Promise.race([parentClose, new Promise((_, reject) => { + timeout = setTimeout(() => reject(new Error('guarded parent did not close')), 5_000); + })]); + } finally { clearTimeout(timeout); } + } catch (error) { + if (fs.existsSync(pidFile)) { + const pid = Number(fs.readFileSync(pidFile, 'utf8')); + if (Number.isInteger(pid) && pid > 0) { try { process.kill(pid); } catch { /* already exited */ } } + } + if (!closed) guarded.kill(); + failure = failure ? new AggregateError([failure, error], 'fork assertion and cleanup failed') : error; + } + if (failure) throw failure; + assert.ok(fs.existsSync(doneFile), 'owned fork completed before sandbox cleanup'); + const { code, signal } = await parentClose; + assert.strictEqual(code, 0, `guarded parent failed (${signal}): ${stderr}`); const lines = readLedger(ledgerFile); - fs.rmSync(ledgerFile, { force: true }); // Each wrapped form by name, not an exact total: execSync goes through a // platform shell, and only its own record is what this test is about. const got = JSON.stringify(lines); @@ -130,15 +198,14 @@ function sliceByCallBoundary(lines, labels) { return slices; } -test('plain ak status spawns nothing on a warm cache; --refresh always re-probes', () => { - inSandbox('ak-status-zero-spawn', {}, ({ project, env: baseEnv }) => { - const ledgerFile = path.join(os.tmpdir(), `ak-status-zero-spawn-${process.pid}.ndjson`); +test('plain ak status spawns nothing on a warm cache; --refresh always re-probes', async () => { + await inSandbox('ak-status-zero-spawn', {}, ({ project, env: baseEnv }) => { + const ledgerFile = path.join(project, 'status-ledger.ndjson'); const env = { ...baseEnv, AK_SPAWN_LEDGER_FILE: ledgerFile }; execFileSync(process.execPath, [ `--import=${SPAWN_GUARD_URL}`, FIXTURE, PKG_ROOT, project, ], { cwd: project, env, encoding: 'utf8', timeout: 30_000 }); const lines = readLedger(ledgerFile); - fs.rmSync(ledgerFile, { force: true }); const { first, second, third } = sliceByCallBoundary(lines, ['first', 'second', 'third']); From 6f716f8589b655219459587ce72bf7bc165cd483 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Tue, 29 Sep 2026 01:43:55 -0700 Subject: [PATCH 14/20] test(runner): verify owned fork exits before sandbox cleanup --- docs/plans/2026-09-28-runner-hygiene.md | 2 +- tests/kit/status-zero-spawn.test.mjs | 127 ++++++++++++++++++------ 2 files changed, 97 insertions(+), 32 deletions(-) diff --git a/docs/plans/2026-09-28-runner-hygiene.md b/docs/plans/2026-09-28-runner-hygiene.md index cd3cc2e4..5c5b499f 100644 --- a/docs/plans/2026-09-28-runner-hygiene.md +++ b/docs/plans/2026-09-28-runner-hygiene.md @@ -6,7 +6,7 @@ Research drafted against `develop` commit `e2f9dcae0554ff63921df618a819fd5e6afe8 LQ1 source inspection refutes the claimed missing selector propagation: the runner copies its input environment and removes only `FORCE_COLOR`. A guarded-child sentinel regression is green on the original implementation and fails when that copy is removed. LQ4 now uses an explicit Chrome launch environment with a private home and temp root; the local macOS system Chrome passed the full guarded UI suite (495 dashboard checks, 15 Node UI tests). Guarded unit, TypeScript and targeted ESLint gates passed. Native Windows and Linux Chrome behavior remains unmeasured. -The Windows smoke lifetime follow-up has a deterministic handshake regression: the previous guarded parent exited while its forked child waited in the project directory. The smoke now waits for that exact child's `close` or `error`, checks its status, and releases it before sandbox removal. The guarded focused suite, typecheck and targeted ESLint pass on macOS. This establishes the lifecycle gap locally; native Windows CI remains the required platform proof for the reported `EPERM`. +The Windows smoke lifetime follow-up uses a held fork and parent close acknowledgment. The smoke waits for the fork's `close` or `error`, checks its exit status, and establishes both exits before sandbox removal; uncertain exits retain the sandbox. A bounded pre-release observation detects early parent exit, and a local removed-wait mutation fails the focused test. The guarded suite also covers fork failure, stalled-fork cleanup, and launch failure. This establishes the lifecycle gap locally; native Windows CI remains the required platform proof for the reported `EPERM`. ## Contract and dependencies diff --git a/tests/kit/status-zero-spawn.test.mjs b/tests/kit/status-zero-spawn.test.mjs index 28892bfa..051897c0 100644 --- a/tests/kit/status-zero-spawn.test.mjs +++ b/tests/kit/status-zero-spawn.test.mjs @@ -35,8 +35,29 @@ function readLedger(file) { return fs.readFileSync(file, 'utf8').split('\n').filter(Boolean).map((line) => JSON.parse(line)); } +async function waitUntil(check, timeoutMs = 5_000) { + const deadline = Date.now() + timeoutMs; + while (!check() && Date.now() < deadline) await new Promise((resolve) => setTimeout(resolve, 10)); + return check(); +} + +async function waitFor(promise, timeoutMs = 5_000) { + let timer; + try { + return await Promise.race([promise, new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error('owned process did not close')), timeoutMs); + })]); + } finally { clearTimeout(timer); } +} + +function processGone(pid) { + if (!Number.isInteger(pid) || pid <= 0) return false; + try { process.kill(pid, 0); return false; } + catch (error) { return error.code === 'ESRCH'; } +} + /** Runs `fn` inside a disposable sandboxed HOME/project, with `extraEnv` - * merged into the child's environment. Cleans up unconditionally. */ + * merged into the child's environment. Retains both roots when a child may still use them. */ async function inSandbox(prefix, extraEnv, fn) { const home = fs.mkdtempSync(path.join(os.tmpdir(), `${prefix}-home-`)); fs.mkdirSync(path.join(home, '.config'), { recursive: true }); @@ -50,8 +71,10 @@ async function inSandbox(prefix, extraEnv, fn) { }); let result; let failure; - try { result = await fn({ home, project, env }); } + let retained = false; + try { result = await fn({ home, project, env, retain: () => { retained = true; } }); } catch (error) { failure = error; } + if (retained) throw failure ?? new Error(`retained uncertain sandbox: ${home}, ${project}`); for (const root of [home, project]) { try { fs.rmSync(root, { recursive: true, force: true }); } catch (error) { failure = failure ? new AggregateError([failure, error], 'sandbox assertion and cleanup failed') : error; } @@ -72,13 +95,17 @@ test('spawn-guard is a no-op when AK_SPAWN_LEDGER_FILE is unset', async () => { }); }); -test('spawn-guard records every wrapped form and waits for its owned fork', async () => { - await inSandbox('ak-spawn-guard-smoke', {}, async ({ project, env: baseEnv }) => { +async function runGuardedSmoke({ childExitCode = 0, childStallsOnRelease = false, + closeTimeoutMs = 5_000, launchFailure = false } = {}) { + await inSandbox('ak-spawn-guard-smoke', {}, async ({ project, env: baseEnv, retain }) => { const ledgerFile = path.join(project, 'spawn-ledger.ndjson'); const readyFile = path.join(project, 'fork-ready'); const releaseFile = path.join(project, 'fork-release'); const doneFile = path.join(project, 'fork-done'); const pidFile = path.join(project, 'fork-pid'); + const attemptFile = path.join(project, 'fork-attempt'); + const waitingFile = path.join(project, 'parent-waiting-for-fork'); + const parentAckFile = path.join(project, 'fork-closed-by-parent'); const env = { ...baseEnv, AK_SPAWN_LEDGER_FILE: ledgerFile }; const forkTarget = path.join(project, 'fork-target.cjs'); fs.writeFileSync(forkTarget, [ @@ -88,8 +115,9 @@ test('spawn-guard records every wrapped form and waits for its owned fork', asyn 'const timer = setInterval(() => {', ' if (!fs.existsSync(release)) return;', ` fs.writeFileSync(${JSON.stringify(doneFile)}, 'done');`, + ...(childStallsOnRelease ? [' return;'] : []), ' clearInterval(timer);', - ' process.exit(0);', + ` process.exit(${childExitCode});`, '}, 10);', ].join('\n')); const script = [ @@ -98,20 +126,26 @@ test('spawn-guard records every wrapped form and waits for its owned fork', asyn "spawnSync(process.execPath, ['-e', '0']);", "ef(process.execPath, ['-e', '0']);", 'try { es(\'true\'); } catch {}', // shell builtin: exercised even with PATH broken + `require('node:fs').writeFileSync(${JSON.stringify(attemptFile)}, 'attempt');`, `const child = fork(${JSON.stringify(forkTarget)}, [], { stdio: 'ignore' });`, `require('node:fs').writeFileSync(${JSON.stringify(pidFile)}, String(child.pid));`, + `require('node:fs').writeFileSync(${JSON.stringify(waitingFile)}, 'waiting');`, 'await new Promise((resolve, reject) => {', ' child.once("error", reject);', ' child.once("close", (code, signal) => code === 0 && !signal', ' ? resolve() : reject(new Error(`fork failed: code=${code}, signal=${signal}`)));', '});', + 'if (child.exitCode !== 0 || child.signalCode) throw new Error("fork has not exited cleanly");', + `require('node:fs').writeFileSync(${JSON.stringify(parentAckFile)}, 'fork closed');`, '}', 'main().catch((error) => { console.error(error); process.exitCode = 1; });', ].join(' '); - const guarded = spawn(process.execPath, [ + const guarded = spawn(launchFailure ? path.join(project, 'missing-node') : process.execPath, [ `--import=${SPAWN_GUARD_URL}`, '-e', script, ], { cwd: project, env, stdio: ['ignore', 'ignore', 'pipe'] }); let stderr = ''; + let launchError; + guarded.once('error', (error) => { launchError = error; }); guarded.stderr.on('data', (chunk) => { stderr += chunk; }); let closed = false; const parentClose = new Promise((resolve) => guarded.once('close', (code, signal) => { @@ -120,40 +154,55 @@ test('spawn-guard records every wrapped form and waits for its owned fork', asyn })); let failure; try { - const deadline = Date.now() + 5_000; - while (!fs.existsSync(readyFile) && Date.now() < deadline) { - await new Promise((resolve) => setTimeout(resolve, 10)); - } + await waitUntil(() => (fs.existsSync(waitingFile) && fs.existsSync(readyFile)) || closed || launchError); + if (launchError) throw launchError; + assert.ok(fs.existsSync(waitingFile), `guarded parent did not reach fork wait: ${stderr}`); assert.ok(fs.existsSync(readyFile), `fork did not become ready: ${stderr}`); - assert.strictEqual(closed, false, 'guarded parent exited while its owned fork was still running'); + // A premature parent may write its acknowledgment or close on a later + // event-loop turn. Give those events time to surface before release. + await waitUntil(() => fs.existsSync(parentAckFile) || closed, 300); + assert.ok(!fs.existsSync(parentAckFile), 'parent acknowledged fork close before child release'); + assert.ok(!closed, 'guarded parent exited while its owned fork was still running'); } catch (error) { failure = error; } - try { - fs.writeFileSync(releaseFile, 'release'); - const deadline = Date.now() + 5_000; - while (!fs.existsSync(doneFile) && Date.now() < deadline) await new Promise((resolve) => setTimeout(resolve, 10)); - if (!fs.existsSync(doneFile) && fs.existsSync(pidFile)) { - const pid = Number(fs.readFileSync(pidFile, 'utf8')); - if (Number.isInteger(pid) && pid > 0) { try { process.kill(pid); } catch { /* already exited */ } } + const cleanupErrors = []; + let parentExited = closed; + let pid = NaN; + try { fs.writeFileSync(releaseFile, 'release'); } catch (error) { cleanupErrors.push(error); } + try { pid = fs.existsSync(pidFile) ? Number(fs.readFileSync(pidFile, 'utf8')) : NaN; } + catch (error) { cleanupErrors.push(error); } + try { await waitFor(parentClose, closeTimeoutMs); parentExited = true; } + catch { + // Keep the guarded parent alive to receive its own fork's close event. + if (Number.isInteger(pid) && pid > 0) { + try { process.kill(pid); } catch (error) { if (error.code !== 'ESRCH') cleanupErrors.push(error); } } - if (!fs.existsSync(doneFile) && !closed) guarded.kill(); - let timeout; - try { - await Promise.race([parentClose, new Promise((_, reject) => { - timeout = setTimeout(() => reject(new Error('guarded parent did not close')), 5_000); - })]); - } finally { clearTimeout(timeout); } - } catch (error) { - if (fs.existsSync(pidFile)) { - const pid = Number(fs.readFileSync(pidFile, 'utf8')); - if (Number.isInteger(pid) && pid > 0) { try { process.kill(pid); } catch { /* already exited */ } } + try { await waitFor(parentClose, closeTimeoutMs); parentExited = true; } + catch { + if (!closed) guarded.kill(); + try { await waitFor(parentClose, closeTimeoutMs); parentExited = true; } + catch (error) { cleanupErrors.push(error); } } - if (!closed) guarded.kill(); - failure = failure ? new AggregateError([failure, error], 'fork assertion and cleanup failed') : error; } + parentExited ||= closed; + // The acknowledgment is written only after the guarded parent receives + // its fork's close event. Otherwise require independent OS exit evidence. + let forkExited = fs.existsSync(parentAckFile) || (!fs.existsSync(attemptFile) && parentExited) + || await waitUntil(() => processGone(pid)); + if (!forkExited && Number.isInteger(pid) && pid > 0) { + try { process.kill(pid); } catch (error) { if (error.code !== 'ESRCH') cleanupErrors.push(error); } + forkExited = await waitUntil(() => processGone(pid)); + } + if (!forkExited) cleanupErrors.push(new Error(`cannot establish owned fork exit: pid=${pid}`)); + if (!parentExited) cleanupErrors.push(new Error('cannot establish guarded parent exit')); + if (!parentExited || !forkExited) retain(); + if (cleanupErrors.length) failure = failure + ? new AggregateError([failure, ...cleanupErrors], 'fork assertion and cleanup failed') + : new AggregateError(cleanupErrors, 'fork cleanup failed'); if (failure) throw failure; assert.ok(fs.existsSync(doneFile), 'owned fork completed before sandbox cleanup'); const { code, signal } = await parentClose; assert.strictEqual(code, 0, `guarded parent failed (${signal}): ${stderr}`); + assert.ok(fs.existsSync(parentAckFile), 'guarded parent did not acknowledge the fork close event'); const lines = readLedger(ledgerFile); // Each wrapped form by name, not an exact total: execSync goes through a // platform shell, and only its own record is what this test is about. @@ -164,6 +213,22 @@ test('spawn-guard records every wrapped form and waits for its owned fork', asyn assert.ok(lines.some((l) => l.cmd === forkTarget), `fork() records the module path as cmd; got ${got}`); assert.ok(lines.every((l) => typeof l.at === 'string' && !Number.isNaN(Date.parse(l.at))), 'every line has an ISO timestamp'); }); +} + +test('spawn-guard records every wrapped form and waits for its owned fork', async () => { + await runGuardedSmoke(); +}); + +test('nonzero owned fork exit fails after the guarded parent reaps it', async () => { + await assert.rejects(runGuardedSmoke({ childExitCode: 7 }), /guarded parent failed/); +}); + +test('stalled owned fork is signaled before its parent is reaped', async () => { + await assert.rejects(runGuardedSmoke({ childStallsOnRelease: true, closeTimeoutMs: 200 }), /guarded parent failed/); +}); + +test('guarded launch failure is reported and its sandbox is cleaned', async () => { + await assert.rejects(runGuardedSmoke({ launchFailure: true }), { code: 'ENOENT' }); }); // A ledger line from the fixture's own npm registry lookups From 1fe3e61ef49206f4f2185d2c8d3155132cde830e Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Tue, 29 Sep 2026 01:55:19 -0700 Subject: [PATCH 15/20] test(runner): retain own root for unresolved child holds --- AGENTS.md | 7 ++- docs/plans/2026-09-28-runner-hygiene.md | 2 + scripts/run-roots.mjs | 60 +++++++++++++++++- scripts/run-tests.mjs | 22 +++++-- tests/kit/run-roots.test.mjs | 33 +++++++++- tests/kit/run-tests-runner.test.mjs | 82 +++++++++++++++++++++++++ tests/kit/status-zero-spawn.test.mjs | 7 +++ 7 files changed, 204 insertions(+), 9 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 120a8109..8f613e30 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -334,10 +334,13 @@ them, and `sandboxHome()` and `redirectToolState()` do the same for in-process c Code's own `~/.claude.json`) are listed as "concurrent writers" and do not fail a local run; CI (or `AK_TRIPWIRE_STRICT=1`) fails on them too. Every command also runs with `TMPDIR`/`TEMP`/`TMP` pointed at a fresh `ak-suite-*` folder: anything left in it afterwards fails the run and is -listed (excluding its private atomic `.ak-suite-owner.json` and Node compile cache). The runner +listed (excluding its private atomic `.ak-suite-owner.json`, child-hold directory and Node compile cache). The runner refuses home/filesystem-root temp bases before allocation and refuses roots inside a git repository (point `TMPDIR` elsewhere). A completed run removes only its own validated direct, -canonical, nonsymlink, current-owner root. It then lists sibling suite roots: missing, invalid, +canonical, nonsymlink, current-owner root. Tests with known child lifetime uncertainty acquire +`acquireRunRootHold()` before launching those children and release only after proving their exits. +An unresolved or unreadable hold retains the own root; it is not a general descendant-exit proof. +The runner then lists sibling suite roots: missing, invalid, foreign or uncertain owner metadata means keep. Sibling handling is list-only on macOS, Linux and Windows because no installed probe proves all descendants have exited; even a dead owner is insufficient. Interrupted runs remove and collect nothing. Sibling listing/collection errors diff --git a/docs/plans/2026-09-28-runner-hygiene.md b/docs/plans/2026-09-28-runner-hygiene.md index 5c5b499f..ff08bf2f 100644 --- a/docs/plans/2026-09-28-runner-hygiene.md +++ b/docs/plans/2026-09-28-runner-hygiene.md @@ -8,6 +8,8 @@ LQ1 source inspection refutes the claimed missing selector propagation: the runn The Windows smoke lifetime follow-up uses a held fork and parent close acknowledgment. The smoke waits for the fork's `close` or `error`, checks its exit status, and establishes both exits before sandbox removal; uncertain exits retain the sandbox. A bounded pre-release observation detects early parent exit, and a local removed-wait mutation fails the focused test. The guarded suite also covers fork failure, stalled-fork cleanup, and launch failure. This establishes the lifecycle gap locally; native Windows CI remains the required platform proof for the reported `EPERM`. +Round 2 review found that local sandbox retention alone did not stop the guarded runner from removing its own root after a failing test. The runner now prepares a private hold directory before running commands. The smoke acquires a run-bound hold before launching its owned processes and releases it only after both exits are established. An unresolved or unreadable hold retains the own root, while clean runs and ordinary failures still remove it. Guarded tests prove retention with a live bounded child, parallel holds, and exit precedence. Native Windows CI is still pending. + ## Contract and dependencies The [v2 scope](2026-09-28-remediation-program-v2.md#v5-testrunner-hygiene-suites-that-clean-up-after-themselves) inherits [archived Branch 9](../archive/2026-09-28-superpowers-plan-branch-9-follow-ups.md) Tasks 5, 7 and 10–13, under B9-R1–R8. V1 is integrated at the baseline. V4/V6 must coordinate before changing environment-helper consumers. Worktree ownership is limited to this branch; shared manifests remain the integration owner's responsibility. diff --git a/scripts/run-roots.mjs b/scripts/run-roots.mjs index 2bcbd358..a077cec5 100644 --- a/scripts/run-roots.mjs +++ b/scripts/run-roots.mjs @@ -7,7 +7,8 @@ import { randomUUID } from 'node:crypto'; export const RUN_ROOT_NAME = /^ak-suite-[A-Za-z0-9]{6}$/; export const OWNER_FILE = '.ak-suite-owner.json'; -export const IGNORED_IN_ROOT = new Set(['node-compile-cache', OWNER_FILE]); +export const HOLD_DIR = '.ak-suite-holds'; +export const IGNORED_IN_ROOT = new Set(['node-compile-cache', OWNER_FILE, HOLD_DIR]); const MAX_OWNER_BYTES = 8192; const currentUid = () => process.getuid?.() ?? null; @@ -66,6 +67,63 @@ export function readOwner(root) { return record; } +/** Create the private hold directory before launching any suite command. */ +export function prepareRunRootHolds(root, runId) { + if (readOwner(root)?.runId !== runId || fs.realpathSync(root) !== root) throw Error('run root owner mismatch'); + fs.mkdirSync(path.join(root, HOLD_DIR), { mode: 0o700 }); +} + +/** A command acquires this hold before launching children that use its run root. + * Outside a guarded run it returns null; local sandbox retention still applies. + * @param {{env?:NodeJS.ProcessEnv}} [options] + */ +export function acquireRunRootHold({ env = process.env } = {}) { + const root = env.AK_SUITE_ROOT; + const runId = env.AK_SUITE_RUN_ID; + if (root === undefined && runId === undefined) return null; + if (!root || !runId || readOwner(root)?.runId !== runId || fs.realpathSync(root) !== root) { + throw Error('cannot establish run-root hold: owner mismatch'); + } + const dir = path.join(root, HOLD_DIR); + const stat = fs.lstatSync(dir); + if (!stat.isDirectory() || stat.isSymbolicLink() || fs.realpathSync(dir) !== dir) { + throw Error('cannot establish run-root hold: unsafe hold directory'); + } + const id = randomUUID(); + const token = randomUUID(); + const file = path.join(dir, id); + fs.writeFileSync(file, token, { flag: 'wx', mode: 0o600 }); + return { root, runId, file, token, pid: process.pid }; +} + +/** Remove only the marker returned to this process by acquireRunRootHold. */ +export function releaseRunRootHold(hold) { + if (hold === null) return; + if (!hold || hold.pid !== process.pid || readOwner(hold.root)?.runId !== hold.runId + || fs.realpathSync(hold.root) !== hold.root + || path.dirname(hold.file) !== path.join(hold.root, HOLD_DIR)) throw Error('run-root hold owner mismatch'); + const dir = path.join(hold.root, HOLD_DIR); + const parent = fs.lstatSync(dir); + if (!parent.isDirectory() || parent.isSymbolicLink() || fs.realpathSync(dir) !== dir) { + throw Error('run-root hold directory changed'); + } + const stat = fs.lstatSync(hold.file); + if (!stat.isFile() || stat.isSymbolicLink() || stat.nlink !== 1 + || fs.readFileSync(hold.file, 'utf8') !== hold.token) throw Error('run-root hold changed'); + fs.unlinkSync(hold.file); +} + +/** Missing or unreadable hold state is uncertainty, never permission to remove. */ +export function inspectRunRootHolds(root, runId) { + try { + if (readOwner(root)?.runId !== runId) throw Error('owner changed'); + const dir = path.join(root, HOLD_DIR); + const stat = fs.lstatSync(dir); + if (!stat.isDirectory() || stat.isSymbolicLink() || fs.realpathSync(dir) !== dir) throw Error('unsafe hold directory'); + return { unresolved: fs.readdirSync(dir).length > 0, reason: 'unresolved child hold' }; + } catch { return { unresolved: true, reason: 'hold inspection uncertain' }; } +} + /** Pure path check also accepts Windows paths in cross-platform unit fixtures. */ export function unsafeTempBase(tmpdir, homedir) { const windows = path.win32.isAbsolute(tmpdir) && !path.posix.isAbsolute(tmpdir); diff --git a/scripts/run-tests.mjs b/scripts/run-tests.mjs index 6ef90a01..166a5324 100644 --- a/scripts/run-tests.mjs +++ b/scripts/run-tests.mjs @@ -9,7 +9,8 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; -import { ownerRecord, writeOwner, unsafeTempBase, removableRunRoot, collectAbandonedRoots, IGNORED_IN_ROOT } from './run-roots.mjs'; +import { ownerRecord, writeOwner, prepareRunRootHolds, inspectRunRootHolds, + unsafeTempBase, removableRunRoot, collectAbandonedRoots, IGNORED_IN_ROOT } from './run-roots.mjs'; import { realStateRoots, snapshotRoots, compareSnapshots, isStrict, formatReport } from './real-state-tripwire.mjs'; const REPO = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); @@ -59,7 +60,8 @@ export function runGuarded(commands, { const unsafe = unsafeTempBase(tmpdir, fs.realpathSync(homedir)); if (unsafe) { log(`unsafe temp base ${tmpdir}: ${unsafe}`); return 2; } const tempRoot = fs.mkdtempSync(path.join(tmpdir, 'ak-suite-')); - try { writeOwner(tempRoot, ownerRecord()); } + const owner = ownerRecord(); + try { writeOwner(tempRoot, owner); } catch (error) { log(`could not record run owner; kept run root ${tempRoot}: ${error.message}`); return 2; } const identity = fs.lstatSync(tempRoot); const removeOwnRoot = () => { @@ -86,8 +88,11 @@ export function runGuarded(commands, { + 'git repository" would write into it. Point TMPDIR outside any repository.'); return 2; } + try { prepareRunRootHolds(tempRoot, owner.runId); } + catch (error) { log(`could not prepare run-root holds; kept ${tempRoot}: ${error.message}`); return 2; } /** @type {NodeJS.ProcessEnv} */ - const childEnv = { ...env, TMPDIR: tempRoot, TEMP: tempRoot, TMP: tempRoot }; + const childEnv = { ...env, TMPDIR: tempRoot, TEMP: tempRoot, TMP: tempRoot, + AK_SUITE_ROOT: tempRoot, AK_SUITE_RUN_ID: owner.runId }; // Tests assert on plain text; a shell's FORCE_COLOR (Claude Code sets 3) // colours console.log into pipes and, beside NO_COLOR, adds a Node warning. delete childEnv.FORCE_COLOR; @@ -109,8 +114,15 @@ export function runGuarded(commands, { log(`could not list own run root; kept ${tempRoot}: ${error.message}`); ownHygieneFailed = true; } - // Unknown contents must remain available for inspection, never count as clean. - if (!ownHygieneFailed && !removeOwnRoot()) ownHygieneFailed = true; + // A child may have explicitly declared unresolved ownership before launch. + // Ordinary test failures still remove their own roots when all holds clear. + if (!ownHygieneFailed) { + const holds = inspectRunRootHolds(tempRoot, owner.runId); + if (holds.unresolved) { + log(`kept own run root ${tempRoot}: ${holds.reason}`); + ownHygieneFailed = true; + } else if (!removeOwnRoot()) ownHygieneFailed = true; + } try { collectAbandonedRoots({ tmpdir, selfRoot: tempRoot, homedir, log }); } catch (error) { log(`could not list sibling run roots: ${error.message}`); } if (leftovers.length) log(`temp folders left behind by the run (${leftovers.length}):\n ${leftovers.join('\n ')}`); diff --git a/tests/kit/run-roots.test.mjs b/tests/kit/run-roots.test.mjs index d52a80e2..ae0b1154 100644 --- a/tests/kit/run-roots.test.mjs +++ b/tests/kit/run-roots.test.mjs @@ -4,7 +4,8 @@ import fs from 'node:fs'; import path from 'node:path'; import { tempDir } from './helpers/temp-dir.mjs'; import { ownerRecord, writeOwner, readOwner, unsafeTempBase, removableRunRoot, - proveAbandoned, collectAbandonedRoots, defaultProbes, OWNER_FILE } from '../../scripts/run-roots.mjs'; + proveAbandoned, collectAbandonedRoots, defaultProbes, OWNER_FILE, + prepareRunRootHolds, acquireRunRootHold, releaseRunRootHold, inspectRunRootHolds } from '../../scripts/run-roots.mjs'; const uid = process.getuid?.() ?? null; const complete = { alive: () => false, startedAfter: () => false, completeExit: () => true }; @@ -31,6 +32,36 @@ test('private atomic owner metadata round trips and does not leave staging data' if (process.platform !== 'win32') assert.equal(fs.statSync(path.join(f.root, OWNER_FILE)).mode & 0o777, 0o600); }); +test('parallel prelaunch holds release only their own marker; uncertain inspection retains', (t) => { + const f = fixture(t); + const owner = readOwner(f.root); + prepareRunRootHolds(f.root, owner.runId); + const env = { AK_SUITE_ROOT: f.root, AK_SUITE_RUN_ID: owner.runId }; + const first = acquireRunRootHold({ env }); + const second = acquireRunRootHold({ env }); + assert.equal(inspectRunRootHolds(f.root, owner.runId).unresolved, true); + assert.throws(() => releaseRunRootHold({ ...first, pid: 0 })); + const dir = path.join(f.root, '.ak-suite-holds'); + const saved = path.join(f.root, 'saved-holds'); + fs.renameSync(dir, saved); + try { + fs.symlinkSync(saved, dir, 'junction'); + assert.throws(() => releaseRunRootHold(first)); + } finally { + if (fs.existsSync(dir)) fs.unlinkSync(dir); + fs.renameSync(saved, dir); + } + releaseRunRootHold(first); + assert.equal(inspectRunRootHolds(f.root, owner.runId).unresolved, true); + releaseRunRootHold(second); + assert.equal(inspectRunRootHolds(f.root, owner.runId).unresolved, false); + assert.throws(() => acquireRunRootHold({ env: { ...env, AK_SUITE_RUN_ID: 'foreign' } })); + assert.equal(acquireRunRootHold({ env: {} }), null); + fs.rmSync(path.join(f.root, '.ak-suite-holds'), { recursive: true }); + assert.equal(inspectRunRootHolds(f.root, owner.runId).unresolved, true); + assert.throws(() => acquireRunRootHold({ env })); +}); + test('native defaults never prove abandonment, including dead owners and reused PIDs', (t) => { const f = fixture(t); for (const platform of ['darwin', 'linux', 'win32', 'other']) { diff --git a/tests/kit/run-tests-runner.test.mjs b/tests/kit/run-tests-runner.test.mjs index a36978df..45cbc621 100644 --- a/tests/kit/run-tests-runner.test.mjs +++ b/tests/kit/run-tests-runner.test.mjs @@ -24,6 +24,15 @@ function sandbox(t) { const stub = (dir, name, body) => { const f = path.join(dir, name); fs.writeFileSync(f, body); return f; }; +async function pidGone(pid, timeoutMs = 5_000) { + const end = Date.now() + timeoutMs; + while (Date.now() < end) { + try { process.kill(pid, 0); } catch (error) { if (error.code === 'ESRCH') return true; } + await new Promise((resolve) => setTimeout(resolve, 20)); + } + return false; +} + test('focus runs a literal clean test file through the guarded root', (t) => { const { home, env } = sandbox(t); const file = stub(home, 'clean.test.mjs', "import { test } from 'node:test'; test('clean', () => {});"); @@ -45,6 +54,79 @@ test('focus reports a leaked temp folder with hygiene exit code', (t) => { assert.match(r.stderr, /temp folders left behind.*focus-leak-/s); }); +test('an unresolved prelaunch hold retains the guarded root and sentinel after test failure', async (t) => { + const { home, repo, env } = sandbox(t); + const pidFile = path.join(home, 'held-child-pid'); + const child = stub(home, 'held-child.cjs', `const fs = require('node:fs'); + const release = process.argv[2]; + fs.writeFileSync(require('node:path').join(process.cwd(), 'held-child-ready'), 'ready'); + const timer = setInterval(() => { if (fs.existsSync(release)) { clearInterval(timer); process.exit(0); } }, 20); + setTimeout(() => process.exit(2), 10000);`); + const script = stub(home, 'unresolved-hold.mjs', `import fs from 'node:fs'; + import path from 'node:path'; import { spawn } from 'node:child_process'; + import { acquireRunRootHold } from ${JSON.stringify(new URL('../../scripts/run-roots.mjs', import.meta.url).href)}; + acquireRunRootHold(); + const root = process.env.AK_SUITE_ROOT; + fs.writeFileSync(path.join(root, 'held-sentinel'), 'keep'); + const child = spawn(process.execPath, [${JSON.stringify(child)}, path.join(root, 'release-child')], + { cwd: root, stdio: 'ignore', detached: true }); + child.once('error', (error) => { throw error; }); + fs.writeFileSync(${JSON.stringify(pidFile)}, String(child.pid)); + child.unref(); + const ready = path.join(root, 'held-child-ready'); + const deadline = Date.now() + 2000; + while (!fs.existsSync(ready) && Date.now() < deadline) await new Promise((resolve) => setTimeout(resolve, 10)); + if (!fs.existsSync(ready)) throw Error('fixture child did not become ready'); + process.exit(7);`); + let root; + let pid; + try { + const r = spawnSync(process.execPath, [RUNNER, 'exec', '--repo', repo, '--', script], { env, encoding: 'utf8' }); + const roots = fs.readdirSync(env.TMPDIR).filter((name) => /^ak-suite-[A-Za-z0-9]{6}$/.test(name)); + if (roots.length === 1) root = path.join(env.TMPDIR, roots[0]); + if (fs.existsSync(pidFile)) pid = Number(fs.readFileSync(pidFile, 'utf8')); + assert.equal(r.status, 7, r.stdout + r.stderr); + assert.match(r.stderr, /kept own run root .*unresolved child hold/); + assert.equal(roots.length, 1, r.stderr); + assert.equal(fs.readFileSync(path.join(root, 'held-sentinel'), 'utf8'), 'keep'); + assert.equal(fs.readFileSync(path.join(root, 'held-child-ready'), 'utf8'), 'ready'); + assert.doesNotThrow(() => process.kill(pid, 0), 'the held child should still own the retained cwd'); + } finally { + if (root) fs.writeFileSync(path.join(root, 'release-child'), 'release'); + if (Number.isInteger(pid) && pid > 0) { + if (!root) { try { process.kill(pid); } catch { /* already exited */ } } + if (!await pidGone(pid)) { try { process.kill(pid); } catch { /* already exited */ } } + assert.ok(await pidGone(pid), `fixture child ${pid} did not exit`); + } + if (root) fs.rmSync(root, { recursive: true, force: true }); + } +}); + +test('unresolved holds preserve command, tripwire, then hygiene exit priority', (t) => { + for (const [mode, expected] of [['command', 7], ['tripwire', 3], ['hygiene', 4]]) { + const { home, repo, env } = sandbox(t); + const script = stub(home, `${mode}-hold.mjs`, `import fs from 'node:fs'; import path from 'node:path'; + import { acquireRunRootHold } from ${JSON.stringify(new URL('../../scripts/run-roots.mjs', import.meta.url).href)}; + acquireRunRootHold(); + if (process.env.HOLD_MODE === 'tripwire') { + const file = path.join(process.env.XDG_CONFIG_HOME, 'agentic-kit', 'kit.json'); + fs.mkdirSync(path.dirname(file), { recursive: true }); fs.writeFileSync(file, '{}'); + } + if (process.env.HOLD_MODE === 'command') process.exit(7);`); + let root; + try { + const r = spawnSync(process.execPath, [RUNNER, 'exec', '--repo', repo, '--', script], { + env: { ...env, HOLD_MODE: mode }, encoding: 'utf8', + }); + const roots = fs.readdirSync(env.TMPDIR).filter((name) => /^ak-suite-[A-Za-z0-9]{6}$/.test(name)); + if (roots.length === 1) root = path.join(env.TMPDIR, roots[0]); + assert.equal(r.status, expected, r.stdout + r.stderr); + assert.match(r.stderr, /kept own run root .*unresolved child hold/); + assert.equal(roots.length, 1); + } finally { if (root) fs.rmSync(root, { recursive: true, force: true }); } + } +}); + test('focus rejects missing files and option-shaped filenames before creating roots', (t) => { const { env } = sandbox(t); const before = fs.readdirSync(env.TMPDIR); diff --git a/tests/kit/status-zero-spawn.test.mjs b/tests/kit/status-zero-spawn.test.mjs index 051897c0..e7a68d86 100644 --- a/tests/kit/status-zero-spawn.test.mjs +++ b/tests/kit/status-zero-spawn.test.mjs @@ -23,6 +23,7 @@ import os from 'node:os'; import path from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; import { spawnEnv, sandboxProject } from './helpers/home-sandbox.mjs'; +import { acquireRunRootHold, releaseRunRootHold } from '../../scripts/run-roots.mjs'; const HERE = path.dirname(fileURLToPath(import.meta.url)); const PKG_ROOT = path.resolve(HERE, '..', '..'); @@ -97,6 +98,9 @@ test('spawn-guard is a no-op when AK_SPAWN_LEDGER_FILE is unset', async () => { async function runGuardedSmoke({ childExitCode = 0, childStallsOnRelease = false, closeTimeoutMs = 5_000, launchFailure = false } = {}) { + // The guarded runner must know about uncertainty before any process uses + // its temp root. Standalone node --test has no runner hold to acquire. + const hold = acquireRunRootHold(); await inSandbox('ak-spawn-guard-smoke', {}, async ({ project, env: baseEnv, retain }) => { const ledgerFile = path.join(project, 'spawn-ledger.ndjson'); const readyFile = path.join(project, 'fork-ready'); @@ -195,6 +199,9 @@ async function runGuardedSmoke({ childExitCode = 0, childStallsOnRelease = false if (!forkExited) cleanupErrors.push(new Error(`cannot establish owned fork exit: pid=${pid}`)); if (!parentExited) cleanupErrors.push(new Error('cannot establish guarded parent exit')); if (!parentExited || !forkExited) retain(); + else { + try { releaseRunRootHold(hold); } catch (error) { cleanupErrors.push(error); } + } if (cleanupErrors.length) failure = failure ? new AggregateError([failure, ...cleanupErrors], 'fork assertion and cleanup failed') : new AggregateError(cleanupErrors, 'fork cleanup failed'); From e0fcc2eb78821071da406aa63ccdefeec5c6bc21 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Tue, 29 Sep 2026 02:08:50 -0700 Subject: [PATCH 16/20] test(runner): await owned process cleanup on cancellation --- tests/kit/helpers/interruption-scope.mjs | 92 +++++++++++++++++ tests/kit/helpers/temp-dir.mjs | 10 +- tests/kit/run-tests-cancellation.test.mjs | 116 ++++++++++++++++++++++ tests/kit/run-tests-interruption.test.mjs | 51 +++------- 4 files changed, 230 insertions(+), 39 deletions(-) create mode 100644 tests/kit/helpers/interruption-scope.mjs create mode 100644 tests/kit/run-tests-cancellation.test.mjs diff --git a/tests/kit/helpers/interruption-scope.mjs b/tests/kit/helpers/interruption-scope.mjs new file mode 100644 index 00000000..512fa0c6 --- /dev/null +++ b/tests/kit/helpers/interruption-scope.mjs @@ -0,0 +1,92 @@ +import fs from 'node:fs'; +import { tempDir } from './temp-dir.mjs'; +import { acquireRunRootHold, releaseRunRootHold } from '../../../scripts/run-roots.mjs'; + +export function childGone(pid) { + try { process.kill(pid, 0); return false; } + catch (error) { return error.code === 'ESRCH'; } +} + +export async function until(check, description, timeout = 5000, signal) { + const deadline = Date.now() + timeout; + while (Date.now() < deadline) { + signal?.throwIfAborted(); + const result = check(); + if (result) return result; + await new Promise((resolve) => setTimeout(resolve, 25)); + } + throw Error(`timed out waiting for ${description}`); +} + +async function stopPid(pid) { + if (childGone(pid)) return; + try { await until(() => childGone(pid), 'private child exit', 2000); return; } catch { /* Escalate exact PID only. */ } + for (const signal of ['SIGTERM', 'SIGKILL']) { + if (childGone(pid)) return; + try { process.kill(pid, signal); } catch (error) { if (error.code !== 'ESRCH') throw error; } + try { await until(() => childGone(pid), 'signaled child exit', 2000); return; } catch { /* Retain on uncertainty. */ } + } + throw Error(`cannot prove owned child ${pid} exited`); +} + +/** One hook owns process shutdown and conditional directory deletion. */ +export function interruptionScope(t, { beforeRemove = () => {} } = {}) { + const hold = acquireRunRootHold(); + const home = tempDir('ak-interrupt', undefined, { manual: true }); + const entries = []; + let cleaning; + let stopping = false; + const active = () => { + t.signal.throwIfAborted(); + if (stopping) throw Error('fixture cleanup has started'); + }; + const cleanup = () => { + stopping = true; + cleaning ??= (async () => { + const results = await Promise.allSettled(entries.map(async (entry) => { + if (entry.stop) fs.writeFileSync(entry.stop, 'exit'); + if (entry.handshake && !(entry.launchError && !entry.child.pid)) { + const data = await until(() => fs.existsSync(entry.handshake) + && JSON.parse(fs.readFileSync(entry.handshake, 'utf8')), 'owned child identity', 2000); + if (!Number.isSafeInteger(data.pid) || data.pid <= 0) throw Error('invalid owned child identity'); + await stopPid(data.pid); + } + if (!entry.closed) { + try { await until(() => entry.closed, 'runner close', 2000); } catch { + entry.child.kill('SIGTERM'); + try { await until(() => entry.closed, 'runner termination', 2000); } catch { + entry.child.kill('SIGKILL'); + await until(() => entry.closed, 'runner forced close', 2000); + } + } + } + if (entry.child.pid && !childGone(entry.child.pid)) throw Error('owned runner PID remains'); + })); + const errors = results.filter(r => r.status === 'rejected').map(r => r.reason); + if (errors.length) throw new AggregateError(errors, 'owned process exit uncertain; retaining fixture and run root'); + releaseRunRootHold(hold); + })(); + return cleaning; + }; + const abort = () => { void cleanup().catch(() => {}); }; + t.after(async () => { + t.signal.removeEventListener('abort', abort); + await cleanup(); + beforeRemove(home); + fs.rmSync(home, { recursive: true, force: true, maxRetries: 3 }); + }); + t.signal.addEventListener('abort', abort, { once: true }); + return { + home, cleanup, active, + wait: (check, description, timeout) => until(check, description, timeout, t.signal), + launch(start, { handshake, stop } = {}) { + active(); + const child = start(); + const entry = { child, handshake, stop, closed: false, launchError: null }; + child.on('error', error => { entry.launchError = error; }); + child.once('close', () => { entry.closed = true; }); + entries.push(entry); + return child; + }, + }; +} diff --git a/tests/kit/helpers/temp-dir.mjs b/tests/kit/helpers/temp-dir.mjs index 817b4a67..70b2df28 100644 --- a/tests/kit/helpers/temp-dir.mjs +++ b/tests/kit/helpers/temp-dir.mjs @@ -8,14 +8,18 @@ import path from 'node:path'; /** * Create `/-XXXXXX` and remove it when the test (or, without - * `t`, the file) finishes. A caller that chdir-ed into it must chdir out first. + * `t`, the file) finishes, unless manual cleanup is requested. A caller that + * chdir-ed into it must chdir out first. * @param {string} prefix * @param {import('node:test').TestContext} [t] + * @param {{manual?:boolean}} [options] Caller owns removal when manual is true. * @returns {string} the real path of the new folder */ -export function tempDir(prefix, t) { +export function tempDir(prefix, t, { manual = false } = {}) { const dir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), `${prefix}-`))); const remove = () => fs.rmSync(dir, { recursive: true, force: true, maxRetries: 3 }); - if (t) t.after(remove); else after(remove); + if (!manual) { + if (t) t.after(remove); else after(remove); + } return dir; } diff --git a/tests/kit/run-tests-cancellation.test.mjs b/tests/kit/run-tests-cancellation.test.mjs new file mode 100644 index 00000000..6bba76ad --- /dev/null +++ b/tests/kit/run-tests-cancellation.test.mjs @@ -0,0 +1,116 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { spawnSync } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; +import { tempDir } from './helpers/temp-dir.mjs'; +import { spawnEnv } from './helpers/home-sandbox.mjs'; + +const runner = fileURLToPath(new URL('../../scripts/run-tests.mjs', import.meta.url)); +const sandbox = new URL('./helpers/home-sandbox.mjs', import.meta.url).href; +const helper = new URL('./helpers/interruption-scope.mjs', import.meta.url).href; + +test('real test timeout closes owned runner and orphan before cwd removal and releases the outer hold', t => { + const home = tempDir('ak-cancel-check', t); + const evidence = path.join(home, 'evidence.json'); + const fixture = path.join(home, 'cancel.test.mjs'); + fs.writeFileSync(fixture, ` + import { test, after } from 'node:test'; + import assert from 'node:assert/strict'; + import fs from 'node:fs'; + import { spawn } from 'node:child_process'; + import { spawnEnv } from ${JSON.stringify(sandbox)}; + import { interruptionScope, childGone, until } from ${JSON.stringify(helper)}; + let scope, child, closed = false, removed = false; + test('intentional cancellation', { timeout: 500 }, async t => { + scope = interruptionScope(t, { beforeRemove(home) { + assert.equal(closed, true, 'close must precede cwd removal'); + assert.equal(childGone(child.pid), true, 'OS must report ESRCH before removal'); + assert.equal(fs.existsSync(home), true); + const orphan = JSON.parse(fs.readFileSync(home + '/ready', 'utf8')); + assert.equal(childGone(orphan.pid), true, 'orphan must also have native ESRCH before removal'); + fs.writeFileSync(${JSON.stringify(evidence)}, JSON.stringify({ pid: child.pid, orphanPid: orphan.pid, closed, gone: childGone(child.pid), home })); + removed = true; + }}); + fs.writeFileSync(scope.home + '/hold.mjs', \` + import fs from 'node:fs'; + fs.writeFileSync(process.argv[2] + '.tmp', JSON.stringify({ pid: process.pid })); + fs.renameSync(process.argv[2] + '.tmp', process.argv[2]); + setInterval(() => { if (fs.existsSync(process.argv[3])) process.exit(0); }, 25); + setTimeout(() => process.exit(8), 10000); + \`); + const env = spawnEnv(scope.home); + delete env.NODE_TEST_CONTEXT; + child = scope.launch(() => spawn(process.execPath, [${JSON.stringify(runner)}, 'exec', '--repo', scope.home, '--', scope.home + '/hold.mjs', scope.home + '/ready', scope.home + '/stop'], { env, stdio: 'ignore' }), + { handshake: scope.home + '/ready', stop: scope.home + '/stop' }); + child.once('close', () => { closed = true; }); + try { + await until(() => fs.existsSync(scope.home + '/ready'), 'orphan readiness', 5000, t.signal); + child.kill('SIGTERM'); + await until(() => closed, 'runner close', 5000, t.signal); + await new Promise(resolve => t.signal.addEventListener('abort', resolve, { once: true })); + } + finally { + await scope.cleanup(); + let attempted = false; + assert.throws(() => scope.launch(() => { attempted = true; return child; })); + assert.equal(attempted, false, 'cancellation must block the launch callback'); + } + }); + after(() => { + assert.equal(removed, true); + assert.equal(fs.existsSync(scope.home), false); + }); + `); + const env = spawnEnv(home, { CI: 'true' }); + delete env.NODE_TEST_CONTEXT; + const result = spawnSync(process.execPath, [runner, 'exec', '--repo', home, '--', '--test', fixture], { + env, encoding: 'utf8', + }); + assert.equal(result.error, undefined); + assert.equal(result.status, 1, result.stdout + result.stderr); + assert.match(result.stdout + result.stderr, /test timed out after 500ms/); + const proof = JSON.parse(fs.readFileSync(evidence, 'utf8')); + assert.equal(proof.closed, true); + assert.equal(proof.gone, true); + assert.equal(fs.existsSync(proof.home), false); + assert.throws(() => process.kill(proof.pid, 0), { code: 'ESRCH' }); + assert.throws(() => process.kill(proof.orphanPid, 0), { code: 'ESRCH' }); + assert.deepEqual(fs.readdirSync(env.TMPDIR), [], 'guarded timeout has no unresolved hold or fixture leftovers'); +}); + +test('uncertain fixture identity retains its local directory and enclosing guarded root', t => { + const home = tempDir('ak-cancel-retain-check', t); + const evidence = path.join(home, 'evidence.json'); + const fixture = path.join(home, 'uncertain.test.mjs'); + fs.writeFileSync(fixture, ` + import { test } from 'node:test'; + import fs from 'node:fs'; + import { spawn } from 'node:child_process'; + import { interruptionScope } from ${JSON.stringify(helper)}; + test('intentional missing child identity', async t => { + const scope = interruptionScope(t); + const child = scope.launch(() => spawn(process.execPath, ['-e', 'process.exit(0)'], { cwd: scope.home, stdio: 'ignore' }), + { handshake: scope.home + '/missing-handshake', stop: scope.home + '/stop' }); + await new Promise(resolve => child.once('close', resolve)); + fs.writeFileSync(${JSON.stringify(evidence)}, JSON.stringify({ pid: child.pid, home: scope.home, root: process.env.AK_SUITE_ROOT })); + await scope.cleanup(); + }); + `); + const env = spawnEnv(home, { CI: 'true' }); + delete env.NODE_TEST_CONTEXT; + const result = spawnSync(process.execPath, [runner, 'exec', '--repo', home, '--', '--test', fixture], { env, encoding: 'utf8' }); + assert.equal(result.error, undefined); + assert.equal(result.status, 1, result.stdout + result.stderr); + assert.match(result.stdout + result.stderr, /owned process exit uncertain/); + assert.match(result.stderr, /unresolved child hold/); + const proof = JSON.parse(fs.readFileSync(evidence, 'utf8')); + assert.throws(() => process.kill(proof.pid, 0), { code: 'ESRCH' }); + assert.equal(fs.existsSync(proof.home), true); + assert.equal(fs.existsSync(proof.root), true); + assert.equal(fs.readdirSync(path.join(proof.root, '.ak-suite-holds')).length, 1); + // This enclosing test knows its exact fixture ran only process.exit(0), and + // independently established ESRCH above; remove only that disposable root. + fs.rmSync(proof.root, { recursive: true }); +}); diff --git a/tests/kit/run-tests-interruption.test.mjs b/tests/kit/run-tests-interruption.test.mjs index 7ef5d60e..2f36f5f8 100644 --- a/tests/kit/run-tests-interruption.test.mjs +++ b/tests/kit/run-tests-interruption.test.mjs @@ -4,32 +4,17 @@ import fs from 'node:fs'; import path from 'node:path'; import { spawn, spawnSync } from 'node:child_process'; import { fileURLToPath } from 'node:url'; -import { tempDir } from './helpers/temp-dir.mjs'; +import { interruptionScope, childGone } from './helpers/interruption-scope.mjs'; import { spawnEnv } from './helpers/home-sandbox.mjs'; const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..'); const RUNNER = path.join(ROOT, 'scripts', 'run-tests.mjs'); const ownerFile = '.ak-suite-owner.json'; -async function until(check, description, timeout = 5000) { - const deadline = Date.now() + timeout; - while (Date.now() < deadline) { - const result = check(); - if (result) return result; - await new Promise((resolve) => setTimeout(resolve, 25)); - } - throw Error(`timed out waiting for ${description}`); -} - function roots(parent) { return fs.readdirSync(parent).filter((name) => /^ak-suite-[A-Za-z0-9]{6}$/.test(name)).sort(); } -function childGone(pid) { - try { process.kill(pid, 0); return false; } - catch (error) { return error.code === 'ESRCH'; } -} - function runnerFor(repo, script, handshake, stop, done, env, log) { const fd = fs.openSync(log, 'w'); try { @@ -43,7 +28,8 @@ test('interrupted and live sibling roots stay listed, including after the orphan skip: process.platform === 'win32' && 'POSIX runner termination probe; Windows retains siblings by list-only policy', timeout: 20000, }, async (t) => { - const home = tempDir('ak-interrupt', t); + const scope = interruptionScope(t); + const { home, wait: until } = scope; const repo = path.join(home, 'repo'); fs.mkdirSync(path.join(repo, '.git'), { recursive: true }); const env = spawnEnv(home, { APPDATA: path.join(home, 'AppData', 'Roaming'), CI: 'true' }); @@ -53,7 +39,8 @@ test('interrupted and live sibling roots stay listed, including after the orphan const script = path.join(home, 'hold.mjs'); fs.writeFileSync(script, `import fs from 'node:fs'; const [handshake, stop, done] = process.argv.slice(2); - fs.writeFileSync(handshake, JSON.stringify({ pid: process.pid, cwd: process.cwd(), tmpdir: process.env.TMPDIR })); + fs.writeFileSync(handshake + '.tmp', JSON.stringify({ pid: process.pid, cwd: process.cwd(), tmpdir: process.env.TMPDIR })); + fs.renameSync(handshake + '.tmp', handshake); const timer = setInterval(() => { if (!fs.existsSync(stop)) return; const request = fs.readFileSync(stop, 'utf8'); @@ -69,7 +56,8 @@ test('interrupted and live sibling roots stay listed, including after the orphan let runner4; let interruptedRoot; try { - runner1 = runnerFor(repo, script, ...first, env, path.join(home, 'run1.log')); + runner1 = scope.launch(() => runnerFor(repo, script, ...first, env, path.join(home, 'run1.log')), + { handshake: first[0], stop: first[1] }); const firstData = await until(() => fs.existsSync(first[0]) && JSON.parse(fs.readFileSync(first[0], 'utf8')), 'first child handshake'); const root1 = await until(() => roots(parent).map((name) => path.join(parent, name)).find((root) => { try { return JSON.parse(fs.readFileSync(path.join(root, ownerFile), 'utf8')).pid === runner1.pid; } @@ -84,13 +72,17 @@ test('interrupted and live sibling roots stay listed, including after the orphan assert.ok(fs.existsSync(root1)); assert.equal(fs.existsSync(first[2]), false, 'orphan has not stopped'); - const runFocus = () => spawnSync(process.execPath, [RUNNER, 'focus', clean], { env, encoding: 'utf8' }); + const runFocus = () => { + scope.active(); + return spawnSync(process.execPath, [RUNNER, 'focus', clean], { env, encoding: 'utf8' }); + }; const second = runFocus(); assert.equal(second.status, 0, second.stdout + second.stderr); assert.match(second.stderr, /kept run root.*cannot prove complete descendant exit \(list-only\)/); assert.deepEqual(roots(parent), [path.basename(root1)]); - runner4 = runnerFor(repo, script, ...live, env, path.join(home, 'run4.log')); + runner4 = scope.launch(() => runnerFor(repo, script, ...live, env, path.join(home, 'run4.log')), + { handshake: live[0], stop: live[1] }); const liveData = await until(() => fs.existsSync(live[0]) && JSON.parse(fs.readFileSync(live[0], 'utf8')), 'live child handshake'); const root4 = await until(() => roots(parent).map((name) => path.join(parent, name)).find((root) => { try { return JSON.parse(fs.readFileSync(path.join(root, ownerFile), 'utf8')).pid === runner4.pid; } @@ -110,22 +102,9 @@ test('interrupted and live sibling roots stay listed, including after the orphan assert.ok(fs.existsSync(root1), 'known stopped child does not authorize sibling removal'); assert.ok(fs.existsSync(root4), 'concurrently live sibling remains'); } finally { - // Stop only children created by this fixture, through their private channels. - fs.writeFileSync(first[1], 'exit'); - fs.writeFileSync(live[1], 'exit'); - if (runner1 && runner1.exitCode === null && runner1.signalCode === null) runner1.kill('SIGTERM'); - if (runner4 && runner4.exitCode === null && runner4.signalCode === null) { - await until(() => runner4.exitCode !== null || runner4.signalCode !== null, 'live runner completion', 14000); - } - if (fs.existsSync(first[0])) { - const { pid } = JSON.parse(fs.readFileSync(first[0], 'utf8')); - await until(() => childGone(pid), 'first child exit', 14000); - } - if (fs.existsSync(live[0])) { - const { pid } = JSON.parse(fs.readFileSync(live[0], 'utf8')); - await until(() => childGone(pid), 'live child exit', 14000); - } + await scope.cleanup(); } + if (t.signal.aborted) return; // Only this test's disposable fixture root is removed after its child stops. assert.deepEqual(roots(parent), [path.basename(interruptedRoot)]); fs.rmSync(interruptedRoot, { recursive: true }); From c9baf6a42a730b1e305950e55c1b81e0a49e2f8c Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Tue, 29 Sep 2026 02:30:49 -0700 Subject: [PATCH 17/20] docs(archive): record completed runner hygiene work --- .../2026-09-28-plan-runner-hygiene.md} | 37 ++++++++++++++----- ...9-28-research-test-temp-folder-cleanup.md} | 10 +++-- docs/archive/README.md | 2 + 3 files changed, 37 insertions(+), 12 deletions(-) rename docs/{plans/2026-09-28-runner-hygiene.md => archive/2026-09-28-plan-runner-hygiene.md} (58%) rename docs/{plans/2026-09-28-test-temp-folder-cleanup-design.md => archive/2026-09-28-research-test-temp-folder-cleanup.md} (97%) diff --git a/docs/plans/2026-09-28-runner-hygiene.md b/docs/archive/2026-09-28-plan-runner-hygiene.md similarity index 58% rename from docs/plans/2026-09-28-runner-hygiene.md rename to docs/archive/2026-09-28-plan-runner-hygiene.md index ff08bf2f..1385f29b 100644 --- a/docs/plans/2026-09-28-runner-hygiene.md +++ b/docs/archive/2026-09-28-plan-runner-hygiene.md @@ -2,19 +2,38 @@ ## Status -Research drafted against `develop` commit `e2f9dcae0554ff63921df618a819fd5e6afe80d2` on `test/runner-hygiene`. Tasks 10/5/7 and Task 11 were independently accepted. Task 12 adds guarded `focus ` and a bounded POSIX interrupted-run proof: a completed focus run retains an interrupted root while its child lives, after that child exits, and while another runner is live. Default sibling handling remains list-only on every platform; native Windows/Linux behavior is unmeasured. No backlog removal, push, PR or merge has occurred. The creator census is complete: 647 sites classified, zero unresolved current lifecycles. - -LQ1 source inspection refutes the claimed missing selector propagation: the runner copies its input environment and removes only `FORCE_COLOR`. A guarded-child sentinel regression is green on the original implementation and fails when that copy is removed. LQ4 now uses an explicit Chrome launch environment with a private home and temp root; the local macOS system Chrome passed the full guarded UI suite (495 dashboard checks, 15 Node UI tests). Guarded unit, TypeScript and targeted ESLint gates passed. Native Windows and Linux Chrome behavior remains unmeasured. - -The Windows smoke lifetime follow-up uses a held fork and parent close acknowledgment. The smoke waits for the fork's `close` or `error`, checks its exit status, and establishes both exits before sandbox removal; uncertain exits retain the sandbox. A bounded pre-release observation detects early parent exit, and a local removed-wait mutation fails the focused test. The guarded suite also covers fork failure, stalled-fork cleanup, and launch failure. This establishes the lifecycle gap locally; native Windows CI remains the required platform proof for the reported `EPERM`. - -Round 2 review found that local sandbox retention alone did not stop the guarded runner from removing its own root after a failing test. The runner now prepares a private hold directory before running commands. The smoke acquires a run-bound hold before launching its owned processes and releases it only after both exits are established. An unresolved or unreadable hold retains the own root, while clean runs and ordinary failures still remove it. Guarded tests prove retention with a live bounded child, parallel holds, and exit precedence. Native Windows CI is still pending. +**Implemented and independently reviewed**, captured before PR integration on 2026-09-29. +Code head `e0fcc2eb` passed all local unit, UI, typecheck, lint, complexity, Markdown, +build and internal-link gates. Unit results: 5,947 passed, zero failed, six native +Windows-only skips; coverage 94.33% lines, 83.27% branches and 93.47% functions. +System Chrome passed 495 dashboard checks and 15 Node UI tests. Native Windows +and Linux Chrome proof remain the feature PR's CI gate at this capture point. + +Delivered: the About renderer regression, exact concurrent-writer exceptions, +owner records and guarded focused runs, list-only sibling handling on every platform, +Chrome environment isolation, tool-selector propagation regression, and owned-process +exit/cancellation checks. A participating test acquires a run-bound hold before launching +its children; uncertainty retains its own fixture and the enclosing run root. Holds do +not discover unregistered descendants or authorize sibling removal. + +Task 13's immutable inventory and literal-path list were independently reviewed against +snapshot `c4aa2015ccd66c47f99f9204d0444faac007e83e9a8b709ddcee04791819e6bb`: +33,978 entries before the runner cutoff, four afterward and three legacy ownerless roots; +27 unattributed, 21 recent and one lsof-matched entry were retained separately. The private +handoff contains machine paths and stays outside git. Age, prefix attribution and lsof +absence do not establish deletion safety. No backlog removal was performed. + +The source-bound research census recorded 647 sites in 279 files at its stated baseline; +it does not count later edits or prove historical leak causes. LQ-1's propagation-defect +premise was refuted, and a mutation-tested regression preserves the existing behavior. +Final review found one cancellation-order defect; `e0fcc2eb` fixed it and passed scoped +rereview. Releases, global installation and the aggregate main merge remain separately gated. ## Contract and dependencies -The [v2 scope](2026-09-28-remediation-program-v2.md#v5-testrunner-hygiene-suites-that-clean-up-after-themselves) inherits [archived Branch 9](../archive/2026-09-28-superpowers-plan-branch-9-follow-ups.md) Tasks 5, 7 and 10–13, under B9-R1–R8. V1 is integrated at the baseline. V4/V6 must coordinate before changing environment-helper consumers. Worktree ownership is limited to this branch; shared manifests remain the integration owner's responsibility. +The [v2 scope](../plans/2026-09-28-remediation-program-v2.md#v5-testrunner-hygiene-suites-that-clean-up-after-themselves) inherits [archived Branch 9](2026-09-28-superpowers-plan-branch-9-follow-ups.md) Tasks 5, 7 and 10–13, under B9-R1–R8. V1 is integrated at the baseline. V4/V6 must coordinate before changing environment-helper consumers. Worktree ownership is limited to this branch; shared manifests remain the integration owner's responsibility. -The [cleanup design](2026-09-28-test-temp-folder-cleanup-design.md) selects B9-R5's explicit list-only fallback. Task 11 must not interpret a dead PID, empty process group, empty registry or empty handle scan as proof of abandonment. Task 12 must keep the interrupted root even after its known child exits on list-only platforms. This conditions the archived example's removal assertion; it does not relax B9-R5. +The [cleanup design](2026-09-28-research-test-temp-folder-cleanup.md) selects B9-R5's explicit list-only fallback. Task 11 must not interpret a dead PID, empty process group, empty registry or empty handle scan as proof of abandonment. Task 12 must keep the interrupted root even after its known child exits on list-only platforms. This conditions the archived example's removal assertion; it does not relax B9-R5. ## File, test and dependency map diff --git a/docs/plans/2026-09-28-test-temp-folder-cleanup-design.md b/docs/archive/2026-09-28-research-test-temp-folder-cleanup.md similarity index 97% rename from docs/plans/2026-09-28-test-temp-folder-cleanup-design.md rename to docs/archive/2026-09-28-research-test-temp-folder-cleanup.md index 58e12eee..a4a58dc0 100644 --- a/docs/plans/2026-09-28-test-temp-folder-cleanup-design.md +++ b/docs/archive/2026-09-28-research-test-temp-folder-cleanup.md @@ -2,9 +2,13 @@ ## Status -Research snapshot: 2026-09-28, `e2f9dcae0554ff63921df618a819fd5e6afe80d2`, macOS Darwin 27.0.0, Node 26.4.0; Node 22.22.3 used for CLI checks. Proposed behavior is **list-only for abandoned sibling roots on macOS, Linux and Windows**. No candidate establishes complete descendant liveness. This uses B9-R5's explicit fallback, preserves B9-R1–R8, and introduces no native sweeper or deletion authority. +**Research complete.** The selected list-only policy was implemented and independently +reviewed in the [execution plan](2026-09-28-plan-runner-hygiene.md), through `e0fcc2eb`. +The remainder of this document preserves the initial research snapshot and its limits. -The [execution plan](2026-09-28-runner-hygiene.md) maps subsequent work. Raw commands, JSON results, full lexical census, counts and literal experiment paths are retained in ignored `.superpowers/sdd/2026-09-28-runner-hygiene/`. No production/test code changed. Current creator lifecycles are fully classified below; native Windows/Linux behavior remains explicitly unmeasured. +Research snapshot: 2026-09-28, `e2f9dcae0554ff63921df618a819fd5e6afe80d2`, macOS Darwin 27.0.0, Node 26.4.0; Node 22.22.3 used for CLI checks. Selected behavior is **list-only for abandoned sibling roots on macOS, Linux and Windows**. No candidate establishes complete descendant liveness. This uses B9-R5's explicit fallback, preserves B9-R1–R8, and introduces no native sweeper or deletion authority. + +The [execution plan](2026-09-28-plan-runner-hygiene.md) maps subsequent work. Raw commands, JSON results, full lexical census, counts and literal experiment paths are retained in ignored `.superpowers/sdd/2026-09-28-runner-hygiene/`. No production/test code changed during this research unit. Creator lifecycles at that baseline are fully classified below; native Windows/Linux behavior remains explicitly unmeasured. ## Verified runner and creator behavior @@ -36,7 +40,7 @@ Literal Windows temp paths in ruflo-memory-location tests are injected path-clas ## The fourteen reported post-runner leaks -The [archived premise table](../archive/2026-09-28-superpowers-plan-branch-9-follow-ups.md#premise-verification-done-by-the-planner-tasks-carry-the-evidence-forward) records fourteen newer folders by prefix. These are historical observations, not fourteen reproduced failures today. +The [archived premise table](2026-09-28-superpowers-plan-branch-9-follow-ups.md#premise-verification-done-by-the-planner-tasks-carry-the-evidence-forward) records fourteen newer folders by prefix. These are historical observations, not fourteen reproduced failures today. | Historical entries | Attributed creator and cleanup | What can be concluded | |---|---|---| diff --git a/docs/archive/README.md b/docs/archive/README.md index d3a40b56..e4c03435 100644 --- a/docs/archive/README.md +++ b/docs/archive/README.md @@ -154,6 +154,8 @@ reconfirmed by this metadata audit. The per-file inventory and limitations are r | [2026-09-27-superpowers-plan-branch-4b-upstream-watch-actions.md](2026-09-27-superpowers-plan-branch-4b-upstream-watch-actions.md) | `docs/superpowers/plans/2026-09-27-branch-4b-upstream-watch-actions.md` | Finished Superpowers plan | Implemented work record preserved after its implementing PR merged. | | [2026-09-27-superpowers-plan-branch-5-aqe-store-integrity.md](2026-09-27-superpowers-plan-branch-5-aqe-store-integrity.md) | `docs/superpowers/plans/2026-09-27-branch-5-aqe-store-integrity.md` | Finished Superpowers plan | Implemented work record preserved after its implementing PR merged. | | [2026-09-27-superpowers-plan-branch-6a-evidence-store.md](2026-09-27-superpowers-plan-branch-6a-evidence-store.md) | `docs/superpowers/plans/2026-09-27-branch-6a-evidence-store.md` | Finished Superpowers plan | Implemented work record preserved after its implementing PR merged. | +| [2026-09-28-plan-runner-hygiene.md](2026-09-28-plan-runner-hygiene.md) | `docs/plans/2026-09-28-runner-hygiene.md` | Completed V5 execution plan | Local implementation and independent review through `e0fcc2eb`; feature CI and integration tracked by the PR. Private backlog list is a manual review aid, not deletion authority. | +| [2026-09-28-research-test-temp-folder-cleanup.md](2026-09-28-research-test-temp-folder-cleanup.md) | `docs/plans/2026-09-28-test-temp-folder-cleanup-design.md` | Runner cleanup research | Source-bound creator census and experiments supporting list-only sibling handling; no complete native descendant proof or cleanup authority. | | [2026-09-28-superpowers-plan-branch-6b-one-refresh-flag.md](2026-09-28-superpowers-plan-branch-6b-one-refresh-flag.md) | `docs/superpowers/plans/2026-09-28-branch-6b-one-refresh-flag.md` | Finished Superpowers plan | Implemented work record preserved after its implementing PR merged. | | [2026-09-28-superpowers-plan-branch-9-follow-ups.md](2026-09-28-superpowers-plan-branch-9-follow-ups.md) | `docs/superpowers/plans/2026-09-28-branch-9-follow-ups.md` | Finished Superpowers plan | Implemented work record preserved after its implementing PR merged. | | [2026-09-28-superpowers-plan-upstream-watch-ledger-branch.md](2026-09-28-superpowers-plan-upstream-watch-ledger-branch.md) | `docs/superpowers/plans/2026-09-28-upstream-watch-ledger-branch.md` | Finished Superpowers plan | Implemented work record preserved after its implementing PR merged. | From 5afaa4bb020a64a46bca4fbd5eddbc5f26fcdaf7 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Tue, 29 Sep 2026 02:43:07 -0700 Subject: [PATCH 18/20] test(runner): keep cancellation fixture alive on Node 22 --- tests/kit/run-tests-cancellation.test.mjs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/kit/run-tests-cancellation.test.mjs b/tests/kit/run-tests-cancellation.test.mjs index 6bba76ad..881edc46 100644 --- a/tests/kit/run-tests-cancellation.test.mjs +++ b/tests/kit/run-tests-cancellation.test.mjs @@ -24,6 +24,10 @@ test('real test timeout closes owned runner and orphan before cwd removal and re import { interruptionScope, childGone, until } from ${JSON.stringify(helper)}; let scope, child, closed = false, removed = false; test('intentional cancellation', { timeout: 500 }, async t => { + // Node 22 unrefs its test timeout; keep this fixture alive until it fires. + // This owned handle is bounded even if cancellation cleanup fails. + const keepAlive = setTimeout(() => {}, 10000); + t.after(() => clearTimeout(keepAlive)); scope = interruptionScope(t, { beforeRemove(home) { assert.equal(closed, true, 'close must precede cwd removal'); assert.equal(childGone(child.pid), true, 'OS must report ESRCH before removal'); From dea099991cf7abf5fc9f79c622a64d6a4bc37141 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Tue, 29 Sep 2026 02:44:02 -0700 Subject: [PATCH 19/20] test(runner): use owned native termination for signal retention --- tests/kit/run-tests-runner.test.mjs | 57 ++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 6 deletions(-) diff --git a/tests/kit/run-tests-runner.test.mjs b/tests/kit/run-tests-runner.test.mjs index 45cbc621..4237109e 100644 --- a/tests/kit/run-tests-runner.test.mjs +++ b/tests/kit/run-tests-runner.test.mjs @@ -304,16 +304,61 @@ test('a reaped owner does not authorize sibling deletion after a completed run', assert.ok(fs.existsSync(sibling)); }); -test('a signalled command retains its run root and performs no sibling collection', (t) => { - const { repo, env } = sandbox(t); +test('native self-termination reports the platform-specific child result', (t) => { + const { env } = sandbox(t); + const result = spawnSync(process.execPath, ['-e', "process.kill(process.pid, 'SIGTERM')"], { env }); + assert.equal(result.error, undefined); + // Windows uv_kill uses TerminateProcess(1); only uv_process_kill records + // exit_signal on the parent's owned handle. A child's self-kill cannot do so. + const expected = process.platform === 'win32' + ? { status: 1, signal: null } : { status: null, signal: 'SIGTERM' }; + assert.deepEqual({ status: result.status, signal: result.signal }, expected); + assert.throws(() => process.kill(result.pid, 0), { code: 'ESRCH' }); + t.diagnostic(`native self-termination: ${JSON.stringify(expected)}`); +}); + +test('a native timeout signal retains its run root and performs no sibling collection', (t) => { + const { home, repo, env } = sandbox(t); const sibling = fs.mkdtempSync(path.join(env.TMPDIR, 'ak-suite-')); - const r = spawnSync(process.execPath, [RUNNER, 'exec', '--repo', repo, '--', '-e', - "process.kill(process.pid, 'SIGTERM')"], { env, encoding: 'utf8' }); - assert.notEqual(r.status, 0); + fs.writeFileSync(path.join(sibling, 'sentinel'), 'preserve'); + const evidence = path.join(home, 'signal-result.json'); + const command = stub(home, 'wait-for-signal.mjs', ` + import fs from 'node:fs'; import path from 'node:path'; + fs.writeFileSync(path.join(process.env.AK_SUITE_ROOT, 'sentinel'), 'preserve'); + setTimeout(() => {}, 10000); + `); + // Alter only the native spawn options in this isolated driver. The actual + // OS result is passed through unchanged; no signal/result is fabricated. + const driver = stub(home, 'owned-timeout.mjs', ` + import cp from 'node:child_process'; import fs from 'node:fs'; + import { syncBuiltinESMExports } from 'node:module'; + import { runGuarded } from ${JSON.stringify(new URL('../../scripts/run-tests.mjs', import.meta.url).href)}; + const nativeSpawn = cp.spawnSync; + cp.spawnSync = (file, args, options) => { + const result = nativeSpawn(file, args, { ...options, timeout: 1000, killSignal: 'SIGTERM' }); + fs.writeFileSync(${JSON.stringify(evidence)}, JSON.stringify({ + pid: result.pid, status: result.status, signal: result.signal, error: result.error?.code, + })); + return result; + }; + syncBuiltinESMExports(); + try { process.exitCode = runGuarded([[${JSON.stringify(command)}]], { repoRoot: ${JSON.stringify(repo)} }); } + finally { cp.spawnSync = nativeSpawn; syncBuiltinESMExports(); } + `); + const r = spawnSync(process.execPath, [driver], { env, encoding: 'utf8' }); + assert.equal(r.error, undefined); + assert.equal(r.status, 1, r.stderr); + const native = JSON.parse(fs.readFileSync(evidence, 'utf8')); + assert.deepEqual({ status: native.status, signal: native.signal, error: native.error }, + { status: null, signal: 'SIGTERM', error: 'ETIMEDOUT' }); + assert.throws(() => process.kill(native.pid, 0), { code: 'ESRCH' }); assert.match(r.stderr, /interrupted run/); assert.doesNotMatch(r.stderr, /^kept run root/m); assert.equal(fs.readdirSync(env.TMPDIR).length, 2); - assert.ok(fs.existsSync(sibling)); + const own = fs.readdirSync(env.TMPDIR).map(name => path.join(env.TMPDIR, name)).find(root => root !== sibling); + assert.equal(fs.readFileSync(path.join(own, 'sentinel'), 'utf8'), 'preserve'); + assert.equal(fs.readFileSync(path.join(sibling, 'sentinel'), 'utf8'), 'preserve'); + t.diagnostic(`native timeout: ${JSON.stringify(native)}`); }); // Inject failures only at this subprocess's disposable temp boundary. From 36d78caf2fca6a61b7fd924499da9541ae85060c Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Tue, 29 Sep 2026 03:12:37 -0700 Subject: [PATCH 20/20] test(runner): retry fixture removal after confirmed child exit --- tests/kit/run-tests-runner.test.mjs | 67 +++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 3 deletions(-) diff --git a/tests/kit/run-tests-runner.test.mjs b/tests/kit/run-tests-runner.test.mjs index 4237109e..6f7b9a6d 100644 --- a/tests/kit/run-tests-runner.test.mjs +++ b/tests/kit/run-tests-runner.test.mjs @@ -54,7 +54,19 @@ test('focus reports a leaked temp folder with hygiene exit code', (t) => { assert.match(r.stderr, /temp folders left behind.*focus-leak-/s); }); -test('an unresolved prelaunch hold retains the guarded root and sentinel after test failure', async (t) => { +async function removeExitedFixture(root) { + for (let attempt = 0; ; attempt++) { + try { fs.rmSync(root, { recursive: true, force: true }); return; } + catch (error) { + // Match the fixture helpers' three-retry policy without depending on + // Node's JS/C++ rm implementation. Exit has already been established. + if (!['EBUSY', 'EPERM', 'ENOTEMPTY', 'EEXIST'].includes(error.code) || attempt === 3) throw error; + await new Promise(resolve => setTimeout(resolve, (attempt + 1) * 100)); + } + } +} + +async function heldRootFixture(t, { check = () => {}, beforeRemove = () => () => {} } = {}) { const { home, repo, env } = sandbox(t); const pidFile = path.join(home, 'held-child-pid'); const child = stub(home, 'held-child.cjs', `const fs = require('node:fs'); @@ -80,6 +92,7 @@ test('an unresolved prelaunch hold retains the guarded root and sentinel after t process.exit(7);`); let root; let pid; + let failure; try { const r = spawnSync(process.execPath, [RUNNER, 'exec', '--repo', repo, '--', script], { env, encoding: 'utf8' }); const roots = fs.readdirSync(env.TMPDIR).filter((name) => /^ak-suite-[A-Za-z0-9]{6}$/.test(name)); @@ -91,15 +104,63 @@ test('an unresolved prelaunch hold retains the guarded root and sentinel after t assert.equal(fs.readFileSync(path.join(root, 'held-sentinel'), 'utf8'), 'keep'); assert.equal(fs.readFileSync(path.join(root, 'held-child-ready'), 'utf8'), 'ready'); assert.doesNotThrow(() => process.kill(pid, 0), 'the held child should still own the retained cwd'); - } finally { + check(); + } catch (error) { failure = error; } + try { if (root) fs.writeFileSync(path.join(root, 'release-child'), 'release'); if (Number.isInteger(pid) && pid > 0) { if (!root) { try { process.kill(pid); } catch { /* already exited */ } } if (!await pidGone(pid)) { try { process.kill(pid); } catch { /* already exited */ } } assert.ok(await pidGone(pid), `fixture child ${pid} did not exit`); } - if (root) fs.rmSync(root, { recursive: true, force: true }); + if (root) { + const restore = beforeRemove(root, pid); + // Exit proof above is required; retries only address post-exit filesystem refusal. + try { await removeExitedFixture(root); } + finally { restore(); } + } + } catch (error) { + failure = failure ? new AggregateError([failure, error], 'fixture assertion and cleanup failed') : error; } + if (failure) throw failure; +} + +test('an unresolved prelaunch hold retains the guarded root and sentinel after test failure', heldRootFixture); + +function injectBusyRemoval(limit) { + let calls = 0; + return { + calls: () => calls, + beforeRemove(root, pid) { + const original = fs.rmSync; + fs.rmSync = (dir, ...args) => { + if (String(dir) === root) { + assert.throws(() => process.kill(pid, 0), { code: 'ESRCH' }); + if (++calls <= limit) throw Object.assign(new Error('injected post-exit EBUSY'), { code: 'EBUSY' }); + } + return original(dir, ...args); + }; + return () => { fs.rmSync = original; }; + }, + }; +} + +test('post-exit fixture removal retries transient EBUSY', async t => { + const busy = injectBusyRemoval(2); + await heldRootFixture(t, busy); + assert.equal(busy.calls(), 3); +}); + +test('exhausted post-exit retries preserve both assertion and cleanup failures', async t => { + const busy = injectBusyRemoval(Infinity); + const original = new assert.AssertionError({ message: 'original fixture assertion' }); + await assert.rejects(heldRootFixture(t, { ...busy, check: () => { throw original; } }), error => { + assert.ok(error instanceof AggregateError); + assert.equal(error.errors[0], original); + assert.equal(error.errors[1].code, 'EBUSY'); + return true; + }); + assert.equal(busy.calls(), 4, 'one attempt plus three bounded retries'); }); test('unresolved holds preserve command, tripwire, then hygiene exit priority', (t) => {