Skip to content

hyp session: take Codex's stated CODEX_THREAD_ID over the rollout mtime guess (#442 A, D) - #450

Merged
philcunliffe merged 3 commits into
masterfrom
fix/issue-442
Jul 29, 2026
Merged

hyp session: take Codex's stated CODEX_THREAD_ID over the rollout mtime guess (#442 A, D)#450
philcunliffe merged 3 commits into
masterfrom
fix/issue-442

Conversation

@philcunliffe

@philcunliffephilcunliffe commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Works the deferred findings from PR #439, one verdict per item. Two of the four moved; two did not, and the reasons are recorded rather than hand-waved.

A - the 30-minute stale-rollout window: FIXED

The deferral rested on a factual claim: "closing it fully needs a liveness signal Codex does not currently expose". That claim is out of date.

  • openai/codex#10096 (merged 2026-02-03, closing the feature request openai/codex#8923) injects CODEX_THREAD_ID into the environment of every shell/exec tool subprocess, and deliberately exempts it from shell_environment_policy.include_only filtering. Verified against the merged diff: create_env(policy, Some(session.conversation_id)) in tools/handlers/shell.rs, tasks/user_shell.rs and unified_exec/process_manager.rs.
  • Its presence is proof, not proxy: a session that has ended cannot have spawned the process the variable is set in. That is precisely the liveness signal the mtime bound was approximating.
  • Its value is session.conversation_id, the thread id, which is the same identifier the rollout's session_meta.payload.id carries and its filename embeds. So this is the same id the disk scan already produced, obtained from a source that states it rather than infers it. No new identity is introduced, only better provenance.

resolveSessionIdForCli now takes a stated id (CLAUDE_CODE_SESSION_ID or CODEX_THREAD_ID, source codex_env) ahead of the disk scan. The scan survives untouched as the fallback for a Codex predating the variable and for a hand invocation from a terminal, keeping its 30-minute bound and its INFERRED from <rollout> provenance line. A stated id carries neither, because neither applies to it: attaching the caveat to a stated id would train the reader to skip it on the one path where it is load-bearing.

Two clients each stating an id (environments nest: Codex runs claude, or the reverse, and the child inherits the parent's variable while setting its own) is ambiguity, and refuses naming both. Preferring either is wrong half the time, and being wrong means opting out a session the user is not in while reporting success. Strictly narrowing, per the same argument round 2 used for the staleness bound: it can only turn a confident answer into a refusal.

Regression test, failure path exercised. 6 new tests in test/plugins/ai-gateway-session-status.test.js (corrected: an earlier draft of this description said 5). 5 of the 6 fail on master and pass here (verified by reverting session_command.js alone: # pass 33 / # fail 5, then restoring: # pass 38 / # fail 0); the sixth, "an explicit session id argument still beats a Codex-stated one", passes unchanged against master too; it pins a property the change does not disturb rather than one it introduces, so it has no teeth of its own but is still worth keeping as a guard against a future regression in argument precedence. The load-bearing one is "the 30-minute stale-rollout window cannot hand out a DEAD id when Codex states the live one": it builds a rollout for a session that ended 60s ago (inside the bound), asserts the disk path still resolves that dead id, then asserts that with CODEX_THREAD_ID set the live id wins. That is the fail-open itself, pinned, not a happy path.

Not claimed: an identity caveat the switch does not change. CODEX_THREAD_ID is the thread; when a live Codex exchange carries metadata.session_id the adapter stamps that as session_id and the thread as conversation_id (LLP 0030). The disk scan has exactly the same property, so this is pre-existing and unchanged, not introduced here. Recorded in LLP 0067 §cli-session-id.

B - residual echo-rogue: DEFERRED (nothing cheap and correct exists)

Judged, not waved through. The obvious fix, an authenticity token the gateway writes beside its bound port and the client requires, does not defend against the stated attacker: any process able to bind that port runs as the same user, and is therefore able to read the same file. A shared secret between two same-uid processes authenticates nothing.

The only signal that would actually separate them is peer-process identity. status.json already carries the daemon pid and endpoint resolution is already liveness-gated on it, so the client could in principle verify that the process listening on the resolved port is that pid. That needs platform-specific machinery with no portable form (/proc/net/tcp plus /proc/<pid>/fd on Linux, lsof on macOS, neither on Windows), which makes it a design decision, not a hardening tweak. Recorded in LLP 0067 §cli-response-check so the next person does not re-derive the dead end. Residual stays mitigated by the endpoint_source disclosure.

C - scope: MOOT, no code action

A packaging preference recorded against a PR that has since merged (3aff41f). Confirmed: nothing to do. The guidance it carried (land route + tests, then the CLI surface, then session-id resolution, as three reviewable changes) stands for future PRs of that shape and needs no artefact of its own.

D - does claude --resume / --fork-session mint a new session id: SETTLED, recorded in LLP 0066

A resume reuses the id; a fork mints a new one. Claude Code's own flag reference is explicit, from claude --help at 2.1.215:

--fork-session When resuming, create a new session ID instead of reusing the original (use with --resume or --continue)

The flag exists precisely because --resume / --continue reuse the id by default. Codex splits the same two behaviours across separate verbs (codex resume <id> / codex fork <id>).

So an opt-out survives a resume and is silently dropped by a fork - and the fork is exactly the case LLP 0066 §readable item 2 exists to surface: re-running hyp session status inside the fork resolves the new id (stated by the client) and reports not_ignored, which is the true answer for that session. PR #439's decision to make this detectable rather than decide it turns out to have been the right call; this just fills in the fact. Doc-only, no code consequence.

Local transcript evidence was checked first and was inconclusive by construction: all 51 transcripts under ~/.claude/projects carry exactly one sessionId each (subagent files carry their parent's), i.e. none of them was resumed or forked, so the CLI's own contract is the evidence. Read-only inspection; nothing under $HOME was modified.

Closing this PR closes#442, so the leftovers need their own issues

Item C is genuinely finished and item D is now recorded. Item B is not fixed and will be silently closed along with #442 unless it is filed separately. Please open:

  • "hyp session cannot prove the control responder is the gateway" - carrying the §cli-response-check analysis above: shared-secret ruled out (same-uid), peer-pid verification named as the only real option, and the trigger to act on it (an unprivileged-user multi-tenant host where port squatting is realistic).

Two smaller follow-ups worth their own issues, both found while working this and both out of scope here:

  • The Codex hypaware-privacy skill body still picks the newest rollout by mtime (codex/skills/hypaware-privacy/SKILL.md) and states "Codex, unlike Claude, exposes no CLAUDE_CODE_SESSION_ID". Both are now wrong: it should short-circuit on CODEX_THREAD_ID. Not changed here because a skill body has no regression test and this PR's gate is a proven one.
  • LLP 0086 has no {#...} anchors, so the two @ref LLP 0086#endpoint-discovery / #manual-attach-reads-the-live-port annotations in session_command.js are broken on master. Pre-existing, untouched.

(Added by neutral triage after CI went green: filed #455, the CLI's own ephemerality caveat names a gateway restart but never a resume/fork session-id change, even though item D above settles that a fork silently drops the opt-out. Nothing lies; hyp session status re-run always reports the current session's true state. But the user has no CLI-visible signal to know they should re-check after a fork. Doc-only fix, not blocking.)

Checks

  • npm test: # pass 2850 / # fail 8 - the 8 known pre-existing failures in test/core/leave-command.test.js, unrelated to this change. No regressions.
  • npm run typecheck: clean (exit 0).
  • House style: no semicolons, no em dashes added.
  • LLP doc edits (0066, 0067) land in the same commit as the code, per CLAUDE.md.

Fixes#442

…me guess
Issue #442 item A. `resolveSessionIdForCli` fell back to a Codex rollout
whose `payload.cwd` matched the invocation cwd, treating an mtime inside a
30-minute bound as evidence the session was live. mtime is a proxy, so a
session that ended inside the window still resolved, and `hyp session ignore`
would opt out the finished session, print "the gateway will drop this
session", and leave the session the user is actually in recording.
PR #439 deferred this on the ground that Codex exposes no liveness signal.
That is no longer true: openai/codex#10096 (merged 2026-02-03, closing
openai/codex#8923) injects `CODEX_THREAD_ID` into the environment of every
shell/exec tool subprocess, exempt from `shell_environment_policy` filtering.
Its presence is proof rather than proxy - a session that has ended cannot
have spawned this process - and its value is `session.conversation_id`, the
same identifier the rollout's `session_meta.payload.id` carries and its
filename embeds. So this is the id the disk scan already produced, from a
source that states it instead of inferring it.
The disk scan stays as the fallback for a Codex predating the variable and
for a hand invocation no client spawned, keeping its staleness bound and its
`INFERRED from <rollout>` provenance line. A stated id carries neither,
because neither applies to it.
Two clients each stating an id (environments nest: Codex runs `claude`, or
the reverse) is ambiguity and refuses naming both, rather than preferring one
and being wrong half the time. Strictly narrowing: it can only turn a
confident answer into a refusal.
Also settles issue #442 item D in LLP 0066: `--resume` / `--continue` reuse
the session id and `--fork-session` mints a new one, per Claude Code
2.1.215's own flag reference, so an opt-out survives a resume and is dropped
by a fork. And records in LLP 0067 why item B (a local listener that echoes
the token) has no cheap fix: a shared secret is readable by any same-uid
process, and peer-pid verification has no portable form.
Items B and C remain deferred; see the PR body.
…var proof of provenance not liveness
Review of #450. The CODEX_THREAD_ID premise holds: verified against the merged
openai/codex#10096 diff (the insert is step 6 of core/src/exec_env.rs, AFTER the
include_only retain, so the exemption is structural) and against
codex-rs/protocol/src/protocol.rs, where `SessionMeta { session_id: SessionId,
id: ThreadId }` confirms the rollout's `payload.id` is the same ThreadId the
variable carries. Two claims around it were tighter than the evidence supports.
1. "Presence is proof of liveness" is proof of PROVENANCE. It is liveness for a
`hyp` run inside a tool call the client is blocked on, which is the path that
matters, but a process that outlives its spawn (a server or tmux pane started
from a tool call) inherits the variable and keeps it after the session ends.
Strictly narrower than the mtime bound it replaces, but not zero.
2. The identity caveat understated its own consequence. The drop keys on the
session container (LLP 0066 §scope); CODEX_THREAD_ID is the thread. Codex
derives the session id from the thread id for a ROOT thread (same uuid), but
a SUBAGENT thread keeps the root's session id and mints its own, so an
opt-out taken inside a subagent tool call states an id the drop never matches
and the verb reports success for a suppression that suppresses nothing.
Pre-existing and shared with the disk scan, so not introduced here, but now
on a live path and now closable: current Codex writes `session_meta.session_id`
beside `session_meta.id`, so the drop key itself is on disk.
Also corrects §cli-response-check's "neither on Windows", which reads as "no
mechanism on Windows"; Windows has GetExtendedTcpTable, it is just a third
implementation and a native dependency. The argument is unchanged.
Docs and comments only. No behaviour change: 2850 pass / 8 known pre-existing
leave-command failures, typecheck clean.
Co-Authored-By: Claude <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

Verdict: the CODEX_THREAD_ID premise holds. Three doc-accuracy findings, all fixed in 9414dd9.

I verified the load-bearing claim against upstream source rather than taking the attribution on trust, because this is a privacy control. The switch is sound and is a real narrowing. Two statements around it were tighter than the evidence supports, and one of them hides a fail-open that is now on a live path.


1. Identifier equivalence: is CODEX_THREAD_ID the id the gateway drops on?

Two different questions, two different answers.

(a) Is it the same id the disk scan produced? YES.codex-rs/protocol/src/protocol.rs:3059-3061 on main:

pubstructSessionMeta{pubsession_id:SessionId,pubid:ThreadId,

The rollout's session_meta.payload.id is a ThreadId. openai/codex#10096 injects thread_id.to_string() where thread_id is session.conversation_id, the same ThreadId. So the PR's central equivalence claim - "the same identifier the rollout's session_meta.payload.id carries" - is correct. No new identity is introduced. Confirmed.

(b) Is it the id the gateway USAGE_POLICY_DROP keys on? NOT ALWAYS.

hypaware-core/plugins-workspace/codex/src/exchange-projector.js:98:

constsessionId=stringValue(codexContext?.session_id)??conversationId

and :148if (ctx?.isSessionIgnored?.(sessionId)). The drop matches the session container (x-codex-turn-metadata.session_id), not the thread. From codex-rs/core/src/session/session.rs:554-579:

  • root thread: session_id = SessionId::from(thread_id), which reuses the same uuid (protocol/src/session_id.rs, impl From<ThreadId> for SessionId). The two coincide. Common case, works.
  • subagent thread (session_source.is_non_root_agent()): session_id = agent_control.session_id() (the root's) while thread_id is its own. They diverge.

A codex subagent runs shell tool calls with its own CODEX_THREAD_ID (tools/handlers/shell.rs, session.conversation_id). So hyp session ignore taken from inside a subagent tool call states an id the drop never matches: nothing is suppressed and the verb prints success. That is a fail-open in a privacy control.

But the PR does not introduce it and does not widen it - the disk scan reads the same payload.id, so the defect is identical on both paths, and the PR flags it under "Not claimed". That framing is honest. The premise is not wrong; do not park this.

What the PR does change, and what LLP 0067 did not say:

  1. LLP 0066 §scope calls the Codex grain mismatch latent "because the only opt-out skill today is Claude-only". This PR is what makes the Codex path first-class, so it is no longer latent.
  2. It is closable, and the LLP implied otherwise by silence. Current Codex writes session_meta.session_id (the actual drop key) beside session_meta.id in the rollout. The resolver could read it. codex/src/backfill.js:564-566 ("the rollout carries no distinct session id") is stale against current Codex for the same reason.

Finding 1 - MEDIUM (fixed).llp/0067-session-opt-out.design.md:377-382 (old) stated the caveat without its consequence. Rewritten to name the subagent divergence, the resulting silent no-op, the "no longer latent" change, and the on-disk session_meta.session_id that makes it closable. Mirrored in the STATED_SESSION_ID_VARS block comment, session_command.js:70-90. Not fixed in code - a resolver + backfill change together is more than this PR is scoped for. Worth its own issue (see below).


2. Does presence prove liveness?

It proves provenance. Provenance is liveness only for a synchronous tool call.

CODEX_THREAD_ID is set on the environment of the process Codex spawns for a tool call, so a session that has ended cannot have spawned that process - the PR's argument is right as far as it goes. It stops holding for a process that outlives its spawn: a server, a tmux pane, any &-detached child started from a tool call inherits the variable and keeps it after the session ends. A later hyp run in that descendant's environment names a finished session with full confidence, which is the same class of defect the PR removes.

It is a much narrower residual (it needs a detached descendant of a tool call, not merely a cwd Codex visited in the last 30 minutes), and for the path the privacy skills actually use - hyp inside a tool call the model is blocked on - the session is live by construction. So the change is a genuine improvement. The claim to make is just narrower than the one that was made.

Checked and cleared, not defects:

  • codex resume reuses the thread id (InitialHistory::Resumed => resumed_history.conversation_id), and the resumed session is live when it spawns. Resume makes the id stable, which is what you want.
  • codex fork mints a new one (InitialHistory::Forked(_) => ThreadId::default()). Correct.

Finding 2 - LOW/MEDIUM (fixed).llp/0067:317-319 "Its presence is therefore proof that the session it names is the one running this command" rewritten to "proof of provenance", with the outlives-its-spawn residual named and bounded. Same correction in the code comment.


3. Upstream attribution: verified, not asserted

All four citations check out against the GitHub API from here:

ClaimVerified
openai/codex#10096 merged 2026-02-03Yes, 66b196a, merged 2026-02-03T19:31:13Z
Injects CODEX_THREAD_IDYes, exec_env.rs: env_map.insert(CODEX_THREAD_ID_ENV_VAR, thread_id.to_string())
Exempt from include_onlyYes, and structurally: the insert is step 6, afterenv_map.retain(|k, _| matches_any(k, &policy.include_only)). Stronger than the PR says. Added to the LLP.
Value is session.conversation_idYes, shell.rs / user_shell.rs both pass Some(session.conversation_id)
Closes openai/codex#8923Issue closed 2026-02-04, title "expose current Codex session ID programmatically". Consistent
openai/codex#19630 merged 2026-04-28Yes, title matches exactly

One nuance not worth a change: create_env takes Option<ThreadId> and app-server's exec path passes None, so "every shell/exec subprocess" holds for the tool-call paths the PR names but not literally every codex-spawned process. The resolver handles absence correctly (falls through to the scan).


4. Fallback ordering and the refusal path - all confirmed

  • 30-minute bound survives on the disk path.session_command.js:520-531, MAX_ROLLOUT_AGE_MS unchanged, refusal message unchanged.
  • INFERRED provenance survives.:347-349 still keys on idSource === 'codex_rollout' only. Test 29 asserts codex_env produces no INFERRED line and session_id_evidence: null.
  • Two stated ids refuse, naming both.:475-481, error interpolates ${s.env}=${s.id} for every candidate. It cannot silently pick one: the stated.length > 1 refusal precedes the stated.length === 1 return, so there is no path where a second candidate is dropped.
  • CLAUDE_CODE_SESSION_ID vs CODEX_THREAD_ID precedence is deliberate: there is none. The array order only fixes the order of names in the refusal string. I confirmed by reversing STATED_SESSION_ID_VARS - 38/38 still pass, i.e. no test depends on the order, which is correct given both-set is an error. Documented in the JSDoc and in LLP 0067 §cli-session-id.
  • Explicit argument still wins over any stated id (:142, :226 short-circuit before resolveSessionIdForCli), and there is a test for it.
  • Blank / whitespace-only values fall through rather than resolving to nothing, and a blank one alongside a real one is not treated as ambiguity. Tested.

5. Test teeth - verified, they have real teeth

Failing-before / passing-after, reproduced exactly as claimed.git checkout origin/master -- session_command.js, tests untouched:

not ok 24 - CODEX_THREAD_ID beats the disk scan: a stated id is not an inference
not ok 25 - the 30-minute stale-rollout window cannot hand out a DEAD id when Codex states the live one
not ok 26 - two clients each stating a session id is ambiguity, and ambiguity refuses
not ok 27 - an empty CODEX_THREAD_ID is not a stated id: it falls through rather than resolving to nothing
not ok 29 - a Codex-stated id is reported as stated, not as INFERRED from disk
# pass 33 / # fail 5

Restored: # pass 38 / # fail 0. Matches the PR body.

Test 25 does exercise the fail-open directly, not a happy path: it asserts the disk path still resolves the dead id for a session that ended 60s ago (inside the bound), then asserts the live id wins once CODEX_THREAD_ID is set. That is the residual pinned in both directions.

Mutation testing (mine, beyond the PR's claim):

MutationResult
if (stated.length > 1) -> if (false) (refusal becomes pick-the-first)not ok 26 only. Precisely targeted.
Delete the stated.length === 1 short-circuit (stated id demoted below the disk scan)12 failures incl. 23, 24, 25, 27, 29
Reverse STATED_SESSION_ID_VARS order38/38 pass - correct, see above

The suite pins the behaviour, not the implementation.

Note: the diff adds 6 tests, not 5. The sixth (an explicit session id argument still beats a Codex-stated one) passes on master too, since the argument path short-circuits above the resolver. The "5 failing-before" count is right.


6. LLP accuracy

Item D (--resume / --fork-session) - accurate, independently confirmed.claude --help at 2.1.215 on this machine returns the quoted text verbatim: --fork-session When resuming, create a new session ID instead of reusing the original (use with --resume or --continue). The Codex half also checks out: codex-rs/cli/src/main.rs:180,192 has both Resume(ResumeCommand) and Fork(ForkCommand) as separate verbs, and session.rs:554-558 confirms the semantics - Resumed reuses conversation_id, Forked takes ThreadId::default(). SessionMeta even carries forked_from_id. Nothing overstated. No change needed.

Item B (echo-rogue dead end) - reasoning sound, one inaccurate clause. The core argument is correct: a gateway-written token is readable by any process that could bind the port, because both run as the same uid, so it authenticates nothing. Peer-process identity is the only real separator, and status.json already carries the pid, so the sketch is right too.

Finding 3 - LOW (fixed). "(/proc/net/tcp plus /proc/<pid>/fd on Linux, lsof on macOS, neither on Windows)" reads as "no mechanism exists on Windows", which is false - GetExtendedTcpTable gives the owning pid. Rewritten to say it is a third implementation plus a native dependency. The conclusion (separate design decision, not a hardening tweak) is unchanged and if anything better supported.

Item C (moot) - confirmed, nothing to check.


7. Refs and house style

  • No new broken @ref anchors. Every anchor this PR touches resolves: 0066#readable, 0066#scope, 0067#cli, 0067#cli-session-id, 0067#cli-provenance, 0067#cli-response-check, 0067#tests. The only break in session_command.js is the pre-existing@ref LLP 0086#endpoint-discovery / #manual-attach-reads-the-live-port against a doc with no {#...} anchors at all. Untouched here, tracked separately, correctly called out in the PR body.
  • No em dashes added by this PR, and none by me. (llp/0067 carries 28 pre-existing ones on master - corpus-wide, out of scope.)
  • No semicolons, JSDoc types only, no inline import('...') types. types.d.ts union widened correctly and threaded through SessionStatusReport.
  • npm test: 2850 pass / 8 fail, the known test/core/leave-command.test.js baseline. npm run typecheck: clean. Unchanged after my commit.

What I fixed

9414dd9 on fix/issue-442, docs and comments only, no behaviour change:

  • llp/0067-session-opt-out.design.md §cli-session-id - "proof of liveness" -> "proof of provenance", with the outlives-its-spawn residual named; the identity caveat rewritten to name the subagent divergence, the silent no-op it causes, the loss of "latent", and the on-disk session_meta.session_id that makes it closable; the include_only exemption noted as structural (step 6, after the retain).
  • llp/0067 §cli-response-check - the Windows clause corrected.
  • hypaware-core/plugins-workspace/ai-gateway/src/session_command.js - STATED_SESSION_ID_VARS block comment brought in line with both corrections.

Re-verified after the commit: 2850 pass / 8 known fail, typecheck clean, no new long lines, no em dashes.

What remains (not blocking)

  • Own issue, MEDIUM:"hyp session ignore from a Codex subagent thread opts out an id the drop never matches" - carry §cli-session-id's analysis: read session_meta.session_id from the rollout, move codex/src/backfill.js:564-566 off "the rollout carries no distinct session id" at the same time, and decide whether a stated CODEX_THREAD_ID should be resolved through the rollout to the container id before it is sent. Pre-existing, not a regression from this PR.
  • The three follow-ups the PR body already asks for (item B's peer-pid issue, the Codex hypaware-privacy skill body still picking newest-by-mtime, LLP 0086's missing anchors) all stand and are correctly scoped out.
  • PR body says 5 new tests; the diff adds 6. Cosmetic, and I did not edit the body.

Still a draft, and I did not mark it ready or merge it.

…retracted claim
Round 2 review of #450. Round 1 downgraded "CODEX_THREAD_ID is proof of
liveness" to "proof of provenance" in LLP 0067 §cli-session-id and in the
STATED_SESSION_ID_VARS comment, naming the residual (a process that outlives
its spawn inherits the variable and keeps it after the session ends). It missed
two other places that state the retracted version, one of them normative.
1. LLP 0066 R11 justified "a stated id carries no staleness bound" with "only a
running session can have spawned the process it is set in" - the liveness
claim itself, in a MUST-shaped requirement. The requirement is right (an env
var has no timestamp to bound), only its rationale overclaimed. Restated as
provenance, with the outlives-its-spawn residual and a pointer to
LLP 0067 §cli-session-id.
2. `provenanceNotes` justified qualifying only the disk inference with "because
only it can name a session that has already ended", which LLP 0067 now says
is false: a hyp run from a detached descendant of a tool call can name a
finished session too. Replaced with the reason LLP 0067 §cli-provenance
actually gives - the stated id's residual is far narrower, and qualifying
both would train the reader to skip the caveat on the one path where it is
load-bearing.
Docs and comments only, no behaviour change. Round 1's premise re-verified
against openai/codex main: SessionMeta { session_id: SessionId, id: ThreadId }
with neither field carrying a serde attribute, root threads take
SessionId::from(thread_id) (a straight uuid copy) while a non-root agent takes
agent_control.session_id() with an independently minted ThreadId, and PR 10096's
insert is post-retain, post-exclude and post-set in exec_env.rs.
Co-Authored-By: Claude <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

Round 2 (final, cap 2) - reviewed 9414dd9

Verdict: approve. The change is correct, the premise is now verified at the source rather than at the PR description, and the tests have real teeth. I found two doc-accuracy findings, both LOW, both fixed here in 63067fe: round 1's liveness->provenance correction was right but incomplete, and the retracted claim survived in two other passages, one of them a normative requirement.


1. Round 1's three doc rewrites: all accurate, none overcorrected

1a. The identity caveat (llp/0067-session-opt-out.design.md:391-418) - ACCURATE

Round 1 rewrote this from "an identity caveat the stated id does not change" into a named subagent divergence. Every load-bearing claim in the rewrite checks out against openai/codexmain @ 85c082cccc:

  • SessionMeta carries both ids.codex-rs/protocol/src/protocol.rs:3053-3065: pub struct SessionMeta { pub session_id: SessionId, pub id: ThreadId, ... }. Those two are the only fields in the struct carrying no serde attribute at all - no skip, no rename, no skip_serializing_if - so both always serialize.
  • Both reach the rollout header.SessionMetaLine#[serde(flatten)] meta (:3149-3155) -> RolloutItem#[serde(tag="type", content="payload")] (:3186-3189) -> RolloutLine (:3382-3388). Write site codex-rs/rollout/src/recorder.rs:832-834. In-repo example header, codex-rs/rollout/src/recorder_tests.rs:109-118: "payload": { "session_id": uuid, "id": uuid, ... }.
  • Root thread: same uuid.codex-rs/core/src/session/session.rs:554-580 takes SessionId::from(thread_id) for a root, and codex-rs/protocol/src/session_id.rs's impl From<ThreadId> for SessionId is a straight uuid copy. The doc's SessionId::from(thread_id) is verbatim correct.
  • Subagent: root's session id, own thread id. Same site, other branch: if session_configuration.session_source.is_non_root_agent() { agent_control.session_id() }, with thread_id independently minted (ThreadId::default() -> Uuid::now_v7()). codex-rs/core/src/agent/control.rs:93-101 documents AgentControl as root-scoped: "every sub-agent spawned from that root ... share the same session ID". Asserted behaviourally by codex-rs/core/tests/suite/prompt_cache_key.rs:40, which pins differentThreadIds: true with one shared sessionId across root and child.

So the divergence the rewrite describes is real, the mechanism is stated correctly, and "closable, because current Codex writes session_meta.session_id beside session_meta.id" is true. It is not overcorrected - if anything it was worth writing down, and #453 now carries it.

One qualification for whoever fixes #453 (not a defect in this PR - the doc's "current Codex" hedge is already the right one): session_id is guaranteed in current writes, not in files on disk. SessionMetaLine has a hand-written Deserialize (protocol.rs:3157-3184) that back-fills it from id when absent, precisely for legacy rollouts. A resolver that starts reading payload.session_id will silently get the thread id back on an old rollout - i.e. the exact wrong value for a subagent - rather than an error. Also note the filename uuid is the thread id, not the session id (recorder.rs:1570). Worth adding to #453 before anyone implements it.

1b. "Proof of provenance, not liveness" (llp/0067:315-334) - ACCURATE, and the right framing

The distinction is real and the residual is stated correctly: a process spawned by a tool call inherits the variable and keeps it after the session ends, so a detached descendant (server, tmux pane) can still name a finished session. "Strictly narrower than the mtime bound" is right - the env path needs an actual descendant of a tool call, where the mtime path needed only a cwd Codex visited in the window. Calling it a residual rather than zero is the honest read.

The injection's trustworthiness is if anything understated. At 66b196a (PR 10096's merge commit), codex-rs/core/src/exec_env.rs:83-94 puts the insert at step 6, after the include_only retain (step 5) and after exclude (step 3) and after the user's set (step 4) - so a user cannot suppress or forge it via shell_environment_policy. The doc comment says so outright: /// CODEX_THREAD_ID is injected when a thread id is provided, even when include_only is set. The doc's "structural rather than a listed exception" is exactly right.

(INFO, no action: the algorithm has since been hoisted to codex-rs/protocol/src/shell_environment.rs:99-107 with identical step 5/6 ordering. The doc's sentence is scoped to the merged PR, so it is accurate as written - just stale about HEAD.)

1c. The Windows claim (llp/0067:285-292) - ACCURATE, and it strengthens rather than weakens the conclusion

GetExtendedTcpTable is real (iphlpapi.dll; with TCP_TABLE_OWNER_PID_LISTENER it returns listening sockets with their owning PID), which is precisely the port->pid mapping the peer-identity check needs. So round 1 was right that "neither on Windows" was false, and the correction matters: the original wording made the argument rest on impossibility, which would not have survived contact with someone who knew the API. The rewritten version rests on cost - three platform implementations for one check - which is both true and sufficient for "separate design decision, not a hardening tweak". Conclusion unchanged, foundation better.

(INFO, no action: "Three implementations and a native dependency" is a shade strong when the same sentence offers netstat -o scraping as an alternative, and macOS lsof is itself a scrape rather than a native dep. Cosmetic; the argument does not turn on it.)


2. Round 1's commit is non-behavioural - CONFIRMED

9414dd9 touches 16 lines of session_command.js. All 16 are inside the single /** ... */ block above STATED_SESSION_ID_VARS (lines 68-93). No statement, no expression, no declaration changed; the array literal itself is untouched. git diff 5ee62bf..9414dd9 -- '*.js' contains no non-comment line. Tests and typecheck are identical either side.


3. Test teeth at 9414dd9 - REPRODUCED EXACTLY

git checkout origin/master -- hypaware-core/plugins-workspace/ai-gateway/src/session_command.js, then node --test test/plugins/ai-gateway-session-status.test.js:

not ok 24 - CODEX_THREAD_ID beats the disk scan: a stated id is not an inference
not ok 25 - the 30-minute stale-rollout window cannot hand out a DEAD id when Codex states the live one
not ok 26 - two clients each stating a session id is ambiguity, and ambiguity refuses
not ok 27 - an empty CODEX_THREAD_ID is not a stated id: it falls through rather than resolving to nothing
not ok 29 - a Codex-stated id is reported as stated, not as INFERRED from disk
# pass 33 / # fail 5

Restore -> # pass 38 / # fail 0. Full suite # pass 2850 / # fail 8 (the known test/core/leave-command.test.js baseline). npm run typecheck exit 0.

INFO - the discrepancy round 1 flagged is confirmed, and is slightly larger than "5 vs 6". The diff adds 6 tests; the PR body says 5 and calls them "all failing on master". Test 28, "an explicit session id argument still beats a Codex-stated one", passes against the reverted resolver: with the old code CODEX_THREAD_ID is never read, and an explicit argument short-circuits regardless, so nothing distinguishes the two states. It is a fine guard test, it just is not a regression test for this change. So the accurate claim is "6 new tests, 5 of them failing on master". Not worth a body edit at this stage; recorded here.


4. The fallback path and the refusal - BOTH INTACT

  • 30-minute bound survives.MAX_ROLLOUT_AGE_MS = 30 * 60 * 1000 (session_command.js:70) is untouched by the diff, and the single-match branch still gates on it (:525-540), still refusing on an un-stat-able file as well as a stale one. Test 25 proves the bound is still live, not vestigial: it asserts the disk path still resolves the 60s-old dead rollout before asserting the env var beats it.
  • INFERRED provenance line survives, and correctly does not spread.provenanceNotes still emits it for codex_rollout only (:359-363); codex_env gets no note, which test 29 pins in both human and --json form (session_id_source: 'codex_env', session_id_evidence: null).
  • Two stated ids refuse and name both.:479-485 builds `${s.env}=${s.id}` for every set variable and returns ok: false - it never picks. Test 26 asserts both values appear in the error and that it points at the explicit-id escape hatch.
  • The "no rollout found" error was correctly updated to stop claiming CLAUDE_CODE_SESSION_ID is not set when a Codex variable is now also consulted (:546).

Findings

LOW-1 (fixed, 63067fe): LLP 0066 R11 still justified itself with the liveness claim round 1 retracted

llp/0066-session-opt-out.spec.md:235-238 read:

An id a client states ... carries no such bound: only a running session can have spawned the process it is set in.

That is the "proof of liveness" claim, restated inside a MUST-shaped requirement - the most authoritative place in the corpus for it to survive, and the one a future reader is most likely to take at face value. It also directly contradicts what llp/0067:321-330 now says two documents over. The requirement itself is right (an env var has no timestamp, so there is nothing to bound); only the rationale overclaimed. Restated as provenance, with the outlives-its-spawn residual named and a pointer to LLP 0067 §cli-session-id.

LOW-2 (fixed, 63067fe): provenanceNotes asserted the exclusivity LLP 0067 had just denied

session_command.js:336-338 read:

Only the inference is qualified, because only it can name a session that has already ended.

llp/0067:325-328 says a hyp run from a detached descendant of a tool call can name a finished session, so "only it" is false. Note this line is not round 1's own text - it came in with 5ee62bf - but round 1 rewrote the retracted claim in two neighbouring places and left this one, which now reads as the authoritative version. Replaced with the justification §cli-provenance actually gives: the stated id's residual is far narrower, and qualifying both paths would train the reader to skip the caveat on the one path where it is load-bearing.

Both are docs/comments only. # pass 38 / # fail 0, typecheck clean, and grep confirms neither phrasing survives anywhere in the tree.

Checked clean

  • @ref anchors. Every anchor in the two changed files resolves: 0067#cli, 0067#cli-session-id, 0066#readable, 0066#control-path, 0066#ephemeral all exist. The only broken pair is the pre-existing LLP 0086#endpoint-discovery / #manual-attach-reads-the-live-port (0086 has zero {#...} anchors) - untouched by this PR, tracked in Docs drift: hypaware-privacy skill states two things about Codex that are now false, and two @ref LLP 0086 anchors are broken #452. No new broken refs.
  • Em dashes. 0 added (git diff origin/master..HEAD | grep -c '^+.*—' = 0). llp/0067 holds at 28, identical to master.
  • House style. No semicolons introduced; no inline import('...') types; @import declared at file top.
  • codex/src/backfill.js genuinely carries the "the rollout carries no distinct session id" comment the LLP cites (hypaware-core/plugins-workspace/codex/src/backfill.js:599), so the "two things must move together" note is grounded.
  • Item D corroborated from source, beyond claude --help. session.rs:554-560: InitialHistory::Forked(_) => ThreadId::default() (new uuid) vs InitialHistory::Resumed(h) => h.conversation_id (reused), and SessionMeta carries forked_from_id / parent_thread_id for exactly this. Codex behaves as LLP 0066 now records.

For triage

  1. hyp session ignore from a Codex subagent thread opts out an id the drop never matches: silent no-op reported as success #453 (subagent thread vs session container) - the substantive one. Add the legacy-rollout qualification from §1a above before anyone implements it: payload.session_id back-fills from id on old rollouts, so reading it is not automatically safe.
  2. Docs drift: hypaware-privacy skill states two things about Codex that are now false, and two @ref LLP 0086 anchors are broken #452 / Collapse the session opt-out skills onto hyp session and update LLP 0066/0067 (blocked by #432) #435 - the Codex hypaware-privacy skill body still resolves by newest-rollout-mtime and still states "Codex, unlike Claude, exposes no CLAUDE_CODE_SESSION_ID" (hypaware-core/plugins-workspace/codex/skills/hypaware-privacy/SKILL.md:22,30,46). Worth stating plainly: this PR closes the fail-open in hyp session, but the documented Codex path does not call hyp session - it curls the control route directly with a newest-by-mtime id. The fix is real and correct; Codex users do not reach it until Collapse the session opt-out skills onto hyp session and update LLP 0066/0067 (blocked by #432) #435 lands. Deferring was the right call for this PR (a skill body has no regression test), but the two should not drift apart for long.
  3. hyp session cannot prove the control responder is the real gateway #451 (item B, peer-process identity) - correctly deferred with the dead end recorded.
  4. INFO, no issue needed: the PR body's "5 new tests, all failing on master" is really "6 added, 5 with teeth" (§3).

No further rounds; cap reached. Nothing here blocks.

@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

Triage verdict: SHIP

Both review rounds approved after unusually thorough verification (upstream
openai/codex#10096 diff checked, structural pre-include_only placement
confirmed, both rounds' test claims reproduced exactly). This triage re-ran
everything independently in a detached worktree at head 63067fe
(merge-base with master, c793698) rather than trusting the record:

  • npm test: 2850 pass / 8 fail, the 8 are the known pre-existing
    test/core/leave-command.test.js failures (present on master too, not
    from this PR).
  • npm run typecheck: clean.
  • Reverting session_command.js alone: test/plugins/ai-gateway-session-status.test.js
    goes from 38 pass / 0 fail to 33 pass / 5 fail, failures at test indices
    24, 25, 26, 27, 29 (matches round 2's citation exactly). Restoring the file
    brings it back to 38/0.

Step 3 - does a stated CODEX_THREAD_ID make #453 more confident / harder to notice?

Yes, marginally, and it is a deliberate, already-reasoned trade-off, not an
oversight.
Traced provenanceNotes in session_command.js:356-370 and its
call sites (writeMutationResult at :285, writeStatus at :312/:319):

functionprovenanceNotes(idSource,idEvidence,endpointSource){constnotes=[]if(idSource==='codex_rollout'){notes.push(`session id: INFERRED from ${idEvidence??'a Codex rollout'} on disk, ...`)}
...
}

The INFERRED caveat fires only for idSource === 'codex_rollout'. A stated
id (claude_env or the new codex_env) prints no provenance note at all.
So: before this PR, the #453 subagent-thread scenario resolved the wrong
(thread, not session-container) id via the disk scan and printed the generic
INFERRED ... If that is not the session you are in, re-run naming it
caveat. After this PR, the same wrong id resolves via CODEX_THREAD_ID
instead and prints nothing. The wrong-id case is real and does get quieter.

Two things bound how much this matters:

  1. The rate is unchanged.CODEX_THREAD_ID and the rollout's
    session_meta.payload.id are the same thread id (LLP 0067
    §cli-session-id, confirmed against the diff). The subagent divergence in
    hyp session ignore from a Codex subagent thread opts out an id the drop never matches: silent no-op reported as success #453 is identical on both paths; this PR does not introduce or widen it,
    only changes which path resolves it and what gets printed alongside.
  2. The removed hedge was generic, not targeted. It never named the
    subagent-vs-root distinction; it fires for every disk-inferred id,
    including all the correctly-resolved ones, which trains readers to treat
    it as boilerplate. LLP 0067 §cli-provenance states the trade-off
    explicitly: attaching the caveat to a stated id too "would train the
    reader to skip it on the one path where it is load-bearing" (the disk
    path's genuine staleness residual). This PR concentrates warning value
    where it is load-bearing rather than spreading a boilerplate hedge
    everywhere; hyp session ignore from a Codex subagent thread opts out an id the drop never matches: silent no-op reported as success #453's actual fix (match on session_meta.session_id, not the
    thread) is the real remedy, and this PR is what makes that fix newly
    possible, since current Codex now writes that field to disk.

Verdict on step 3: does not block. Same defect, same owner (#453,
already filed with a closable path), a marginal and reasoned confidence
change, not a new hole.

Step 4 - is item D's fork/resume finding adequately surfaced?

Follow-up, not blocking, and not covered by #451/#452/#453/#435 - filed
#455.
The read-verb
mechanism itself is sound: re-running hyp session status after a fork
resolves the new stated id and correctly reports not_ignored - nothing
lies. But nothing in the CLI's own output tells a user this can happen.
writeMutationResult's ignored branch already prints a caveat for the
other way an opt-out silently stops applying ("this opt-out is in-memory
only: a gateway restart drops it. Re-check with hyp session status.") and
says nothing about a fork/resume changing the session id underneath it. That
is a discoverability gap this PR's own review surfaced but did not close in
the tool's own words, only in LLP 0066 §readable. Filed as a doc/UX-only
follow-up so it is not silently closed along with #442.

Overall classification

All open residuals are non-blocking:

ItemStatus
#453 subagent thread-vs-session divergencepre-existing, not widened, filed, closable path named
#451 echo-rogue (item B)pre-existing, analyzed dead end, filed
#452 privacy-skill mtime + broken LLP 0086 anchorspre-existing, filed
#435 skill collapse onto hyp sessionpre-existing enhancement, filed
#455 (new) fork-drop caveat not in CLI textdoc/UX gap, filed, non-blocking

Bar per LLP 0017: a TRUE BLOCKER needs a production defect this PR causes or
worsens - wrong behavior, data loss, a security/privacy hole, a crash, or a
perf regression. Nothing here rises to that; every item is either genuinely
pre-existing and unwidened, or a documentation/UX completeness gap sitting on
a sound underlying mechanism. Weighed against what this PR actually closes -
a demonstrated stale-session false-assurance hole, reproduced end-to-end
above - the residuals do not outweigh shipping it.

Correction made to the PR body: the diff adds 6 new tests, not 5;
5 of them fail on master and pass here (the fail-open regression teeth),
the 6th ("an explicit session id argument still beats a Codex-stated one")
passes unchanged on both, pinning existing precedence rather than exercising
new behavior. Corrected in place, rest of the body preserved verbatim, with a
note added pointing at #455.

@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

Merge this BEFORE #458. Do not merge #458 first and then this one.

Neutral is flagging a cross-PR hazard it found while reviewing #458 (fix/issue-453), because both PRs are approved and mergeable right now and the order is not interchangeable.

Why. This PR resolves a Codex session id and returns it directly as the drop key:

session_command.js:97 { env: 'CODEX_THREAD_ID', source: 'codex_env' }

CODEX_THREAD_ID carries a thread id. The gateway drops on the session container (codex/src/exchange-projector.js:98). For a root thread these are the same uuid, so this PR is correct in the common case - but for a Codex subagent thread they diverge, and the stated id is then one the drop never matches: hyp session ignore reports success while recording continues. That defect is pre-existing (the old disk scan produced the same thread id), and this PR does not widen it - it is tracked as #453 and fixed by #458.

#458 replaces codex_env with codex_env_rollout: CODEX_THREAD_ID becomes a selector that names the live thread, and the session container is then read out of that thread's rollout, refusing when it cannot be established.

So the two orders are not symmetric:

Why not just close this PR and keep #458?#458 was branched from master, not from this PR, so it does not contain this PR's other work: the --fork-session / --resume finding recorded in LLP 0066 §readable, the provenance disclosure (session_id_source / session_id_evidence / endpoint_source, and the INFERRED line on both read and write verbs), the response-shape validation, the 64 KiB response cap, the wall-clock deadline, and six tests. All of that is worth keeping.

Nothing to do on this PR. It is approved on its own merits. This note exists only so the order is visible at merge time. Once this lands, #458 rebases on top of it and its codex_env_rollout resolution is the one that survives.

@philcunliffe
philcunliffe merged commit c551d6e into masterJul 29, 2026
8 checks passed
@philcunliffe
philcunliffe deleted the fix/issue-442 branch July 29, 2026 23:32
philcunliffe pushed a commit that referenced this pull request 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>
philcunliffe pushed a commit that referenced this pull request Jul 29, 2026
#450 landed on master and this branch is its follow-up, so where the two
disagree about session-id resolution the conflict resolves as a SUPERSEDE
rather than a merge, per this PR's stated ordering.
Kept from #450: the liveness benefit of CODEX_THREAD_ID (Codex sets it on the
process it spawns, so a finished thread cannot have set it), the
two-stated-clients refusal, and the blank-variable fallthrough.
Kept from this branch: the correct grain. CODEX_THREAD_ID is a selector, not an
answer - it names the live thread, the rollout is looked up by payload.id, and
the session container the gateway drops on is read out of it. `codex_env_rollout`
replaces `codex_env`.
Reconciled beyond the marked conflicts, where master's #450 text auto-merged but
its claims no longer hold:
- session_command.js `provenanceNotes` doc: CODEX_THREAD_ID no longer "states"
the session id, so its path is qualified rather than presented as stated.
Master's residual (a process outliving its spawn keeps the variable) is kept,
scoped to CLAUDE_CODE_SESSION_ID.
- LLP 0066 R10: the removed age bound is about liveness alone; the resolved
container stays an inference, so R12 still applies to it.
- LLP 0067 test plan: the stated-thread path reports `codex_env_rollout` with
the rollout named in `session_id_evidence`, not `codex_env` with none.
- Dropped #450's six resolver tests, which assert the superseded behaviour
(source `codex_env`, the thread id as the answer, no evidence). This branch's
tests cover the same intent at the corrected grain. Its one assertion with no
equivalent here - a blank CLAUDE_CODE_SESSION_ID beside a real Codex thread is
not ambiguity - is carried over, adapted.
Local run: npm test 2875 pass / 8 fail (all 8 are the pre-existing
test/core/leave-command.test.js baseline, identical on pristine origin/master);
the 96 tests in ai-gateway-session-status, codex-backfill and
codex-exchange-projector all pass; npm run typecheck clean; smokes
session_optout_capture_drop and backfill_codex_fixture ok.
Co-Authored-By: Claude <noreply@anthropic.com>
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

Development

Successfully merging this pull request may close these issues.

Follow-up: deferred review findings from PR #439

1 participant

@philcunliffe