Repository navigation
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: c08fb169-6d53-4fff-97ca-850303c715f9) |
There was a problem hiding this comment.
This is an auto review done by revuto.
This PR isn't ready to merge yet. Two blockers:
- The new
T465 temporary probetest always ends inassert.failon win32.npm testruns it on all four Windows shards for every main push, so Windows CI would fail on every run, not one in three. changelog.d/T465.mdsays the test "no longer fails", but nothing here fixes it. The harness change only prints the inner attempt's reason to stderr, and the assertion change only adds diagnostics.
The new 30-pass repeat step also exits 1 on any failure. Until the real fix lands, the repeats job will keep failing on main.
| await child.containmentDone; | ||
| report.push(`inner round ${round}: ${child.resourceReason} after ${Date.now() - started} ms`); | ||
| } | ||
| assert.fail(report.join('\n')); |
There was a problem hiding this comment.
This is an auto review done by revuto.
This probe always fails on Windows: it ends in an unconditional assert.fail(report.join('\n')), and the only skip is for non-win32. npm test runs on the four windows-latest shards for every push to main (ci.yml lines 54-64), so merging this would turn a one-in-three flake into a failure on every run. That's fine as a measurement commit on the branch, but remove the probe (or gate it behind an opt-in env var) before merge.
| @@ -0,0 +1 @@ | |||
| - Windows CI: the nested fork storm containment test no longer fails when its inner attempt stops the storm first. | |||
There was a problem hiding this comment.
This is an auto review done by revuto.
This entry says the nested fork storm test "no longer fails when its inner attempt stops the storm first", but the PR has no fix for that. The harness change only writes inner attempt: <reason> to stderr. The outer watchdog and result.resource_reason don't change, so the assert.equal(result.resource_reason, 'resource-tasks-max', ...) at line 589 still fails whenever the inner cap fires first. Either add the fix, for example by having the outer attempt read the inner attempt's reason or by changing the assertion, or hold this entry until the fix lands.
…on Windows (temporary job) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 87bad439-4106-4adf-9810-470ccf8bf7bd) |
There was a problem hiding this comment.
This is an auto review done by revuto.
Both earlier findings are fixed:
- The always-failing
T465 temporary probetest is gone fromtest/containment.test.js. - The changelog entry now has a real fix behind it. The inner attempt's memory cap in
nested-stormis raised to 4 GiB. Its tasks cap of 128 is already well above the storm's process count. The outer watchdog checks the task count before memory (lib/containment.js:640-641), so the outertasks_max: 4now wins.
One blocker remains. The probe moved into a new t465-probe CI job and test/fixtures/t465-probe.js. The fixture describes itself as "Temporary … removed before submit", but both would land on main. Details are inline.
|
|
||
| t465-probe: | ||
| name: T465 probe (temporary) | ||
| if: github.event_name != 'pull_request' |
There was a problem hiding this comment.
This is an auto review done by revuto.
This job is temporary. Its name says so, and test/fixtures/t465-probe.js:3 says "removed before submit". But merging this PR puts both on main.
There, if: github.event_name != 'pull_request' runs the job on every main push and on every daily run. Each run holds a windows-latest runner for up to 30 minutes and runs 40 nested-storm gate runs plus 3 CIM snapshots.
The same guard keeps it from running in this PR's CI, so it can't collect its evidence before merge either. It only runs if someone dispatches the workflow manually on this branch.
The probe never sets a failing exit code unless it throws, so leaving it in would cost runner time without anyone noticing. Get the probe's data from a manual dispatch on this branch, then remove the job and the fixture before merging. The 10-pass repeat step plus the 4 GiB inner cap are enough to show the fix.
…changelog Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: a3688e11-4ae2-4ddf-afbc-0c6398b3acc2) |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 8b516e3d-7b8a-4f27-8940-745714395158) |
The Windows test
finite nested fork storm fails the gate command and leaves another attempt runningfailed on the main push at 7938043 (shard 2/4) withresource_reasonundefined. It now gets a memory cap the storm cannot reach, and the Windows repeat job runs it ten times.Cause: sampler side (the storm ended between outer samples), not reporting and not a containment gap
nested-stormlaunches the storm as an inner contained attempt: 128 tasks, 512 MiB. On Windows its watchdog sums the working sets of the storm's tree. A probe on a hosted runner (run 39490666666, temporary jobT465 probe) measured the storm's 13 Node processes at 492 to 495 MiB, about 38 MiB each. That is 17 MiB under the inner cap.resource-memory-maxbefore an outer snapshot counts it above 4 tasks. By design a nested attempt's cap does not fail its parent, so the outer correctly records no reason, and the gate command fails with plain exit 1.resource-tasks-max. That held in 40 of 40 probe rounds, detected 2.0 to 5.4 s after launch (CIM snapshot 0.3 s warm, 2.6 s cold).Change
test/fixtures/containment-harness.js: thenested-storminner attempt gets a 4 GiB memory cap, about 8 times the measured storm, so only the outer task cap can stop the storm. The harness printsinner attempt: <reason>when the inner attempt hits a cap.test/containment.test.js: the assertion message carries the exit status and stderr..github/workflows/ci.yml:repeat (windows-latest, node 26)runs the test ten times, right after the wakeup step, with a 15-minute step limit so a hung pass still leaves the job log.test/ci-required.test.jsanddocs/testing.mdlist the step.lib/.Verification
repeat (windows-latest, node 26)passedFinite nested fork storm10/10 (3.2 to 3.6 s each). Windows shards 2/4, 3/4 and 4/4 passed.test/isolation.test.js:3138(ladder args allowlist,actual: ''), which this branch does not touch. Run 39477442480 also failed shard 1/4 intest/mutants.test.js, also untouched here.npm test -- test/containment.test.js test/ci-required.test.js53/53 pass;npm run check:shared -- --base origin/mainexit 0.Limits
Pre-submit review
Finite nested fork storm: pass 1/10to10/10, all passed, at f5a14f6; since then the test code changed only by a harness comment (git diff f5a14f6 d2ebbd7 -- test/containment.test.js test/fixtures/containment-harness.js). "names the cause and fixes it": Cause section above; fix attest/fixtures/containment-harness.js(nested-storm, 4 GiB inner memory cap).npm run check:shared -- --base origin/mainexit 0🤖 Generated with Claude Code