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
Last pass on the scripts/check-test-remove-tree.mjs lint gate, following #846, #847, and #848.
Migrations
This moves the three real recursive rmSync call sites onto the canonical helper:
packages/rsc-runtime/tests/plugin-root.test.ts: the removal was in the finally of a synchronous it. That test can await, so it is now async and calls await removeTree(directory).
packages/agent-bundle/tests/playground-service.test.ts: the removal is inside a synchronous now() clock callback, which has to delete .pending and write a blocker file before it returns. It now calls removeTreeSync(pendingRoot).
packages/agent-bundle/tests/support/shared-pack.ts: the removal is in a process.once('exit') handler, where async work never runs. It now calls removeTreeSync(destination).
removeTreeSync
removeTreeSync sits next to removeTree in tests/support/remove-tree.ts and runs the same retry loop:
the same retry codes, through one shared isRetryable check
6 attempts, with a retryDelay * (attempt + 1) backoff
a synchronous sleep via Atomics.wait
It does not rely on Node's own rmSync({ maxRetries }), because Node 24 routes that through the C++ binding.rmSync, and there the backoff behavior is up to Node's internals. A test covers one ENOTEMPTY followed by success (2 attempts, at least 45 ms elapsed), and a persistent ENOTEMPTY that throws after 6 attempts.
Gate
rmSync, rmdir, and rmdirSync are now flagged under the same literal-options and shadowing rules as rm. That covers named and aliased imports, ns.X, ns.promises.X, and wrapped callees.
A literal maxRetries: 0, including forms like "maxRetries": 0x0, now counts as not retried.
The failure text now reads bare recursive rm. Use removeTree (removeTreeSync if it cannot await).
One ponytail: line in the header names the ceiling. Options held in a variable or spread, a non-literal recursive or maxRetries, and indirect bindings are not followed; closing that needs data-flow analysis. The indirect bindings are local aliases, destructuring, dynamic import or require, ns['rm'], and .call.
The new gate tests failed on main. Before the migration, the extended gate flagged exactly the three sites above. After it, the gate reports zero violations on the real tree.
Checks
Run on the merged head 0610bdbb68, which contains origin/mainc74702b9cc:
pnpm build: exit 0
pnpm typecheck: exit 0
pnpm lint (includes the gate, 0 violations): exit 0
pnpm test:unit: 4437 passed, 0 failed, 6 skipped
Focused run of the gate test, remove-tree.test.ts, plugin-root.test.ts, and playground-service.test.ts: 103 passed
pnpm test:packed, the pool that imports shared-pack.ts: 46 passed, 0 failed, 1 skipped
A Node process that registers removeTreeSync in a process.once('exit') handler confirmed a nested tree is removed on exit.
Independent review by change-risk-reviewer (Claude Opus 5.5) found no material findings, on both the first pass and the follow-up that rewrote removeTreeSync as a retry loop. Its first-pass notes, the backoff claim and the message text, are addressed.
No changeset: only packages/*/tests/** and root scripts/ change, and both are exempt per .changeset/README.md.
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
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
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.
Last pass on the
scripts/check-test-remove-tree.mjslint gate, following #846, #847, and #848.Migrations
This moves the three real recursive
rmSynccall sites onto the canonical helper:packages/rsc-runtime/tests/plugin-root.test.ts: the removal was in thefinallyof a synchronousit. That test can await, so it is now async and callsawait removeTree(directory).packages/agent-bundle/tests/playground-service.test.ts: the removal is inside a synchronousnow()clock callback, which has to delete.pendingand write a blocker file before it returns. It now callsremoveTreeSync(pendingRoot).packages/agent-bundle/tests/support/shared-pack.ts: the removal is in aprocess.once('exit')handler, where async work never runs. It now callsremoveTreeSync(destination).removeTreeSyncremoveTreeSyncsits next toremoveTreeintests/support/remove-tree.tsand runs the same retry loop:isRetryablecheckretryDelay * (attempt + 1)backoffAtomics.waitIt does not rely on Node's own
rmSync({ maxRetries }), because Node 24 routes that through the C++binding.rmSync, and there the backoff behavior is up to Node's internals. A test covers one ENOTEMPTY followed by success (2 attempts, at least 45 ms elapsed), and a persistent ENOTEMPTY that throws after 6 attempts.Gate
rmSync,rmdir, andrmdirSyncare now flagged under the same literal-options and shadowing rules asrm. That covers named and aliased imports,ns.X,ns.promises.X, and wrapped callees.maxRetries: 0, including forms like"maxRetries": 0x0, now counts as not retried.bare recursive rm. Use removeTree (removeTreeSync if it cannot await).ponytail:line in the header names the ceiling. Options held in a variable or spread, a non-literalrecursiveormaxRetries, and indirect bindings are not followed; closing that needs data-flow analysis. The indirect bindings are local aliases, destructuring, dynamic import orrequire,ns['rm'], and.call.The new gate tests failed on main. Before the migration, the extended gate flagged exactly the three sites above. After it, the gate reports zero violations on the real tree.
Checks
Run on the merged head
0610bdbb68, which containsorigin/mainc74702b9cc:pnpm build: exit 0pnpm typecheck: exit 0pnpm lint(includes the gate, 0 violations): exit 0pnpm test:unit: 4437 passed, 0 failed, 6 skippedremove-tree.test.ts,plugin-root.test.ts, andplayground-service.test.ts: 103 passedpnpm test:packed, the pool that importsshared-pack.ts: 46 passed, 0 failed, 1 skippedremoveTreeSyncin aprocess.once('exit')handler confirmed a nested tree is removed on exit.Independent review by change-risk-reviewer (Claude Opus 5.5) found no material findings, on both the first pass and the follow-up that rewrote
removeTreeSyncas a retry loop. Its first-pass notes, the backoff claim and the message text, are addressed.No changeset: only
packages/*/tests/**and rootscripts/change, and both are exempt per.changeset/README.md.