[jayden1711]

Fixes #966

Critical set: yes. src/push-check.ts (the push guard), src/guardrails/glob.ts (the guardrails) and test/push-check.test.ts are locked as self-modification in src/review.ts (SELF_MODIFICATION_PARTS and the locked test list); test/push-check.test.ts is also in .worklane/review.yaml (measured: rg -n "push-check|guardrails/" src/review.ts .worklane/review.yaml). Two independent reviews are needed.

What changed

Three commits, one per item of the issue.

1. Range ends are commits only (ef7681d2). Main already refuses a tree, blob or tag-of-a-tree end since push check: refuse, never throw, when git fails mid-read; read and push by commit id (#981) #1088's ^{commit} peel in commitIds (measured, below). This commit makes the rule structural: judge now takes a branded CommitId that only commitIds produces, so no later caller can hand it a name or a tree id (type-level; inferred from npm run typecheck passing). A new test pins the refusal for a tree head, a blob head, a tag-of-a-tree head, a tree base and a blob base, and checks that checkedPush pushes nothing.
2. Glob ? and astral characters (78a93f37). In globToRegExp, ? now compiles to (?:[\uD800-\uDBFF][\uDC00-\uDFFF]|[^/]): one surrogate pair or one UTF-16 unit. No u flag is added, so other regex semantics stay as they were for every caller. Because the new regex is a superset of the old [^/], no glob matches fewer paths: ?? still matches a one-emoji name.
3. NFC/NFD for push.refuse_paths (e61eea7f). The push check matches each touched path through one matcher. It tries the path as written, in NFC and in NFD against each glob in those same three spellings. This only ever widens what is refused, and the issue's pending case-fold handling (Push gate routes on main: log config, crafted tree entries, replace refs in the size check, skill dir symlinks, case folding #951 item 5) can be added in the same spellings function. A changelog fragment is at changelog.d/966-push-check-range-glob-unicode.md.

Review of every globToRegExp caller for the ? change (inferred, from reading the code)

• Callers that test paths directly: the protected, built-in, agents-md and editable globs in guardrails/engine.ts, the review categories, self-modification and money globs in review.ts, vacuity test/app globs, weakening, hotspots, owners, chat, browser gate, pr-rules docs/lockfile/generated, and push refuse_paths. Each sees only a superset of its old matches.
  • For deny-style lists, wider is stricter.
  • For allow-style lists (agent-editable, docs-only, lockfile/generated skips, test globs), the only paths added are those where a ? meets a character outside the BMP. That is what ? is documented to mean. No built-in glob list in src/ contains a ? (measured with rg for quoted glob strings holding ? in the guardrails, review, pr-rules, vacuity, weakening, hotspots, chat, browser gate, coordinator, config and push-check sources). So only project-configured globs see the difference, plus the globs review.ts generates from a project's money regexes, where [^/] becomes ?. A wider money glob means more money paths, which is stricter, and the money-list suites passed in the full run (measured).
• review.ts compares re.source === globToRegExp(re.glob).source and keys by source/flags. Both sides are built by the same function, so both stay consistent.
• guardrails/access.ts splices globToRegExp(s).source.slice(1, -1) into new RegExp(..., 's'). The flags are the same and the ^/$ it strips are unchanged.
• Not changed: src/confusables.ts has its own glob matcher (globParts), whose ? is still one code unit. It is a separate matcher in the review levels and isn't named by this issue, so it is left as a follow-up.

Evidence

Fail before, on origin/main 246e3f68 with this PR's two test files copied in (measured: npx tsc -p . && node --import ./dist/test/isolate.js --test dist/test/glob.test.js dist/test/push-check.test.js in a detached worktree):

not ok 1 - ? matches one character outside the BMP, as it does one inside it (#966)
not ok 9 - refuse_paths: a ? stands for one character outside the BMP, as ** does (#966)
not ok 10 - refuse_paths: a name in NFD is refused by a glob in NFC, and the other way round (#966)
# pass 24
# fail 3

For item 1, the same run on d5650da8 (main before #1088) fails the new tree/blob test. The push check passed a tree head with no names read, and only the remote refused it (measured): "remote: error: cannot update ref 'refs/heads/task': trying to write non-commit object ...". On main that test passes, because #1088 already refuses such an end (measured, same command).

Pass after, on this branch (measured, same command): # pass 27, # fail 0. This includes the no-narrowing test, which compares the old and new ? on 13 globs over about 22k generated paths (emoji, lone surrogates, NFC and NFD accents, slashes). Every path the old regex matched, the new one matches.

Other checks (all measured):

• npm run typecheck: exit 0.
• npm run check:denylist: denylist: 1693 files clean.
• node scripts/push-gate.mjs --range origin/main..HEAD: push gate: 3 commit(s) and 1 tip(s) clean.
• Full suite, once, flock /var/tmp/wl-coord/fulltest.lock nice -n 15 ionice -c3 npm test: 6037 tests, 5924 pass, 18 fail, 69 skipped, 26 todo.
  • 17 of the 18 also fail on origin/main in this environment when the same files are run there: 15 in machine-sudoers-* (13 in closed, one each in narrow and removal), plus one each in coordinator (worktree dependency cache) and desktop-removed (round 19 packages). On main those files gave 117 pass, 17 fail, the same tests (measured).
  • The 18th (discord-seam-ci) was a port collision (EADDRINUSE 127.0.0.1:46009). It passed when rerun alone (measured: # pass 1).
  • None is in a file this PR touches. CI is the authoritative run.

Not done / follow-ups

• Case folding for refuse_paths and the workflow globs is still Push gate routes on main: log config, crafted tree entries, replace refs in the size check, skill dir symlinks, case folding #951 item 5. It is not done here, though spellings is where it would go.
• NFC/NFD matching is only in the push check. Protected paths in guardrails/engine.ts, which has another owner, still compare spellings as written. Look-alike spellings at review time are covered separately by the confusable-path skeleton in review.ts (inferred).
• confusables.ts globParts: its ? is still one UTF-16 unit (inferred from reading the code).

Round 2 (head 691ed1c): answers R123a and R123b

This replaces round 1's item 2 description and its "Review of every caller" section, which were wrong. Round 1 widened ? for every caller. R123a measured that this made one money-list reading narrower than main. R123b found that it also widened allow-style globs and left read-denials disagreeing with the hook. Critical set: yes, still. Every changed src/ file and the locked push-check test are as before, and the PR now also touches src/guardrails/access.ts, src/read-denials.ts and src/review.ts.

What changed

• One compiler (src/guardrails/glob.ts). One parser builds two readings of ?: one UTF-16 unit (main) and one character (exactly what the u flag gives, written without it). From them come globRegExp(glob, style) and the linear-time globMatcher(glob, style), where style is required and is 'deny' or 'allow'.
  • deny is the union of the two readings, so it never matches less than main.
  • allow is their intersection, so it never matches more than main.
  • A glob both readings read alike (any glob without ?) compiles to the same source as main's.
• read-denials' own matcher is deleted. It now uses globMatcher. Its positive patterns are deny and its ! exceptions are allow, as in the hook (access.ts: secret globs and search filters are deny, ! exceptions are allow).
• Every caller chose a style (from reading the code, inferred; the full list is in the changelog fragment).
  • Deny: refuse_paths, secrets, money globs, review categories as membership, self-modification, the weakening scan, revert_first, ui_paths, hotspots, owners, the tamper and vacuity-command test globs, and the study eval-cover check.
  • Allow: ! exceptions, docs/tests/harness-config when they exempt a path from the app code, docs-only, lockfile/generated skips, experiment editable paths, and the money-list cover test (R123a's fix).
• What earlier versions counted stays main's exact reading.
  • A money glob's before: the new glob test shows a path in the money set that isn't in moneyBefore. So a widened money glob can't take a large change out of the large-change L2.
  • keep(...): main's kept readings.
  • Both use globToRegExp, now documented as the pre-push check: refuse a non-commit range end; glob ? and astral characters; NFC/NFD #966 reading.
• globToRegExp is a closed list. A never-grow table in test/glob.test.ts names the only files that may still call it: review.ts (the uses just above), and guardrails/engine.ts, merge-policy-vacuity.ts and vacuity.ts. The last three are owned by guardrails: refuse more spellings git reads as a hook-skipping push (#993) [critical: tool guard] #1101 and fix(vacuity): pair moved code git doesn't pair, only ever taking measure away (#1014) #1068, and they read exactly as on main (follow-up). A new caller elsewhere fails that test.
• Tests:
  • Sandbox: a task's hidden paths and the project's secret paths are denied by the OS sandbox, not only by the guard hook #890 corpora gain astral characters and lone surrogates, and both styles are checked against an independent oracle (the unit reading OR/AND the u-flag reading).
  • R123a's and R123b's probes.
  • The two surviving mutants are killed: a mixed-spelling glob, and secrets/cafe* against an NFC name.
• Changelog fragment rewritten: each caller's style, and what still reads as before (engine.ts, the vacuity files, confusables.ts).

Fail before (on round 1's head e61eea7), measured

• Command: npx tsc -p . && node --import ./dist/test/isolate.js --test dist/test/glob-styles.test.js dist/test/read-denials-matcher.test.js, in a detached worktree with this round's test/glob-styles.test.ts copied in. For the Sandbox: a task's hidden paths and the project's secret paths are denied by the OS sandbox, not only by the guard hook #890 file, e61eea7's own test was used with only keys/?.pem, **/?, ?/**, [ab]? and four astral paths added to its corpora.
• Result: 5 fail, 2 pass. All four caller probes fail:
  • R123a billing/**/? # see /🔑;
  • the money before reading;
  • R123b's !keys/?.pem exception;
  • a docs ? lowering a level ('L0' !== 'L1').
• Sandbox: a task's hidden paths and the project's secret paths are denied by the OS sandbox, not only by the guard hook #890 fails on "keys/?.pem" on "keys/🔑.pem", so that test would have caught item 2.

Pass after (head 691ed1c), measured

• node --import ./dist/test/isolate.js --test dist/test/glob.test.js dist/test/glob-styles.test.js dist/test/read-denials-matcher.test.js dist/test/push-check.test.js: 39 pass, 0 fail.
• Mutants on the compiled head (each edited into dist, then the files above rerun), all caught:
  • spellings without NFD: 1 fail;
  • globs not normalised: 1 fail;
  • globRegExp always deny: 7 fail;
  • always allow: 6 fail;
  • globMatcher always deny: 3 fail.
• Every test file that imports a changed module (86 files, --test-timeout=180000): 775 pass, 2 fail, plus the adversarial file timing out at 180 s under that load.
  • The 2 failures are coordinator (worktree dependency cache) and desktop-removed (round 19 packages). Both fail on origin/main in this environment too (measured in round 1).
  • adversarial.test.js alone, with --test-timeout=900000: 269 pass, 0 fail, in 11m46s.
  • The full npm test wasn't rerun this round, because the brief allows one; CI is authoritative.
• npm run typecheck: exit 0. npm run check:denylist: 1695 files clean. node scripts/push-gate.mjs --range origin/main..HEAD: 5 commits and 1 tip clean.
• git merge-tree --write-tree:
  • origin/main (246e3f6): clean.
  • fix(vacuity): pair moved code git doesn't pair, only ever taking measure away (#1014) #1068 (29bb7f6) and guardrails: refuse more spellings git reads as a hook-skipping push (#993) [critical: tool guard] #1101 (62b020d) heads: clean.
  • git: one runner for every engine git call, with replace refs and grafts off (#807) [PROTECTED: owner merges] #927 (021f6f1): conflicts. It already conflicts with main on push-check.ts, coordinator.ts and others, and this PR adds read-denials.ts to that overlap.

Weakening scan (inferred unless marked)

• No caller matches more than main where a match allows, and none matches less where a match denies.
  • For the callers moved to a style, this follows from the union and intersection construction, and the corpus test checks exactly that against the oracle (measured).
  • engine.ts and the vacuity files call the unchanged pre-push check: refuse a non-commit range end; glob ? and astral characters; NFC/NFD #966 reading.
• One residual: a review category used for raising, when it is also docs or tests, now counts a deny-wider path as in the category but not exempt. That only raises.
• The access.ts shell-word filter splices each segment's deny source. That is a per-segment union, a superset of the whole-glob union, so it is still only wider for a filter whose match refuses.

Follow-ups

• Move guardrails/engine.ts (protected, agent-editable and AGENTS.md paths) and the vacuity files to a style once guardrails: refuse more spellings git reads as a hook-skipping push (#993) [critical: tool guard] #1101 and fix(vacuity): pair moved code git doesn't pair, only ever taking measure away (#1014) #1068 land.
• confusables.ts' look-alike glob matcher still reads ? as one unit.
• NFC/NFD matching is still only in push.refuse_paths.
• Case folding is still Push gate routes on main: log config, crafted tree entries, replace refs in the size check, skill dir symlinks, case folding #951 item 5.

[jayden1711]

Independent final check (R123a), posted by the coordinator.

VERDICT: FAIL @e61eea7fa47e11d4c5948ca75a2037722d39da4e (final check)

Reviewer R123a. Head checked at start and end (gh pr view 1123 --json headRefOid): unchanged.

Blocking finding

The ? change narrows a money-path reading in src/review.ts versus main (measured), so the body's "each caller sees only a superset of its old matches" is false for one caller. moneyListLines (review.ts:832) uses the glob inversely. When the glob left after cutting a # comment covers the whole line, it reads that narrower glob. Otherwise it reads glob**. A wider ? makes "covers" true more often, so the reading gets narrower. Line billing/**/? # see /🔑 (probe R123a-money.mjs, moneyListEntries):

• main: ^billing\/(?:.*\/)?[^/].*$, so billing/charge.ts is a money path.
• head: ^billing\/(?:.*\/)?(?:pair|[^/])$, so billing/charge.ts is not a money path.

The brute probe (R123a-probe.mjs) found 13 of 4,632 glob+comment pairs that differ, all of the shape …**/? # …/<one astral char>. The input is contrived. Head now treats the emoji the way main already treats /x. Still, this is a regression versus main in a review-level guard (brief FAIL bar items a and d). Two possible fixes:

• make the cover test fail closed: read the cut glob only when the old one-unit ? reading covers the line too;
• or get the owner's OK and correct the PR body.

Claims verified (all measured unless marked)

• Fail before, pass after: I copied the PR's two test files onto origin/main 246e3f68, ran npx tsc -p . && node --import ./dist/test/isolate.js --test dist/test/glob.test.js dist/test/push-check.test.js and got 3 of 27 failing (tests 1, 9 and 10). Head: 27/27. On d5650da8 (first parent of push check: refuse, never throw, when git fails mid-read; read and push by commit id (#981) #1088's merge e80bac69), the tree/blob test fails with the remote's "trying to write non-commit object", as the body says.
• Item 1: main already refuses a non-commit end (^{commit}, de6922d, push check: refuse, never throw, when the repo changes between git calls #981 via PR push check: refuse, never throw, when git fails mid-read; read and push by commit id (#981) #1088). The changelog's "as since push check: refuse, never throw, when the repo changes between git calls #981" is accurate.
• Mutants on the compiled head:
  • Killed: no ^{commit} peel (test 23), the path tested as written only (test 10), NFC dropped (test 10), and a pair-only ? (tests 1, 2, 9, 10).
  • Survived: the glob tested as written only, and NFD dropped. These are test gaps, not wrong behaviour.
• NFC/NFD coverage: the code matches 3 path spellings against 3 glob spellings. My probe through pushProblems refused all four shapes it should and passed the unrelated path:
  • a prefix glob ending in NFD against an NFC name;
  • a glob in one mixed spelling against a name in another;
  • a directory glob in NFC against an NFD name;
  • Hangul in NFC against NFD.
• Built-in glob lists: none contains ?. Checked at runtime: DEFAULT/LOCKED categories, SELF_MODIFICATION_PARTS, BUILTIN_PROTECTED, AGENT_EDITABLE, AGENTS_MD, DOCS_ONLY, LOCKFILE, FALLBACK_TEST, and weakening's TEST_PATHS, GUARD_FILES and NEVER_GROW.
• npm run typecheck: exit 0. npm run check:denylist: 1694 files clean. node scripts/push-gate.mjs --range origin/main..HEAD: clean. The changelog fragment is present.
• CI: 7 of 8 test shards finished; 6 passed. Shard 3/4 on node 22 failed in merge-idempotent.test.js, where git clone reported "failed to copy file … No such file". That file passes locally at head (20/20), and the PR doesn't touch it, so I infer a flake. Shard 2/4 was still pending on both node versions, so required CI is not green yet.

Weakening scan flags

• push refuse_paths gets a wider ? and the NFC/NFD spellings: strengthening.
• Protected, AGENTS.md, self-modification and review categories, money globs (as globs), fix-forward, and owners/hotspots routing: strengthening or neutral.
• review.ts:832 money comment-cut covers test: real weakening (above).
• Allow-style lists that projects configure (AGENT_EDITABLE-style project paths, generated size skips, vacuity test_globs, experiments editable, docs/lockfile skips): limit. They widen only when a project's glob has ? where a path has an astral character, which is the documented meaning. No built-in list has ? (measured). The body says this already.
• review.ts:1244 source equality and access.ts source splicing: same function and flags on both sides, so neutral (inferred).
• confusables.ts globParts is still one unit per ?: limit, a follow-up the body names.

Follow-ups (not blocking)

• Add tests that kill the two surviving mutants: a mixed-spelling glob, and the glob secrets/cafe* against an NFC name.
• Rerun the flaky shard.

[jayden1711]

Independent adversarial review (R123b), posted by the coordinator.

VERDICT: FAIL @e61eea7fa47e11d4c5948ca75a2037722d39da4e (adversarial)

R123b, PR #1123 (#966). Head e61eea7 confirmed at start and end (gh pr view, git ls-remote); main 246e3f6.

FAIL reason (bar a/b: overclaim)

The changelog says ? now matches an astral character "in every glob the engine reads (... and the rest)". That is false for src/read-denials.ts globMatcher, whose doc comment and test/read-denials-matcher.test.ts (#890) say it reads every glob "exactly as the guard hook's globToRegExp does". The PR doesn't name it, unlike confusables.ts.

• measured (node probe-glob.mjs on the head's dist): globToRegExp('keys/?.pem') matches keys/🔑.pem, but globMatcher doesn't. The same holds for **/?, ?/** and [ab]?.
• inferred: the hook (access.ts) and the sandbox read-deny entries from expand() now disagree on such names. Not a regression versus main. The equivalence tests still pass (measured, 40 pass) because neither corpus has an astral character.
• Fix: have globMatcher's one step take a surrogate pair (with astral cases in the corpora), or reword the changelog.

Other findings (not blocking)

• Fail before / pass after, measured: head tests on main's src (npx tsc -p . && node --import ./dist/test/isolate.js --test dist/test/glob.test.js dist/test/push-check.test.js) fail 3 (tests 1, 9, 10). The head passes 27/27.
• Range ends, measured (probe-push.mjs): refused are annotated tags of a tree or blob, a tag of a tag of a tree, ann-tree^{}, a zero id as base or head (as on main), --all and an empty head. Judged normally are commit tags (single and double), ^{}, @{u} and abbreviated ids. A branch named like a tree-id prefix is judged by its commit id, so it doesn't fail open.
• NFC/NFD, measured: Hangul both ways and with ??, a decomposed directory, mixed globs, a trailing /, café*, U+212B and reordered marks are all refused. Ordinary paths pass clean.
• Mutants, measured (mutate.sh): dropping the [^/] alternative, dropping ^{commit} and leaving the path unnormalised are each caught. Two survive (follow-up): dropping NFD from spellings, and not normalising the globs. The probe shows a mixed glob needs glob normalisation.
• npm run typecheck exit 0; denylist clean.
• CI (gh pr checks): shard 3/4 on node 22 failed in merge-idempotent Plugins (6/6): update offers, update diffs, and what each run loaded #533 (git clone ... failed to copy file). The file isn't touched and shard 3 on node 24 passed, so it looks like a flake (inferred). Shards 1 and 2 are pending.

Weakening scan flags

weak-scan.mjs (measured): 1 reason, "removes or changes a line of a guard":

• glob.ts ?: deny lists get stricter. Allow-style globs widen only where ? meets an astral character: agent-editable, docs-only, lockfile/generated skips, test globs, experiments' editable list, and secret ! exceptions (the PR doesn't list these). No built-in list has ? (measured, rg). Classification: semantic widening of allow-style project globs only. Owner-delegated decision.
• commitIds/judge comment and signature lines: type brand only. Benign.
• refuseRes/refused replaced by matcher: a superset. Benign (stricter).

Follow-ups

Astral ? in confusables.ts; tests pinning glob normalisation and NFD; case folding (#951 item 5); NFC/NFD for engine.ts protected paths.

[jayden1711]

Independent final check (R123c), posted by the coordinator.

VERDICT: PASS @691ed1c8f34d308c5a9f5af3cc23bf133f4d04a2 (final check)

Reviewer R123c, PR #1123 (Fixes #966). Head confirmed at start and end (gh pr view --json headRefOid, git ls-remote origin refs/pull/1123/head). Main is still 246e3f6. Worktree: TMPDIR:

Findings

• Every caller picks the right style (inferred, from reading each call site at head).
  • Deny, where a match refuses, holds or raises: refuse_paths, the hook's secrets and search filters, chat Grep globs, read-denials patterns and expand, money globs, category membership, self-modification, weakening, revert_first, ui_paths, hotspots, owners, the tamper and vacuity-command test globs, and the study's eval-cover check.
  • Allow, where a match exempts or lowers: ! exceptions in the hook and in read-denials, docs/tests/harness-config exemption, docs-only, size skips, proposal editable paths, and the money cover test.
  • engine.ts and the two vacuity files still call main's unit reading. review.ts uses it only for main's money readings.
  • The never-grow test enforces this list of legacy callers (measured, passes).
• R123a's finding is fixed and pinned (measured): glob-styles.test 1 fails on e61eea7 and passes on main and head.
• R123b's finding is fixed (measured): read-denials now uses the shared globMatcher, and the corpus checks it against an oracle in both styles. The hook ! probe (test 3) fails on e61eea7 and on main. The docs-lowering probe (test 4) fails on e61eea7.
  • Command: head tests copied onto each tree, then npx tsc -p . && node --import ./dist/test/isolate.js --test ….
  • On main, push-check tests 6 and 7 also fail (item 2 and NFC/NFD).
  • Head: 39/39.
• Mutants (measured, mutate.py editing the compiled head one at a time). Caught:
  • NFD dropped from spellings;
  • globs not normalised;
  • paths not normalised;
  • refuse_paths as allow;
  • money cover test as deny;
  • money before widened;
  • exemption as deny;
  • hook ! as deny;
  • hook secret as allow;
  • globRegExp always deny;
  • globMatcher always deny.
• Changelog and body are accurate (inferred against the call sites; measured where probed).
  • One wording nit: a ?-free glob with a lone surrogate ("\uD83D*") doesn't compile to main's source, and its allow reading is narrower than main's (measured, node probe). It only narrows; follow-up.
• npm run typecheck: exit 0. check:denylist: 1695 files clean. git merge-tree --write-tree against 246e3f6: clean (all measured).
• CI (gh pr checks): all 8 test shards and every required check pass (measured).

Weakening scan

Run with main's engine: weak-scan.mjs from the 246e3f6 dist. 5 reasons (measured):

1. read-denials-matcher PATTERNS changed (a never-shrink table): main's 31 patterns and 36 paths are all still there; the head has 45 and 50 (measured, corpus.mjs). Benign (stricter).
2. Assertion lines changed in the same file: the old comparison with globToRegExp is now a comparison with the two-reading oracle, which equals main for ?-free globs. The linear-time test pattern gains a ?, so it exercises both automata. Benign.
3. SELF_MODIFICATION_PARTS lookup now reads deny: superset of main. Benign (stricter).
4. Guard lines in access.ts and 42 more: style changes and read-denials' own matcher deleted. Each was audited above. Benign.
5. "skip" mark in pr-rules: a false positive on the variable name skip. The line reads allow, so it skips no more than main. Benign.

Follow-ups (not blocking)

• Surviving mutants (measured): read-denials ! as deny, read-denials expand as allow, docs-only as deny, size skip as deny, proposal editable as deny. These call-site styles are unpinned.
• A wider deny ? in read-denials overlaps can match a literal exception naming an astral character in a ? slot (for example keys/?.pem with !keys/🔑.pem). That pattern is then expanded at start instead of passed whole, so a matching file created later isn't sandbox-denied (the hook still denies it). This follows main's overlap design and agrees with the hook. Inferred; contrived.
• Already disclosed in the body: engine.ts and the vacuity files not yet given a style, confusables.ts, NFC/NFD outside push, case folding (Push gate routes on main: log config, crafted tree entries, replace refs in the size check, skill dir symlinks, case folding #951).

[jayden1711]

Independent adversarial review (R123d), posted by the coordinator.

VERDICT: PASS @691ed1c8f34d308c5a9f5af3cc23bf133f4d04a2 (adversarial)

Reviewer R123d, PR #1123 (#966), round 2. I checked the head with gh pr view --json headRefOid at the start and at the end, and it didn't change. Main is 246e3f6 (git ls-remote). git merge-tree --write-tree origin/main HEAD is clean. None of my findings meets the FAIL bar.

Findings

• The compiler matches its claim (measured, node rev/R123d-probe.mjs 4). I tested 4,272 globs against 30,408 paths, about 130M pairs. The globs used every combination up to 3 atoms of a ? * / ** 🔑, a lone high surrogate, a lone low surrogate, [ ] \ { ! . -, e+U+0301 (an NFD accent) and -. The paths used up to 4 atoms, newline and lone surrogates included. Results:
  • deny never matched less than main's globToRegExp, and allow never matched more.
  • deny equalled the union of main's reading and the u-flag reading on every pair, and allow equalled their intersection.
  • globMatcher equalled globRegExp in both styles.
  • The unit reading equalled main's regex, and main's read-denials matcher equalled main's regex.
  • 0 mismatches in every category.
• R123a's money line is fixed (measured, node rev/R123a-money.mjs R123d). On billing/**/? # see /🔑, the head counts billing/charge.ts as money, as main does.
• R123b's exception is handled (measured, node rev/R123d-rd.mjs). With keys/*.pem and !keys/?.pem, the sandbox entries keep keys/🔑.pem on both the head and main, and the hook (Secrets.at) says it is a secret.
• Every caller's style fits what its list does. I read every globRegExp call and its use (inferred).
  • ownerFor gets paths=[] at its only call site, so its glob match never runs.
  • The weakening scan's isTest also exempts pure test renames, which is an allow-style use compiled as deny. It is neutral because none of TEST_PATHS, GUARD_FILES or NEVER_GROW holds a ? (measured at runtime).
• Performance stays within about 2x of main (measured, node rev/R123d-perf.mjs). On the backtracking-prone shapes, deny and allow ranged from 0.3x to 2x main's time. globMatcher stayed under 0.3 ms.
• Fail before, pass after (measured). Head tests on e61eea7: glob-styles fails 4 of 4. On main: 2 of 4 fail, and the other 2 pass as intended, because they pin parity with main. The head passes 39 of 39 in glob, glob-styles, read-denials-matcher and push-check.
• Other checks. npm run typecheck exits 0 (measured). gh pr checks is all green on run 40366589581 at headSha 691ed1c (measured).

Mutants (measured, rev/R123d-mutants.sh, each flipped in dist, the 4 files above rerun)

• Killed: access.ts exception and secret styles, the review cover test, the review exemption, the MoneyGlob style, push-check's style.
• Survived: read-denials negRes and lateNegRes turned to deny, overlaps and expand turned to allow, access.ts search filters turned to allow, review categories and self-modification turned to allow, pr-rules docs-only and size skips turned to deny, proposal's editable list turned to deny, study's eval cover turned to allow.
  • The head behaves correctly in each case. The tests just don't pin these callers. For example, read-denials negRes as deny drops /keys/🔑.pem from the sandbox entries (measured). These are follow-ups, not FAILs.

Weakening scan (weak-scan.mjs, measured: 5 reasons)

• read-denials-matcher corpus and assertions changed: strengthening. The corpora are supersets, the random cases grew from 3000 to 4000, and the linear-time test is kept.
• SELF_MODIFICATION_PARTS line: the deny style makes it stricter.
• access.ts guard lines: secret globs are deny and ! exceptions are allow, which keeps them at or above main.
• pr-rules "skip mark": a false positive; it is a variable named skip.

Follow-ups (not blocking)

1. The changelog's no-? claim isn't exact. It says "A glob with no ? reads exactly as before". That is false for a glob holding a lone-surrogate literal in allow style: docs/\uD83D* no longer matches docs/🔑x (measured). The change is always narrower, so it errs stricter, and such a glob isn't ordinary input. Reword the claim.
2. The globRegExp doc comment overstates splicing. It says the source minus its first and last character "is a pattern of its own". That is false for allow: the spliced ? plus /b doesn't match a/b (measured). Only deny is spliced today, by access.ts. Restrict the claim to deny, or make allow unspliceable.
3. An astral literal exception now changes the sandbox entries. Under keys/?.pem it now overlaps, so read-denials expands the glob and drops the late-worktree glob entries. Main does the same for any overlapping exception (measured, R123d-rd.mjs cases a-exc and astral-exc).
4. Pin the surviving caller-style mutants with tests. Move isTest in the weakening scan to allow, and use allow for the rename exemption.
5. As the PR names: engine.ts, the vacuity files and confusables.ts, case folding (Push gate routes on main: log config, crafted tree entries, replace refs in the size check, skill dir symlinks, case folding #951 item 5), and NFC/NFD beyond refuse_paths.

[jayden1711]

Weakening scan decision for head 691ed1c8f34d308c5a9f5af3cc23bf133f4d04a2, recorded by the coordinator under the owner's delegation.

The owner's instruction (verbatim): "Stp asking me quesgtions and just take recommended option each time". This PR is in the critical set (the push check and the guardrails' glob compiler). Both independent reviews of this exact head passed: the final check (R123c) and the adversarial review (R123d). Both classified each of the scan's flags as a strengthening or a false positive; neither found a real weakening. R123d's exhaustive probe of about 130M glob/path pairs found no case where a deny-style glob matches less than main or an allow-style glob matches more (measured).

The weakening scan reports 5 reasons (measured, R123c and R123d, main's engine):

1. read-denials-matcher never-shrink corpus changed: STRENGTHENING. Main's 31 patterns and 36 paths are all still present; the head has 45 and 50.
2. Assertion lines changed in the same file: NOT A WEAKENING. The comparison with globToRegExp became a comparison with the two-reading oracle, which equals main for ?-free globs; the linear-time test now exercises both automata.
3. SELF_MODIFICATION_PARTS lookup reads deny: STRENGTHENING (a superset of main).
4. Guard lines in access.ts and others: NOT A WEAKENING. Secret globs read deny and ! exceptions read allow, keeping both at or above main; read-denials' own matcher was replaced by the shared one, checked against an oracle in both styles.
5. "skip" mark in pr-rules: FALSE POSITIVE (a variable named skip; the line reads allow, so it skips no more than main).

Decision: proceed (recommended option). This covers only this head; any new push needs a fresh scan and review.