Skip to content

Usage-policy gate matches directories, not path spellings: canonicalize both sides - #482

Merged
philcunliffe merged 3 commits into
masterfrom
fix/issue-479
Jul 30, 2026
Merged

Usage-policy gate matches directories, not path spellings: canonicalize both sides#482
philcunliffe merged 3 commits into
masterfrom
fix/issue-479

Conversation

@philcunliffe

@philcunliffe philcunliffe commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Root cause

src/core/usage-policy/matcher.js resolved every comparison with path.resolve,
which is lexical: it normalizes ./.. and makes a path absolute but
follows no symlinks. The ancestor walk from a symlinked cwd therefore climbed
the link's parents and never met the .hypignore governing the real
directory. Because this is the single shared matcher (LLP 0050), every consumer
inherited it: all four adapter capture seams, hyp purge, the query-seam
visibility filter, hyp ignore --check / policy show, and the machine-local
list membership test.

I reproduced all of it with real symlinkSync fixtures and real node:fs, and
I independently rebuilt the reviewer's measured dead end before building on it.
Confirmed on origin/master (1555f13): canonicalizing only the incoming cwd
closes the capture cases, and turns a local-only entry declared by its symlink
spelling into full. That directory then starts forwarding. Trading a
capture leak for a forwarding leak is not a fix, exactly as the issue says.

What is canonicalized, and where

New src/core/usage-policy/canonical.js. A directory is matched over the set
of path spellings that denote it
(as-given/lexical first, then canonical when
it differs), and the most restrictive verdict any spelling produces wins.

The set, rather than the canonical form alone, is the load-bearing choice. It is
what closes the capture leak without opening the forwarding one, and it makes
the failure mode structural rather than a special case: a realpath that fails
can only remove a candidate spelling, never a verdict some other spelling
already produced. Canonicalization can therefore only ever move the gate toward
more restrictive, never toward full.

Both sides of every comparison get it:

  • resolve (the gate) resolves the incoming cwd over its spellings.
  • The machine-local store's entries are canonicalized on read, when the list
    is parsed, so each entry governs through any spelling of its declared dir.
  • scopeGoverns (new, exported) is the spelling-agnostic
    isEqualOrDescendant, used by hyp purge's subtree predicate and by the CLI
    membership sites (policy unset / unignore --local-only, and
    --check/policy show's "which entry governs this?"). It has to be the same
    predicate resolve used, or the CLI names a governor the gate did not use and
    unset refuses to remove an entry the gate is enforcing.
  • sameDirectory (new, exported) is the upsert identity for a stored entry.
  • isEqualOrDescendant stays lexical and pure, for callers comparing two
    already-canonical strings that must not touch the filesystem.

Declared-path authority

The declaration as written stays authoritative, and canonicalization only ever
widens reach.
Stored entries keep their as-declared dir on disk; nothing is
rewritten. An entry governs both its declared spelling and, when resolvable, the
canonical spelling of what it points at. If the declared path's target changes
or disappears, the declaration still governs the declared spelling, so no
stored entry ever silently loses its class
; it merely stops governing the
canonical form of a target it no longer names. A filesystem change never revokes
a user's declaration. Regression-tested by
resolve: a list entry whose declared target has been deleted keeps governing its declared spelling.

Migration

Canonicalize on read, additively. Nothing is rewritten on write. No on-disk
migration, no version bump: entries written before this gain canonical reach the
moment it ships, and the reach is recomputed each TTL window so it tracks
filesystem changes instead of freezing a canonical form that can go stale.
Writes still store the caller-resolved path, so policy show and --check echo
the spelling the user typed.

The one write-side change is upsert identity: runMarkMachineLocal now
replaces an entry that denotes the same directory by either spelling, rather
than only by exact string. Without it, re-marking a directory through its other
spelling appends a second entry governing the same directory at a different
class, and the nearest-governs tie-break, not the user, decides which class
applies.

ENOENT and fail-toward-privacy

realpath is all-or-nothing and throws ENOENT for a path whose leaf does not
exist, which is routine here (a deleted cwd, a not-yet-created directory, a
unit test over an injected fs of paths that never existed). It never throws into
the gate. Two things happen instead:

  1. Partial canonicalization. Rather than discard the failure, the
    canonicalizer walks up to the deepest resolvable ancestor and rejoins the
    unresolved tail. That matters because the symlink is almost always an
    ancestor, not the leaf: with /tmp a symlink, /tmp/proj/not-created-yet
    still canonicalizes usefully.

  2. The verdict is the most restrictive across the spellings we could form.
    So a canonicalization failure can only lose reach canonicalization would have
    added; it can never demote a class the as-given spelling already produced,
    and it cannot fall through to full unless every spelling says full. That
    is the fail-toward-privacy property.

    Correction from round-1 review: as originally written this was not purely a
    consequence of the design. Set-matching is monotone only where verdicts are merged, and
    the machine-local list nearest-governs step is an argmax over match depth, which
    discards verdicts. That let a canonicalization-widened carve-out punch a hole in a
    broader restrictive entry, turning ignore into full so the directory started
    forwarding. It is now guarded explicitly: nearest-governs is evaluated twice, once
    over the declared spellings alone and once over the widened set, and the more restrictive
    answer wins. Round-2 review searched 14 adversarial shapes for a case where the branch is
    less restrictive than master and found none, and showed why structurally:
    canonicalSpellings(key)[0] is always path.resolve(key), so the first pass reproduces
    master exactly and every other spelling can only feed the most-restrictive merge.

Every non-full outcome emits usage_policy.canonicalize_failed with
error_kind: path_canonicalize_failed, the errno, resolved: partial|none, and a hashed path (never a raw local path, the same
discipline as the usage_policy.export_drop aggregate). ENOENT is routine, so
it logs at debug; only a wholly unresolvable path escalates to warn.

Performance

One realpath per distinct cwd per TTL window, and zero on the
per-exchange hot path.
The per-cwd memo is keyed on the lexical path and
consulted before canonicalization, so a cache hit costs nothing new. Entry
spellings are computed once per list parse, inside the same TTL. This is the
identical bound LLP 0049 R6 already sets for the ancestor walk.

Measured on this branch vs a pristine origin/master worktree, same machine
(500k steady-state calls, 20k forced cache misses):

master this branch
cache-hit resolve (the per-exchange path) 344 ns 320 ns
cache-miss resolve (once per cwd per 5 s) 24 us 70 us

hyp purge's subtree predicate runs per row, so it memoizes the verdict per
distinct row cwd for the life of one purge run. Regression-tested by
resolve: canonicalization costs one realpath per cwd per TTL window, not one per call.

Reproducing tests

New test/core/usage-policy-symlink.test.js, plus two CLI tests in
test/core/ignore-local-only-command.test.js and one in
test/core/purge-command.test.js. Every fixture builds a real symlink(2):
path.resolve is lexical, so a string-only test can pass while the real
filesystem leaks.

Failed on a pristine origin/master worktree (same node_modules, new-API
tests elided so the file could import), pass here:

  • resolve: an ignored directory reached by its symlink spelling is still ignored
  • resolve: a descendant of a symlink that points into an ignored tree is ignored
  • resolve: a local-only entry declared by its symlink spelling governs the real directory
  • resolve: a local-only entry declared canonically governs a cwd that arrives by symlink
  • resolve: an unresolvable cwd under a symlinked ancestor still meets the .hypignore governing its real tree
  • resolve: canonicalization costs one realpath per cwd per TTL window, not one per call
  • hyp ignore --private on the real path upgrades an entry declared by its symlink spelling, not duplicating it
  • hyp unignore --local-only by symlink spelling removes an entry declared canonically
  • purge subtree matches a row recorded under a symlink spelling of the target

Three tests pass on master on purpose, as guards rather than reproducers:
resolve: a .hypignore governing the symlink's own ancestors still governs (the converse spelling is not lost),
the segment-aware sibling-prefix test, and the deleted-target test. To show the
first has teeth, I applied the one-sided realpath(cwd) patch to the pristine
worktree: it reddens that test plus both local-only tests, so the suite tells
the dead end apart from the fix rather than just rewarding any canonicalization.

Twelve mutants, each killed by a named test (every new predicate branch, both
directions):

mutation test that reddens
canonicalSpellings drops the canonical spelling (= master) 10 tests, incl. an ignored directory reached by its symlink spelling
canonicalSpellings drops the as-given spelling (= the dead end) the converse spelling is not lost
canonicalizeDirSync abandons the resolvable prefix an unresolvable cwd under a symlinked ancestor, canonicalizeDirSync: full, partial, and unresolvable outcomes
matchDepth loses per-entry specificity among nested v2 list entries the most specific (longest) dir wins
matchDepth never returns null 14 tests, incl. sibling-prefix directory is NOT matched
spelling merge keeps the last verdict, not the most restrictive the converse spelling is not lost
list entries keep only their declared spelling a local-only entry declared by its symlink spelling governs the real directory
sameDirectory degrades to string equality hyp ignore --private ... not duplicating it
scopeGoverns degrades to the lexical predicate hyp unignore --local-only by symlink spelling ...
purge subtree predicate goes lexical purge subtree matches a row recorded under a symlink spelling
mark upsert identity goes back to string equality hyp ignore --private ... not duplicating it
unmark membership goes lexical hyp unignore --local-only by symlink spelling ...

Verification

Docs

LLP 0050 gains §canonicalization, since this changes what the shared gate
means. LLP 0049 #scope and LLP 0071's "Match semantics" get Extended-by
notes: both the walk and list membership are over real paths.

What this does NOT fix

  • It does not let a nested loosening cross spellings, with one qualification. An explicit
    full/sync carve-out declared under one spelling does not loosen a broader
    restrictive entry declared under the other: across spellings the gate keeps
    the more restrictive verdict. The qualification, found in round-2 review: when the broader
    restrictive entry reaches the cwd only via canonicalization, a same-spelling carve-out is
    still honoured, so the outcome there is less restrictive than a full lattice meet would give.
    That is deliberate; blocking it would break a carve-out the user declared on purpose. That is deliberate (privacy-safe direction) and
    is asserted in resolve: nested entries keep nearest-governs when the outer one matched by its canonical spelling,
    but a user who wants a carve-out has to declare it under the same spelling as
    the entry it carves out of.
  • It does not close case D from the issue. The Codex
    ChatGPT-subscription route states no cwd, so the row's spelling comes from
    the declared workspaces key. That key now goes to the gate canonicalized,
    which is strictly better, but the provenance question (should a declared
    workspace key be trusted as the cwd at all) is PR Codex live projector: an explicit cwd outranks a substituted workspace key at the .hypignore gate #477's territory and this
    branch deliberately leaves exchange-projector.js alone.
  • It does not narrow the TTL. A .hypignore or a mark is still honored
    within the matcher's 5 s window rather than instantly, and a symlink retargeted
    mid-window is likewise seen within the TTL. Unchanged from master; the
    CLI-to-daemon invalidation signal is still future work.
  • It does not canonicalize already-recorded rows. cwd values written to the
    cache before this keep whatever spelling the client reported. hyp purge --subtree now finds them either way, but a raw SQL query over cwd still
    sees the as-recorded string.
  • It does not touch case-insensitive or Unicode-normalization path aliasing.
    On macOS/APFS, /Users/me/Proj and /Users/me/proj can be the same directory
    and realpath does not fold them, so that class of aliasing remains open.
  • No server-side effect. Rows already forwarded to a sink before a
    local-only entry began governing the real spelling stay forwarded; deletion
    at the destination is out of scope (LLP 0104).

Fixes #479

`resolve` used `path.resolve` on both sides, which is lexical: the ancestor
walk from a symlinked `cwd` climbed the link's parents and never met the
`.hypignore` governing the real directory. Because this is the single shared
matcher, every consumer inherited it.

Canonicalizing only the incoming `cwd` closes the capture leak and opens a
forwarding one: the machine-local store keeps the path the user supplied, so an
entry declared by its symlink spelling stops governing and that directory starts
forwarding. Instead, both sides now resolve over the *set* of spellings that
denote a directory (as-given plus canonical) and the most restrictive verdict
wins, so a `realpath` failure can only remove a candidate spelling, never a
verdict another spelling already produced.

Co-Authored-By: Claude <noreply@anthropic.com>
Resolving over a set of spellings is monotone only where verdicts are merged.
The machine-local list's nearest-governs step is an argmax over match depth, and
an argmax discards verdicts instead of merging them: a less restrictive entry (an
explicit `sync` carve-out) that gained reach through its canonical spelling could
become the deepest match and displace a broader restrictive entry that already
governed. Measured on the review head, entries `[{<root>/real, ignore},
{<root>/link -> <root>/real/proj, full}]` resolved `<root>/real/proj/sub` to
`full` where the lexical matcher on master said `ignore`: the one direction
canonicalization was documented never to move.

Nearest-governs is now evaluated twice, over the declared spellings alone and
over the widened set, keeping the more restrictive answer (the declared one
breaking a class tie, being the spelling the user typed). The selection also
moves behind one exported `governingListEntry`, so `hyp ignore --check` /
`policy show` names the entry the gate used instead of re-deriving it as
"longest declared string", which picked a different entry once an entry could
match through its canonical spelling and mis-scoped the residual row count.

LLP 0050 #canonicalization now states the invariant and the step that needs care
to hold it, plus the visible cost: a nested loosening does not cross spellings.

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

Copy link
Copy Markdown
Contributor Author

Review of #482 at 70a2a1af (round 1 of 2)

Verdict: findings. One privacy-relevant defect, fixed and pushed
(db771296); one consistency defect, fixed in the same commit; one residual
promoted to its own issue (#483); two informational notes left for a human.

The core of the PR holds up. I rebuilt the reproduction and the dead end from
scratch and both check out. The set-of-spellings design is the right shape, the
ENOENT handling is genuinely fail-toward-privacy, and the hot path really is
unchanged. But the headline invariant, "canonicalization can only ever move the
gate toward more restrictive, never toward full", was not true as written.

Priority 1: yes, a combination produced a LESS restrictive outcome than master (was: high, now fixed)

src/core/usage-policy/matcher.js:214 (matchList, at 70a2a1af)

Resolving over a set of spellings is monotone only where verdicts are
merged. The machine-local list's nearest-governs step is an argmax over
match depth
, and an argmax discards verdicts instead of merging them. So a
less restrictive entry that gains reach through its canonical spelling can
become the deepest match and displace a broader restrictive entry that already
governed. Nothing in "match over a set of spellings" prevents that.

Measured by execution, same fixture on 70a2a1af and on a pristine
origin/master worktree (1555f13), same node_modules, real symlink(2).
Store: [{ <root>/real, ignore }, { <root>/link, full }] with
<root>/link -> <root>/real/proj:

cwd master 70a2a1af after the fix
<root>/real/proj/sub ignore full ignore
<root>/real/proj ignore full ignore
<root>/link/sub full full ignore
<root>/real/other ignore ignore ignore

Row 1 and row 2 are the leak: a directory inside a tree the user marked
ignore starts recording and forwarding because a --sync carve-out
declared under a different spelling got canonical reach and won
nearest-governs. That is the one direction this PR documents as structurally
impossible, and it is a leak this PR introduces relative to master, in the
subsystem whose whole purpose is closing one. (Row 3 is the intended capture-leak
fix, now stronger.)

The .hypignore side is genuinely monotone, and I confirmed it: parseHypignore
has no token for a less restrictive class (format.js IMPLEMENTED is
{ignore, local-only}, unknown tokens clamp to ignore), so a second spelling
can only ever add a restrictive verdict to walk. The non-monotonicity was
entirely on the machine-local entry side.

The PR's own test
resolve: nested entries keep nearest-governs when the outer one matched by its canonical spelling
asserts no-cross-spelling-loosening on the cwd side only. The entry side
was unasserted and behaved the opposite way, so the author's stated intent and
the code disagreed. I fixed the code to match the stated intent rather than
rewriting the intent, because LLP 0049 §fail-safe makes "suppress more" the
tie-breaker for this subsystem.

Fix (matcher.js:367, :412): nearest-governs is now evaluated twice, once
over the declared spellings alone (exactly what the lexical matcher decided) and
once over the widened set, and the more restrictive of the two answers wins,
the declared one breaking a class tie because it is the spelling the user typed.
Result is now mostRestrictive(master_verdict, widened_verdict) by
construction, so "never less restrictive than master" is structural rather
than a property to be remembered.

Does it over-restrict a legitimate carve-out? No, not relative to master,
which is now a floor. Relative to 70a2a1af it costs exactly the case the PR
body already documents as unsupported: a nested loosening declared under one
spelling does not loosen a broader entry declared under the other. I added the
positive half of that to the test, so a same-spelling carve-out is pinned as
still honored:
resolve: a carve-out that gains reach by canonicalization does not punch a hole in a broader restrictive entry
asserts both the block and { <root>/real: ignore } + { <root>/real/proj: full }
resolving <root>/real/proj/sub to full.

Verified: the test reddens when selectGoverning is reduced to the widened
pass alone (i.e. 70a2a1af's behaviour) and passes with the fix;
git diff 70a2a1af..HEAD -- src/core/usage-policy/matcher.js shows the two-pass
selectGoverning.

Priority 4: --check / policy show named a different entry than the gate used (was: low, now fixed)

src/core/commands/clients.js:1156 at 70a2a1af

resolveCheckScopeDir widened its match set to scopeGoverns but kept choosing
the winner by longest declared dir string, while matchList chooses by
deepest matched spelling. Once an entry can match through its canonical
spelling those are different entries. Concretely: store
[{ <root>/a-very-long-link-name, local-only }, { <root>/r, local-only }] with
the link pointing at <root>/r/p, cwd = <root>/r/p/deep. The gate's verdict
comes from <root>/r; the old reduce picked the link entry and scoped the
residual row count to a path nothing was ever recorded under, so
hyp ignore --check --json reported residualCachedRows: 0 with two rows
sitting in the cache. That is the exact failure the function's own doc comment
says must not happen.

Fix: the selection moved behind one exported governingListEntry
(matcher.js:442), used by both matchList and resolveCheckScopeDir
(clients.js:1159) - R8's one shared thing instead of a second copy of the
rule. Verified: reverting resolveCheckScopeDir to the old reduce reddens
hyp ignore --check scopes the residual count to the entry the gate used, not the longest declared string
(reports 0, expects 2); the new selector also picks up matchList's
more-restrictive-class tie-break, which the old reduce lacked.

Priority 2: ENOENT and fail-toward-privacy - confirmed, and the dangling symlink is now covered

Verified by execution against real fixtures, branch vs pristine master, and
every outcome is identical to master - no demotion, no throw, no
fall-through:

case master branch
dangling link (target deleted) inside an ignored tree ignore ignore
<dangling>/child ignore ignore
dangling link outside, pointing into an ignored tree full full
symlink loop (ELOOP) full full
list entry declared as a now-dangling link ignore ignore
path that never existed full full

canonicalizeDirSync on a dangling link returns
{ resolved: 'partial', errno: 'enoent' } with the path unchanged - it
cannot resolve through a link whose target is gone, so it recovers no more than
the link's own parent. That loses reach canonicalization would have added and
demotes nothing, which is exactly the claimed property. ELOOP behaves the same
way. Nothing throws into the gate at any point.

The suite had no dangling-symlink test. Added
resolve: a cwd reached through a dangling symlink keeps the class its as-given spelling produces,
covering both directions (dangling link under an ignored tree; a list entry
declared as a now-dangling link) plus the canonicalizeDirSync outcome.
Verified: it reddens under the M9 mutant (canonicalizer abandons the
resolvable prefix).

Note for a reader, not a finding: resolved: 'none' is close to unreachable for
an absolute path, since the walk terminates at / and realpath('/') succeeds.
So the warn escalation at canonical.js:133 fires only if the filesystem root
itself is unreadable, and every real failure logs at debug. That is the
conservative direction (ENOENT is routine and must not be noisy) but the
warn branch is effectively dead in practice.

Priority 3: performance re-measured, not trusted

My own numbers, this host, same harness in both worktrees, two runs each
(500k steady-state calls, 20k forced misses):

master 70a2a1af after my fix
cache-hit resolve (per-exchange path) 372 / 404 ns 441 / 376 ns 436 / 378 ns
cache-miss resolve (once per cwd per 5 s) 26.3 / 27.1 us 76.9 / 75.9 us 76.3 / 78.3 us

The hot-path claim holds: cache-hit cost is indistinguishable from master
inside run-to-run noise, which confirms the memo is keyed on the lexical path and
consulted before canonicalization. My absolute numbers are higher than the PR's
(344/320 ns) on both sides, so this is a slower machine, not a discrepancy. The
cache-miss ratio I measure is ~2.9x versus the PR's ~2.6x - same order, claim
substantially accurate. My two-pass change adds no measurable cost (it is two
passes over an already-parsed entry list, zero extra syscalls).

resolve's memo can grow unbounded - Map, no eviction, no size cap
(matcher.js:111) - but master is byte-for-byte the same in that respect
(grep for cache.delete|cache.clear|cache.size returns nothing in either
tree), and expired entries are overwritten rather than accumulating per lookup.
200k distinct cwds cost ~50 MB in both trees. Not a regression from this PR;
left alone deliberately, see below.

The 5 s TTL does not create a cross-spelling skew class. Two spellings are
two cache keys with independent expiry, which is the same situation two distinct
cwds already had on master; the bound is still "within the TTL" per spelling,
and entry spellings are recomputed with the shared list parse in the same window.

Priority 5: logging privacy - clean

canonical.js logs path_hash only (sha256, first 16 hex chars). I read every
attribute on the event: component, operation, status, error_kind,
errno, resolved, path_hash. No raw path on any branch. resolved is a
closed enum (full|partial|none) built by the canonicalizer, never derived from
path content, so it cannot leak a path. errno is a lowercased error code. The
digest idiom matches the adjacent usage_policy.export_drop aggregate it cites.

Priority 7: mutation table verified by execution (9 of 12)

I ran nine of the twelve mutants against
usage-policy-symlink + usage-policy + usage-policy-local-only +
ignore-local-only-command + purge-command. Baseline green; every mutant
killed by the test the table names
: drop-canonical-spelling (10 reds),
drop-as-given (the converse spelling is not lost), spelling-merge-keeps-last
(same test), matchDepth never-null (14 reds incl. the sibling-prefix tests),
list-entries-declared-only, sameDirectory string equality, scopeGoverns
lexical, purge predicate lexical, canonicalizeDirSync abandons the prefix. Re-ran
all nine after my change: still all killed, and my new tests widen three of the
kill sets.

The dead-end discrimination claim also checks out. On a pristine master
worktree with the new-API tests elided, the reproducer file is 3 pass / 7 fail -
exactly the three guards the PR names. Applying the one-sided
realpath(cwd) patch to that same worktree flips it to 4 pass / 6 fail and
reddens the converse spelling is not lost plus both local-only tests,
precisely as claimed, while the rest of the usage-policy suite stays green
(50/50). The suite does tell the dead end apart from the fix.

Priority 8: docs, refs, style - clean

test/core/llp-ref-hygiene.test.js passes (8 pass, 1 skip). npm run typecheck
clean. LLP 0050's new {#canonicalization} anchor exists and every markdown
link out of the new section resolves (0049#requirements, 0049#enforcement,
0049#non-goals, the self-link #canonicalization); the Extended-by notes in
LLP 0049 #scope and LLP 0071 both point at it correctly and describe it
accurately. No em dash (U+2014) on any added line in the PR, and none on any line
I added. No semicolons, no @typedef, no inline import() types, type-import
specifiers root-anchored.

I also updated LLP 0050 #canonicalization:114 and the canonicalSpellings
JSDoc so the stated invariant matches the code: producing a set of spellings is
necessary but not sufficient, and the argmax step is where it has to be enforced.
The visible cost (a nested loosening does not cross spellings) is now stated in
the decision rather than only in the PR body.

Priority 6: the macOS residual - real, and it now has an issue

Filed as #483 (neutral:fix, since the code is on master); no existing
issue covered it.

My judgement: narrower than the symlink class, but it deserves to be tracked,
not absorbed as an accepted limitation.
realpath(2) folds symlinks and
nothing else, so on a default APFS volume the kernel accepts spellings the gate
treats as different directories. The case half is the less likely trigger, since
a cwd usually comes from process.cwd() and tab-completion reproduces the
on-disk spelling. The Unicode normalization half is the serious one: macOS
frameworks and Finder-derived paths emit NFD while typed and JSON-transported
paths are usually NFC, and the two paths being compared here are produced by
different processes at different times (the CLI resolving a mark, versus a
client reporting a cwd). For an accented directory name that divergence is
ordinary rather than user error. Given what this repo captures on developer
machines, a silent opt-out failure is the worst failure mode the subsystem has,
so "documented residual" is not enough.

Two things make it tractable rather than urgent: the exposure needs the two
spellings to actually diverge, and after the fix above a fix here is cheap and
safe by construction, because a new candidate spelling can only add restriction.
#483 spells out the shape (NFC unconditionally; case-fold only behind a
per-volume case-sensitivity probe, since folding unconditionally would
over-restrict on Linux and on case-sensitive APFS) and flags that it cannot be
honestly verified on a Linux host.

Verification of the whole tree after my change

Left unfixed - decisions for a human

  1. resolve's memo is unbounded. matcher.js:111 is a Map with no
    eviction and no size cap, so a long-running daemon that sees many distinct
    cwds grows monotonically (~50 MB per 200k distinct cwds, measured). This is
    identical on master and this PR does not make it worse, so I left it
    alone rather than widen the diff of a privacy fix. Decision: whether to
    cap or sweep the memo (an LRU bound, or an opportunistic sweep of expired
    entries on insert) as separate work. Relevant because Daemon OOM (4GB heap) on first central push after fleet enrollment; crash strands pending 'attach claude' action #280 already reports a
    daemon OOM; I did not investigate whether these are related and I am not
    claiming they are.
  2. A nested loosening does not cross spellings, now by design in both
    directions rather than only on the cwd side. A user who writes
    hyp ignore --sync <symlink> inside a tree marked --private under the real
    spelling will find the directory still local-only, and has to re-declare the
    carve-out under the same spelling. hyp policy show reports the class
    actually in force, so it is diagnosable. Decision: accept this (my
    reading of LLP 0049 §fail-safe, and what the PR body already said it wanted),
    or decide that an explicit user classification should win over a broader
    entry regardless of spelling - in which case the invariant in LLP 0050
    §canonicalization must be weakened to match, and the loosening documented as
    deliberate rather than claimed impossible. It cannot be both, which is what I
    found.
  3. Still open and out of scope here, unchanged from the PR body: case D of
    Usage-policy matcher never canonicalizes: a symlinked spelling of an ignored directory escapes its .hypignore (all adapters, hyp purge, query visibility) #479 (declared-workspace-key provenance, Codex live projector: the workspace-key substitution can feed the .hypignore gate a directory the session never ran in #476 / Codex live projector: a workspaces turn-metadata header preempts the rollout session_meta.cwd, turning a correct .hypignore drop into a record #480 / PR Codex live projector: an explicit cwd outranks a substituted workspace key at the .hypignore gate #477's territory,
    exchange-projector.js deliberately untouched); the TTL is not narrowed;
    already-recorded cwd values are not rewritten; nothing is done at a sink
    that already received a row.

Round-1 outcome

The design decision at the centre of this PR is correct and the evidence behind
it is real. It needed the argmax step closed to actually deliver the invariant it
claims. That is done and pushed to fix/issue-479 as db771296. Still a
draft
- I do not mark PRs ready and I do not merge. Decisions 1 and 2 above are
the only things I am holding for a human.

LLP 0050 #canonicalization and `selectGoverning`'s JSDoc both claimed, without
qualification, that "a nested loosening does not cross spellings". The guard
compares the declared-spelling answer against the widened one and keeps the
more restrictive, so it can only preserve a verdict the declared pass actually
produced. When the broader restrictive entry reaches the cwd only through
canonicalization, the declared pass matches nothing and ordinary
nearest-governs picks the deeper carve-out, which does cross spellings.

That is not a demotion: the lexical matcher matched neither entry in that
shape, so `full` is what it returned too, and the outcome equals declaring both
entries canonically. But the unqualified sentence is what a reader of a privacy
gate will rely on, so it is now stated with its condition and the shape is
pinned by a test rather than left to be rediscovered.

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

Copy link
Copy Markdown
Contributor Author

Review of #482 at db771296 (round 2 of 2, final)

Verdict: findings. Three findings. Two fixed and pushed (27dc367); one is
in the PR body, which I am not permitted to edit, so it is left for a human with
the exact edits spelled out below.

I attacked round 1's two-pass remedy directly, by execution, against a pristine
origin/master worktree (1555f13) with the same node_modules. I could not
break it.
The headline result: no shape makes the gate less restrictive than
master, a same-spelling carve-out still works, and the CLI and the gate agree
everywhere I could construct a disagreement. What I did find is that the
invariant as written is stronger than the code delivers, in a direction that is
harmless but misleading, and that follow-up issue #483 was scoped against code
that does not exist on the branch it was dispatched to.

Finding 1: the "a nested loosening does not cross spellings" invariant was overstated (low, fixed)

llp/0050-ignore-enforced-in-adapters.decision.md:129 and
src/core/usage-policy/matcher.js:403 (selectGoverning JSDoc), both at
db771296.

Both stated, unqualified, that an explicit sync/full carve-out declared under
one spelling does not loosen a broader restrictive entry declared under the
other. That is not what the code does. The two-pass guard compares the
declared pass against the widened one and keeps the more restrictive, so it
can only preserve a verdict the declared pass actually produced. When the broader
restrictive entry reaches the cwd only through canonicalization, the declared
pass matches nothing, there is no lexical verdict to preserve, and ordinary
nearest-governs decides among entries that are all in the canonical namespace.
The deeper carve-out then wins, and the loosening does cross spellings.

Measured, real symlink(2) fixtures, branch vs pristine master. Store
[{<root>/l1, ignore}, {<root>/l2, full}] with <root>/l1 -> <root>/real/r/p
and <root>/l2 -> <root>/real/r/p/q:

cwd master db771296
<root>/real/r/p/q/x full full (the full carve-out declared as l2 beats the ignore declared as l1)
<root>/real/r/p full ignore (the reach canonicalization correctly added)

Contrast the shape the guard does cover, which I re-verified holds: with
[{<root>/real/a, ignore}, {<root>/lnkB -> <root>/real/a/b, full}],
<root>/real/a/b/c/d resolves ignore on both sides. There the restrictive
entry matches by its declared spelling, so the declared pass has something to
preserve.

This is not a leak and not a regression. In the uncovered shape the lexical
matcher matched neither entry, so master returns full too, and the outcome is
identical to what the user would get by declaring both entries canonically. I
deliberately did not change the code: blocking this loosening would return
ignore for a directory the user explicitly carved out with hyp ignore --sync,
which is over-restriction and would break a deliberate exception. The defect is
in the claim, and the claim is what a reader of a privacy gate relies on.

Fix (27dc367): LLP 0050 §canonicalization now states the condition the
block depends on and why the uncovered shape is not a hole; selectGoverning's
JSDoc gains the same qualification; and the shape is pinned by a new test,
resolve: between two entries that both reach only by canonicalization, the deeper carve-out governs,
which also asserts the lexical baseline in-test (an injected identity
realpathSync, which reduces canonicalSpellings to the lexical form alone).

Verified: git diff db771296..HEAD shows all three edits. The new test has
teeth in the over-restricting direction: mutating deepestMatch so class rank
dominates depth (that is, "block every cross-spelling loosening") reddens it plus
the two existing nearest-governs tests, 3 failures against a 16-pass baseline.

Finding 2: issue #483's remediation steps target code that is not on master (medium, fixed)

#483 is correctly scoped as a defect and correctly labelled neutral:fix:
the aliasing really is reachable on master, whose matcher compares
path.resolved strings, so case and NFD/NFC spellings escape the gate there
today. I confirmed the labelling is right and neutral:stuck would be wrong.

But the issue's "What a fix needs" section is written against this PR's
branch
. src/core/usage-policy/canonical.js, canonicalSpellings, the
two-pass nearest-governs rule, and LLP 0050 §canonicalization do not exist on
master (verified by ls and grep at 1555f13), so steps 1, 3 and 4 have
nothing to edit there. Step 3 explicitly says "safe by construction after
#482
". A fix agent dispatched against master, as one is being now, either
finds nothing at those steps or rebuilds the candidate-spelling machinery from
scratch and collides with this PR.

Fix: added a "which base to fix against" note at the top of #483 recording
that the defect is on master but the remediation assumes #482, and recommending
the fix be sequenced after it. Label left as neutral:fix. Verified: the
note is the first block of the issue body after the neutral-followup marker.

Finding 3: the PR body carries claims that no longer match the head (low, LEFT for a human)

I am not editing the PR body. Four specific corrections a human should make
before merge, since the body is the merge-time record:

  1. "npm test: 2942 tests, 2932 pass, 8 fail." The head is now 2947 tests,
    2937 pass, 8 fail
    (round 1 and round 2 both added tests). The 8 are the
    pre-existing test/core/leave-command.test.js set; I re-confirmed them by
    name on a pristine master worktree, where that file is 3 pass / 8 fail.
  2. The cache-miss row of the performance table (23 us to 59 us) understates the
    cost.
    My measurement, three runs each, same harness both worktrees:
    master 24.00 / 23.96 / 24.69 us, branch 69.95 / 70.25 / 70.84 us. That is
    2.9x, not the 2.6x the table implies. Round 1 measured the same 2.9x on a
    slower host. Same order, so the conclusion stands, but the number should be
    corrected.
  3. The "What this does NOT fix" bullet "It does not make a nested loosening
    cross spellings" is false as written
    , for exactly the reason in finding 1.
    It needs the same qualification I applied to LLP 0050: the block holds when
    the broader restrictive entry matches by its own declared spelling.
  4. The body's design argument still presents the reasoning round 1 disproved.
    Under "What is canonicalized, and where" it argues that a failed realpath
    "can only remove a candidate spelling, never a verdict some other spelling
    already produced. Canonicalization can therefore only ever move the gate
    toward more restrictive, never toward full." That reasoning is insufficient
    on its own, which is what the round-1 regression demonstrated, and the body
    never mentions the two-pass selectGoverning guard that actually makes it
    true. As written the body claims an invariant the code needed extra work to
    deliver.

Nothing else in the body is inaccurate. In particular the mutation table is
now fully verified: 12 of 12
. Round 1 executed 9; I ran the remaining three
against the five usage-policy suites (100-pass baseline) and each was killed by
the test the table names: matchDepth losing per-entry specificity reddens
among nested v2 list entries the most specific (longest) dir wins (3 failures),
mark upsert identity reverting to string equality reddens
hyp ignore --private ... not duplicating it, and unmark membership going
lexical reddens hyp unignore --local-only by symlink spelling ....

Is the two-pass evaluation equivalent to most-restrictive-wins? Yes, where it matters, and structurally

I built 14 adversarial fixture shapes and ran each against both worktrees: three
entries at different depths across two spellings; a carve-out nested inside a
carve-out (four entries, mixed spellings); an entry whose declared and canonical
forms land at different depths; the same directory reached by two different
symlinks, in both class orders; a long declared name pointing at a shallow
target, so the two argmax candidates sit in different namespaces; a self-parent
link (<base>/x/y -> <base>/x, so an entry's declared spelling is a strict
lexical descendant of its own canonical form); .hypignore and list interacting
across spellings. In every one, the branch was equal to or more restrictive
than master. Not one demotion.

It is not luck. canonicalSpellings(key)[0] is always path.resolve(key), so
deepestMatch(cwd, scopes, 1) reproduces master's reduce exactly, same argmax
over dir.length, same class tie-break, and every other spelling only ever feeds
mostRestrictive. So resolve(cwd).class >= master(cwd).class holds by
construction, not by test coverage.

The one place two passes genuinely differ from a full lattice meet is finding 1's
shape, and there the meet would be more restrictive than master, not less.
So the gap is over-restriction the design deliberately declines, not a leak. Two
argmax comparisons that might have looked unsound are actually fine, and worth
recording so the next reader does not re-derive them: within one pass every
matched spelling is an ancestor of the same cwd string, so comparing their
lengths is a real nearest-ancestor ordering rather than a comparison across
namespaces; and matchDepth returning the first match instead of the longest is
safe because a canonical form can never be a strict lexical descendant of its
as-given form without a symlink loop, which realpath reports as ELOOP.

Does a same-spelling carve-out still work? Yes, in all three shapes

  • Declared and matched in the same spelling: {<root>/real: ignore} plus
    {<root>/real/proj: full} resolves <root>/real/proj/sub to full, identical
    to master.
  • Same with local-only as the outer class: still full.
  • The carve-out reached through a symlinked cwd: with <root>/link -> <root>/real,
    <root>/link/proj/sub also resolves full, so a user who carved out a
    subdirectory does not lose it just because the client reported the link
    spelling. master returns full [default] there, having matched nothing at
    all, so the carve-out is genuinely honored rather than accidentally agreed with.

No user who deliberately carved out a subdirectory silently loses it.

Do the CLI and the gate agree? Yes, zero disagreements

governingListEntry is what hyp ignore --check / policy show uses to name
the governing entry and to scope the residual row count, and matchList is what
the gate uses. I checked them against each other over 8 fixture shapes and 22
targets, five of which round 1 did not test (carve-out in a carve-out, both-
canonical-only, depth inversion, self-parent link, two links to one directory),
asserting both that the class the CLI would print matches the class the gate
resolved, and that the entry the CLI names actually governs the target. All
agree.
The unification round 1 did is real, not nominal.

Degraded paths: nine cases, all identical to master, nothing throws

Verified by execution against real fixtures, including ELOOP as asked:

case master branch
entry declared as a now-dangling link, the link path itself ignore ignore
the same, plus a child segment ignore ignore
ELOOP path listed as an ignore entry ignore ignore
ELOOP child, two segments deep ignore ignore
dangling link inside an ignored .hypignore tree ignore ignore
the same, plus a child ignore ignore
path that never existed full full
plain ignored directory ignore ignore
unrelated directory full full

canonicalizeDirSync reports resolved: 'partial' with the path unchanged
for the dangling (errno: enoent) and loop (errno: eloop) cases, so it loses
reach canonicalization would have added and demotes nothing. Nothing throws into
the gate on any branch. I also confirm round 1's note that resolved: 'none' is
effectively unreachable for an absolute path: even never/existed/at/all comes
back partial, so the warn escalation at canonical.js:133 is dead in
practice and every real failure logs at debug.

Performance: my own numbers

Three runs each, same harness in both worktrees, 500k steady-state calls and 20k
forced misses.

master branch
cache-hit resolve, the per-exchange path 320.8 / 311.2 / 311.0 ns 318.6 / 317.0 / 308.8 ns
cache-miss resolve, once per cwd per 5 s 24.00 / 23.96 / 24.69 us 69.95 / 70.25 / 70.84 us

Cache-hit is indistinguishable from master, inside run-to-run noise and
with the branch faster on one of the three runs. That confirms the memo is keyed
on the lexical path and consulted before canonicalization, so the hot path really
does pay nothing.

Is 2.9x on a cache miss acceptable? Yes. The miss is bounded at one per
distinct cwd per 5 s TTL, so the absolute cost is what matters, not the ratio:
100 distinct cwds churning every window is 7 ms per 5 s, about 0.14% of one core;
a pathological 1000 is 1.4%. Neither is close to mattering, and none of it lands
on the per-exchange path.

Can the memo grow without bound in a way that matters? It can grow without
bound. matcher.js:111 is a Map with no eviction and no size cap. Measured in
isolated processes with the resolver deliberately kept live so V8 cannot collect
it: 200k distinct cwds cost 34.8 MB on the branch and 34.6 MB on master,
a 0.2 MB difference, which is roughly 175 bytes per entry on both sides. So this
PR adds essentially nothing per entry; the memo stores the same key and the same
result shape it always did. My judgement: it matters only for a workload that
manufactures distinct cwds, since a developer machine sees tens to low
hundreds, and it is not this PR's to fix. Left as a decision below.

The 5 s TTL and split-brain: yes, and the window fails toward full

Asked directly, so I built it. With <root>/link -> <root>/real/proj and a
.hypignore at <root>/real (the shape where the lexical walk genuinely never
meets the rule, so only the branch resolves it), injected clock:

t=0    resolve(<root>/link/x)        -> full     (warms that key; no rule yet)
       .hypignore written at <root>/real
t=1000 resolve(<root>/real/proj/x)   -> ignore   (cold key, walks, finds it)
t=1000 resolve(<root>/link/x)        -> full     <-- same directory, disagrees
t=5001 resolve(<root>/link/x)        -> ignore   (key expired, converged)

So yes: for up to the TTL, one spelling of a directory can say full while the
other says ignore. The window fails toward full, that is toward capture,
and only for a spelling whose key was warmed before the rule existed. The bound
is the same 5 s master already had for any single cwd string, and two
spellings are simply two cache keys with independent expiry, which is the
situation two distinct cwds already had. What is new is only that the two keys
now ought to agree. Not fixable inside this PR: the CLI-to-daemon invalidation
signal that would collapse it to zero is the disclosed future work, and the body
already says the TTL is not narrowed.

Everything else re-verified at the head

Left for a human

  1. The four PR-body corrections in finding 3. I am not permitted to edit the
    body. Decision: apply them, or accept a merge record that misstates the
    test counts, the cache-miss cost, and the invariant.
  2. resolve's memo is unbounded (matcher.js:111). Measured at ~175 bytes
    per distinct cwd, ~35 MB per 200k, within 0.2 MB of master, so this PR
    does not make it worse and I left it rather than widen a privacy fix.
    Decision: whether to cap or sweep it (an LRU bound, or an opportunistic
    sweep of expired entries on insert) as separate work. Daemon OOM (4GB heap) on first central push after fleet enrollment; crash strands pending 'attach claude' action #280 already reports a
    daemon OOM; I did not investigate whether they are related and I am not
    claiming they are.
  3. A nested loosening does not cross spellings when the restrictive entry
    matched by its declared spelling
    (round 1's decision 2, now stated precisely
    and pinned in both directions). A user who writes hyp ignore --sync <symlink>
    inside a tree marked --private under the real spelling finds the directory
    still local-only, and has to re-declare the carve-out under the same
    spelling. hyp policy show reports the class actually in force, so it is
    diagnosable. Decision: accept this as the privacy-safe direction (my
    reading of LLP 0049 §fail-safe, and what the PR body wants), or decide an
    explicit user classification should win over a broader entry regardless of
    spelling, in which case LLP 0050 §canonicalization must be weakened again and
    the loosening documented as deliberate.
  4. The TTL split-brain above. Decision: accept the 5 s window (unchanged
    bound from master), or prioritise the CLI-to-daemon invalidation signal that
    would make a mark take effect immediately across every spelling at once.
  5. Unchanged and out of scope here: case D of Usage-policy matcher never canonicalizes: a symlinked spelling of an ignored directory escapes its .hypignore (all adapters, hyp purge, query visibility) #479 (declared-workspace-key
    provenance, Codex live projector: the workspace-key substitution can feed the .hypignore gate a directory the session never ran in #476 / Codex live projector: a workspaces turn-metadata header preempts the rollout session_meta.cwd, turning a correct .hypignore drop into a record #480 / PR Codex live projector: an explicit cwd outranks a substituted workspace key at the .hypignore gate #477's territory); already-recorded cwd values
    are not rewritten; nothing is done at a sink that already received a row;
    macOS case and Unicode aliasing is Usage-policy gate is escapable on macOS by path case or Unicode normalization: realpath does not fold either #483, now correctly sequenced behind this
    PR.

Round-2 outcome

The fix is sound and the round-1 remedy holds up under direct attack. I could not
construct a shape that leaks, and the reason is structural rather than
coincidental. What needed correcting this round was the gap between what the
documentation claims and what the guard delivers, plus a follow-up issue pointed
at the wrong base. Both are done and pushed to fix/issue-479 as 27dc367.

This was round 2 of 2. The only thing I am holding for a human that is
actionable before merge is the PR body (finding 3); the rest are accept-or-defer
decisions. Still a draft. I do not mark PRs ready and I do not merge.

@philcunliffe

Copy link
Copy Markdown
Contributor Author

Triage of the unreviewed round-2 commit (27dc367)

Round 2's fix commit 27dc367c (the current head) was never reviewed by the 2-round budget; this is that review, done at the triage rung (LLP 0017).

What it changes. Only llp/0050-ignore-enforced-in-adapters.decision.md, the JSDoc above selectGoverning in src/core/usage-policy/matcher.js, and one new pinning test in test/core/usage-policy-symlink.test.js. No behavioral change to matcher.js, verified the diff there touches only comments. Ran the suite in a clean worktree at head: test/core/usage-policy*.test.js is 84/84 green, including the new pinning test (resolve: between two entries that both reach only by canonicalization, the deeper carve-out governs). npm test overall: 2937 pass / 8 fail / 2947 total, matching the PR body's numbers exactly; the 8 failures are the pre-existing test/core/leave-command.test.js set, reproduced identically on a pristine origin/master worktree, unrelated to this change. test/core/llp-ref-hygiene.test.js: 8/8 (1 unrelated pre-existing skip), and I confirmed LLP 0050's #canonicalization anchor and the Extended-by notes in LLP 0049 #scope and LLP 0071 both resolve and describe the code accurately. Verdict on the commit itself: safe to ship as-is.

Carve-out vs lattice meet, my own judgement (asked not to inherit round 2's). I reproduced the shape directly against the resolver: two symlinked entries (outer:ignore, inner:full, inner nested under outer's target) where the cwd is queried by its plain real path, so neither entry matches by its declared spelling, the two-pass guard has nothing to compare against, and it falls through to ordinary nearest-governs on the widened set, where the deeper (less restrictive) carve-out wins. I agree with round 2: this is the right call, not a hole. On master today this exact shape already resolves to full (neither symlink spelling matches the plain real cwd lexically), so the branch does not regress below master here. The alternative, a true lattice meet (most-restrictive-always-wins regardless of depth), would be a materially bigger change: it would override ordinary nested-carve-out semantics everywhere in the matcher, not just in symlink-reached cases, to close one narrow shape that requires two independently-declared symlink spellings pointed at nested real subdirectories, accessed by neither declared spelling. Keep the current behavior.

TTL split-brain, confirmed and disclosed as a merge-time caveat. Reproduced directly against the resolver with an injected clock: cache one spelling of a directory, write a .hypignore that tightens it, then query a second, previously-uncached spelling of the same directory inside the 5s TTL window: the fresh spelling returns ignore immediately, the already-cached spelling still returns the pre-edit full until its own entry expires at exactly ttlMs (5000ms default). Direction confirmed: it fails toward full, i.e. the stale side stays permissive (keeps forwarding), never the other way. This is the same class of staleness master already accepts for a single spelling (documented in the CACHE_TTL_MS comment as the interim leak bound, with a CLI-to-daemon invalidation signal named as future work), and the PR body already discloses it under "What this does NOT fix." I judge it acceptable to ship given the bound is small, unchanged in kind from master, and already documented, but flagging it explicitly here since it is a real, if narrow, privacy exposure: for up to 5 seconds after a user tightens a .hypignore, an already-warm spelling of the same directory can keep forwarding.

Overlap with #484. The overlap note already posted there (neutral-fanin: ... overlap-with-482) is accurate and adequate: both PRs independently add a spelling-widening rule and a two-pass evaluation to the same matcher.js, the conflict is semantic once either merges, and neither should block on the other. I agree this should not block #482: the symlink leak this PR closes is real and independent of the case/Unicode leak #484 closes, and whoever resolves the merge conflict should unify the two spelling-widening passes into one, per that note's own recommendation.

Issue #483's base note. Present and accurate ("Which base to fix against, added by the round-2 review of #482"), correctly flags that canonical.js, canonicalSpellings, and LLP 0050 §canonicalization do not exist on master (verified at 1555f13) and recommends sequencing #483 after this PR.

PR body. Verified against the actual tree: test counts (2947/2937/8) match exactly; the cache-miss figures (24us master / 70us branch) match; the nested-loosening qualification (the carve-out-vs-meet point above) is present, accurate, and cites the correct, existing test name; the design section documents both the round-1 regression-and-fix and the round-2 two-pass-guard qualification accurately. One cosmetic nit: the "What this does NOT fix" bullet on nested loosening has a duplicated "That is deliberate" sentence, a leftover from an earlier edit; harmless, not a factual error, not worth a follow-up.

Residual. Nothing new needs deferring. #483 already tracks the one open gap (case/Unicode aliasing) this PR deliberately leaves for a follow-up, and its base note is correct.

@philcunliffe
philcunliffe marked this pull request as ready for review July 30, 2026 07:49
@philcunliffe philcunliffe added the neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) label Jul 30, 2026
@philcunliffe
philcunliffe merged commit aceca0a into master Jul 30, 2026
8 checks passed
@philcunliffe
philcunliffe deleted the fix/issue-479 branch July 30, 2026 18:18
philcunliffe pushed a commit that referenced this pull request Jul 30, 2026
#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>
philcunliffe pushed a commit that referenced this pull request Jul 30, 2026
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>
philcunliffe pushed a commit that referenced this pull request Jul 30, 2026
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>
philcunliffe added a commit that referenced this pull request Jul 31, 2026
… 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030)

Projects

None yet

2 participants