Usage-policy gate folds path spellings a volume treats as one directory (NFC verified, case unverified) - #484
Conversation
`realpath(2)` folds symlinks and nothing else. On a default macOS (APFS) volume the kernel accepts, stats, and chdirs to spellings the gate compared as different strings: `Cafe` composed (NFC) versus decomposed (NFD), and `Proj` versus `proj`. The machine-local list's membership test therefore returned `full` for a directory the user opted out of, because the `dir` a CLI declared and the `cwd` a client reported are produced by different processes at different times. List membership now compares a folded spelling of both sides: Unicode NFC unconditionally (total, no filesystem access, identity on an already-composed path), and case only behind a per-volume probe that is inert off darwin. Producing a folded spelling is necessary but not sufficient. Nearest-governs is an argmax over match depth, and an argmax discards verdicts rather than merging them, so a carve-out that gains reach only by folding could displace a broader restrictive entry and start a private directory forwarding. The rule is therefore evaluated twice, once over the spellings exactly as declared and once folded, and the more restrictive answer wins. The resolved class is max(pre_fold, folded) by construction. The per-cwd memo stays keyed on the lexical path and consulted before any folding, so the per-exchange hot path is unchanged. Fixes #483 Co-Authored-By: Claude <noreply@anthropic.com>
🤖 neutral: this PR and #482 independently built the same mechanism in the same functionRecording a merge hazard that neither PR could see, because they were written in parallel by workers blind to each other. The overlapBoth PRs modify
They are solving the same shape of problem: a directory denoted by more than one spelling, where the gate compares strings. And both arrived independently at the same remedy for the same subtle defect - that nearest-governs is an argmax over match depth, so argmax discards verdicts and widening the spelling set is not monotone on its own. #482's round-1 review found that regression and fixed it with a two-pass evaluation, more-restrictive-wins. This PR's author rediscovered it and applied the same two-pass structure, pinning it with a 108-case exhaustive check. That convergence is good evidence the remedy is right. It is also a duplication: after both merge, the matcher would carry two spelling-widening rules and two two-pass evaluations, which is precisely the class of drift issue #465 existed to remove from a different file. What this means for merging
Not blocking either PRBoth stand on their own merits and each is verified against Related
|
…es, correct two cost claims Round-1 review of #484. - `createVolumeCaseProbe` deliberately does not memoize a non-`ENOENT` failure of the case-flipped `stat`, but nothing pinned it: mutating the branch to cache the undetermined verdict left the suite green. On a real volume a transient `EACCES`/`EIO` would then disable case folding for every path on that disk for the life of the resolver, silently reopening the gap the module exists to close. New test kills the mutant. - The NFC/NFD fixtures were raw UTF-8, while the comment above them claimed they were spelled as escapes. Re-normalizing the source really would have collapsed them (verified: it reddens 3 tests), so the tripwire worked, but the stated mechanism did not exist. Now actually `\u`-escaped, so the file is pure ASCII and the collapse is unrepresentable rather than merely detectable. The tripwire test stays, for anyone who reintroduces raw characters. - The probe's JSDoc claimed "one pair of stat calls per distinct volume, not one per directory". The memo is keyed on `dev`, which must be learned first, so every call stats `dir` itself; only the flipped stat is saved. Corrected to the actual bound (n stats plus one per volume, per TTL window). - LLP 0050 quoted normalize('NFC') at ~60 ns, measured on a pure-ASCII path, which is the case where the fold does nothing. On a genuinely decomposed path, the macOS case the section exists for, it is ~550 ns and scales with length. Added the real numbers and the reason the cost is affordable (miss path only). - LLP 0050 said only that the unfolded CLI sites "can name a different governor". Named each site and its direction instead: the CLI can never promise more protection than the gate delivers, `--check` reports the right class but can name a narrower scope, `policy unset` can refuse to remove an entry the gate enforces (fails toward privacy), and `hyp purge --subtree` can report success while retaining rows (the one site that fails away from privacy). No behaviour change: the only non-test edits are JSDoc and documentation. Co-Authored-By: Claude <noreply@anthropic.com>
Review of #484 at
|
| sweep | shape | cases |
|---|---|---|
| 1 | three entries at three depths, every class triple, every spelling assignment over {NFC, NFD, Proj, proj}, both case verdicts |
10,368 |
| 2 | carve-out inside a carve-out inside a carve-out (four entries), mixed spellings | 2,592 |
| 3 | declared and folded forms landing at different depths (six accented chars, so NFD outranks NFC on declared length) | 432 |
| 4 | case and normalization aliasing at once, case verdict on | 576 |
| 5 | .hypignore walk interacting with folded list entries |
4,608 |
| 6 | random: 1 to 5 entries, depths 1 to 4, random classes and spellings, random case verdict | 200,000 |
| 7 | same-depth entries, tie-break stress across spellings | 576 |
223,184 checked, 0 violations, 0 throws. Sweeps 1 to 4 are exactly the
adversarial shapes the brief asked me to construct (three entries at differing
depths, carve-out inside a carve-out, declared and folded forms at different
depths, a directory aliased two ways at once).
It is not luck, and the argument here is cleaner than #482's. deepestMatch(cwd, scopes, false) is bit-for-bit master's filter plus reduce: same argmax
over dir.length, same class tie-break, same declared strings. So asDeclared
is the master verdict. selectGoverning then returns the folded pass only
when its class rank is strictly greater, and asDeclared otherwise, with a
null declared pass ranked as full (the lattice bottom). The returned class is
therefore max(pre_fold, folded) by construction. matchList uses only
governing.entry.class, and the .hypignore walk is untouched, so the property
survives the merge with the dotfile result. resolve(cwd).class >= master(cwd).class holds for every input, not merely for every tested input.
One robustness note, not a defect. In the folded pass, entries whose volume
probed case-insensitive compare against a lowered cwd while the others compare
against the merely composed one, and toLowerCase is not length-preserving
(U+0130 lowers to two code points). So folded depths can in principle be
compared across two slightly different namespaces. This cannot leak, because the
folded pass is only ever consulted when it is more restrictive, so the worst
case is over-restriction. Worth knowing before anyone refactors deepestMatch
into a single pass.
The 108-case check is really 72, and is exhaustive over the space it claims
3 classes x 3 classes x 2 x 2 x 2 = 72, not 108. I confirmed the loop trip
count by execution. The factorisation the body prints is right; the product is
wrong. The test's own JSDoc claims no count and is accurate.
It is genuinely exhaustive over its space: every class pair, every spelling
assignment to outer entry, inner entry and cwd, for one nested pair at two
fixed depths. It does not cover three or more entries, same-depth entries,
depth inversion, or the case dimension. My sweeps above cover all four, which is
why I am reporting the count as a body correction rather than a coverage gap.
The probe is provably inert on Linux
Three independent confirmations, not one:
createVolumeCaseProbe()constructed with no injection at all on this
host returns a closure whosetoString()is literally() => false. The
darwin body is not merely skipped, it is unreachable from the returned
function.- I booby-trapped every function on the
node:fsdefault export and counted
calls. Seven directprobe()calls on real paths (/etc,/tmp,/usr/Lib,
/etc/Hosts,process.cwd(),/): 0 fs calls. The same through the full
resolver on a real on-disk entry list: 0 fs calls on the probe path. - Static reachability:
platformandcaseInsensitiveVolumeare settable only
through thedepsparameter.grepover the whole repo shows the only
callers passing either are intest/core/usage-policy-fold.test.js, and every
test that injectsplatform: 'darwin'also injects a fakestatSync. No env
var, no config key, no CLI flag reaches either. Nothing can make a Linux
filesystem be probed or a wrong verdict be cached.
Case-sensitive macOS volume: the flipped spelling does not resolve,
statSync throws ENOENT, and the code treats ENOENT as the informative
outcome and memoizes false. Correct. If a genuinely different sibling happens
to occupy the flipped name, dev/ino differ and the verdict is still false.
Correct.
Wrong-way answers: a wrong true over-restricts (merges two real
directories) which is privacy-safe; a wrong false leaves the pre-fold
behaviour, so it preserves the original bug rather than creating a new one.
Neither direction can open the gate wider than master. The code and LLP 0050
both say exactly this, which is the honest framing: it fails to the status quo,
not "toward privacy" in the strong sense.
Finding 1: a non-ENOENT probe failure was not pinned by any test (medium, FIXED)
src/core/usage-policy/fold.js:214 at d8652484.
The code deliberately does not memoize an undetermined probe, and says so in a
comment: "Any other errno is a genuinely undetermined probe and is not
memoized." I mutated that branch to byDev.set(st.dev, false) and the suite
stayed green, 22 pass 0 fail. The existing does not memoize an undetermined answer test only covers the no_cased_character path, not the errno path.
This matters on the target platform. Under the mutant, one transient EACCES on
a directory the daemon cannot traverse, or an EIO on a flaky mount, would cache
"case-sensitive" for the whole volume for the life of the resolver, silently
disabling case folding for every path on that disk. That is the exact failure
class this module exists to prevent, and it would have been invisible.
Fix (1bf7baa): new test createVolumeCaseProbe does not memoize a non-ENOENT failure of the flipped stat. It fails the flipped stat with
EACCES, asserts false, then lets a later probe of the same dev reach a
definite answer and asserts it is believed rather than served a stale false.
Verified: re-applying the mutant now reddens exactly that test (23 pass 0
fail baseline, 22 pass 1 fail mutated).
This was one of the two mutants the brief listed as fixed in the first pass. The
other, folded-depth-on-declared-spelling, is genuinely fixed and covered: I
mutated const depth = dir.length to scope.entry.dir.length and it reddens
resolve: nearest-governs is measured on the folded spelling, not on the declared one, precisely the test the body names.
Finding 2: the tripwire's stated mechanism did not exist (medium, FIXED)
test/core/usage-policy-fold.test.js:36-41 at d8652484.
The comment read "Spelled as escapes so the file cannot be silently
re-normalized by an editor, a merge tool, or a git filter". The literals were
raw UTF-8: caf\303\251 and cafe\314\201, confirmed by od -c. A third
raw literal sat at line 154.
The tripwire itself is real, and I verified it rather than assuming it. I
re-normalized the whole source file to NFC, simulating the editor the comment
warns about: 3 tests go red, including the premise test. So the file could
not have passed vacuously. But the protection was detection after the fact, not
the prevention the comment claimed.
Fix (1bf7baa): all three literals are now \u escapes, so the file is pure
ASCII (grep -cP '[^\x00-\x7F]' returns 0) and the collapse is unrepresentable
rather than merely detectable. The tripwire test stays, for anyone who
reintroduces raw characters, and the comment now describes what the code does.
Verified: git diff d8652484..HEAD shows the escaped literals; suite 23 pass
0 fail.
Finding 3: the probe's memoization cost claim is false (low, FIXED)
src/core/usage-policy/fold.js:123-129 at d8652484.
The JSDoc claimed "one pair of stat calls per distinct volume for the life of
the resolver, not one per directory". The memo is keyed on dev, and dev
has to be learned before the memo can be consulted, so probe(dir) calls
statSync(dir) unconditionally on every invocation. It is one stat per
directory. Only the second, case-flipped stat is saved. The branch's own test at
line 298 demonstrates this (['/vol/Proj', '/vol/pROJ', '/vol/Other', '/vol/deep/Nested']), so the code and test were right and only the claim was
wrong.
The real bound is n stats plus one extra per distinct volume, per TTL window,
for an n-entry list. Still inside LLP 0049 R6, which already stats every
ancestor, so this is a claim defect and not a performance defect.
Fix (1bf7baa): JSDoc corrected to the actual bound and the reason for it.
Verified: git diff d8652484..HEAD -- src/core/usage-policy/fold.js.
Finding 4: LLP 0050 quotes the one normalize cost that does not apply (low, FIXED)
llp/0050-ignore-enforced-in-adapters.decision.md:181-182 at d8652484.
The Cost section said normalize('NFC') is "roughly 60 ns on a pure-ASCII path".
Accurate, and I reproduce it (56 to 61 ns). But that is the case where the fold
does nothing. On a genuinely decomposed path, which is the entire macOS case the
section exists for, normalize has to recompose:
| input | length | already NFC | cost |
|---|---|---|---|
| ASCII path | 49 | yes | 56 ns |
| accented, composed | 114 | yes | 93 ns |
| accented, decomposed | 134 | no | 552 ns |
| pathological all-decomposed | 1001 | no | 9.65 us |
So the documented figure understates the target-platform cost by about 9x. This
is not a hot-path problem, because folding is on the cache-miss path and the list
parse only, and I measured the end-to-end worst case to confirm: a cache miss
over a 20-entry list with a long NFD cwd is 8.2 us on master and 9.7 us on
the branch. But a reader planning against 60 ns would be wrong by an order of
magnitude, and this is the number a future caller would reuse when deciding
whether the fold can move to a per-row path.
Fix (1bf7baa): the Cost section now carries both regimes, the measured
numbers above, and an explicit instruction to re-measure with a decomposed path
before moving the fold anywhere hotter. Verified: git diff d8652484..HEAD -- llp/.
Finding 5: the CLI seam disclosure omitted the one direction that loses data (medium, FIXED)
llp/0050-ignore-enforced-in-adapters.decision.md:193-201 at d8652484.
The "Not covered" section said the unfolded CLI sites "can still name a different
governor than the gate used". True but flattening: the four sites fail in
different directions, and one of them loses user data. I traced each.
- The CLI can never promise more protection than the gate delivers. The
lexical predicate matches a subset of the folded one, and the gate returns
max(declared, folded), so any entry the CLI finds the gate also found. There
is no spelling on which--checksays protected and the gate forwards. This is
the invariant that matters and it holds. hyp ignore --check/policy showreport the right class, because it
comes fromresolve(). OnlyresolveCheckScopeDir
(src/core/commands/clients.js:1145) is lexical, so when an entry reaches
cwdonly by folding it falls back to the queried path. Right class, narrower
named scope and residual count.policy unset/unignore --local-only(clients.js:1017) can report
"not governed" and exit 0 for an entry the gate is enforcing. Fails toward
privacy: the opt-out stays on. Annoying, not dangerous.hyp purge --subtree(src/core/cache/purge.js:94) can report success
while leaving rows on disk, when the rows were recorded under a different
spelling. This is the one site that fails away from privacy. The user asked
for deletion, the command succeeded, and the data remains.
Is the seam defensible? Yes, but it should be named honestly. I agree with
leaving these to #482's shared predicate rather than growing a second folding
rule, which is exactly the duplicated-invariant pattern #465 existed to remove.
And the purge behaviour is unchanged from master, so this PR ships no
regression. But "can name a different governor" did not convey that a deletion
command can silently retain data.
Fix (1bf7baa): "Not covered" now names each site, its direction, and states
the never-promises-more-than-the-gate invariant explicitly. Verified: git diff d8652484..HEAD -- llp/.
Finding 6: the PR body says 108 cases, the loop runs 72 (low, LEFT for a human)
PR body, "exhaustive rather than illustrative" bullet. The listed factorisation
3 x 3 x 2 x 2 x 2 is 72. I confirmed the trip count by execution.
Decision for a human: change "(108 cases)" to "(72 cases)". No code change;
the check is exhaustive over the space it claims and the test's own JSDoc is
accurate. I cannot edit the body.
Finding 7: the cache-hit performance numbers do not reproduce (low, LEFT for a human)
PR body performance table: master 257 / 236 ns versus branch 220 / 214 ns,
which reads as the branch being about 10 percent faster than master.
My measurement, three runs each, same harness in this worktree and against the
extracted origin/master matcher, 500k steady-state calls after a 100k warmup:
master |
branch | |
|---|---|---|
cache-hit resolve (per-exchange path) |
285.2 / 273.0 / 246.4 ns | 292.9 / 266.8 / 259.4 ns |
cache-miss resolve (per cwd per TTL) |
2.90 / 2.98 us | 3.09 / 2.48 us |
The two are indistinguishable inside run-to-run noise, which is what the body
itself concludes in the sentence after the table. The substantive claim is
correct and I verified its mechanism directly: the memo is keyed on the lexical
path and consulted before any folding, and resolve: folding happens on the cache miss only pins it. The numbers as printed just imply a speedup that is not
there.
Decision for a human: either replace the cache-hit row with a range that
spans both worktrees, or drop the per-run digits and keep the prose conclusion.
The claim needs no retraction, only the false precision removed.
On the ASCII fast path the brief asked about: it is real but partial. normalize
does short-circuit the recomposition when the string is already NFC (56 ns ASCII
and 93 ns composed-accented, versus 552 ns decomposed), but it is not free, and
it is not a cheap identity check either, since cost still scales with length.
There is no case where a long non-ASCII path blows the budget at the position the
fold occupies.
Overlap with #482: they compose, with two specific hazards
Recorded, not resolved, as instructed. Both branches arrive at the same two-pass
max(declared, widened) shape for the same reason, so unification is
semantically sound: one declared pass plus one widened pass over the union of
spellings gives the same guarantee both deliver separately. Two things will make
it harder than it looks:
- Composition order is not free.
realpathmust run before the fold, never
after. Folding first can produce a spelling that does not exist on a
case-sensitive volume, andrealpathwould thenENOENTon a path that was
fine. The unified candidate builder has to befold(realpath(p)), and Usage-policy gate matches directories, not path spellings: canonicalize both sides #482's
partial-canonicalization walk has to fold each resolved ancestor, not the
rejoined result. - The two disagree about where depth is measured. This PR measures
specificity on the folded spelling and pins it
(nearest-governs is measured on the folded spelling), because NFD is longer
in code units than NFC and a declared-string depth inverts the ordering.
Usage-policy gate matches directories, not path spellings: canonicalize both sides #482'smatchDepthmeasures on whichever spelling matched. A naive union
keeps whichever convention the merge happens to preserve, and the failure is
silent and shape-dependent. Whoever unifies must pick the folded convention
and re-run this PR's depth-inversion test.
Both branches also independently reach for an entry-side representation: this one
precomputes a single foldedDir per entry, #482 stores a spelling set. The
unified form has to be the set, which means this PR's ListScope is the shape
that gives way.
Docs and style
LLP 0050 §normalization and the LLP 0049 #scope Extended-by note are
accurate. I checked the one claim that could have been hand-waved: LLP 0049's
note says the ancestor walk itself is unaffected "because it stats each
candidate rather than comparing two strings". Confirmed at matcher.js:146, the
walk is existsSync(candidate), which the macOS kernel resolves across both
aliasing mechanisms, so the walk genuinely does not need folding and only list
membership does. All anchors resolve; test/core/llp-ref-hygiene.test.js is 8
pass, 1 skip (the pre-existing #463 renumber skip).
CLAUDE.md: no semicolons, no @typedef, no inline import() types,
root-anchored .js type-import specifiers, JSDoc types throughout. Zero em
dashes added by the branch or by my commit (git diff | grep -cP '\x{2014}'
returns 0 for both).
hypaware-core/plugins-workspace/codex/src/exchange-projector.js is untouched,
so this adds nothing to the conflict surface of #462 / #474 / #477.
Verification
npm test: 2950 tests, 2940 pass, 8 fail, 2 skipped. The 8 are the pre-existing
test/core/leave-command.test.jsset. I verified them by name on a
pristineorigin/masterworktree (1555f13) with the samenode_modules:
identical 8. Unrelated to this branch.npm run typecheck: clean.test/core/usage-policy-fold.test.js: 23 pass, 0 fail (22 before my commit).- All five usage-policy suites plus the ignore and purge command suites: 122
pass, 0 fail. test/core/llp-ref-hygiene.test.js: 8 pass, 1 skip.- Differential sweep re-run after my commit: 223,184 cases, 0 violations,
unchanged. My edits are JSDoc, documentation and one test, so no behaviour
moved.
What a human still needs to decide
- Body corrections 6 and 7 above (the 108, and the cache-hit digits). Both are
body-only and I am not permitted to edit it. - Whether the CLI seam is acceptable to ship. My read is yes, and the fixed
LLP 0050 now states the risk plainly, but thehyp purge --subtreeretention
case is the one a human should sign off on knowingly rather than inherit. It
is not a regression frommaster, and Usage-policy gate matches directories, not path spellings: canonicalize both sides #482 is the right place to close it. - The case half remains unverified and may not work. Nothing on this host
can witness it, the branch says so, and I confirmed the failure mode is inert
rather than wrong on Linux. Someone with a macOS box should run the two checks
the body names, on both a case-insensitive and a case-sensitive APFS volume,
before this is trusted on darwin.
…the purge seam it must not be reused in
LLP 0050 justified folding NFC unconditionally with "there is no volume on
which folding NFC and NFD together is wrong, because no filesystem this
codebase targets lets two paths that differ only by normalization name two
different directories". That premise is false, and demonstrably so on this
host: `caf`+U+00E9 and `cafe`+U+0301 are two directories with two inodes,
both present in one parent, each holding different content.
The decision is still right, but for a gate-specific reason: the resolved
class is max(declared, folded), so a fold that merges two distinct
directories can only over-restrict. Recorded as such, with an explicit
warning not to reuse foldPath where widening deletes or discloses.
Also in "Not covered":
- the `hyp purge <path>` retention gap now carries the observed transcript
("purged 0 rows from 0 partitions", exit 0, empty stderr) and notes the
inversion that the succeeding purge is the noisier of the two;
- records that `hyp purge --ignored` IS covered by this change, because it
classifies through resolver.resolve(); verified against master, which
leaves the row. That is the durable workaround for the subtree gap;
- notes that closing the subtree gap is not a foldPath drop-in, because on
a Linux volume that would delete a genuinely different sibling's rows.
Docs only; no behaviour change.
Co-Authored-By: Claude <noreply@anthropic.com>
Review of #484 at
|
| rows recorded as | purge argument | exit | stdout | stderr | rows surviving |
|---|---|---|---|---|---|
| NFC | NFC (control) | 0 | purged 1 row from 1 partition |
resurrection warning | 0 |
| NFD | NFC | 0 | purged 0 rows from 0 partitions |
(empty) | 1 |
| NFC | NFD | 0 | purged 0 rows from 0 partitions |
(empty) | 1 |
/home/u/proj |
/home/u/Proj |
0 | purged 0 rows from 0 partitions |
(empty) | 1 |
Byte-identical results on master (1555f13) and on this branch. What a user actually sees:
$ hyp purge ~/café/proj --yes # typed NFC; rows were recorded under NFD
purged 0 rows from 0 partitions
$ echo $?
0
Three things make it nastier than the row count implies:
- Exit 0 and an empty stderr. Indistinguishable from "that directory had nothing cached". There is no signal to act on.
- The failure is quieter than the success. A purge that actually deletes prints the LLP 0104 resurrection warning to stderr. The purge that silently retains prints nothing. Silence reads as clean.
- It is the one seam that fails away from privacy. Every other spelling-divergence symptom over-suppresses. This one keeps data the user explicitly asked to destroy.
Cause is src/core/cache/purge.js:94: isEqualOrDescendant(path.resolve(row.cwd), base) is a pure lexical prefix test, and path.resolve folds neither normalization nor case.
hyp purge --ignored is a different story and it is good news. That target runs resolver.resolve(row.cwd).class !== 'ignore', so it inherits this PR's fold. With an ignore entry declared NFC and rows recorded NFD: master purges 0 rows and leaves the row; this branch purges it. Both directions. So this PR already closes the retention gap for the --ignored sweep, and marking the directory then running hyp purge --ignored is the workaround for the subtree gap. That was not written down anywhere; it is now.
Finding 1: LLP 0050 justified unconditional NFC folding with a claim that is false (medium, FIXED)
llp/0050-ignore-enforced-in-adapters.decision.md:126-130 at 1bf7baac:
There is no volume on which folding NFC and NFD together is wrong, because no filesystem this codebase targets lets two paths that differ only by normalization name two different directories.
Every Linux volume this codebase targets does exactly that. Executed on this host:
NFC hex: 636166c3a9 NFD hex: 63616665cc81
NFC ino: 95818260 | NFD ino: 95818408 | SAME DIRECTORY? false
read NFC/who.txt => I am the NFC directory
read NFD/who.txt => I am the NFD directory
dir entries: [ '63616665cc81', '636166c3a9' ]
Two directories, two inodes, both in one parent, different contents.
The decision is still correct, but for a reason specific to the gate: the resolved class is max(declared, folded), so a fold that merges two genuinely distinct directories can only ever over-restrict. That is a usability cost and never a privacy or data-loss one.
Why the wrong reason matters rather than being pedantry: it is stated as a universal property of filesystems, which licenses reuse anywhere, and it sits directly above a bullet that correctly gates case folding behind a probe precisely because it "would merge two genuinely different directories". The asymmetry the document draws between the two mechanisms is not real. The real asymmetry is that macOS folds normalization on every APFS volume whereas case depends on format. Anyone closing the purge seam would read "safe everywhere" and fold inside a deletion predicate.
Fix (aee8e6f): the bullet now states the true reason, carries the executed counter-evidence, and ends with an explicit "do not reuse foldPath in a predicate where widening is not free". The summary table row is corrected to match. Verified: git diff 1bf7baac..HEAD -- llp/ shows the change; no behaviour moved, test/core/usage-policy-fold.test.js 23 pass 0 fail, llp-ref-hygiene 8 pass 1 skip.
Finding 2: the purge disclosure lacked the symptom, the workaround, and the over-deletion hazard (medium, FIXED)
llp/0050:193-201 at 1bf7baac. Round 1 correctly named hyp purge --subtree as the one site failing away from privacy. Three things were still missing, and each is load-bearing for whoever picks this up:
- the observed transcript (exit 0,
purged 0 rows, empty stderr) and the inversion that the succeeding purge is the noisier one. "Can fail to purge rows it reports as purged" also slightly misdescribes it: it does not claim to have purged rows, it reports zero, which is exactly what makes it invisible; - that
hyp purge --ignoredis already covered by this change, with themastercomparison, so the workaround is on record; - that closing the seam is not a
foldPathdrop-in, because purge deletes. On a Linux volume, folding would delete cached rows for a genuinely different sibling. The fix needs the fold gated on the volume actually being normalization-insensitive, and no such probe exists:createVolumeCaseProbeanswers only the case question.
Fix (aee8e6f). Verified: git diff 1bf7baac..HEAD -- llp/, all five named markers present.
Finding 3: nothing tracked the purge gap, and closing #483 would have retired the only record (medium, FIXED by filing #485)
gh issue list showed no issue for it. #479 names hyp purge but for the symlink class, and #482 closes that half. #483 names it, but this PR says Fixes #483, so merging would auto-close the only written record of a deletion command that does not delete.
Filed #485, label neutral:fix only. Not neutral:stuck: the defect is on master (reproduced at 1555f13), not branch-only, so it is directly actionable. It carries the reproduction table, the user-visible symptom, the cause, the --ignored workaround, the over-deletion hazard, three concrete fix options, and acceptance criteria.
I did not fix it in this PR. Round 1 deferred the CLI seams to #482's shared predicate and I agree, now with a technical reason on top of the process one:
- Usage-policy gate matches directories, not path spellings: canonicalize both sides #482 rewrites this exact predicate to call
scopeGoverns. A fix here collides head-on with an approved PR awaiting a human merge. - It would be the second folding rule, the pattern Two independent readers of Codex session_meta enforce the same privacy-relevant invariant; it has silently drifted twice already #465 exists to remove.
- It is not safe as a one-liner, per finding 1. Doing it properly needs a normalization-insensitivity probe that does not exist. That is a design decision with real options, not a patch, and it belongs in an issue where a human can pick.
I also declined to add a regression test to test/core/purge-command.test.js, which #482 modifies, rather than grow the conflict surface of a terminal PR for a non-regression.
Findings 4 and 5: stale figures in the PR body (low, LEFT for a human)
The orchestrator corrected the "108 cases" bullet and the cache-hit table. Two stale claims survive. I cannot edit the body.
Finding 4. The VERIFIED section still reads:
- Folding is monotone (108-case exhaustive check above, plus the two named hole-punching guards).
The corrected bullet 40 lines up now says 72. Decision: change 108-case to 72-case. Same error, second occurrence, missed by the first pass.
Finding 5. Test counts predate round 1's added test:
| body says | actual (executed at 1bf7baac) |
|---|---|
"test/core/usage-policy-fold.test.js, 22 tests" / "All 22 pass here" |
23 |
"npm test: 2949 tests, 2939 pass, 8 fail" |
2950 tests, 2940 pass, 8 fail, 2 skipped |
| "Fourteen mutants, each killed by a named test" | 15; the table has no row for round 1's non-ENOENT probe mutant |
The 8 failures are the pre-existing test/core/leave-command.test.js set, unrelated. Decision: bump the three counts and add the M15 row, or drop the digits. No code implication.
Round 1's five fixes: all held, all load-bearing
Re-verified rather than assumed.
fold.js:214, the unmemoized non-ENOENT probe failure. Re-applied the mutant (byDev.set(st.dev, false)before thereturn false). Baseline 23 pass 0 fail; mutated 22 pass, 1 fail, reddening exactlycreateVolumeCaseProbe does not memoize a non-ENOENT failure of the flipped stat. Genuinely load-bearing.- The escaped fixtures.
grep -cP '[^\x00-\x7F]'ontest/core/usage-policy-fold.test.jsreturns 0; the literals are'café','café'and'ééééé'. The collapse is now unrepresentable, not merely detectable. Worth noting that this trap is live: my own shell heredocs silently collapsed an NFD literal to NFC twice during this review, once producing a false negative I nearly reported. The tripwire is earning its place. fold.js:123-129, the memo-cost JSDoc now states the real bound (nstats plus one per distinct volume, not one pair per volume). Accurate.- LLP 0050 Cost carries both normalize regimes with measured numbers and the instruction to re-measure with a decomposed path. Accurate.
- LLP 0050 "Not covered" names each CLI site and its direction. Accurate, and extended by finding 2.
Rebased on #482: still correct
Checked concretely rather than by reading. git merge-tree of this head against #482 (27dc367c): three conflicting files (matcher.js, src/core/usage-policy/index.js, llp/0049), 7 conflict hunks in matcher.js, two of them large and covering exactly the selectGoverning / deepestMatch region. llp/0050 auto-merges, including my edits. Textual conflict is total in that region, as both PRs predicted.
The monotonicity argument survives, and canonicalSpellings does not weaken it. The load-bearing fact is that it always returns the lexical spelling, first: return outcome.path === lexical ? [lexical] : [lexical, outcome.path]. So #482's declared pass (deepestMatch(cwd, scopes, 1), spellings[0] only) is the same master verdict this PR's declared pass reproduces. Both PRs compute max(master, widened_k) for their own widening. A unified evaluation that takes the max over the union of passes is therefore at least as restrictive as either PR alone, because a max over a superset is monotone. Nothing in canonicalSpellings can lose the lexical spelling, so no unification can drop below master.
Two conditions the unifier must meet, both already visible:
- Order:
fold(realpath(p)), never the reverse. Folding first can produce a spelling that does not exist on a case-sensitive volume and makerealpathENOENTa path that was fine. Folding the outputs ofcanonicalSpellingsgets this right automatically. - Depth must be measured on the folded form of the matched spelling. Usage-policy gate matches directories, not path spellings: canonicalize both sides #482's
matchDepthmeasures on whichever spelling matched; this PR measures on the folded one, and pins why (NFD is longer in code units than NFC, so a declared-string depth inverts nearest-governs). In a unified spelling set containing both a lexical NFD form and a folded NFC form, raw lengths become incomparable within one pass. Cross-pass combination is by class rank only, so that part is safe, but the within-pass metric must be the folded length. Whoever unifies must keep the folded convention and re-run this PR'snearest-governs is measured on the folded spellingtest.
Shapes: this PR's ListScope is {entry, caseInsensitive, foldedDir}, #482's is {entry, spellings}. The unified form is the set, plus a per-entry caseInsensitive (the probe is per-entry because it is per-volume). This PR's ListScope is the shape that gives way, as round 1 said. Natural target: spellings = dedupe([lexical, canonical, fold(lexical), fold(canonical)]) with limit = 1 still meaning the declared pass.
Note also that after both land, hyp purge <path> folds symlinks but not normalization or case. That residue is #485.
Verification
All on the pushed head aee8e6f, in a detached worktree at the PR head with the repo's node_modules.
npm test: 2950 tests, 2940 pass, 8 fail, 2 skipped. The 8 are the pre-existingtest/core/leave-command.test.jsset, identical by name on a pristineorigin/masterworktree (1555f13).npm run typecheck: clean.test/core/usage-policy-fold.test.js: 23 pass, 0 fail.test/core/llp-ref-hygiene.test.js: 8 pass, 1 skip, 0 fail. LLP 0050§normalizationand the LLP 0049#scopeExtended-bynote are accurate and every anchor resolves. I re-confirmed the 0049 note's substantive claim: the ancestor walk isexistsSync(candidate)atmatcher.js:146, so it genuinely needs no folding and only list membership does.- Branch is based directly on current
master(1555f13); merge-base is1555f13itself, so no merge was needed for the hygiene run. - CLAUDE.md: no semicolons in code, no
@typedef, no inlineimport()types,@importuses a root-anchored.jsspecifier, JSDoc types throughout. Em dashes added by the branch or by my commit: 0 (git diff | grep -cP '\x{2014}'). hypaware-core/plugins-workspace/codex/src/exchange-projector.jsuntouched, so nothing added to the Codex rollout cwd keys on the thread, not the session container (#459) #462 / Codex live projector: an unusable in-band cwd is a miss, not a path (#471) #474 / Codex live projector: an explicit cwd outranks a substituted workspace key at the .hypignore gate #477 conflict surface.- My commit is documentation only. No source file changed, so no behaviour moved.
What a human must decide
- Body corrections 4 and 5 (the surviving
108, and the three stale test counts plus the missing M15 row). Body-only; I cannot edit it. - Whether to ship with the
hyp purge <path>seam open. My read is yes: it is not a regression,--ignorednow works and is a real workaround, and the proper fix needs a probe that does not exist. But this is a deletion command that reports success without deleting, in a tool that captures LLM traffic, so it should be signed off knowingly rather than inherited. Tracked as hyp purge <path> reports success and exits 0 while silently retaining rows recorded under an aliased spelling of that directory #485. - Merge order and the unification with Usage-policy gate matches directories, not path spellings: canonicalize both sides #482. Usage-policy gate matches directories, not path spellings: canonicalize both sides #482 is
neutral:approvedand awaiting a human merge. If it lands first, this PR needs the rebase described above; the conflict is 7 hunks inmatcher.jsand must be resolved into one spelling set with one most-restrictive-wins pass, not two stacked passes. Do not let it be resolved mechanically. - The case half is still unverified and may not work. Nothing on this host can witness it. I re-confirmed it is inert rather than wrong on Linux, so the risk is bounded to "the macOS case bug is not actually fixed", never "a new bug on Linux". Someone with a Mac should run
test/core/usage-policy-fold.test.json a default and on a case-sensitive APFS volume and checkcreateVolumeCaseProbe()('~/Documents')before this is trusted on darwin.
Round 2 of 2. Fixed and pushed: aee8e6f. Filed: #485.
|
Neutral triage (rung: Round 2's last commit, The disproved-premise correction (unreviewed commit). Round 2's commit Purge retention gap. I re-verified the mechanism directly: Rebase compatibility with #482. I sanity-checked round 2's monotonicity PR body corrections. Verified against the running repo, not just read: Also verified: ref-hygiene passes on this branch, which already contains Nothing above duplicates #485; the purge-gap discussion is my own |
philcunliffe
added
neutral:approved
#482 (symlink canonicalization) and this branch (NFC/NFD and per-volume case folding) are the same fix for two different mechanisms by which a filesystem spells one directory several ways, found by the same review. Both had independently arrived at the same shape, and #484's own note said whichever landed second should collapse them into one pass rather than stack two. This merge does that. The composition is a map, not a union: `realpath` yields a *set* of spellings (as-given, canonical), the fold is a *function* on a spelling, so a scope's widened set is the fold's image of the canonical set. `matchList` therefore carries one `ListScope` per entry holding the declared spelling, the widened set, and the case verdict the `cwd` side must be folded through, and there is exactly **one** two-pass argmax guard (declared-and-unfolded versus widened-and-folded, most restrictive wins) rather than one per mechanism: the displacement hazard the guard exists for is identical whichever widening gave a carve-out its extra reach, so running it twice would buy nothing. The invariant both sides claim, `class = max(pre_widening, widened)`, now holds for both at once. `resolve()` keeps #482's outer loop over the `cwd`'s own spellings, and #484's `usage_policy.fold_tightened` signal now reports any widening that tightened a verdict, not only a fold. `matchDepth` takes the longest matching spelling rather than the first. #482's argument that the first is the longest relies on `canonicalSpellings` ordering and on no spelling being length-reduced; NFC is length-reducing, so folding breaks it. Taking the maximum makes nearest-governs independent of arrangement. The fold stays gate-only, which is the one place the two mechanisms deliberately do *not* compose. #482 routed `hyp purge --subtree`, `policy unset` and the upsert-identity check through shared predicates (`scopeGoverns`, `sameDirectory`, `governingListEntry`); those keep symlink widening and deliberately skip `foldPath`, per LLP 0050 #normalization's "do not reuse foldPath in a predicate where widening is not free": unconditional NFC folding is sound at the gate only because the gate answers `max(...)`, and that argument does not survive in a predicate that deletes rows, removes an opt-out, or replaces a stored declaration. The split is now explicit as `listScope` versus `canonicalScope`, and LLP 0050 records what closing the remaining gap needs. Both regression suites survive intact and pass: `usage-policy-symlink.test.js` (#482, incl. the two-pass guard's exact reach) and `usage-policy-fold.test.js` (#484, incl. per-entry case verdicts and the hashed tightening signal). Co-Authored-By: Claude <noreply@anthropic.com>
|
Neutral triage (rung: What moved
The merge, verified by execution1. Argmax guard / never-less-restrictive-than-plain-lexical. I wrote a 2. Depth convention. Reading the merged code: I confirmed the pinning test 3. Composition order. 4. Stacked or unified? Unified, not stacked. There is exactly one 5. Repo checks at this head, clean worktree,
Residual findings and classification
VerdictCase A — every residual finding is non-blocking. No true blocker found in the |
philcunliffe
added
the
neutral:approved
Three files conflicted. Each resolution is a union of both intents, not a side-picking, because the two sides changed different things about the same subject (Codex session identity and the opt-out caveat). session_command.js: keep this branch's EPHEMERAL_NOTE constant (#455: the caveat names the fork as well as the restart) and master's object-argument provenanceNotes({ idSource, idEvidence, threadId, endpointSource }) call. Only one of the two call sites conflicted; the other already carried the combination, so this makes the two branches of the if/else identical again. types.d.ts: keep master's prose, which is the correct one now - it explains sessionId as the session container versus threadId, and the merged union already carries master's rename of codex_env to codex_env_rollout, which this branch's prose still called codex_env. Re-attached this branch's gloss on the @ref LLP 0067#cli-session-id annotation, which master had left bare. Anchor {#cli-session-id} verified present at llp/0067:305. codex privacy skill SKILL.md: this branch's rollout-selection policy wins (match payload.cwd, refuse on zero/ambiguous/stale, never newest-by-mtime - issue #452), since master's side still said "pick the newest rollout". Folded master's readRolloutMeta-mirroring guards into that same walk rather than dropping them: the session_meta type check and payload-dict check (as skips, since this side walks many files), plus the non-string and whitespace-in-id refusals, which this side needs too because it reads the result through `read -r`. Took master's `hyp session ignore --json` over the bare verb. Semantics checked, not just textual cleanliness: - The claim "the gateway's drop keys on the container" is STILL TRUE after PR #477. exchange-projector.js line 113 is unchanged: `const sessionId = stringValue(codexContext?.session_id) ?? conversationId`, i.e. metadata.session_id falling back to the conversation (thread) id, exactly as the skill prose and the test comment describe. #477 (bcad4a4) only changed the separate .hypignore gate, which keys on a cwd path, not on any session id. So no correction was needed there. - The three-way agreement still holds between the CODEX_THREAD_ENV comment in session_command.js (master renamed it from STATED_SESSION_ID_VARS), the skill's Step 1 prose, and the drop code: thread id is a selector, the container is the answer. - Corrected one thing that HAD gone stale: this branch's prose said issue #453 "puts CODEX_THREAD_ID to its real use" in the future tense, but #453 shipped on master (5d270a5, c551d6e). The prose now describes the codex_env_rollout source as live, matching llp/0067:317 ("selector, not an answer") and llp/0067:585. - The fork caveat agrees across all three surfaces it appears on: EPHEMERAL_NOTE, the Codex skill, and the Claude skill (`claude --fork-session` / `codex fork`). The usage-policy unification (#482/#484, fold(realpath(p))) touches no file on this branch and needed no reconciliation here. Checks: npm test 3041 tests, 8 failures, all of them the pre-existing test/core/leave-command.test.js "leave ..." set, name-for-name identical to a pristine origin/master run (3037 tests, same 8). Test count rose by 4, which is this branch's new tests passing. npm run typecheck clean. Co-Authored-By: Claude <noreply@anthropic.com>
The behaviour is kept; the reason given for it was wrong and is replaced. LLP 0160, the `workspaceCoversCwd` JSDoc and the warn-site comment all claimed that when the key is an ancestor of the in-band `cwd`, `resolve(cwd)` is at least as restrictive as `resolve(key)`, so refusing the substitution "can only tighten". That is false on this resolver. Nearest-governs means a declaration between the key and the `cwd` overrides the key's and may be less restrictive. Swept against the real `createUsagePolicyResolver` over `.hypignore` bodies and machine-local list classes at four depths on one ancestor chain: 131 of 576 arrangements resolve the `cwd` LESS restrictively than the key, spanning ignore->local-only, local-only->full and ignore->full. Restated on the ground that actually holds: an ancestor key is not a guess about where the session ran, it is a less specific name for the same tree, and nearest-governs makes the `cwd`'s own declaration authoritative. So the signal reports the location inference, not a verdict change - and the residue (an ancestor key resolving more restrictively than its `cwd`, now silent) is disclosed rather than implicitly denied, and pinned by a test. Also corrected: - the symlink residue cited #479 as an open gap; #479 was closed for the gate by #482/#484. It is knowingly retained here, at this reporting-only predicate, and the converse direction (a lexical descendant that is really a symlink out of the tree reads as covered, so stays silent) was undisclosed. - LLP 0160 and LLP 0083 pointed the undecided enrichment question at #481, which this PR closes. It is split out to #492. Co-Authored-By: Claude <noreply@anthropic.com>
One conflict, in `llp/0083`'s `Related:` header line: #462 added LLP 0066 and LLP 0067, this branch added LLP 0160. Resolved as the union, since the two edits are independent additions to a list. Everything else merged textually clean, including `codex/src/exchange-projector.js`, where the two sides turned out to touch disjoint concerns: #462 rewrote the rollout fallback's lookup key (session container to thread id, the new `resolveRolloutCwd`), while this branch changed `refused_workspace_cwd`'s predicate in `resolveCodexContext` and the warn's comment block above it. Checked rather than assumed: - `isEqualOrDescendant` is byte-identical on master and still exported from the usage-policy index, so the shared predicate this branch reuses (LLP 0069 R8) is the same function it was written against. - The two review-established facts survive. Nearest-governs is intact (nearest `.hypignore` walk byte-identical, deepest list entry via `matchDepth`), so the disclosed non-monotonicity is still real and the pinning test still has something to pin. The narrowed warn still carries no `class` / `declared` / `governed_by`, which is the executed ground the disposition rests on. - `scopeGoverns` and `canonicalSpellings` still sit next door after #484, so the deliberate choice of the lexical predicate on this hot path (LLP 0049 R6) remains a live choice rather than a stale one. - LLP 0160 §corrections-0083 says 0083's closing #476 sentence went stale. That bullet is byte-identical between the merge base and master, so the correction still lands where it says it does. - `pathsEqual` is still used by `selectCodexWorkspace`, so it is not orphaned by the predicate swap. Checks on the merge result: `npm test` 3081 pass / 8 fail, `npm run typecheck` clean. The 8 are `test/core/leave-command.test.js`, pre-existing: a pristine `origin/master` (bd7dd0b) worktree fails the identical 8 by name, diffed as sorted name lists with zero difference, at 3078 pass / 8 fail. The merge adds exactly the 3 new #481 passes and no new failures. Co-Authored-By: Claude <noreply@anthropic.com>
… finding 2 of 2) (#491) * Codex workspace-cwd refusal is an ancestor test, not a byte test (#481) The refusal predicate was "a `workspaces` key was substituted and it is not byte-equal to the in-band cwd", so the completely ordinary shape "a session running in a subdirectory of its declared workspace" emitted `plugin.codex.usage_policy_workspace_cwd_refused` at `warn` on every single turn, with both directories clean and no `.hypignore` anywhere. A privacy warn that fires constantly on the common case is read as noise, and the signals beside it are read as noise with it. Narrow it to keys off the in-band cwd's ancestor chain. The justification is not "close enough": when the key is an ancestor, the cwd's `.hypignore` walk passes through the key and every machine-local entry governing the key also governs the cwd, so resolving the cwd is at least as restrictive as resolving the key would have been. The refusal can only tighten, so there is nothing to report. Off the chain the two walks are incomparable, in both directions - a sibling tree, and a key BELOW the cwd whose own walk covers strictly more - and those still warn. The test is the shared `isEqualOrDescendant` (LLP 0069 R8), not a second copy of the path rule. Lexical rather than spelling-agnostic on purpose: `scopeGoverns` buys its extra reach with realpath syscalls this per-exchange seam must not spend (LLP 0049 R6), and the residue errs toward reporting. LLP 0160 records the decision, the ancestor-monotonicity argument, and what is now stale in LLP 0083's #476 limit sentence; LLP 0083 gains the forward-ref. Does NOT address the other finding deferred from PR #477: a row recorded where it used to drop still carries an ignored workspace's identity through the key's surviving enrichment role. That is a privacy-relevant default the corpus does not settle, and it stays open under #481. Co-Authored-By: Claude <noreply@anthropic.com> * Review: the ancestor test's justification was a false monotonicity proof The behaviour is kept; the reason given for it was wrong and is replaced. LLP 0160, the `workspaceCoversCwd` JSDoc and the warn-site comment all claimed that when the key is an ancestor of the in-band `cwd`, `resolve(cwd)` is at least as restrictive as `resolve(key)`, so refusing the substitution "can only tighten". That is false on this resolver. Nearest-governs means a declaration between the key and the `cwd` overrides the key's and may be less restrictive. Swept against the real `createUsagePolicyResolver` over `.hypignore` bodies and machine-local list classes at four depths on one ancestor chain: 131 of 576 arrangements resolve the `cwd` LESS restrictively than the key, spanning ignore->local-only, local-only->full and ignore->full. Restated on the ground that actually holds: an ancestor key is not a guess about where the session ran, it is a less specific name for the same tree, and nearest-governs makes the `cwd`'s own declaration authoritative. So the signal reports the location inference, not a verdict change - and the residue (an ancestor key resolving more restrictively than its `cwd`, now silent) is disclosed rather than implicitly denied, and pinned by a test. Also corrected: - the symlink residue cited #479 as an open gap; #479 was closed for the gate by #482/#484. It is knowingly retained here, at this reporting-only predicate, and the converse direction (a lexical descendant that is really a symlink out of the tree reads as covered, so stays silent) was undisclosed. - LLP 0160 and LLP 0083 pointed the undecided enrichment question at #481, which this PR closes. It is split out to #492. Co-Authored-By: Claude <noreply@anthropic.com> * Review round 2: purge the disproved monotonicity claim from the places round 1 missed, and ground the retained justification by execution Round 1 replaced the false "an ancestor key can only tighten" proof in LLP 0160 §decision and in the `workspaceCoversCwd` JSDoc, but the same claim survived in three other places, one of them LLP 0160's own abstract, where it contradicted the §decision body directly: - `llp/0160-...md:13` - the summary blockquote still said "An ancestor key cannot have changed the `.hypignore` verdict". - `llp/0083-...md:144` - the `Extended-by` block said the same. - `test/plugins/codex-exchange-projector.test.js` - the `@ref LLP 0160#decision [tests]` block said "taking its own `cwd` over an ancestor key can only tighten the verdict". A ref that states a disproved premise is exactly what CLAUDE.md's "keep refs honest" rule is for. All three now state the ground that actually holds, and say explicitly that it is NOT a monotonicity argument. Round 1 also carried the disposition on a reasoned claim: "in every one of the 131 cases the loosening is the user's own nested declaration". Two changes: 1. A better, EXECUTED ground is now stated and pinned by a test. The warn this PR narrows carries no usage class at all. On `origin/master` it fires with an identical field set on a subdirectory turn with no `.hypignore` anywhere, on one where key and `cwd` both resolve `ignore`, and on the loosening arrangement. It could never have distinguished them, so narrowing it removes a constant, not privacy information. The genuine-refusal test now asserts the warn carries no `class`, `declared` or `governed_by`. 2. The provenance claim is corrected where it was too strong. The machine-local list, the only source that reaches an explicit `full`, has exactly two writers, both behind explicit `hyp ignore`/`unignore`/`policy set` verbs, and LLP 0071 §not-central forbids anything central writing one - so the `->full` transitions really are the user's own. But a `.hypignore` is a COMMITTABLE file by design (LLP 0071 §not-dotfiles) and the ancestor walk has no vendored-tree exclusion, so the `ignore->local-only` transition can come from a dependency's own file. Bounded (the walk only goes up, and `.hypignore` cannot express `full`), unchanged by this PR, and now disclosed rather than asserted away. Also notes that the 131/576 count's mixed slice is enumeration-dependent; the two source-pure slices, 50 and 75, reproduce exactly. No behaviour change: predicate, gate, row and drop path untouched. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: test <test@test.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: test <test@example.com>
1 participant
Root cause
src/core/usage-policy/matcher.jscompares strings. A filesystem hands onedirectory several strings, and
realpath(2)folds exactly one of themechanisms that produce them (symlinks). On a default macOS (APFS) volume the
kernel accepts,
stats, andchdirs to two more kinds of spelling it will notfold:
Cafécomposed (U+00E9) and decomposed(
e+ U+0301) are one directory. macOS frameworks and Finder-derived pathsemit NFD while typed and JSON-transported paths are usually NFC.
Projandprojare one directory, on a case-insensitivevolume.
So the machine-local list's membership test could be handed a
cwdwhosespelling differs from the
diran entry was declared with, and the shared gatereturned
fullfor a directory the user opted out of. The two paths beingcompared are produced by different processes at different times (a CLI
resolving a mark, versus a client reporting a
cwd), so a divergence betweenthem is ordinary rather than user error.
The
.hypignoreancestor walk is not affected: itstats each candidaterather than comparing two strings, and
existsSyncalready succeedscase-insensitively for the file itself.
Reproduced first, on this host
Against
origin/master(1555f13), with the two spellings built as stringliterals and fed to the real resolver over an in-memory list:
cwdmaster{/root/café(NFC)/proj, local-only}/root/café(NFD)/projfull{/root/café(NFD), ignore}/root/café(NFC)/proj/subfullWhat is folded, and where
New
src/core/usage-policy/fold.js.filesystem access, cannot fail, and is the identity on a path that is already
composed. There is no volume on which folding NFC and NFD together is wrong.
the mounted volume, not of the platform: an APFS volume can be formatted
case-sensitive and every ext4 volume is. Folding it unconditionally would
merge two genuinely different directories, which is a correctness bug in the
other direction.
createVolumeCaseProbecompares thedev/inoof a pathagainst a case-flipped spelling of its last segment (so the parent chain
stays traversable under its exact spelling and the answer is about the volume
the directory is on), memoizes by
dev, and returns constantfalsewithno syscall at all off darwin. An undetermined probe resolves to
"case-sensitive", which is the pre-fold behaviour.
The fold is applied at the gate:
matchListfolds the incomingcwdand eachentry's declared
dir.isEqualOrDescendantstays lexical and pure.The fold has to distribute over the path separator, since its only consumer
is a segment-aware prefix test:
fold(a + '/' + b) === fold(a) + '/' + fold(b).Both halves do, and the property is asserted rather than assumed.
The proof that a folded spelling only ever adds restriction
Producing a folded spelling is necessary but not sufficient, and this is the
part that took the work. Nearest-governs is an argmax over match depth, and
an argmax discards verdicts instead of merging them. A less restrictive entry
that gains reach through its folded spelling can become the deepest match and
displace a broader restrictive entry that already governed: a carve-out spelled
NFC punches a hole in a private tree spelled NFD, and the directory starts
recording and forwarding. That is the same defect #482's round-1 review found
and fixed for the symlink class, arrived at by a different mechanism.
So nearest-governs is evaluated twice, once over the spellings exactly as
declared (which reproduces the pre-fold verdict bit for bit) and once folded,
and the more restrictive of the two answers wins, the declared one breaking
a class tie because it is the spelling the user typed. The resolved class is
max(pre_fold, folded)on the restrictiveness lattice by construction,which makes "folding never opens the gate" structural rather than a property
someone has to remember.
Three tests pin it, and the third is the one that makes the claim mean
something:
resolve: a carve-out that gains reach only by folding does not punch a hole in a broader restrictive entryis the direct analogue of Usage-policy gate matches directories, not path spellings: canonicalize both sides #482's regression fixture:
[{/root/real, ignore}, {/root/real/café(NFC), full}]againstcwd = /root/real/café(NFD)/subresolvesignore, notfull.resolve: a carve-out declared in the same spelling as the entry it carves out of is still honoredis the positive half, so the guard is not just over-restriction.
resolve: folding never loosens, over every arrangement of a nested pairisexhaustive rather than illustrative over the arrangements it enumerates:
the loop runs 72 cases (round-1 review counted them; the 108 figure here was
wrong), each
asserting the folded verdict is at least as restrictive as the pre-fold rule
recomputed independently in the test.
VERIFIED on this Linux host
These are executed facts on this machine, not reasoning:
origin/masterand is closed here. Themechanism is entirely string-level, so it is fully witnessable on ext4:
String.prototype.normalizeis a pure function of the string, the twospellings are literals, and what is under test is whether the comparison
logic folds the spellings it is handed.
hole-punching guards).
/, and a sibling sharing a folded prefix(
/root/café-other,/root/caféxyz) is still not matched.units than NFC for the same name, so a declared-string depth ranks a
decomposed ancestor above a composed descendant and inverts nearest-governs.
Found by a surviving mutant, then fixed and pinned.
/root/Projdoes not govern/root/projhere.hole-punching guard still holds under it.
createVolumeCaseProbeis inert off darwin: constantfalse, zero syscalls.hashed-path skip event, all over an injected
statSync.path.
REASONED BUT UNVERIFIED, needs a macOS host
Nothing below has been executed:
is dead code on Linux. Its darwin path is not exercised by any test in this
repo, and the injected-verdict tests cover the matcher's behaviour given a
verdict, never the probe that produces one. Specifically unverified: that
dev/inocomparison across a case-flipped spelling gives the right answerin the presence of APFS firmlinks and
/System/Volumes/Data; that flippingthe last segment does not hit a real differently-cased sibling often enough to
matter (the
inocheck should catch it, but "should" is the operative word);and that the probe's cost is acceptable on a real volume.
directory. That is APFS's documented normalization-insensitivity, and it is
the premise of the whole issue, but it is a premise here, not an observation.
written NFC, a
cwdreported NFD, or the reverse). The fix is symmetric, sodirection does not change the outcome, but no field evidence was collected.
A reviewer with a Mac should run
test/core/usage-policy-fold.test.json adefault volume and on a case-sensitive one, and check that
createVolumeCaseProbe()('~/Documents')returnstrueon the former andfalseon the latter. This PR does not claim that has been done.Performance
The per-
cwdmemo is keyed on the lexical path and consulted before anyfolding, so a cache hit costs exactly what it did before. Entry spellings and
the per-volume case verdict are computed once per list parse, inside the TTL
window LLP 0049 R6 already bounds, so resolving many distinct
cwds in onewindow folds each entry once rather than once per
cwd. Pinned byresolve: folding happens on the cache miss only, so a repeated cwd re-reads nothing.Measured on this host, two runs each in this worktree and a pristine
origin/masterworktree with the samenode_modulessymlink (500ksteady-state calls, 20k forced misses, 3-entry list):
masterresolve(the per-exchange path)resolve(once per cwd per 5 s)The cache-hit column is indistinguishable inside run-to-run noise, which is the
claim that matters. The numbers above are round-1 review re-measurements (3 runs,
500k calls after a 100k warmup); the original table implied the branch was ~10%
faster, which did not reproduce. There is no speedup, and none is claimed.
Cache-miss is up about 4%, also close to noise.
String.prototype.normalize('NFC')measured in isolation: 59 ns on a pure-ASCII path, 134 ns on an NFD one.
Mutation coverage
Fifteen mutants, each killed by a named test (the table below omits a row for the non-ENOENT probe mutant found in round-1 review). Two of them survived the first
pass and are the reason two tests exist at all (M6 and M10 below), so this table
is evidence rather than decoration.
foldPathdrops the NFC normalization (=master)a local-only entry declared NFC still governs a cwd that arrives NFDfoldPathcase-folds unconditionallycase is NOT folded by default, because this volume is case-sensitiveselectGoverningkeeps only the folded passa carve-out that gains reach only by folding does not punch a hole ...,folding never loosens, over every arrangement of a nested pairselectGoverningkeeps only the declared pass (=master)deepestMatchloses per-entry specificitya carve-out declared in the same spelling ... is still honoreddeepestMatchmeasures folded depth on the declared spellingnearest-governs is measured on the folded spelling, not on the declared oneflipCaseflips the whole path, not the last segmentmemoizes a definite verdict per volume, not per pathcreateVolumeCaseProbe is inert off darwinstatas case-insensitivefails toward the pre-fold behaviour and logs a hashed skipdoes not memoize an undetermined answer as the volume verdictreportFoldemits on every list match, not only a tighteningemits a hashed usage_policy.fold_tightened only when folding changed the verdictreportFoldlogs the rawcwdinstead of a digestresolvethe case verdict is asked per entry, so a per-volume answer applies per entryemits a hashed usage_policy.fold_tightened only when ...Reproducing tests
test/core/usage-policy-fold.test.js, 23 tests. Run against a pristineorigin/masterworktree with the 7 new-module tests elided so the fileimports: 7 pass, 8 fail. The 7 that pass there pass on purpose, as guards
rather than reproducers (the sibling-prefix test, both hole-punching guards, the
exhaustive monotonicity check,
case is NOT folded by default, and the fixturepremise). All 23 pass here.
The file opens with
the two fixture spellings are genuinely different strings that NFC folds together,because an editor, a merge tool, or a
gitfilter that re-normalized the sourcewould collapse both constants to one string and make every test below it pass
vacuously. That test is the tripwire for it.
Structured signal
usage_policy.fold_tightened(debug) fires only when the folded pass reached amore restrictive verdict than the declared spellings did, i.e. exactly the
spelling divergence that would otherwise have opened the gate:
hyp_component,hyp_operation,status,declared_class,folded_class, and acwd_hash(sha256, first 16 hex).
usage_policy.case_probe_skipped(debug) carrieserror_kind: path_case_probe_failed, areason, anerrno, and apath_hash. No raw path appears in any attribute on any branch, asserted inboth tests.
Relationship to #482
This branch is based on
master, not on #482, and does not depend on it.The two are independent aliasing mechanisms:
realpathcannot fold case ornormalization, and folding cannot resolve a symlink. #482 documents this class
under "What this does NOT fix"; its round-1 review filed it as #483.
They do converge on the same structural fix, which is worth saying plainly:
both discovered that widening an entry's reach is unsafe while nearest-governs
is an argmax, and both answer it with a two-pass declared-versus-widened
evaluation keeping the more restrictive answer. That is not a copy: #482's
helper (
canonical.js) is not vendored here and nothing on that branch wasmerged in.
They will conflict in
matcher.js, and that is the intended outcome.Whichever lands second should collapse the two two-pass evaluations into one
pass over one spelling set (lexical, canonical, folded) rather than keep two,
since running the argmax guard twice buys nothing. Neither PR should be
rebased onto the other while both are in review. This PR does not supersede,
subsume, or dominate #482.
What this does NOT fix
section. If a macOS reviewer finds the probe wrong, the NFC half stands on its
own and the probe can be reverted to constant
falseby deleting one branch.hyp purge --subtreestill comparelexically.
hyp ignore --check,policy show,policy unsetand the purgesubtree predicate can still name a different governor than the gate used. Those
are exactly the lines Usage-policy gate matches directories, not path spellings: canonicalize both sides #482 reroutes through its new shared spelling-aware
predicate; adding a second, differently-named folded predicate at the same
three call sites now would collide with a PR in review and leave the repo with
two folding rules instead of one. The right sequencing is: gate now (that is
where the privacy leak is), CLI and purge fold through the one shared
predicate once Usage-policy gate matches directories, not path spellings: canonicalize both sides #482 lands.
the same directory still sit in
local-only.jsonas two entries. The gate nowapplies the most restrictive of them, so this is no longer a privacy hole, but
hyp policy showstill lists both. Upsert identity is Usage-policy gate matches directories, not path spellings: canonicalize both sides #482's territory.cwdvalues, and has no effect on rows already forwarded to a sink.
NFKC, no locale-sensitive case folding.toLowerCaseis anapproximation of the Unicode case-folding APFS uses; a path where they
disagree would simply not fold, which is the pre-fold behaviour.
Verification
npm test: 2950 tests, 2940 pass, 8 fail, 2 skipped. The 8 are the pre-existingtest/core/leave-command.test.jsset, confirmed identical by name againsta pristine
origin/masterworktree with the samenode_modulessymlink(
diffof the sorted failure names is empty).npm run typecheck: clean.test/core/llp-ref-hygiene.test.js: 8 pass, 1 skip, 0 fail.npm run smoke -- session_optout_capture_drop: ok.hypaware-core/plugins-workspace/codex/src/exchange-projector.jsdoes notappear in
git diff origin/master..HEAD, so the conflict surface againstCodex rollout cwd keys on the thread, not the session container (#459) #462 / Codex live projector: an unusable in-band cwd is a miss, not a path (#471) #474 / Codex live projector: an explicit cwd outranks a substituted workspace key at the .hypignore gate #477 is unchanged.
Docs
LLP 0050 gains
§normalization: "the set of spellings that denote a directory"is volume-dependent, not just symlink-dependent, plus the argmax
non-monotonicity, the separator-distribution requirement, the cost model, and
the relationship to #482. LLP 0049
#scopegets anExtended-bynote recordingthat the ancestor walk is unaffected but list membership is not.
Fixes #483