Codex rollout cwd keys on the thread, not the session container (#459)

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

@@ -130,10 +130,10 @@ export function createCodexExchangeProjector(opts = {}) {
       // session_meta.cwd so `.hypignore` coverage is client-independent and live
       // rows carry the same cwd the codex backfill reads. The `??` keeps the
       // rollout lookup LAZY (a fresh in-band cwd never scans), and it is keyed on
-      // the codex session id — only a real Codex session has a rollout — so
+      // a Codex thread id (only a real Codex thread has a rollout), so
       // non-codex traffic never scans.
       const cwd = usableInBandCwd(firstString(codexContext?.cwd, readRecordedCwd(reqBody)), ctx)
-        ?? (codexContext?.session_id ? rolloutCwd?.resolve(codexContext.session_id) : undefined)
+        ?? resolveRolloutCwd(rolloutCwd, codexContext)
       // @ref LLP 0083#decision [implements]: a refused workspace substitution is
       // observable, not silent - it means the gate is measuring a different
       // directory than it would have. Paths are hashed: this seam sees LLM traffic.
@@ -236,6 +236,72 @@ export function createCodexExchangeProjector(opts = {}) {
   }
 }
 
+/**
+ * The rollout cwd fallback's lookup key.
+ *
+ * A Codex rollout is one **thread's** file and its name embeds that thread's id
+ * (`session_meta.payload.id`), NOT the session container (`payload.session_id`)
+ * the row partitions and the session opt-out drops on
+ * (@ref LLP 0030#decision). The two are the same uuid on a root thread, so
+ * handing over the container looked correct: a **subagent** thread inherits its
+ * root's container but mints its own thread id, so the container resolved the
+ * ROOT thread's rollout and the subagent turn was judged against a directory it
+ * never ran in. `.hypignore` is directory-scoped, so that recorded turns that
+ * should have been dropped.
+ * @ref LLP 0083#decision [implements]: the thread is what selects the rollout
+ *
+ * When the client states no thread id the container is still the right key for a
+ * ROOT thread (there the two are one uuid). That was once the common
+ * subscription-route shape, a bare `session-id` header; it is not any more, and
+ * that header name is not one Codex emits or this file reads
+ * (@ref LLP 0151#real-header-names). Since the adapter began reading the body's
+ * flat `client_metadata` map, which Codex fills with BOTH ids on every request
+ * (@ref LLP 0151#body-is-authority), an ordinary Codex turn states its thread and
+ * is answered by the branch above. What is left for the two lines below is a turn
+ * that names a container on a Codex-owned surface while naming no `thread_id` on
+ * any of them, which no `codex-rs` surface is known to produce.
+ *
+ * **Which lineage counts, and why it cannot be `thread_source` alone.**
+ * `thread_source` and `parent_thread_id` are read out of `x-codex-turn-metadata`,
+ * and that blob states `session_id` and `thread_id` as a pair or not at all (both
+ * gated on the same `has_turn_identity`), so it can never supply the container
+ * this fallback needs while withholding the thread that pre-empts it. Note the
+ * blob is NOT what answers such a turn: for the one kind with no turn identity
+ * (memory consolidation) the blob states the lineage and neither id, and the turn
+ * is answered by the body map, which carries both ids ungated. A refusal keyed
+ * only on those blob fields therefore cannot fire for a real Codex turn, but the
+ * reason is the map, not the blob. The lineage that
+ * would survive a turn stating no thread id is the lineage Codex sends as a
+ * DIRECT header, gated on nothing else: `x-codex-parent-thread-id` and
+ * `x-openai-subagent` (see `subagent_signal` in `resolveCodexContext`). Those are
+ * what make this refusal reachable at all, so both are consulted here.
+ *
+ * A turn stating a container, no thread id, and no lineage of any kind is then
+ * taken as the root thread it claims to be. That is a bounded residual: it can
+ * only mis-resolve for a client that both withholds its thread id and withholds
+ * every lineage signal on a subagent turn, and Codex withholds neither together.
+ * The mirror residual is the refusal itself: `subagent_signal` is value-blind, so
+ * `review`, `compact` and `memory_consolidation` (same-workspace sub-threads,
+ * where the root's cwd is the correct answer) refuse a container the root would
+ * have resolved, and LLP 0049 then fails OPEN and records the turn. Both residuals
+ * need the same unobserved shape (a container with no thread id anywhere), so
+ * neither is reachable from Codex traffic as `codex-rs` is documented to emit it.
+ * Dropping the fallback is not the safer half of that trade: it returns every turn
+ * that states only a container, root threads included, to `cwd = NULL`, which
+ * fails `.hypignore` open for that whole traffic class and is the regression
+ * LLP 0083 exists to prevent. @ref LLP 0083#container-fallback-gap [constrained-by]
+ *
+ * @param {RolloutCwdResolver | undefined} rolloutCwd
+ * @param {ReturnType<typeof resolveCodexContext>} codexContext
+ * @returns {string | undefined}
+ */
+function resolveRolloutCwd(rolloutCwd, codexContext) {
+  if (!rolloutCwd || !codexContext) return undefined
+  if (codexContext.thread_id) return rolloutCwd.resolve(codexContext.thread_id)
+  if (codexContext.thread_source === 'subagent' || codexContext.subagent_signal) return undefined
+  return codexContext.session_id ? rolloutCwd.resolve(codexContext.session_id) : undefined
+}
+
 // ---------------------------------------------------------------------
 // Provider routing
 // ---------------------------------------------------------------------
@@ -727,6 +793,26 @@ function resolveCodexContext(input, provider, path, reqBody) {
     readStringKey(metadata, 'parent_thread_id'),
     readHeader(input.request_headers, X_CODEX_PARENT_THREAD_ID),
   )
+  // Any evidence at all that this turn is a subagent's, in the names Codex's own
+  // source defines rather than only the ones read above. `codex-rs`
+  // `CodexResponsesMetadata::compatibility_headers` emits
+  // `x-codex-parent-thread-id` and `x-openai-subagent` (`review`, `compact`,
+  // `collab_spawn`, `memory_consolidation`) as DIRECT headers, gated only on
+  // their own value and NOT on the turn-metadata blob, so they are the one
+  // lineage signal that survives a turn stating no `thread_id`. That makes them
+  // the only usable guard on the container fallback below: every field read
+  // above travels inside `x-codex-turn-metadata`, which also carries `thread_id`,
+  // so a refusal keyed on those alone can never fire before the thread-id path
+  // has already returned. Deliberately NOT mirrored into `attributes` or the
+  // `parent_thread_id` column: widening what a row records is a separate change
+  // with its own migration story, and this value only has to gate a fallback.
+  // @ref LLP 0083#container-fallback-gap [implements]: the container fallback is
+  // refused on lineage Codex states as a header, not only in the metadata blob
+  const subagent_signal = firstString(
+    parent_thread_id,
+    readHeader(input.request_headers, 'x-codex-parent-thread-id'),
+    readHeader(input.request_headers, 'x-openai-subagent'),
+  )
   const originator = firstString(
     readHeader(input.request_headers, 'originator'),
     client.entrypoint,
@@ -777,6 +863,7 @@ function resolveCodexContext(input, provider, path, reqBody) {
     thread_id,
     session_id,
     parent_thread_id,
+    subagent_signal,
     turn_id,
     thread_source,
     // @ref LLP 0083#decision [implements]: an explicit in-band cwd outranks the
@@ -1053,7 +1140,7 @@ function resolveConversationSource(provider) {
  * @param {Record<string, unknown>} reqBody
  * @param {ReturnType<typeof resolveCodexContext>} codexContext
  * @param {string | undefined} cwd The cwd the caller already resolved (in-band
- *   fast path, else the rollout fallback). Passed in — not recomputed — so the
+ *   fast path, else the rollout fallback). Passed in (not recomputed) so the
  *   row's stamped cwd is exactly the value the `.hypignore` check used, and so
  *   the subscription route records the rollout cwd instead of NULL.
  *   @ref LLP 0083 [implements]

hypaware-core/plugins-workspace/codex/src/index.js

@@ -101,7 +101,14 @@ export async function activate(ctx) {
   // `.hypignore` fails open for that whole traffic class and its rows record
   // cwd = NULL.
   gateway.registerExchangeProjector(createCodexExchangeProjector({
-    rolloutCwd: createRolloutCwdResolver({ sessionsDir: path.join(codexHome, 'sessions') }),
+    rolloutCwd: createRolloutCwdResolver({
+      sessionsDir: path.join(codexHome, 'sessions'),
+      // So the identity guard's refusal (a rollout whose `session_meta.payload.id`
+      // is not the thread it was located for) is visible instead of silent: the
+      // consequence is a row recorded with cwd = NULL, which otherwise looks
+      // identical to a not-yet-written rollout.
+      log: ctx.log,
+    }),
     localOnlyListPath: localOnlyList,
   }))
 

hypaware-core/plugins-workspace/codex/src/rollout-cwd.js

@@ -4,40 +4,55 @@ import fs from 'node:fs'
 import path from 'node:path'
 
 import { readRolloutSessionMeta } from '../../../../src/core/codex/rollout_session_meta.js'
+
+// `sessionIdFromPath` predates the thread/container distinction: what it lifts
+// out of a rollout file name is the THREAD id (`session_meta.payload.id`), which
+// is the only id a rollout name carries. Kept as the shared helper so this
+// resolver and the backfill read the same convention out of the same code.
 import { sessionIdFromPath } from './backfill.js'
 
 /**
  * @import { RolloutCwdResolver, RolloutCwdResolverOptions, RolloutDirent } from './types.js'
  */
 
-// A negative resolution (no cwd found — the rollout is not yet written on the
+// A negative resolution (no cwd found: the rollout is not yet written on the
 // session's first exchange, or a momentary read error) is trusted only briefly
 // before it is re-checked, mirroring the usage-policy resolver's 5s TTL. A
 // positive cwd is cached for the session's life. Bounding the miss cache this
 // way stops a session-start race or a transient EMFILE/EIO from recording
-// `cwd = NULL` for a session's whole life — which would silently fail
+// `cwd = NULL` for a session's whole life, which would silently fail
 // `.hypignore` open for that session once the rollout became readable.
 const NEGATIVE_CACHE_TTL_MS = 5_000
 
 /**
- * Resolve a Codex session's `cwd` from its rollout file's `session_meta` line.
+ * Resolve a Codex **thread's** `cwd` from its rollout file's `session_meta` line.
  *
  * The ChatGPT-subscription route (`provider='chatgpt'`, `/backend-api/codex/*`)
- * carries no in-band cwd — `codex-tui` sends no `x-codex-turn-metadata` header
- * and the subscription protocol has no `metadata.cwd` field — so the live
+ * carries no in-band cwd (`codex-tui` sends no `x-codex-turn-metadata` header
+ * and the subscription protocol has no `metadata.cwd` field), so the live
  * exchange projector would record `cwd = NULL` and `.hypignore` would fail open
  * for the whole traffic class. Codex nonetheless writes `session_meta.cwd` into
- * the rollout (`<sessionsDir>/.../rollout-<ts>-<session_id>.jsonl`, line 1) at
- * session start, for both auth modes — the same value the codex backfill reads.
+ * the rollout (`<sessionsDir>/.../rollout-<ts>-<thread_id>.jsonl`, line 1) at
+ * session start, for both auth modes: the same value the codex backfill reads.
  * This resolver gives the live projector that fallback, so folder coverage is
  * client-independent and live rows carry the cwd backfill already sees.
  * @ref LLP 0083 [implements]: rollout is the live cwd fallback for Codex
  *
- * A resolved cwd is cached per session id for the session's life; a miss is
+ * **The lookup key is the THREAD id, never the session container.** A rollout is
+ * one thread's file: its name embeds `session_meta.payload.id` (the thread), not
+ * `payload.session_id` (the container the gateway partitions and drops on). The
+ * two are the same uuid on a root thread and diverge on a subagent one, which is
+ * exactly where handing over the container went wrong: a subagent turn resolved
+ * the ROOT thread's rollout, so a directory-scoped privacy control was evaluated
+ * against a directory the turn never ran in.
+ * @ref LLP 0083#decision [implements]: keyed on the thread id, and the located
+ * rollout must say so
+ *
+ * A resolved cwd is cached per thread id for the thread's life; a miss is
  * cached only briefly (`NEGATIVE_CACHE_TTL_MS`) so a not-yet-written or
  * momentarily-unreadable rollout is re-checked on a later exchange rather than
  * fixed at NULL. The scan itself is newest-first and returns on first match, so
- * a resolution touches the filesystem at most once per session per TTL window —
+ * a resolution touches the filesystem at most once per thread per TTL window:
  * bounded, not one walk per exchange. @ref LLP 0049#requirements R6
  *
  * @param {RolloutCwdResolverOptions} opts
@@ -48,26 +63,27 @@ export function createRolloutCwdResolver(opts) {
   const now = opts.now ?? Date.now
   const ttlMs = opts.ttlMs ?? NEGATIVE_CACHE_TTL_MS
   const readdirSync = opts.readdirSync ?? defaultReaddir
+  const log = opts.log
   /** @type {Map<string, { cwd: string | undefined, expiresAt: number }>} */
   const cache = new Map()
   return {
-    resolve(sessionId) {
-      if (typeof sessionId !== 'string' || sessionId.length === 0) return undefined
-      const cached = cache.get(sessionId)
+    resolve(threadId) {
+      if (typeof threadId !== 'string' || threadId.length === 0) return undefined
+      const cached = cache.get(threadId)
       if (cached !== undefined && cached.expiresAt > now()) return cached.cwd
-      const cwd = readRolloutCwd(sessionsDir, sessionId, readdirSync)
-      // A resolved cwd is trusted for the session's life (Infinity); a miss is
+      const cwd = readRolloutCwd(sessionsDir, threadId, readdirSync, log)
+      // A resolved cwd is trusted for the thread's life (Infinity); a miss is
       // trusted only for the TTL, so a transient miss is re-resolved instead of
       // becoming a permanent NULL cwd (which fails `.hypignore` open).
-      // @ref LLP 0083 [constrained-by]: a transient miss must not fix the cwd at NULL for the session's life
-      cache.set(sessionId, { cwd, expiresAt: cwd === undefined ? now() + ttlMs : Infinity })
+      // @ref LLP 0083 [constrained-by]: a transient miss must not fix the cwd at NULL for the thread's life
+      cache.set(threadId, { cwd, expiresAt: cwd === undefined ? now() + ttlMs : Infinity })
       return cwd
     },
   }
 }
 
 /**
- * Find the rollout whose filename embeds `sessionId` (via `sessionIdFromPath`,
+ * Find the rollout whose filename embeds `threadId` (via `sessionIdFromPath`,
  * shared with the backfill) and read its `session_meta.cwd`. Best-effort: a
  * missing sessions root, no matching rollout, an unreadable file, or a first
  * line that is not a `session_meta` record all yield `undefined` (fail open on
@@ -79,35 +95,79 @@ export function createRolloutCwdResolver(opts) {
  * @ref LLP 0150 [constrained-by]: one reader for `session_meta`, not one per caller
  *
  * @param {string} sessionsDir
- * @param {string} sessionId
+ * @param {string} threadId
  * @param {(dirPath: string, options: { withFileTypes: true }) => RolloutDirent[]} readdirSync
+ * @param {{ warn?: (message: string, fields?: Record<string, unknown>) => void }} [log]
  * @returns {string | undefined}
  */
-function readRolloutCwd(sessionsDir, sessionId, readdirSync) {
-  const rolloutPath = findRolloutFile(sessionsDir, sessionId, readdirSync)
+function readRolloutCwd(sessionsDir, threadId, readdirSync, log) {
+  const rolloutPath = findRolloutFile(sessionsDir, threadId, readdirSync)
   if (!rolloutPath) return undefined
-  return readRolloutSessionMeta(rolloutPath)?.cwd
+  // A line that is not a `session_meta` header, and an unreadable or empty
+  // file, are one answer: this file establishes nothing.
+  const meta = readRolloutSessionMeta(rolloutPath)
+  if (!meta) return undefined
+  // Identity guard: the file was located by NAME, and the naming convention is
+  // Codex's, not ours. Require the body to agree that this rollout records the
+  // thread that was asked for, so a renamed, copied, or convention-changed file
+  // yields "cwd unknown" rather than letting some OTHER thread's cwd silently
+  // decide this turn's `.hypignore` outcome and get stamped on its row. The
+  // shared reader takes the id off the RAW JSONL line rather than through
+  // Codex's `Deserialize` (LLP 0150 rule 1), which is what makes an absent
+  // `payload.id` read as absent here and refuse instead of matching.
+  // `meta.sessionId` (`payload.session_id`) is deliberately NOT consulted: the
+  // container is not what selects a rollout, and a rollout too old to carry one
+  // still records a perfectly good cwd for its thread.
+  // @ref LLP 0083#decision [implements]: a filename/body disagreement is a
+  // refusal, not a guess
+  const rolloutThreadId = meta.threadId
+  if (rolloutThreadId !== threadId) {
+    // The two refusals have different diagnoses, so they get different
+    // `error_kind`s: `thread_id_absent` is the one rollout shape the backfill
+    // still accepts (`buildSession` falls back to the filename id), so it points
+    // at the live/backfill divergence LLP 0083 records, while
+    // `thread_id_mismatch` points at a renamed or copied file. Same message so
+    // one query finds both.
+    log?.warn?.('plugin.codex.rollout_cwd_thread_mismatch', {
+      component: 'codex',
+      operation: 'rollout_cwd_resolve',
+      status: 'refused',
+      error_kind: rolloutThreadId === undefined ? 'thread_id_absent' : 'thread_id_mismatch',
+      wanted_thread_id: threadId,
+      rollout_thread_id: rolloutThreadId ?? null,
+      rollout: path.basename(rolloutPath),
+    })
+    return undefined
+  }
+  // `meta.cwd` is already predicated: core's `sessionMetaCwd` refuses a blank or
+  // relative `session_meta.cwd`, so a value that arrives here is an absolute
+  // path the policy matcher can resolve without supplying a base of its own.
+  // @ref LLP 0150#usable-cwd [constrained-by]
+  return meta.cwd
 }
 
 /**
- * Scan the sessions root for the rollout whose filename embeds `sessionId`,
+ * Scan the sessions root for the rollout whose filename embeds `threadId`,
  * newest-first: entries are visited in *descending* name order, so the
  * most-recent date dirs (`…/YYYY/MM/DD`) and rollout files come first. The
- * active session — the common lookup on the capture hot path — lives in the
+ * active session (the common lookup on the capture hot path) lives in the
  * newest date dir, so a typical resolution returns after touching only the
  * newest branch instead of walking the whole history oldest-first. Returns the
  * first match. A missing or unreadable directory contributes nothing rather
  * than throwing, and a genuinely absent rollout still yields `undefined`.
  *
+ * The name is a cheap prefilter, not the answer: the caller re-checks the
+ * located file's `session_meta.payload.id`, so a name that lies is caught.
+ *
  * The directory reader is injected (defaulting to `node:fs`) so tests can count
  * scans and prove the walk stays bounded. @ref LLP 0049#requirements R6
  *
  * @param {string} sessionsDir
- * @param {string} sessionId
+ * @param {string} threadId
  * @param {(dirPath: string, options: { withFileTypes: true }) => RolloutDirent[]} readdirSync
  * @returns {string | undefined}
  */
-function findRolloutFile(sessionsDir, sessionId, readdirSync) {
+function findRolloutFile(sessionsDir, threadId, readdirSync) {
   /** @type {string[]} */
   const dirs = [sessionsDir]
   while (dirs.length > 0) {
@@ -126,7 +186,7 @@ function findRolloutFile(sessionsDir, sessionId, readdirSync) {
     const subdirs = []
     for (const entry of entries) {
       const entryPath = path.join(dir, entry.name)
-      if (entry.isFile() && isRolloutFileName(entry.name) && sessionIdFromPath(entry.name) === sessionId) {
+      if (entry.isFile() && isRolloutFileName(entry.name) && sessionIdFromPath(entry.name) === threadId) {
         return entryPath
       }
       if (entry.isDirectory()) subdirs.push(entryPath)