Codex live projector: an unusable in-band cwd is a miss, not a path (#471) - #474
Conversation
…471) The in-band cwd (`codexContext.cwd` / `reqBody.cwd` / `metadata.cwd` / `metadata.user_id.cwd`, the LLP 0083 fast path) reached `resolver.resolve(cwd)` unpredicated. The matcher's first act is `path.resolve(cwd)`, so a relative value was measured against the DAEMON's process cwd: a confident `.hypignore` verdict for a directory the session never ran in, plus that bogus value stamped on the row as its container. `usableInBandCwd` now requires non-blank and absolute before the value reaches the gate, and logs a `plugin.codex.usage_policy_cwd_unusable` warn with an `error_kind` and a hashed cwd so the refusal is observable rather than silent. This does NOT convert the path to fail-closed: a refused cwd falls through to the rollout fallback, and when that states nothing too the row records `cwd = NULL` and is recorded, the existing precedent (LLP 0049 R1 as extended by LLP 0085). What changes is that an unconfirmable cwd yields an honest NULL instead of a verdict computed for the wrong directory. Co-Authored-By: Claude <noreply@anthropic.com>
… trim stays Review round 1 on #474. Two disclosure gaps in `usableInBandCwd`, both comment-only and both inside the new helper, so no extra conflict surface against the held PRs that also touch this file. - The PR reasoned that refuse-and-NULL strictly dominates accept-and-guess with "identical fail-open exposure for the should-be-ignored case". It does not. When the daemon's own process cwd sits under an ignoring `.hypignore`, the guessed base reached the correct verdict and this change now records where it previously dropped. Neither the daemon's launchd plist nor its systemd unit renders a working directory, so that cwd is `$HOME` for a `--user` unit and whatever shell started a foreground daemon. Refusing is still right (the same base produced false drops for every session that ran elsewhere), but the narrowing is real and belongs next to the code, not only in a PR body. - `cwd.trim().length > 0` gates nothing: a blank string is never absolute on either platform, so `isAbsolute` already refuses it. Dropping that conjunct leaves the whole suite green, which reads as dead code to the next person to simplify the predicate. It is load-bearing only for the `error_kind` split that tells `cwd_blank` from `cwd_not_absolute`. Say so. Co-Authored-By: Claude <noreply@anthropic.com>
Neutral review, round 1 of 2Verdict: findings. The fix is correct and the direction is right. Four findings, Reviewed head Verified by execution
(a) A row whose in-band cwd is refused and whose rollout fallback is empty is
So the PR's central disclosure ("recorded with (b) No real Codex cwd shape is wrongly refused. This is the failure mode I weighted
None of those is the real directory, so refusing them removes a wrong verdict rather (c) The duplicated predicate agrees with #466's // #466: src/core/codex/rollout_session_meta.js
function metaField(value) { return typeof value === 'string' && value.trim().length > 0 ? value : undefined }
export function sessionMetaCwd(value) { const cwd = metaField(value); return cwd !== undefined && path.isAbsolute(cwd) ? cwd : undefined }
// this PR: exchange-projector.js:930
if (cwd.trim().length > 0 && isAbsolute(cwd)) return cwdSame Findings1. The PR body argues "Refuse-and-NULL strictly dominates accept-and-guess: identical That configuration is not exotic. Neither daemon installer renders a working directory Refusing is still the right call, and I did not change the behaviour: the same guessed Decision left to a human: the PR body still contains the incorrect domination claim 2. The PR states "dropping the blank test (and its
So the blank conjunct is not load-bearing as a gate, and not because the test is Everything else in the test file is non-vacuous. All three new tests redden on a full 3. Two consequences. First, this is good news for finding (b): real I did not fix this: it lives at lines 653 and 773, outside the new helper, in exactly 4. Checked and clean
How the fixes were verifiedBoth fixes are documentation-only, so there is no test that reddens without them; I am |
Round-2 review found that LLP 0083's new bullet asserted "the rollout-stated cwd is held to the same rule", which is not true on this branch: rollout-cwd.js returns session_meta.cwd as written and the fallback at the gate applies no predicate, so a refused in-band value falls through to a source that is still unpredicated until #466 lands. Verified by execution (a stub rollout returning `sub` still drives a drop from a .hypignore under the daemon's cwd). The same bullet also read as though the in-band cwd is now predicated in general. On the Codex route the value the predicate sees is usually the workspace key selectCodexWorkspace picked, and that substitutes the first workspace when none matches, so an absolute-but-unrelated directory still reaches the gate. Verified by execution: workspaces {'/work/clean/proj': {}} with request cwd /work/ignored/real RECORDS the exchange and stamps /work/clean/proj, and the mirror case drops a session nothing covers. Tracked as #476, which is where the decision about row content belongs. Also state the one diagnostics gap in usableInBandCwd: a cwd of exactly '' never arrives, because readStringKey and firstString both require a non-empty string, so it is refused upstream with no log. Documentation only, all inside the bullet this PR added and the helper it added, so the reapplication cost against held #467 / #462 is unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
Neutral review, round 2 of 2 (final round)Verdict: findings. Reviewed head Round-2 fixes pushed to Round 1's finding 3, answered: yes,
|
workspaces map |
request cwd |
result | row cwd |
log |
|---|---|---|---|---|
{'/work/ignored/proj':{}} |
/work/ignored/proj |
DROP (correct) | n/a | info usage_policy_drop |
{'/work/ignored/proj':{}} |
/work/clean/real |
DROP | n/a | info usage_policy_drop |
{'/work/clean/proj':{}} |
/work/ignored/real |
RECORDED | /work/clean/proj |
none |
{'/work/ignored/proj':{}} |
sub |
DROP | n/a | no cwd_unusable warn at all |
Rows 2 and 3 are the two halves of #471 reached through a different door. Row 2 is a false
drop for a session no .hypignore covers. Row 3 is the failure the control exists to prevent:
a session whose real directory is ignored gets recorded, with a cwd from an unrelated tree
stamped on the row (LLP 0083's "One resolved cwd, used twice" means the substitution is both
gate input and row content). Row 4 shows the substitution runs ahead of this PR's
predicate, so the raw value never reaches usableInBandCwd and nothing is logged. Because
workspacePaths[0] is normally absolute, usableInBandCwd accepts it: this PR's predicate
cannot see the problem.
So it is the same hazard class as #471, and it survives this PR intact. Severity LOW to
MEDIUM on exposure, not on class: in the single-workspace common case the one key is the
session root, an ancestor of the real cwd, so the substitution is ancestor-conservative
(a deeper .hypignore under the cwd is missed, which is fail-open) rather than wrong-tree.
The wrong-tree verdicts need a multi-root session whose cwd sits under a root that is not
workspacePaths[0], or a normalization disagreement (pathsEqual only trims trailing
slashes, so symlinked spellings miss).
Not fixed here, deliberately, and this is the one decision left for a human. The fallback
is intentional and covered by an existing test ("Codex workspace selection prefers recorded
cwd over first metadata key"), and it is load-bearing for coverage: on the
ChatGPT-subscription route the request often carries no cwd, and workspacePaths[0] is then
the only in-band source of one, so deleting the substitution removes real .hypignore
coverage. Fixing it means deciding what codexContext.cwd means when the workspace map and
the request cwd disagree, which changes row content (cwd,
attributes.codex.workspace), not only the drop decision. That is the same reason #466
deferred #471. Filed as #476 (neutral:fix) with this evidence and three costed options.
It should land after #467 and #462 are resolved, since it is in the neighbourhood they rewrite.
Round 1's finding 4, answered: not a one-or-two-line change
Closing the cwd: '' diagnostics gap requires loosening a shared helper, not editing the new
one. Three layers strip '' before usableInBandCwd is reached: readStringKey
(:1057-1059, value.length > 0), the firstString inside readRecordedCwd (:884), and
the firstString at the call site (:122). readStringKey has 16 call sites in this
file. So making '' observable means changing shared-helper semantics plus the call site plus
a test, on the exact line held #467 / #462 must reapply, for a value no real client sends and
whose user-visible outcome is already identical (absent cwd, NULL column, and it was absent
before this PR too, so not a regression). Fixed the claim instead, which was the actual
defect: the "Never silently" comment in usableInBandCwd now records the '' gap and why it
stays. Disposition: accepted, documented in place.
Fix 2: LLP 0083's new bullet overclaimed (MEDIUM, fixed)
llp/0083-codex-live-cwd-from-rollout.decision.md, added bullet, last sentence: "The
rollout-stated cwd is held to the same rule." That is not true on this branch.
hypaware-core/plugins-workspace/codex/src/rollout-cwd.js:95 returns
stringValue(payload?.cwd) as written, and the ?? fallback at exchange-projector.js:123
sits outside the usableInBandCwd call, so nothing predicates it. Verified by execution:
with a rollout resolver returning sub and the governing .hypignore at
path.resolve('sub'), the exchange drops on a verdict computed against the daemon's cwd.
The rule holds only on #466's unmerged branch, which the helper docstring says correctly and
the LLP bullet did not.
This matters more than a wording nit because the two claims compose badly: the bullet read as
though the whole cwd seam is now predicated, while in reality this PR's refusal hands off to
a source that is not yet predicated, and (see above) the value it predicates is usually
workspace-mediated. Not a regression either way (a relative rollout cwd was already reachable
pre-PR whenever the in-band cwd was absent), but a reader would have been misled about the
current fail-open surface.
Fixed at 50919416: the bullet now names both limits, and usableInBandCwd's docstring
gained a matching paragraph so the code says what the doc says.
Verified by execution (not by reading)
-
npm testat50919416: 2882 tests, 2873 pass, 8 fail, 1 skipped. The 8 are exactly
test/core/leave-command.test.js(not ok 841-843, 845-849), the known pre-existing set,
identical to round 1's baseline onorigin/master. No new failures.npm run typecheck
clean before and after my push. Projector suite 40/40. -
Both predicate branches, mutated independently (round 1's result reproduces exactly):
mutation result drop isAbsolutenot ok 5,not ok 7, 38/40drop cwd.trim().length > 040/40 green drop the error_kindsplit onlynot ok 6, 39/40remove the warn entirely not ok 6,not ok 7, 38/40revert the call-site wrap at :122(pre-PR)not ok 5,not ok 6,not ok 7, 37/40So
isAbsoluteis the whole gate and the trim gates nothing: a whitespace-only string is
never absolute, soisAbsolutealone would be behaviourally identical. The trim is
load-bearing only for theerror_kindsplit, which is exactly what the round-1 comment at
:938-942now says, so the code is honest about it. The blank-cwd test is honest too: it
is named for the verdict ("computes no .hypignore verdict from a BLANK in-band cwd"), which
is true and is supplied byisAbsolute, and its one trim-dependent assertion is the
error_kindequality. Nothing in the test file claims the trim is a gate. The residual
inaccuracy is the PR body's mutation sentence ("dropping the blank test ... Neither
branch is decoration"), which round 1 already flagged and which I am also not permitted to
edit. -
No leak (priority 3). The raw path is never logged. The warn carries only
component,
operation,status,error_kind,cwd_sha256. The digest expression is byte-identical to
the adjacent drop log's:sha256Hex(cwd).slice(0, 16)at:135and at:953, so the two
correlate as the comment claims. Both leak mutations redden test 7: replacing the digest with
the raw value gives 39/40, and adding a rawcwdfield alongside the digest also gives
39/40.error_kindderives only a two-way classification. Nothing on the new path records
payload content, prompts or reasoning. Clean. -
Refusal does not change fallback ordering or lose the rollout's turn. An absolute clean
in-band cwd still short-circuits ahead of a wiredrolloutCwd(row recorded,cwd
/work/clean, rollout never consulted). A refused in-band cwd with a session id present
falls through and the rollout's ignored directory does drop (warn cwd_not_absolutethen
info usage_policy_drop). A refused in-band cwd with no rollout answer records the row with
nocwdkey at all (stripUndefined), confirming round 1's central check: no misleading
path is stamped. -
@refanchors (priority 5). The PR's two new refs are bothLLP 0083#decision; LLP 0083
exists, isStatus: Accepted, and has a## Decisionheading, so both resolve. Checked with
a slugifier that also honours this repo's explicit{#anchor}heading attributes: the PR
head andorigin/masterproduce the same repo-wide broken count over 1029/1031 refs, so
this PR adds zero broken anchors. My round-2 edits add no new@ref. -
CLAUDE.md (priority 6) clean on every added line, mine included: no semicolons, no U+2014
anywhere in the added diff, no@typedef, no inlineimport()types, JSDoc only, one import
(node:path). The two pre-existing em dashes at:120and:854that round 1 noted are
still pre-existing and still untouched, correctly, since fixing them would push the diff into
lines Codex lineage reads the durable body client_metadata, not header names Codex never sends (#464) #467 / Codex rollout cwd keys on the thread, not the session container (#459) #462 rewrite. -
Diff discipline held. Total against master: 3 files, +158/-1. One wrapped expression at
:122, one import, one appended self-contained helper, one LLP bullet, three tests. My
additions are comment lines inside that new helper and prose inside that new bullet, so they
add no conflict surface at all: those lines do not exist on either held branch, and One reader for the Codex session_meta header, in core (#465) #466
edits no existing LLP text.
Findings summary
| # | file:line | severity | disposition |
|---|---|---|---|
| 1 | exchange-projector.js:653, :773 |
LOW to MEDIUM | Filed as #476. Same hazard class as #471, confirmed by execution, survives this PR. Not fixable surgically: the fix changes row content and sits where #467 / #462 rewrite. |
| 2 | llp/0083-...decision.md, added bullet |
MEDIUM | Fixed at 50919416. The "rollout-stated cwd is held to the same rule" claim was false on this branch; both limits now named, and the helper docstring matches. |
| 3 | exchange-projector.js:122 with response-items.js:269-271 |
LOW | Accepted, documented in place at 50919416. Not a one-or-two-line change: three layers strip '', and the innermost has 16 other callers. Outcome already identical and not a regression. |
| 4 | PR body: "strictly dominates" and the mutation claim | LOW | Not fixed, cannot be. Both were corrected in code by round 1; the body still carries the round-1 wording and reviewers are instructed not to edit PR bodies. |
| 5 | rollout-cwd.js:95, unpredicated rollout cwd |
LOW | Not this PR's, already owned by #466. Documented now rather than left implicit. Not a regression: reachable pre-PR whenever the in-band cwd was absent. |
The decisions a human must make
- Codex live projector: the workspace-key substitution can feed the .hypignore gate a directory the session never ran in #476, the only substantive one. Pick option 1 (separate the workspace-enrichment role
from the gate/stamp role, which requiresresolveCodexContextto return them separately),
option 2 (prefer the request's owncwdon disagreement, one expression at:722, but it
changes the recordedcwdfor the cwd-is-a-subdir case and needs an LLP 0083 amendment plus
a test update), or option 3 (accept, and let the LLP note added here be the final answer).
Sequence it after Codex lineage reads the durable body client_metadata, not header names Codex never sends (#464) #467 and Codex rollout cwd keys on the thread, not the session container (#459) #462. - The PR body's two superseded sentences. Either soften "strictly dominates" and the
"neither branch is decoration" mutation claim to match the code comments, or accept the body
as round-1 reasoning that the merged code corrects. No code consequence either way. - Whether an unconfirmable cwd should fail closed, which this PR explicitly defers. The
evidence for taking that decision is now in the helper docstring (the case where the
daemon's own cwd sits under an ignoring.hypignoreand the old guess was accidentally
right). The existingcwd = NULLprecedent (LLP 0049 R1 / LLP 0085) supports what this PR
chose, so this is not a reason to hold the PR.
On merge readiness: nothing above blocks this PR on correctness. Findings 1 and 5 are
adjacent exposures this PR does not own and now no longer hides, findings 2 and 3 are fixed,
and finding 4 is cosmetic in a field I may not touch. This was round 2 of 2; the PR now
routes to triage, and #476 is the vehicle for the one open piece of work.
|
Neutral triage rung, assessing head Round 2's fix commit was never independently reviewed. Verdict: no true blocker. Safe to merge on review content. Everything below Two things the human should see explicitly before merging, because they are
Residuals, all non-blocking:
New, filed separately (not a blocker for this PR): master has moved by six Mergeability has also drifted underneath this PR during this triage: GitHub Ref hygiene ( |
…nes #466 aged `llp/0083-codex-live-cwd-from-rollout.decision.md` was the only conflict: this branch inserts an "an unusable in-band cwd is a miss" bullet into the Decision list, and master (#467) rewrote the adjacent "keyed on the codex session id" bullet to name `client_metadata.session_id` and LLP 0151. Both wanted; both kept. Two claims this branch made stopped being true when #466 landed, so they are corrected rather than carried across: - The bullet said the rollout-stated `cwd` is "not yet" held to the same rule and that PR #466 would close that half. #466 landed: `rollout-cwd.js` reads through `readRolloutSessionMeta`, which applies `sessionMetaCwd`, so a refused in-band value now falls through to an already-predicated source. Stated that way, and the "two limits" count drops to the one that remains (#476). - `usableInBandCwd`'s docstring pointed at "LLP 0143 #usable-cwd, PR #466" and said the core module "is not on `master` yet; unify them once it lands". That LLP is now 0150, and the module has landed. The duplication is kept with its real reason: LLP 0150 scopes the in-band path out of its own mandate, so borrowing its predicate here would widen 0150's stated scope, and the `error_kind` split needs the two conjuncts apart. The merged text now says explicitly that this bullet is not a consequence of LLP 0150, so nothing implies the in-band path is covered by 0150's rule. No code behavior changes in this merge: the projector, its helper, and the tests merged textually clean and are re-verified against what #466/#467 landed. Co-Authored-By: Claude <noreply@anthropic.com>
…ssues (#476 review round 2) Round 2 of the #477 review filed each residual as its own issue so it does not live only in this paragraph: #480 (the key preempts the rollout session_meta.cwd, pre-existing), #481 (a newly-recorded row carries an ignored workspace's identity), #479 (the shared matcher never canonicalizes, so a symlinked spelling of an ignored directory escapes its .hypignore). Also corrects the third limit. It was written as a trade this amendment makes; execution against a real on-disk symlink shows it is a property of the shared matcher that predates the amendment, that the amendment swaps which of two symmetric spellings trips it rather than opening a new leak, and that the widest case (a declared symlinked key with no in-band cwd at all) is identical before and after. Notes that the matcher fix must canonicalize the local-only list entries too, since canonicalizing only the incoming cwd un-governs an entry a user marked by its symlink spelling. Doc only: exchange-projector.js stays byte-identical, so the conflict surface against #462 and #474 is unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
|
Neutral triage for head The prior triage covered head The merge resolved one textual conflict ( 1. The "one limit" correction: TRUE. Before #466, LLP 0083 said the rollout-stated 2. The dangling 3. The "not a consequence of LLP 0150" claim: TRUE. LLP 0150's own Consequences section says it explicitly: "Still outstanding: the live projector's in-band cwd ... reaches the same 4. The #476 limit and the Residuals #476 and #478: both re-read against the current tree and still accurate; not duplicating them here. Confirmed they're mutually consistent: #478 (dedup ref-hygiene:
No further semantic drift found beyond the disclosed llp/0083 conflict and its accompanying docstring correction in The standing trade, re-judged independently, not carried over from the last triage: this PR still leaves a fail-open path. An unusable in-band I checked whether PR #482 or PR #477 change this. Neither does. #482 canonicalizes symlink spellings in the shared matcher, but its own body states it leaves My independent judgement: this remains a preference, not a blocker. It is disclosed at three levels (code docstring, LLP 0083, PR body), it is narrow (only fires when the daemon's own deployment cwd is itself under an ignoring rule and no rollout cwd is available), and the fail-closed alternative is explicitly named and deliberately deferred as a separate decision rather than silently dropped. I note, per the task brief, that an earlier round of this same PR asserted the fix "strictly dominates" the old behaviour, and review already disproved that; the current PR body no longer makes that claim and states the trade honestly. Flagging it here so it's in view at merge time, not to reopen it. Verdict: no true blocker in the unreviewed merge commit. Marking this PR's head as triaged. |
philcunliffe
added
the
neutral:approved
…e key at the .hypignore gate (#477) * Codex live projector: an explicit cwd outranks a substituted workspace key at the .hypignore gate (#476) `selectCodexWorkspace` falls back to the first `workspaces` turn-metadata key when none matches the request's `cwd`, and `resolveCodexContext` stamped that result as the one resolved `cwd` (LLP 0083) that feeds the `.hypignore` gate. So when the request's real cwd matched no declared workspace, the privacy verdict was computed for an unrelated directory: an opted-out session could be recorded (the leak), and a session no `.hypignore` covered could be dropped. An explicit in-band `cwd` now outranks the workspace key for the gate and the stamp. The key keeps its enrichment role and still supplies the `cwd` on the subscription route, where the request states none and the key is the only in-band source there is. A refused substitution is reported as `plugin.codex.usage_policy_workspace_cwd_refused` with hashed paths. `selectCodexWorkspace` itself is untouched to keep the conflict surface with PRs #462, #467 and #474 as small as possible. Co-Authored-By: Claude <noreply@anthropic.com> * LLP 0083: record the limits of the workspace-key refusal (#476 review) Review of PR #477 verified by execution that the fix closes #476 case (a) and several unreported variants of it, and found three consequences the amendment did not state: - the workspace key still outranks the rollout fallback, so a subscription-route session that declares a `workspaces` map never consults `session_meta.cwd` and a first-key guess can still decide its verdict (true on `master` too); - because the key keeps enriching, a row recorded where it used to drop (clean in-band cwd, ignored declared workspace) carries that ignored workspace's identity; - the gate does not canonicalize, so a symlinked in-band spelling of an ignored directory is now recorded where the key's canonical spelling used to drop it. Doc only: no code change, so the projector's contended lines are untouched. Co-Authored-By: Claude <noreply@anthropic.com> * LLP 0083: the stated limits of the workspace-key refusal cite their issues (#476 review round 2) Round 2 of the #477 review filed each residual as its own issue so it does not live only in this paragraph: #480 (the key preempts the rollout session_meta.cwd, pre-existing), #481 (a newly-recorded row carries an ignored workspace's identity), #479 (the shared matcher never canonicalizes, so a symlinked spelling of an ignored directory escapes its .hypignore). Also corrects the third limit. It was written as a trade this amendment makes; execution against a real on-disk symlink shows it is a property of the shared matcher that predates the amendment, that the amendment swaps which of two symmetric spellings trips it rather than opening a new leak, and that the widest case (a declared symlinked key with no in-band cwd at all) is identical before and after. Notes that the matcher fix must canonicalize the local-only list entries too, since canonicalizing only the incoming cwd un-governs an entry a user marked by its symlink spelling. Doc only: exchange-projector.js stays byte-identical, so the conflict surface against #462 and #474 is unchanged. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: neutral-reconciler <neutral@example.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: neutral-reconciler <neutral-reconciler@users.noreply.github.com>
A semantic merge, not a textual one: master reworked the same Codex cwd path under this branch's feet (#466 LLP 0150, #467 LLP 0151, #474, #477), so the conflicts had to be resolved on what the combined behaviour means, not on which side's hunk looked cleaner. Four files, and what each side wanted: - exchange-projector.js: master wrapped the in-band cwd in `usableInBandCwd` (#474) and added the refused-workspace warn (#477), both around the very expression this branch replaced. Kept both, with this branch's `resolveRolloutCwd` as the fallback rather than master's inline `rolloutCwd.resolve(session_id)`, which is the container key #459 is about. - rollout-cwd.js: master replaced the local first-line read with core's one `readRolloutSessionMeta` (LLP 0150); this branch added a thread-identity guard on top of that read. Composed rather than chosen: the guard now compares `meta.threadId` from the shared reader. The two fit exactly, because LLP 0150 rule 1 (raw JSONL line, never Codex's `Deserialize`) is the property the guard depends on to see an absent `payload.id` as absent. `meta.cwd` also arrives pre-predicated by `sessionMetaCwd`, so a blank or relative rollout cwd is now refused here too. Cache key stays the thread id. - LLP 0083: took master's Context correction and its unusable-in-band bullet, kept this branch's thread-keying thesis over master's superseded "keyed on the codex session id" bullet, and reconciled the prose that #467 falsified: the thread now comes from the body's `client_metadata`, not from `thread-id` / `session-id` header names Codex never emitted. The Consequences bullet promising a shared-reader follow-up was stale (that fold has landed) and now says so. - test/plugins/codex-rollout-cwd.test.js: git merged this file cleanly and the result was wrong in both directions, which is the part worth reading. Master's #257 fixtures key the fake resolver on the session id while stating a distinct thread id, so thread keying missed; rekeyed onto the thread id, which keeps master's deliberately-distinct pair. More seriously, this branch's #459 fixtures state identity through the bare `session-id` / `thread-id` / `parent-thread-id` headers, which #467 established are names no Codex version emits and removed the reads for. Left alone, four leak-direction tests failed outright and the refusal tests would have passed VACUOUSLY, for want of any id rather than because a refusal fired, silently gutting the gate. Ported the fixtures to the body `client_metadata` surface (LLP 0151), assertions unchanged. Checked, not assumed: - Regression gate still bites: master's two source files under this merged test file fail 11 of 23, including every #459 leak-direction case and all four refusal cases, so the ported fixtures are not vacuous. - `npm test`: 3039 pass / 8 fail, exactly the `leave-command` 8 that fail identically on a pristine `origin/master` worktree (73b4618), by name. - `npm run typecheck`: clean. No em dashes, no semicolons in changed lines. The LLP anchors cited (0150#usable-cwd, 0151#body-is-authority, 0083#container-fallback-gap) all resolve. Not touched, deliberately: the open `subagent_signal` finding at `resolveRolloutCwd`. The refusal is still value-blind and its shape is unchanged, but #467 narrowed its reachability, since a turn now has to carry neither a Codex-owned `client_metadata` map nor a turn-metadata blob to reach the container fallback at all. LLP 0083 records that narrowing without pretending it closes the question. Co-Authored-By: Claude <noreply@anthropic.com>
…st LLP 0083 provenance Two review findings on PR #513, both in the round's own scope. R1 (test hygiene): the new equivalence test was the only case in `codex-exchange-projector.test.js` that built a projector with no injected resolver, so the shared matcher walked the REAL ancestors of `/work/repo` looking for a `.hypignore`. A file above the checkout on the machine running the suite would turn `projection.cwd` into a `USAGE_POLICY_DROP` sentinel and redden an assertion that has nothing to do with the gate. Inject a resolver whose fs holds no list anywhere, matching how every neighbouring case in the file works, and hoist the projector out of the loop. Re-checked the mutation after the change: loosening `usableInBandCwd` back to `cwd.trim().length > 0` still reddens the test on the "repo" case, so the hermetic resolver did not make it vacuous. R2 (doc accuracy): the LLP 0083 bullet claimed the local copy was written "while 0150 was still unmerged". It was not. LLP 0150 / `sessionMetaCwd` merged in #466 on 2026-07-29; the copy landed in #474 on 2026-07-30, and that commit's own docstring cited LLP 0150 `#usable-cwd` by anchor while explicitly declining to borrow it, on the scoping argument the bullet already states. Attributing the copy to timing rewrites the recorded rationale. Say what actually changed instead: the weight given to drift, not the scoping. Co-Authored-By: Claude <noreply@anthropic.com>
) * Codex live projector reads the cwd predicate from core, not a copy (#478) `usableInBandCwd` inlined its own `trim() && isAbsolute()` copy of the rule core exports as `sessionMetaCwd`. PR #474 added it deliberately and temporarily, while LLP 0150 was unmerged; #466 landed that predicate, so the copy now has an owner to defer to. Behaviour is unchanged. Only a non-empty string reaches the in-band seam (`readStringKey` and `firstString` refuse the rest upstream), and over that whole domain the two copies already agreed byte for byte. What changes is that there is one place left for the rule to drift from, which is the whole reason LLP 0150 exists: this exact rule, stated twice, shipped the wrong answer twice (#453, #459). The wrapper survives only for the refusal diagnosis: `error_kind` needs blank told apart from relative, and the shared predicate's single `undefined` cannot carry that. It now decides nothing. Docs: LLP 0083's bullet said the checks are restated locally and that 0150's predicate is not borrowed; LLP 0150's consequences listed the in-band path as "Still outstanding ... no such predicate", which #474 already falsified. Both now describe the shared predicate, and 0150 keeps its scoping caveat: sharing the rule was LLP 0083's call, not an invariant 0150 imposed. Co-Authored-By: Claude <noreply@anthropic.com> * Review round (521cf88): hermetic resolver for the new seam test, honest LLP 0083 provenance Two review findings on PR #513, both in the round's own scope. R1 (test hygiene): the new equivalence test was the only case in `codex-exchange-projector.test.js` that built a projector with no injected resolver, so the shared matcher walked the REAL ancestors of `/work/repo` looking for a `.hypignore`. A file above the checkout on the machine running the suite would turn `projection.cwd` into a `USAGE_POLICY_DROP` sentinel and redden an assertion that has nothing to do with the gate. Inject a resolver whose fs holds no list anywhere, matching how every neighbouring case in the file works, and hoist the projector out of the loop. Re-checked the mutation after the change: loosening `usableInBandCwd` back to `cwd.trim().length > 0` still reddens the test on the "repo" case, so the hermetic resolver did not make it vacuous. R2 (doc accuracy): the LLP 0083 bullet claimed the local copy was written "while 0150 was still unmerged". It was not. LLP 0150 / `sessionMetaCwd` merged in #466 on 2026-07-29; the copy landed in #474 on 2026-07-30, and that commit's own docstring cited LLP 0150 `#usable-cwd` by anchor while explicitly declining to borrow it, on the scoping argument the bullet already states. Attributing the copy to timing rewrites the recorded rationale. Say what actually changed instead: the weight given to drift, not the scoping. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: test <test@example.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: test <test@test.com>
1 participant
Root cause
hypaware-core/plugins-workspace/codex/src/exchange-projector.js:120fed thein-band cwd (
codexContext.cwd, elsereqBody.cwd/metadata.cwd/metadata.user_id.cwd, the LLP 0083fast path) straight into
resolver.resolve(cwd).firstStringonly asks "isthis a non-empty string", and the usage-policy matcher's first act is
path.resolve(cwd)(src/core/usage-policy/matcher.js:109). So a relativevalue was silently measured against the daemon's process cwd, and the gate
returned a confident
.hypignoreverdict for a directory the session never ranin. The same value is also handed to
resolveRecordedContext, so it was stampedon the row as that session's container.
Accepting it was wrong in both directions at once: it can drop a session no
.hypignorecovers, and (the failure the control exists to prevent) it canrecord a session whose real directory is ignored, because the verdict was
computed elsewhere entirely. A whitespace-only
cwdhad the same shape.Direction chosen, and why
(a) refuse: an unusable in-band cwd counts as absent.
usableInBandCwdrequires non-blank and
path.isAbsolutebefore the value reaches the gate.This is explicitly not a conversion to fail-closed. Refusing makes
cwdfalsy, the caller's
if (cwd)skips the check, the rollout fallback still getsits turn, and when that states nothing either the row is recorded with
cwd = NULL(verified atexchange-projector.js:122andbackfill.js:241; thealready-modeled state of LLP 0049 R1 as extended by LLP 0085). What changes is
only that an unconfirmable cwd produces an honest NULL instead of a verdict for
the wrong directory plus a bogus stamped container. Refuse-and-NULL is better than accept-and-guess in
every case but one: it removes the false-drop mode and the poisoned folder/git
attribution, and for a should-be-ignored session whose real directory cannot be
confirmed the exposure is the same fail-open either way. The exception, found in
review and recorded in the helper docstring: when the daemon own process cwd
happens to sit under an ignoring
.hypignore, the old guess reached the rightverdict by accident and this change records instead. No installer renders a
working directory, so that cwd is
$HOMEfor a systemd--userunit or theshell directory for a foreground start. That is a real, narrow loss of coverage
rather than a wrong verdict, and it is the concrete evidence for the fail-closed
decision this PR defers. Whether an unusable cwd should instead fail closed is
a separate, larger decision and is not taken here: the existing
cwd = NULLprecedent says recorded.
One deliberate consequence: a refused in-band cwd now falls through to the
rollout lookup, which is the better-trusted source for the same field. LLP 0083's
"in-band stays the fast path" bullet is amended to say so.
Duplicate predicate: stated locally, on purpose
sessionMetaCwd(the same non-blank-and-absolute rule,LLP 0143
#usable-cwd) lives onPR #466's unmerged branch, so this cannot import it without depending on a held
PR. The two checks (
cwd.trim().length > 0 && isAbsolute(cwd)) are inlined in alocal
usableInBandCwdhelper whose docstring namessessionMetaCwdand says tounify them once #466 lands. Duplicating two boolean checks was preferred over
either blocking this fix on #466 or moving a shared predicate into core in the
same change that #466 is already moving code around in.
Observability
An unusable cwd does not vanish silently.
usableInBandCwdemitsplugin.codex.usage_policy_cwd_unusableat warn withcomponent/operation/status: 'refused', anerror_kindofcwd_blankorcwd_not_absolute(theerror_kindidiom frombackfill.js), andcwd_sha256- the same 16-hex digest the adjacentusage_policy_droplog uses,so the two correlate and the raw path is never logged.
Reproducing tests
All three in
test/plugins/codex-exchange-projector.test.js, against the realshared matcher via the file's existing
ignoringResolverhelper:project() computes no .hypignore verdict from a RELATIVE in-band cwd- theonly governing
.hypignoresits atpath.resolve('sub'), i.e. exactly themistaken base, and the exchange sends
cwd: 'sub'. Asserts noUSAGE_POLICY_DROPand no stampedcwd. Before:not ok(
operator: 'notStrictEqual', both sidesusagePolicyDrop: true- the wrongverdict really did fire). After:
ok.project() computes no .hypignore verdict from a BLANK in-band cwd-cwd: ' '. Asserts no drop, no blank path stamped on the row, anderror_kind: 'cwd_blank'. Before:not ok. After:ok.project() logs an unusable in-band cwd rather than skipping the gate silently-cwd: '../elsewhere'; asserts the warn's fields and that the rawpath is hashed, never logged. Before:
not ok. After:ok.Mutation-checked: dropping
isAbsolutereddens tests 1 and 3; dropping the blanktest (and its
error_kindsplit) reddens test 2. Neither branch is decoration.Verification
npm test: 2882 tests, 2873 pass, 8 fail, 1 skipped. The 8 are exactlytest/core/leave-command.test.js, confirmed identical on a pristineorigin/masterworktree with the samenode_modules. No new failures,+3 tests.
npm run typecheckandnpm run build:types: clean.gateway_codex_captureandsession_optout_capture_drop: ok.added-lines diff), no
@typedef, no inlineimport()types, JSDoc only. Thesingle
@ref(LLP 0083#decision [implements]) carries a relation and a glossand points at a section that exists on
master.Overlap with held PRs
exchange-projector.jsis also modified by held #467 (neutral:approved)and #462 (
neutral:stuck), so this will conflict there. The diff isdeliberately minimal for that reason: one changed line at the cwd seam
(wrapping the existing
firstString(...)inusableInBandCwd(..., ctx)), onenew import line, and a new self-contained helper appended next to
readRecordedCwd. No refactor, no reformat, no rename, no import reordering.Resolving against either held PR should be a one-line reapplication at line 120.
The LLP edit is a 7-line bullet inserted into LLP 0083's Decision list; #466
edits no existing LLP text, so there is no doc conflict with it.
Fixes #471