Uh oh!
There was an error while loading. Please reload this page.
hyp session ignore names the session container Codex drops on, not a thread id (#453) - #458
Conversation
…#453) `hyp session ignore` could report success while recording continued. The verb resolved a Codex THREAD id (the rollout's `session_meta.payload.id`), while the adapter drop keys on the session CONTAINER (`exchange-projector.js`: `session_id = metadata.session_id ?? conversation_id`). A root thread takes `session_id = SessionId::from(thread_id)`, so the two coincide and nothing looks wrong; a subagent thread inherits the root's session id and mints its own thread id, so an opt-out taken from inside a subagent tool call named an id the gateway never matches, and the verb printed "the gateway will drop this session". - `resolveSessionIdForCli` now resolves the container. `CODEX_THREAD_ID`, when set, is used as a SELECTOR rather than an answer: it names the live thread (no mtime liveness proxy), the rollout recording that thread is looked up, and the container is read out of it. The cwd + staleness path is unchanged except that it too reports the container. The thread id is carried alongside for provenance and display (`thread_id` in `--json`, a whole-session grain note in the human form), so the output stays honest about which value came from where. - The rollout's raw JSONL line is what is parsed. Codex's `SessionMetaLine` has a hand-written `Deserialize` that back-fills `session_id` from `id`, so a struct-shaped read hands back the thread id under the container's name on any pre-field rollout: the same defect, reintroduced invisibly through its own fix. An absent field is therefore visible as absent and REFUSES (fail-closed, LLP 0066 R10/R13) instead of substituting the thread. - Two clients each stating a session (nested Codex/Claude) refuses; a stated thread with no readable rollout refuses rather than falling back to the cwd scan, which would answer about a thread nothing tied to this invocation. - `codex/src/backfill.js` agrees about the partition key: `session_id` from `session_meta.session_id` (thread as the fallback for a pre-field rollout), the thread in `conversation_id`, mirroring the live projector. Row identity is untouched: the fallback-hash scope is `conversation_id ?? session_id`. - The Codex `hypaware-privacy` skill body read `payload.id` for the same POST; it now reads `payload.session_id`, stops on a rollout that records none, and points at `hyp session ignore --json` first. LLP 0066 gains R13 (name the container or refuse) and a corrected §scope: the Codex over-drop is live, not latent, and the mirror-image under-drop is the worse direction. LLP 0067 gains §cli-drop-key (the liveness-vs-correct-key trade-off), §cli-legacy-rollout (the back-fill trap) and §backfill-partition-key. LLP 0030's consequences record that backfill reads the container. Tests exercise the failure path: a subagent-shaped rollout resolves the container on both paths and that id makes the real projector return USAGE_POLICY_DROP for a subagent turn while the thread id does not; a legacy rollout refuses on both paths and `hyp session ignore` on it prints nothing that reads as done, exits unknown, and adds nothing to the set. Fixes#453 Co-Authored-By: Claude <noreply@anthropic.com>
Review of #458 found two ways the raw-line read could still hand back a confident key the gateway can never match, plus one new refusal with no test. - A blank (or non-string) `session_meta.payload.session_id` resolved `ok: true` with that value as the drop key: `readRolloutMeta` tested `length > 0` while `statedEnv` in the same file tests `trim()`. `hyp session ignore` would print "ignored" for a key no row is ever stamped with, which is the silent no-op #453 exists to remove, reached by another route. It now refuses like the legacy case, value still passed on byte-identical (R5). - The envelope `type` was never checked, so a first record that is not the `session_meta` header resolved whenever it happened to carry `id` and `cwd`. Only the header states the container; `codex/src/rollout-cwd.js` type-checks the same line for the same reason. - The refusal for rollouts that disagree about which session contains a stated thread had no test: mutating it away left the suite green. It has one now, with the agreeing case beside it so the test is about disagreement. LLP 0066 R13 and LLP 0067 §cli-legacy-rollout record that present-but-unusable refuses too, and the 0067 test plan lists the three cases.
philcunliffe
commented
Jul 29, 2026
VerdictThe design is right and the central claim holds. Reviewed at Facts verified against Codex source (not taken on trust)
1. Refusal boundary, path by pathEvery case below was run against the real
2. The back-fill defence works
3. The drop fires, and does not over-dropAgainst the real
Over-drop is structurally bounded: the drop compares one string to the stamped 4. Backfill partition change does not move row identityRan the real provider + real
5. Test teeth
FindingsA. Medium - a blank |
…as the CLI Round-2 review of #458. Round 1 closed two holes in `readRolloutMeta` (the first record must be the `session_meta` header; a present-but-blank `session_id` counts as absent) and wrote both into LLP 0066 as a MUST. The `hypaware-privacy` fallback script reads the same line in the same PR and did neither: it took `r["payload"]` whatever the record type was, so a first line typed `turn_context` carrying `session_id` would have been POSTed as the container, and `if not sid` accepts `" "` and any truthy non-string. It also could not carry an id containing whitespace at all: `read -r SESSION_ID ROLLOUT_CWD` splits on it, so such an id would have been POSTed truncated - a key the gateway never matches, reported as an opt-out. It now refuses and points at `hyp session ignore --json`, which keeps the token verbatim. Also correct the `resolveFromStatedThread` docblock. "An unread file cannot invalidate a hit" holds for FINDING the thread (an identity test) but not for the agreement check below it, which does read on the listing: on a truncated scan a disagreeing rollout may never be reached and a lone match is taken as agreement. Measured at maxScan=1 over two rollouts stating one `payload.id` under two containers: the resolver answers confidently. Accepted as a known limit (recorded in LLP 0067) rather than closed, because refusing on truncation would disable auto-resolution for any history past the bound and the trigger takes hand-copied history: a fork copies the parent `session_meta` as a LATER line, and only line 1 is read. LLP 0067 also now records why the stated-thread path carries no staleness bound, which was reviewed and accepted but undocumented.
philcunliffe
commented
Jul 29, 2026
Neutral review, round 2 of 2 (FINAL) - reviewed |
| claim | result at a308fe1 |
|---|---|
| stated-thread path yields the container for a subagent thread | session-root, source: codex_env_rollout, thread_id: thread-sub |
| real projector drops on that key | USAGE_POLICY_DROP from codex/src/exchange-projector.js |
| real projector records on the thread id (the old no-op) | not dropped, rows produced, session_id: session-root / conversation_id: thread-sub |
| the container key also drops the root and a sibling subagent turn | both USAGE_POLICY_DROP (the documented over-drop) |
| no over-drop: a different container sharing the thread id | records, session_id: session-other |
| no over-drop: two containers sharing one cwd | refuses, never yields a key |
legacy rollout (no session_id) | refuses on both paths, never offers the thread id as the answer |
| stated thread with no rollout | refuses, and does not leak the cwd-scan answer |
Row identity vs origin/master (f9b9667): ran the real backfill provider through aiGatewayBackfillMaterializer on subagent / root / legacy rollout shapes and diffed. All 12 part_id / message_id / native_id values byte-identical. The only diffs anywhere in the output are the intended session_id (thread-subagent to session-root) and the new thread_id codex attribute; the root and legacy shapes are identical throughout. This holds because the fallback-hash scope is conversation_id ?? session_id (LLP 0030 decision 3) and conversation_id still carries the thread.
4. New findings (both fixed in bcb464a)
E (Medium) - the hypaware-privacy fallback script did not apply the two guards this PR's own LLP now makes a MUST
codex/skills/hypaware-privacy/SKILL.md is the other implementation of the same read, and this PR edits that exact python block. After round 1 it had diverged from the resolver on both of the holes round 1 had just closed, while llp/0066 (added in a308fe1) says a field "carried on a record that does not state the container ... MUST refuse the same way":
- no
typecheck:p = r.get("payload", {})ran whatever the record type was, so a first line typedturn_contextcarryingsession_idwould have been POSTed as the container - round 1's finding B, in the file the same PR touches; if not sid:accepts" "(truthy in python) and any truthy non-string, e.g.12345posted as"12345"- round 1's finding A.
Fixed, mirroring readRolloutMeta shape for shape. One extra guard while there: read -r SESSION_ID ROLLOUT_CWD splits on whitespace, so an id containing any whitespace would have been POSTed truncated - a key the gateway never matches, printed as an opt-out, which is precisely the #453 failure class. It now refuses and points at hyp session ignore --json, which keeps the token verbatim. Verified by porting the guard faithfully (this box has no python3) and running every shape:
PRINT "s-ok" "/c" <- header, good id
EXIT not-session_meta <- turn_context first line
EXIT no-usable-session_id <- legacy / blank / numeric / null
EXIT whitespace-in-id <- padded id the shell would have truncated
EXIT no-payload <- header with no payload
PRINT "s-ok" "/c d" <- cwd containing spaces still fine
Also checked the block stays safe inside python3 -c '...' (no single quote, no backslash) and that a sys.exit fails the script the same way the pre-existing guard already did (read sees EOF, returns nonzero, set -e exits).
F (Low) - resolveFromStatedThread's docblock claimed an immunity the agreement check does not have
The comment read "an unread file cannot invalidate a hit". True for finding the thread, which is an identity test. Not true for the distinct.length > 1 agreement check three lines below, which reads on the listing: on a truncated scan a disagreeing rollout may never be reached, and a lone match is then taken as agreement. Measured, two rollouts stating one payload.id under two containers:
untruncated -> refuses, names session-A and session-B
maxScan=1 -> ok: CONFIDENT ANSWER session-B (evidence: rollout-dup-b.jsonl)
maxScan>=2 -> refuses
rolloutFiles walks with a LIFO stack, so which files fall inside the bound is arbitrary. I fixed the reasoning, not the behaviour, and want that choice visible: refusing on truncation would disable auto-resolution for every history past MAX_ROLLOUT_SCAN (5000 rollouts is reachable for a heavy user), and the trigger needs two rollouts whose first line states one payload.id under two different containers, which Codex does not write - a fork copies the parent session_meta as a later line and only line 1 is read - so it takes hand-copied history. Recorded as a known limit in LLP 0067 §cli-drop-key and named in the docblock. For triage if anyone disagrees with that trade; the honest options are to disclose truncation in the provenance (needs a new field, plumbing, and a test-plan entry) or to refuse and accept the usability cost.
5. Merge order with #450 - still correct at this head, restated
Merge #450 first, then #458. Re-verified rather than assumed: #450 (fix/issue-442) is still OPEN and not draft, and its diff adds { env: 'CODEX_THREAD_ID', source: 'codex_env' }, returning the thread id directly as the session id. At this head there is no bare codex_env anywhere in the tree (grep finds only codex_env_rollout, in session_command.js:375/:658, types.d.ts:185/:214, llp/0067:436 and two tests). So merging #450after#458 would reintroduce codex_env and silently restore the thread id as the drop key - the exact defect #453 is about, reintroduced by the merge rather than by any commit. Resolving the conflict is a supersede: where the two disagree, take #458's version. Note already posted on #450.
6. For triage
- F above - the agreement check under a truncated scan. Documented as a known limit, not closed.
- Codex subscription route resolves .hypignore against the ROOT thread's cwd for a subagent turn: rollout-cwd matches the filename thread id, not the container it is given #459 (already filed, not re-filed here) -
rollout-cwd.jsresolves a subagent's.hypignorecwd from the root thread's rollout. Untouched by this PR.
Host state
No state-mutating hyp subcommand was run. No hyp init / attach / detach / join / leave. ~/.claude/settings.json, ~/.codex/config.toml and ~/.config/systemd/ untouched; every probe used its own mktempCODEX_HOME and the real ~/.codex/sessions was never read. All work in a detached worktree; /work/hypaware never switched branches. Only origin/fix/issue-453 was pushed to.
… not a heading-only form Review of PR #456 (head 94fde3f). Round 1 verified, two actionable findings fixed, both in surfaces this PR already rewrites. codex/skills/hypaware-privacy/SKILL.md - Step 1 no longer sends a thread id. Round 1 disclosed, in prose, that `CODEX_THREAD_ID` is a thread id while the drop keys the session container (`codex/src/exchange-projector.js:98`), and left the code path in place because #458 rewrites this block. The disclosure is not sufficient: the thing that runs is the bash block, `ai-gateway/src/control.js` sets `ignored = true` unconditionally on POST for whatever opaque token it was handed, so the block prints `opt-out confirmed` and the review then discusses the machine's most sensitive content believing it is not recorded. The prose caveat gives an agent no verb with which to re-check. The block is also doomed text: #458's version of Step 1 has no `CODEX_THREAD_ID` in it, so keeping the risky form buys nothing that survives the rebase. Step 1 now resolves the id from the rollout's `payload.session_id`, the container the gateway matches, and refuses when a rollout predates it rather than substituting `payload.id`. That is #458's decision, so this shrinks the conflict instead of widening it. #452(a)'s win is untouched: cwd matching, refusal on zero / ambiguous / stale, and the `INFERRED from <rollout>` label. `CODEX_THREAD_ID` keeps a paragraph explaining why it is not the answer and what #453 makes of it. The verification comment now states its own bound: a true `ignored` proves the token is in the drop set, not that it is this session's. Verified with a fixture harness over the extracted block (fake CODEX_HOME, fake gateway, stubbed curl for the default-port case): 12 cases pass, including a subagent fixture where `CODEX_THREAD_ID` is exported and diverges from the container - the container is what reaches the gateway and the thread id never does. Round 1's `|| true` fix is intact: the documented `http://127.0.0.1:8787` default is still reached with no config.toml. test/plugins/codex-privacy-skill-session-id.test.js - new. #452(a) and the grain rule had no ground-truth gate; the PR body says so. Pins both decisions a later edit could silently reverse, in the manner of test/plugins/query-skill-description.test.js. Fails on each of five mutations: reverting to head's `CODEX_THREAD_ID` assignment, collecting `payload['id']`, substituting the thread id when no container is recorded, restoring newest-by-mtime, and dropping the staleness refusal. .claude/skills/ref-check/SKILL.md §3 - `{#slug}` is not a heading-only form. Form 2 was written as an explicit *heading* anchor. The corpus's usual placement is a list item, one bullet per named proposition (`- **Deadline rule** {#deadline}: ...`): 25 markers across 8 docs, and 47 references target them. A checker following §3 literally reports all 47 as BROKEN, which is the same false-positive class this PR exists to remove, and it is most of the "69 pre-existing broken" baseline. With `{#...}` indexed on any line, 22 references are genuinely broken, so #457's count wants correcting. Attribution numbers re-measured, since they were understated: heading-slugs-only breaks 632 references (366 `{#...}`, 266 inline `<a id>`), not 498 (229/269). Whitespace collapsing adds 30, as stated. The heading-count parenthetical said 709 headings / 718 ids; the corpus has 1098 / 1107 (709 is close to the count of *distinct* heading texts). The slug rule itself is confirmed: rendered all 132 `llp/` files through GitHub's `POST /markdown` and diffed id-for-id, 1107 of 1107 exact. Round 1's two narrowings are confirmed necessary and sufficient - stripping all inline HTML misses 4 ids, stripping none misses 9. llp/0001 records the list-item placement alongside the heading form. npm test 2848 pass / 8 fail (the leave-command.test.js baseline) / 1 skipped; typecheck clean. Co-Authored-By: Claude <noreply@anthropic.com>
philcunliffe
commented
Jul 29, 2026
Triage verdict: SHIP (non-blocking residuals only)Round 1 and round 2 both hit exceptional depth (26 refusal paths probed, upstream
R1 - verified myself, not just re-read: the confident-wrong-answer under a truncated scan is real, and reproduces exactly as round 2 describedBuilt the trigger directly against Cause, read at
But the precondition is stronger than "5000-rollout scan cap" makes it sound, and I went one Verdict: PREFERENCE, not a blocker. The bug is real and the mechanism is exactly the failure R2 - staleness bound on the stated-thread path: reasoning holds, checked independentlyVerified Stale PR-body detail correctedThe body's ground-truth gate said Merge order (restated per the standing instruction)#450 must merge before #458. Merging #450 after this one would reintroduce a bare Host stateNo state-mutating Not readying this PR and not merging it - that stays the author's/maintainer's call. |
#450 landed on master and this branch is its follow-up, so where the two disagree about session-id resolution the conflict resolves as a SUPERSEDE rather than a merge, per this PR's stated ordering. Kept from #450: the liveness benefit of CODEX_THREAD_ID (Codex sets it on the process it spawns, so a finished thread cannot have set it), the two-stated-clients refusal, and the blank-variable fallthrough. Kept from this branch: the correct grain. CODEX_THREAD_ID is a selector, not an answer - it names the live thread, the rollout is looked up by payload.id, and the session container the gateway drops on is read out of it. `codex_env_rollout` replaces `codex_env`. Reconciled beyond the marked conflicts, where master's #450 text auto-merged but its claims no longer hold: - session_command.js `provenanceNotes` doc: CODEX_THREAD_ID no longer "states" the session id, so its path is qualified rather than presented as stated. Master's residual (a process outliving its spawn keeps the variable) is kept, scoped to CLAUDE_CODE_SESSION_ID. - LLP 0066 R10: the removed age bound is about liveness alone; the resolved container stays an inference, so R12 still applies to it. - LLP 0067 test plan: the stated-thread path reports `codex_env_rollout` with the rollout named in `session_id_evidence`, not `codex_env` with none. - Dropped #450's six resolver tests, which assert the superseded behaviour (source `codex_env`, the thread id as the answer, no evidence). This branch's tests cover the same intent at the corrected grain. Its one assertion with no equivalent here - a blank CLAUDE_CODE_SESSION_ID beside a real Codex thread is not ambiguity - is carried over, adapted. Local run: npm test 2875 pass / 8 fail (all 8 are the pre-existing test/core/leave-command.test.js baseline, identical on pristine origin/master); the 96 tests in ai-gateway-session-status, codex-backfill and codex-exchange-projector all pass; npm run typecheck clean; smokes session_optout_capture_drop and backfill_codex_fixture ok. Co-Authored-By: Claude <noreply@anthropic.com>
philcunliffe
commented
Jul 30, 2026
Triage verdict: SHIP (non-blocking residual only) - head |
philcunliffe
commented
Jul 30, 2026
🤖 neutral: triage marker repaired (was a dangling |
Uh oh!
There was an error while loading. Please reload this page.
Two things, both forced by what landed on master while this was in review. ## The conflict: #458 rewrote the file this PR collapses #458 ("hyp session ignore names the session container Codex drops on") and this PR both touch `readRolloutMeta` in `ai-gateway/src/session_command.js`, from opposite directions. #458 grew it: a `type === 'session_meta'` envelope guard, a third field (`payload.session_id`), a blank-after-trim test on that field, and a new resolution path (`resolveFromStatedThread`) built on it. This PR deletes it: issue #465's whole value is that no second copy of the `session_meta` rules survives, so the function becomes a delegation to `src/core/codex/rollout_session_meta.js`. Resolved by keeping both. Every behaviour #458 added is intact - the container is still the answer, `CODEX_THREAD_ID` is still a selector rather than an answer, a legacy or blank `session_id` still refuses rather than falling back to the thread id - and none of the predicates behind it live here any more. The three rules (raw line, envelope type, blank-is-absent) are stated once, in the core reader, which is the point of #465. ## Third-copy check #458's own resolution logic does NOT read `session_meta` fields directly. `resolveFromStatedThread` matches on `meta.threadId` and reads `meta.sessionId`, both from `readRolloutMeta`, so it is a second *resolution path*, not a second *reader*, and routing it through the shared reader needed no change to it. What had become a full second copy is `readRolloutMeta` itself: #458 gave it its own envelope guard and its own blank test on the new field, which is precisely the duplicate this PR removes. `statedEnv` is a blankness test on an environment variable, not on the header, and stays. The only other site that reads `session_meta` fields itself is `codex/src/backfill.js`, which walks whole rollout files (folding `turn_context`) and so cannot call a first-line reader. It shares the one `cwd` predicate (`sessionMetaCwd`) and deliberately does not share rule 3's id refusal: a backfilled row must land in some partition, where the CLI can refuse. Now stated in the LLP rather than only in the code comment. ## One behaviour change the merge required `readRolloutMeta` no longer discards a rollout whose `cwd` is unusable; it passes `cwd: undefined` through. The shared reader refuses a blank or relative `cwd` (LLP 0150 #usable-cwd), and on the cwd-matching path that is what we want. On #458's stated-thread path `cwd` is never consulted, so requiring it would have turned a field-level predicate into a file-level one and refused a session whose container is plainly on disk - a regression of #458 introduced by tightening a field it does not use. Pinned by a new test, mutation-checked: restoring the cwd requirement reddens it. ## Reconciled beyond the conflict markers Four claims that auto-merged cleanly but stopped being true once #458 landed: - LLP 0150 said `sessionId` "has no consumer yet on purpose" and that moving the verb onto the container "is #453's job". #453 is closed and the verb is moved; the section now records that, and that #458 added a resolution path but no second reader. - LLP 0150 said the `hyp session` caller "compares `meta.cwd` against an absolute invocation cwd, so a relative value never matched". True of one of its two paths now. Rewritten to say why the other path must not refuse on `cwd` at all. - LLP 0150's rule 3 read as a blanket "callers refuse", which the backfill does not. Scoped to the reader, with the backfill's different answer explained. - The `readRolloutMeta` doc comment said `meta.sessionId` "is deliberately not consulted here". It is consulted now. Also: LLP 0150's Context bullet no longer implies `CODEX_THREAD_ID` makes the rollout unnecessary, and `resolveSessionIdForCli`'s legacy-rollout note points at `legacyRolloutError`, which is where that refusal now lives. ## The renumber: collision avoidance, NOT a ruling on #469#475 landed `llp/0143-openclaw-registers-no-attach-probe.decision.md` while this PR held `llp/0143-one-reader-for-codex-session-meta.decision.md`. Different filenames, so git flags no conflict, but merging as-is would put two documents at 0143 on master, a fifth duplicate after 0098, 0099, 0111 and 0142. This document moves to 0150 (0149 is the highest on master) and all 12 references follow: `@ref` annotations in `session_command.js`, `backfill.js`, `rollout-cwd.js`, `rollout_session_meta.js`, `types.d.ts` and four test files, plus the heading and one self-reference in the document. **This is mechanical collision avoidance and sets no precedent.** Issue #469 asks whether the later claimant renumbers or whether citations become filename-qualified, and that question is still open and unowned. Renumbering here is only what avoids adding a sixth duplicate today; whichever way #469 is decided, nothing about this commit should be read as having decided it. The human's 0143 is untouched, as are LLP 0142's two references to it. Verified: `npm test` 2915 tests, 2905 pass, 8 fail (the pre-existing `test/core/leave-command.test.js` set, identical on a pristine `origin/master` worktree), +12 tests and no new failures. `npm run typecheck` clean. Smokes `gateway_codex_capture`, `session_optout_capture_drop`, `backfill_codex_fixture` all ok. All `@ref LLP 0150` targets and anchors resolve. Co-Authored-By: Claude <noreply@anthropic.com>
…esent, and ref-check's snapshot note scopes itself #453 landed on master via #458, so `hyp session ignore` already uses CODEX_THREAD_ID as a selector. Round 3 corrected that tense in the skill's Step 1 prose and left the same stale "until then" in the test that pins the decision, which is the surface a later reader checks the rule against. Also scopes round 3's snapshot note to the section it measured: the sample report under "Report findings" carries illustrative counts that were never measured at `79d147c`, so "every count on this page" overclaimed. Co-Authored-By: Claude <noreply@anthropic.com>
…THREAD_ID, and the ignore caveat names the fork (#452, #455) (#456) * docs: Codex session id comes from CODEX_THREAD_ID, and the ignore caveat names the fork too (#452, #455) Two documentation-accuracy fixes plus the one user-visible string they both point at. #455 - `hyp session ignore` printed "a gateway restart drops it", which reads as the exhaustive list of ways the opt-out stops applying. LLP 0066 §readable names a second: the client minting a new session id for what the user experiences as one conversation. The caveat now names both, hoisted into a single `EPHEMERAL_NOTE` so the writer's wording and `status`'s cannot drift apart. LLP 0066 R9 gains the requirement; LLP 0067's annotation map gains the site. #452(a) - the Codex `hypaware-privacy` skill identified the current session by picking the newest rollout by mtime, and asserted Codex exposes no session env var. Codex states `CODEX_THREAD_ID` in the environment of the subprocesses it spawns, and newest-by-mtime is exactly the heuristic that resolves a finished session, so a privacy skill built on it can mark or purge another session's rows. The skill now prefers the stated id and describes the disk scan as the fallback it is: cwd-matched, refusing on ambiguity, and bounded by the ~30 minute staleness window, reported as "inferred from <rollout>". #452(b) - all ten `@ref LLP 0086#*` annotations resolve: LLP 0086 carries its anchors as inline `<a id="...">`, the corpus's normal way to give one section several named propositions, so the `grep '{#'` that reported them dangling was too narrow. The real defect was the checker's contract: ref-check's index step described heading slugs only, which reports 269 resolvable refs across the corpus as broken. It now indexes all three anchor forms, warns on duplicate LLP numbers, and slugifies the way the renderer does. LLP 0001's "heading-slug anchors" line is amended to match. The sibling annotation at `ai-gateway/src/types.d.ts` carried no gloss, which LLP 0000 requires; it and its two neighbours now have one. Co-Authored-By: Claude <noreply@anthropic.com> * review fixes: reachable base fallback, CODEX_THREAD_ID grain disclosure, exact slug rule Review of PR #456 (head fa7c3bc). Three fixes, all in the surfaces this PR already rewrites. codex/skills/hypaware-privacy/SKILL.md - The `[model_providers.hypaware]` base_url lookup aborted the whole Step 1 script under `set -e -o pipefail` whenever config.toml was missing or had no `base_url` (grep exits nonzero, pipefail propagates, the assignment fails). The documented `http://127.0.0.1:8787` default on the next line was therefore unreachable, and because grep's stderr is discarded the operator saw "resolved session <id>" followed by nothing and exit 1. Pre-existing, but it is the happy path of the block this PR rewrites. `|| true` restores it. Verified with a fixture harness: the five Step 1 cases (stated id, zero matches, ambiguous, stale, one fresh match) now all behave as documented, refusals still exit 1 without reaching the gateway. - `CODEX_THREAD_ID` is a thread id; the gateway drops on the session container (`codex/src/exchange-projector.js:98` keys on `metadata.session_id`, falling back to the conversation id). They coincide for a root thread and diverge for a subagent, which exports its own thread id, so an opt-out taken there names a token the drop never matches while the control route still echoes `ignored: true`. Step 1 said the stated id "needs no inference" with no grain caveat; it now states the grain, names issue #453 as the correction, and tells the operator not to treat such a confirmation as proven. .claude/skills/ref-check/SKILL.md §3 - The slug rule is right about whitespace but silent on two details. Headings that carry their own `<a id>` (9 in this corpus) must have the anchor tags stripped before slugifying, and only those: `<target>` inside a code span is text the renderer keeps. Repeated slugs take the renderer's `-1` suffix. Verified against GitHub's markdown API over all 709 llp/ headings (718 ids): the stated rule now reproduces every one exactly. - Corrected the false-BROKEN attribution. Measured on this corpus: indexing only heading slugs breaks 498 refs (229 `{#...}`, 269 inline `<a id>`); collapsing whitespace adds 30. The anchor forms dominate, not the whitespace. - Noted that `{#...}` is a corpus convention GitHub does not honor, so such an anchor resolves for ref-check but does not navigate in a rendered view. ai-gateway/src/types.d.ts - The two remaining glossless `@ref`s in the file this PR was de-glossing (`0066#control-path`, `0066#ephemeral`) now carry one, per LLP 0000. npm test 2845 pass / 8 fail (the leave-command.test.js baseline) / 1 skipped; typecheck clean; ref-check unchanged at 69 pre-existing broken, 0 for LLP 0086. Co-Authored-By: Claude <noreply@anthropic.com> * review round 2: the skill sends the session container, and {#slug} is not a heading-only form Review of PR #456 (head 94fde3f). Round 1 verified, two actionable findings fixed, both in surfaces this PR already rewrites. codex/skills/hypaware-privacy/SKILL.md - Step 1 no longer sends a thread id. Round 1 disclosed, in prose, that `CODEX_THREAD_ID` is a thread id while the drop keys the session container (`codex/src/exchange-projector.js:98`), and left the code path in place because #458 rewrites this block. The disclosure is not sufficient: the thing that runs is the bash block, `ai-gateway/src/control.js` sets `ignored = true` unconditionally on POST for whatever opaque token it was handed, so the block prints `opt-out confirmed` and the review then discusses the machine's most sensitive content believing it is not recorded. The prose caveat gives an agent no verb with which to re-check. The block is also doomed text: #458's version of Step 1 has no `CODEX_THREAD_ID` in it, so keeping the risky form buys nothing that survives the rebase. Step 1 now resolves the id from the rollout's `payload.session_id`, the container the gateway matches, and refuses when a rollout predates it rather than substituting `payload.id`. That is #458's decision, so this shrinks the conflict instead of widening it. #452(a)'s win is untouched: cwd matching, refusal on zero / ambiguous / stale, and the `INFERRED from <rollout>` label. `CODEX_THREAD_ID` keeps a paragraph explaining why it is not the answer and what #453 makes of it. The verification comment now states its own bound: a true `ignored` proves the token is in the drop set, not that it is this session's. Verified with a fixture harness over the extracted block (fake CODEX_HOME, fake gateway, stubbed curl for the default-port case): 12 cases pass, including a subagent fixture where `CODEX_THREAD_ID` is exported and diverges from the container - the container is what reaches the gateway and the thread id never does. Round 1's `|| true` fix is intact: the documented `http://127.0.0.1:8787` default is still reached with no config.toml. test/plugins/codex-privacy-skill-session-id.test.js - new. #452(a) and the grain rule had no ground-truth gate; the PR body says so. Pins both decisions a later edit could silently reverse, in the manner of test/plugins/query-skill-description.test.js. Fails on each of five mutations: reverting to head's `CODEX_THREAD_ID` assignment, collecting `payload['id']`, substituting the thread id when no container is recorded, restoring newest-by-mtime, and dropping the staleness refusal. .claude/skills/ref-check/SKILL.md §3 - `{#slug}` is not a heading-only form. Form 2 was written as an explicit *heading* anchor. The corpus's usual placement is a list item, one bullet per named proposition (`- **Deadline rule** {#deadline}: ...`): 25 markers across 8 docs, and 47 references target them. A checker following §3 literally reports all 47 as BROKEN, which is the same false-positive class this PR exists to remove, and it is most of the "69 pre-existing broken" baseline. With `{#...}` indexed on any line, 22 references are genuinely broken, so #457's count wants correcting. Attribution numbers re-measured, since they were understated: heading-slugs-only breaks 632 references (366 `{#...}`, 266 inline `<a id>`), not 498 (229/269). Whitespace collapsing adds 30, as stated. The heading-count parenthetical said 709 headings / 718 ids; the corpus has 1098 / 1107 (709 is close to the count of *distinct* heading texts). The slug rule itself is confirmed: rendered all 132 `llp/` files through GitHub's `POST /markdown` and diffed id-for-id, 1107 of 1107 exact. Round 1's two narrowings are confirmed necessary and sufficient - stripping all inline HTML misses 4 ids, stripping none misses 9. llp/0001 records the list-item placement alongside the heading form. npm test 2848 pass / 8 fail (the leave-command.test.js baseline) / 1 skipped; typecheck clean. Co-Authored-By: Claude <noreply@anthropic.com> * review round 3: the ambiguity refusal survives a non-string session_id, and ref-check's counts are dated Two low-severity fixes from the round-3 review of the merge head dc79fc5. codex/skills/hypaware-privacy/SKILL.md: the ambiguity refusal built its candidate list with `', '.join(m[0] or m[1] for m in matches)`. The merge folded in master's non-string `payload.session_id` guard, but that guard runs only on matches[0], after the ambiguity branch has already returned. So two rollouts recording this cwd, one of them carrying a non-string session_id, raised `TypeError: sequence item 0: expected str instance, int found` and printed a Python traceback instead of "N rollouts record cwd X: ambiguous, confirm the session id with the user". It failed closed (exit 1, no gateway call), but the refusal text is what the agent relays to the user, and a traceback tells them nothing to act on. `str(...)` around the element. Verified by execution: extracted the Step 1 block verbatim and ran it against fixture CODEX_HOME trees with a stub gateway. Before: traceback. After: "2 rollouts record cwd /tmp/... (777, C2): ambiguous, confirm the session id with the user", exit 1, 0 gateway calls. 20 cases total re-run, unchanged. .claude/skills/ref-check/SKILL.md: section 3 presented "22 references are genuinely broken" as "the number a run should reproduce". Those figures were measured at 79d147c over 132 files / 129 LLP numbers; this head carries 149 files / 149 numbers, 1220 headings and ~1495 `@ref LLP` occurrences, because master added LLPs 0157-0159 and others while this branch was open. A number that drifts with the corpus reads as a regression signal when it is not. The counts are now dated to the commit they were taken at, and the paragraph says what a run must actually reproduce: the rule, and no BROKEN report for a reference the three anchor forms resolve. npm test 3041 tests, 3032 pass, 8 fail, 1 skipped - the 8 are the pre-existing test/core/leave-command.test.js "leave ..." set, name-for-name identical to a pristine origin/master worktree (3037 tests, same 8). npm run typecheck clean. Co-Authored-By: Claude <noreply@anthropic.com> * review round 4: the CODEX_THREAD_ID test comment states the landed present, and ref-check's snapshot note scopes itself #453 landed on master via #458, so `hyp session ignore` already uses CODEX_THREAD_ID as a selector. Round 3 corrected that tense in the skill's Step 1 prose and left the same stale "until then" in the test that pins the decision, which is the surface a later reader checks the rule against. Also scopes round 3's snapshot note to the section it measured: the sample report under "Report findings" carries illustrative counts that were never measured at `79d147c`, so "every count on this page" overclaimed. 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-loop <neutral-loop@users.noreply.github.com> Co-authored-by: test <test@test.com> Co-authored-by: neutral <neutral@hyparam.dev> Co-authored-by: test <test@example.com>
The defect
hyp session ignorecould report success while recording continued, which in a privacy control is worse than an error.The verb resolved a Codex thread id (the rollout's
session_meta.payload.id). The drop keys on the session container:exchange-projector.js:98computessession_id = stringValue(codexContext?.session_id) ?? conversationIdand drops at:148on that value. A root thread takessession_id = SessionId::from(thread_id), the same uuid, so the two coincide and nothing looks wrong; a subagent thread inherits the root'ssession_idand mints its ownthread_id(and exports that thread id asCODEX_THREAD_IDto its shell tool calls). So an opt-out taken from inside a subagent tool call named an id the gateway never matches, and printed "the gateway will drop this session".The trade-off chosen: liveness proof and the correct key, by splitting their jobs
CODEX_THREAD_IDis the better liveness signal (Codex sets it on the process it spawns for a tool call, so a finished thread cannot have set it, where rollout mtime is only a proxy) but it is the wrong grain. Neither source carries the answer alone:CODEX_THREAD_IDsession_metasession_id(container),id(thread),cwdSo the variable is used as a selector, not an answer: it names the live thread, the rollout for that thread is looked up by
payload.id, and the container is read out of it. The cwd + staleness path stays as the fallback (old Codex, or a hand invocation), and it too now reports the container. Where only one of the two can be had, the key wins and the verb refuses: a refusal costs a re-run with an explicit id, a confident wrong key costs the user the recording they believed they had stopped (LLP 0066 R10/R13, LLP 0067 §cli-drop-key).The thread id is not discarded, it is reported beside the container (
thread_idin--json,session_id_source: codex_env_rollout, and a human-form note that the drop covers every thread in the session). Reporting one id and calling it "the session" is how the two got conflated in the first place.The trap, and how a present
session_idis told from a back-filled oneCodex's
SessionMetaLinehas a hand-writtenDeserializethat back-fillssession_idfromidwhen the field is absent (codex-rs/protocol/src/protocol.rs:3157-3184), so a resolver reading a deserializedsession_metagets the thread id back under the container's name on every pre-field rollout: this exact defect, reintroduced invisibly through its own fix, on the only files where nothing else would reveal it.readRolloutMetatherefore parses the raw JSONL line, so an absent field is visible as absent, and absent means refuse (naming the file, saying why a thread id will not do, pointing athyp session status <session-id>). It does not fall back to the thread id, and a stated thread whose rollout cannot be read does not fall back to the cwd scan either, since that scan answers about a thread nothing tied to this invocation.What moved together
ai-gateway/src/session_command.js: resolution returns the container;CODEX_THREAD_IDselects the rollout; legacy/orphan/two-stated-clients all refuse; provenance carriesthread_idand the grain disclosure.codex/src/backfill.js:session_idfromsession_meta.session_id(thread as fallback for a pre-field rollout), thread inconversation_id, mirroring the live projector, so the opt-out names one identifier rather than one per ingestion path. Row identity is untouched: the fallback-hash scope isconversation_id ?? session_id(LLP 0030 decision 3), andconversation_idstill holds the thread, sopart_ids are unchanged and keep deduping against live rows.codex/skills/hypaware-privacy/SKILL.md: its step-1 scriptPOSTedpayload.id. It now readspayload.session_id, stops on a rollout that records none, and points athyp session ignore --jsonfirst.Ground-truth gate: the failure path is what the tests exercise
test/plugins/ai-gateway-session-status.test.jssession_id!= ownpayload.id) resolvessession-rooton both the stated-thread and cwd paths; that id makescodex/src/exchange-projector.jsreturnUSAGE_POLICY_DROPfor a subagent turn, andthread-subagent(the id the verb used to state) does not - the silent no-op, pinned so it cannot come back.session_idfield refuses on both paths, and the same thread in a rollout that records a container resolves, so the refusal is provably about the absent field.hyp session ignoreon it prints nothing that reads as done, exitsunknown, and adds nothing to the ignored set.CODEX_THREAD_IDbeats a cwd match inside the staleness window; a stated thread with no rollout refuses; a blank variable falls through; both client variables set at once refuses; the grain/provenance disclosure fromstatusandignorealike.test/plugins/codex-backfill.test.js: a subagent rollout partitions onsession_meta.session_idwith the thread inconversation_id, through the real materializer to the row columns; a rollout with nosession_idkeeps the thread.Verified failing-then-passing: with the source changes stashed, the 6 new session-status tests and the new backfill test fail; with them applied all pass.
npm test: 2855 pass, 8 fail - all 8 are the pre-existingtest/core/leave-command.test.jsbaseline, unrelated to these files.npx tsc -p tsconfig.json --noEmit: clean.session_optout_capture_drop,backfill_codex_fixture,walkthrough_backfill_client_historyall ok.Ordering with PR #450
#450 (
fix/issue-442) is still OPEN, so this branch is based onorigin/masterand #450 should land first. They both rewriteresolveSessionIdForCliand its types, so expect a conflict insession_command.js,types.d.ts,llp/0066,llp/0067andtest/plugins/ai-gateway-session-status.test.js.Resolving it is a supersede, not a merge: #450 returns
CODEX_THREAD_IDdirectly as the session id (source: 'codex_env') and records in LLP 0067 that this leaves the thread/container divergence open for a follow-up. This change is that follow-up, and it keeps #450's liveness benefit while fixing the grain, so where the two disagree take this version: the variable becomes a selector, the answer comes from the rollout, andcodex_env_rolloutreplacescodex_env. The two-stated-clients refusal is semantically identical in both, so keep one copy. #449 touches unrelated files.Fixes#453