test: find capability-token reads in esbuild output, not by tokenizing #46

Merged
jercik merged 1 commit from test/token-scan-skips-prose into main 2026-10-05 09:35:46 +00:00
Owner

Follow-up to #44 (thread 126092, claim 01M458KM7J7NJA94HEFQTW7B61): the capability-token scan now runs on esbuild's output for each module instead of a hand-written tokenizer, so prose in comments stops counting as a read and a regex literal can no longer hide one. It is stacked on test/single-capability-token-reader, which adds the scan, and merges after #44. It conflicts with #45 on the esbuild import line in src/credentials.test.ts.

🤖 Generated with Claude Code

Follow-up to #44 (thread 126092, claim 01M458KM7J7NJA94HEFQTW7B61): the capability-token scan now runs on esbuild's output for each module instead of a hand-written tokenizer, so prose in comments stops counting as a read and a regex literal can no longer hide one. It is stacked on `test/single-capability-token-reader`, which adds the scan, and merges after #44. It conflicts with #45 on the `esbuild` import line in `src/credentials.test.ts`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
test: stop counting prose mentions of the capability token as reads
All checks were successful
commit-msg / commitlint (pull_request) Successful in 27s
Checks / quality-checks (pull_request) Successful in 1m0s
Review / Review (pull_request_target) Successful in 4m35s
499aaab5cd
The scan matched a backtick code span in a comment, and a prose string
inside an object literal before a ternary colon, as reads of
REVIEW_CAPABILITY_TOKEN. Comments and one-line strings are now blanked
before the code patterns run, and a string literal counts as a key only
when it is exactly the name, so template-literal keys still count.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Review 01M45PNGCFGC9WAE8NQ3G8CGRZ — head f04f14bb4f78ba66d9bbd020acbdb8d8181ceada

Review — j4k-oss/review-wrapper @ 6fc1cae5b0

Scope: diff against base tree 98caf66c9b60
Status: dispatched — coverage complete (5/5 slots terminal)
Facts: current review-wide projection

Computed under:

{
  "abandonment": "abandonment-v1",
  "anchor_recipe": 1,
  "batch_policy": "batch-v1",
  "coverage": "coverage-v3",
  "dispatch_policy": "dispatch-v2",
  "grounder_version": 1,
  "grounding_read_rule": "grounding-read-v1",
  "promotion_policy": "promotion-v1",
  "report": "report-v4",
  "tally": "tally-v1",
  "triage_settle": "triage-settle-v2"
}

Findings (0)

No findings survived.

Reviewed:

  • general-bug (whole/default): unit-failed
  • writing-quality (whole/default): unit-failed
  • test-trimming (whole/default): unit-failed
  • restated-sets (whole/default): unit-failed
  • project-docs (whole/default): unit-failed

Other claims

  • grounding-pending (0)
  • ungrounded (0)
  • rejected (0)
  • duplicate-of (0)
  • unadjudicated (0)

Coverage

Coverage pass: 01M45PNGF3EXGZ1T5QWKMHKZF0
Accounting: complete
Slot health: general-bug (whole/default): unit-failed; writing-quality (whole/default): unit-failed; test-trimming (whole/default): unit-failed; restated-sets (whole/default): unit-failed; project-docs (whole/default): unit-failed

lens part arm unit status runs loss
general-bug whole default unit-failed 1 no
writing-quality whole default unit-failed 1 no
test-trimming whole default unit-failed 1 no
restated-sets whole default unit-failed 1 no
project-docs whole default unit-failed 1 no
  • general-bug run 01M45PNGF4GAFSTVH9YQXCMPRC: sandbox infrastructure failed.
  • writing-quality run 01M45PNGF5Q65DAZG6C4BZ72BZ: sandbox infrastructure failed.
  • test-trimming run 01M45PNGF64FHHDVVE1RB2J016: sandbox infrastructure failed.
  • restated-sets run 01M45PNGF64FHHDVVE1RB2J019: sandbox infrastructure failed.
  • project-docs run 01M45PNGF7YGFK1E7J7JCNDCCC: sandbox infrastructure failed.
<!-- review:summary --> **Review** `01M45PNGCFGC9WAE8NQ3G8CGRZ` — head `f04f14bb4f78ba66d9bbd020acbdb8d8181ceada` # Review — j4k-oss/review-wrapper @ 6fc1cae5b0f0 Scope: diff against base tree `98caf66c9b60` Status: dispatched — coverage complete (5/5 slots terminal) Facts: current review-wide projection Computed under: ```json { "abandonment": "abandonment-v1", "anchor_recipe": 1, "batch_policy": "batch-v1", "coverage": "coverage-v3", "dispatch_policy": "dispatch-v2", "grounder_version": 1, "grounding_read_rule": "grounding-read-v1", "promotion_policy": "promotion-v1", "report": "report-v4", "tally": "tally-v1", "triage_settle": "triage-settle-v2" } ``` ## Findings (0) No findings survived. Reviewed: - general-bug (whole/default): unit-failed - writing-quality (whole/default): unit-failed - test-trimming (whole/default): unit-failed - restated-sets (whole/default): unit-failed - project-docs (whole/default): unit-failed ## Other claims - grounding-pending (0) - ungrounded (0) - rejected (0) - duplicate-of (0) - unadjudicated (0) ## Coverage Coverage pass: 01M45PNGF3EXGZ1T5QWKMHKZF0 Accounting: complete Slot health: general-bug (whole/default): unit-failed; writing-quality (whole/default): unit-failed; test-trimming (whole/default): unit-failed; restated-sets (whole/default): unit-failed; project-docs (whole/default): unit-failed | lens | part | arm | unit status | runs | loss | | --- | --- | --- | --- | --- | --- | | general-bug | whole | default | unit-failed | 1 | no | | writing-quality | whole | default | unit-failed | 1 | no | | test-trimming | whole | default | unit-failed | 1 | no | | restated-sets | whole | default | unit-failed | 1 | no | | project-docs | whole | default | unit-failed | 1 | no | - general-bug run 01M45PNGF4GAFSTVH9YQXCMPRC: sandbox infrastructure failed. - writing-quality run 01M45PNGF5Q65DAZG6C4BZ72BZ: sandbox infrastructure failed. - test-trimming run 01M45PNGF64FHHDVVE1RB2J016: sandbox infrastructure failed. - restated-sets run 01M45PNGF64FHHDVVE1RB2J019: sandbox infrastructure failed. - project-docs run 01M45PNGF7YGFK1E7J7JCNDCCC: sandbox infrastructure failed.
Lines 15-16
@ -16,0 +12,5 @@
// Comments and messages elsewhere mention the variable in prose, so only the name
// spelled as a key counts as a read: a string literal that is exactly the name, or the
// name in code once comments and one-line strings are blanked. A literal that spans lines
// or interpolates stays as code, because a quote inside a regex literal can open a
// literal that runs past its real end. A key built at run time is invisible to this scan.

low — Comment gives the regex-quote hazard as the reason for keeping interpolating templates as code, but that hazard only explains the multi-line case

The comment above COMMENT_OR_STRING attaches one reason to two conditions, and the reason fits only one of them. A maintainer who later teaches the scan to skip regex literals would read the ${ check as part of the same workaround and could delete it. Then a template such as `${process.env.REVIEW_CAPABILITY_TOKEN}` would be blanked to "", and the "only module that reads REVIEW_CAPABILITY_TOKEN by name" test would stop seeing that read.

The comment says: "A literal that spans lines or interpolates stays as code, because a quote inside a regex literal can open a literal that runs past its real end." In blankProse, token.includes("\n") || token.includes("${") ? token : '""' implements both conditions. Only the newline condition guards against a mis-paired quote. The " and ' alternatives in COMMENT_OR_STRING exclude \n, so only the backtick alternative can run on across lines. The ${ condition exists for a different reason: an interpolation holds real code, and that code can read the token through .REVIEW_CAPABILITY_TOKEN or a destructuring pattern. The skill's Use Precise Language and Give rationale guidance applies, because a reason helps only when it lets the reader adapt the rule correctly.

Correction: give each condition its own reason, for example: "A literal that spans lines stays as code, because a quote inside a regex literal can open a literal that runs past its real end; a template that interpolates stays as code, because its ${…} holds code." This keeps both conditions and the regex hazard, and adds only the missing reason.

I examined blankProse, COMMENT_OR_STRING, CAPABILITY_TOKEN_CODE_READS, and readsCapabilityToken in src/credentials.test.ts. I also grepped src for current reads of the name and found the only one at requireEnv("REVIEW_CAPABILITY_TOKEN") in src/credentials.ts. I did not run the test. The reasoning is static. The claim fails if the ${ check was in fact meant to work around regex quotes. It holds if the check's purpose is to keep interpolated code visible, which the code-read patterns it feeds suggest.

lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M45AEEWTV53AS0D0277G2F12 of review 01M45AC6XDDTSZ61889EB2TB9A

<!-- review:claim:01M45AEEWTV53AS0D0277G2F12 --> **low** — Comment gives the regex-quote hazard as the reason for keeping interpolating templates as code, but that hazard only explains the multi-line case > The comment above `COMMENT_OR_STRING` attaches one reason to two conditions, and the reason fits only one of them. A maintainer who later teaches the scan to skip regex literals would read the `${` check as part of the same workaround and could delete it. Then a template such as `` `${process.env.REVIEW_CAPABILITY_TOKEN}` `` would be blanked to `""`, and the "only module that reads REVIEW_CAPABILITY_TOKEN by name" test would stop seeing that read. > > The comment says: "A literal that spans lines or interpolates stays as code, because a quote inside a regex literal can open a literal that runs past its real end." In `blankProse`, `token.includes("\n") || token.includes("${") ? token : '""'` implements both conditions. Only the newline condition guards against a mis-paired quote. The `"` and `'` alternatives in `COMMENT_OR_STRING` exclude `\n`, so only the backtick alternative can run on across lines. The `${` condition exists for a different reason: an interpolation holds real code, and that code can read the token through `.REVIEW_CAPABILITY_TOKEN` or a destructuring pattern. The skill's Use Precise Language and Give rationale guidance applies, because a reason helps only when it lets the reader adapt the rule correctly. > > Correction: give each condition its own reason, for example: "A literal that spans lines stays as code, because a quote inside a regex literal can open a literal that runs past its real end; a template that interpolates stays as code, because its `${…}` holds code." This keeps both conditions and the regex hazard, and adds only the missing reason. > > I examined `blankProse`, `COMMENT_OR_STRING`, `CAPABILITY_TOKEN_CODE_READS`, and `readsCapabilityToken` in src/credentials.test.ts. I also grepped `src` for current reads of the name and found the only one at `requireEnv("REVIEW_CAPABILITY_TOKEN")` in src/credentials.ts. I did not run the test. The reasoning is static. The claim fails if the `${` check was in fact meant to work around regex quotes. It holds if the check's purpose is to keep interpolated code visible, which the code-read patterns it feeds suggest. lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M45AEEWTV53AS0D0277G2F12` of review `01M45AC6XDDTSZ61889EB2TB9A`
Author
Owner

Fixed in fb3a1eb by removing the code it described. blankProse and its newline and ${ conditions are gone, along with the tokenizer, because esbuild now parses each module. An interpolated read such as `Bearer ${process.env.REVIEW_CAPABILITY_TOKEN}` is a fixture case that must count, so a later change can't drop it silently. The new comment gives one reason for each choice: why the test parses instead of tokenizing, and why a prose string spelled like a key is allowed to count.

<!-- gh-feedback:reply-to:126458 --> Fixed in fb3a1eb by removing the code it described. `blankProse` and its newline and `${` conditions are gone, along with the tokenizer, because esbuild now parses each module. An interpolated read such as `` `Bearer ${process.env.REVIEW_CAPABILITY_TOKEN}` `` is a fixture case that must count, so a later change can't drop it silently. The new comment gives one reason for each choice: why the test parses instead of tokenizing, and why a prose string spelled like a key is allowed to count.
jercik marked this conversation as resolved
Lines 26-28
@ -18,2 +23,6 @@
];
function blankProse(token: string): string {
if (token.startsWith("/")) {
return " ";
}

medium — readsCapabilityToken blanks every comment token, so /* or // inside a regex literal hides real REVIEW_CAPABILITY_TOKEN reads from the isolation test

The "is the only module that reads REVIEW_CAPABILITY_TOKEN by name" test can pass while another module reads the token. COMMENT_OR_STRING tokenizes without knowing about regex literals, so a regex that contains an escaped slash followed by * or / (/^\/*/u to strip leading slashes, /^https:\/\//u to match a URL scheme) gets read as the start of a comment. blankProse then replaces any token starting with / with a space without checking its size. A false /* blanks everything up to the next real */ anywhere later in the file, possibly hundreds of lines. A false // blanks the rest of its line. A process.env.REVIEW_CAPABILITY_TOKEN read inside the blanked span never reaches CAPABILITY_TOKEN_CODE_READS.

The new comment above COMMENT_OR_STRING already names this mechanism for strings: "A literal that spans lines or interpolates stays as code, because a quote inside a regex literal can open a literal that runs past its real end." blankProse applies that protection only to quote and backtick tokens. A block comment that spans lines is blanked anyway, so the same over-long-token problem the comment guards against still applies when the token is a comment. Single-line quote tokens have the same problem within one line: in const Q = /"/u; const t = process.env.REVIEW_CAPABILITY_TOKEN; const s = "x";, the " inside the regex opens a string that swallows the read.

Correction: treat comment tokens the same way as string tokens. Keep a /* ... */ token that contains a newline as code, as already happens for multi-line literals. Alternatively, run the code-read patterns over both the raw source and the blanked source, and use the blanked version only to discard matches that fall entirely inside a real comment or string.

What I examined: I copied COMMENT_OR_STRING, CAPABILITY_TOKEN_KEY, CAPABILITY_TOKEN_CODE_READS, blankProse and readsCapabilityToken from the subject into a plain Node script and ran it. 'const GLOB = /^\\/*/u;\nconst t = process.env.REVIEW_CAPABILITY_TOKEN;\n/* done */' returned false, and so did 'const U = /^https:\\/\\//u; const t = process.env.REVIEW_CAPABILITY_TOKEN;'. Plain process.env.REVIEW_CAPABILITY_TOKEN, bracket access and destructuring all returned true. Against the current src/ tree the scan returns only src/credentials.ts, so the test passes today. No current non-test module has a regex containing \/* or \// (grep), so this is a hole that has not been triggered yet, not a read the test is currently missing. I did not run vitest itself, because there is no node_modules in the sandbox.

To confirm: add a fixture source with a slash-star regex followed by a token read, call readsCapabilityToken on it, and check that it returns false.

lens general-bug · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M45AF53AQ8DMWXVJQB1KEX3Z of review 01M45AC6XDDTSZ61889EB2TB9A

<!-- review:claim:01M45AF53AQ8DMWXVJQB1KEX3Z --> **medium** — `readsCapabilityToken` blanks every comment token, so `/*` or `//` inside a regex literal hides real `REVIEW_CAPABILITY_TOKEN` reads from the isolation test > The "is the only module that reads REVIEW_CAPABILITY_TOKEN by name" test can pass while another module reads the token. `COMMENT_OR_STRING` tokenizes without knowing about regex literals, so a regex that contains an escaped slash followed by `*` or `/` (`/^\/*/u` to strip leading slashes, `/^https:\/\//u` to match a URL scheme) gets read as the start of a comment. `blankProse` then replaces any token starting with `/` with a space without checking its size. A false `/*` blanks everything up to the next real `*/` anywhere later in the file, possibly hundreds of lines. A false `//` blanks the rest of its line. A `process.env.REVIEW_CAPABILITY_TOKEN` read inside the blanked span never reaches `CAPABILITY_TOKEN_CODE_READS`. > > The new comment above `COMMENT_OR_STRING` already names this mechanism for strings: "A literal that spans lines or interpolates stays as code, because a quote inside a regex literal can open a literal that runs past its real end." `blankProse` applies that protection only to quote and backtick tokens. A block comment that spans lines is blanked anyway, so the same over-long-token problem the comment guards against still applies when the token is a comment. Single-line quote tokens have the same problem within one line: in `const Q = /"/u; const t = process.env.REVIEW_CAPABILITY_TOKEN; const s = "x";`, the `"` inside the regex opens a string that swallows the read. > > Correction: treat comment tokens the same way as string tokens. Keep a `/* ... */` token that contains a newline as code, as already happens for multi-line literals. Alternatively, run the code-read patterns over both the raw source and the blanked source, and use the blanked version only to discard matches that fall entirely inside a real comment or string. > > What I examined: I copied `COMMENT_OR_STRING`, `CAPABILITY_TOKEN_KEY`, `CAPABILITY_TOKEN_CODE_READS`, `blankProse` and `readsCapabilityToken` from the subject into a plain Node script and ran it. `'const GLOB = /^\\/*/u;\nconst t = process.env.REVIEW_CAPABILITY_TOKEN;\n/* done */'` returned `false`, and so did `'const U = /^https:\\/\\//u; const t = process.env.REVIEW_CAPABILITY_TOKEN;'`. Plain `process.env.REVIEW_CAPABILITY_TOKEN`, bracket access and destructuring all returned `true`. Against the current `src/` tree the scan returns only `src/credentials.ts`, so the test passes today. No current non-test module has a regex containing `\/*` or `\//` (grep), so this is a hole that has not been triggered yet, not a read the test is currently missing. I did not run vitest itself, because there is no `node_modules` in the sandbox. > > To confirm: add a fixture source with a slash-star regex followed by a token read, call `readsCapabilityToken` on it, and check that it returns `false`. lens `general-bug` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M45AF53AQ8DMWXVJQB1KEX3Z` of review `01M45AC6XDDTSZ61889EB2TB9A`
Author
Owner

Fixed in fb3a1eb. The test no longer has its own comment and string tokenizer. readsCapabilityToken runs esbuild's transformSync over each module, which parses regex literals as regex literals and drops comments, and the read patterns run on that output. The two sources you quoted are now fixture cases that must count as reads. I also checked it on the real tree: adding const SLASHES = /^\/*/u; followed by export const leakedToken = process.env.REVIEW_CAPABILITY_TOKEN; to src/markdown/neutralize-html.ts makes the test fail, and it passes again once I removed them.

<!-- gh-feedback:reply-to:126456 --> Fixed in fb3a1eb. The test no longer has its own comment and string tokenizer. `readsCapabilityToken` runs esbuild's `transformSync` over each module, which parses regex literals as regex literals and drops comments, and the read patterns run on that output. The two sources you quoted are now fixture cases that must count as reads. I also checked it on the real tree: adding `const SLASHES = /^\/*/u;` followed by `export const leakedToken = process.env.REVIEW_CAPABILITY_TOKEN;` to `src/markdown/neutralize-html.ts` makes the test fail, and it passes again once I removed them.
jercik marked this conversation as resolved
Lines 37-38
@ -22,0 +34,5 @@
if (literals.some((literal) => CAPABILITY_TOKEN_KEY.test(literal))) {
return true;
}
const code = source.replace(COMMENT_OR_STRING, blankProse);
return CAPABILITY_TOKEN_CODE_READS.some((pattern) => pattern.test(code));

medium — Token-isolation scan misses process.env["REVIEW_CAPABILITY_TOKEN"] when a stray backtick or quote in a regex literal opens a fake multi-line literal

The test is the only module that reads REVIEW_CAPABILITY_TOKEN by name no longer catches a bracket-key read in parts of existing source files. One example is the code just after DESTINATION_UNSAFE in src/markdown/neutralize-html.ts. The old scanner caught that read, and this is the form credentials.ts itself uses (requireEnv("REVIEW_CAPABILITY_TOKEN")).

Here is how it happens. readsCapabilityToken treats a key as read only when a whole token matched by COMMENT_OR_STRING equals "REVIEW_CAPABILITY_TOKEN" (CAPABILITY_TOKEN_KEY is anchored with ^…$). A backtick inside a regex literal opens a template-literal match, and that match runs to the next backtick. One example is const DESTINATION_UNSAFE = /\\[!-/:-@[-{-~]|[\\s<>()\p{Cc}]/gu;. The fake template can cover many lines of real code. Any quoted key in those lines is swallowed into that one token, so it never shows up as a token of its own. blankProsethen keeps the multi-line token as code, as the new comment intends. ButCAPABILITY_TOKEN_CODE_READSmatches only.REVIEW_CAPABILITY_TOKEN` and destructuring, never a quoted name. The new comment covers this case for code reads ("A literal that spans lines … stays as code") but not for string-key reads.

To fix it, add the unanchored quoted-name pattern /(?<quote>["'])REVIEW_CAPABILITY_TOKEN\k<quote>/utoCAPABILITY_TOKEN_CODE_READS. One-line strings and comments are already blanked, so in code` that pattern can only match inside a literal span the scanner kept. That restores the old detection without bringing back the prose false positives the change removed.

I checked this by running the scanner copied verbatim from the test, plus the old pattern list from the diff, under node. On the unmodified tree, both flag only src/credentials.ts. Mutation: I inserted export const leakedToken = process.env["REVIEW_CAPABILITY_TOKEN"]; after the Segment interface, at top level of neutralize-html.ts. The new scanner returned false for that file and the old one returned true, so the test stays green on that mutant. Injecting the bracket form before every line of every non-test src file found 16 such insertion points in neutralize-html.ts (lines 23–38) and 3 in reconcile/summary.ts where only the bracket form went unflagged. With the proposed pattern added, the unmodified tree still yields only src/credentials.ts and the mutant is flagged. I did not run vitest, because no node_modules is present. I found no other guard for this rule: oxlint.config.ts only composes node and vitest presets, and no other test checks token reads. It is rated medium because the test protects the fork-gate boundary described in credentials.ts ("a fork run never reads the capability token"), yet it misses reads of the same shape as the real one, in whole regions of the current tree.

lens test-trimming · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M45AGKBM46A7Y5HN29773MJH of review 01M45AC6XDDTSZ61889EB2TB9A

<!-- review:claim:01M45AGKBM46A7Y5HN29773MJH --> **medium** — Token-isolation scan misses `process.env["REVIEW_CAPABILITY_TOKEN"]` when a stray backtick or quote in a regex literal opens a fake multi-line literal > The test `is the only module that reads REVIEW_CAPABILITY_TOKEN by name` no longer catches a bracket-key read in parts of existing source files. One example is the code just after `DESTINATION_UNSAFE` in `src/markdown/neutralize-html.ts`. The old scanner caught that read, and this is the form `credentials.ts` itself uses (`requireEnv("REVIEW_CAPABILITY_TOKEN")`). > > Here is how it happens. `readsCapabilityToken` treats a key as read only when a whole token matched by `COMMENT_OR_STRING` equals `"REVIEW_CAPABILITY_TOKEN"` (`CAPABILITY_TOKEN_KEY` is anchored with `^…$`). A backtick inside a regex literal opens a template-literal match, and that match runs to the next backtick. One example is `const DESTINATION_UNSAFE = /\\[!-/:-@[-`{-~]|[\\\s<>()\p{Cc}]/gu;`. The fake template can cover many lines of real code. Any quoted key in those lines is swallowed into that one token, so it never shows up as a token of its own. `blankProse` then keeps the multi-line token as code, as the new comment intends. But `CAPABILITY_TOKEN_CODE_READS` matches only `.REVIEW_CAPABILITY_TOKEN` and destructuring, never a quoted name. The new comment covers this case for code reads ("A literal that spans lines … stays as code") but not for string-key reads. > > To fix it, add the unanchored quoted-name pattern <code>/(?&lt;quote&gt;&#91;"'</code>])REVIEW_CAPABILITY_TOKEN\k&lt;quote>/u` to `CAPABILITY_TOKEN_CODE_READS`. One-line strings and comments are already blanked, so in `code` that pattern can only match inside a literal span the scanner kept. That restores the old detection without bringing back the prose false positives the change removed. > > I checked this by running the scanner copied verbatim from the test, plus the old pattern list from the diff, under node. On the unmodified tree, both flag only `src/credentials.ts`. Mutation: I inserted `export const leakedToken = process.env["REVIEW_CAPABILITY_TOKEN"];` after the `Segment` interface, at top level of `neutralize-html.ts`. The new scanner returned false for that file and the old one returned true, so the test stays green on that mutant. Injecting the bracket form before every line of every non-test `src` file found 16 such insertion points in `neutralize-html.ts` (lines 23–38) and 3 in `reconcile/summary.ts` where only the bracket form went unflagged. With the proposed pattern added, the unmodified tree still yields only `src/credentials.ts` and the mutant is flagged. I did not run vitest, because no `node_modules` is present. I found no other guard for this rule: `oxlint.config.ts` only composes `node` and `vitest` presets, and no other test checks token reads. It is rated medium because the test protects the fork-gate boundary described in `credentials.ts` ("a fork run never reads the capability token"), yet it misses reads of the same shape as the real one, in whole regions of the current tree. lens `test-trimming` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M45AGKBM46A7Y5HN29773MJH` of review `01M45AC6XDDTSZ61889EB2TB9A`
Author
Owner

Fixed in fb3a1eb. The scan now reads esbuild's output for each module, so a backtick or quote inside a regex literal can't open a fake string any more. The quoted-name pattern is unanchored and runs on that output alongside the other two, so a bracket key counts wherever it sits. I ran your mutation: export const leakedToken = process.env["REVIEW_CAPABILITY_TOKEN"]; after the Segment interface in neutralize-html.ts now fails the test. A bracket-key read after a backtick regex is also a fixture case now.

<!-- gh-feedback:reply-to:126457 --> Fixed in fb3a1eb. The scan now reads esbuild's output for each module, so a backtick or quote inside a regex literal can't open a fake string any more. The quoted-name pattern is unanchored and runs on that output alongside the other two, so a bracket key counts wherever it sits. I ran your mutation: `export const leakedToken = process.env["REVIEW_CAPABILITY_TOKEN"];` after the `Segment` interface in `neutralize-html.ts` now fails the test. A bracket-key read after a backtick regex is also a fixture case now.
jercik marked this conversation as resolved
test: find capability-token reads with esbuild instead of a tokenizer
Some checks failed
commit-msg / commitlint (pull_request) Successful in 24s
Checks / quality-checks (pull_request) Successful in 48s
Review / Review (pull_request_target) Has been cancelled
fb3a1eb360
The hand-written comment and string tokenizer misread regex literals: a
`/*` or `//` inside one blanked the code after it, and a stray quote or
backtick swallowed a bracket-key read into a fake string. esbuild now
parses each module and drops its comments, and the read patterns run on
its output. Fixture cases pin the regex-literal reads and the prose
mentions the scan must ignore.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Author
Owner

Replying to comment #126450

Round 1, review 01M45AC6XDDTSZ61889EB2TB9A of 499aaab, all five slots delivered.

  • 01M45AF53AQ8DMWXVJQB1KEX3Z, 01M45AGKBM46A7Y5HN29773MJH and 01M45AEEWTV53AS0D0277G2F12 are fixed in fb3a1eb, which finds token reads in esbuild's output instead of a hand-written tokenizer. They're answered in threads #126456, #126457 and #126458.
  • 01M45AH1232V48HFC8H5YQH11D (summary only) is about IMPORT_SPECIFIER, which this PR doesn't change. It's tracked in #45, which replaces that regex with esbuild's import graph, so wrapped, bare and dynamic imports all count.
> Replying to comment #126450 Round 1, review `01M45AC6XDDTSZ61889EB2TB9A` of `499aaab`, all five slots delivered. - `01M45AF53AQ8DMWXVJQB1KEX3Z`, `01M45AGKBM46A7Y5HN29773MJH` and `01M45AEEWTV53AS0D0277G2F12` are fixed in fb3a1eb, which finds token reads in esbuild's output instead of a hand-written tokenizer. They're answered in threads #126456, #126457 and #126458. - `01M45AH1232V48HFC8H5YQH11D` (summary only) is about `IMPORT_SPECIFIER`, which this PR doesn't change. It's tracked in https://code.j4k.dev/j4k-oss/review-wrapper/pulls/45, which replaces that regex with esbuild's import graph, so wrapped, bare and dynamic imports all count.
jercik changed title from test: stop counting prose mentions of the capability token as reads to test: find capability-token reads in esbuild output, not by tokenizing 2026-10-05 06:27:30 +00:00
jercik changed target branch from test/single-capability-token-reader to main 2026-10-05 09:35:02 +00:00
jercik force-pushed test/token-scan-skips-prose from fb3a1eb360
Some checks failed
commit-msg / commitlint (pull_request) Successful in 24s
Checks / quality-checks (pull_request) Successful in 48s
Review / Review (pull_request_target) Has been cancelled
to f04f14bb4f
Some checks failed
commit-msg / commitlint (pull_request) Successful in 15s
Checks / quality-checks (pull_request) Successful in 32s
Review / Review (pull_request_target) Failing after 57s
2026-10-05 09:35:05 +00:00
Compare
jercik merged commit 0fd11bf7dd into main 2026-10-05 09:35:46 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
j4k-oss/review-wrapper!46
No description provided.