From 9dcf7333731a13dea68f603681aebe4d265f7228 Mon Sep 17 00:00:00 2001 From: neutral-loop Date: Wed, 29 Jul 2026 23:11:52 +0000 Subject: [PATCH 1/6] Codex rollout cwd keys on the thread, not the session container (#459) `createRolloutCwdResolver` located a rollout by the id embedded in its FILENAME, which is the thread (`session_meta.payload.id`), while the projector called it with the session CONTAINER (`metadata.session_id` / the `session-id` header). The two are the same uuid on a root thread, so every hand-check passed; a subagent thread inherits its root's container and mints its own thread id, so a subagent turn on the ChatGPT-subscription route resolved the ROOT thread's cwd. That cwd is what `.hypignore` is evaluated against (LLP 0083 / LLP 0050), so a subagent running in an `ignore` directory whose root was not got RECORDED: a directory-scoped privacy control silently not applying. The same value is stamped on the row, so the row also claimed a directory the turn never ran in. - The resolver's key is now the thread id, and its contract says so. - The located rollout must confirm it: `payload.id`, read off the raw JSONL line (never a deserialized `session_meta`, which Codex back-fills `session_id` from `id` in), must equal the id asked for. A filename/body disagreement is a refusal, logged as `plugin.codex.rollout_cwd_thread_mismatch`, not another thread's cwd deciding this turn. `payload.session_id` is deliberately not read here: the container does not select a rollout, and a legacy rollout carrying none still records a good cwd for its thread. - The projector passes the thread id, falling back to the container only for a turn that states no thread and no subagent lineage (a root thread, where the two ids are one value). A turn that announces lineage without naming its own thread resolves no cwd rather than the root's. LLP 0083 amended in the same commit: the keying bullet was the source of the defect, plus a Correction section and the two-readers note. Fixes #459 Co-Authored-By: Claude --- .../codex/src/exchange-projector.js | 38 ++- .../plugins-workspace/codex/src/index.js | 9 +- .../codex/src/rollout-cwd.js | 80 ++++-- .../plugins-workspace/codex/src/types.d.ts | 19 +- ...83-codex-live-cwd-from-rollout.decision.md | 71 ++++- test/plugins/codex-rollout-cwd.test.js | 257 +++++++++++++++++- 6 files changed, 431 insertions(+), 43 deletions(-) diff --git a/hypaware-core/plugins-workspace/codex/src/exchange-projector.js b/hypaware-core/plugins-workspace/codex/src/exchange-projector.js index 496e77b9..68ed2b86 100644 --- a/hypaware-core/plugins-workspace/codex/src/exchange-projector.js +++ b/hypaware-core/plugins-workspace/codex/src/exchange-projector.js @@ -115,10 +115,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 = firstString(codexContext?.cwd, readRecordedCwd(reqBody)) - ?? (codexContext?.session_id ? rolloutCwd?.resolve(codexContext.session_id) : undefined) + ?? resolveRolloutCwd(rolloutCwd, codexContext) if (cwd) { const policy = resolver.resolve(cwd) if (policy.class === 'ignore') { @@ -208,6 +208,40 @@ 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), and that is the common + * subscription-route shape (a `session-id` header and nothing else). But the + * container is only usable while nothing says otherwise: if the turn announces + * subagent lineage without naming its own thread, its rollout is not identifiable + * from the wire, and an unknown cwd (LLP 0049 fails open, the row records NULL) + * is preferred to confidently stamping and enforcing the root's directory. A + * wrong cwd is a false statement about where a turn ran; an absent one is true. + * + * @param {RolloutCwdResolver | undefined} rolloutCwd + * @param {ReturnType} 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.parent_thread_id) return undefined + return codexContext.session_id ? rolloutCwd.resolve(codexContext.session_id) : undefined +} + // --------------------------------------------------------------------- // Provider routing // --------------------------------------------------------------------- diff --git a/hypaware-core/plugins-workspace/codex/src/index.js b/hypaware-core/plugins-workspace/codex/src/index.js index 3a29637b..59dac9e3 100644 --- a/hypaware-core/plugins-workspace/codex/src/index.js +++ b/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, })) diff --git a/hypaware-core/plugins-workspace/codex/src/rollout-cwd.js b/hypaware-core/plugins-workspace/codex/src/rollout-cwd.js index b87164b5..ab5c9d98 100644 --- a/hypaware-core/plugins-workspace/codex/src/rollout-cwd.js +++ b/hypaware-core/plugins-workspace/codex/src/rollout-cwd.js @@ -3,6 +3,10 @@ import fs from 'node:fs' import path from 'node:path' +// `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 { isPlainObject, parseMaybeJson, stringValue } from 'hypaware/core/util' @@ -26,24 +30,34 @@ const FIRST_LINE_MAX_BYTES = 64 * 1024 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 * 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 (`/.../rollout--.jsonl`, line 1) at + * the rollout (`/.../rollout--.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 @@ -54,49 +68,76 @@ 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} */ 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 }) + 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 * a genuinely absent rollout, matching the nullable `cwd` column). * * @param {string} sessionsDir - * @param {string} sessionId + * @param {string} threadId * @param {(dirPath: string, options: { withFileTypes: true }) => RolloutDirent[]} readdirSync + * @param {{ warn?: (message: string, fields?: Record) => 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 const firstLine = readFirstLine(rolloutPath) if (!firstLine) return undefined const row = parseMaybeJson(firstLine) if (!isPlainObject(row) || stringValue(row.type) !== 'session_meta') return undefined const payload = isPlainObject(row.payload) ? row.payload : 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 id + // is read off the raw JSONL line (not a deserialized `session_meta`), so an + // absent `payload.id` reads as absent and refuses instead of matching. + // `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 = stringValue(payload?.id) + if (rolloutThreadId !== threadId) { + log?.warn?.('plugin.codex.rollout_cwd_thread_mismatch', { + component: 'codex', + operation: 'rollout_cwd_resolve', + status: 'refused', + error_kind: 'thread_id_mismatch', + wanted_thread_id: threadId, + rollout_thread_id: rolloutThreadId ?? null, + rollout: path.basename(rolloutPath), + }) + return undefined + } return stringValue(payload?.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 @@ -105,15 +146,18 @@ function readRolloutCwd(sessionsDir, sessionId, readdirSync) { * 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) { @@ -132,7 +176,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) diff --git a/hypaware-core/plugins-workspace/codex/src/types.d.ts b/hypaware-core/plugins-workspace/codex/src/types.d.ts index 1ae7364a..e8fe4c91 100644 --- a/hypaware-core/plugins-workspace/codex/src/types.d.ts +++ b/hypaware-core/plugins-workspace/codex/src/types.d.ts @@ -58,14 +58,19 @@ export interface CodexRolloutItem { } /** - * Resolves a Codex session's `cwd` from its rollout `session_meta` line, the + * Resolves a Codex **thread's** `cwd` from its rollout `session_meta` line, the * live projector's fallback when the request carries no in-band cwd (the * ChatGPT-subscription route). Injectable so the projector can be tested * without a real sessions tree. */ export interface RolloutCwdResolver { - /** The rollout-recorded cwd for `sessionId`, or `undefined` when unknown. */ - resolve(sessionId: string): string | undefined + /** + * The rollout-recorded cwd for the THREAD `threadId` + * (`session_meta.payload.id`, the id a rollout file name embeds), or + * `undefined` when unknown. Passing the session CONTAINER instead resolves the + * root thread's cwd for every subagent thread in it. @ref LLP 0083#decision + */ + resolve(threadId: string): string | undefined } /** @@ -93,6 +98,14 @@ export interface RolloutCwdResolverOptions { ttlMs?: number /** Injectable `withFileTypes` directory reader; defaults to `node:fs.readdirSync`. */ readdirSync?: (dirPath: string, options: { withFileTypes: true }) => RolloutDirent[] + /** + * Optional logger for the identity guard's refusal (a located rollout whose + * `session_meta.payload.id` is not the thread that was asked for). Optional + * because the resolver is otherwise dependency-free and unit-testable; the + * plugin wires `ctx.log` in `index.js`. Structurally a `PluginLogger` subset + * so either can be passed. + */ + log?: { warn?: (message: string, fields?: Record) => void } } export interface CodexAttachOptions { diff --git a/llp/0083-codex-live-cwd-from-rollout.decision.md b/llp/0083-codex-live-cwd-from-rollout.decision.md index cd5c3143..89128a80 100644 --- a/llp/0083-codex-live-cwd-from-rollout.decision.md +++ b/llp/0083-codex-live-cwd-from-rollout.decision.md @@ -5,7 +5,7 @@ **Systems:** Plugins, Gateway, Sources **Author:** Phil / Claude **Date:** 2026-07-07 -**Related:** LLP 0030, LLP 0032, LLP 0049, LLP 0050 +**Related:** LLP 0030, LLP 0032, LLP 0049, LLP 0050, LLP 0066, LLP 0067 > The `@hypaware/codex` **live** exchange projector resolves an exchange's `cwd` > from the session's local rollout (`session_meta.cwd`) when the request carries @@ -58,14 +58,35 @@ Codex now has the symmetric fallback. - **In-band stays the fast path.** A fresh in-band `cwd` short-circuits before any filesystem work; the rollout is consulted **only** on a miss. -- **Keyed on the codex session id.** The live path already resolves it - (`session-id` header / turn metadata); the rollout filename embeds it, matched - via the `sessionIdFromPath` helper shared with the backfill. Only a real Codex - session has a rollout, so non-codex traffic never scans. -- **First line only, cached per session id.** The rollout is written at session +- **Keyed on the codex thread id, and the rollout must confirm it.** A rollout is + one **thread's** file: its name embeds `session_meta.payload.id` (the thread), + matched via the `sessionIdFromPath` helper shared with the backfill (a helper + whose name predates this distinction). It does **not** embed the session + container `payload.session_id`, which is the row's partition key and the session + opt-out's key ([LLP 0030](./0030-session-id-partition-key.decision.md#decision)), + so the container is not what selects a rollout. The live path resolves the + thread from the turn metadata's `thread_id` or the `thread-id` header. Only a + real Codex thread has a rollout, so non-codex traffic never scans. The name is a + cheap prefilter, not the answer: the located file's `payload.id` is re-checked + against the id asked for, read off the **raw** JSONL line so an absent field + reads as absent, and a disagreement is a **refusal** (cwd unknown) rather than + another thread's cwd deciding this turn. `payload.session_id` is deliberately + not consulted here: a rollout too old to carry a container still records a + perfectly good cwd for its thread. +- **When no thread is stated, the container is usable only for a root thread.** + The common subscription-route request carries a `session-id` header and nothing + else, and for a root thread that value *is* the thread id, so the fallback still + works. It is abandoned the moment the turn announces subagent lineage + (`thread_source = subagent`, or a `parent_thread_id`) without naming its own + thread: that turn's rollout is not identifiable from the wire, and an unknown + cwd (fails open per [LLP 0049](./0049-hypignore-usage-policy.spec.md), row + records NULL) is preferred to confidently enforcing and stamping the root's + directory. A wrong cwd is a false statement about where a turn ran; an absent + one is true. +- **First line only, cached per thread id.** The rollout is written at session start, so it exists before the first exchange projects (earlier and more reliably than Claude's sidecar, which has a known session-start race). Reading a - bounded prefix and caching per session id — including misses — keeps the capture + bounded prefix and caching per thread id (including misses) keeps the capture hot path free of unbounded fs work ([LLP 0049 R6](./0049-hypignore-usage-policy.spec.md#requirements)). - **One resolved `cwd`, used twice.** The same value feeds the `.hypignore` drop and the row's stamped `cwd`, so live rows now carry the cwd the backfill reads @@ -88,6 +109,32 @@ Codex now has the symmetric fallback. projector on its existing synchronous seam (the usage-policy resolver it already uses is synchronous too). +## Correction: the first cut keyed on the container (issue #459) + +As first landed, the resolver was **called** with the session container +(`metadata.session_id` / the `session-id` header) while it **located** the rollout +by the thread id in the filename. The two coincide on a root thread, so every +hand-check passed; they diverge on a **subagent** thread, which inherits its +root's container and mints its own thread id. So a subagent turn on the +subscription route resolved the **root's** rollout, and the root's `cwd` then +decided the `.hypignore` outcome ([LLP 0050](./0050-ignore-enforced-in-adapters.decision.md)) +and was stamped on the row. + +That is a directory-scoped privacy control silently not applying: a subagent +running in an `ignore` directory whose root did not was **recorded**. (The mirror +case, an over-drop, loses data but leaks nothing.) The identifier bug is the same +root cause as [issue #453](https://github.com/hyparam/hypaware/issues/453) on the +`hyp session` resolver ([LLP 0067](./0067-session-opt-out.design.md#cli-session-id)), +on a different call path: there the container had to be read *out of* the thread's +rollout, here the thread is what selects the rollout in the first place. Both now +refuse rather than accept an id whose provenance does not check out. + +The bullets above state the corrected contract. Two rules carried across from +that resolver rather than re-derived: parse the **raw** `session_meta` line (Codex +back-fills `session_id` from `id` in its own deserializer, so a struct-shaped read +cannot tell a legacy rollout from a root one), and treat an id that cannot be +confirmed as unresolvable. + ## Consequences - Code that lands this carries `@ref LLP 0083 [implements]` on the new @@ -101,6 +148,16 @@ Codex now has the symmetric fallback. - No cache schema, export driver, or gateway change: purely a projection-time cwd source, exactly like the existing Codex/Claude adapter drops ([LLP 0050](./0050-ignore-enforced-in-adapters.decision.md)). +- The identity guard's refusal is **logged** (`plugin.codex.rollout_cwd_thread_mismatch`, + warn), because its only other trace is a row with `cwd = NULL`, which is + indistinguishable from the ordinary not-yet-written rollout. The resolver takes + an optional `log` and `index.js` passes `ctx.log`. +- `codex/src/rollout-cwd.js` and `ai-gateway/src/session_command.js`'s + `readRolloutMeta` remain **two readers of the same first line** with different + needs (a hot-path cwd lookup per thread versus a one-shot CLI scan that must + report the container). They now agree on the discipline (raw line, `session_meta` + type guard, absent means refuse). Folding them into one shared reader is a + worthwhile follow-up, not a requirement of this correction. - **Prospective only.** Like the rest of [LLP 0049](./0049-hypignore-usage-policy.spec.md#prospective-only), this gates *future* live recording; rows already written with `cwd = NULL` are untouched (a `hyp backfill` re-import, which reads the rollout, is the path to diff --git a/test/plugins/codex-rollout-cwd.test.js b/test/plugins/codex-rollout-cwd.test.js index bf8cb041..4aa7e27b 100644 --- a/test/plugins/codex-rollout-cwd.test.js +++ b/test/plugins/codex-rollout-cwd.test.js @@ -30,18 +30,22 @@ function ignoringResolver(ignoredDir) { } /** - * A fake rollout cwd resolver: maps a session id to the cwd its rollout would + * A fake rollout cwd resolver: maps a thread id to the cwd its rollout would * carry. Used for the projector-wiring tests (no fs needed); the file-reading * behaviour of the real resolver is covered separately below. * - * @param {Record} bySession - * @returns {{ resolve(sessionId: string): string | undefined }} + * @param {Record} byThread + * @returns {{ resolve(threadId: string): string | undefined }} */ -function fakeRolloutCwd(bySession) { - return { resolve: (sessionId) => bySession[sessionId] } +function fakeRolloutCwd(byThread) { + return { resolve: (threadId) => byThread[threadId] } } -// A realistic subscription-route session id: a UUID the rollout filename embeds. +// A realistic subscription-route ROOT session: the container and the root thread +// id are the same uuid, which is the id the rollout filename embeds. The tests +// below that state only a `session-id` header therefore exercise the +// root-thread fallback (LLP 0083); the subagent case, where the two ids diverge, +// is the #459 block further down. const SUBSCRIPTION_SESSION_ID = '019e60b5-1111-4222-8333-444455556666' // --------------------------------------------------------------------- @@ -117,9 +121,9 @@ test('an in-band cwd stays the fast path and short-circuits the rollout lookup', }) // --------------------------------------------------------------------- -// createRolloutCwdResolver: reads session_meta.cwd from the session's rollout -// file (the same source backfill reads), keyed by the session id embedded in -// the rollout filename, cached per session id (LLP 0049 R6). +// createRolloutCwdResolver: reads session_meta.cwd from the thread's rollout +// file (the same source backfill reads), keyed by the thread id embedded in +// the rollout filename, cached per thread id (LLP 0049 R6). // --------------------------------------------------------------------- test('createRolloutCwdResolver reads session_meta.cwd from the session rollout', async () => { @@ -280,17 +284,246 @@ test('a newest-dir rollout is found without descending the older-date branch; an assert.equal(resolver2.resolve(OLD_SESSION_ID), '/work/old') }) +// --------------------------------------------------------------------- +// Regression (#459): a rollout file is one THREAD's file - its name embeds +// `session_meta.payload.id`, not the session container `payload.session_id`. A +// subagent thread inherits its root's container but mints its own thread id, so +// keying the lookup on the container resolved the ROOT thread's rollout and the +// root's cwd decided the subagent turn's `.hypignore` outcome (LLP 0083 / +// LLP 0050). Both directions are exercised: the leak (recorded when it should +// have been dropped) is the one that matters. +// --------------------------------------------------------------------- + +// Codex mints a session container from its root thread's id, so for the ROOT +// thread the two are the same uuid. That coincidence is what hid the defect. +const ROOT_THREAD_ID = '019e60b5-aaaa-4222-8333-444455556666' +const ROOT_SESSION_ID = ROOT_THREAD_ID +// A subagent thread: inherits the container above, mints its own thread id, and +// (the point of the bug) can be running somewhere else entirely. +const SUBAGENT_THREAD_ID = '019e60b5-bbbb-4222-8333-444455556666' + +test('a subagent turn is .hypignore-dropped by ITS OWN rollout cwd, not the root thread\'s', async () => { + // The root ran in a recorded directory; the subagent ran in an ignored one. + const sessionsDir = await writeSubagentPair({ + rootCwd: '/work/clean/root', + subagentCwd: '/work/ignored/sub', + }) + const projector = createCodexExchangeProjector({ + resolver: ignoringResolver('/work/ignored'), + rolloutCwd: createRolloutCwdResolver({ sessionsDir }), + }) + const projection = projector.project(subagentTurn(), context()) + // Keyed on the container, this resolved `/work/clean/root` and RECORDED a turn + // the user's `.hypignore` said to drop: a directory-scoped privacy control + // silently not applying (LLP 0049 R1). + assert.equal(projection, USAGE_POLICY_DROP) +}) + +test('a subagent turn outside an ignored root is recorded, with its own cwd on the row', async () => { + // The reverse pair: the ROOT is the ignored one, the subagent is not. + const sessionsDir = await writeSubagentPair({ + rootCwd: '/work/ignored/root', + subagentCwd: '/work/clean/sub', + }) + const projector = createCodexExchangeProjector({ + resolver: ignoringResolver('/work/ignored'), + rolloutCwd: createRolloutCwdResolver({ sessionsDir }), + }) + const projection = /** @type {any} */ (projector.project(subagentTurn(), context())) + // Keyed on the container this over-dropped (loses data, leaks nothing) AND + // would have stamped the root's directory on the row. + assert.ok(projection && projection !== USAGE_POLICY_DROP) + assert.equal(projection.cwd, '/work/clean/sub') +}) + +test('the root thread of the same session still resolves its own cwd', async () => { + // The fix must not trade the subagent for the root: a root turn states its + // thread id too (equal to the container here), and resolves the root rollout. + const sessionsDir = await writeSubagentPair({ + rootCwd: '/work/ignored/root', + subagentCwd: '/work/clean/sub', + }) + const projector = createCodexExchangeProjector({ + resolver: ignoringResolver('/work/ignored'), + rolloutCwd: createRolloutCwdResolver({ sessionsDir }), + }) + const projection = projector.project(exchange({ + path: '/backend-api/codex/responses', + provider: 'chatgpt', + request_headers: JSON.stringify({ + 'session-id': ROOT_SESSION_ID, + 'thread-id': ROOT_THREAD_ID, + }), + request_body: JSON.stringify({ model: 'gpt-5-codex', input: 'root work' }), + response_body: JSON.stringify({ output_text: 'ok' }), + }), context()) + assert.equal(projection, USAGE_POLICY_DROP) +}) + +test('a legacy rollout pair with no session_id still resolves each thread\'s own cwd', async () => { + // A Codex old enough to predate `session_meta.session_id` records no container + // at all. The container is not what selects a rollout, so its absence must not + // make the cwd unresolvable - the thread id is on the filename and in + // `payload.id`, which is all this lookup needs. + const sessionsDir = await writeSubagentPair({ + rootCwd: '/work/clean/root', + subagentCwd: '/work/ignored/sub', + legacy: true, + }) + const resolver = createRolloutCwdResolver({ sessionsDir }) + assert.equal(resolver.resolve(ROOT_THREAD_ID), '/work/clean/root') + assert.equal(resolver.resolve(SUBAGENT_THREAD_ID), '/work/ignored/sub') + + const projector = createCodexExchangeProjector({ + resolver: ignoringResolver('/work/ignored'), + rolloutCwd: createRolloutCwdResolver({ sessionsDir }), + }) + assert.equal(projector.project(subagentTurn(), context()), USAGE_POLICY_DROP) +}) + +test('a rollout whose body disagrees with its filename is refused, not silently used', async () => { + // The filename convention is Codex's, not ours, so the name is only a + // prefilter. A copied, renamed, or convention-changed file must yield "cwd + // unknown" rather than let some OTHER thread's cwd decide this turn. + const sessionsDir = await fs.mkdtemp(path.join(os.tmpdir(), 'codex-rollout-cwd-')) + await fs.writeFile( + path.join(sessionsDir, `rollout-2026-07-07T10-00-00-${SUBAGENT_THREAD_ID}.jsonl`), + metaLine({ id: ROOT_THREAD_ID, session_id: ROOT_SESSION_ID, cwd: '/work/some/other/thread' }), + 'utf8' + ) + const log = recordingLog() + const resolver = createRolloutCwdResolver({ sessionsDir, log }) + assert.equal(resolver.resolve(SUBAGENT_THREAD_ID), undefined) + // The refusal's only other trace would be a row with cwd = NULL, which looks + // exactly like the ordinary not-yet-written rollout, so it is logged. + assert.deepEqual( + log.warns.map((w) => w.message), + ['plugin.codex.rollout_cwd_thread_mismatch'] + ) + assert.equal(log.warns[0].fields?.wanted_thread_id, SUBAGENT_THREAD_ID) + assert.equal(log.warns[0].fields?.rollout_thread_id, ROOT_THREAD_ID) +}) + +test('a subagent turn that states lineage but not its own thread id resolves no cwd', async () => { + // A defensive branch: Codex states `thread_id` in the same metadata as the + // lineage, so this shape is not observed in practice. If it ever arrives, the + // turn's own rollout is not identifiable from the wire, and the container + // would resolve the ROOT's rollout - the exact defect. Refusing (cwd unknown, + // which LLP 0049 fails open on, row records NULL) follows the same + // refuse-rather-than-guess direction PR #458 set for the `hyp session` + // resolver: do not act on an identifier whose provenance does not check out. + const sessionsDir = await writeSubagentPair({ + rootCwd: '/work/ignored/root', + subagentCwd: '/work/clean/sub', + }) + const projector = createCodexExchangeProjector({ + resolver: ignoringResolver('/work/ignored'), + rolloutCwd: createRolloutCwdResolver({ sessionsDir }), + }) + const projection = /** @type {any} */ (projector.project(exchange({ + path: '/backend-api/codex/responses', + provider: 'chatgpt', + request_headers: JSON.stringify({ + 'session-id': ROOT_SESSION_ID, + 'parent-thread-id': ROOT_THREAD_ID, + }), + request_body: JSON.stringify({ model: 'gpt-5-codex', input: 'hi' }), + response_body: JSON.stringify({ output_text: 'ok' }), + }), context())) + assert.ok(projection && projection !== USAGE_POLICY_DROP) + assert.equal(projection.cwd, undefined, 'an unknown cwd is recorded as unknown, not as the root\'s') +}) + // --------------------------------------------------------------------- // Fixtures // --------------------------------------------------------------------- /** * A `session_meta` first line (plus trailing newline) carrying a `cwd`. - * @param {string} sessionId + * @param {string} threadId * @param {string} cwd */ -function sessionMeta(sessionId, cwd) { - return JSON.stringify({ type: 'session_meta', payload: { id: sessionId, cwd } }) + '\n' +function sessionMeta(threadId, cwd) { + return JSON.stringify({ type: 'session_meta', payload: { id: threadId, cwd } }) + '\n' +} + +/** + * A `session_meta` first line (plus trailing newline) from an explicit payload, + * so a test can state the thread id, the container, and the lineage separately. + * + * @param {Record} payload + */ +function metaLine(payload) { + return JSON.stringify({ + timestamp: '2026-07-07T10:00:00.000Z', + type: 'session_meta', + payload, + }) + '\n' +} + +/** + * A fresh sessions tree holding a subagent-shaped rollout PAIR: a root thread + * (whose id is also the session container) and a subagent thread that inherits + * that container, mints its own thread id, and records its own `cwd`. With + * `legacy: true` neither rollout carries a `session_id` field at all, the shape + * a pre-container Codex writes. + * + * @param {{ rootCwd: string, subagentCwd: string, legacy?: boolean }} opts + * @returns {Promise} the sessions root + */ +async function writeSubagentPair(opts) { + const sessionsDir = await fs.mkdtemp(path.join(os.tmpdir(), 'codex-rollout-cwd-')) + const day = path.join(sessionsDir, '2026', '07', '07') + await fs.mkdir(day, { recursive: true }) + const container = opts.legacy ? {} : { session_id: ROOT_SESSION_ID } + await fs.writeFile( + path.join(day, `rollout-2026-07-07T10-00-00-${ROOT_THREAD_ID}.jsonl`), + metaLine({ id: ROOT_THREAD_ID, ...container, cwd: opts.rootCwd, originator: 'codex-tui' }), + 'utf8' + ) + await fs.writeFile( + path.join(day, `rollout-2026-07-07T10-05-00-${SUBAGENT_THREAD_ID}.jsonl`), + metaLine({ + id: SUBAGENT_THREAD_ID, + ...container, + parent_thread_id: ROOT_THREAD_ID, + thread_source: 'subagent', + cwd: opts.subagentCwd, + originator: 'codex-tui', + }), + 'utf8' + ) + return sessionsDir +} + +/** + * A subscription-route turn from the SUBAGENT thread of `ROOT_SESSION_ID`. Shaped + * as codex-tui sends it: no `x-codex-turn-metadata` (so no in-band cwd), just the + * identity headers, which name the thread and the container separately. + */ +function subagentTurn() { + return exchange({ + path: '/backend-api/codex/responses', + provider: 'chatgpt', + request_headers: JSON.stringify({ + 'session-id': ROOT_SESSION_ID, + 'thread-id': SUBAGENT_THREAD_ID, + 'parent-thread-id': ROOT_THREAD_ID, + }), + request_body: JSON.stringify({ model: 'gpt-5-codex', input: 'secret subagent work' }), + response_body: JSON.stringify({ output_text: 'ok' }), + }) +} + +/** A logger that records the `warn` calls a test wants to assert on. */ +function recordingLog() { + /** @type {{ message: string, fields?: Record }[]} */ + const warns = [] + return { + warns, + /** @param {string} message @param {Record} [fields] */ + warn(message, fields) { warns.push({ message, fields }) }, + } } /** From 636cb087ed03983bc99c9d6aa44b34a43a6e805a Mon Sep 17 00:00:00 2001 From: neutral-reconciler Date: Wed, 29 Jul 2026 23:27:03 +0000 Subject: [PATCH 2/6] Review: name the container fallback's residual gap and cover the two untested guards Review of PR #462 (head 9dcf733) found the container fallback's safety argument does not hold for the client it exists for, and two of the new rules were unenforced by the suite. - The lineage refusal (`thread_source = subagent` / `parent_thread_id`) is only reachable when the client volunteers its lineage: `thread_source` comes from `x-codex-turn-metadata` alone and `parent_thread_id` from that header or `parent-thread-id`. `codex-tui` sends none of them on the subscription route, which is precisely why the rollout fallback exists. So for that client the refusal cannot fire, the container fallback is the ONLY path, and a `codex-tui` subagent turn still resolves the ROOT thread's cwd: the #459 defect, narrowed to one shape rather than closed. Verified against the real projector and the real usage-policy resolver: a turn with just a `session-id` header records `/work/clean/root` while its own rollout says `/work/ignored/sub`. Asserted as a DOCUMENTED GAP test and named in LLP 0083 (`#container-fallback-gap`) with the open empirical question, rather than left as "not observed in practice". Dropping the fallback is not the answer: it returns every `codex-tui` turn, root threads included, to `cwd = NULL`. - Mutation testing: deleting the `thread_source === 'subagent'` disjunct left the suite green (the existing case states lineage via the `parent-thread-id` header only), and so did treating an absent `payload.id` as a match, though both code and LLP state that rule. One test each; both now redden their own guard. - LLP 0083 records the one live/backfill divergence the identity guard introduces: a `session_meta` with a `cwd` and no `id` is tolerated by `buildSession` (filename fallback) and refused live, and refusing means the turn is recorded, not dropped. Co-Authored-By: Claude --- .../codex/src/exchange-projector.js | 17 ++++ ...83-codex-live-cwd-from-rollout.decision.md | 36 ++++++++ test/plugins/codex-rollout-cwd.test.js | 82 +++++++++++++++++++ 3 files changed, 135 insertions(+) diff --git a/hypaware-core/plugins-workspace/codex/src/exchange-projector.js b/hypaware-core/plugins-workspace/codex/src/exchange-projector.js index 68ed2b86..484aaccf 100644 --- a/hypaware-core/plugins-workspace/codex/src/exchange-projector.js +++ b/hypaware-core/plugins-workspace/codex/src/exchange-projector.js @@ -231,6 +231,23 @@ export function createCodexExchangeProjector(opts = {}) { * is preferred to confidently stamping and enforcing the root's directory. A * wrong cwd is a false statement about where a turn ran; an absent one is true. * + * **Known residual gap, not closed by the branch above.** The lineage the refusal + * keys on is only readable when the client volunteers it: `thread_source` comes + * from `x-codex-turn-metadata` alone, and `parent_thread_id` from that header or + * a `parent-thread-id` header. `codex-tui` sends none of them on the subscription + * route (that is why this fallback exists at all), so for that client the refusal + * cannot fire and the container fallback is not a defensive branch but the ONLY + * path. A `codex-tui` subagent turn would therefore still resolve the root's cwd, + * which is the #459 defect. Whether that shape exists is an open **empirical** + * question about a client HypAware does not own (does the subscription route ever + * carry the turn's own thread id, and does `codex-tui` spawn subagent threads?), + * and it is the check issue #459 asked for. Dropping the fallback is not the + * answer: it would return every `codex-tui` turn, root threads included, to + * `cwd = NULL` and fail `.hypignore` open for the whole traffic class, which is + * the regression LLP 0083 exists to prevent. + * @ref LLP 0083#decision [constrained-by]: the container fallback is bounded by + * what the wire states, so its residual gap is documented rather than silent + * * @param {RolloutCwdResolver | undefined} rolloutCwd * @param {ReturnType} codexContext * @returns {string | undefined} diff --git a/llp/0083-codex-live-cwd-from-rollout.decision.md b/llp/0083-codex-live-cwd-from-rollout.decision.md index 89128a80..3261ab53 100644 --- a/llp/0083-codex-live-cwd-from-rollout.decision.md +++ b/llp/0083-codex-live-cwd-from-rollout.decision.md @@ -83,6 +83,33 @@ Codex now has the symmetric fallback. records NULL) is preferred to confidently enforcing and stamping the root's directory. A wrong cwd is a false statement about where a turn ran; an absent one is true. +- **The container fallback leaves a documented residual gap.** {#container-fallback-gap} + The lineage the refusal above keys on is only readable when the client + volunteers it: `thread_source` comes from `x-codex-turn-metadata` only, and + `parent_thread_id` from that header or a `parent-thread-id` header. `codex-tui` + sends none of them on the subscription route, which is the very reason this + fallback exists, so for that client the refusal **cannot fire** and the + container fallback is not a defensive branch but the only path. A `codex-tui` + subagent turn would therefore still resolve the root thread's cwd: the #459 + defect, narrowed to one client shape rather than closed. Whether that shape + exists is an open **empirical** question about a client HypAware does not own + (does the subscription route ever state the turn's own thread id, and does + `codex-tui` spawn subagent threads at all?), and it is exactly the check + [issue #459](https://github.com/hyparam/hypaware/issues/459) asked for before + option 1 was adopted. It is not answerable from this repo: the only + turn-metadata shapes here are synthetic smoke fixtures, and the live Desktop + route has never been confirmed against real hardware + ([LLP 0141](./0141-codex-desktop-rides-the-codex-adapter.decision.md)). + Removing the fallback is **not** the answer: it returns every `codex-tui` turn, + root threads included, to `cwd = NULL` and fails `.hypignore` open for the whole + traffic class, the regression this document exists to prevent. The narrower + option, deciding ambiguity from disk (refuse the container key when some other + rollout in the tree declares `session_id = ` with a different `id`, + so the container demonstrably holds more than one thread), is decidable locally + but costs the newest-first short-circuit: proving uniqueness means visiting + every candidate, not returning on the first name match, so it trades + [LLP 0049 R6](./0049-hypignore-usage-policy.spec.md#requirements) and is left + open rather than taken here. - **First line only, cached per thread id.** The rollout is written at session start, so it exists before the first exchange projects (earlier and more reliably than Claude's sidecar, which has a known session-start race). Reading a @@ -158,6 +185,15 @@ confirmed as unresolvable. report the container). They now agree on the discipline (raw line, `session_meta` type guard, absent means refuse). Folding them into one shared reader is a worthwhile follow-up, not a requirement of this correction. +- **One narrow live/backfill divergence the identity guard introduces.** A + `session_meta` payload carrying a `cwd` but **no** `id` is tolerated by the + backfill (`buildSession` falls back to the id on the filename) and now refused + by the live resolver. That is deliberate (an unconfirmable id is unresolvable), + and the direction is safe by [LLP 0049](./0049-hypignore-usage-policy.spec.md)'s + fail-open rule rather than by fail-closed: refusing means `cwd` unknown, which + means the turn is **recorded**. Codex's own `SessionMeta` always writes `id`, so + the shape is not expected; it is named here because the parity claim above + ("live rows carry the cwd backfill reads") now has this one exception. - **Prospective only.** Like the rest of [LLP 0049](./0049-hypignore-usage-policy.spec.md#prospective-only), this gates *future* live recording; rows already written with `cwd = NULL` are untouched (a `hyp backfill` re-import, which reads the rollout, is the path to diff --git a/test/plugins/codex-rollout-cwd.test.js b/test/plugins/codex-rollout-cwd.test.js index 4aa7e27b..b09b6edc 100644 --- a/test/plugins/codex-rollout-cwd.test.js +++ b/test/plugins/codex-rollout-cwd.test.js @@ -434,6 +434,88 @@ test('a subagent turn that states lineage but not its own thread id resolves no assert.equal(projection.cwd, undefined, 'an unknown cwd is recorded as unknown, not as the root\'s') }) +test('a turn whose metadata states thread_source=subagent but no thread id resolves no cwd', async () => { + // The other half of the lineage refusal. The test above states lineage via the + // `parent-thread-id` HEADER; `thread_source` is readable only out of + // `x-codex-turn-metadata`, so without this case the `thread_source` disjunct + // could be deleted with the suite still green. + const sessionsDir = await writeSubagentPair({ + rootCwd: '/work/ignored/root', + subagentCwd: '/work/clean/sub', + }) + const projector = createCodexExchangeProjector({ + resolver: ignoringResolver('/work/ignored'), + rolloutCwd: createRolloutCwdResolver({ sessionsDir }), + }) + const projection = /** @type {any} */ (projector.project(exchange({ + path: '/backend-api/codex/responses', + provider: 'chatgpt', + request_headers: JSON.stringify({ + 'session-id': ROOT_SESSION_ID, + 'x-codex-turn-metadata': JSON.stringify({ session_id: ROOT_SESSION_ID, thread_source: 'subagent' }), + }), + request_body: JSON.stringify({ model: 'gpt-5-codex', input: 'hi' }), + response_body: JSON.stringify({ output_text: 'ok' }), + }), context())) + assert.ok(projection && projection !== USAGE_POLICY_DROP) + assert.equal(projection.cwd, undefined, 'stated lineage without a thread id must not fall back to the container') +}) + +test('a session_meta line with a cwd but no payload.id is refused, not matched by its filename', async () => { + // The identity guard reads the RAW line, so an absent `payload.id` is visible + // as absent and refuses. Pinned because it is a real divergence from the + // backfill, which falls back to the id on the filename (`buildSession`), and + // because refusing here means cwd unknown, which LLP 0049 fails OPEN on: the + // turn is recorded. Codex always writes `id`, so this pins the rule, not a + // shape in the field. + const sessionsDir = await fs.mkdtemp(path.join(os.tmpdir(), 'codex-rollout-cwd-')) + await fs.writeFile( + path.join(sessionsDir, `rollout-2026-07-07T10-00-00-${ROOT_THREAD_ID}.jsonl`), + metaLine({ cwd: '/work/idless', originator: 'codex-tui' }), + 'utf8' + ) + const log = recordingLog() + const resolver = createRolloutCwdResolver({ sessionsDir, log }) + assert.equal(resolver.resolve(ROOT_THREAD_ID), undefined) + assert.deepEqual(log.warns.map((w) => w.message), ['plugin.codex.rollout_cwd_thread_mismatch']) + assert.equal(log.warns[0].fields?.rollout_thread_id, null, 'an absent id is reported as absent, not as the wanted id') +}) + +test('DOCUMENTED GAP: a codex-tui-shaped subagent turn (container only) still resolves the ROOT cwd', async () => { + // @ref LLP 0083#container-fallback-gap [tests]: the container fallback is + // bounded by what the wire states, so the residual #459 exposure is asserted + // rather than left to be discovered again. + // + // The refusal above needs the client to VOLUNTEER its lineage, and codex-tui + // volunteers nothing on the subscription route (no `x-codex-turn-metadata`, no + // `parent-thread-id`) - which is the whole reason the rollout fallback exists. + // So for that client the container fallback is the only path, and a subagent + // turn is indistinguishable on the wire from its root: it resolves the ROOT's + // cwd and is RECORDED even though its own rollout says otherwise. Whether + // codex-tui ever produces this shape is an empirical question about a client + // this repo cannot observe (issue #459's "needs checking that the route + // actually has it"). Deleting the fallback is not the fix: it returns every + // codex-tui turn, root threads included, to `cwd = NULL` and fails + // `.hypignore` open for the entire traffic class. + const sessionsDir = await writeSubagentPair({ + rootCwd: '/work/clean/root', + subagentCwd: '/work/ignored/sub', + }) + const projector = createCodexExchangeProjector({ + resolver: ignoringResolver('/work/ignored'), + rolloutCwd: createRolloutCwdResolver({ sessionsDir }), + }) + const projection = /** @type {any} */ (projector.project(exchange({ + path: '/backend-api/codex/responses', + provider: 'chatgpt', + request_headers: JSON.stringify({ 'session-id': ROOT_SESSION_ID }), + request_body: JSON.stringify({ model: 'gpt-5-codex', input: 'secret subagent work' }), + response_body: JSON.stringify({ output_text: 'ok' }), + }), context())) + assert.ok(projection && projection !== USAGE_POLICY_DROP, 'still recorded: the gap this asserts') + assert.equal(projection.cwd, '/work/clean/root', 'still the ROOT thread\'s cwd, not the subagent\'s') +}) + // --------------------------------------------------------------------- // Fixtures // --------------------------------------------------------------------- From 629fdba9b049dc1fd86a24fa3b3a6cad58395541 Mon Sep 17 00:00:00 2001 From: neutral-reconciler Date: Wed, 29 Jul 2026 23:34:46 +0000 Subject: [PATCH 3/6] Review round 2: make the lineage refusal reachable, and correct it from Codex's own source Round 1 recorded the container fallback's residual gap as a doc note. Codex's own source (the `codex-rs` snapshot this PR already cites for `protocol.rs:3157-3184`) settles it, and the answer is a code fix. `CodexResponsesMetadata::compatibility_headers` shows what a Codex request actually carries: `x-codex-turn-metadata`, `x-codex-window-id`, `x-codex-parent-thread-id`, `x-openai-subagent`. The bare `thread-id`, `session-id` and `parent-thread-id` names the adapter reads appear nowhere in it. That made the refusal added by this PR dead in every branch: the fields it keyed on (`thread_source`, `parent_thread_id`) travel inside `x-codex-turn-metadata`, which also carries `thread_id`, so the thread-id path had already returned; and its header half read a name Codex does not send. - `resolveCodexContext` gains `subagent_signal`, folding in the two lineage headers Codex emits DIRECTLY, gated on their own value and not on the metadata blob. Those are the only lineage that survives a turn stating no thread id, so they are what make the refusal reachable. Deliberately not mirrored into `attributes` or the `parent_thread_id` column: widening what a row records is a separate change. - The refusal now consults it. One test per header; each reddens its own guard, as does removing the guard's use of the signal. - The remaining accepted case is narrowed to a turn stating a container and no lineage at all, which needs a client withholding its thread id AND every lineage signal on a subagent turn. The DOCUMENTED GAP test is rewritten to assert that, not a client-specific claim. - LLP 0083 records the header-name reading, that its "codex-tui does not send x-codex-turn-metadata" premise is at best version-specific (core emits it for every ordinary turn, `request_kind = Turn`), and that the durable fix is the body's `client_metadata.thread_id` - always present, unread today, and left out because it would newly populate `conversation_id` on rows that record null. Co-Authored-By: Claude --- .../codex/src/exchange-projector.js | 56 ++++++++++----- ...83-codex-live-cwd-from-rollout.decision.md | 69 +++++++++++------- test/plugins/codex-rollout-cwd.test.js | 70 +++++++++++++------ 3 files changed, 131 insertions(+), 64 deletions(-) diff --git a/hypaware-core/plugins-workspace/codex/src/exchange-projector.js b/hypaware-core/plugins-workspace/codex/src/exchange-projector.js index 484aaccf..e5513a0e 100644 --- a/hypaware-core/plugins-workspace/codex/src/exchange-projector.js +++ b/hypaware-core/plugins-workspace/codex/src/exchange-projector.js @@ -231,22 +231,23 @@ export function createCodexExchangeProjector(opts = {}) { * is preferred to confidently stamping and enforcing the root's directory. A * wrong cwd is a false statement about where a turn ran; an absent one is true. * - * **Known residual gap, not closed by the branch above.** The lineage the refusal - * keys on is only readable when the client volunteers it: `thread_source` comes - * from `x-codex-turn-metadata` alone, and `parent_thread_id` from that header or - * a `parent-thread-id` header. `codex-tui` sends none of them on the subscription - * route (that is why this fallback exists at all), so for that client the refusal - * cannot fire and the container fallback is not a defensive branch but the ONLY - * path. A `codex-tui` subagent turn would therefore still resolve the root's cwd, - * which is the #459 defect. Whether that shape exists is an open **empirical** - * question about a client HypAware does not own (does the subscription route ever - * carry the turn's own thread id, and does `codex-tui` spawn subagent threads?), - * and it is the check issue #459 asked for. Dropping the fallback is not the - * answer: it would return every `codex-tui` turn, root threads included, to - * `cwd = NULL` and fail `.hypignore` open for the whole traffic class, which is - * the regression LLP 0083 exists to prevent. - * @ref LLP 0083#decision [constrained-by]: the container fallback is bounded by - * what the wire states, so its residual gap is documented rather than silent + * **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 also carries `thread_id`, so a turn that states them has already + * been answered by the branch above: a refusal keyed only on those can never fire. + * The lineage that survives 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, 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. + * Dropping the fallback anyway is worse, not safer: 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} codexContext @@ -255,7 +256,7 @@ export function createCodexExchangeProjector(opts = {}) { 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.parent_thread_id) return undefined + if (codexContext.thread_source === 'subagent' || codexContext.subagent_signal) return undefined return codexContext.session_id ? rolloutCwd.resolve(codexContext.session_id) : undefined } @@ -722,6 +723,26 @@ function resolveCodexContext(input, provider, path, reqBody) { readStringKey(metadata, 'parent_thread_id'), readHeader(input.request_headers, '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, @@ -758,6 +779,7 @@ function resolveCodexContext(input, provider, path, reqBody) { thread_id, session_id, parent_thread_id, + subagent_signal, turn_id, thread_source, cwd: workspace?.path, diff --git a/llp/0083-codex-live-cwd-from-rollout.decision.md b/llp/0083-codex-live-cwd-from-rollout.decision.md index 3261ab53..f9f4f96d 100644 --- a/llp/0083-codex-live-cwd-from-rollout.decision.md +++ b/llp/0083-codex-live-cwd-from-rollout.decision.md @@ -83,33 +83,50 @@ Codex now has the symmetric fallback. records NULL) is preferred to confidently enforcing and stamping the root's directory. A wrong cwd is a false statement about where a turn ran; an absent one is true. -- **The container fallback leaves a documented residual gap.** {#container-fallback-gap} - The lineage the refusal above keys on is only readable when the client - volunteers it: `thread_source` comes from `x-codex-turn-metadata` only, and - `parent_thread_id` from that header or a `parent-thread-id` header. `codex-tui` - sends none of them on the subscription route, which is the very reason this - fallback exists, so for that client the refusal **cannot fire** and the - container fallback is not a defensive branch but the only path. A `codex-tui` - subagent turn would therefore still resolve the root thread's cwd: the #459 - defect, narrowed to one client shape rather than closed. Whether that shape - exists is an open **empirical** question about a client HypAware does not own - (does the subscription route ever state the turn's own thread id, and does - `codex-tui` spawn subagent threads at all?), and it is exactly the check - [issue #459](https://github.com/hyparam/hypaware/issues/459) asked for before - option 1 was adopted. It is not answerable from this repo: the only - turn-metadata shapes here are synthetic smoke fixtures, and the live Desktop - route has never been confirmed against real hardware - ([LLP 0141](./0141-codex-desktop-rides-the-codex-adapter.decision.md)). - Removing the fallback is **not** the answer: it returns every `codex-tui` turn, - root threads included, to `cwd = NULL` and fails `.hypignore` open for the whole - traffic class, the regression this document exists to prevent. The narrower - option, deciding ambiguity from disk (refuse the container key when some other - rollout in the tree declares `session_id = ` with a different `id`, - so the container demonstrably holds more than one thread), is decidable locally - but costs the newest-first short-circuit: proving uniqueness means visiting - every candidate, not returning on the first name match, so it trades +- **The lineage that guards the container fallback has to be a header, not the + metadata blob.** {#container-fallback-gap} `thread_source` and + `parent_thread_id` are read out of `x-codex-turn-metadata`, and that blob also + carries `thread_id`, so a turn stating them has already been resolved by the + thread-id key: a refusal keyed only on those can never fire. What survives a + turn stating no thread id is the lineage `codex-rs` emits as a **direct** + header, gated on its own value and not on the blob + (`CodexResponsesMetadata::compatibility_headers`): + `x-codex-parent-thread-id`, and `x-openai-subagent` (`review`, `compact`, + `collab_spawn`, `memory_consolidation`). Both are therefore consulted, which is + what makes the refusal reachable at all. The same source shows the two flat + identity keys, `session_id` and `thread_id`, are also always present in the + request **body** under `client_metadata`, which the adapter does not read + today: reading them would replace this fallback with the turn's own thread id + outright, and is the better long-term answer. It is not taken here because it + would newly populate `thread_id`, hence `conversation_id`, on rows that record + null for it today, which is a recorded-shape change needing its own decision. +- **What remains after that is bounded, and removing the fallback is worse.** A + turn stating a container, no thread id, and no lineage of any kind is taken as + the root thread it claims to be. That can only mis-resolve for a client that + withholds its thread id **and** withholds every lineage signal on a subagent + turn, and Codex withholds neither together. Dropping the fallback instead + 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. The remaining alternative, deciding + ambiguity from disk (refuse the container key when another rollout in the tree + declares `session_id = ` with a different `id`, so the container + demonstrably holds more than one thread), is decidable locally but costs the + newest-first short-circuit, since proving uniqueness means visiting every + candidate rather than returning on the first name match. That trades [LLP 0049 R6](./0049-hypignore-usage-policy.spec.md#requirements) and is left - open rather than taken here. + open rather than taken. +- **A note on the header names, which are not all Codex's.** `codex-rs` defines + `x-codex-turn-metadata`, `x-codex-window-id`, `x-codex-parent-thread-id` and + `x-openai-subagent`; the bare `thread-id`, `session-id` and `parent-thread-id` + the adapter also reads appear nowhere in it. Those reads are older than this + document and are kept (they cost nothing and some traffic shape motivated them), + but nothing should be *guarded* by them alone. The same reading says the premise + in Context below, that `codex-tui` does not send `x-codex-turn-metadata`, is at + best version-specific: the header is emitted from core for every ordinary turn + (`request_kind = Turn`), not from Desktop specifically. Confirming the live + header set against a real client is the open acceptance check, and it is + [LLP 0141](./0141-codex-desktop-rides-the-codex-adapter.decision.md)'s point + that no hermetic smoke can supply it. - **First line only, cached per thread id.** The rollout is written at session start, so it exists before the first exchange projects (earlier and more reliably than Claude's sidecar, which has a known session-start race). Reading a diff --git a/test/plugins/codex-rollout-cwd.test.js b/test/plugins/codex-rollout-cwd.test.js index b09b6edc..f25120b2 100644 --- a/test/plugins/codex-rollout-cwd.test.js +++ b/test/plugins/codex-rollout-cwd.test.js @@ -405,9 +405,7 @@ test('a rollout whose body disagrees with its filename is refused, not silently }) test('a subagent turn that states lineage but not its own thread id resolves no cwd', async () => { - // A defensive branch: Codex states `thread_id` in the same metadata as the - // lineage, so this shape is not observed in practice. If it ever arrives, the - // turn's own rollout is not identifiable from the wire, and the container + // The turn's own rollout is not identifiable from the wire, and the container // would resolve the ROOT's rollout - the exact defect. Refusing (cwd unknown, // which LLP 0049 fails open on, row records NULL) follows the same // refuse-rather-than-guess direction PR #458 set for the `hyp session` @@ -435,9 +433,8 @@ test('a subagent turn that states lineage but not its own thread id resolves no }) test('a turn whose metadata states thread_source=subagent but no thread id resolves no cwd', async () => { - // The other half of the lineage refusal. The test above states lineage via the - // `parent-thread-id` HEADER; `thread_source` is readable only out of - // `x-codex-turn-metadata`, so without this case the `thread_source` disjunct + // The other half of the lineage refusal. `thread_source` is readable only out + // of `x-codex-turn-metadata`, so without this case the `thread_source` disjunct // could be deleted with the suite still green. const sessionsDir = await writeSubagentPair({ rootCwd: '/work/ignored/root', @@ -461,6 +458,39 @@ test('a turn whose metadata states thread_source=subagent but no thread id resol assert.equal(projection.cwd, undefined, 'stated lineage without a thread id must not fall back to the container') }) +for (const header of ['x-codex-parent-thread-id', 'x-openai-subagent']) { + test(`a turn stating lineage only via ${header} resolves no cwd`, async () => { + // The lineage names `codex-rs` actually emits as DIRECT headers + // (`CodexResponsesMetadata::compatibility_headers`), gated on their own value + // and NOT on `x-codex-turn-metadata`. That independence is the whole point: + // every field the refusal previously keyed on travels inside the metadata + // blob, which also carries `thread_id`, so a refusal keyed only on those can + // never fire before the thread-id path has returned. These two are what make + // it reachable, so a subagent turn that names no thread of its own resolves + // no cwd instead of the root's. + const sessionsDir = await writeSubagentPair({ + rootCwd: '/work/ignored/root', + subagentCwd: '/work/clean/sub', + }) + const projector = createCodexExchangeProjector({ + resolver: ignoringResolver('/work/ignored'), + rolloutCwd: createRolloutCwdResolver({ sessionsDir }), + }) + const projection = /** @type {any} */ (projector.project(exchange({ + path: '/backend-api/codex/responses', + provider: 'chatgpt', + request_headers: JSON.stringify({ + 'session-id': ROOT_SESSION_ID, + [header]: header === 'x-openai-subagent' ? 'collab_spawn' : ROOT_THREAD_ID, + }), + request_body: JSON.stringify({ model: 'gpt-5-codex', input: 'hi' }), + response_body: JSON.stringify({ output_text: 'ok' }), + }), context())) + assert.ok(projection && projection !== USAGE_POLICY_DROP) + assert.equal(projection.cwd, undefined, `${header} must abandon the container fallback`) + }) +} + test('a session_meta line with a cwd but no payload.id is refused, not matched by its filename', async () => { // The identity guard reads the RAW line, so an absent `payload.id` is visible // as absent and refuses. Pinned because it is a real divergence from the @@ -481,22 +511,20 @@ test('a session_meta line with a cwd but no payload.id is refused, not matched b assert.equal(log.warns[0].fields?.rollout_thread_id, null, 'an absent id is reported as absent, not as the wanted id') }) -test('DOCUMENTED GAP: a codex-tui-shaped subagent turn (container only) still resolves the ROOT cwd', async () => { - // @ref LLP 0083#container-fallback-gap [tests]: the container fallback is - // bounded by what the wire states, so the residual #459 exposure is asserted - // rather than left to be discovered again. +test('DOCUMENTED GAP: a turn stating a container and NO lineage at all is taken as its root thread', async () => { + // @ref LLP 0083#container-fallback-gap [tests]: what the container fallback + // still accepts, asserted rather than left to be rediscovered. // - // The refusal above needs the client to VOLUNTEER its lineage, and codex-tui - // volunteers nothing on the subscription route (no `x-codex-turn-metadata`, no - // `parent-thread-id`) - which is the whole reason the rollout fallback exists. - // So for that client the container fallback is the only path, and a subagent - // turn is indistinguishable on the wire from its root: it resolves the ROOT's - // cwd and is RECORDED even though its own rollout says otherwise. Whether - // codex-tui ever produces this shape is an empirical question about a client - // this repo cannot observe (issue #459's "needs checking that the route - // actually has it"). Deleting the fallback is not the fix: it returns every - // codex-tui turn, root threads included, to `cwd = NULL` and fails - // `.hypignore` open for the entire traffic class. + // Every refusal above needs the turn to state SOMETHING - a thread id, a + // metadata `thread_source`, or one of the two lineage headers. A turn that + // states a container and nothing else is indistinguishable on the wire from the + // root thread it claims to be, so it resolves the container's rollout: correct + // for a root, the #459 defect for a subagent. It is bounded (it needs a client + // that withholds its thread id AND every lineage signal on a subagent turn, and + // Codex withholds neither together) and deleting the fallback is worse, since + // it returns every container-only turn, root threads included, to `cwd = NULL` + // and fails `.hypignore` open for that traffic class. The durable fix is the + // body's `client_metadata.thread_id`, which the adapter does not read yet. const sessionsDir = await writeSubagentPair({ rootCwd: '/work/clean/root', subagentCwd: '/work/ignored/sub', From 31b50b31bb46356c291cff80e4b1d7cf7336d31d Mon Sep 17 00:00:00 2001 From: neutral-reconciler Date: Wed, 29 Jul 2026 23:51:11 +0000 Subject: [PATCH 4/6] Review: correct LLP 0083's stale Decision thesis and clear the touched files of em dashes Three review findings on this branch, all in the convention/doc layer; no behavior change to the capture seam. 1. LLP 0083's `## Decision` still stated the superseded key. Its bolded thesis read "keyed on the session id the adapter already resolves" and the Context still wrote the rollout name as `rollout--.jsonl`, which is exactly the sentence the correction section says the defect came from. The keying bullet below it says "thread id", and the code annotates `@ref LLP 0083#decision [implements]: keyed on the thread id`, so an annotation pointed at prose that contradicted it. Both restated. 2. The identity guard logged one `error_kind` for two different diagnoses. An absent `payload.id` is the one rollout shape the backfill still accepts (LLP 0083 records it as a live/backfill divergence); a mismatching id is a renamed or copied file. They now report `thread_id_absent` and `thread_id_mismatch` under the same message, and both are asserted. 3. CLAUDE.md forbids the em dash anywhere. The branch added none, but left 27 in the files it rewrites, some inside the very JSDoc blocks it edited. Removed from all six touched files, with the punctuation each sentence wants. Checks: npm test 2857 pass / 8 fail, the pre-existing leave-command baseline verified identical on origin/master; npm run typecheck clean; npm run smoke -- gateway_codex_capture ok. The #459 regression gate was re-verified by restoring the three src files from origin/master: 9 of the 20 tests fail pre-fix, including the leak-direction case, and all 20 pass after. Co-Authored-By: Claude --- .../codex/src/exchange-projector.js | 2 +- .../codex/src/rollout-cwd.js | 24 ++++++++----- .../plugins-workspace/codex/src/types.d.ts | 2 +- ...83-codex-live-cwd-from-rollout.decision.md | 34 +++++++++++-------- test/plugins/codex-rollout-cwd.test.js | 18 ++++++---- 5 files changed, 49 insertions(+), 31 deletions(-) diff --git a/hypaware-core/plugins-workspace/codex/src/exchange-projector.js b/hypaware-core/plugins-workspace/codex/src/exchange-projector.js index e5513a0e..c2d7f5a5 100644 --- a/hypaware-core/plugins-workspace/codex/src/exchange-projector.js +++ b/hypaware-core/plugins-workspace/codex/src/exchange-projector.js @@ -922,7 +922,7 @@ function resolveConversationSource(provider) { * @param {Record} reqBody * @param {ReturnType} 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] diff --git a/hypaware-core/plugins-workspace/codex/src/rollout-cwd.js b/hypaware-core/plugins-workspace/codex/src/rollout-cwd.js index ab5c9d98..6610f85c 100644 --- a/hypaware-core/plugins-workspace/codex/src/rollout-cwd.js +++ b/hypaware-core/plugins-workspace/codex/src/rollout-cwd.js @@ -20,12 +20,12 @@ import { isPlainObject, parseMaybeJson, stringValue } from 'hypaware/core/util' // a long, large rollout. const FIRST_LINE_MAX_BYTES = 64 * 1024 -// 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 @@ -33,15 +33,15 @@ const NEGATIVE_CACHE_TTL_MS = 5_000 * 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 (`/.../rollout--.jsonl`, line 1) at - * session start, for both auth modes — the same value the codex backfill reads. + * 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 + * @ref LLP 0083 [implements]: rollout is the live cwd fallback for Codex * * **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 @@ -80,7 +80,7 @@ export function createRolloutCwdResolver(opts) { // 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 + // @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 }, @@ -122,11 +122,17 @@ function readRolloutCwd(sessionsDir, threadId, readdirSync, log) { // refusal, not a guess const rolloutThreadId = stringValue(payload?.id) 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: 'thread_id_mismatch', + error_kind: rolloutThreadId === undefined ? 'thread_id_absent' : 'thread_id_mismatch', wanted_thread_id: threadId, rollout_thread_id: rolloutThreadId ?? null, rollout: path.basename(rolloutPath), @@ -140,7 +146,7 @@ function readRolloutCwd(sessionsDir, threadId, readdirSync, log) { * 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 diff --git a/hypaware-core/plugins-workspace/codex/src/types.d.ts b/hypaware-core/plugins-workspace/codex/src/types.d.ts index e8fe4c91..83dd84bc 100644 --- a/hypaware-core/plugins-workspace/codex/src/types.d.ts +++ b/hypaware-core/plugins-workspace/codex/src/types.d.ts @@ -74,7 +74,7 @@ export interface RolloutCwdResolver { } /** - * A directory entry as far as the rollout scan needs it — the structural subset + * A directory entry as far as the rollout scan needs it: the structural subset * of `node:fs`'s `Dirent` the walk touches. Declared so the reader can be * injected (and its calls counted) in tests without pulling the whole fs type. */ diff --git a/llp/0083-codex-live-cwd-from-rollout.decision.md b/llp/0083-codex-live-cwd-from-rollout.decision.md index f9f4f96d..fda21cb4 100644 --- a/llp/0083-codex-live-cwd-from-rollout.decision.md +++ b/llp/0083-codex-live-cwd-from-rollout.decision.md @@ -9,7 +9,7 @@ > The `@hypaware/codex` **live** exchange projector resolves an exchange's `cwd` > from the session's local rollout (`session_meta.cwd`) when the request carries -> none — the same source the codex backfill already reads. This makes +> none: the same source the codex backfill already reads. This makes > `.hypignore` folder coverage ([LLP 0049](./0049-hypignore-usage-policy.spec.md)) > client-independent for Codex and stamps a non-null `cwd` on subscription-route > rows. @@ -18,7 +18,7 @@ `.hypignore` enforcement matches an exchange to a scope by its `cwd` ([LLP 0049](./0049-hypignore-usage-policy.spec.md#scope)), and the drop lives in -the client adapter — the only place that resolves a `cwd` +the client adapter, the only place that resolves a `cwd` ([LLP 0050](./0050-ignore-enforced-in-adapters.decision.md)). The Codex live projector resolved `cwd` **only** from the request in flight: the `x-codex-turn-metadata` header, then the body `cwd` / `metadata.cwd` / @@ -30,10 +30,10 @@ carry `cwd` in-band in `metadata`, so no enrichment was ever built. The **ChatGPT-subscription** route (`provider='chatgpt'`, `/backend-api/codex/*`) has no such field, and `codex-tui` does not send the `x-codex-turn-metadata` header on it (that is Codex Desktop behavior). So "cwd is always available at -projection time" was really "cwd is available when the client volunteers it" — +projection time" was really "cwd is available when the client volunteers it", and for an entire first-class traffic class, it never did: -- `.hypignore` was a silent **no-op** for subscription-mode Codex — the same gap +- `.hypignore` was a silent **no-op** for subscription-mode Codex, the same gap class as raw-proxy/OTEL ([LLP 0049 §non-goals](./0049-hypignore-usage-policy.spec.md#non-goals)), except this *is* a supported Codex adapter pathway, not a folder-blind source. - The subscription-route rows recorded `cwd = NULL`, so they also escaped the @@ -41,20 +41,22 @@ and for an entire first-class traffic class, it never did: scoping. - It diverged from **backfill**: the codex backfill reads `session_meta.cwd` from the rollout and *does* skip an ignored session, so the two halves of one policy - treated the same session oppositely — recorded live, skipped on backfill. + treated the same session oppositely: recorded live, skipped on backfill. The `cwd` was available locally the whole time: Codex writes `session_meta.cwd` -into its rollout (`/…/rollout--.jsonl`, line 1) at +into its rollout (`/…/rollout--.jsonl`, line 1) at session start, for both auth modes. The live projector just never read it. ## Decision **When the request carries no in-band `cwd`, the Codex live projector falls back -to the session rollout's `session_meta.cwd`, keyed on the session id the adapter -already resolves.** Contrast the `@hypaware/claude` projector, which — because -Anthropic requests never carry `cwd` — *had* to build enrichment (the -hook-written `session-context.jsonl` sidecar) and therefore works on every route. -Codex now has the symmetric fallback. +to the thread rollout's `session_meta.cwd`, keyed on the Codex thread id the +adapter already resolves** (the thread, *not* the session container: see the +keying bullets below and the [correction for issue #459](#correction-the-first-cut-keyed-on-the-container-issue-459)). +Contrast the `@hypaware/claude` projector, which, because Anthropic requests +never carry `cwd`, *had* to build enrichment (the hook-written +`session-context.jsonl` sidecar) and therefore works on every route. Codex now +has the symmetric fallback. - **In-band stays the fast path.** A fresh in-band `cwd` short-circuits before any filesystem work; the rollout is consulted **only** on a miss. @@ -139,12 +141,12 @@ Codex now has the symmetric fallback. ## Why not the alternatives - **Wait for the client to volunteer `cwd`** (an `x-codex-turn-metadata` on the - subscription route, or a caller-supplied `X-Hyp-Cwd` header — the future hook + subscription route, or a caller-supplied `X-Hyp-Cwd` header, the future hook [LLP 0049 non-goal 1](./0049-hypignore-usage-policy.spec.md#non-goals) leaves open). This keeps a privacy control's coverage hostage to client behavior we do not own, indefinitely. The rollout makes coverage **client-independent** today; if a future client *does* send the header, the adapter already parses it - route-agnostically and coverage simply resumes via the fast path — no conflict. + route-agnostically and coverage simply resumes via the fast path, no conflict. - **Accept it as structural folder-blindness** like raw-proxy/OTEL. Those paths have no adapter and no local `cwd`; Codex has both. Treating a recoverable leak as structural would be a privacy regression dressed as a non-goal. @@ -195,7 +197,11 @@ confirmed as unresolvable. - The identity guard's refusal is **logged** (`plugin.codex.rollout_cwd_thread_mismatch`, warn), because its only other trace is a row with `cwd = NULL`, which is indistinguishable from the ordinary not-yet-written rollout. The resolver takes - an optional `log` and `index.js` passes `ctx.log`. + an optional `log` and `index.js` passes `ctx.log`. One message, two + `error_kind`s, because the two refusals have different diagnoses: + `thread_id_mismatch` (a name that lies: a renamed or copied file) and + `thread_id_absent` (the divergence recorded below, which the backfill still + accepts). - `codex/src/rollout-cwd.js` and `ai-gateway/src/session_command.js`'s `readRolloutMeta` remain **two readers of the same first line** with different needs (a hot-path cwd lookup per thread versus a one-shot CLI scan that must diff --git a/test/plugins/codex-rollout-cwd.test.js b/test/plugins/codex-rollout-cwd.test.js index f25120b2..a3c08af3 100644 --- a/test/plugins/codex-rollout-cwd.test.js +++ b/test/plugins/codex-rollout-cwd.test.js @@ -50,7 +50,7 @@ const SUBSCRIPTION_SESSION_ID = '019e60b5-1111-4222-8333-444455556666' // --------------------------------------------------------------------- // Regression (#257): the ChatGPT-subscription route carries no in-band cwd, so -// the live projector must fall back to the session rollout's session_meta.cwd — +// the live projector must fall back to the session rollout's session_meta.cwd, // otherwise `.hypignore` fails open for the whole traffic class and the row // records cwd = NULL (diverging from backfill, which DOES read the rollout). // --------------------------------------------------------------------- @@ -178,7 +178,7 @@ test('createRolloutCwdResolver returns undefined when the sessions root is missi // --------------------------------------------------------------------- // Review round 1, Major 1: a miss (not-yet-written rollout on a session's // first exchange, or a transient read error) must NOT be cached as a permanent -// NULL cwd — that would silently fail `.hypignore` open for the session's whole +// NULL cwd: that would silently fail `.hypignore` open for the session's whole // life once the rollout became readable. A resolved cwd stays cached for life. // --------------------------------------------------------------------- @@ -197,7 +197,7 @@ test('a missing-then-present rollout is re-resolved after the negative TTL, but // A repeat within the TTL window is served from the negative cache: no rescan. assert.equal(resolver.resolve(SUBSCRIPTION_SESSION_ID), undefined) - assert.equal(scan.calls.length, scansAfterMiss, 'a miss is cached within its TTL — no re-scan') + assert.equal(scan.calls.length, scansAfterMiss, 'a miss is cached within its TTL, no re-scan') // The rollout appears (the session-start race resolves) and the TTL lapses. const rolloutPath = path.join(sessionsDir, `rollout-2026-07-07T10-00-00-${SUBSCRIPTION_SESSION_ID}.jsonl`) @@ -212,7 +212,7 @@ test('a missing-then-present rollout is re-resolved after the negative TTL, but const scansAfterResolve = scan.calls.length clock += 1_000_000 assert.equal(resolver.resolve(SUBSCRIPTION_SESSION_ID), '/work/late') - assert.equal(scan.calls.length, scansAfterResolve, 'a resolved cwd is cached for the session life — never re-scanned') + assert.equal(scan.calls.length, scansAfterResolve, 'a resolved cwd is cached for the session life, never re-scanned') }) test('a transient read error is retried rather than cached as a permanent miss', async () => { @@ -245,7 +245,7 @@ test('a transient read error is retried rather than cached as a permanent miss', // --------------------------------------------------------------------- // Review round 1, Major 2: the first lookup walks newest-date dirs first and // returns on first match, so the active session's rollout (newest date dir) is -// found without walking the whole history — while an older/dormant session's +// found without walking the whole history, while an older/dormant session's // rollout (older date dir) still resolves. // --------------------------------------------------------------------- @@ -268,7 +268,7 @@ test('a newest-dir rollout is found without descending the older-date branch; an ) // The active (newest-date) session resolves after touching only the newest - // branch — the older-date branch (…/2026/01) is never even scanned. + // branch: the older-date branch (…/2026/01) is never even scanned. const scan = countingReaddir() const resolver = createRolloutCwdResolver({ sessionsDir, readdirSync: scan.readdirSync }) assert.equal(resolver.resolve(SUBSCRIPTION_SESSION_ID), '/work/new') @@ -402,6 +402,9 @@ test('a rollout whose body disagrees with its filename is refused, not silently ) assert.equal(log.warns[0].fields?.wanted_thread_id, SUBAGENT_THREAD_ID) assert.equal(log.warns[0].fields?.rollout_thread_id, ROOT_THREAD_ID) + // A name that lies is a different diagnosis from a rollout that states no id, + // so the two refusals are distinguishable without re-reading the fields. + assert.equal(log.warns[0].fields?.error_kind, 'thread_id_mismatch') }) test('a subagent turn that states lineage but not its own thread id resolves no cwd', async () => { @@ -509,6 +512,9 @@ test('a session_meta line with a cwd but no payload.id is refused, not matched b assert.equal(resolver.resolve(ROOT_THREAD_ID), undefined) assert.deepEqual(log.warns.map((w) => w.message), ['plugin.codex.rollout_cwd_thread_mismatch']) assert.equal(log.warns[0].fields?.rollout_thread_id, null, 'an absent id is reported as absent, not as the wanted id') + // The one shape the backfill still accepts, so the log has to say which + // refusal this is: it is the live/backfill divergence, not a renamed file. + assert.equal(log.warns[0].fields?.error_kind, 'thread_id_absent') }) test('DOCUMENTED GAP: a turn stating a container and NO lineage at all is taken as its root thread', async () => { From 0ca69488e9aafbb8ec3c30ce4c3e836bbf471705 Mon Sep 17 00:00:00 2001 From: neutral Date: Thu, 30 Jul 2026 23:20:51 +0000 Subject: [PATCH 5/6] Correct the container fallback's stale rationale and pin the value-blind cost Round 3 review of #462. Three corrections, all established by executing the real projector over every surface combination rather than by reading: - `resolveRolloutCwd`'s JSDoc still justified the container fallback with "the common subscription-route shape (a `session-id` header and nothing else)". Since #467 / LLP 0151 that header name is neither emitted by Codex nor read by this file, so the shape it names resolves nothing. Restated against the body's `client_metadata` map, which is what actually decides reachability now. - LLP 0083 said a turn reaches the fallback "only when it carries neither a Codex-owned `client_metadata` map nor a turn-metadata blob". Executed counter-example: either surface can be present and still state no `thread_id`, and then the fallback IS entered. The accurate condition is that no surface states a `thread_id`. - The value-blind refusal's cost (`review` / `compact` / `memory_consolidation` are same-workspace sub-threads, so refusing records a turn the root's cwd would have dropped) was unasserted. Two parameterized loops now pin both halves: the cost, and the bound on it (a turn stating its thread never reaches the guard). Each mutates red on its own mutation. Co-Authored-By: Claude --- .../codex/src/exchange-projector.js | 41 ++++++++----- ...83-codex-live-cwd-from-rollout.decision.md | 32 +++++++--- test/plugins/codex-rollout-cwd.test.js | 60 +++++++++++++++++++ 3 files changed, 109 insertions(+), 24 deletions(-) diff --git a/hypaware-core/plugins-workspace/codex/src/exchange-projector.js b/hypaware-core/plugins-workspace/codex/src/exchange-projector.js index 22b72123..44ea332f 100644 --- a/hypaware-core/plugins-workspace/codex/src/exchange-projector.js +++ b/hypaware-core/plugins-workspace/codex/src/exchange-projector.js @@ -251,31 +251,40 @@ export function createCodexExchangeProjector(opts = {}) { * @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), and that is the common - * subscription-route shape (a `session-id` header and nothing else). But the - * container is only usable while nothing says otherwise: if the turn announces - * subagent lineage without naming its own thread, its rollout is not identifiable - * from the wire, and an unknown cwd (LLP 0049 fails open, the row records NULL) - * is preferred to confidently stamping and enforcing the root's directory. A - * wrong cwd is a false statement about where a turn ran; an absent one is true. + * 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 also carries `thread_id`, so a turn that states them has already - * been answered by the branch above: a refusal keyed only on those can never fire. - * The lineage that survives 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 + * and that blob states `thread_id` whenever it states any turn identity at all, + * so a turn stating them has already been answered by the branch above: a refusal + * keyed only on those can never fire for a real Codex turn. 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, so both are consulted here. + * 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. - * Dropping the fallback anyway is worse, not safer: 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] + * 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} codexContext diff --git a/llp/0083-codex-live-cwd-from-rollout.decision.md b/llp/0083-codex-live-cwd-from-rollout.decision.md index 691f699e..d67d6207 100644 --- a/llp/0083-codex-live-cwd-from-rollout.decision.md +++ b/llp/0083-codex-live-cwd-from-rollout.decision.md @@ -138,14 +138,30 @@ has the symmetric fallback. ([LLP 0151](./0151-codex-lineage-from-body-client-metadata.decision.md#body-is-authority)), which settles the recorded-shape question there rather than here. The consequence for this document: a turn now reaches the container fallback at all - only when it carries **neither** a Codex-owned `client_metadata` map **nor** a - turn-metadata blob, because either one states the thread id and the thread-id - key answers first. The refusal below is therefore much harder to reach than it - was, but it is not dead, and nothing about the trade it encodes changed. The - guard is deliberately value-blind (any `x-openai-subagent` value refuses, not - only the ones that name a different workspace); whether that is the right grain - is an open question on the PR that introduced it, recorded there rather than - settled here. + only when **no surface it carries states a `thread_id`**. That is stricter than + "carries neither surface", and the difference is the whole reachable set: the + body map and the turn-metadata blob each state a `thread_id` whenever they state + any identity at all (the map on every request, the blob whenever the request + kind has turn identity), so a turn reaches the fallback only by naming a + container on a Codex-owned surface while naming no thread on any of them. No + `codex-rs` surface is documented to produce that. + + **The value-blind grain, and what it costs.** The guard refuses on any + `x-openai-subagent` value, not only the ones that name a different workspace, so + `review`, `compact` and `memory_consolidation` (same-workspace sub-threads, + where the root's `cwd` is the correct answer) refuse a container that would have + resolved correctly, and [LLP 0049](./0049-hypignore-usage-policy.spec.md) then + fails **open** and records the turn. That is the same leak direction this + document exists to close, and it is accepted here only because it needs the same + unobserved request shape the fallback itself now needs: verified by executing + the real projector over every surface combination, the refusal and the + container fallback are entered by exactly the same shapes, so neither the + residual nor its mirror is reachable from Codex traffic as `codex-rs` is + documented to emit it. The narrowing rests on that documented emission and not + on anything this repo can assert hermetically + ([LLP 0141](./0141-codex-desktop-rides-the-codex-adapter.decision.md)), which is + why the guard is kept rather than deleted: it is the cheap half of a trade whose + premise is another program's source. - **What remains after that is bounded, and removing the fallback is worse.** A turn stating a container, no thread id, and no lineage of any kind is taken as the root thread it claims to be. That can only mis-resolve for a client that diff --git a/test/plugins/codex-rollout-cwd.test.js b/test/plugins/codex-rollout-cwd.test.js index a0a8ccfc..322f3fa3 100644 --- a/test/plugins/codex-rollout-cwd.test.js +++ b/test/plugins/codex-rollout-cwd.test.js @@ -603,6 +603,66 @@ for (const header of ['x-codex-parent-thread-id', 'x-openai-subagent']) { }) } +// `x-openai-subagent`'s real values are `review`, `compact`, `collab_spawn` and +// `memory_consolidation`. Three of the four are sub-threads of the ROOT's own +// workspace, so the root's cwd is the CORRECT answer for them, and the guard +// refuses anyway because it is value-blind. These two loops pin both halves of +// that trade, because only the pair of them says how far it reaches. +for (const value of ['review', 'compact', 'memory_consolidation']) { + test(`DOCUMENTED MIRROR: x-openai-subagent=${value} with no thread id refuses a container the root would have resolved`, async () => { + // @ref LLP 0083#container-fallback-gap [tests]: the cost of the value-blind + // grain, asserted rather than left to be rediscovered. The root ran in an + // IGNORED directory and this is a sub-thread of the root's own workspace, so + // the container fallback would have dropped the turn. The refusal makes the + // cwd unknown instead, LLP 0049 fails OPEN, and the turn is RECORDED. That is + // the same leak direction #459 closes, in the guard that bounds the fix. + const sessionsDir = await writeSubagentPair({ + rootCwd: '/work/ignored/root', + subagentCwd: '/work/clean/sub', + }) + const projector = createCodexExchangeProjector({ + resolver: ignoringResolver('/work/ignored'), + rolloutCwd: createRolloutCwdResolver({ sessionsDir }), + }) + const projection = /** @type {any} */ (projector.project(exchange({ + path: '/backend-api/codex/responses', + provider: 'chatgpt', + request_headers: JSON.stringify({ 'x-openai-subagent': value }), + request_body: codexLineageBody({ session_id: ROOT_SESSION_ID }), + response_body: JSON.stringify({ output_text: 'ok' }), + }), context())) + assert.ok(projection && projection !== USAGE_POLICY_DROP, 'recorded, not dropped: the cost this asserts') + assert.equal(projection.cwd, undefined) + }) + + test(`x-openai-subagent=${value} does NOT cost anything once the turn states its thread`, async () => { + // The bound on the loop above. Codex fills the body's `client_metadata` map + // with `thread_id` on every request (@ref LLP 0151#body-is-authority), so a + // real turn carrying this header carries a thread id too, the thread-id key + // answers first, and the value-blind refusal is never consulted. Same header, + // same ignored root, opposite outcome: the drop is restored. + const sessionsDir = await writeSubagentPair({ + rootCwd: '/work/ignored/root', + subagentCwd: '/work/clean/sub', + }) + const projector = createCodexExchangeProjector({ + resolver: ignoringResolver('/work/ignored'), + rolloutCwd: createRolloutCwdResolver({ sessionsDir }), + }) + const projection = projector.project(exchange({ + path: '/backend-api/codex/responses', + provider: 'chatgpt', + request_headers: JSON.stringify({ 'x-openai-subagent': value }), + request_body: codexLineageBody({ + session_id: ROOT_SESSION_ID, + thread_id: ROOT_THREAD_ID, + }), + response_body: JSON.stringify({ output_text: 'ok' }), + }), context()) + assert.equal(projection, USAGE_POLICY_DROP, 'a turn that states its thread is judged by its own rollout') + }) +} + test('a session_meta line with a cwd but no payload.id is refused, not matched by its filename', async () => { // The identity guard reads the RAW line, so an absent `payload.id` is visible // as absent and refuses. Pinned because it is a real divergence from the From 4dcd2467976dec4225cf095c1acdea4af5093097 Mon Sep 17 00:00:00 2001 From: test Date: Thu, 30 Jul 2026 23:48:02 +0000 Subject: [PATCH 6/6] Attribute the guard's unreachability to the surface that actually earns it Round 4 review of #462, the last granted round. Rounds 2 and 3 narrowed the value-blind `subagent_signal` refusal to "unreachable from Codex traffic" but could not verify the premise it rests on, because the `codex-rs` snapshot was gone. Verified here by reading the emitting source directly (`openai/codex`, `codex-rs/core/src/responses_metadata.rs`, commit `1def0a8`, 2026-07-28), and the premise holds, but not for the stated reason. - The load-bearing invariant is not "each surface states a `thread_id`". That is false of the turn-metadata blob. It is that both surfaces emit `session_id` and `thread_id` AS A PAIR: `client_metadata` inserts them into one map literal, unconditionally, from two non-`Option` `String` fields, and `turn_metadata_payload` gates both on the same `has_turn_identity`. Neither surface can supply the container the fallback needs while withholding the thread that pre-empts it. - `has_turn_identity` is false for exactly one kind, `Memory`, which still emits the lineage the refusal keys on (`thread_source`, `parent_thread_id`, `x-openai-subagent: memory_consolidation`) with no id pair in the blob. So memory consolidation is the closest real Codex shape to the refusal's trigger, and what keeps it out is the flat body map ALONE, not the blob. The JSDoc and LLP 0083 both said or implied the blob answers such a turn. - Pinned by a test rather than left as prose: a memory-consolidation turn whose blob states lineage and neither id. It reddens if the body-map read for `thread_id` is removed, which is the change that would send this shape into the refusal and fail `.hypignore` open on it. LLP 0083 also now records the citation and the standing of the claim (upstream `main` snapshot, emitting code rather than captured traffic) so the next reader re-checks it in one step instead of re-deriving it a fifth time. Co-Authored-By: Claude --- .../codex/src/exchange-projector.js | 11 +++-- ...83-codex-live-cwd-from-rollout.decision.md | 47 +++++++++++++++---- test/plugins/codex-rollout-cwd.test.js | 47 +++++++++++++++++++ 3 files changed, 92 insertions(+), 13 deletions(-) diff --git a/hypaware-core/plugins-workspace/codex/src/exchange-projector.js b/hypaware-core/plugins-workspace/codex/src/exchange-projector.js index 44ea332f..8fa783e8 100644 --- a/hypaware-core/plugins-workspace/codex/src/exchange-projector.js +++ b/hypaware-core/plugins-workspace/codex/src/exchange-projector.js @@ -263,9 +263,14 @@ export function createCodexExchangeProjector(opts = {}) { * * **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 `thread_id` whenever it states any turn identity at all, - * so a turn stating them has already been answered by the branch above: a refusal - * keyed only on those can never fire for a real Codex turn. The lineage that + * 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 diff --git a/llp/0083-codex-live-cwd-from-rollout.decision.md b/llp/0083-codex-live-cwd-from-rollout.decision.md index d67d6207..5eb32879 100644 --- a/llp/0083-codex-live-cwd-from-rollout.decision.md +++ b/llp/0083-codex-live-cwd-from-rollout.decision.md @@ -139,12 +139,33 @@ has the symmetric fallback. which settles the recorded-shape question there rather than here. The consequence for this document: a turn now reaches the container fallback at all only when **no surface it carries states a `thread_id`**. That is stricter than - "carries neither surface", and the difference is the whole reachable set: the - body map and the turn-metadata blob each state a `thread_id` whenever they state - any identity at all (the map on every request, the blob whenever the request - kind has turn identity), so a turn reaches the fallback only by naming a - container on a Codex-owned surface while naming no thread on any of them. No - `codex-rs` surface is documented to produce that. + "carries neither surface", and the difference is the whole reachable set: a turn + reaches the fallback only by naming a container on a Codex-owned surface while + naming no thread on any of them. + + The invariant that makes that unreachable is not "each surface states a + `thread_id`", which is false of the blob. It is that **on both surfaces the two + ids are emitted as a pair, never one without the other**, so no surface can + supply the container the fallback needs while withholding the thread that would + pre-empt it. Verified by reading the emitting source rather than inferred + (`openai/codex`, `codex-rs/core/src/responses_metadata.rs`, commit `1def0a8`, + 2026-07-28): + + - `CodexResponsesMetadata::client_metadata` inserts `session_id` and `thread_id` + into the same map literal, unconditionally and ungated, from two non-`Option` + `String` fields. Every `/responses` request carries this map + (`client.rs`, `client_metadata: Some(responses_metadata.client_metadata())`). + - `turn_metadata_payload` gates `session_id` and `thread_id` on the **same** + `has_turn_identity` boolean, so the blob states both or neither. + + The blob's `has_turn_identity` is false for exactly one request kind, + `CodexResponsesRequestKind::Memory`, and that kind still emits the lineage this + refusal keys on (`thread_source`, `parent_thread_id`, and an + `x-openai-subagent: memory_consolidation` header) with **no** id pair in the + blob. So memory consolidation is the closest real Codex shape to the refusal's + trigger, and what keeps it out is the flat body map alone, not the blob. That is + pinned by a test rather than left as prose, because the surface doing the work + is not the one the argument reads as naming. **The value-blind grain, and what it costs.** The guard refuses on any `x-openai-subagent` value, not only the ones that name a different workspace, so @@ -156,10 +177,16 @@ has the symmetric fallback. unobserved request shape the fallback itself now needs: verified by executing the real projector over every surface combination, the refusal and the container fallback are entered by exactly the same shapes, so neither the - residual nor its mirror is reachable from Codex traffic as `codex-rs` is - documented to emit it. The narrowing rests on that documented emission and not - on anything this repo can assert hermetically - ([LLP 0141](./0141-codex-desktop-rides-the-codex-adapter.decision.md)), which is + residual nor its mirror is reachable from Codex traffic as `codex-rs` emits it. + + That last clause is a claim about a program this repo does not build, so it is + worth being exact about its standing. It has been checked against `codex-rs` + source directly (the pair invariant above), which is stronger than the earlier + "documented to emit" phrasing, but it is still a **snapshot of an upstream + `main`**, not a pinned release, and it is a claim about emitting code rather + than about captured traffic: no hermetic smoke in this repo can supply it + ([LLP 0141](./0141-codex-desktop-rides-the-codex-adapter.decision.md)). Re-check + it at the two functions named above if the guard's cost ever matters. That is why the guard is kept rather than deleted: it is the cheap half of a trade whose premise is another program's source. - **What remains after that is bounded, and removing the fallback is worse.** A diff --git a/test/plugins/codex-rollout-cwd.test.js b/test/plugins/codex-rollout-cwd.test.js index 322f3fa3..2943e0a2 100644 --- a/test/plugins/codex-rollout-cwd.test.js +++ b/test/plugins/codex-rollout-cwd.test.js @@ -663,6 +663,53 @@ for (const value of ['review', 'compact', 'memory_consolidation']) { }) } +test('a memory-consolidation turn is answered by the body map, whose id pair the blob withholds', async () => { + // @ref LLP 0083#container-fallback-gap [tests]: which surface actually keeps + // the real subagent-flavoured request kind out of the value-blind refusal. + // + // `CodexResponsesRequestKind::Memory` is the one kind `codex-rs` marks + // `has_turn_identity() == false`, so `turn_metadata_payload` omits BOTH + // `session_id` and `thread_id` from the blob while still emitting the lineage + // that trips the refusal (`thread_source`, `parent_thread_id`, and the + // `x-openai-subagent: memory_consolidation` compatibility header). So this is + // the closest real Codex shape to the refusal's trigger, and what keeps it out + // is the FLAT BODY MAP: `client_metadata()` inserts the id pair + // unconditionally, ungated by `has_turn_identity`. + // + // Stated as a test because the reasoning is easy to get backwards: the blob is + // NOT what answers this turn, so a future change that stopped reading the body + // map would send precisely this shape into the refusal and fail `.hypignore` + // OPEN on it. The assertion below is what would catch that. + const sessionsDir = await writeSubagentPair({ + rootCwd: '/work/ignored/root', + subagentCwd: '/work/clean/sub', + }) + const projector = createCodexExchangeProjector({ + resolver: ignoringResolver('/work/ignored'), + rolloutCwd: createRolloutCwdResolver({ sessionsDir }), + }) + const projection = projector.project(exchange({ + path: '/backend-api/codex/responses', + provider: 'chatgpt', + request_headers: JSON.stringify({ + 'x-openai-subagent': 'memory_consolidation', + // The blob as the Memory kind serialises it: lineage, no id pair. + 'x-codex-turn-metadata': JSON.stringify({ + request_kind: 'memory', + thread_source: 'subagent', + parent_thread_id: ROOT_THREAD_ID, + }), + }), + // The body map, which states both ids regardless of the request kind. + request_body: codexLineageBody({ + session_id: ROOT_SESSION_ID, + thread_id: ROOT_THREAD_ID, + }), + response_body: JSON.stringify({ output_text: 'ok' }), + }), context()) + assert.equal(projection, USAGE_POLICY_DROP, 'the body map names the thread, so the refusal is never consulted') +}) + test('a session_meta line with a cwd but no payload.id is refused, not matched by its filename', async () => { // The identity guard reads the RAW line, so an absent `payload.id` is visible // as absent and refuses. Pinned because it is a real divergence from the