Follow-up: deferred review findings from PR #462

[philcunliffe]

What

Deferred findings from four review rounds on PR #462 (fix/issue-459 - "Codex rollout cwd keys on the thread, not the session container"), classified at triage as non-blocking. All are backlinked here rather than gating merge; PR #462 fixes a real, always-reachable privacy leak (#459) and should not be held open for these.

1. The value-blind subagent_signal refusal is a documented, evidence-bounded residual - its own statement

hypaware-core/plugins-workspace/codex/src/exchange-projector.js:301 (at head 4dcd246):

if (codexContext.thread_source === 'subagent' || codexContext.subagent_signal) return undefined

subagent_signal is set from the bare x-openai-subagent header with no value check. That header's real values (per codex-rs) are review, compact, collab_spawn, memory_consolidation. Three of the four are same-workspace sub-threads of the root session, where the root's cwd is the correct answer. When such a turn states a container and no thread id, this guard refuses to resolve a cwd; .hypignore then fails open (LLP 0049) and the turn is recorded rather than dropped - the same leak direction issue #459 exists to close, reintroduced by the guard meant to bound the container fallback that fixes it.

Why it shipped anyway - the reachability chain, and its evidentiary status:

• Round 2 found the mechanism live in code but declined to redesign it from the reviewer's seat.
• Round 3 argued, by reasoning from this repo's own codex-rs audit (LLP 0151), that no realistic Codex request shape reaches the guard: client_metadata on the request body carries session_id and thread_id together on every request, and the turn-metadata blob gates both on the same has_turn_identity flag - so nothing can state a container while withholding the thread that pre-empts the guard. Round 3 flagged honestly that this half was reasoned, not executed: the codex-rs snapshot a prior worker had left in the shared scratchpad was gone.
• Round 4 closed that gap by reading openai/codex main over the network directly: CodexResponsesMetadata::client_metadata() (codex-rs/core/src/responses_metadata.rs:219-253) inserts session_id and thread_id into one map literal, unconditionally, from two non-Option String struct fields, and every /responses request carries that map (client.rs:928). Round 4 bounded the claim precisely: this is upstream main, not the pinned release this repo depends on, and it is a reading of emitting code, not captured traffic - LLP 0141 is explicit that no hermetic smoke in this repo can supply real Codex traffic.
• Round 4 also found a genuine misattribution three rounds had missed: has_turn_identity is false for exactly one request kind, Memory (memory consolidation), which still emits the lineage the refusal keys on (thread_source, parent_thread_id, the x-openai-subagent: memory_consolidation header) while its turn-metadata blob carries neither id. What keeps that real Codex shape out of the guard is the flat body map alone, not the blob - the JSDoc and LLP 0083 had credited the blob. Fixed and pinned by a test (test/plugins/codex-rollout-cwd.test.js:666).
• Triage independently re-verified round 4's claim by execution, not by inheriting it: fetched codex-rs/core/src/responses_metadata.rs and client.rs from openai/codex at main (commit 1def0a8, 2026-07-28 - the same commit and date round 4 cited), and confirmed directly: client_metadata() unconditionally inserts both ids (responses_metadata.rs:219-253); every /responses request carries it (client.rs:928); has_turn_identity is false only for Memory (responses_metadata.rs:137-138); SessionId/ThreadId are UUID-backed newtypes (session_id.rs, thread_id.rs) that can never serialize to an empty string, so the ids inserted are never blank. Triage also independently mutated the guard (removed the subagent_signal disjunct) and confirmed it reddens the DOCUMENTED MIRROR tests plus the two lineage-only tests - the mechanism is live code, not dead, and matters when it fires.

Net: the guard is real, live, value-blind, and fail-open when it fires - but per every read of the emitting source available (upstream main, not the pinned release, and not captured traffic), no Codex client can currently construct the input shape that fires it. That is a materially stronger position than "unreachable in practice" prose; it rests on unconditional, non-Option struct fields with no branch that could omit them. It is not proof against a future upstream change or an undiscovered client that behaves differently from documented codex-rs.

Round 4's recommendation, carried forward

The guard and the container fallback it bounds are now entered by exactly the same (currently unobserved) shape: a turn naming a container and no thread id anywhere. Round 3 and round 4 both recommend, as the coherent eventual fix, retiring the guard and the container fallback together rather than narrowing the guard by value - since neither protects real traffic any more, per the same audit. This is a design call for whoever picks it up, not a mechanical fix:

• Retiring the fallback alone (without also removing the guard) returns every container-only turn - root threads included - to cwd = NULL, which fails .hypignore open for that whole traffic class (the regression LLP 0083 exists to prevent), so the two changes are coupled.
• The durable alternative floated across rounds 1-3 - reading client_metadata.thread_id from the request body as the rollout lookup key, which the code already reads for the thread-id-present path - would make both the fallback and the guard's fail-open scenario unreachable by construction rather than by argument, and is likely the simplest version of "retire together."
• Whoever files the follow-up PR should re-verify the codex-rs audit against a pinned release version if one is vendored/pinned by then, not just main, since that is the gap every round from 3 onward has flagged honestly and left open.

2. Minor nit carried forward from round 1 (N2, not filed at the time)

rollout-cwd.js's identity-mismatch negative cache: a mismatch refusal is cached only for NEGATIVE_CACHE_TTL_MS, but a renamed or copied rollout does not heal, so a persistently-mismatched thread re-scans and re-warns every ~5s for its life. Bounded and consistent with the existing negative-cache design, and a should-never-happen in practice - round 1 judged it worth noting so it is not a surprise in a log, not worth blocking on. No action required unless it shows up as log noise in practice.

Already resolved, not carried forward

Round 1's F2 (fictional header names / client_metadata.thread_id unread) and F6 (two-readers duplication between rollout-cwd.js and session_command.js) were filed as #464 and #465 respectively and have both since been merged (#467 and #466). Not residual.

---

Backlink: #462

[philcunliffe]

🤖 neutral: stuck - needs a design decision that reverses an Active LLP

What neutral was doing: the issue-fix rung on this issue (reproduce -> root-cause -> fix -> Fixes #498 PR), against origin/master @ bd7dd0b.

Why it cannot proceed. Neither enumerated finding is a mechanical fix:

Finding 1 - value-blind subagent_signal refusal. Architectural, not mechanical. This issue's own carried-forward recommendation (rounds 3 and 4, restated at triage) is to retire the guard and the container fallback together, and it explicitly rejects the only mechanical alternative ("rather than narrowing the guard by value"). Executing that recommendation would:

• Directly reverse an Active decision. llp/0083-codex-live-cwd-from-rollout.decision.md#container-fallback-gap states that dropping the fallback "returns every turn that states only a container, root threads included, to cwd = NULL, failing .hypignore open for that whole traffic class: the regression this document exists to prevent," and that "the guard is kept rather than deleted." That is a supersede, not an annotation - and it trades a privacy-relevant fail-open against a capture gap, which is a product call, not a worker's.
• Rewrite the tests pinning the current contract (test/plugins/codex-rollout-cwd.test.js: DOCUMENTED MIRROR at :606-660, the two lineage-only tests at :571-604, the DOCUMENTED GAP test at :736).

New evidence, verified against a pinned release (openai/codex tag rust-v0.146.0, not main - every round from 3 onward flagged that gap):

• codex-rs/core/src/responses_metadata.rs:215-223 - client_metadata() inserts session_id and thread_id from two non-Option String fields (:159-160) into one unconditional map literal. The pair invariant holds in a released version, not just main.
• codex-rs/core/src/client.rs:922 - client_metadata: Some(...) on every /responses request.
• responses_metadata.rs:135-136 - has_turn_identity is false only for Memory, matching round 4.
• responses_metadata.rs:314-332: SubAgentSource::Other(label) => Some(label.clone()). The x-openai-subagent value set is open, not the closed four. LLP 0083 #container-fallback-gap, LLP 0151 #real-header-names, and the comment at exchange-projector.js:799-800 all state it as closed. This independently kills the "narrow by value" option from source rather than judgement: a value-aware guard must default-refuse unknown labels anyway, so narrowing buys nothing.

Finding 2 - negative-cache re-warn. No action, per this issue's own disposition: "Bounded and consistent with the existing negative-cache design, and a should-never-happen in practice ... No action required unless it shows up as log noise in practice." Confirmed against rollout-cwd.js:79 and :131-139. Nothing to reproduce, so no regression test exists to write - and fixing an explicitly-declined nit while finding 1 stands would not justify a PR.

Filed separately: #514. While auditing, a real defect turned up that is not in this issue's list: the guard reads x-openai-subagent from headers only (exchange-projector.js:811-815) while codex-rs also writes it into the body's client_metadata map (responses_metadata.rs:228-233), contradicting LLP 0151 #body-is-authority and mattering most on the API-key route. It is filed without neutral:fix on purpose: fixing it pushes the guard in the opposite direction from finding 1 (the refusal would fire more often), so it should be sequenced after this decision, not ahead of it.

What it needs from you: a call on finding 1:

• (a) Retire the guard and the container fallback together, as rounds 3/4 recommend - accepting cwd = NULL for container-only turns, i.e. .hypignore failing open for that traffic class. Needs a decision LLP superseding LLP 0083 #container-fallback-gap.
• (b) Keep the guard as designed, and record the open-value-set correction plus the released-version verification as an amendment to LLP 0083/0151 - closing finding 1 as working-as-intended.
• (c) Authorize a request LLP so this re-enters the design pipeline and a Designer works the trade-off properly before any code moves.

Neutral will not reverse an Active decision autonomously, which is why this is held rather than attempted.

How to unstick: reply with a comment on this issue; neutral monitors this thread and will re-engage with your guidance on its next tick.