Skip to content

Codex workspace-cwd refusal is an ancestor test, not a byte test (#481, finding 2 of 2) - #491

Merged
philcunliffe merged 4 commits into
masterfrom
fix/issue-481
Jul 31, 2026
Merged

Codex workspace-cwd refusal is an ancestor test, not a byte test (#481, finding 2 of 2)#491
philcunliffe merged 4 commits into
masterfrom
fix/issue-481

Conversation

@philcunliffe

@philcunliffe philcunliffe commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Issue #481 carries two findings deferred from PR #477's round-2 review. This
PR fixes one of them. The other needs a human decision and is stated below.

Fixed: finding 2, the refusal warn fires on every turn of an ordinary subdirectory session

The predicate was "a workspaces key was substituted and it is not byte-equal
to the in-band cwd", so the commonest Codex shape there is - a session running
in a subdirectory of its declared workspace - emitted
plugin.codex.usage_policy_workspace_cwd_refused at warn on every turn, with
both directories clean and no .hypignore anywhere.

It is now an ancestor test.

Correction (round-1 review). This section originally justified the change
with a monotonicity claim: that an ancestor key means resolve(cwd) is always
"at least as restrictive" as resolve(key), so refusing "can only tighten".
That claim is false and was disproved by execution. The resolver takes the
nearest .hypignore (matcher.js:169-189) and the deepest list entry
(matcher.js:520-530), so a declaration sitting between the key and the cwd
overrides the key's and may loosen it. A sweep over one strict ancestor
chain at 4 depths, against the real createUsagePolicyResolver with real temp
trees, real .hypignore files and a real v2 list, found 131 of 576
arrangements where the cwd resolves less restrictively than the key
(hypignore 50, list 75, mixed 6). End to end through the real projector, on a
config that loosens ignore to local-only, master emits 1 refusal warn and
this head emits 0. The codebase already said as much: selectGoverning's own
comment (matcher.js:545-560) calls nearest-governs "not monotone".

Round 2 reproduced this independently: the two source-pure slices come out
identical (50 from .hypignore alone, 75 from the list alone, 256 cases each),
and in every violation the declaration governing the cwd sits strictly
below the key. The mixed slice is enumeration-dependent, so read 131 as "on
the order of a fifth of the space", not as a constant of the resolver.

The predicate was not changed, and the behaviour is still the right one, but
on different grounds: an ancestor key is a less specific name for the same
tree
, not a guess about where the session ran, so declining to substitute it
is not a refusal worth reporting.

Correction (round-2 review). Round 1 rested that disposition on a
reasoned claim - "in every one of the 131 loosening cases the loosening is
the user's own nested declaration" - and flagged it as the place a second
opinion was most worth having. Round 2 replaced it with an executed ground
and corrected the part that was too strong.

The executed ground: the warn being narrowed carries no usage class at all.
On origin/master the same plugin.codex.usage_policy_workspace_cwd_refused,
with an identical field set, fires on a subdirectory turn with no .hypignore
anywhere, on one where key and cwd both resolve ignore, and on the
loosening arrangement. All three were run through the real projector. Nobody
could ever have told a loosening from an ordinary subdirectory turn by this
signal, so narrowing it removes a constant, not privacy information. The
genuine-refusal test now asserts the warn carries no class, declared or
governed_by, so that stays true.

The correction. The ->full transitions really are the user's own: the
machine-local list has exactly two writers, both behind the explicit
hyp ignore / hyp unignore / hyp policy set verbs, and LLP 0071
§not-central forbids anything central, layered or org-pushed writing one. 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 be driven by a dependency's own committed file rather than by
the user. That is bounded (the walk only goes up, so a vendored file cannot
reach the parent project, and .hypignore cannot express full), it is
identical on master, and it is now disclosed in LLP 0160 §decision rather
than asserted away.

When the key is an ancestor of the in-band cwd, these two things do hold:

  • the cwd's .hypignore ancestor walk passes through the key and continues
    above it, so every governing file the key would have found, the cwd finds;
  • every machine-local list entry that governs the key also governs the cwd,
    because membership is equal-or-descendant.

What does not follow is that the resulting verdict is at least as restrictive,
because which of those governing declarations wins is decided by nearness,
not by union. Off the ancestor 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. Demoting to info was the other option in
the issue; it is rejected in LLP 0160 because it would quiet the interesting case
too, to fix a frequency problem that is really a predicate problem.

Shared predicate, reused, not copied. The test is isEqualOrDescendant from
src/core/usage-policy/matcher.js (LLP 0069 R8), exported through the
usage-policy index the projector already imports. It is deliberately the
lexical one rather than the spelling-aware scopeGoverns / canonicalSpellings
pair from #482/#484: those buy their reach with realpath syscalls, and this
runs once per exchange on the capture hot path that LLP 0049 R6 keeps free of
unbounded fs work. The residue is that this compares paths, not directories,
in both directions: a symlinked spelling of one tree reads as a refusal (a
spurious warn), and a lexical descendant that is really a symlink out of the
key's tree reads as covered, so it stays silent. Both were checked against
the real projector. (An earlier version of this paragraph claimed it "errs toward
reporting, never toward silence"; round 1 disproved the second half.) This is the
gap LLP 0083 filed as #479 for the gate, which #482/#484 have since closed
there; it is knowingly retained here, at this reporting-only predicate, for the
hot-path reason above. Nothing about the gate's own verdict depends on it.

Evidence

Three new tests in test/plugins/codex-exchange-projector.test.js, over
real temp directories and the real shared resolver
(createUsagePolicyResolver, no injected fs, no mock), matching the bar the
issue's own verification used.

Against pristine origin/master (73b4618) with only the test file applied:

not ok 43 - an ordinary subdirectory-of-workspace session logs no workspace-cwd refusal (#481)
  error: |-
    a subdirectory of the declared workspace is not a refused substitution
    + actual - expected
    + [ { fields: { component: 'codex', cwd_sha256: 'd78d5ca075e8bf66',
    +     error_kind: 'workspace_cwd_mismatch', exchange_id: 'ex-1',
    +     operation: 'usage_policy_workspace_cwd_refused',
    +     workspace_sha256: 'db131c2cc850695c' },
    +   message: 'plugin.codex.usage_policy_workspace_cwd_refused' } ]
    - []
ok 44 - a workspace key off the session cwd ancestry is still a refusal (#481)
# pass 59
# fail 1

On this branch:

# pass 61
# fail 0

Test 44 (the genuine refusals, both directions) passes on master too, on purpose:
it is the guard that the narrowing did not silence the signal it exists for. It
now also asserts the warn carries no class / declared / governed_by, which
is the executed premise the disposition rests on.

Test 45, added in review, pins the disclosed residue: an ancestor key that
resolves more restrictively than its cwd is silent, so the
non-monotonicity is a checked decision rather than an accident. Mutation-tested
in round 2: forcing workspaceCoversCwd to false kills it, and making the
.hypignore walk monotone (most-restrictive ancestor instead of nearest) kills
it too, so it is not vacuous in either direction.

PR #477's test "a refused workspace substitution still enriches the row from the
workspace key (#476)"
uses a clean, unrelated workspace key
(/work/clean/proj against a cwd of /work/clean/elsewhere, siblings), so it
is a genuine refusal under the new predicate too. It is untouched and still
passes, and its assertions are still about enrichment only.

NOT fixed: finding 1, ignored-workspace identity on newly-recorded rows

A human has to decide this one. It is a privacy-relevant default and the LLP
corpus does not settle it, so guessing would be worse than reporting.

The trade, crisply: suppress enrichment sourced from a workspaces key that
itself resolves to ignore - costing a second resolver lookup per exchange (the
per-cwd TTL cache absorbs it) and the graph-bridge identity of genuinely
multi-root sessions (LLP 0032 §capture) - or accept that a row whose cwd is
clean may name an opted-out tree's path, git remote, and head sha (identity only,
never content; the ignored tree's own sessions still drop).

What the corpus actually says, having read it rather than assumed it:

  • LLP 0049 §scope scopes the gate by cwd, which makes the current
    behaviour defensible but says nothing about enrichment sources.
  • LLP 0083 already writes the case down as a stated limit, and files it as
    Follow-up: deferred review findings from PR #477 (ignored-workspace identity on newly-recorded rows; refusal warn fires every turn) #481 - i.e. it recorded the behaviour and explicitly deferred the decision
    rather than taking it.
  • LLP 0032 §capture wants exactly this workspace/git identity for the graph
    bridge, which is what suppression costs.
  • LLP 0049 §fail-safe's "suppress more" rule is about an uninterpretable
    privacy signal; here the signal is perfectly interpretable and the exchange did
    not originate in the flagged tree, so it does not reach.

Nothing decides it. LLP 0160 §not-decided records the fork, and it is tracked as
its own issue #492 (labelled neutral:stuck, awaiting a suppress-or-accept
call), not under #481 - this PR's Fixes #481 trailer would otherwise retire
the question along with the issue. One point worth weighing that the issue does
not raise: a
cwd-scoped hyp purge --subtree of the ignored tree would not catch these
rows, so accepting also means accepting that the identity is not reachable by the
obvious cleanup verb.

LLP

Checks

  • npm test: 3031 pass / 8 fail. The 8 are test/core/leave-command.test.js,
    pre-existing: pristine origin/master (73b4618) fails the identical 8 by name
    (3028 pass / 8 fail), diffed as sorted name lists with zero difference. This
    branch adds exactly the 3 new passes.
  • npm run typecheck: clean.
  • No duplicate LLP number: 0160 is free on master, on every origin/* ref,
    and against the tombstones. Re-confirmed in review round 2: fix/issue-481 is
    the sole claimant, and the repo's own llp-check.yml duplicate-numbers command
    run verbatim on the tree returns empty.
  • Reproducing note: there is no lockfile, so npm ci fails. Link or copy
    node_modules into any fresh worktree first; without it the suite silently
    under-collects to ~1760 tests and reports ~147 spurious failures.

Held, not ready. Do not merge on my say-so.

Fixes #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>
@philcunliffe

Copy link
Copy Markdown
Contributor Author

Scope note: this PR fixes #481 finding 2 only. Finding 1 (enrichment from an ignored workspaces key placing an opted-out tree identity on a clean-cwd row) is a decision the LLP corpus does not settle, and is split out to #492 so Fixes #481 does not silently retire it.

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

Copy link
Copy Markdown
Contributor Author

Neutral review - round 1 of 2

Verdict: changes made, behaviour kept. The predicate change is right and I did not
touch it. Its justification is a stated proof that is false on this codebase's
resolver, and it was asserted in three places. I corrected the record, disclosed the
residue the false proof was hiding, and pinned it with a test. Two stale issue
references also fixed.

code-review is not installed on this host and codex is not on PATH, so I ran the
audit directly: a fixture sweep against the real resolver, an end-to-end run through
the real projector, three mutations, and the full suite plus typecheck on both this
head and a pristine origin/master worktree.

Everything labelled executed below was run. Everything labelled reasoned was not.


Finding 1 - HIGH - the load-bearing monotonicity claim is false (fixed)

The PR, LLP 0160 §decision, and the workspaceCoversCwd JSDoc all assert that when
the key is an ancestor of the in-band cwd:

So resolve(cwd) is at least as restrictive as resolve(key) would have been:
taking the cwd can only tighten. Nothing was lost, so nothing is reported.

This is not true. Both of its two premises are individually correct and jointly
insufficient, because the resolver does not take the maximum over everything the walk
finds - it takes the nearest, and the list takes the deepest:

  • src/core/usage-policy/matcher.js:169-189 (walk) returns at the first
    .hypignore found walking up. A less restrictive file between the key and the cwd
    wins outright.
  • src/core/usage-policy/matcher.js:520-530 (deepestMatch) picks the longest
    matching entry, ties broken by class. A nested full entry displaces a broader
    ignore one.

The codebase already knows this. selectGoverning's own doc comment
(src/core/usage-policy/matcher.js:545-560) says nearest-governs "is not monotone",
and names the visible cost "a nested loosening".

Sweep (executed)

createUsagePolicyResolver with no injected fs, real temp trees, real .hypignore
files and a real v2 machine-local list. One strict ancestor chain
above/key/mid/leaf, key a strict ancestor of cwd, sweeping .hypignore bodies
(absent/ignore/local-only/empty) and list classes
(absent/full/local-only/ignore) at four depths, plus a mixed sweep.

cases swept: 576
violations (resolve(cwd) LESS restrictive than resolve(key)): 131
by mechanism: { hypignore: 50, list: 75, mixed: 6 }
by transition: { ignore -> local-only: 81, local-only -> full: 25, ignore -> full: 25 }

Minimal cases:

VIOLATION key=ignore      cwd=local-only :: hypignore key=ignore      cwd=local-only
VIOLATION key=ignore      cwd=full       :: list      key=ignore      cwd=full
VIOLATION key=local-only  cwd=full       :: list      key=local-only  cwd=full

End-to-end through the real projector (executed)

workspaces = {<root>/private} with .hypignore ignore; session cwd
<root>/private/scratch with .hypignore local-only. Same fixture, both trees:

resolve(key) resolve(cwd) row refusal warns
origin/master 73b4618 ignore local-only recorded 1
this head fe46515 ignore local-only recorded 0

So there is a real, supported configuration where the substitution is refused, the
verdict genuinely loosens, and this PR makes it silent. That is exactly the case the
review brief set as a HIGH bar.

Disposition: justification replaced, behaviour kept

I did not change the predicate. On the evidence the behaviour is still right,
just not for the stated reason: in every one of the 131 violations the loosening is
the user's own nested declaration, which nearest-governs exists to honour, so the
cwd's verdict is the intended one and no restriction the user asked for is lost.

What was wrong was the reasoning, and what it concealed. Replaced with 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 there is no doubt about the location to report. The
signal now honestly answers "is the location inference in doubt", not "could the
verdict have differed", and LLP 0160 says so, with the sweep numbers and the pointer
that a real verdict-change detector would have to compare resolve(key) with
resolve(cwd) and is deliberately not taken.

Design note, recorded not acted on: if you do want the verdict-change signal, the
cost argument against it is weaker than the PR implies - resolve is TTL-cached per
cwd, and this PR's own §not-decided already concedes "the per-cwd TTL cache absorbs
it" for the second lookup in the other finding. That is the author's call, not mine.

Finding 2 - MEDIUM - #479 is closed, and the symlink residue is disclosed one-directionally (fixed)

LLP 0160 called the symlink residue "the status quo ... the same gap LLP 0083 already
files as #479". #479 is CLOSED - #482/#484 closed it for the gate. Citing it as a
live gap points a reader at a closed issue and hides that this PR knowingly re-opens
that gap locally, at this predicate.

The claim that it "errs toward reporting, never toward silence" is also incomplete.
Executed: with key/sub a symlink pointing out of the key's tree, the lexical test
reads cwd as covered and stays silent, on a cwd that is not really inside the
key. (I could not turn that into a restriction loss - the gate resolves over every
spelling and takes the most restrictive - so this is an accuracy defect in the claim,
not a second privacy hole.)

Rewritten to state both directions, to say #479 was closed for the gate and is
knowingly retained here for the hot-path reason, and to note the gate's own verdict
does not depend on it.

Finding 3 - LOW - #481 is closed by this PR but cited as where an open question lives (fixed)

LLP 0160 §not-decided ended "It stays open under #481", and LLP 0083's new
Extended-by said "leaves the enrichment question (#481) open" - while the PR body
ends Fixes #481. Finding 1 was split out to #492 (created 22:30:22Z, three
minutes after this head at 22:27:27Z), so the PR body and both LLP passages predate the
split and never mention #492. Repointed to #492 in both documents.


Checks that passed, and things I tried to break and could not


What I changed

Pushed 5bfc464 to fix/issue-481. No behaviour
change: the predicate, the gate, the row and the drop path are untouched.

Verified positively in the committed remote tree (not merely by a green suite):
git show origin/fix/issue-481:<file> reports 0 occurrences of "at least as
restrictive" and 0 of "can only tighten" in both the LLP and the projector; 1
occurrence of "131 of 576" in the LLP; 2 of "not monotone" in the projector; 1
of the new test. Remote head confirmed 5bfc464.

Still open for the author

The PR body itself still carries the false claim ("So resolve(cwd) is at least as
restrictive as resolve(key) would have been ... refusing the substitution can only
tighten, never loosen"), and still points finding 1 at #481 without mentioning #492. I
deliberately did not rewrite the narrative - please update those two passages so the
body matches the corrected LLP.

Otherwise this looks good, and the scope is stated honestly: it fixes finding 2, says
so, and the absence of finding 1 is a disclosed deferral, not a gap.

…s 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>
@philcunliffe

Copy link
Copy Markdown
Contributor Author

Neutral review, round 2 of 2 (next rung: triage)

Head reviewed: 5bfc4645ad27de057a43b606ce8e96071cd6e482. Fixes pushed as
1685288b84e44a99877705e17acf4033f2bb36af.

The code-review skill is not installed in this environment and codex is not
on PATH, so this is a direct audit: I read the delta, reconstructed round 1's
sweep from scratch against the real resolver, and mutation-tested the new tests.
Everything labelled executed was run. Everything labelled reasoned was not.


The headline: my independent verdict on the retained justification

Round 1 kept the predicate and replaced its justification, then flagged one step as
reasoned, not executed, and asked for a second opinion:

In every one of the 131 loosening cases the loosening is the user's own nested
declaration, which nearest-governs exists to honour.

Verdict: the disposition is right, the claim used to defend it is not quite true,
and it was never the claim that needed to be true.
No HIGH finding. In detail:

1. The structural half of the claim is true (executed)

I rebuilt the sweep independently: one strict ancestor chain above/key/mid/leaf,
key a strict ancestor of the cwd, real createUsagePolicyResolver with no
injected fs, real temp trees, real .hypignore files, real v2 machine-local list,
576 arrangements.

cases swept: 576
violations (resolve(cwd) LESS restrictive than resolve(key)): 125
by mechanism: { hypignore: 50, list: 75, mixed: 0 }
by transition: { 'ignore -> local-only': 75, 'local-only -> full': 25, 'ignore -> full': 25 }
governing declaration for the cwd lives at: { hypignore@leaf: 40, hypignore@mid: 10, list@leaf: 60, list@mid: 15 }
violations whose governing declaration sits STRICTLY BELOW the key: 125 / 125

The two source-pure slices reproduce round 1 exactly (50 and 75, 256 cases
each). My total differs only because the mixed slice is enumeration-dependent -
round 1 got 6 mixed, my construction gets 0. So 131 is not a constant of the
resolver, and the doc now says so.

The structural claim holds without exception: in all 125, the declaration that
governs the cwd sits strictly below the key.
It is always a nested declaration.

2. The provenance half is overstated (executed)

"Nested" is not the same as "the user's own". Split by source:

  • The list (75 cases, every ->full transition): round 1 is right. The
    machine-local store has exactly two writers in the tree,
    runMarkMachineLocal / runUnmarkMachineLocal
    (src/core/commands/clients.js:922, :1041), reachable only from
    hyp ignore / hyp unignore (clients.js:774,781,788) and
    hyp policy set|unset (src/core/commands/policy.js:232,308). full is
    written from exactly two places, both the explicit sync token. No config key,
    no central/org push, no join/attach/leave action, no daemon, no installer,
    no migration writes an entry, and LLP 0071 §not-central makes that doctrine. The
    v1→v2 migration only ever yields local-only. These really are the user's own
    deliberate answer.

  • .hypignore (50 cases, the ignore->local-only transition): round 1 is
    wrong.
    A .hypignore is a committable file by design -
    src/core/usage-policy/format.js:8 says so, LLP 0071 §not-dotfiles calls it "a
    repo-wide decision the whole team should see and that belongs in version
    control" - and walk (src/core/usage-policy/matcher.js:170-190) is a naive
    parent-chain loop with no node_modules skip, no vendor-dir skip, no
    repo-boundary stop. Executed:

    user declared at <root>/work                              => ignore
    session cwd     <root>/work/node_modules/some-pkg         => local-only
        (governed by <root>/work/node_modules/some-pkg/.hypignore)
    walk excludes node_modules / vendored trees?              NO
    

    The loosening declaration there belongs to the package author, not the user.

3. Why that is still not a HIGH finding (executed, and this is the real ground)

I went looking for the class the brief asks about - "a genuine privacy expectation
silently lost" - and it is not there, for a reason neither round stated: the warn
this PR removes carries no verdict information whatsoever.
Same three
arrangements, run through the real projector on both trees:

arrangement resolve(key) resolve(cwd) loosens origin/master warns this head
no .hypignore anywhere full full no 1 0
ignore at key and at cwd ignore ignore no 1 0
ignore at key, local-only at cwd ignore local-only yes 1 0

Field set on master, identical in all three:
["component","cwd_sha256","error_kind","exchange_id","operation","workspace_sha256"].
No class, no declared, no governed_by.

So on master nobody could ever have told a nested loosening from an ordinary
subdirectory turn by this signal. Narrowing it removes a constant, not privacy
information. And the underlying exposure - a vendored .hypignore local-only
downgrading a tree the user marked ignore - is a property of nearest-governs that
is byte-identical on master and on this head; the row records local-only either
way. It is also bounded twice: the walk only goes up, so a vendored file cannot
reach the parent project, and .hypignore cannot express full
(format.js:13 IMPLEMENTED), so a third party can downgrade ignore to
local-only but can never re-enable forwarding.

Conclusion: keep the predicate. Whether a third-party .hypignore should be
able to loosen a tree the user marked ignore is a real question, but it is a
resolver question for LLP 0049/0070, not a reporting one for this PR, and it is
not made worse here. I did not change the predicate.


Findings

Finding A - MEDIUM - the disproved monotonicity claim survived in three more places, one of them LLP 0160's own abstract (fixed)

Round 1 corrected llp/0160 §decision and the workspaceCoversCwd JSDoc. It
missed three others, which meant the document contradicted itself:

file:line text
llp/0160-...decision.md:13 (the summary blockquote) "An ancestor key cannot have changed the .hypignore verdict, so refusing it is not a refusal worth a warn."
llp/0083-...decision.md:144 (Extended-by) "...ancestor chain, because an ancestor key cannot have changed the verdict."
test/plugins/codex-exchange-projector.test.js:1321 (@ref LLP 0160#decision [tests]) "taking its own cwd over an ancestor key can only tighten the verdict"

The abstract is the worst of the three: 0160:13 asserted exactly what 0160:50
says four lines into §decision is not the justification. And the test comment is
a live @ref stating a disproved premise, which is precisely what CLAUDE.md's
"keep refs honest" rule exists to catch.

Fixed. All three now state the ground that holds and say explicitly that it is
not a monotonicity argument.

Finding B - MEDIUM - the PR body still carried round 1's disproved symlink claim, and its counts were stale (fixed)

Round 1 corrected the symlink residue in the LLP and the JSDoc but not in the body,
which still read:

The residue - a symlinked spelling of the same tree still reads as a refusal - is
the status quo and errs toward reporting, never toward silence.

Round 1 disproved that second direction by execution. Also stale: "Two new
tests" (there are three), "# pass 60 / # fail 0" (61/0), "npm test: 3030 pass
... adds exactly the 2 new passes" (3031, 3), and "LLP 0160 - the decision, the
ancestor-monotonicity argument" (that argument was removed).

Fixed via gh pr edit 491 --body-file. Verified against the live body: 0
occurrences of any of the five stale strings, round-2 correction block present.

Finding C - LOW - the justification's provenance claim, softened not removed (fixed; see the verdict above)

Recorded rather than acted on unilaterally where it touches design: the predicate
is unchanged. What I changed is the prose, so LLP 0160 no longer claims something
executably false. It now (a) leads with the executed ground (the warn has no class
field), (b) splits the provenance question by source with the writer inventory, and
(c) notes the 131 count's mixed slice is enumeration-dependent.

For triage / a human, recorded not acted on: a .hypignore committed into a
dependency can downgrade ignore to local-only for its own subtree, and the
ancestor walk has no vendored-tree exclusion. That predates this PR and is
unchanged by it, but it may deserve its own issue against LLP 0049/0070.


Audit of the round-1 delta fe46515..5bfc464

Round-2 scope items 3 and 4

Checks

tree npm test typecheck
pristine origin/master 73b4618 3028 pass / 8 fail clean
this head 5bfc464 3031 pass / 8 fail clean
after my fixes 1685288 3031 pass / 8 fail clean

The 8 failures are pre-existing, all in test/core/leave-command.test.js
(leave after join ..., leave clears an applied central slot ..., and six more).
Diffed as sorted name lists against pristine origin/master: zero difference.

test/plugins/codex-exchange-projector.test.js: 61 tests, 61 pass.
PR #477's test "a refused workspace substitution still enriches the row from the
workspace key (#476)"
passes (test 40) and still means what it did: its key
/work/clean/proj and cwd /work/clean/elsewhere are siblings, so a genuine
refusal under the new predicate too, and its assertions are about enrichment only.
#476 cases a/b/c (tests 37-39) also pass.

Style: no em dashes and no stray semicolons in the delta.

Reproducing note: there is no lockfile, so npm ci fails. Link node_modules
into any fresh worktree first; without it the suite under-collects to ~1760 tests
and reports ~147 spurious failures.

What I changed and how I verified it landed

Pushed 1685288 to fix/issue-481. No behaviour change: the predicate,
the gate, the row and the drop path are untouched.

  • llp/0160-...decision.md - abstract corrected; executed ground stated; provenance
    split by source with the writer inventory; the 131 count qualified.
  • llp/0083-...decision.md - Extended-by sentence corrected.
  • hypaware-core/plugins-workspace/codex/src/exchange-projector.js - JSDoc's
    "accepted because" clause regrounded on the executed fact.
  • test/plugins/codex-exchange-projector.test.js - stale @ref block corrected;
    three assertions added to test 44 pinning that the warn carries no verdict.

Verified positively against the committed remote tree (git show origin/fix/issue-481:<file>), not a green suite: remote head is 1685288;
"cannot have changed" / "can only tighten" / "at least as restrictive" appear 0
times in both LLPs and in the projector, and once in the test file only as my own
explicit negation ("NOT because taking the cwd can only tighten - it cannot only
tighten"); the new assertion, the executed-ground paragraph, the committable-file
disclosure and the corrected 0083 sentence are each present. PR body re-fetched
after edit: 0 of the five stale strings remain.


Round 2 of 2. The next rung is triage. Nothing here blocks: the predicate is
right, the disposition is now grounded on something executed rather than reasoned,
and the one place I disagreed with round 1 (the provenance claim) is a prose
correction plus a note for a human, not a design change. The open enrichment fork
(#492) still needs a human call and is correctly excluded from this PR's scope.

@philcunliffe

Copy link
Copy Markdown
Contributor Author

Correction to the round-2 review above. Its closing caveat - that 29 pre-existing @refs in the two touched files point at nonexistent LLP 0151#... / LLP 0035#... anchors, and that "someone should file it" - is wrong. Please disregard it; there is nothing to file.

Those anchors do exist. Both documents define them as inline <a id="..."> HTML tags rather than as Markdown headings or {#slug} markers, e.g. llp/0151-codex-lineage-from-body-client-metadata.decision.md:60 is <a id="body-is-authority"></a>**The body's flat client_metadata map is the ...**. All six anchors exchange-projector.js cites for LLP 0151 are present, as are all four it cites for LLP 0035.

Verified by running the repo's own gate rather than by grepping: node --test test/core/llp-ref-hygiene.test.js at origin/master (73b4618) passes 9/9, including "every @ref resolves to a live LLP document and one of its anchors", with 0 broken refs and 0 of the TOLERATED_BROKEN budget spent. That test scans every git-tracked file via git ls-files with no path exclusion, so hypaware-core/ is covered, and its documentAnchors() accepts all three anchor forms including <a id|name="...">.

The reviewer reached the wrong conclusion by applying the /ref-check skill's documented algorithm, which describes heading-slugification only. The one real (small) issue here is that .claude/skills/ref-check/SKILL.md under-documents the anchor forms the repo actually uses - it omits <a id>, so anyone following it literally gets a flood of false positives (15 of 30 in this file alone). That is a docs gap in the skill, not a defect in the code or the corpus, and it is not this PR's business.

Nothing in the round-2 review's substantive findings depends on this caveat; findings A, B and C stand.

@philcunliffe

Copy link
Copy Markdown
Contributor Author

Neutral triage (LLP 0017)

Verdict: A — every residual is non-blocking. Both review rounds are read. Nothing here blocks.

I re-verified the two things worth independently testing rather than accepting on the record, against the real code (real temp dirs, real .hypignore files, real createUsagePolicyResolver, no injected fs), on both origin/master (73b4618) and this head (1685288).

Round 2's ground for keeping the predicate — verified true

Round 2's claim: the plugin.codex.usage_policy_workspace_cwd_refused warn this PR narrows carries no verdict information — an identical field set fires on master whether or not the substitution actually loosened anything, so narrowing it removes a constant, not a privacy signal.

Independently reproduced end-to-end through the real projector:

tree: master
no .hypignore anywhere:            warnCount=1  fields=[component,cwd_sha256,error_kind,exchange_id,operation,workspace_sha256]
ignore at key AND at cwd:          warnCount=1  fields=[component,cwd_sha256,error_kind,exchange_id,operation,workspace_sha256]
ignore at key, local-only at cwd:  warnCount=1  fields=[component,cwd_sha256,error_kind,exchange_id,operation,workspace_sha256]  (this one loosens)

tree: this head (1685288)
all three cases: warnCount=0

Field set is byte-identical across all three arrangements on master, with no class, declared, or governed_by — matching the review's table exactly. Confirmed by reading the warn call site too (hypaware-core/plugins-workspace/codex/src/exchange-projector.js:141-149 on master): it logs component, operation, error_kind, workspace_sha256, cwd_sha256, exchange_id only, built from codexContext.refused_workspace_cwd and the in-band cwd, never from policy.class. So the loosening case was never distinguishable from an ordinary subdirectory turn by this signal on master. My independent verdict: the removed warn did not carry verdict information. Not a blocker.

Vendored-.hypignore exposure — confirmed pre-existing and byte-identical, not worsened

Reproduced live: a user marks /work ignore; a dependency ships a committed /work/node_modules/some-pkg/.hypignore of local-only; a session cwd inside that package resolves local-only. Identical on both trees:

{"userRootClass":"ignore","vendorSubtreeClass":"local-only","downgradedFromIgnore":true,"wouldDropAtGate":false}

on both origin/master and this head, byte-for-byte. Confirmed by diff, not just by execution: src/core/usage-policy/matcher.js has zero diff between origin/master and this head, and the drop-gate block in exchange-projector.js (resolver.resolve(cwd)return USAGE_POLICY_DROP, independent of any workspaces key) is also byte-identical. This PR touches only the refused_workspace_cwd warn's predicate and its comments; the resolver, the gate, and the drop path are untouched. Round 2's framing — pre-existing, bigger than this PR, an LLP 0049/0070 resolver question — is correct, confirmed by execution rather than trusted from the record.

Other residuals (non-blocking, filed for follow-up)

  • The provenance claim in LLP 0160 ("the loosening is generally the user's own nested declaration") is now accurately split by source rather than asserted as a blanket claim — a precision item, not a defect.
  • The retracted @ref-anchor caveat: confirmed retracted correctly. node --test test/core/llp-ref-hygiene.test.js passes 9/9 on this tree, 0 broken refs. The real (small) kernel of that caveat — .claude/skills/ref-check/SKILL.md under-documents the <a id="..."> anchor form this repo uses — is filed in the follow-up issue as a docs gap, not a code defect.

Checks re-run independently

  • npm test: this head 3031 pass / 8 fail; origin/master 3028 pass / 8 fail. The 8 failures are all test/core/leave-command.test.js, and a sorted-name diff against pristine origin/master is empty — pre-existing, confirmed by name, not assumed.
  • node --test test/core/llp-ref-hygiene.test.js: 9/9 pass on this head.
  • grep for the disproved monotonicity phrasing ("at least as restrictive", "can only tighten", "cannot have changed") across llp/0160*.md, llp/0083*.md, the projector, and the test file: the only hit is the test file's own explicit negation ("NOT because ... can only tighten - it cannot only tighten").

Follow-up issue filed: #496, covering the vendored-.hypignore resolver question (given its own clear statement there, as the most consequential thing this PR surfaced without fixing) and the smaller precision/docs items. #492 already tracks the separate deferred finding-1 decision from #481 and is correctly not duplicated.

Fixes/blocks: none. Proceeding to append the triage marker to the PR body.

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>
@philcunliffe philcunliffe removed the neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) label Jul 31, 2026
@philcunliffe

Copy link
Copy Markdown
Contributor Author

Neutral triage (LLP 0017), re-run at head d3ced55d

Verdict: A — every residual finding is non-blocking. This re-runs the triage call already made against 1685288b (see the triage comment above and the PR-body marker for that sha) because the head moved: d3ced55d is a merge of origin/master into fix/issue-481, not new review-loop work. gh pr diff 491 at the current head shows the identical 4-file change set (exchange-projector.js, llp/0083-...md, llp/0160-...md, test/plugins/codex-exchange-projector.test.js) that round 2 left in place — the merge pulled in unrelated master commits (PR #501, #495, #493, #462, #456) that happen to touch some of the same files, but git's three-way merge kept this PR's own hunks intact rather than altering them.

Re-verified directly against the actual current head, not just diffed:

  • node --test test/plugins/codex-exchange-projector.test.js on d3ced55d: 61/61 pass.
  • npm test on d3ced55d: 3081 pass / 8 fail. The 8 failures are all in test/core/leave-command.test.js, by name identical to the pre-existing failures both review rounds and the prior triage documented on origin/master — confirmed unrelated to this PR, not a new regression from the merge.

Findings status

Both review rounds' findings (round 1: the false monotonicity claim, the #479-closed/one-directional symlink claim, the stale #481 cite; round 2: three more places the disproved claim survived, a stale PR body, the overstated provenance claim) were all fixed by the author and independently confirmed fixed in round 2 and the prior triage. Nothing from either round is still open.

What remains is the residual, non-blocking material already catalogued:

None of the residuals are a production defect: no behavior change to the .hypignore gate or drop path, no data loss, no security hole, no crash, no perf regression. The code change here only narrows which cases emit a logging warn, is covered by 3 new tests (2 pinning both refusal directions, 1 pinning the disclosed non-monotonicity residue), and was mutation-tested in round 2.

Follow-up issue #496 ("Follow-up: deferred review findings from PR #491") already exists, is open, and correctly enumerates these residuals — reused rather than duplicated.

Proceeding to append the triage marker for d3ced55d to the PR body.

@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 31, 2026
@philcunliffe
philcunliffe merged commit 234e432 into master Jul 31, 2026
9 checks passed
@philcunliffe
philcunliffe deleted the fix/issue-481 branch July 31, 2026 04:50
philcunliffe pushed a commit that referenced this pull request Jul 31, 2026
Conflict: `usableInBandCwd`'s "what this does not reach" JSDoc paragraph in
`exchange-projector.js`, rewritten by both sides. Resolved by keeping both:
this branch's "it bounds the SHAPE, never the provenance; the ranking at the
call site is what keeps a guess from deciding" framing, master's "#476 closed,
a request that states a `cwd` no longer reaches here through the key" and its
`@ref LLP 0160#corrections-0083`. Master's closing clause "the key still
outranks the rollout fallback (#480)" is the thing this branch changes, so it
now reads as the ranking the demotion established.

#491's behaviour is untouched: `workspaceCoversCwd`/`isEqualOrDescendant` and
the narrowed warn-site comment merged clean and stand.

Two prose reconciliations where each side's text asserted the other's
pre-merge state:

- `llp/0083` #481 limit said widening the refusal to the rollout was blocked on
  a frequency objection "already open". #491 settled that objection with the
  ancestor test, so the limit now says widening is unblocked but must reuse
  0160's predicate rather than a byte comparison.
- `llp/0160` §corrections-0083 called #480 "the open ranking question". It is
  answered here, so it points at 0083 §workspace-key-ranks-last.

Co-Authored-By: Claude <noreply@anthropic.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

Development

Successfully merging this pull request may close these issues.

Follow-up: deferred review findings from PR #477 (ignored-workspace identity on newly-recorded rows; refusal warn fires every turn)

1 participant