You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(server): ACP teardown survives processes exiting during the /proc scan - #36719
This round needed no code change. The brief said nothing had to change yet; this step was only to watch PR #36719. The fix (unchanged): makeLinuxProcessTreeController().snapshot now lists /proc by name with readdirSync("/proc") and keeps numeric entries, so Node no longer lstats each dirent; that lstat threw ENOENT when a process exited mid-scan and poisoned the ACP session. readLinuxProcessIdentity already skips vanished processes. All 37 checks passed on 53935ea. Macroscope approved with no line comments. CodeRabbit reported no actionable comments and minimal merge risk. No high-or-above findings were open, so I stopped watching the PR.
Commit origin/main 96735d7, Node v26.13.1, Linux. (1) Real run: a script imported packages/provider-acp/src/server/AcpSessionRuntime.ts, built the real controller with makePosixProcessTreeController("linux") and called snapshot() in a tight loop for 12s while a separate bash loop started batches of 20 short-lived /bin/true processes. Result: 61 of 7697 snapshot() calls threw Error: ENOENT: no such file or directory, lstat '/proc/83724', the reporter's exact error from readdirSync("/proc", { withFileTypes: true }). (2) Deterministic test: added a Linux-only case to apps/server/src/provider/acp/AcpSessionRuntime.processTree.test.ts. It patches fs.readdirSync (via createRequire + syncBuiltinESMExports) to throw ENOENT/lstat for the withFileTypes read of /proc and expects snapshot() to still return the current process. vp test run src/provider/acp/AcpSessionRuntime.processTree.test.ts fails on main (1 failed | 17 passed). With the reporter's proposed fix temporarily applied (readdirSync("/proc") by name, numeric entries only), all 18 tests pass; the fix was reverted and not committed.
Evidence
Stop an ACP thread during real Linux process churn, then send another message to the same session. Fresh synthetic databases; Chromium, Linux x64, Node v26.12.1; identical viewport and theme. A free ACP surrogate supplies model responses; production teardown and /proc reads run unchanged. Cgroups are read-only in a private mount namespace to exercise the process-tree fallback.
Across 30 live server cycles, base poisoned 9 sessions; head poisoned none and resumed all 30. The recordings show the same additional Stop-and-continue interaction with eight churn workers.
Paid Grok model behavior and authentication were not exercised; a free ACP surrogate supplied protocol responses through the real adapter.
The scenario runner was not used because its replay provider does not select the Linux process-ownership path involved here.
Tests
No new runs this round. CI on 53935ea: all 37 checks passed (Build and Typecheck, Test, Test Server 1-8, Test Web 1-2, Macroscope checks, CodeRabbit). Earlier: vp test run src/provider/acp/AcpSessionRuntime.processTree.test.ts 18/18 passed and vp run check:changed passed everything.
Not in this change
sweepStaleLinuxCgroupSiblings and the cgroup remove() helper also use readdirSync withFileTypes, but on cgroupfs (which reports entry types and is not affected by process exit), so they were left unchanged.
sweepStaleLinuxCgroupSiblings and the cgroup remove() helper also use readdirSync withFileTypes, but on cgroupfs, which reports entry types, so they were left unchanged.
Who did the work
Written by Claude Opus 5.5, in Claude Code
Reviewed by GPT-6.1 Sol, in Codex, and by Claude Opus 5.5, each on its own
Proof captured by GPT-6.1 Sol and checked by Claude Opus 5.5
…id-scan
Fails on main: the Linux process-tree snapshot reads /proc with
withFileTypes, and the lstat Node does for an entry of unknown type
throws ENOENT when that process exits during the scan (#36707).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… scan
The Linux process-tree snapshot listed /proc with withFileTypes, and the
lstat Node does for entries of unknown type threw ENOENT when a process
exited mid-scan, poisoning the session. List /proc by name instead;
readLinuxProcessIdentity already skips vanished processes.
Fixes#36707
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Macroscope's review found this PR approvable — Linux ACP teardown now lists /proc by name so processes exiting during enumeration do not abort the snapshot. Existing identity and ownership safeguards remain unchanged, keeping the change narrowly scoped.
You can add or adjust custom eligibility rules. Learn more.
No actionable comments were generated in the recent review.
Walkthrough
The Linux process-tree snapshot now reads /proc entry names without requesting directory-type metadata. A Linux-only test simulates an ENOENT error during entry resolution and checks that the snapshot includes the current process.
Changes: Linux ACP process snapshot
Layer / File(s)
Summary
Read /proc entries and test vanished-entry handling packages/provider-acp/src/server/AcpSessionRuntime.ts, apps/server/src/provider/acp/AcpSessionRuntime.processTree.test.ts
The snapshot reads entry names and resolves numeric names through readLinuxProcessIdentity. A Linux-only test simulates ENOENT, checks that the snapshot includes the current process, and restores the patched filesystem function and builtin exports.
The change makes Linux process-tree teardown tolerate processes that exit during the /proc scan, so Stop should complete instead of failing the session. No actionable merge-blocking risk remains.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title names the fix and its scope: ACP teardown no longer fails when processes exit during the /proc scan. This matches the Linux process-tree snapshot change in the diff and the linked issue #367…
Linked Issues check
[#36707] Requirement: on Linux process-tree teardown, a process that exits during the /proc scan must be ignored, so the snapshot returns the running processes and Stop completes. The diff changes mak…
Out of Scope Changes check
The only production change is the /proc snapshot listing in makeLinuxProcessTreeController, which is the fix named by #36707. The only test change adds a Linux-only regression case for that snapshot a…
Description check
The description is detailed and covers the problem (ENOENT from lstat during /proc scan poisoning ACP sessions, linked to issue #36707), the change (listing /proc by name with readdirSync and keeping …
Full details: Title check
Explanation
The title names the fix and its scope: ACP teardown no longer fails when processes exit during the /proc scan. This matches the Linux process-tree snapshot change in the diff and the linked issue #36707. The title follows the conventional commit form the template asks for.
Full details: Linked Issues check
Explanation
[#36707] Requirement: on Linux process-tree teardown, a process that exits during the /proc scan must be ignored, so the snapshot returns the running processes and Stop completes. The diff changes makeLinuxProcessTreeController().snapshot in packages/provider-acp/src/server/AcpSessionRuntime.ts from readdirSync("/proc", { withFileTypes: true }) to readdirSync("/proc"). Names are returned as strings, so Node does not lstat each entry and the ENOENT from a vanished process cannot be raised there. Numeric names still pass the anchored regex ^[1-9][0-9]*$ and go to readLinuxProcessIdentity, which the issue states already skips vanished processes. I did not re-read readLinuxProcessIdentity; that step relies on the issue and PR text. The added test in apps/server/src/provider/acp/AcpSessionRuntime.processTree.test.ts forces the withFileTypes-style ENOENT for /proc and asserts that snapshot() still includes process.pid. This covers the deterministic reproduction the issue asks for. The cgroup and macOS/Windows paths are unchanged, as the issue scope requires. The PR body's real-app before/after evidence is still pending. It is not a coding requirement, so it does not affect this result.
Full details: Out of Scope Changes check
Explanation
The only production change is the /proc snapshot listing in makeLinuxProcessTreeController, which is the fix named by #36707. The only test change adds a Linux-only regression case for that snapshot and restores the patched fs.readdirSync and ESM builtin exports in a finally block. The PR leaves sweepStaleLinuxCgroupSiblings and the cgroup remove() helper unchanged, which matches the issue's exclusion of cgroup teardown. No unrelated files appear in the reviewed diffs. The PR description and PR summary are documentation of the same change, not code scope.
Full details: Description check
Explanation
The description is detailed and covers the problem (ENOENT from lstat during /proc scan poisoning ACP sessions, linked to issue #36707), the change (listing /proc by name with readdirSync and keeping numeric entries), scope (fix for a reported bug, linked issue, cgroup paths deliberately excluded), and verification (reproduction on main with a deterministic test that fails before and passes with the fix, CI results, and before/after evidence). It also names the model and harness used. It has some gaps: the Problem section does not separate a clear expected-behavior statement from the reproduction, the Change section is brief, the Scope section does not explicitly justify the absence of a maintainer request, and the Tests section contains a duplicated bullet about cgroup paths in the 'Not in this change' section. The description also includes run-process narrative ("the brief said nothing had to change yet", "I stopped watching the PR") that does not match the template's requirements. Overall the template's required information is present, so the check passes.
Finishing Touches
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
size:XS0-9 changed lines (additions + deletions).vouch:trustedAuthor is trusted by repo permissions or the VOUCHED list.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
This round needed no code change. The brief said nothing had to change yet; this step was only to watch PR #36719. The fix (unchanged): makeLinuxProcessTreeController().snapshot now lists /proc by name with readdirSync("/proc") and keeps numeric entries, so Node no longer lstats each dirent; that lstat threw ENOENT when a process exited mid-scan and poisoned the ACP session. readLinuxProcessIdentity already skips vanished processes. All 37 checks passed on 53935ea. Macroscope approved with no line comments. CodeRabbit reported no actionable comments and minimal merge risk. No high-or-above findings were open, so I stopped watching the PR.
Fixes #36707
Reproduction on main
Commit origin/main 96735d7, Node v26.13.1, Linux. (1) Real run: a script imported packages/provider-acp/src/server/AcpSessionRuntime.ts, built the real controller with makePosixProcessTreeController("linux") and called snapshot() in a tight loop for 12s while a separate bash loop started batches of 20 short-lived
/bin/trueprocesses. Result: 61 of 7697 snapshot() calls threwError: ENOENT: no such file or directory, lstat '/proc/83724', the reporter's exact error from readdirSync("/proc", { withFileTypes: true }). (2) Deterministic test: added a Linux-only case to apps/server/src/provider/acp/AcpSessionRuntime.processTree.test.ts. It patches fs.readdirSync (via createRequire + syncBuiltinESMExports) to throw ENOENT/lstat for the withFileTypes read of /proc and expects snapshot() to still return the current process.vp test run src/provider/acp/AcpSessionRuntime.processTree.test.tsfails on main (1 failed | 17 passed). With the reporter's proposed fix temporarily applied (readdirSync("/proc") by name, numeric entries only), all 18 tests pass; the fix was reverted and not committed.Evidence
Stop an ACP thread during real Linux process churn, then send another message to the same session. Fresh synthetic databases; Chromium, Linux x64, Node v26.12.1; identical viewport and theme. A free ACP surrogate supplies model responses; production teardown and /proc reads run unchanged. Cgroups are read-only in a private mount namespace to exercise the process-tree fallback.
Across 30 live server cycles, base poisoned 9 sessions; head poisoned none and resumed all 30. The recordings show the same additional Stop-and-continue interaction with eight churn workers.
Reproduction scripts, instructions and runtime transcripts · Server driver
Not shown:
Tests
No new runs this round. CI on 53935ea: all 37 checks passed (Build and Typecheck, Test, Test Server 1-8, Test Web 1-2, Macroscope checks, CodeRabbit). Earlier:
vp test run src/provider/acp/AcpSessionRuntime.processTree.test.ts18/18 passed andvp run check:changedpassed everything.Not in this change
Who did the work
Run by the t3code factory in T3 Code.