Skip to content

Codex session identity is the rollout's session container, not CODEX_THREAD_ID, and the ignore caveat names the fork (#452, #455) - #456

Merged
philcunliffe merged 7 commits into
masterfrom
fix/issue-452
Jul 31, 2026
Merged

Codex session identity is the rollout's session container, not CODEX_THREAD_ID, and the ignore caveat names the fork (#452, #455)#456
philcunliffe merged 7 commits into
masterfrom
fix/issue-452

Conversation

@philcunliffe

@philcunliffephilcunliffe commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Two documentation-accuracy issues, filed separately but close enough in subject
to review together: both are about a surface telling the user something that is
no longer true about session identity. One PR, three independent changes.

#455 - the ephemerality caveat named one of two ways it can lapse

hyp session ignore printed "this opt-out is in-memory only: a gateway restart
drops it"
. LLP 0066 §readable lists two ways the opt-out stops applying
while the user still believes it holds: the restart, and the client minting a new
session_id for what the user experiences as one conversation. Naming only the
restart reads as an exhaustive list, so a user who forks is taught that the other
way cannot happen.

  • hypaware-core/plugins-workspace/ai-gateway/src/session_command.js: the string
    is hoisted into one module constant, EPHEMERAL_NOTE, and extended:
    ... a gateway restart drops it, and a fork (claude --fork-session, codex fork) mints a new session id it no longer covers. Re-check with hyp session status.
  • Should hyp session status carry it too? It already did, as a second,
    byte-identical copy of the same sentence in writeStatus's ignored branch.
    Both now read the one constant, which is why this is a hoist rather than two
    edits: the reader's caveat and the writer's cannot drift apart again.
    unignore stays silent (it has no opt-out to qualify), and --json is
    untouched.
  • Codex's codex fork: named in the string alongside claude --fork-session,
    since LLP 0066 records that Codex splits the same two behaviours across
    separate verbs.
  • LLP, same commit:LLP 0066 R9 gains the requirement (readability is
    necessary but not sufficient; the user has to know there is something to
    re-check, so the caveat MUST name both ways). LLP 0067's annotation map gains
    the EPHEMERAL_NOTE row.
  • Test (test/plugins/ai-gateway-session-status.test.js): "the ephemerality
    caveat names the fork that mints a new session id, not only a restart"
    asserts
    the new text on the ignore path and the status path, that the restart
    half survives, and that unignore says neither. Verified failing on
    origin/master (not ok 19 ... did not match /mints a new session id it no longer covers/) and passing after.

Deliberately worded to be true under both master and #450: a fork mints the
new id, a plain resume reuses it. master's §readable parenthetical still says
"(a resume, a fork)"; #450 refines exactly those lines, so this PR does not touch
them.

#452(a) - the Codex privacy skill identified the session by mtime

hypaware-core/plugins-workspace/codex/skills/hypaware-privacy/SKILL.md Step 1
asserted Codex exposes no session env var and told the user to pick the newest
rollout by mtime
. Both were true when written. Codex states CODEX_THREAD_ID
in the environment of the shells and exec subprocesses it spawns, and
newest-by-mtime is precisely the heuristic that resolves a finished session
(the stale-rollout defect #450 fixes in the verb). A privacy skill built on it
can walk a user into marking or purging another session's rows.

[Corrected by triage after round 2's reversal of this PR's own R1 finding -
see the round-2 comment.]
Step 1 now prefers hyp session ignore, the
tested CLI verb, first. Only where it cannot resolve the session does the
fallback below apply, and the fallback is notCODEX_THREAD_ID: round 2
found that env var names the thread, not the session container the
gateway drops on, and the two diverge on a subagent thread, so a thread id
sent as the opt-out would name a token the drop never matches. The fallback
is a disk scan matched on payload.cwd, reading payload.session_id (the
container, not payload.id, the thread), refusing rather than guessing on
zero or multiple matches, refusing on a single match older than ~30 minutes (a
live session appends every turn), refusing when the matched rollout predates
the session_id field entirely, and reporting the result as
INFERRED from <rollout> on disk so an unrecognised session is visible.

Is this sufficient before #435? Partly. Read this bit.

Yes for the filed defect, no for the duplication. The skill body still
curls /_hypaware/ignore/session directly and never calls hyp session, so it
does not inherit the verb's fix by reference; the resolution order had to be
corrected in the skill, and it now is. That part is complete and standalone.

What it does not get, and cannot until #435 collapses these skills onto the
verb, is everything else hyp session owns: control-response validation (the
session_id echo check), endpoint discovery from status.json rather than a
port literal, and the single tested implementation of the refusal rules this
change now states twice, once in JS and once in embedded bash. So: correct now,
still a duplicate until #435. #435's work (deleting the bash, adding the Codex
hypaware-ignore/hypaware-unignore skills, rewording LLP 0066/0067
§motivation) is not done here.

Also carried into both privacy skills, for #455: the restart note in Step 1 now
names the fork case too (claude --fork-session / codex fork <id>), and points
at hyp session status.

#452(b) - the LLP 0086 anchors are not broken; the checker's contract was

Verified, and it contradicts the issue: all ten @ref LLP 0086#* annotations
in the tree resolve.
LLP 0086 carries its anchors as inline
<a id="endpoint-discovery"> / <a id="manual-attach-reads-the-live-port"> etc.
(lines 48, 78, 89, 100), which is the corpus's normal mechanism, used by 27
decision docs, for giving one section several named propositions, as D1
and D2 each do. The git grep -c '{#' in the issue only looks for the other
anchor spelling, so it reported a resolvable ref as dangling.

The real defect is one level up, in the tool that was supposed to catch this:
.claude/skills/ref-check/SKILL.md §3 told the checker to index "all heading
anchors ... generated by slugifying each heading"
and nothing else. Run that
literally and 269 resolvable references across the corpus report BROKEN,
which is exactly how a false positive got filed and how a real one would be lost
in the noise. Fixed:

  • §3 now indexes all three anchor forms (heading slugs, {#slug} suffixes,
    inline <a id>), with the renderer's actual slug rule spelled out (each space
    becomes one -, no collapsing, hence the double hyphen a dash-titled heading
    leaves behind: part-1--the-client-seam). Collapsing whitespace alone accounts
    for 269 of the 294 false BROKENs.
  • §3 also now says to record duplicate LLP numbers and resolve an anchor
    against any claimant. Three numbers are claimed twice (see below); a checker
    that keeps only the last file it walks reports every ref aimed at the other one
    as broken.
  • llp/0001's "heading-slug anchors" line is amended to match what the corpus
    actually does.
  • The sibling annotation the issue mentions is
    ai-gateway/src/types.d.ts:187, and it did have a real defect: no gloss,
    which LLP 0000 and CLAUDE.md require. It and its two glossless neighbours in
    that file now have one.

/ref-check across the tree

1356 refs, 860 files, 129 LLPs. 0 broken @ref LLP 0086#*. Everything below
is pre-existing and not fixed here, so it can be tracked:

  • BROKEN, 11 in code: @ref LLP 0103#cli.llp/0103 has no cli anchor in
    any form. src/core/commands/clients.js 710, 755, 891, 938, 1008, 1074 and
    src/core/commands/policy.js 33, 218, 254, 291, 338, plus three more in
    llp/0111-hyp-policy-verb.design.md (21, 247) and llp/0112 (42). Fix is one
    anchor in LLP 0103; deliberately left for its own issue.
  • BROKEN, 11 in skill docs: illustrative @ref examples in
    .claude/skills/ref-check/SKILL.md and ref-story/SKILL.md that point at
    placeholder LLPs (LLP 0042#token-strategy, docs/vendor/spec.md). Cosmetic,
    but they mean a scripted ref-check can never exit 0 on this repo.
  • BROKEN, 1: llp/0135 line 959 quotes a line-wrapped
    @ref LLP 0011#interactive- in prose. Harmless artefact of quoting.
  • WARNING, 4: refs to superseded LLPs.
    src/core/usage-policy/local_only.js:28 and matcher.js:215 both
    @ref LLP 0080#fail-safe, and LLP 0080 is Superseded. Worth an issue: per
    CLAUDE.md these should point at the superseding doc. The other two are
    intentional supersession prose in llp/0101/llp/0102 headers.
  • Corpus hygiene: LLP numbers 0098, 0099 and 0111 are each claimed by two
    files
    , so @ref LLP 0111#surface is formally ambiguous (it means
    0111-hyp-policy-verb.design.md, not 0111-report-cli.decision.md). Renumbering
    would touch dozens of refs; wants its own issue.
  • HINT, 459: mostly @ref ... [relation] <dash> gloss using a long dash
    instead of the : LLP 0000 specifies. Also a house-style violation (no em
    dashes). A mechanical sweep, not attempted here.

Other copies of the mtime heuristic, from a tree-wide grep: only
docs/ACCEPTANCE.md step 5 (xargs -0 ls -t | head -1), and there it is sound.
An operator runs it seconds after the session it is checking, nothing
destructive keys off the result, and it is a release gate rather than user
guidance. Left alone. No other skill or doc repeats it.

Checks

Ordering against #450 (resolved)

Superseded.#450merged on 2026-07-29 (c551d6e, now an ancestor of master),
and this branch already carries master through its merge commit dc79fc5. There is
no ordering constraint left and nothing to merge first. The text below is kept only as
the record of what the constraint was.

gh pr view 450 --json state -> OPEN, branch fix/issue-442. Based on
origin/master, so session_command.js will conflict with #450, which rewrites
resolveSessionIdForCli and the SessionIdResolution type in the same file.
Merge #450 first. The conflict is mechanical and disjoint: #450 touches the
resolver and the type, this PR touches a constant near the top of the file plus
two call sites. LLP 0066 is disjoint by hunk (this edits R9; #450 edits §readable
and R10). LLP 0067 likewise (this edits the annotation map; #450 edits
§cli-session-id and the test plan). #453 is in flight on fix/issue-453 and also
touches this file; not pushed to.

Fixes#452
Fixes#455

…eat 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>
…re, 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>
@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

review rung, PR #456 @ fa7c3bc

Reviewed in a detached worktree at fa7c3bc. No codex on PATH and no dual-review/code-review skill in this clone, so this is a direct review. Baseline reproduced exactly: npm test 2845 pass / 8 fail, all test/core/leave-command.test.js / 1 skipped; npm run typecheck clean.

Pushed 3 fixes as 94fde3f. One HIGH finding is left deliberately unfixed because an open PR owns it; see the merge order at the bottom.


1. The premise correction is correct, and §3's slug rule is exactly right

LLP 0086's anchors are not dangling.llp/0086-attach-tracks-ephemeral-port.decision.md lines 48, 78, 89, 100 carry inline <a id="...">, including manual-attach-reads-the-live-port at line 78. 28 LLP docs use inline <a id>; 33 use {#slug}; 129 docs total. The issue's git grep '{#' looks for the other spelling only. Confirmed.

I implemented §3 as a script and ran it (Node; note this container has no python3 at all). Results over 1356 refs / 353 files / 129 LLPs:

§3 asBROKEN@ref LLP 0086#* broken
written in this PR (3 forms, no-collapse)69, all pre-existing0
pre-PR (heading slugs only)656many

So yes: §3 followed literally now resolves all ten LLP 0086#* refs.

The slug rule is right, and I verified it against the renderer rather than trusting the claim. I rendered every heading in llp/ through GitHub's own markdown API (POST /markdown) and diffed id-for-id: 709 headings, 718 ids, 0 mismatches for "lowercase, strip punctuation keeping -/_, replace each whitespace with -, no collapsing". ## Part 1 — The client seam in the reconcile context really does render id="part-1--the-client-seam-in-the-reconcile-context", and - really does yield a triple hyphen (d2---manual-hyp-attach-...). The no-collapse rule also strictly dominates: no ref is broken under it that collapsing would have saved.

But the PR body's arithmetic is inverted, and it matters. The body says "Collapsing whitespace alone accounts for 269 of the 294 false BROKENs." Measured by attribution over every anchored ref:

  • resolved by heading slug: 554 (only 30 of which depend on the no-collapse rule)
  • resolved by a {#...} suffix: 229
  • resolved by an inline <a id>: 269

269 is the inline-<a id> count, not the whitespace count. Indexing only heading slugs breaks 498 refs; collapsing adds 30. The dominant cause is the un-indexed anchor forms, and a reader who trusts the body could conclude the whitespace rule was the whole fix and drop form 3. §3 itself carried the same superlative ("the single most common way to get this wrong"). I corrected it in §3 with the measured numbers. The PR body I left alone as instructed.

Two real gaps in §3 that I fixed:

  1. Nine headings in the corpus carry their own <a id> on the heading line (llp/0054 x6, llp/0097, llp/0098 x2). GitHub slugifies the rendered text, yielding both memory-invariant and peak-execution-memory-is-budget-bounded; §3's literal "slugifying each heading" yields a-idmemory-invariantapeak-.... Latent only (0 refs currently target those heading slugs, and wrapper-duties happens to coincide with its own inline id), but it is exactly the false-BROKEN class §3 exists to prevent. My first attempt at the wording said "strip inline HTML", which is wrong and broke 4 other headings: <target> in ## D1: Login is a browser mode of \hyp remote login `is a code span the renderer keeps. Narrowed to "strip<a ...>/` and only those" and re-verified to 0/718.
  2. §3 point 2 presents {#surface} under point 1's "the way the Markdown renderer does". GitHub does not honor {#...} - it emits id="the-command-surface-surface" and no surface id. ref-check should still index it (it is what @refs target), but such an anchor never navigates in a rendered view. Now stated.

Also confirmed and unchanged: 3 duplicate LLP numbers (0098, 0099, 0111), so §3's "resolve against any claimant" rule is load-bearing.

2. The skill snippet, read and run as an operator would

I extracted the Step 1 bash block verbatim and ran it against fixture ~/.codex trees with a stub gateway.

The good news, and it is the important part: the refusal logic is expressible in this shell and it works.resolved="$(python3 ... )" is a plain assignment, so set -e does abort on a nonzero substitution; there is no fall-through to a wrong session. Zero matches, ambiguous (2 matches), and stale (2h) all print the intended message to stderr, exit 1, and never reach the gateway. A stated CODEX_THREAD_ID bypasses the disk entirely; a single fresh match reports INFERRED from rollout-... on disk. All five cases behave as documented.

One shell quirk did break it, on the happy path (pre-existing, unchanged by this PR, but it is the block being rewritten). Under set -euo pipefail:

BASE="$(grep -A6 '^\[model_providers.hypaware\]' "$CODEX_HOME/config.toml" 2>/dev/null \
| grep -m1 'base_url' | sed -E 's/.*"([^"]+)".*/\1/')"

If config.toml is missing or has no base_url, grep -m1 exits 1, pipefail propagates it, the assignment fails and set -e kills the script - so the documented BASE="${BASE:-http://127.0.0.1:8787}" default on the next line is dead code. bash -x trace: + BASE= then exit 1, never reaching the [ -z ] test. Because grep's stderr is discarded, the operator sees

resolved session thread-abc (stated by Codex (CODEX_THREAD_ID))

and then nothing, exit 1. It fails closed, but silently and in a way that reads as if resolution succeeded. Fixed with || true; all five cases re-verified, and case A now completes end to end.

On the two-places-that-can-drift question, the PR is only partly honest. It says plainly that the skill still curls the control route, that the refusal rules are now stated twice (once in JS, once in bash), and that #435 owns collapsing them. That much is accurate and well flagged. What it does not say is that the two copies already disagree, today, about the primary resolution source: resolveSessionIdForCli (session_command.js:447-451) reads only CLAUDE_CODE_SESSION_ID and otherwise scans rollouts; CODEX_THREAD_ID appears nowhere in the tree except this new SKILL.md. Meanwhile this PR newly points the user at hyp session status in both skills and in the CLI caveat string. So in a Codex session where the variable is set but the cwd scan is ambiguous or stale, the skill opts out session X and the recommended re-check refuses with unknown. That is #450's job (below), not a defect to patch here, but the body frames the #450 relationship as a merge conflict plus wording compatibility and never says #450 is what makes the skill's primary source available to the verb it tells users to trust.

3. HIGH - CODEX_THREAD_ID is the wrong grain for the key the gateway drops on

This is the one finding I did not code-fix.

  • The skill sets SESSION_ID="${CODEX_THREAD_ID:-}" (SKILL.md:43) and POSTs it to /_hypaware/ignore/session.
  • The drop keys on the session container: codex/src/exchange-projector.js:98, const sessionId = stringValue(codexContext?.session_id) ?? conversationId, then USAGE_POLICY_DROP at :136/:156.
  • The control route never validates: ai-gateway/src/control.js:33, "The session_id is an opaque token: the gateway never [checks it]".

For a root thread session_id == thread_id, so nothing looks wrong. For a subagent thread, per #458's analysis of codex-rs, it inherits the root's session_id and exports its ownthread_id as CODEX_THREAD_ID to its shell tool calls. Skills run as tool calls. So an opt-out taken there POSTs a token the projector never matches, the route echoes ignored: true, and Step 1 prints opt-out confirmed for session <thread> while the review transcript keeps being recorded. I reproduced the shape end to end against a stub gateway: GATEWAY HIT session_id=thread-abc followed by opt-out confirmed.

That is precisely the "reports success while recording continued" defect of #453, in the one step whose stated purpose is that the review "must never itself become a captured, forwardable transcript". Pre-PR the skill also POSTed a thread id (payload.id), but it got it from the newest rollout, which for a root session is the container; elevating CODEX_THREAD_ID to the primary source makes divergence guaranteed rather than incidental in exactly the context skills run in.

I did not rewrite it because #458 already implements the correct design with tests (variable as selector, container read from payload.session_id, refuse when absent) and rewrites this same Step 1 block. Duplicating that here would collide. Instead I added the grain disclosure to the PR's own new prose (SKILL.md:24) so the skill stops asserting the stated id is unconditionally better, names #453, and tells the operator not to treat such a confirmed as proven. The real resolution is sequencing - see below.

4. The #455 caveat holds up

  • One constant, two call sites: session_command.js:37EPHEMERAL_NOTE, written at :267 (runMutation, inside if (ignored)) and :298 (writeStatus, ignored branch). Genuinely a hoist of a byte-identical string, so reader and writer cannot drift.
  • status's existing text is not broken. The restart half survives verbatim (a gateway restart drops it), and the full suite is unchanged at the 8-failure baseline - no existing assertion regressed.
  • unignore stays silent. The write is guarded by if (ignored) at :266, and the test asserts doesNotMatch(/in-memory only/).
  • --json untouched. Both EPHEMERAL_NOTE writes sit after the if (parsed.json) { ... return 0 } early return (:246), and writeStatus returns early on json too. No EPHEMERAL_NOTE reference anywhere in a json path.
  • Wording is true under both master and hyp session: take Codex's stated CODEX_THREAD_ID over the rollout mtime guess (#442 A, D) #450. It attributes the new id to a fork (claude --fork-session, codex fork), not to resume. It does not touch llp/0066 §readable, which hyp session: take Codex's stated CODEX_THREAD_ID over the rollout mtime guess (#442 A, D) #450 refines; it adds to R9 at llp/0066:205. Compatible.

5. Test teeth: verified, and the test kills 3 mutants

Claim verified literally. The PR's test file run against origin/master's source:

not ok 19 - the ephemerality caveat names the fork that mints a new session id, not only a restart
The input did not match the regular expression /mints a new session id it no longer covers/

and ok 19 at fa7c3bc. Mutation testing on test/plugins/ai-gateway-session-status.test.js:271:

mutationresult
revert EPHEMERAL_NOTE to the restart-only wordingnot ok 19
writeStatus stops printing the caveat (status half)not ok 19
drop the if (ignored) guard so unignore prints itnot ok 19

All three assertions carry weight.

6. Style / refs

  • 0 em dashes added, in the PR's diff and in mine.
  • No semicolons added to JS (the one ; in the diff is inside a comment).
  • All new/changed anchors resolve: 0066#readable (15 refs), 0067#cli-session-id (3), 0086#endpoint-discovery (2), and after my fix 0066#control-path (12), 0066#ephemeral (7). Corpus BROKEN unchanged at 69 pre-existing.
  • The three glossless-@ref fixes in ai-gateway/src/types.d.ts are real (5 glossless on master -> 2 at fa7c3bc), but the body's "it and its two glossless neighbours" reads as exhaustive and 2 remained at :124 and :158. Fixed; that file is now 0 glossless.

7. What I pushed - fa7c3bc -> 94fde3f

fixfileverified
|| true so the default-port fallback is reachablecodex/skills/hypaware-privacy/SKILL.md:92-975-case fixture harness, case A now exits 0
grain disclosure for CODEX_THREAD_IDsame, :22-24prose
exact slug rule + measured attribution + {#} caveat.claude/skills/ref-check/SKILL.md §3718/718 ids vs GitHub API
2 remaining glossless @refsai-gateway/src/types.d.ts:124,1580 glossless in file

npm test 2845/8/1 (baseline), typecheck clean, ref-check 69 (unchanged), LLP 0086 0 broken. fa7c3bc is still an ancestor. Did not touch session_command.js, llp/0066, llp/0067, or the resolution algorithm - #450 and #458 own those.

8. Merge order across #450, #456, #458

The body's "merge #450 first" is right but incomplete, and checking it against #458 changes the answer for this PR.

#450 -> #458 -> #456.

Conflicts to expect: all three touch session_command.js, types.d.ts, llp/0066, llp/0067 and test/plugins/ai-gateway-session-status.test.js; #456 and #458 both touch codex/skills/hypaware-privacy/SKILL.md, which the body notes only as "#453 is in flight and also touches this file" without flagging that it is the same Step 1 block. #456's EPHEMERAL_NOTE hoist and R9 addition are hunk-disjoint from both and should replay cleanly.

If a human prefers to land #456 before #458, then the CODEX_THREAD_ID preference should be dropped from the skill first and only the cwd-match/refusal improvements kept, so no release ships a privacy step that can print confirmed off a thread id.

Findings by severity

  • HIGH - CODEX_THREAD_ID is a thread id POSTed as the session container; can print opt-out confirmed while recording continues in a subagent tool call. Not code-fixed (owned by hyp session ignore names the session container Codex drops on, not a thread id (#453) #458); disclosed in the skill, and drives the merge order.
  • MEDIUM - pipefail aborted Step 1 silently on the happy path, making the documented default-port fallback dead code. Fixed.
  • MEDIUM - skill and verb already disagree on the primary resolution source while the PR newly points users at hyp session status. Resolved by ordering hyp session: take Codex's stated CODEX_THREAD_ID over the rollout mtime guess (#442 A, D) #450 first; body should say why.
  • LOW - PR body and §3 misattribute 269 (the inline-<a id> count) to whitespace collapsing; real split is 498 forms / 30 whitespace. Fixed in §3; body left alone per instruction.
  • LOW - §3 did not say to strip anchor tags from heading lines before slugifying (9 headings), nor that {#...} is not honored by GitHub. Fixed.
  • LOW - 2 glossless @refs remained in the file the PR was de-glossing. Fixed.
  • Pre-existing, not fixed (agreeing with the body's own list): 11 LLP 0103#cli, 11 placeholder refs in the two skill docs, 1 line-wrapped quote in llp/0135, 4 superseded-LLP warnings, 3 duplicate LLP numbers. All want their own issues.

No host state mutated: no hyp subcommand run, no ~/.claude/settings.json, ~/.codex/config.toml, or ~/.config/systemd/ touched. Fixtures were temp dirs with a fake CODEX_HOME and a stub localhost responder.

… 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

Copy link
Copy Markdown
ContributorAuthor

Review round 2 of 2 (final) - head 94fde3f

Round 1 (fa7c3bc, comment) was rigorous and its self-corrections hold. I re-derived its measurements independently rather than reading them, found two things it got wrong, and fixed one thing it decided not to fix. Pushed as 79d147c.

Baseline at 94fde3f: npm test 2845 pass / 8 fail (all test/core/leave-command.test.js, the known baseline) / 1 skipped; npm run typecheck clean.


1. Re-verification of round 1's §3 work

The slug rule is right, and now verified over the whole corpus

I reimplemented §3.1 literally as written and diffed it id-for-id against GitHub's renderer (POST /markdown, live), over all 132 files under llp/ including tombstones/.

headings rendered1098
ids emitted by GitHub1107
ids predicted by §3.1 as written1107
mismatches0

Both of round 1's narrowings are confirmed necessary, not defensive:

variant of the stated rulemismatched ids
strip <a ...> / </a> only (what §3.1 says)0
strip all inline HTML (round 1's first wording)4 - llp/0058...-login-target, llp/0075...-skills_instructions-..., llp/0111 x2
strip nothing9 - the nine <a id>-bearing headings, all in llp/0054

So round 1's account of its own error is accurate: it broke exactly 4, narrowed the wording, and the narrowed wording is exact. I also spot-checked the three worked examples in §3 against the API directly; all three render as claimed, including part-1--the-client-seam with the double hyphen and {#surface} emitting id="the-command-surface-surface" with no surface id at all.

FINDING (MEDIUM, fixed): §3 form 2 says {#slug} is a heading anchor. In this corpus it usually is not.

.claude/skills/ref-check/SKILL.md:86 describes form 2 as "Explicit heading anchors, ## The command surface {#surface}". The corpus's dominant placement is a list item, one bullet per named proposition:

llp/0101-first-sync-review-window.decision.md:43:- **Deadline rule** {#deadline}: the next local 11:59pm; if that is less than
llp/0105-query-seam-local-only-visibility.decision.md:58:- **Explicit override** {#override}: `--include-local-only` on the query
llp/0107-skills-ride-attach.decision.md 5 markers: every-attach, currency, consent, reversal, gating
llp/0138-client-assets-one-install.decision.md 7 markers: one-materializer, one-command, attach-installs, currency, failure-is-not-fatal, marker-undo, refusal-is-not-failure

25 markers across 8 docs (0001, 0049, 0101, 0105, 0106, 0107, 0122, 0138). A checker that follows §3 literally and scans heading lines only reports 47 resolvable references as BROKEN - LLP 0105#override (4), LLP 0107#reversal (4), LLP 0107#every-attach (4), LLP 0122#session-end-part (4), LLP 0138#one-materializer (4), LLP 0101#hold / #deadline / #no-release (3 each), and so on. That is the same false-positive class this PR exists to eliminate, in the sentence that was rewritten to eliminate it.

It also means the 69-broken baseline is mostly noise. With {#...} indexed on any line:

§3 as written at 94fde3f : 69 broken
+ {#slug} on any line : 22 broken (47 resolved)

The 22 genuinely dangling: 15x LLP 0103#cli, 6 placeholder examples in ref-check/ref-story SKILL.md, 1 line-wrap artefact at llp/0135:959. #457's count wants correcting from 69 to 22 - flagging for that issue rather than re-filing.

Fixed: §3 form 2 now carries two sub-bullets, the first saying the marker is not confined to heading lines, naming the list-item form and the eight docs, and stating the 47. llp/0001's amendment updated to match.

FINDING (LOW, fixed): the attribution numbers are understated, and the heading count is wrong

Measured on the tree as pushed (1355 refs, 353 files, 129 LLP numbers):

claim in §3 at 94fde3fmeasured
heading-slugs-only breaks 498632
of which {#...}: 229366
of which inline <a id>: 269266 (274 references target one; 8 of those also match a heading slug)
whitespace collapsing adds 3030 - exact
"over every heading in llp/ (709 headings, 718 ids)"1098 headings, 1107 ids

The two errors share one cause: round 1 measured {#...} as a heading suffix, which is the gap above. 269 is defensible (it sits inside 266-274). 709/718 looks like a count of distinct heading texts (706 unique texts, 714 unique ids in llp/*.md) rather than every heading, so the phrase "over every heading" overstated the sample. The conclusion survives, because my run covers 100% of the corpus and still finds 0 mismatches.

Fixed with the measured figures, plus the number a clean run should reproduce (22).

@ref LLP 0086#*: 10 references, 0 broken - round 1's central correction of #452(b)'s premise confirmed. Duplicate numbers 0098 / 0099 / 0111 confirmed. All 8 @refs this PR adds resolve; none appear in the broken set.


2. The Step 1 snippet, run end to end

Extracted the first fenced bash block verbatim from the head SKILL.md and ran it as an operator would, against fixture CODEX_HOME trees and a stand-in gateway that logs every request, with curl stubbed for the two default-port cases so nothing could reach a real gateway.

At 94fde3f, 11 of 11 cases behave as documented:

caseresult
A stated id + OPENAI_BASE_URLopt-out confirmed, 1 gateway call carrying that id
B base_url from config.tomlhonoured, 1 call
C no config.tomlreaches http://127.0.0.1:8787/_hypaware/ignore/session
D stanza present, no base_urlreaches the same default
E zero cwd matchesexit 1, no Codex rollout ... records cwd, 0 gateway calls
F two cwd matchesexit 1, 2 rollouts record cwd ...: ambiguous, 0 calls
G single match, 90m oldexit 1, FINISHED session rather than this one, 0 calls
H one fresh matchresolves, labelled INFERRED from rollout-...jsonl on disk
I ignored:falseexit 1, opt-out NOT confirmed
K HTTP 500exit 22, no confirmed printed
L unparsable rollout beside a good oneskipped, not fatal

Round 1's || true fix is confirmed correct and confirmed necessary. C and D are the cases that aborted before it; both now reach the documented default. It does not swallow anything that changes the outcome: with config.toml at mode 000 the read error is masked and the default port used, which then fails loudly at curl if the real gateway is elsewhere, so no path silently targets the wrong endpoint. No refusal path reaches the gateway, and every refusal precedes the curl in the script.


3. Verdict on the HIGH: disclosure is not sufficient. Dropped the thread id.

Round 1 disclosed the CODEX_THREAD_ID grain in prose and left the code path, reasoning that #458 supersedes the block. I disagree, on four counts:

  1. The prose is not what runs. A skill is executed by an agent. The bash block prints opt-out confirmed, and the caveat above it offers no verb the agent can run to discover otherwise. This is exactly the standard hyp session ignore's ephemerality caveat names a gateway restart but not a resume/fork session-id change #455 sets in this same PR: readability is necessary but not sufficient, "the user has to know there is something to re-check" (llp/0066-session-opt-out.spec.md R9, added here).
  2. The verification cannot catch it, by construction.ai-gateway/src/control.js:112-113 does ignoredSessions.add(sessionId); ignored = true unconditionally on POST, and echoes the token verbatim. ignored: true is therefore a receipt for "the gateway received a string", not a verification. Step 1's stated contract is "opt this Codex session out of capture and verify it took effect"; against a thread id it satisfies the letter and fails the purpose.
  3. The consequence is the worst-ordered one available. The false confirmed prints at the top of a flow whose next step surveys the machine's most sensitive content. codex/src/exchange-projector.js:98 keys the drop on metadata.session_id ?? conversationId, so recording continues throughout.
  4. The block is doomed text either way.hyp session ignore names the session container Codex drops on, not a thread id (#453) #458's rewrite of this same Step 1 contains no CODEX_THREAD_ID. Whether Codex session identity is the rollout's session container, not CODEX_THREAD_ID, and the ignore caveat names the fork (#452, #455) #456 lands before or after, that text does not survive. Keeping the risky form buys interim exposure and nothing else.

A fifth point decided the shape of the fix: the disk fallback had the same defect, and the disclosure did not cover it. The prose at line 26 already knew about payload.session_id, calling it "the container", yet the script collected matches.append((payload['id'], ...)) - the thread. Run inside a subagent whose rollout matches cwd, the fallback resolves the subagent's own thread id: same false confirmed, no CODEX_THREAD_ID involved.

Action (hypaware-core/plugins-workspace/codex/skills/hypaware-privacy/SKILL.md):

Re-ran the harness on the fixed block: 12 of 12 pass, including the case that matters -

=== N2. SUBAGENT: container != thread -> the CONTAINER must be sent ===
PASS N2 container sent even with CODEX_THREAD_ID exported
PASS N2 thread id never reached the gateway
gateway body: {"url":"/_hypaware/ignore/session","body":"{\"session_id\":\"CONTAINER-uuid\"}"}
=== N3. rollout predates the container (no session_id) -> REFUSE ===
PASS N3 no container refuses, gateway untouched
thread id never sent

CODEX_THREAD_ID=SUBAGENT-thread is exported and diverges from the container; the container is what goes on the wire and the thread id never does. The default-port fallback (N7) and the config.toml path (N7b) still work, so round 1's fix survived the rewrite.

New: test/plugins/codex-privacy-skill-session-id.test.js

The PR body concedes that "#452(a) and #452(b) are pure documentation" with no ground-truth gate. The grain rule is one regex away from silent reversal inside a privacy-critical block, so it now has one, in the manner of test/plugins/query-skill-description.test.js. It fails on each of five mutations:

mutationcaught by
revert to 94fde3f's SESSION_ID="${CODEX_THREAD_ID:-}"test 1
collect payload['id'] as the session idtest 1
substitute the thread id when no container is recordedtest 1
restore newest-by-mtime (drop the cwd filter)test 2
drop the staleness refusaltest 2

4. #455 still intact at this head

  • One constant, both paths.EPHEMERAL_NOTE at session_command.js:36, read by the writer at :267 and the reader at :298. No third copy (git grep 'in-memory only' finds only the constant plus the two privacy skills' own prose).
  • Restart half survives, and the fork half names claude --fork-session and codex fork.
  • unignore silent: the write sits inside if (ignored).
  • --json untouched: writeStatus's json branch returns before any prose line (:288-290).
  • The test has teeth.test/plugins/ai-gateway-session-status.test.js:271 fails on all three mutations I tried: reverting the constant to master's text, letting the reader drift to a hardcoded restart-only copy while the writer keeps the constant, and making unignore print the note. A real regression gate, not a tautology.
  • LLP 0066 R9 and LLP 0067's annotation-map row match the code.

5. House style

No em dash on any line this PR adds (checked over the full 74aea66..79d147c range). No stray semicolons in added JS. Types in JSDoc, @import at the top, no inline import('...') types. All 8 @refs the PR adds resolve; the 2 added this round (LLP 0066#enforcement via a {#} marker, LLP 0030#decision via a heading slug) resolve through two different anchor forms, which is a small live test of the corrected §3.


Findings by severity

sevfindingstate
HIGHStep 1 sent a thread id (CODEX_THREAD_ID, and payload['id'] in the fallback) where the drop keys the container; control.js answers ignored: true for any token, so it printed opt-out confirmed while the review session kept recordingfixed in 79d147c
MEDIUM§3 form 2 restricted {#slug} to heading lines; the corpus puts 25 of them on list items, breaking 47 resolvable refs and distorting the #457 baseline (69 -> 22)fixed
LOW§3's attribution numbers understated (498/229 -> 632/366) and its heading count wrong (709/718 -> 1098/1107)fixed
LOW#452(a) and the grain rule had no ground-truth gatefixed (new test, 5 mutations caught)
INFOBoth privacy skills hard-depend on python3, twice each, with no fallback and no version note. Step 1 is the first thing a newly enrolled machine runs, and on a box without python3 it aborts there (I had to provision a standalone CPython to run it at all). Pre-existing - master's block uses it too - and out of scopetriage
INFOcontrol.js:112 makes the POST answer unconditional, so "verify it took effect" is a receipt. The GET at :82 is a genuine membership read; a GET after the POST would make the step's own claim true. Belongs with #435 / #453triage
INFOThe snippet prints the gateway's echoed session_id with no comparison against the one it sent. Unreachable against this gateway (it echoes verbatim) but the printed line can name a different session if anything sits in betweentriage
INFO|| true masks an unreadable config.toml (a fixture at mode 000 falls to the default port silently). Degrades to a loud curl failure rather than a wrong target, so not worth complicatingaccepted
INFOCODEX_THREAD_ID appears nowhere else in the repo - no resolver, no test, no LLP - so no surface validates the assumption. #450 introduces it in the verb and is the right place to record ittriage

Pre-existing, already tracked, not re-filed: #457 (the @ref breakage, though its count should read 22 rather than 69), #459 (rollout-cwd.js subagent cwd), #435 (collapse the skills onto the verb), #453 (the grain in the verb, implemented by #458).

Merge order

Round 1 concluded #450 -> #458 -> #456. Sanity-checked against the branches, and it holds:

  1. hyp session: take Codex's stated CODEX_THREAD_ID over the rollout mtime guess (#442 A, D) #450 (fix/issue-442, open, not draft) takes CODEX_THREAD_ID over the rollout mtime guess in the verb.
  2. hyp session ignore names the session container Codex drops on, not a thread id (#453) #458 (fix/issue-453, open, draft) corrects that grain to the session container. This ordering is required, not merely convenient: land hyp session ignore names the session container Codex drops on, not a thread id (#453) #458 first and hyp session: take Codex's stated CODEX_THREAD_ID over the rollout mtime guess (#442 A, D) #450 re-introduces the thread id in session_command.js behind it.
  3. Codex session identity is the rollout's session container, not CODEX_THREAD_ID, and the ignore caveat names the fork (#452, #455) #456 (this PR) lands last. It shares session_command.js, types.d.ts, llp/0066, llp/0067 and test/plugins/ai-gateway-session-status.test.js with both, and the codex hypaware-privacy Step 1 with hyp session ignore names the session container Codex drops on, not a thread id (#453) #458.

The #456 <-> #458 conflict is smaller after this round than before it, because both now resolve the id from payload.session_id and refuse when a rollout predates the container. What remains is a genuine merge rather than a pick: #458 supplies the framing (prefer hyp session ignore --json), #456 supplies the resolution (cwd match plus refusal on zero / ambiguous / stale, replacing #458's surviving newest-by-mtime). Rebase #456's Step 1 onto #458's and keep both halves - a resolution that keeps #458's ls -t fallback loses #452(a), and one that drops #458's verb-first lead-in loses #453's.

Rebasing against #450/#458 in session_command.js stays mechanical and disjoint, as the PR body describes: they rewrite resolveSessionIdForCli and SessionIdResolution, this adds EPHEMERAL_NOTE near the top and changes two ctx.stdout.write call sites. LLP 0066 is disjoint by hunk (R9 here, §readable/R10 there) and LLP 0067 likewise (annotation map here, §cli-session-id and the test plan there).

Not merged, not marked ready. No hyp subcommand was run and no host state was touched; every probe used a fixture CODEX_HOME and a stand-in gateway. Review cap reached (2 of 2); everything marked triage needs a human.

@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

Triage verdict: SHIP (non-blocking residuals only)

Re-verified independently in a detached worktree off origin/master
(git worktree add --detach, PR ref fetched and checked out on top,
node_modules symlinked in - no state-mutating hyp command run anywhere,
/work/hypaware never touched). Merge-base with origin/master is 74aea66;
npm test at head 79d147c: 2848 pass / 8 fail / 1 skipped, all 8 the
pre-existing test/core/leave-command.test.js baseline (confirmed by name -
leave after join removes the seed and reports the server, leave clears an applied central slot..., etc.). The new
test/plugins/codex-privacy-skill-session-id.test.js passes (3/3) on its own.

Both rounds were exceptionally rigorous, and round 2's exhaustive slug-rule
verification (132 files through GitHub's live renderer, id-for-id) and its
reversal of its own R1 finding on the CODEX_THREAD_ID grain are the kind of
work this triage exists to reward, not second-guess. This triage's job was
the two residuals round 2 flagged but did not resolve.

T1 - python3 hard dependency: PREFERENCE, already tracked, not this PR's problem

Checked the diff, not just the current file: git diff <merge-base>..79d147c
on hypaware-core/plugins-workspace/codex/skills/hypaware-privacy/SKILL.md
shows python3 was already a hard dependency of this exact skill on
master, in two places - the pre-existing resolve step (head -1 "$rollout" | python3 -c '...') and the pre-existing verify step. This PR extends the
existing resolve-step script (more logic, same interpreter); it does not
introduce a new dependency. It's also not isolated: claude/skills/hypaware- ignore/SKILL.md and the Claude hypaware-privacy skill's verify step have
the identical python3 -c dependency today, and no in-repo skill offers a
jq-or-similar fallback anywhere. So this is a real, repo-wide condition of
the embedded-bash-skill design, not something #456 makes worse.

It's also already tracked precisely: issue #435's acceptance criteria state
"Both Claude skills are <10 lines and contain no curl, no python3, no port
literal" - collapsing onto hyp session (a compiled verb) is the fix, and it
explicitly targets removing python3. No new issue needed for T1.

Verdict: not a blocker. A privacy skill that hard-fails on a python3-less
machine is worth fixing, but it's a pre-existing, already-tracked condition,
and blocking this PR on it would hold back a real correctness fix
(cwd-match-with-refusal replacing newest-by-mtime) for an unrelated
packaging gap.

T2 - the POST receipt: real, structural, and does warrant its own tracking - filed as #460

Confirmed at the code level: control.js:112-113 is exactly
ignoredSessions.add(sessionId); ignored = true, unconditional on POST.
Traced the consumer too - source.js:114,
isSessionIgnored: (id) => state.ignoredSessions.has(id) - the drop is a bare
Set-membership check against whatever id the exchange projector resolves. So
the round-2 characterization is correct: the response proves the token got
added to a Set, not that it's the token this session's exchanges will carry.

One correction to round 2's own suggested remedy, worth recording: a GET
after the POST does not fix this.
control.js's GET handler (line 82)
performs the identical ignoredSessions.has(raw) check. Calling it
immediately with the same token you just POSTed is tautological - of course
it returns true, you just added it. GET is a different HTTP verb over the
same read, not additional verification. The actual gap has no fix available
in the current design: there's no oracle for "is this the id my traffic
carries" short of running an exchange through and checking the drop, which
GET/POST semantics alone can't provide.

Checked whether this is already covered elsewhere before filing:

None of the three actually captures this, so I filed #460
("hyp session ignore's POST receipt cannot prove the id matches live
traffic - it's Set membership, not drop verification"), folding in T3 (the
missing echo check in the skill's shell path) as a related, smaller item in
the same issue rather than a fourth ticket.

Verdict: not a blocker for #456. The gap is pre-existing (control.js is
untouched by this PR's diff against merge-base 74aea66) and architectural -
closing it is a design decision (per #451's own precedent for this class of
problem), not a fix this PR should absorb. Blocking on it would hold back a
real win (refusing rather than confidently resolving the wrong session) to
demand a proof-of-drop mechanism that doesn't exist anywhere in this control
plane yet. #452(a)'s own framing already discloses the bound honestly in the
skill's comments; #460 exists so that honesty doesn't quietly disappear when
this PR closes #452.

T4 (CODEX_THREAD_ID now unused elsewhere in the repo): confirmed by
grep, but this is inherent to round 2 correctly dropping it from the script
for the reason above, not a loose end - no action needed.

Classification

PREFERENCE, all four residuals. Bar applied: could either surviving item
cause a production defect - wrong behavior, data loss, a privacy hole, a
crash, a perf regression? No. T1 is a pre-existing packaging condition
already scheduled for removal by #435's own acceptance criteria. T2/T3 are a
pre-existing, structural proof-of-drop gap that #452(a) discloses rather than
hides, and closing it requires a design decision out of scope for a
docs/skill-correction PR. The PR's actual production-code change (#455's
EPHEMERAL_NOTE hoist) is small, tested both directions, and correctly
scoped. Round 2's own ground-truth gate (5 mutation-killing tests) is real
verification, not a description of one.

Stale PR-body detail corrected

The body's #452(a) section said "Step 1 now prefers CODEX_THREAD_ID and
demotes the disk scan to the fallback it is" - written before round 2
reversed its own R1 finding and dropped CODEX_THREAD_ID from the script
entirely (it names the thread, not the session container the gateway drops
on, and the two diverge on a subagent thread). Corrected in place, bracketed,
to state what actually ships: Step 1 prefers hyp session ignore first, and
where that can't resolve the session, falls back to the cwd-and-staleness-
refusing disk scan that reads payload.session_id - not CODEX_THREAD_ID -
per round 2. Rest of the body left verbatim.

Merge order (restated per the standing instruction)

#450 -> #458 -> #456, #456 last.

Confirmation

No hyp init/attach/detach/join/leave or other state-mutating hyp
command was run. ~/.claude/settings.json, ~/.codex/config.toml, and
~/.config/systemd/ were not touched. All work happened in a detached
worktree; /work/hypaware was read-only throughout and no git switch
occurred there. gh pr ready and merge were not invoked.

@philcunliffe
philcunliffe marked this pull request as ready for review July 29, 2026 22:58
@philcunliffephilcunliffe added the neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) label Jul 29, 2026
@philcunliffephilcunliffe removed the neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) label Jul 29, 2026
Resolves the one conflict in ai-gateway/src/types.d.ts on SessionIdResolution's
JSDoc: master's #450 prose (which adds the codex_env source alongside argument
and claude_env) is kept, together with this PR's gloss on the
@ref LLP 0067#cli-session-id annotation, which #452(b) added because a glossless
ref is what CLAUDE.md forbids.
Co-Authored-By: Claude <noreply@anthropic.com>
@philcunliffephilcunliffe added the neutral:stuck neutral attempted this but cannot complete it autonomously — needs a human label Jul 30, 2026
@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

triage rung, PR #456 @ 24d7e76

What neutral was doing. The review budget (2 rounds) was already exhausted at
head 79d147c, and that head was triaged SHIP by a prior triage pass (see the
<!-- neutral-triage: 79d147c... #460 --> marker still in the PR body). Since
then a merge commit landed on top of the reviewed head -
24d7e76 = merge of 79d147c (this branch) with c551d6e (origin/master,
which includes PR #450) - to resolve one textual conflict in types.d.ts. No
review or triage has ever seen 24d7e76, and the rung ladder will not route it
to review again, so this triage is the only gate this head passes through. Per
instructions I judged the whole head, not just the old findings, and re-verified
every claim the merge commit message makes from the tree rather than trusting it.

The merge commit's own claims - verified, all correct:

  • types.d.ts conflict resolution: kept master's codex_env addition to
    SessionIdResolution['source']and this PR's gloss on the
    @ref LLP 0067#cli-session-id annotation. Confirmed both survive
    (hypaware-core/plugins-workspace/ai-gateway/src/types.d.ts:174-183).
  • EPHEMERAL_NOTE hoist: one constant (session_command.js:37), read at both
    call sites - runMutation (:297) and writeStatus (:328). Confirmed.
  • PR Trim the skill set; move what the skills summarized into the CLI #436's privacy-skill edits (description/intro/Step 2/"After the review")
    are hunk-disjoint from this PR's Step 1 rewrite and both survive, in both
    claude/skills/hypaware-privacy/SKILL.md and
    codex/skills/hypaware-privacy/SKILL.md. Confirmed by diffing each PR's
    hunks against the merged file.
  • The two texts the merge commit says should agree - session_command.js's
    STATED_SESSION_ID_VARS comment (:98-105) and the Codex privacy skill's
    Step 1 prose - do agree: both correctly say CODEX_THREAD_ID is the
    thread, coincides with the session container on a root thread, and
    diverges on a subagent thread, and both say the gateway's drop keys on the
    container. Cross-checked against the actual drop code,
    exchange-projector.js:98,148, which does key on the container-or-thread-
    fallback value, not the thread directly. No textual contradiction here.
  • #452(b)'s premise correction: verified independently. All 10
    @ref LLP 0086#* annotations in the tree resolve against the 4 inline
    <a id> anchors at llp/0086-attach-tracks-ephemeral-port.decision.md:48,78,89,100.
    The issue's git grep '{#' only checked the other anchor spelling. The
    ref-check skill fix in this PR (.claude/skills/ref-check/SKILL.md §3) is
    real and does what it claims.

Why it cannot proceed - one true blocker the merge itself creates:

The merge brought origin/master (including PR #450, already merged) into
this branch. #450 added Codex's CODEX_THREAD_ID as a stated session-id
source to resolveSessionIdForCli (session_command.js:112-115,
STATED_SESSION_ID_VARS), used directly, with no cwd/staleness check, the
instant it is set (:494-513, confirmed by this PR's own merged test,
test/plugins/ai-gateway-session-status.test.js:361 "CODEX_THREAD_ID beats the
disk scan: a stated id is not an inference"). This is exactly the
thread-vs-container confusion this PR's own round 2 spent its HIGH finding
eliminating from the Codex privacy skill's disk-scan fallback - except #450
put the un-fixed version of the same bug into the CLI verb,
hyp session ignore / status.

Separately, and merged in the same commit, this PR's own Step 1
(hypaware-core/plugins-workspace/codex/skills/hypaware-privacy/SKILL.md:26)
now reads:

"Prefer hyp session ignore, which resolves the id and verifies the opt-out
in one tested implementation and refuses rather than guessing. Only where it
cannot resolve the session does the script below apply."

That sentence was written by round 2 anticipating the merge order the PR body
and both review rounds explicitly require - "#450 -> #458 -> #456" - where
#458 (fix/issue-453, still OPEN, not merged) corrects #450's grain before
#456 ships. Round 2 said this in so many words: "land #458 first and #450
re-introduces the thread id in session_command.js behind it"
(round-2
comment, "Merge order", point 2) and "if a human prefers to land #456 before
#458, the CODEX_THREAD_ID preference should be dropped from the skill first
... so no release ships a privacy step that can print confirmed off a thread
id"
(round 1). That is exactly what this merge commit ships.#450 landed,
#458 did not, and the merge carried both #450's un-fixed verb and this PR's
"prefer the verb" text into the same tree.

Consequence, reproduced by static trace (no hyp command run): a Codex
subagent thread runs hyp session ignore (directly, or by following Step
1's own "prefer" instruction) - CODEX_THREAD_ID is set in its shell
environment to the subagent's own thread id, which differs from the session
container the gateway's drop keys on
(exchange-projector.js:98, sessionId = codexContext?.session_id ?? conversationId). resolveSessionIdForCli takes the stated CODEX_THREAD_ID
directly (no refusal path - stated ids skip the cwd/staleness checks that
protect the disk-scan fallback). The control route accepts any token
unconditionally (control.js:112-113) and echoes it back;
validateControlResponse (:703-721) only checks the echo matches what was
sent, which it always will, since the route is a bare Set.add + echo. Result:
hyp session ignore prints session <thread-id>: ignored - the gateway will drop this session, plus EPHEMERAL_NOTE, while the session actually being
recorded (keyed on the container) is untouched. hyp session status, which
Step 1's own caveat and EPHEMERAL_NOTE tell the user to "re-check" with,
resolves the same wrong id the same way and reports the same false
ignored, so the recommended re-check does not catch it either.

This is a privacy skill telling the user/agent their session is opted out of
capture when, for a real and Codex-native class of session (a subagent tool
call), it is not - the exact "wrong instructions on a privacy control" bar
this triage was told to treat as a true blocker, in the one PR whose entire
point is fixing this class of defect. It is not hypothetical or pre-existing
in the sense of being already accepted: the prior triage at 79d147c (see
#460) never saw it, because hyp session ignore did not use
CODEX_THREAD_ID at all on this branch before the merge (confirmed:
STATED_SESSION_ID_VARS and the codex_env source do not exist at 79d147c,
only CLAUDE_CODE_SESSION_ID was a stated var there). The merge is what
introduced this exact hazard into the tree, and both review rounds on record
say it should not ship in this order.

CI is green at this head and stays the authority for what it checks; it does
not and cannot check this - it is a semantic/privacy-correctness question,
which is why this triage exists as a gate.

Non-blockers, for completeness (all PREFERENCE, unaffected by the merge):

What is needed from you. A human call on how to resequence, since neutral
cannot reorder already-merged PRs or force #458 to land:

  1. Merge hyp session ignore names the session container Codex drops on, not a thread id (#453) #458 (fix/issue-453) into this branch first, then let this PR's
    Step 1 (which already resolves via payload.session_id in its own fallback,
    per round 2) inherit a verb that resolves the same way, and re-triage. This
    matches the merge order both review rounds already settled on.
  2. Or, keep this branch's current merge but edit Step 1 to drop or heavily
    caveat the "Prefer hyp session ignore" line until hyp session ignore names the session container Codex drops on, not a thread id (#453) #458 lands, so the skill
    does not claim a guarantee the verb does not currently hold - then
    re-triage.
  3. Or, tell neutral the risk is acceptable to ship now (e.g., if subagent
    Codex sessions running this skill are believed rare or already otherwise
    mitigated) and it will not re-raise this specific finding at this head.

How to unstick: reply with a comment on this PR (or push to the branch);
neutral monitors this thread and will re-engage with your guidance on its next
tick.

@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

let's give PRs 456 and 462 2 more review rounds each

— relayed from Slack

@philcunliffephilcunliffe removed the neutral:stuck neutral attempted this but cannot complete it autonomously — needs a human label Jul 30, 2026
@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

Re-engaging with your guidance - taking the two additional review rounds you granted for this PR, resuming at the current head rather than triaging the residual findings.

testand others added 2 commits July 30, 2026 22:56
Three files conflicted. Each resolution is a union of both intents, not a
side-picking, because the two sides changed different things about the same
subject (Codex session identity and the opt-out caveat).
session_command.js: keep this branch's EPHEMERAL_NOTE constant (#455: the
caveat names the fork as well as the restart) and master's object-argument
provenanceNotes({ idSource, idEvidence, threadId, endpointSource }) call. Only
one of the two call sites conflicted; the other already carried the combination,
so this makes the two branches of the if/else identical again.
types.d.ts: keep master's prose, which is the correct one now - it explains
sessionId as the session container versus threadId, and the merged union already
carries master's rename of codex_env to codex_env_rollout, which this branch's
prose still called codex_env. Re-attached this branch's gloss on the
@ref LLP 0067#cli-session-id annotation, which master had left bare. Anchor
{#cli-session-id} verified present at llp/0067:305.
codex privacy skill SKILL.md: this branch's rollout-selection policy wins (match
payload.cwd, refuse on zero/ambiguous/stale, never newest-by-mtime - issue #452),
since master's side still said "pick the newest rollout". Folded master's
readRolloutMeta-mirroring guards into that same walk rather than dropping them:
the session_meta type check and payload-dict check (as skips, since this side
walks many files), plus the non-string and whitespace-in-id refusals, which this
side needs too because it reads the result through `read -r`. Took master's
`hyp session ignore --json` over the bare verb.
Semantics checked, not just textual cleanliness:
- The claim "the gateway's drop keys on the container" is STILL TRUE after
PR #477. exchange-projector.js line 113 is unchanged:
`const sessionId = stringValue(codexContext?.session_id) ?? conversationId`,
i.e. metadata.session_id falling back to the conversation (thread) id, exactly
as the skill prose and the test comment describe. #477 (bcad4a4) only changed
the separate .hypignore gate, which keys on a cwd path, not on any session id.
So no correction was needed there.
- The three-way agreement still holds between the CODEX_THREAD_ENV comment in
session_command.js (master renamed it from STATED_SESSION_ID_VARS), the skill's
Step 1 prose, and the drop code: thread id is a selector, the container is the
answer.
- Corrected one thing that HAD gone stale: this branch's prose said issue #453
"puts CODEX_THREAD_ID to its real use" in the future tense, but #453 shipped on
master (5d270a5, c551d6e). The prose now describes the codex_env_rollout source
as live, matching llp/0067:317 ("selector, not an answer") and llp/0067:585.
- The fork caveat agrees across all three surfaces it appears on: EPHEMERAL_NOTE,
the Codex skill, and the Claude skill (`claude --fork-session` / `codex fork`).
The usage-policy unification (#482/#484, fold(realpath(p))) touches no file on
this branch and needed no reconciliation here.
Checks: npm test 3041 tests, 8 failures, all of them the pre-existing
test/core/leave-command.test.js "leave ..." set, name-for-name identical to a
pristine origin/master run (3037 tests, same 8). Test count rose by 4, which is
this branch's new tests passing. npm run typecheck clean.
Co-Authored-By: Claude <noreply@anthropic.com>
…d, 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>
@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

review rung, PR #456 @ dc79fc5 - round 3 of 4

Read this first, future orchestrator. The CLI's review budget is 2 rounds and it was spent at 94fde3f. This round runs past that cap only because the maintainer explicitly granted two more review rounds on this thread:

"let's give PRs 456 and 462 2 more review rounds each"
(comment, relayed from Slack, 2026-07-30)

So this is round 3 of 4, and one granted round remains after this one. The grant lives in this thread and nowhere else, so do not re-derive the cap from the two <!-- neutral-review --> markers alone.

Reviewed in a detached worktree at dc79fc5 with node_modules symlinked from the clone. The code-review skill is not installed in this clone and codex is not on PATH, so this is a direct audit. python3 is also absent from this container, so I provisioned a standalone CPython 3.11.13 in order to execute the privacy skill's Step 1 block rather than only read it.

dc79fc5 is the merge of 24d7e76 with 73b4618 (origin/master) and no review had seen it. Outcome: findings - 2 LOW, both fixed and pushed as 8e345cd. Nothing blocking. The blocker the 24d7e76 stuck report raised is resolved by master, not worked around; details in section 4.


1. Conflict resolution 1 of 3: session_command.js - correct

Verdict: correct union, and the symmetry claim is true. (executed)

git diff origin/master...dc79fc5 on this file is exactly three hunks: the EPHEMERAL_NOTE constant plus its two ctx.stdout.write substitutions. Master's object-argument provenanceNotes({ idSource, idEvidence, threadId, endpointSource }) is context, not a changed line, in every one of the three places it appears, so master's side arrived intact.

Both call sites checked for the symmetry the merge commit claims:

siteEPHEMERAL_NOTEprovenanceNotes({...}) object arg
runMutation:288 / :294 (inside if (ignored))yesyes, 4 fields
writeStatus:322 / :326 (ignored branch)yesyes, same 4 fields
writeStatus:334 (not_ignored branch)n/a, correctly absentyes, same 4 fields, byte-identical block

The ignored and not_ignored branches of writeStatus now carry the same provenanceNotes call verbatim, which is what "restored if/else symmetry" has to mean here. The unknown branch correctly carries neither. --json paths still return before any prose line (:246 in runMutation, :316 in writeStatus), so no EPHEMERAL_NOTE reaches a JSON consumer. (executed: read at head, diffed against both parents.)

2. Conflict resolution 2 of 3: types.d.ts - correct, and the staleness call was right

Verdict: both intents survive, and taking master's prose was the correct side-pick. (executed)

I diffed all three trees at the SessionIdResolution block:

  • Branch (24d7e76) prose: "an argument, claude_env or codex_env id is what the caller or the client that spawned this process stated", with source: 'argument' | 'claude_env' | 'codex_env' | 'codex_rollout' and no threadId field.
  • Master prose: names codex_env_rollout, adds the whole "sessionId is always the session container ... threadId is carried separately" paragraph, and the union is 'argument' | 'claude_env' | 'codex_rollout' | 'codex_env_rollout'with threadId?: string.
  • Head: master's prose and union verbatim, plus the branch's gloss re-attached to @ref LLP 0067#cli-session-id.

So the resolver's reasoning holds literally: the auto-merged union carried codex_env_rollout, and the branch's prose named a source value that no longer exists. Keeping the branch prose would have shipped a doc comment describing a variant the type does not have. Master had left that @ref bare (verified against origin/master); the gloss is back. Anchor {#cli-session-id} confirmed at llp/0067-session-opt-out.design.md:305.

The other @ref glosses this PR adds to the file (0066#control-path, 0066#ephemeral, 0086#endpoint-discovery, 0066#readable) all survived the merge and all resolve. The 5 em dashes in this file are on untouched pre-existing lines; the PR adds none (git diff origin/master...dc79fc5 has no added line containing one, and my own commit adds none either).

3. Conflict resolution 3 of 3: the Codex privacy SKILL.md - correct, and I proved the folded guards in a real shell

This was the genuine semantic conflict, so I did not read it. I extracted the first fenced bash block verbatim and executed it against 20 fixture CODEX_HOME trees with a stub gateway that logs every request, plus a stubbed curl for the default-port cases so nothing could reach anything real. No hyp subcommand run, no host state touched.

Verdict: the branch's policy correctly won, and master's folded guards are correct in a shell context, not merely plausible. (executed)

Master's side really did say "Pick the newest rollout" and used find ... | xargs -0 ls -t | head -1; the head has no ls -t anywhere and resolves by payload.cwd with refusal on zero / ambiguous / stale. #452's thesis survived the merge.

The folded guards, executed

casefixtureresult
N1 root threadid == session_idresolves, POSTs the container
N2 subagentid=SUBAGENT-thread, session_id=CONTAINER-uuid, CODEX_THREAD_ID=SUBAGENT-thread exportedPOSTs CONTAINER-uuid. The thread id never reaches the wire.
G1 session_meta type guardfirst line is a turn_context carrying a matching cwdand an idskipped, refuses "no Codex rollout ... records cwd"
G1b same decoy beside a real headerresolves the real one, and the decoy creates no false ambiguity
G2 payload not a dictskipped
G3 first line is a JSON arrayskipped
G4 session_id non-string (12345)refuses "unusable payload.session_id"
G5 session_id with interior whitespacerefuses
G6 session_id whitespace-onlyrefuses (matches the CLI reader's "blank is absent")
G7 no session_id (legacy)refuses "predates the session container", names the thread id only to identify the file
G9 two cwd matchesrefuses, names both containers
G10 single match 90m oldrefuses "FINISHED session"
G11 zero matches, and missing sessions/ dirrefuses
G12 unparsable rollout beside a good oneskipped, not fatal
C / D endpoint fallbackno config.toml; stanza with no base_urlboth reach http://127.0.0.1:8787, so round 1's `
B base_url in config.tomlhonoured
P python3 absentexit 127, loud, 0 gateway calls

Every refusal made 0 gateway calls. Confirmed by the stub's request log, not by reading the script.

The session_meta type guard is load-bearing, and I proved it by regression. I extracted the same block from the branch side (24d7e76) and ran it on the G1 fixture. It resolved TURNCTX-container off a turn_context record and POSTed it:

resolved session TURNCTX-container (INFERRED from rollout-decoy.jsonl on disk)
opt-out confirmed for session TURNCTX-container (total ignored: 1)

The head refuses the same fixture. So folding master's guard in rather than dropping it closed a real "confident id belonging to no session" path, which is the class src/core/codex/rollout_session_meta.js rule 2 exists to close. (executed)

Guard-for-guard against the CLI reader (src/core/codex/rollout_session_meta.js), which is what "mirrors readRolloutMeta" has to mean: type === 'session_meta' yes; payload-must-be-a-dict yes; non-string session_id refused yes; blank or whitespace-only session_id refused yes (the reader's metaField trim-test equivalent). The script is stricter on one axis, interior whitespace, and correctly explains why: read -r SESSION_ID ROLLOUT would split it. That strictness fails closed and points at hyp session ignore --json, which keeps the token verbatim. Correct. (executed, plus reasoned for the equivalence argument)

FINDING 1 (LOW, fixed in 8e345cd)

hypaware-core/plugins-workspace/codex/skills/hypaware-privacy/SKILL.md:81

% (len(matches), cwd, ', '.join(m[0] orm[1] forminmatches)))

The merge folded in the non-string session_id guard, but that guard runs on matches[0]after the ambiguity branch has already returned. Two rollouts recording this cwd, one with a non-string payload.session_id, therefore hit str.join over an int:

Traceback (most recent call last):
File "<stdin>", line 32, in <module>
TypeError: sequence item 0: expected str instance, int found

It fails closed (exit 1, 0 gateway calls, set -e aborts before the curl), which is why this is LOW and not higher. But the refusal text is the artifact the agent relays to the user in a step whose whole contract is "on failure, say so plainly", and a traceback tells them nothing to act on. Fixed with str(...); the case now prints 2 rollouts record cwd /tmp/... (777, C2): ambiguous, confirm the session id with the user, exit 1, 0 gateway calls. All 20 cases re-run unchanged. (executed)

4. The three-way agreement: it holds (executed)

Re-verified from the tree at this head rather than from the merge commit message.

  1. The drop code.hypaware-core/plugins-workspace/codex/src/exchange-projector.js:113 is const sessionId = stringValue(codexContext?.session_id) ?? conversationId, consumed at :176 by ctx?.isSessionIgnored?.(sessionId). So the drop keys on the container, falling back to the thread. Unchanged by Codex live projector: an explicit cwd outranks a substituted workspace key at the .hypignore gate #477: bcad4a4's four hunks in this file land at old lines 86, 131, 683 and 765, none of which contains line 113, and git log -S on the exact expression returns 30aa737 (ai-gateway: split session_id from conversation_id (schema v6) (#104) #117) as its last change. The resolver's claim checks out exactly.
  2. The CODEX_THREAD_ENV comment, session_command.js:88-99 (the constant master renamed from STATED_SESSION_ID_VARS): "It carries a thread id, which is NOT the key the gateway drops on (that is the session container). So it is used to identify WHICH rollout is live, not as the answer."
  3. The skill's Step 1 prose, SKILL.md:28,30: the same three propositions, in the same grain, plus the root/subagent divergence.

They agree, and so do two surfaces the brief did not name: types.d.ts:176-181 ("the two coincide on a root thread and diverge on a subagent one") and llp/0067:305-320 §cli-session-id ("selector, not an answer"). The verb's runtime behaviour matches the prose: resolveFromStatedThread looks the rollout up by payload.id and returns source: 'codex_env_rollout' carrying the container, and refuses rather than falling back to the cwd scan when the thread's rollout is missing or carries no container.

The 24d7e76 stuck report's blocker is gone, and gone for the right reason. That report was correct at the time: c551d6e (#450) had landed CODEX_THREAD_ID as a stated answer while #458 had not. 5d270a5 ("hyp session ignore names the session container Codex drops on, not a thread id (#453) (#458)") is now on master, so the verb this skill tells the user to prefer resolves the same grain the skill does. The "Prefer hyp session ignore --json" sentence is now true rather than anticipatory. Confirmed by reading the merged verb, not by trusting the commit subject.

5. Residuals the prior triage deferred

6. The resolver's own corrections: both accurate (executed)

7. FINDING 2 (LOW, fixed in 8e345cd)

.claude/skills/ref-check/SKILL.md:108

Section 3 ended: "With all three forms indexed as described, 22 references are genuinely broken (issue #457), which is the number a run should reproduce." Those figures were measured at 79d147c. The corpus has moved underneath them, and this merge is what brought the movement in:

79d147cdc79fc5
files under llp/132149
distinct LLP numbers129149
headings10981220
@ref LLP occurrences13551495

A figure presented as reproducible, in the doc whose job is telling a checker what a false BROKEN looks like, will itself read as a regression the next time master adds an LLP. I dated every count on the page to 79d147c and replaced "the number a run should reproduce" with what a run actually must reproduce: the rule, and no BROKEN report for a reference the three anchor forms resolve. I did not substitute a new number, because I implemented section 3 independently and got a different count (8 broken over 1497 refs) whose delta is explained by scanning scope and by the literal NNNN placeholder refs rather than by the rule, so quoting mine would just re-create the problem. (executed for the corpus measurements; reasoned for the wording)

8. Style and refs

  • 0 em dashes added, in the PR's diff against master and in my commit. (executed)
  • No semicolons added to JS. Types stay in JSDoc; no @typedef, no inline import('...'), @import at the top with root-anchored .js specifiers. The one type change in the diff is the union in types.d.ts, which came from master. (executed)
  • All @refs this PR adds resolve: 0066#readable, 0066#control-path, 0066#ephemeral, 0067#cli-session-id, 0086#endpoint-discovery. LLP 0066 R9 and LLP 0067's annotation-map row still match EPHEMERAL_NOTE as written. (executed)

9. Checks

checkbaseline (origin/master, pristine worktree, node_modules symlinked)head dc79fc5after my 8e345cd
npm test3037 tests / 3028 pass / 8 fail / 1 skipped3041 / 3032 / 8 fail / 1 skipped3041 / 3032 / 8 fail / 1 skipped
npm run typecheck-cleanclean

The 8 failures are confirmed pre-existing by name against the pristine baseline, and the two name lists diff identically: leave after join removes the seed and reports the server, leave clears an applied central slot, not just the seed, leave reverses org-driven attaches and drops the forward identity, leave after join also warns about a local central sink that keeps forwarding, leave is idempotent: a second leave is the not-connected no-op, leave still tears down when only a stale attach marker survives a prior partial leave, leave removes the assets its attach marker records, and leaves manual copies alone, leave self-heals an org attach whose plugin is gone: drops the marker, warns, stays clean. All 8 are test/core/leave-command.test.js. (executed)

The branch's tests run and pass, they are not skipped. The +4 test delta is entirely this PR's, and both files were also run in isolation: test/plugins/codex-privacy-skill-session-id.test.js3/3 pass, 0 skipped; test/plugins/ai-gateway-session-status.test.js44/44 pass, 0 skipped. The single skipped test in the suite is pre-existing and present in the baseline run too. (executed)

Findings

sevfindingfile:linedisposition
LOWambiguity refusal raises TypeError instead of its message when a candidate's session_id is non-string; fails closed, but the operator-facing text becomes a tracebackcodex/skills/hypaware-privacy/SKILL.md:81fixed8e345cd, verified by execution
LOWref-check section 3 presents 79d147c-era counts as "the number a run should reproduce"; the corpus grew from 132 to 149 files under this very merge.claude/skills/ref-check/SKILL.md:108fixed8e345cd
INFOthe "why not select on CODEX_THREAD_ID" rationale answers grain but not the liveness fail-open llp/0067:580-587 pinscodex/skills/hypaware-privacy/SKILL.md:30recorded, not rewritten (residual disclosed at :38; the verb is the documented answer)
INFOpython3 hard dependency, 5 skills across both clientspre-existing, byte-identical to mastertracked by #435; not a blocker, agreeing with the prior triage
INFOthe POST receipt is a Set.add plus echo, and the shell path never compares the echo to what it sentcontrol.js:112-113tracked by #460
INFOStep 1 prose says the drop keys on metadata.session_id; exchange-projector.js:712-714 prefers the durable body client_metadata.session_id firstsame grain either way, pre-existing on masterno action

Verification of what I pushed

Per LLP 0002 I verified against the committed remote tree, not a green suite. gh api repos/hyparam/hypaware/contents/<path>?ref=8e345cd returns ', '.join(str(m[0] or m[1]) for m in matches) at SKILL.md:81 and the dated-counts paragraph in .claude/skills/ref-check/SKILL.md. PR head is now 8e345cd.

Status

Not merged, not marked ready. No hyp subcommand was run and no host state was mutated: every probe used a temporary CODEX_HOME, a stub localhost gateway, and a stubbed curl for the two default-port cases. /work/hypaware was never written to; all work happened in detached worktrees, now removed.

One granted review round remains after this one. Nothing found here blocks; if the remaining round is not spent, this head is shippable on my read, with the three INFO items going to their existing issues (#435, #460).

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

Copy link
Copy Markdown
ContributorAuthor

review rung, PR #456 @ 8e345cd - round 4 of 4

Read this first, future orchestrator. This is round 4 of 4: the last round the maintainer granted, and the grant is now exhausted. The CLI's 2-round cap was spent at 94fde3f; the maintainer then granted two more on this thread ("let's give PRs 456 and 462 2 more review rounds each", comment, relayed from Slack, 2026-07-30), round 3 used the first and this one uses the second. There are no review rounds left. The next rung for this PR is triage. Do not re-derive a budget from the <!-- neutral-review --> markers; the grant and its exhaustion live only in this thread.

Reviewed in a detached worktree at 8e345cd with node_modules symlinked from the clone, plus a pristine origin/master (73b4618) worktree as the baseline. The code-review skill is not installed in this clone and codex is not on PATH, so this is a direct audit. python3 is absent from this host, so I reused round 3's standalone CPython 3.11.13 to execute the privacy skill's Step 1 block rather than read it.

Outcome: findings - 1 MEDIUM, 4 LOW, 2 INFO. 2 fixed and pushed as 0bc6336 (PR head is now 0bc6336). Nothing blocking merge. The MEDIUM is a documentation defect in the PR's own title, which I have deliberately not edited myself - it needs one gh command from triage, given verbatim below.


1. The delta dc79fc5..8e345cd is exactly two lines, and I re-exercised both

git diff dc79fc5 8e345cd is 4 insertions / 2 deletions across 2 files: the str(...) coercion at codex/skills/hypaware-privacy/SKILL.md:81, and the dated-counts paragraph at .claude/skills/ref-check/SKILL.md:110. No code, no LLP, no test touched. (executed)

1a. The ambiguity-refusal fix: correct, and it changed nothing for valid input (executed)

The brief's specific worry - that a str(...) coercion can quietly change what gets printed for an ordinary string session_id - is the right worry, and it is not what happened. I ran the extracted Step 1 block over 22 fixture CODEX_HOME trees against a request-logging stub gateway, at dc79fc5 (pre-fix) and at 8e345cd (post-fix), and diffed the two transcripts after normalising tmpdir paths. The diff is three hunks, all of them the non-string case:

< Traceback (most recent call last):
< File "<stdin>", line 32, in <module>
< TypeError: sequence item 0: expected str instance, int found
---
> 2 rollouts record cwd TMPDIR/work (777, C2): ambiguous, confirm the session id with the user

...and the same for a dict and a list container. Every other case is byte-identical, including every exit code and every gateway-call count (the === case (exit=N, gatewayCalls=M) === header lines do not appear in the diff at all). The ordinary all-strings ambiguity cases are unchanged:

casefixturepre-fixpost-fix
g9two rollouts, C1 / C22 rollouts record cwd ... (C1, C2): ambiguous ...identical
s1 (new)three rollouts, C1/C2/C33 rollouts ... (C3, C1, C2): ambiguous ...identical
s2 (new)one legacy rollout with no session_id, so the join falls back to the thread id... (LEGACY-thread, C2) ...identical
s3 (new)explicit "session_id": null... (T1, C2) ...identical
g8session_id: 777TypeError traceback... (777, C2): ambiguous ...
s4 (new)session_id: {"a":1}TypeError traceback... ({'a': 1}, C2): ambiguous ...
s5 (new)session_id: ["x"]TypeError traceback... (['x'], C2): ambiguous ...

str() on a str is the identity, and the m[0] or m[1] fallback is evaluated before the coercion, so the None/absent/blank paths still fall back to the thread id exactly as before. Confirmed by execution rather than by that argument. Two more single-match non-string containers I added (true, 1.5) still hit the laterisinstance guard and refuse with the designed "unusable payload.session_id" message: the fix did not swallow them. (executed)

1b. The refusal invariant still holds end to end: 0 gateway calls on every refusal (executed)

Re-verified at the head, from the stub's request log rather than from the script text. Across all 22 cases, 15 refuse and all 15 made 0 POSTs; the 4 that resolve (n1 root thread, n2 subagent, g1b real header beside a turn_context decoy, g12 good rollout beside an unparsable one) each made exactly 1 POST, and each POSTed the container:

  • n2 runs with CODEX_THREAD_ID=SUBAGENT-thread exported and divergent, and puts CONTAINER-uuid on the wire. The thread id never reaches it.
  • The session_meta type guard still refuses the turn_context decoy (g1), still lets the real header through beside it (g1b), and the whitespace / whitespace-only / non-string / legacy-no-container guards all still refuse.

The fix opened no path that POSTs. The pre/post transcript diff proves it directly: no gatewayCalls= value changed anywhere.

1c. The ref-check dating fix: the numbers it commits to are exact (executed)

The new sentence anchors everything to 79d147c "over 132 files and 129 LLP numbers". Measured at that commit: git ls-tree -r --name-only 79d147c -- llp | grep -c '\.md$' = 132, distinct four-digit prefixes = 129. Both exact. Corpus today (origin/master = 73b4618): 149 files, 149 distinct numbers, which is exactly why dating them was the right fix rather than substituting a new figure.


2. FINDING 1 (MEDIUM, recorded for triage - one command)

The PR title states the opposite of what the PR ships.

"Codex session identity comes from CODEX_THREAD_ID, and the ignore caveat names the fork (#452, #455)"

The shipped skill says, at codex/skills/hypaware-privacy/SKILL.md:30:

"It is a thread id, so it is not the answer this step needs and the script below deliberately does not send it."

and the PR's own test asserts 'CODEX_THREAD_ID must never reach the control route'. Round 2 reversed the CODEX_THREAD_ID approach and the body carries the [Corrected by triage after round 2's reversal ...] note, but the title was never corrected. Session identity for the drop comes from payload.session_id read out of the rollout, not from that variable.

This matters more than a cosmetic nit for one reason: on a squash merge the title becomes the commit subject in master's history, so the repo would permanently record a false statement about the design this PR exists to fix, in a subject line that git log --oneline and every future git log -S reader sees. Three prior rounds on this PR each found prose/behaviour drift; this is the last and most durable instance of it.

I did not edit it. A PR's title is the author's and triage's, not a review-rung artifact, and my mandate is the worktree. It is one command:

gh pr edit 456 --repo hyparam/hypaware \
--title "Codex session identity is the rollout's session container, not CODEX_THREAD_ID, and the ignore caveat names the fork (#452, #455)"

(reasoned, from the executed reading of the skill and its test at head)

3. FINDING 2 (LOW, fixed in 0bc6336)

test/plugins/codex-privacy-skill-session-id.test.js:74

// Issue #453 puts it to work as a selector; until then it must not be sent.

Future tense, and the future has arrived: #453 landed on master as 5d270a5 via #458 (merged 2026-07-30T03:16Z), and hyp session ignore already resolves CODEX_THREAD_ID as a selector, returning source: 'codex_env_rollout'. Round 3 corrected exactly this tense at SKILL.md:30 ("already puts it to its real use") and left the same stale claim in the test that pins the decision - which is the worse of the two places, because the test comment is what a later reader consults to learn why the assertion exists, and "until then" invites them to conclude the assertion is now obsolete and delete it.

Replaced with the landed present, naming both the issue and the PR that landed it, and keeping the distinction the assertion actually enforces: selecting on the variable is the verb's job, sending it is what this step must never do. The three assertions are untouched; the file still passes 3/3. (executed)

4. FINDING 3 (LOW, fixed in 0bc6336)

.claude/skills/ref-check/SKILL.md:110

Round 3's own fix overclaimed by one word. It opens "Every count on this page is a snapshot, taken at commit 79d147c", but the page also carries the sample report under Report findings (ref-check found 47 references in 23 files, Broken: 3, ...), which is illustrative and was never measured at 79d147c or anywhere else. A note whose entire job is stopping a stale figure from being read as current should not itself make a claim broader than what was measured - that is the same defect one level up. Narrowed to "Every corpus measurement in this section", with a parenthetical naming the sample report as illustrative. Every real measurement on the page (the 1098/1107 heading check, the 25-marker/47-reference {#...} count, the 1355/632/366/266/30/47/22 figures) is in section 3, so the narrowed scope still covers all of them. (executed for the placement check, reasoned for the wording)

5. FINDING 4 (LOW, recorded): the PR body's ordering section is stale and reads as a hold

The body's final section says:

gh pr view 450 --json state -> OPEN, branch fix/issue-442. ... Merge #450 first.

#450merged 2026-07-29T23:32Z as c551d6e, which git merge-base --is-ancestor c551d6e origin/master confirms is an ancestor of master, and 8e345cd already contains origin/master (verified: git merge-base --is-ancestor origin/master 8e345cd succeeds). The stated precondition is satisfied, and the conflict it predicted was resolved in dc79fc5 and audited in round 3. Left as written, a triage rung reading the body top-to-bottom sees a live "merge X first" instruction and may hold the PR on a dependency that landed two days ago. Same for the §452(b) corpus figures ("129 LLPs"), now 149. Body-only, so I have not rewritten the author's prose; flagging it so triage reads that section as history rather than as a gate. (executed)

6. FINDING 5 (LOW, recorded, deliberately not fixed): the script's candidate filter is looser than the CLI's, in the ambiguity direction

codex/skills/hypaware-privacy/SKILL.md:72

ifpayload.get('cwd') !=cwdornotpayload.get('id'):
continue

The block's own comment claims it "Mirrors readRolloutMeta in the CLI". On the session_meta type guard and the payload-is-a-dict guard it does. On payload.id it does not: the CLI's cwd scan (session_command.js:582-586) does if (!meta) continue; if (meta.cwd !== args.cwd) continue and admits a candidate with no thread id (metaField(payload?.id) simply yields undefined), whereas the script drops it. Since the ambiguity refusal is a count over candidates, dropping one makes the script resolve where the CLI refuses.

Executed, fixture x1 - two rollouts recording this cwd, one carrying session_id: "C1-noid" and nopayload.id, one carrying both:

=== x1 (exit=0, gatewayCalls=1) ===
resolved session C2 (INFERRED from rollout-b.jsonl on disk)
opt-out confirmed for session C2 (total ignored: 1)
{"url":"/_hypaware/ignore/session","method":"POST","body":"{\"session_id\":\"C2\"}"}

The CLI on the same evidence would report 2 Codex rollouts record cwd ... Pass the intended session id explicitly rather than guessing. A related fixture x2 (that rollout alone) refuses with no Codex rollout ... records cwd X, which is also mildly untrue - one does record it, it just has no thread id.

I am not changing it, and the reason is the brief's own gate. The obvious fix (drop the id requirement, use payload.get('id') in the tuple) makes x1 refuse correctly, but it simultaneously flips x2 from a refusal into a resolution that POSTs C1-noid - opening a new path to the wire on the last review round, in the one script whose whole contract is refusing rather than guessing. That trade is not worth taking without a round to verify it. The finding is bounded: it needs a session_meta header that states payload.session_id but omits payload.id, which is not something Codex emits (both fields are written into the same header at session start), so it is a divergence from the mirroring claim rather than a reachable defect. Recording it fully so triage owns the decision rather than inheriting a silent gap. (executed for both fixtures; reasoned for the "Codex does not emit this" bound)

7. INFO: a " in the container still produces a malformed body (fails closed against the real route)

SKILL.md:96 refuses a container carrying whitespace but not one carrying " or \, and printf '{"session_id":"%s"}' at :128 then emits malformed JSON. Executed, fixture x3 (session_id = C"X): the body on the wire is {"session_id":"C"X"} and my echoing stub answered with a null id, so the block printed opt-out confirmed for session None. Against the real route this fails closed: ai-gateway/src/control.js:95-107 answers 400 for a body it cannot read or one with no extractable session_id, and curl --fail-with-body under set -e aborts before the confirmation line. So this is a stub artefact, the same printf is on master, and it is a clean illustration of #460's "compare the echo against what you sent". Round 3 hit the same thing; re-confirmed at this head, no new issue. (executed)

8. Re-verified from round 3 rather than taken on trust

  • The three-way agreement holds.session_command.js:88-99 (CODEX_THREAD_ENV: "carries a thread id, which is NOT the key the gateway drops on"), the skill's Step 1 prose at :28/:30, and codex/src/exchange-projector.js:113 (const sessionId = stringValue(codexContext?.session_id) ?? conversationId, consumed at :176 by isSessionIgnored). Unchanged in this delta. (executed)
  • readRolloutMeta is a real symbol, not a stale name: session_command.js:839, a local wrapper delegating to core's readRolloutSessionMeta (src/core/codex/rollout_session_meta.js:97, LLP 0150's one reader). The skill comment's citation resolves. (executed)
  • Every @ref this PR adds resolves, including the two in the new test file that round 3 did not enumerate: LLP 0066#enforcement (llp/0066:86, {#enforcement}) and LLP 0030#decision (llp/0030-session-id-partition-key.decision.md:54, ## Decision -> slug decision). The other six (0066#readable x2, 0066#control-path, 0066#ephemeral, 0067#cli-session-id, 0086#endpoint-discovery) all resolve too. (executed)
  • LLP and code agree. LLP 0066 R9's added requirement ("MUST name both ways ... in the writer's confirmation and the reader's own ignored line alike") matches EPHEMERAL_NOTE and both its call sites; LLP 0067's annotation-map row names the constant. LLP 0001's amendment matches what the ref-check skill now says about the three anchor forms. (executed)
  • python3 hard dependency: unchanged by this delta, byte-identical to master, 5 skills across both clients, fails closed at exit 127 with 0 gateway calls. Tracked by Collapse the session opt-out skills onto hyp session and update LLP 0066/0067 (blocked by #432) #435. Agreeing with the prior triage.
  • Style: 0 em dashes (U+2014) in the PR's added lines against master, and 0 in my commit. No JS statement terminators added. Types stay in JSDoc; no @typedef, no inline import('...'). (executed)

9. Checks

checkpristine origin/master (73b4618, node_modules symlinked)head 8e345cdafter my 0bc6336
npm test3037 tests / 3028 pass / 8 fail / 1 skipped3041 / 3032 / 8 fail / 1 skipped3041 / 3032 / 8 fail / 1 skipped
npm run typecheck-clean (exit 0)clean (exit 0)

The 8 failures are confirmed pre-existing by name against the pristine baseline: I sorted both not ok name lists and diff returned empty. All 8 are test/core/leave-command.test.js (leave after join removes the seed and reports the server, leave clears an applied central slot, not just the seed, leave reverses org-driven attaches and drops the forward identity, leave after join also warns about a local central sink that keeps forwarding, leave is idempotent: a second leave is the not-connected no-op, leave still tears down when only a stale attach marker survives a prior partial leave, leave removes the assets its attach marker records, and leaves manual copies alone, leave self-heals an org attach whose plugin is gone: drops the marker, warns, stays clean). (executed)

The PR's own tests run and pass, they are not skipped. The +4 test delta over the baseline is entirely this PR's, and both files were run in isolation at my head: test/plugins/codex-privacy-skill-session-id.test.js3/3 pass, 0 skipped; test/plugins/ai-gateway-session-status.test.js44/44 pass, 0 skipped. The single skipped test in the full suite is pre-existing and present in the baseline run too. (executed)

Findings

sevfindingfile:linedisposition
MEDIUMthe PR title says session identity "comes from CODEX_THREAD_ID"; the shipped skill deliberately never sends it and the PR's own test forbids it. On a squash merge this becomes master's commit subjectPR titlerecorded for triage, gh pr edit command in section 2. Not edited: PR metadata is not a worktree artifact
LOWtest comment still says "#453 puts it to work as a selector; until then it must not be sent"; #453 landed via #458, and round 3 fixed this same tense in the skill but not heretest/plugins/codex-privacy-skill-session-id.test.js:74fixed0bc6336, verified on the remote tree
LOWround 3's snapshot note claims "every count on this page"; the sample report's counts are illustrative and were never measured.claude/skills/ref-check/SKILL.md:110fixed0bc6336, verified on the remote tree
LOWPR body's "Ordering against #450" still reports #450OPEN and instructs "Merge #450 first"; it merged 2026-07-29 (c551d6e, ancestor of master) and this head already contains master. Reads as a live holdPR bodyrecorded; body prose left to the author/triage
LOWcandidate filter requires a truthy payload.id, unlike the CLI's cwd scan, so the script can resolve and POST where the CLI refuses as ambiguous (executed, fixture x1)codex/skills/hypaware-privacy/SKILL.md:72recorded, deliberately not fixed: the fix flips another fixture from refusal into a POST, which is not a change to make unverified on the final round
INFOa " in the container yields a malformed body; fails closed against the real route (control.js:95-107 400s, curl --fail-with-body + set -e aborts)codex/skills/hypaware-privacy/SKILL.md:96,128tracked by #460
INFOpython3 hard dependency, 5 skills across both clients, byte-identical to master, exit 127 and 0 gateway callspre-existingtracked by #435

Verification of what I pushed

Per LLP 0002, verified against the committed remote tree, not a green suite, by two independent paths:

  • git show origin/fix/issue-452:test/plugins/codex-privacy-skill-session-id.test.js returns the corrected comment at lines 73-77, and git show origin/fix/issue-452:.claude/skills/ref-check/SKILL.md returns the narrowed sentence at line 110.
  • gh api repos/hyparam/hypaware/contents/test/plugins/codex-privacy-skill-session-id.test.js?ref=0bc6336 returns the same bytes.
  • origin/fix/issue-452 = 0bc63364027bcf229e8610129294eeeafa94d529, and gh pr view 456 --json headRefOid agrees. PR head is now 0bc6336.

Status

Not merged, not marked ready. No hyp subcommand was run and no host state was mutated: every probe used a temporary CODEX_HOME, a stub localhost gateway that logs every request, and a stubbed curl for the default-port cases. /work/hypaware was never written to; all work happened in detached worktrees, now removed.

The review grant is exhausted. The next rung is triage, and the two things triage inherits are the MEDIUM title correction (one command, section 2) and the LOW body-staleness in section 5, plus the recorded-not-fixed divergence in section 6. Nothing found across four rounds blocks merge; on my read this head is shippable once the title is corrected.

@philcunliffephilcunliffe changed the title Codex session identity comes from CODEX_THREAD_ID, and the ignore caveat names the fork (#452, #455)Codex session identity is the rollout's session container, not CODEX_THREAD_ID, and the ignore caveat names the fork (#452, #455)Jul 30, 2026
@philcunliffe

philcunliffe commented Jul 31, 2026

Copy link
Copy Markdown
ContributorAuthor

Triage rung: reviewed the full four-round history and re-verified every residual by execution at head 0bc63364027bcf229e8610129294eeeafa94d529 (fresh worktree, origin/master baseline built in this same tick, node_modules symlinked, no host state mutated). All three residuals are non-blocking:

npm test at head: 3041/3032 pass, 8 fail (all test/core/leave-command.test.js, confirmed pre-existing by name against a pristine origin/master baseline run fresh in this tick). npm run typecheck clean. Also sanity-checked the two self-corrections made outside the review rungs (PR title, and the "Merge #450 first" body section now marked resolved) - both intact and accurate.

Full writeup, including the executed evidence for the filter non-finding: #499

Verdict: SHIP.

@philcunliffephilcunliffe 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 a2b4a6f into masterJul 31, 2026
9 checks passed
@philcunliffe
philcunliffe deleted the fix/issue-452 branch July 31, 2026 02:14
philcunliffe pushed a commit that referenced this pull request Jul 31, 2026
The counting rule this change documents governs `hyp session`'s resolver only.
The Codex `hypaware-privacy` skill body still drops an id-less `session_meta`
header before its own count, so on the two-rollouts fixture the verb refuses
while the script resolves and POSTs the survivor: round 4 of #456 predicted that
divergence, triage found it did not reproduce because both sides discarded, and
fixing one side makes it real. Left standing because #435 retires the script
onto this verb, but recorded so the doc does not read as a rule both surfaces
follow.
Co-Authored-By: Claude <noreply@anthropic.com>
philcunliffe added a commit that referenced this pull request Jul 31, 2026
…mbiguity, not vanish before the count (#499) (#518)
* session: a cwd-matching rollout with no thread id must count, not vanish
The Codex cwd path's answer rests on a count: "exactly one rollout records
this cwd, so it is the session I am in". `readRolloutMeta` returned
`undefined` for a `session_meta` header stating no `payload.id`, and the
scan loop reads `undefined` as "not a rollout at all", so a header that
demonstrably recorded the invocation cwd never reached the candidate list.
Two sessions having run in one directory therefore resolved at `ok: true`
to whichever of them happened to carry a thread id, with no disclosure
that a rival had been discarded. That is the artefact-of-the-bound failure
LLP 0067 refuses on for a truncated scan, reached through a discard the
bound never touches, and it lands on a privacy verb: `hyp session ignore`
opts out one of two indistinguishable sessions and prints success.
The thread id is not what this path resolves (it returns the container and
carries the thread only as provenance), so requiring one to be counted was
never load-bearing. `readRolloutMeta` now means only "this file's first
line is not a session_meta header", and the callers refuse on what they
each need:
- the cwd scan counts every header recording the cwd, then refuses a lone
unvouchable one by name rather than through the old (and untrue) "no
Codex rollout records cwd" message
- the stated-thread path skips a header naming no thread explicitly: it is
an identity test, not a count, so nothing there changes
- the ambiguity message names an id-less candidate as such, so the count
cannot exceed the names
Found while working #499 §1, which recorded round 4 of #456's `x1`/`x2`
fixtures and asked that anyone touching this filter re-run them. Executed
rather than reasoned about: `x1` resolved, `x2` refused. Both are now
pinned. `x2` keeps refusing, which was the trade round 4 declined to give
up.
Refs #499
* llp 0067: name the skill-body divergence the cwd-count rule now creates
The counting rule this change documents governs `hyp session`'s resolver only.
The Codex `hypaware-privacy` skill body still drops an id-less `session_meta`
header before its own count, so on the two-rollouts fixture the verb refuses
while the script resolves and POSTs the survivor: round 4 of #456 predicted that
divergence, triage found it did not reproduce because both sides discarded, and
fixing one side makes it real. Left standing because #435 retires the script
onto this verb, but recorded so the doc does not read as a rule both surfaces
follow.
Co-Authored-By: Claude <noreply@anthropic.com>
* session: a header stating neither id is not an old Codex, so do not say "upgrade"
Round-2 review finding on #518 (issue #499). Once an id-less `session_meta`
header survives the reader to reach the single-candidate checks, a header
stating neither `payload.id` nor `session_id` becomes newly reachable, and it
hit the missing-container branch first: `legacyRolloutError`, whose text is
"Upgrade Codex (current versions write session_meta.session_id)" and whose
argument is "its thread id is NOT that container".
Neither clause holds for this file. The back-fill that rule exists to defeat
needs an `id` to back-fill from, so a header without one is not a pre-field
Codex, and the message names a value the header does not carry. The two
refusals are therefore asked in the other order: the unvouchable-header
refusal, which names the field that is actually missing, goes first, and
`legacyRolloutError` is reached only with a thread id in hand, which is what
both its clauses assume.
Fail-closed either way and both point at the explicit-id escape hatch, so this
is the diagnosis, not the behaviour. The genuine old-Codex case (a thread id,
no `session_id`) keeps its own message, pinned by a test that passes both
before and after.
LLP 0067 §cli-session-id records the ordering and why. Its earlier
self-reference to §cli-legacy-rollout, which sits in the section the paragraph
is already inside, is replaced with a plain forward reference.
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: test <test@test.com>
Co-authored-by: test <test@example.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

1 participant

@philcunliffe