test: fail when a second module reads the capability token #44

jercik · 2026-10-05T05:10:14Z

jercik commented

2026-10-05 05:10:14 +00:00

Follows this comment on #39: a test now fails when a non-test module under src/ other than src/credentials.ts reads REVIEW_CAPABILITY_TOKEN by name. It cannot catch a read through a name built at run time.

Stacked on #39; merge after it.

🤖 Generated with Claude Code

Follows [this comment](https://code.j4k.dev/j4k-oss/review-wrapper/pulls/39#issuecomment-125745) on #39: a test now fails when a non-test module under `src/` other than `src/credentials.ts` reads `REVIEW_CAPABILITY_TOKEN` by name. It cannot catch a read through a name built at run time. Stacked on #39; merge after it. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

jercik added 1 commit

2026-10-05 05:10:14 +00:00

test: fail when a second module reads the capability token

commit-msg / commitlint (pull_request) Successful in 22s

Details

Checks / quality-checks (pull_request) Successful in 47s

Details

Review / Review (pull_request_target) Successful in 2m36s

Details

bfebf35c80

The README says exactly one module reads `REVIEW_CAPABILITY_TOKEN`, and
until now nothing failed when a second one did. The test scans the
non-test modules under `src/` for the name used as an environment key,
so it cannot see a key built at run time.

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

jercik referenced this pull request

2026-10-05 05:10:26 +00:00

docs: reduce the fork-gate bullet to its guarantee #39

forgejo-actions commented

2026-10-05 05:12:49 +00:00

Review 01M45PN9MJMKNMJV15K1B6CSNG — head 1f32183010e315f509c74e55013c284f0c063162

Review — j4k-oss/review-wrapper @ `2872c765de`

Scope: diff against base tree e69943819640
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: 01M45PN9P9TH8CY2T0K2QD01Q6
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 01M45PN9PBGZ9JHMGE32Q5JWW6: sandbox infrastructure failed.
writing-quality run 01M45PN9PDX0CE1PKQMVES6DR8: sandbox infrastructure failed.
test-trimming run 01M45PN9PF4M7JC8QBGN1RFCSN: sandbox infrastructure failed.
restated-sets run 01M45PN9PHSW2EETWDRG8MBAA7: sandbox infrastructure failed.
project-docs run 01M45PN9PJGGQDPW81CKCTN6T8: sandbox infrastructure failed.

**Review** `01M45PN9MJMKNMJV15K1B6CSNG` — head `1f32183010e315f509c74e55013c284f0c063162` # Review — j4k-oss/review-wrapper @ 2872c765de48 Scope: diff against base tree `e69943819640` 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: 01M45PN9P9TH8CY2T0K2QD01Q6 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 01M45PN9PBGZ9JHMGE32Q5JWW6: sandbox infrastructure failed. - writing-quality run 01M45PN9PDX0CE1PKQMVES6DR8: sandbox infrastructure failed. - test-trimming run 01M45PN9PF4M7JC8QBGN1RFCSN: sandbox infrastructure failed. - restated-sets run 01M45PN9PHSW2EETWDRG8MBAA7: sandbox infrastructure failed. - project-docs run 01M45PN9PJGGQDPW81CKCTN6T8: sandbox infrastructure failed.

🚀 1

forgejo-actions reviewed

2026-10-05 05:12:50 +00:00

src/credentials.test.ts Outdated

					
				@ -12,0 +12,4 @@

				// Messages elsewhere mention the variable in prose, so only the name spelled as a

				// key counts as a read. A key built at run time is invisible to this scan.

				const CAPABILITY_TOKEN_READS = [

				  /(?<quote>["'])REVIEW_CAPABILITY_TOKEN\k<quote>/u,

low — Capability-token isolation scan misses a key spelled in a backtick literal, contrary to its comment

The new test "is the only module that reads REVIEW_CAPABILITY_TOKEN by name" guards the invariant in src/credentials.ts and README.md that only credentials.ts touches the capability token (a fork run must never read it). The scan does not see a static read written with a template literal: a module containing process.env[REVIEW_CAPABILITY_TOKEN] or requireEnv(REVIEW_CAPABILITY_TOKEN) passes the test. The comment above CAPABILITY_TOKEN_READS says "A key built at run time is invisible to this scan", which tells a maintainer the only blind spot is dynamic keys; an interpolation-free backtick key is fully static and is also invisible.

The first pattern's quote class is ["'], so only double- and single-quoted keys match. The second pattern needs a leading dot and the third needs a destructuring { ... } =, so neither covers a bracketed or argument-position template literal.

Correction: add the backtick to the class, i.e. (["']), so the backreference also accepts REVIEW_CAPABILITY_TOKEN`. Prose mentions inside template literals stay unmatched because the backreference still requires the closing quote to come directly after the name.

Observed: I ran the three CAPABILITY_TOKEN_READS patterns under Node against sample strings. process.env[REVIEW_CAPABILITY_TOKEN] and requireEnv(REVIEW_CAPABILITY_TOKEN) returned false. Destructuring forms and process.env?.REVIEW_CAPABILITY_TOKEN returned true. The same scan over the current src tree flags only src/credentials.ts, so the test passes today. The prose mentions in src/review/session.ts and src/wrapper/orchestrator.ts are correctly ignored.

Not verified: whether the shared @j4k/oxlint-config rejects an interpolation-free template literal and so blocks this spelling before the test runs. node_modules is not in the snapshot. If that config enforces such a rule, the gap only affects the comment's accuracy.

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

**low** — Capability-token isolation scan misses a key spelled in a backtick literal, contrary to its comment > The new test "is the only module that reads REVIEW_CAPABILITY_TOKEN by name" guards the invariant in src/credentials.ts and README.md that only credentials.ts touches the capability token (a fork run must never read it). The scan does not see a static read written with a template literal: a module containing process.env[`REVIEW_CAPABILITY_TOKEN`] or requireEnv(`REVIEW_CAPABILITY_TOKEN`) passes the test. The comment above CAPABILITY_TOKEN_READS says "A key built at run time is invisible to this scan", which tells a maintainer the only blind spot is dynamic keys; an interpolation-free backtick key is fully static and is also invisible. > > The first pattern's quote class is ["'], so only double- and single-quoted keys match. The second pattern needs a leading dot and the third needs a destructuring `{ ... } =`, so neither covers a bracketed or argument-position template literal. > > Correction: add the backtick to the class, i.e. (["'`]), so the backreference also accepts `REVIEW_CAPABILITY_TOKEN`. Prose mentions inside template literals stay unmatched because the backreference still requires the closing quote to come directly after the name. > > Observed: I ran the three CAPABILITY_TOKEN_READS patterns under Node against sample strings. process.env[`REVIEW_CAPABILITY_TOKEN`] and requireEnv(`REVIEW_CAPABILITY_TOKEN`) returned false. Destructuring forms and process.env?.REVIEW_CAPABILITY_TOKEN returned true. The same scan over the current src tree flags only src/credentials.ts, so the test passes today. The prose mentions in src/review/session.ts and src/wrapper/orchestrator.ts are correctly ignored. > > Not verified: whether the shared @j4k/oxlint-config rejects an interpolation-free template literal and so blocks this spelling before the test runs. node_modules is not in the snapshot. If that config enforces such a rule, the gap only affects the comment's accuracy. lens `general-bug` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M457JMNJW07K17GK4HCMP3BM` of review `01M457GJG60PRA8WXE4NQ5Q347`

👍 1

Fixed in 86b79e1. The quoted-key pattern's delimiter class now includes the backtick, so process.env[`REVIEW_CAPABILITY_TOKEN`] counts as a read. Prose mentions still don't match, since the closing delimiter has to follow the name directly. I checked it by adding a template-literal read to src/main.ts: the test failed, and it passed again once I reverted that.

Fixed in 86b79e1. The quoted-key pattern's delimiter class now includes the backtick, so ``process.env[`REVIEW_CAPABILITY_TOKEN`]`` counts as a read. Prose mentions still don't match, since the closing delimiter has to follow the name directly. I checked it by adding a template-literal read to `src/main.ts`: the test failed, and it passed again once I reverted that.

jercik marked this conversation as resolved

jercik added 1 commit

2026-10-05 05:14:32 +00:00

test: count a template-literal key as a capability-token read

commit-msg / commitlint (pull_request) Successful in 17s

Details

Checks / quality-checks (pull_request) Successful in 49s

Details

Review / Review (pull_request_target) Successful in 2m36s

Details

86b79e1904

The key pattern accepted only double and single quotes, so a module
reading process.env[`REVIEW_CAPABILITY_TOKEN`] passed the scan, although
the comment names a key built at run time as its only blind spot.
Backticks now count as quotes. A prose mention inside a template literal
still does not match, because the closing quote must follow the name.

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

jercik commented

2026-10-05 05:16:35 +00:00

Replying to comment #126000

Round 1, review 01M457GJG60PRA8WXE4NQ5Q347 of bfebf35; run 65094 delivered all five slots. Its finding 01M457JMNJW07K17GK4HCMP3BM is fixed in 86b79e1 and answered in thread #126001. The duplicate 01M457K49TA6TSYFBZ2Q3YJ16X is covered by that fix.

> Replying to comment #126000 Round 1, review `01M457GJG60PRA8WXE4NQ5Q347` of `bfebf35`; run 65094 delivered all five slots. Its finding `01M457JMNJW07K17GK4HCMP3BM` is fixed in 86b79e1 and answered in thread #126001. The duplicate `01M457K49TA6TSYFBZ2Q3YJ16X` is covered by that fix.

forgejo-actions reviewed

2026-10-05 05:17:08 +00:00

src/credentials.test.ts Outdated

					
				@ -12,0 +14,4 @@

				const CAPABILITY_TOKEN_READS = [

				  /(?<quote>["'`])REVIEW_CAPABILITY_TOKEN\k<quote>/u,

				  /\.REVIEW_CAPABILITY_TOKEN\b/u,

				  /\{[^{}]*\bREVIEW_CAPABILITY_TOKEN\b[^{}]*\}[\s,}]*=/u,

low — Capability-token read scan misses a type-annotated destructure of process.env, so a second reader keeps the isolation test green

The new test is the only module that reads REVIEW_CAPABILITY_TOKEN by name passes when another module reads the token through a TypeScript-annotated destructure. Examples: const { REVIEW_CAPABILITY_TOKEN }: NodeJS.ProcessEnv = process.env; or a parameter function token({ REVIEW_CAPABILITY_TOKEN }: NodeJS.ProcessEnv) called with process.env. Either spelling names the variable as a key. The test's own comment limits the blind spot to runtime-built keys: "only the name spelled as a key counts as a read. A key built at run time is invisible to this scan." These spellings therefore fall inside the promise the comment makes. The token-isolation invariant in src/credentials.ts depends on this guard: "a fork run never reads the capability token".

The destructuring pattern is /\{[^{}]*\bREVIEW_CAPABILITY_TOKEN\b[^{}]*\}[\s,}]*=/u. It requires = right after the closing brace(s). A : Type annotation sits between } and =, and in a parameter = never appears, so the pattern does not match. The other two patterns need the name either wrapped in quotes or preceded by ., so they miss these spellings too.

Correction: accept a type annotation after the pattern, e.g. /\{[^{}]*\bREVIEW_CAPABILITY_TOKEN\b[^{}]*\}[\s,}]*[:=]/u. This keeps the existing destructure detection and adds both annotated forms.

How I checked: I ran the three CAPABILITY_TOKEN_READS patterns in node against sample lines. Both annotated forms above returned false. { REVIEW_CAPABILITY_TOKEN = "" } = process.env, { REVIEW_CAPABILITY_TOKEN: t } = process.env, { env: { REVIEW_CAPABILITY_TOKEN } } = process, process.env?.REVIEW_CAPABILITY_TOKEN and process.env["REVIEW_CAPABILITY_TOKEN"] returned true, and so did the prose template rotate REVIEW_CAPABILITY_TOKEN now. I ran the patterns over every non-test src/**/*.ts file. Only src/credentials.ts matches, and the proposed [:=] variant also matches only that file, so the change adds no false positive in the current tree (src/wrapper/orchestrator.ts and src/review/session.ts mention the name in prose). I did not check whether an oxlint rule forbids annotated destructuring of process.env. If such a rule exists, the gap is narrower but the parameter form stays open.

lens test-trimming · arm default · tally 2 valid / 0 invalid / 0 uncertain
claim 01M457V0J56VVCQSTFGXDQP605 of review 01M457RE65WG21QRPFJK1TNCHQ

**low** — Capability-token read scan misses a type-annotated destructure of process.env, so a second reader keeps the isolation test green > The new test `is the only module that reads REVIEW_CAPABILITY_TOKEN by name` passes when another module reads the token through a TypeScript-annotated destructure. Examples: `const { REVIEW_CAPABILITY_TOKEN }: NodeJS.ProcessEnv = process.env;` or a parameter `function token({ REVIEW_CAPABILITY_TOKEN }: NodeJS.ProcessEnv)` called with `process.env`. Either spelling names the variable as a key. The test's own comment limits the blind spot to runtime-built keys: "only the name spelled as a key counts as a read. A key built at run time is invisible to this scan." These spellings therefore fall inside the promise the comment makes. The token-isolation invariant in `src/credentials.ts` depends on this guard: "a fork run never reads the capability token". > > The destructuring pattern is `/\{[^{}]*\bREVIEW_CAPABILITY_TOKEN\b[^{}]*\}[\s,}]*=/u`. It requires `=` right after the closing brace(s). A `: Type` annotation sits between `}` and `=`, and in a parameter `=` never appears, so the pattern does not match. The other two patterns need the name either wrapped in quotes or preceded by `.`, so they miss these spellings too. > > Correction: accept a type annotation after the pattern, e.g. `/\{[^{}]*\bREVIEW_CAPABILITY_TOKEN\b[^{}]*\}[\s,}]*[:=]/u`. This keeps the existing destructure detection and adds both annotated forms. > > How I checked: I ran the three `CAPABILITY_TOKEN_READS` patterns in node against sample lines. Both annotated forms above returned false. `{ REVIEW_CAPABILITY_TOKEN = "" } = process.env`, `{ REVIEW_CAPABILITY_TOKEN: t } = process.env`, `{ env: { REVIEW_CAPABILITY_TOKEN } } = process`, `process.env?.REVIEW_CAPABILITY_TOKEN` and `process.env["REVIEW_CAPABILITY_TOKEN"]` returned true, and so did the prose template ``rotate REVIEW_CAPABILITY_TOKEN now``. I ran the patterns over every non-test `src/**/*.ts` file. Only `src/credentials.ts` matches, and the proposed `[:=]` variant also matches only that file, so the change adds no false positive in the current tree (`src/wrapper/orchestrator.ts` and `src/review/session.ts` mention the name in prose). I did not check whether an oxlint rule forbids annotated destructuring of `process.env`. If such a rule exists, the gap is narrower but the parameter form stays open. lens `test-trimming` · arm `default` · tally 2 valid / 0 invalid / 0 uncertain claim `01M457V0J56VVCQSTFGXDQP605` of review `01M457RE65WG21QRPFJK1TNCHQ`

👍 1

Fixed in 1f32183. The destructuring pattern now accepts : as well as = after the closing brace, so both annotated forms count as reads. I checked it by appending export function mutatedRead({ REVIEW_CAPABILITY_TOKEN }: NodeJS.ProcessEnv) to src/main.ts: the test failed, and it passed again once I reverted that. Over the current tree the patterns still match only src/credentials.ts.

An unannotated parameter destructure is still not caught, because ) follows the brace. Under strict, such a parameter is an implicit any unless a caller's type supplies one, so I left ) out rather than flag an object literal passed to a call that mentions the name in a message.

Fixed in 1f32183. The destructuring pattern now accepts `:` as well as `=` after the closing brace, so both annotated forms count as reads. I checked it by appending `export function mutatedRead({ REVIEW_CAPABILITY_TOKEN }: NodeJS.ProcessEnv)` to `src/main.ts`: the test failed, and it passed again once I reverted that. Over the current tree the patterns still match only `src/credentials.ts`. An unannotated parameter destructure is still not caught, because `)` follows the brace. Under `strict`, such a parameter is an implicit `any` unless a caller's type supplies one, so I left `)` out rather than flag an object literal passed to a call that mentions the name in a message.

jercik marked this conversation as resolved

jercik added 1 commit

2026-10-05 05:27:36 +00:00

test: count a type-annotated destructure as a capability-token read

commit-msg / commitlint (pull_request) Successful in 17s

Details

Checks / quality-checks (pull_request) Successful in 50s

Details

Review / Review (pull_request_target) Failing after 1m4s

Details

1f32183010

The destructuring pattern required `=` right after the closing brace,
so `const { REVIEW_CAPABILITY_TOKEN }: NodeJS.ProcessEnv = process.env`
and an annotated parameter destructure kept the isolation test green.
It now accepts `:` there too.

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

jercik commented

2026-10-05 05:27:46 +00:00

Replying to comment #126000

Round 2, review 01M457RE65WG21QRPFJK1TNCHQ of 86b79e1, all five slots delivered. Its finding 01M457V0J56VVCQSTFGXDQP605 is fixed in 1f32183 and answered in thread #126010. The duplicate 01M457V801SHGDPHGK8Z17KKT3 is covered by that fix.

> Replying to comment #126000 Round 2, review `01M457RE65WG21QRPFJK1TNCHQ` of `86b79e1`, all five slots delivered. Its finding `01M457V0J56VVCQSTFGXDQP605` is fixed in 1f32183 and answered in thread #126010. The duplicate `01M457V801SHGDPHGK8Z17KKT3` is covered by that fix.

forgejo-actions reviewed

2026-10-05 05:31:43 +00:00

src/credentials.test.ts

					
				@ -12,0 +12,4 @@

				// Messages elsewhere mention the variable in prose, so only the name spelled as a

				// key counts as a read. A key built at run time is invisible to this scan.

				const CAPABILITY_TOKEN_READS = [

				  /(?<quote>["'`])REVIEW_CAPABILITY_TOKEN\k<quote>/u,

low — Capability-token read scan counts a backtick code span in a comment as a read, failing the isolation test on prose mentions

The new isolation test is the only module that reads REVIEW_CAPABILITY_TOKEN by name fails for any src module whose comment or plain string writes the variable as a markdown code span, e.g. // Never forward REVIEW_CAPABILITY_TOKEN to the agent. That is a prose mention, which the comment above the patterns says the scan is built to ignore: "Messages elsewhere mention the variable in prose, so only the name spelled as a key counts as a read." A contributor documenting the variable in the repo's usual style gets a red test that claims their module reads the token.

The first pattern accepts a backtick as the quote character, /(?<quote>["'])REVIEW_CAPABILITY_TOKEN\k<quote>/u, and it runs over the whole file text, comments included. A code span puts a backtick directly on both sides of the name, so it matches. The repo writes identifiers this way in comments: src/credentials.tssays "The runner exposes the action'sexpected-service-origininput under this name", 13 non-test comment lines insrccontain a backtick code span, and the README writes the variable itself as ``REVIEW_CAPABILITY_TOKEN`` five times. The third pattern has the same problem with a prose string inside an object literal followed by a ternary colon:bad ? { hint: "rotate REVIEW_CAPABILITY_TOKEN" } : undefinedmatches{[^{}]\bREVIEW_CAPABILITY_TOKEN\b[^{}]}[\s,}]*[:=]`.

To fix this, strip // and /* */ comments before scanning. Then accept the backtick quote only for a template literal used as a key, such as env[`REVIEW_CAPABILITY_TOKEN`], or drop it from the first pattern.

Observed: I copied the three patterns and sourceFiles into a Node script and ran it over the unpacked tree. It reports only src/credentials.ts, so the test passes on this tree. The same pattern returns true for the text // Never forward REVIEW_CAPABILITY_TOKEN to the agent., and the pattern set returns true for the ternary probe above. No current src file contains either form, so the failure needs a future edit and is not live. The patterns are copied verbatim from the test, so the probe matches the test's own behavior.

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

**low** — Capability-token read scan counts a backtick code span in a comment as a read, failing the isolation test on prose mentions > The new isolation test `is the only module that reads REVIEW_CAPABILITY_TOKEN by name` fails for any `src` module whose comment or plain string writes the variable as a markdown code span, e.g. `// Never forward `REVIEW_CAPABILITY_TOKEN` to the agent.` That is a prose mention, which the comment above the patterns says the scan is built to ignore: "Messages elsewhere mention the variable in prose, so only the name spelled as a key counts as a read." A contributor documenting the variable in the repo's usual style gets a red test that claims their module reads the token. > > The first pattern accepts a backtick as the quote character, <code>/(?<quote>["'</code>])REVIEW_CAPABILITY_TOKEN\k<quote>/u`, and it runs over the whole file text, comments included. A code span puts a backtick directly on both sides of the name, so it matches. The repo writes identifiers this way in comments: `src/credentials.ts` says "The runner exposes the action's `expected-service-origin` input under this name", 13 non-test comment lines in `src` contain a backtick code span, and the README writes the variable itself as `` `REVIEW_CAPABILITY_TOKEN` `` five times. The third pattern has the same problem with a prose string inside an object literal followed by a ternary colon: `bad ? { hint: "rotate REVIEW_CAPABILITY_TOKEN" } : undefined` matches `\{[^{}]*\bREVIEW_CAPABILITY_TOKEN\b[^{}]*\}[\s,}]*[:=]`. > > To fix this, strip `//` and `/* */` comments before scanning. Then accept the backtick quote only for a template literal used as a key, such as `` env[`REVIEW_CAPABILITY_TOKEN`] ``, or drop it from the first pattern. > > Observed: I copied the three patterns and `sourceFiles` into a Node script and ran it over the unpacked tree. It reports only `src/credentials.ts`, so the test passes on this tree. The same pattern returns `true` for the text `// Never forward `REVIEW_CAPABILITY_TOKEN` to the agent.`, and the pattern set returns `true` for the ternary probe above. No current `src` file contains either form, so the failure needs a future edit and is not live. The patterns are copied verbatim from the test, so the probe matches the test's own behavior. lens `general-bug` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M458KM7J7NJA94HEFQTW7B61` of review `01M458GC4R76134GRQ6GCFRBT3`

🚀 1

jercik marked this conversation as resolved

jercik referenced this pull request

2026-10-05 05:59:00 +00:00

test: read the credentials isolation imports from the esbuild graph #45

jercik referenced this pull request

2026-10-05 06:00:17 +00:00

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

jercik commented

2026-10-05 06:00:46 +00:00

Replying to review comment #126092

Tracked in #46, which is stacked on this PR. The scan there blanks comments and one-line strings before the code patterns run, and counts a string literal as a key only when it is exactly the name. A backtick code span in a comment and the ternary prose string no longer count, while process.env[`REVIEW_CAPABILITY_TOKEN`] and the other key spellings still do. The test passes today, so this PR keeps its scan as it is.

> Replying to review comment #126092 Tracked in https://code.j4k.dev/j4k-oss/review-wrapper/pulls/46, which is stacked on this PR. The scan there blanks comments and one-line strings before the code patterns run, and counts a string literal as a key only when it is exactly the name. A backtick code span in a comment and the ternary prose string no longer count, while ``process.env[`REVIEW_CAPABILITY_TOKEN`]`` and the other key spellings still do. The test passes today, so this PR keeps its scan as it is.

jercik commented

2026-10-05 06:00:48 +00:00

Replying to comment #126000

Round 3, review 01M458GC4R76134GRQ6GCFRBT3 of 1f32183. Neither finding breaks what this PR adds, so both go to follow-ups and this branch stays at 1f32183.

01M458MP2PD8H6ZQM3R1NWVR34 (medium, summary only): the import scan stops at the first newline, so a wrapped import yields no specifier. Tracked in #45, which targets main, where the scan and both import tests already live. There the gap before from crosses lines but not statements, and the builtins test fails when it finds no import. A wrapped importer of credentials.ts fails the new scan and passed the old one.
01M458KM7J7NJA94HEFQTW7B61 (low, thread #126092): tracked in #46.

> Replying to comment #126000 Round 3, review `01M458GC4R76134GRQ6GCFRBT3` of `1f32183`. Neither finding breaks what this PR adds, so both go to follow-ups and this branch stays at 1f32183. - `01M458MP2PD8H6ZQM3R1NWVR34` (medium, summary only): the import scan stops at the first newline, so a wrapped import yields no specifier. Tracked in https://code.j4k.dev/j4k-oss/review-wrapper/pulls/45, which targets `main`, where the scan and both import tests already live. There the gap before `from` crosses lines but not statements, and the builtins test fails when it finds no import. A wrapped importer of `credentials.ts` fails the new scan and passed the old one. - `01M458KM7J7NJA94HEFQTW7B61` (low, thread #126092): tracked in https://code.j4k.dev/j4k-oss/review-wrapper/pulls/46.

jercik changed target branch from docs/fork-gate-bullet-guarantee to main

2026-10-05 09:34:58 +00:00

jercik merged commit a804d845b2 into main

2026-10-05 09:35:00 +00:00

jercik referenced this pull request from a commit

2026-10-05 09:35:01 +00:00

test: fail when a second module reads the capability token (#44)

Sign in to join this conversation.

No reviewers

No labels

No milestone