Skip to content

fix(test): flag recursive rm whose options carry a non-null assertion - #847

Merged
ScriptedAlchemy merged 3 commits into
mainfrom
fix/remove-tree-gate-nonnull
Sep 25, 2026
Merged

ScriptedAlchemy merged 3 commits into
mainfrom
fix/remove-tree-gate-nonnull

Conversation

@ScriptedAlchemy

Copy link
Copy Markdown
Owner

Follow-up to #825 and #846. The scripts/check-test-remove-tree.mjs lint gate missed recursive removals whose options object had a non-null assertion, such as rm(root, { recursive: true }!). The cause was that unwrapExpression stopped at the NonNullExpression, so optionsFlags never reached the object literal. Trunk had the same miss.

Wrappers

unwrapExpression now also unwraps:

  • Non-null assertions ({...}!). This fixes the reported miss for named rm, namespace fs.rm, and promises as fsp + fsp.rm. It also fixes the nested forms ({...} as const)!, {...}! satisfies O, and <O>{...}!.
  • Instantiation expressions (({...})<O>). The parser produces these, but TypeScript rejects them on an object literal (TS2635). The review flagged them as the only remaining wrapper, so they're covered for completeness.

Parentheses, as, satisfies, and <T>x were already handled. PartiallyEmittedExpression only comes from transformers and never from createSourceFile. A JSDoc @type cast in .js parses as a plain parenthesized expression, which was already handled.

A maxRetries inside a wrapped literal still counts as retried. Both new cases failed before the script change.

Out of scope, known gap

Wrappers on the callee are still missed by isNodeBoundRmCall: (rm)(...), rm!(...), and fs!.rm(...). So is fs.promises.rm on a default node:fs import.

Checks

Run on the merged head 41e0842bf6, which contains origin/main c42b93d043:

  • pnpm build: exit 0
  • pnpm typecheck: exit 0
  • pnpm lint (includes the gate): exit 0
  • pnpm test:unit: 4432 passed, 0 failed, 6 skipped
  • rstest packages/agent-bundle/tests/check-test-remove-tree.test.ts: 18/18

Independent review by change-risk-reviewer (Claude Opus 5.5) found no material findings. Its one nit, instantiation expressions, is fixed in this PR.

No changeset: only root scripts/ and packages/agent-bundle/tests/** change, and both are exempt per .changeset/README.md.

@changeset-bot

changeset-bot Bot commented Sep 25, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 41e0842

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

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@ScriptedAlchemy
ScriptedAlchemy merged commit ab25ee3 into main Sep 25, 2026
5 checks passed
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-25T02:54:04.989318Z 41e0842 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant