fix(codex): carry uploaded-file retention through compact and keep the denial lookup behind the read fence

scripts/test-layout/layout.json

@@ -1329,6 +1329,7 @@
     "subagent-context-staleness.test.ts": "routing",
     "subagent-defaults.test.ts": "routing",
     "subagent-fallback-handle-responses.test.ts": "routing",
+    "subagent-fallback-preview-sites.test.ts": "routing",
     "subagent-model-fallback-api.test.ts": "routing",
     "subagent-model-fallback.test.ts": "routing",
     "subagent-roster-retention.test.ts": "routing",

src/codex/auth-context.ts

@@ -989,7 +989,18 @@ export async function resolveCodexAuthContext(
     // already gathered -- no upstream fetch joins the request path for the most commonly
     // requested models in the product -- and passed to selection as a preference that is dropped
     // whenever honouring it would leave no candidate.
-    const deniedModelAccountIds = cachedDeniedCodexAccountIdsForModel(options.modelId);
+    //
+    // Under the SAME exclusion the entitlement snapshot above uses. The reader validates each
+    // cached roster against the account's current credential, and for native main that is a
+    // synchronous read of the physical stored token -- exactly what this request is forbidden to
+    // touch while a profile switch drains it or while it is served by a request-owned credential.
+    // Excluding main here costs nothing: the preference is an ordering hint, so main becomes
+    // unknown rather than denied, and unknown leaves selection exactly as it was.
+    const deniedModelAccountIds = cachedDeniedCodexAccountIdsForModel(
+      options.modelId,
+      undefined,
+      { excludeAccountIds },
+    );
     const selectionOptions = {
       // Temporary switch drain keeps the candidate until the atomic claim rejects
       // it. Retained recovery makes main wholly ineligible so pool routing continues.

src/codex/model-entitlements.ts

@@ -1209,6 +1209,7 @@ export const ENTITLEMENT_PREFERRED_NATIVE_OPENAI_MODELS: ReadonlySet<string> = n
 export function cachedDeniedCodexAccountIdsForModel(
   modelId: string | undefined,
   now = Date.now(),
+  options: { excludeAccountIds?: ReadonlySet<string> } = {},
 ): ReadonlySet<string> | undefined {
   if (!modelId || !ENTITLEMENT_PREFERRED_NATIVE_OPENAI_MODELS.has(modelId)) return undefined;
   const denied = new Set<string>();
@@ -1217,6 +1218,13 @@ export function cachedDeniedCodexAccountIdsForModel(
     const accountId = accountIdOfCacheKey(key);
     // A forwarded Direct credential is one request's caller, never a pool candidate.
     if (accountId.startsWith(DIRECT_CALLER_ACCOUNT_PREFIX)) continue;
+    // The caller's read fence, honoured BEFORE `currentCredentialIdentity` below, because that
+    // is the read: for native main it resolves the physical stored token, once per cached client
+    // version. A request that is forbidden to read main -- a profile switch draining it, or a
+    // request-owned credential that owns no main state -- must not reread account storage just to
+    // score an ordering preference. Dropping the account leaves it UNKNOWN rather than denied,
+    // which is the same outcome as having no cached roster for it and changes no selection.
+    if (options.excludeAccountIds?.has(accountId)) continue;
     if (entry.expiresAt <= now) continue;
     // A credential we can currently read AND that differs is proof the entry answers for a
     // different account than this id now names, so its denial is not evidence about the current

src/server/responses/compact.ts

@@ -738,6 +738,14 @@ export async function handleResponsesCompact(
           beginCodexAccountSelection: codexAccountSelectionForTurn(turnAdmissionLease),
           signal: req.signal,
           nativeMainRefreshDependencies: options.nativeMainRefreshDependencies,
+          // #4778: the same retention the regular Responses path passes at its final auth. The
+          // post-429 guard below only declines to move AFTER this resolution has already bound
+          // an account, so without the bit here a quota-driven rebind could have carried the
+          // conversation off its issuing account before that guard is ever consulted -- and an
+          // uploaded file is readable only by the account that received it. Answered from `raw`,
+          // the same object and the same predicate the guard below uses, so the two can never
+          // disagree about which conversations are in scope.
+          retainAccountForUploadedFiles: conversationCarriesUploadedFiles(raw),
         });
         logCtx.accountLogLabel = codexAuthContextLogLabel(authCtx, config);
         const selected = await materializeCodexUpstreamAuthAsync(req.headers, authCtx, {

src/server/responses/request-prepare.ts

@@ -88,6 +88,7 @@ import {
   cachedDeniedCodexAccountIdsForModel,
   resolveCodexModelEntitlements,
 } from "../../codex/model-entitlements";
+import { MAIN_CODEX_ACCOUNT_ID } from "../../codex/main-account";
 import {
   previewCodexAccountForRequest,
   codexQuotaScopeForModel,
@@ -542,7 +543,13 @@ export async function prepareResponsesRequest(
         // Per CANDIDATE model, like the scope and the eligible set above: the preference is
         // model-specific, so hoisting it out of the closure would score every fallback
         // candidate against the requested model's evidence and diverge from final auth (#4768).
-        deniedModelAccountIds: cachedDeniedCodexAccountIdsForModel(modelId, previewNow),
+        // Under the same native-main read fence final auth applies: the reader validates each
+        // cached roster against the account's current credential, and for main that is a
+        // synchronous read of the stored token. A preview that read it would both cross the
+        // fence and score main differently than the resolution it is supposed to predict.
+        deniedModelAccountIds: cachedDeniedCodexAccountIdsForModel(modelId, previewNow, {
+          excludeAccountIds: nativeMainReadsForbidden ? new Set([MAIN_CODEX_ACCOUNT_ID]) : undefined,
+        }),
       },
       modelId,
       poolLineage,
@@ -677,6 +684,13 @@ export async function prepareResponsesRequest(
               const recoverySelectionOptions = {
                 nativeMainSelectionOnly: !recoveryNativeMainBlocked
                   && recoverySelectionAdmission?.mainProfileDraining === true,
+                // #4778, same reason as `previewSelectionOptions` above: this preview decides
+                // which account subagent fallback scores against, and final auth passes the
+                // retention. Recovery is exactly where the two could diverge -- it re-previews
+                // against the DECRYPTED body, which is the first point at which a file reference
+                // that was ciphertext-only becomes readable, so reconstructing the options
+                // without the bit lets preview report a quota move the request will not make.
+                retainAccountForUploadedFiles: conversationCarriesUploadedFiles(parsed._rawBody),
               };
               const recoveryNow = Date.now();
               // Carry the entitlement filter through recovery too (#2509/#2623). The scope was
@@ -692,7 +706,14 @@ export async function prepareResponsesRequest(
                 {
                   ...recoverySelectionOptions,
                   modelEligibleAccountIds,
-                  deniedModelAccountIds: cachedDeniedCodexAccountIdsForModel(modelId, previewNow),
+                  // Same read fence as the first preview, evaluated against recovery's own view
+                  // of the drain rather than the one captured before decryption.
+                  deniedModelAccountIds: cachedDeniedCodexAccountIdsForModel(modelId, previewNow, {
+                    excludeAccountIds: recoveryNativeMainBlocked
+                      || recoverySelectionAdmission?.mainProfileDraining === true
+                      ? new Set([MAIN_CODEX_ACCOUNT_ID])
+                      : undefined,
+                  }),
                 },
                 modelId,
                 poolLineage,

tests/codex-integration/codex-model-entitlements.test.ts

@@ -40,6 +40,7 @@ import upstreamModelsSnapshot from "../../src/codex/data/upstream-models.json";
 import { readCodexAccountRecord, saveCodexAccountCredential } from "../../src/codex/account-store";
 import { installIsolatedCodexHome } from "../helpers/isolated-codex-home";
 import { removeTreeWithRetry } from "../helpers/remove-tree";
+import { repoPath } from "../helpers/repo-root";
 
 const TEST_CLIENT_VERSION = "0.146.0";
 const DAYBREAK = "gpt-daybreak-blue-latest";
@@ -1865,3 +1866,91 @@ describe("cached per-account denials for always-visible natives", () => {
     expect(cachedDeniedCodexAccountIdsForModel(undefined, now)).toBeUndefined();
   });
 });
+
+/**
+ * The denial reader validates every cached roster against the account's CURRENT credential, and
+ * for native main that validation is a synchronous read of the physical stored token -- once per
+ * cached client version, on the request path.
+ *
+ * Some requests are forbidden to make that read: a profile switch draining the native identity,
+ * and a request served by its own forwarded credential which owns no main state. Those callers
+ * already exclude main from every other account question they ask, and reaching the token here
+ * anyway crossed the fence for an ordering hint. Excluding it is safe precisely because the hint
+ * is soft: the account becomes UNKNOWN rather than denied, which is indistinguishable from
+ * having no cached roster for it and leaves selection exactly as it was.
+ */
+describe("the denial reader honours a caller's account read fence", () => {
+  test("an excluded account contributes no denial while the others still do", () => {
+    const now = 1_800_000_000_000;
+    seedCodexModelEntitlementsForTests("plus", ["gpt-5.5"], now, TEST_CLIENT_VERSION);
+    seedCodexModelEntitlementsForTests("free", ["gpt-5.5"], now, TEST_CLIENT_VERSION);
+
+    expect([...(cachedDeniedCodexAccountIdsForModel(ASTRA, now) ?? [])].sort())
+      .toEqual(["free", "plus"]);
+    expect([...(cachedDeniedCodexAccountIdsForModel(ASTRA, now, {
+      excludeAccountIds: new Set(["plus"]),
+    }) ?? [])]).toEqual(["free"]);
+  });
+
+  test("excluding the only denied account answers undefined, never an empty set", () => {
+    // Same contract the unexcluded reader keeps: a caller must not be able to read "everything I
+    // was allowed to look at grants the model" as "no candidates exist".
+    const now = 1_800_000_000_000;
+    seedCodexModelEntitlementsForTests("free", ["gpt-5.5"], now, TEST_CLIENT_VERSION);
+
+    expect(cachedDeniedCodexAccountIdsForModel(ASTRA, now, {
+      excludeAccountIds: new Set(["free"]),
+    })).toBeUndefined();
+  });
+
+  test("native main is dropped without its stored credential being consulted", () => {
+    // Written so the answer cannot depend on whether a main token is readable in this
+    // environment: readable, absent, or mismatched, an excluded main is the same nothing. That
+    // is the whole point -- the excluded reader must reach no verdict about main, which is what
+    // makes not reading the token safe.
+    const now = 1_800_000_000_000;
+    seedCodexModelEntitlementsForTests(MAIN_CODEX_ACCOUNT_ID, ["gpt-5.5"], now, TEST_CLIENT_VERSION);
+
+    expect(cachedDeniedCodexAccountIdsForModel(ASTRA, now, {
+      excludeAccountIds: new Set([MAIN_CODEX_ACCOUNT_ID]),
+    })).toBeUndefined();
+  });
+
+  test("an absent or empty exclusion set leaves the reader exactly as it was", () => {
+    const now = 1_800_000_000_000;
+    seedCodexModelEntitlementsForTests("free", ["gpt-5.5"], now, TEST_CLIENT_VERSION);
+
+    expect([...(cachedDeniedCodexAccountIdsForModel(ASTRA, now, {}) ?? [])]).toEqual(["free"]);
+    expect([...(cachedDeniedCodexAccountIdsForModel(ASTRA, now, {
+      excludeAccountIds: new Set<string>(),
+    }) ?? [])]).toEqual(["free"]);
+  });
+
+  /**
+   * The fence is a property of the CALLERS, not of this reader: the reader cannot know which
+   * request is draining main. Asserted from source because the failure mode is an omitted
+   * argument -- every behavioural assertion above passes with the callers unchanged, which is
+   * exactly how the original violation survived review.
+   */
+  test("every request-path caller passes its own fence", () => {
+    const authContext = readFileSync(repoPath("src", "codex", "auth-context.ts"), "utf8");
+    const prepare = readFileSync(repoPath("src", "server", "responses", "request-prepare.ts"), "utf8");
+
+    // Auth context reuses the very set it already built for the entitlement snapshot, so the two
+    // account questions on this request can never disagree about what it may read.
+    const authCalls = [...authContext.matchAll(/cachedDeniedCodexAccountIdsForModel\(/g)]
+      .map(match => authContext.slice(match.index ?? 0, (match.index ?? 0) + 400));
+    expect(authCalls).toHaveLength(1);
+    expect(authCalls[0]).toContain("excludeAccountIds");
+
+    // Both previews in request-prepare, including the one rebuilt after encrypted-task recovery.
+    const previewCalls = [...prepare.matchAll(/cachedDeniedCodexAccountIdsForModel\(/g)]
+      .map(match => prepare.slice(match.index ?? 0, (match.index ?? 0) + 400));
+    expect(previewCalls).toHaveLength(2);
+    for (const call of previewCalls) expect(call).toContain("excludeAccountIds:");
+    expect(previewCalls[0]).toContain("nativeMainReadsForbidden");
+    // Recovery evaluates the drain against its own, later view rather than the one captured
+    // before decryption.
+    expect(previewCalls[1]).toContain("recoveryNativeMainBlocked");
+  });
+});

tests/fixtures/test-layout-expected.json

@@ -1157,6 +1157,7 @@
   "subagent-context-staleness.test.ts": "routing",
   "subagent-defaults.test.ts": "routing",
   "subagent-fallback-handle-responses.test.ts": "routing",
+  "subagent-fallback-preview-sites.test.ts": "routing",
   "subagent-model-fallback-api.test.ts": "routing",
   "subagent-model-fallback.test.ts": "routing",
   "subagent-roster-retention.test.ts": "routing",

tests/responses/responses-account-change-scrub.test.ts

@@ -1,4 +1,6 @@
 import { afterEach, describe, expect, spyOn, test } from "bun:test";
+import { readFileSync } from "node:fs";
+import { repoPath } from "../helpers/repo-root";
 import {
   applyAccountChangeConversationStateScrub,
   canPortConversationState,
@@ -305,3 +307,83 @@ describe("uploaded-file detection answers before an alternate account is chosen
     expect(ACCOUNT_CHANGE_FILE_SCOPE_MESSAGE).toContain("no file was removed");
   });
 });
+
+/**
+ * #4778 retention is only correct if it reaches EVERY resolution that can bind this
+ * conversation to an account, because the sites do not fail the same way.
+ *
+ * The regular Responses path passes it at final auth. Native compact did not pass it at its
+ * initial resolution, and the refusal it does carry runs only after a 429 -- by which time a
+ * quota-driven rebind has already moved the conversation, so the guard declines a move that
+ * happened one step earlier. Encrypted-agent-task recovery rebuilds the preview options from
+ * scratch, and it does so against the DECRYPTED body, which is the first point at which a file
+ * reference that was ciphertext-only becomes readable; a preview reconstructed without the bit
+ * reports one account and final auth binds another.
+ *
+ * Asserted from source because the failure is an omitted option on a call, not a value any
+ * reachable seam returns: a body-level test of the predicate (above) passes either way, and the
+ * behavioural difference only appears against a live pool that is mid-rebind. The claim is
+ * narrow and mechanical -- this exact call carries this exact expression -- so it fails on the
+ * regression and on nothing else.
+ */
+describe("uploaded-file retention reaches every account resolution (#4778)", () => {
+  const source = (...relative: string[]): string => readFileSync(repoPath(...relative), "utf8");
+
+  /** The argument list of the single call whose head is `marker` (which must end at its own `(`). */
+  function callArguments(src: string, marker: string): string {
+    const at = src.indexOf(marker);
+    expect(at).toBeGreaterThan(-1);
+    // A second occurrence would make the assertion below ambiguous about which call it read.
+    expect(src.indexOf(marker, at + 1)).toBe(-1);
+    const open = at + marker.length - 1;
+    let depth = 0;
+    for (let i = open; i < src.length; i++) {
+      if (src[i] === "(") depth++;
+      else if (src[i] === ")" && --depth === 0) return src.slice(open, i + 1);
+    }
+    throw new Error("unbalanced call arguments for: " + marker);
+  }
+
+  /** The object literal opened by `marker` (which must end at its own `{`). */
+  function objectLiteral(src: string, marker: string): string {
+    const at = src.indexOf(marker);
+    expect(at).toBeGreaterThan(-1);
+    expect(src.indexOf(marker, at + 1)).toBe(-1);
+    const open = at + marker.length - 1;
+    let depth = 0;
+    for (let i = open; i < src.length; i++) {
+      if (src[i] === "{") depth++;
+      else if (src[i] === "}" && --depth === 0) return src.slice(open, i + 1);
+    }
+    throw new Error("unbalanced object literal for: " + marker);
+  }
+
+  test("native compact passes the retention at its own initial resolution", () => {
+    const compact = source("src", "server", "responses", "compact.ts");
+    const initialAuth = callArguments(
+      compact,
+      "if (route.codexAccountMode) authCtx = await resolveCodexAuthContext(",
+    );
+
+    expect(initialAuth).toContain("retainAccountForUploadedFiles: conversationCarriesUploadedFiles(raw)");
+  });
+
+  test("the post-429 compact guard stays, because it answers a different question", () => {
+    // The guard is not redundant with the retention above: retention declines a VOLUNTARY quota
+    // move, while this refuses an alternate account after the issuing one has already rejected
+    // the send. Removing either one reopens half of #4778.
+    const compact = source("src", "server", "responses", "compact.ts");
+
+    expect(compact).toContain("const alternate = conversationCarriesUploadedFiles(raw)");
+  });
+
+  test("both Responses previews answer the same question final auth does", () => {
+    const prepare = source("src", "server", "responses", "request-prepare.ts");
+    const retention = "retainAccountForUploadedFiles: conversationCarriesUploadedFiles(parsed._rawBody)";
+
+    expect(objectLiteral(prepare, "const previewSelectionOptions = {")).toContain(retention);
+    expect(objectLiteral(prepare, "const recoverySelectionOptions = {")).toContain(retention);
+    expect(callArguments(prepare, "const finalAuth = await resolveResponsesCodexAuth("))
+      .toContain("conversationCarriesUploadedFiles(parsed._rawBody)");
+  });
+});