From 6c07c5d20d7c2beb120d7de2c0b4e7a69243fd13 Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Sat, 31 Oct 2026 01:13:01 +0900 Subject: [PATCH 1/3] fix(codex): admit automatic v1 reconciliation before CLI selection changes --- .../docs/fr/reference/configuration/agents.md | 7 ++ .../docs/reference/configuration/agents.md | 7 ++ .../docs/tr/reference/configuration/agents.md | 7 ++ src/codex/inject.ts | 9 +- src/codex/inject/multi-agent-v2.ts | 34 +++++- structure/codex-cli-update.md | 8 +- structure/config.md | 2 +- .../codex-inject-v1-reconcile.test.ts | 100 ++++++++++++++++++ 8 files changed, 167 insertions(+), 7 deletions(-) diff --git a/docs-site/src/content/docs/fr/reference/configuration/agents.md b/docs-site/src/content/docs/fr/reference/configuration/agents.md index 495c22ffdb6..e73e248598f 100644 --- a/docs-site/src/content/docs/fr/reference/configuration/agents.md +++ b/docs-site/src/content/docs/fr/reference/configuration/agents.md @@ -122,3 +122,10 @@ Un plafond ne peut que réduire l’effort. Le niveau retenu est le niveau annon Les plafonds configurés s’appliquent aussi aux tours Chat Completions natifs admissibles sans effort épinglé pour le modèle. La conversion vers la valeur du fournisseur intervient lorsqu’un épinglage est appliqué ou qu’un plafond modifie la valeur ; sinon, la valeur de l’appelant natif conserve sa forme d’origine. Pour une présentation destinée aux débutants des comportements v1, default et v2, consultez [Surfaces de sous-agents](/fr/guides/sub-agent-surface/). + +## Mise à jour de la CLI pendant la synchronisation v1 + +Si la synchronisation avec `multiAgentMode: "v1"` doit désactiver la substitution v2 globale +de Codex, une mise à jour de la CLI Codex en cours peut refuser temporairement cette +synchronisation. Réessayez après la mise à jour. Un répertoire Codex utilisant déjà v1 +peut toujours être synchronisé, car aucune commande native de fonctionnalité n'est nécessaire. diff --git a/docs-site/src/content/docs/reference/configuration/agents.md b/docs-site/src/content/docs/reference/configuration/agents.md index d2474b94f1c..e2c9f1faf34 100644 --- a/docs-site/src/content/docs/reference/configuration/agents.md +++ b/docs-site/src/content/docs/reference/configuration/agents.md @@ -393,3 +393,10 @@ after a compaction model switch or combo failover. alongside the existing caps: omitted fields stay unchanged, `null` clears the map, and a map entry set to `null` or `""` deletes only that key. Invalid combined updates leave both caps and pins unchanged. Saving a pin does not alter the featured subagent roster. + +## CLI updates during v1 synchronization + +When an explicit `multiAgentMode: "v1"` synchronization must disable Codex's global v2 +override, an ongoing Codex CLI update can temporarily refuse the synchronization. +Retry after the update finishes. A home already using v1 can still synchronize during +the update because no native feature command is needed. diff --git a/docs-site/src/content/docs/tr/reference/configuration/agents.md b/docs-site/src/content/docs/tr/reference/configuration/agents.md index 5fec449caa1..b51ba27d89f 100644 --- a/docs-site/src/content/docs/tr/reference/configuration/agents.md +++ b/docs-site/src/content/docs/tr/reference/configuration/agents.md @@ -263,3 +263,10 @@ Yapılandırılmış sınırlar, model effort sabitlemesi olmayan uygun yerel Ch v1, varsayılan ve v2 davranışının yeni başlayanlara yönelik açıklaması için [Alt ajan yüzeyleri](/tr/guides/sub-agent-surface/) sayfasına bakın. + +## V1 eşitlemesi sırasında CLI güncellemeleri + +`multiAgentMode: "v1"` eşitlemesinin Codex'in genel v2 geçersiz kılmasını kapatması +gerekiyorsa, devam eden bir Codex CLI güncellemesi eşitlemeyi geçici olarak reddedebilir. +Güncelleme bittikten sonra yeniden deneyin. Zaten v1 kullanan bir Codex dizini, yerel +özellik komutu gerekmediği için güncelleme sırasında da eşitlenebilir. diff --git a/src/codex/inject.ts b/src/codex/inject.ts index 4d30336ce0f..634f7314144 100644 --- a/src/codex/inject.ts +++ b/src/codex/inject.ts @@ -446,7 +446,7 @@ async function injectCodexConfigImpl( if (v1Reconcile) { const reconciled = v1Reconcile.run(heldConfigWriteLock); if (!reconciled.ok) { - throw new CodexInjectRefusal({ success: false, message: reconciled.message }); + throw new CodexInjectRefusal({ success: false, message: reconciled.message, retryable: reconciled.retryable }); } nativeInput = reconciled.content; if (reconciled.content !== rawContent) { @@ -548,6 +548,10 @@ async function injectCodexConfigImpl( */ let effectivePlan: CodexInjectionPlanOk = admittedPlan; + if (v1Reconcile && !await v1Reconcile.admit()) { + return { success: false, retryable: true, message: "Codex config injection refused: the CLI selection is busy. Retry after the CLI update finishes." }; + } + try { const configLock = await acquireConfigWriteLock(CODEX_CONFIG_PATH, { timeoutMs: Math.min(options.lockTimeoutMs ?? CONFIG_WRITE_LOCK_WAIT_MS, CONFIG_WRITE_LOCK_WAIT_MS), }); @@ -725,6 +729,9 @@ async function injectCodexConfigImpl( releaseConfigWriteLock(configLock.handle); heldConfigWriteLock = undefined; } + } finally { + v1Reconcile?.release(); + } // Both write paths have committed, including the coordinator's post-callback checks. // Compare against the admitted original, not post-v1-reconcile input: the native // feature transition is part of this transaction too. Refusals/rollbacks return diff --git a/src/codex/inject/multi-agent-v2.ts b/src/codex/inject/multi-agent-v2.ts index fb242c81a94..f3168972b2d 100644 --- a/src/codex/inject/multi-agent-v2.ts +++ b/src/codex/inject/multi-agent-v2.ts @@ -2,6 +2,7 @@ import { readFileSync } from "node:fs"; import type { OcxConfig } from "../../types"; import { ConfigWriteDestinationChanged, type LockHandle } from "../config-write-lock"; import { CODEX_CONFIG_PATH } from "../paths"; +import type { CodexCliSelectionAsyncParentWrite } from "../cli-update-selection-gate"; /** * Reconcile the native `features.multi_agent_v2` override when an injection carries an @@ -17,7 +18,7 @@ import { CODEX_CONFIG_PATH } from "../paths"; */ export type InjectedV1SurfaceReconcile = | { ok: true; content: string; changed: boolean } - | { ok: false; message: string }; + | { ok: false; message: string; retryable?: boolean }; let toggleForTests: ((enabled: boolean, env: NodeJS.ProcessEnv, validateBeforeSpawn: () => void) => void) | undefined; @@ -44,6 +45,9 @@ export interface PreparedV1SurfaceReconcile { * only — `run()` re-checks the flag on the bytes present under the lock. */ readonly enabledAtPrepare: boolean; + /** Acquire before the config lock; an already-v1 home needs no selection lease. */ + admit(): Promise; + release(): void; /** * `heldConfigWriteLock` is the injector's own held write-lock handle: the * transition would otherwise re-acquire the same file and refuse itself. @@ -74,17 +78,39 @@ export async function prepareInjectedV1SurfaceReconcile( toggle = (enabled, env, validate) => runCodexFeaturesCommand(enabled ? "enable" : "disable", "multi_agent_v2", env, validate); } const resolvedToggle = toggle; + const { beginCodexCliSelectionParentWriteAsync, CodexCliSelectionMutationBusyError } = await import("../cli-update-selection-gate"); + let selection: CodexCliSelectionAsyncParentWrite | undefined; + const busy = (): InjectedV1SurfaceReconcile => ({ + ok: false, retryable: true, + message: "Codex config injection refused: the v1 transition has no current CLI selection admission. Retry the injection after the CLI update finishes.", + }); return { enabledAtPrepare, + async admit() { + if (!enabledAtPrepare) return true; + if (selection) return selection.entered; + selection = await beginCodexCliSelectionParentWriteAsync(); + return selection.entered; + }, + release() { selection?.exit(); }, run(heldConfigWriteLock) { // Decide on the bytes present NOW, under the lock — the prepare-time // answer is stale the moment another writer could have touched the file. if (!isMultiAgentV2Enabled()) { return { ok: true, content: readFileSync(CODEX_CONFIG_PATH, "utf-8"), changed: false }; } - const transition = transitionMultiAgentV2(false, resolvedToggle, { - ...(heldConfigWriteLock !== undefined ? { heldConfigWriteLock } : {}), - }); + // A prepare-time no-op may have become a transition. Refuse without a child; + // a fresh pass can acquire admission before taking the config lock. + if (!selection?.entered) return busy(); + let transition: ReturnType; + try { + transition = selection.commit(() => transitionMultiAgentV2(false, resolvedToggle, { + ...(heldConfigWriteLock !== undefined ? { heldConfigWriteLock } : {}), + })); + } catch (error) { + if (error instanceof CodexCliSelectionMutationBusyError) return busy(); + throw error; + } if (!transition.ok) { if (transition.retryable === false) throw new ConfigWriteDestinationChanged(transition.error); return { diff --git a/structure/codex-cli-update.md b/structure/codex-cli-update.md index 310df791db4..2fdfe06f8aa 100644 --- a/structure/codex-cli-update.md +++ b/structure/codex-cli-update.md @@ -412,7 +412,13 @@ inherit an earlier async verdict. Ordinary atomic secret-file publication and na retain their synchronous leaf contracts; the nonblocking guarantee covers recovery admission. CLI, machine API, restart and Remote Workspace callers use the async entry points. Runtime publication rechecks selection inputs, cache epoch, cancellation and caller -currentness after recovery. Codex feature-setting callers acquire admission before their synchronous +currentness after recovery. Automatic v1 injection uses the same async admission before its config +lock when the global v2 flag needs disabling. Its synchronous feature-transition commit permits +nested runtime-pin writes while excluding the update worker; already-v1 no-ops acquire no lease, +and a newly enabled flag without admission refuses retryably before spawning. +The injection releases admission after config commit or rollback, including config-lock acquisition +failure, before its history worker starts. The feature transition retains the shared artifact preimage. +Codex feature-setting callers acquire admission before their synchronous transition; management keeps the lease across config-root hardening, then rechecks its mode plan and commits in the existing no-await callback. Refusal precedes combined setting writes and returns a retryable `codex_cli_selection_busy` response. Diagnostics classify a held async parent by its exact diff --git a/structure/config.md b/structure/config.md index 5e33e304171..ec7f68f19ba 100644 --- a/structure/config.md +++ b/structure/config.md @@ -217,7 +217,7 @@ Snapshot restore rechecks default ownership after hash matches; root `model_cont An injection whose OpenCodex config explicitly selects the v1 multi-agent surface also reconciles Codex's higher-precedence global `features.multi_agent_v2` override to disabled before taking the journal baseline. It uses the same format-preserving feature transition as explicit -mode selection, and it runs inside the injection's coordinated write boundary: the transition and +mode selection with [CLI selection admission](codex-cli-update.md#worker-realm-and-the-selection-commit-window). Inside the coordinated write boundary, the transition and the artifact commit share one preimage. A publication conflict after the toggle, or final coordinator validation or commit failure after the artifact writes, restores the flag, config, profile and journal while the native and config locks are held. No competing writer can land diff --git a/tests/codex-integration/codex-inject-v1-reconcile.test.ts b/tests/codex-integration/codex-inject-v1-reconcile.test.ts index 1f1946b720c..27f5b59f63c 100644 --- a/tests/codex-integration/codex-inject-v1-reconcile.test.ts +++ b/tests/codex-integration/codex-inject-v1-reconcile.test.ts @@ -90,6 +90,79 @@ function runV1CoordinatorFailure( return JSON.parse(child.stdout); } +function runSelectionScenario(codexHome: string, ocxHome: string, scenario: string) { + const script = ` + const fs = require("node:fs"), { join } = require("node:path"); + const { spyOn } = require("bun:test"); + const gate = require("./src/codex/cli-update-selection-gate"); + const recovery = require("./src/codex/cli-update-recovery"); + const features = require("./src/codex/features"); + const locks = require("./src/codex/config-write-lock"); + const runtime = require("./src/codex/runtime"); + const { injectCodexConfig } = require("./src/codex/inject"); + const { setCodexMultiAgentV2ToggleForTests } = require("./src/codex/inject/multi-agent-v2"); + const scenario = process.env.TEST_SCENARIO; + const path = join(process.env.CODEX_HOME, "config.toml"); + const original = 'model = "gpt-5.5"\\n[features]\\nmulti_agent_v2 = ' + (scenario === "noop" ? "false" : "true") + '\\n'; + fs.writeFileSync(path, original); + let asyncProbes = 0, syncProbes = 0, toggles = 0, seenGate, nestedEntered, workerEntered; + spyOn(recovery, "codexCliUpdateRecoveryBlocksAdmissionAsync").mockImplementation(async () => { asyncProbes++; return false; }); + spyOn(recovery, "codexCliUpdateRecoveryBlocksAdmission").mockImplementation(() => { syncProbes++; throw new Error("unexpected sync recovery"); }); + const worker = gate.createWorkerCommitWindow({ gate: gate.codexCliUpdateSelectionGate(), fence: gate.createCodexCliUpdateFence() }); + if (scenario === "busy" || scenario === "noop") worker.tryEnter(); + if (scenario === "aba") { + const inspect = features.isMultiAgentV2Enabled; + spyOn(features, "isMultiAgentV2Enabled").mockImplementationOnce(() => { + fs.writeFileSync(path, original.replace("= true", "= false")); + const value = inspect(); + fs.writeFileSync(path, original); + return value; + }); + } + if (scenario === "lock-failure") spyOn(locks, "acquireConfigWriteLock").mockResolvedValue({ ok: false, error: "locked" }); + if (scenario === "lock-throw") spyOn(locks, "acquireConfigWriteLock").mockRejectedValue(new Error("fixture lock failure")); + if (scenario === "lost-owner") { + const acquire = locks.acquireConfigWriteLock; + spyOn(locks, "acquireConfigWriteLock").mockImplementation(async (...args) => { + const acquired = await acquire(...args); + const buffer = gate.codexCliUpdateSelectionGate(); + gate.releaseCodexCliUpdateGateOwner(buffer, Atomics.load(new Int32Array(buffer), 0)); + worker.tryEnter(); + return acquired; + }); + } + setCodexMultiAgentV2ToggleForTests(enabled => { + toggles++; + seenGate = gate.codexCliSelectionGateState(); + const nested = gate.beginCodexCliSelectionParentWrite(); + nestedEntered = nested.entered; + nested.exit(); + workerEntered = worker.tryEnter(); + if (workerEntered) worker.exit(); + if (scenario === "persist") runtime.persistCodexRuntime({ command: "fixture-codex", source: "path", version: "1.0.0" }); + if (scenario === "toggle-failure") throw new Error("fixture toggle failure"); + fs.writeFileSync(path, fs.readFileSync(path, "utf8").replace("= true", "= " + enabled)); + }); + let result; + try { result = await injectCodexConfig(10100, { multiAgentMode: "v1" }); } + catch (error) { result = { threw: String(error) }; } + const after = fs.readFileSync(path, "utf8"); + const beforeExitGate = gate.codexCliSelectionGateState(); + worker.exit(); + const runtimePath = join(process.env.OPENCODEX_HOME, "codex-runtime.json"); + console.log(JSON.stringify({ result, toggles, asyncProbes, syncProbes, seenGate, nestedEntered, workerEntered, + unchanged: after === original, disabled: after.includes("multi_agent_v2 = false"), beforeExitGate, + finalGate: gate.codexCliSelectionGateState(), runtime: fs.existsSync(runtimePath) ? JSON.parse(fs.readFileSync(runtimePath, "utf8")) : null })); + `; + const child = spawnSync(process.execPath, ["--eval", script], { + cwd: repoRoot, + env: { ...process.env, CODEX_HOME: codexHome, OPENCODEX_HOME: ocxHome, TEST_SCENARIO: scenario }, + encoding: "utf8", timeout: SPAWN_BUDGET_MS - 5_000, + }); + expect(child.status, child.stderr).toBe(0); + return JSON.parse(child.stdout); +} + describe("injectCodexConfig v1-surface reconcile", () => { let codexHome: string; let ocxHome: string; @@ -104,6 +177,33 @@ describe("injectCodexConfig v1-surface reconcile", () => { removeTreeWithRetry(ocxHome); }); + test.each(["busy", "aba", "lost-owner"])("selection admission refuses %s before the native toggle", scenario => { + const out = runSelectionScenario(codexHome, ocxHome, scenario); + expect(out.result).toMatchObject({ success: false, retryable: true }); + expect(out).toMatchObject({ toggles: 0, unchanged: true, syncProbes: 0, finalGate: "free" }); + }); + + test.each(["selected", "persist"])("selection admission owns the toggle and nested %s runtime writes", scenario => { + const out = runSelectionScenario(codexHome, ocxHome, scenario); + expect(out.result).toMatchObject({ success: true }); + expect(out).toMatchObject({ toggles: 1, asyncProbes: 1, syncProbes: 0, seenGate: "parent_write", + nestedEntered: true, workerEntered: false, disabled: true, beforeExitGate: "free", finalGate: "free" }); + if (scenario === "persist") expect(out.runtime).toMatchObject({ command: "fixture-codex", selectedVersion: "1.0.0" }); + }); + + test("selection admission leaves an already-v1 injection available during worker commit", () => { + const out = runSelectionScenario(codexHome, ocxHome, "noop"); + expect(out.result).toMatchObject({ success: true }); + expect(out).toMatchObject({ toggles: 0, asyncProbes: 0, syncProbes: 0, disabled: true, beforeExitGate: "worker_commit", finalGate: "free" }); + }); + + test.each(["lock-failure", "lock-throw", "toggle-failure"])("selection admission releases ownership after %s", scenario => { + const out = runSelectionScenario(codexHome, ocxHome, scenario); + if (scenario === "lock-throw") expect(out.result.threw).toContain("fixture lock failure"); + else expect(out.result.success).toBe(false); + expect(out).toMatchObject({ asyncProbes: 1, unchanged: true, beforeExitGate: "free", finalGate: "free" }); + }); + test("inject does not turn on multi_agent_v2; fresh installs stay on Codex's default v1 surface until the user opts in", () => { writeFileSync(join(codexHome, "config.toml"), 'model = "gpt-5.5"\n', "utf8"); From afb7010a56f738ca23e10a9ceabcedeee7e2eb9a Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Sat, 31 Oct 2026 03:06:19 +0900 Subject: [PATCH 2/3] test(copilot): drain case-owned work before restoring fixtures --- .../github-copilot-account-origin.test.ts | 141 ++++++++++++++---- 1 file changed, 113 insertions(+), 28 deletions(-) diff --git a/tests/providers/github-copilot/github-copilot-account-origin.test.ts b/tests/providers/github-copilot/github-copilot-account-origin.test.ts index 845d3e29cb5..05894168fc7 100644 --- a/tests/providers/github-copilot/github-copilot-account-origin.test.ts +++ b/tests/providers/github-copilot/github-copilot-account-origin.test.ts @@ -3,8 +3,8 @@ import { mkdtempSync} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { clearGenericFailoverHealth } from "../../../src/oauth/generic-account-failover"; -import { getAccountSet, saveCredential, setActiveAccount } from "../../../src/oauth/store"; -import { handleResponses } from "../../../src/server/responses"; +import { getAccountSet, saveCredential as persistCredential, setActiveAccount as selectAccount } from "../../../src/oauth/store"; +import { handleResponses as dispatchResponses } from "../../../src/server/responses"; import { saveConfig } from "../../../src/config"; import { setActiveProviderApiKey } from "../../../src/providers/api-keys"; import type { OcxConfig } from "../../../src/types"; @@ -21,15 +21,67 @@ const originalHome = process.env.OPENCODEX_HOME; let home = ""; let beforeBuildReturns: (() => Promise) | undefined; let beforePacingReturns: (() => Promise) | undefined; +let currentCase: ReturnType; +function createCaseLifetime(cleanup: () => void) { + const controller = new AbortController(); + const pending = new Set>(); + const responses = new Set(); + let closed = false, closing: Promise | undefined; + function track(promise: Promise): Promise { + const observed = promise.finally(() => pending.delete(observed)); + pending.add(observed); + // A timed-out test no longer awaits its work, but teardown still owns its rejection. + void observed.catch(() => {}); + return observed; + } + return { controller, track, responses, get closed() { return closed; }, + assertClosed() { if (!closed) throw new Error("previous case teardown did not settle"); }, + close(): Promise { + return closing ??= (async () => { + controller.abort(); + while (pending.size) await Promise.allSettled([...pending]); + for (const response of responses) { + if (!response.bodyUsed) await response.body?.cancel(); + } + responses.clear(); + cleanup(); + closed = true; + })(); + }, + }; +} +function handleResponses(...args: Parameters): Promise { + const owner = currentCase; + if (owner.controller.signal.aborted) return Promise.reject(owner.controller.signal.reason); + return owner.track(dispatchResponses(...args).then(response => { + owner.responses.add(response); owner.controller.signal.throwIfAborted(); return response; + })); +} +function responseText(response: Response): Promise { + const owner = currentCase; + return owner.track(response.text().then(text => { owner.controller.signal.throwIfAborted(); return text; })); +} +function saveCredential(...args: Parameters) { + const owner = currentCase; + if (owner.controller.signal.aborted) return Promise.reject(owner.controller.signal.reason); + return owner.track(persistCredential(...args).then(value => { owner.controller.signal.throwIfAborted(); return value; })); +} +function setActiveAccount(...args: Parameters) { + const owner = currentCase; + if (owner.controller.signal.aborted) return Promise.reject(owner.controller.signal.reason); + return owner.track(selectAccount(...args).then(value => { owner.controller.signal.throwIfAborted(); return value; })); +} const actualPacing = await import("../../../src/providers/request-pacing"); const originalWaitForSlot = actualPacing.waitForProviderRequestSlot; mock.module("../../../src/providers/request-pacing", () => ({ ...actualPacing, waitForProviderRequestSlot: async (...args: Parameters) => { + const owner = currentCase; const result = await originalWaitForSlot(...args); + if (args[3]?.aborted || owner.controller.signal.aborted) return result; const gate = beforePacingReturns; beforePacingReturns = undefined; - await gate?.(); + if (gate) await owner.track(gate()); return result; }, })); @@ -38,13 +90,15 @@ const originalResolveAdapter = actualAdapterResolver.resolveAdapter; mock.module("../../../src/server/adapter-resolve", () => ({ ...actualAdapterResolver, resolveAdapter: (...args: Parameters) => { + const owner = currentCase; const adapter = originalResolveAdapter(...args); const build = adapter.buildRequest.bind(adapter); adapter.buildRequest = async (...buildArgs) => { const built = await build(...buildArgs); + if (buildArgs[1]?.abortSignal?.aborted || owner.controller.signal.aborted) return built; const gate = beforeBuildReturns; beforeBuildReturns = undefined; - await gate?.(); + if (gate) await owner.track(gate()); return built; }; return adapter; @@ -79,6 +133,7 @@ function request(wire: Wire, extra: Record = {}): Request { return new Request("http://localhost/v1/responses", { method: "POST", headers: { "content-type": "application/json" }, + signal: currentCase.controller.signal, body: JSON.stringify({ model: `github-copilot/${model}`, input: "hello", stream: false, ...extra }), }); } @@ -188,12 +243,22 @@ function installFetch(options: { return { dispatches }; } -let releaseSpendHome: (() => void) | undefined; - beforeEach(() => { + // A hook timeout must not install a new fetch/home while an old teardown is still draining. + currentCase?.assertClosed(); beforeBuildReturns = undefined; beforePacingReturns = undefined; home = mkdtempSync(join(tmpdir(), "ocx-copilot-origin-")); + const ownedHome = home; + let releaseSpendHome: (() => void) | undefined; + currentCase = createCaseLifetime(() => { + releaseSpendHome?.(); + globalThis.fetch = originalFetch; + clearGenericFailoverHealth(); + if (originalHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = originalHome; + removeTreeWithRetry(ownedHome); + }); process.env.OPENCODEX_HOME = home; clearGenericFailoverHealth(); // Dispatches without starting a server, so it takes the spend-journal lease itself. Taken @@ -201,19 +266,39 @@ beforeEach(() => { releaseSpendHome = acquireOwnedSpendHome(); }); -afterEach(() => { - // Released before this case's home is removed: an open lease inside a directory being - // deleted fails the removal on Windows and leaves an unlinked live database on POSIX. - releaseSpendHome?.(); - releaseSpendHome = undefined; - globalThis.fetch = originalFetch; - clearGenericFailoverHealth(); - if (originalHome === undefined) delete process.env.OPENCODEX_HOME; - else process.env.OPENCODEX_HOME = originalHome; - removeTreeWithRetry(home); -}); +afterEach(async () => { await currentCase?.close(); }); describe("GitHub Copilot bearer/origin snapshot atomicity", () => { + test.each(["pacing", "build"] as const)("teardown joins a held %s gate before replacing the case fetch", async phase => { + await seedAccounts(); + const entered = Promise.withResolvers(); + const release = Promise.withResolvers(); + const held = async () => { entered.resolve(); await release.promise; }; + if (phase === "pacing") beforePacingReturns = held; + else beforeBuildReturns = held; + installFetch({ wire: "chat", statuses: [200], switchOn: "never" }); + const ownedFetch = globalThis.fetch, ownedHome = home, owner = currentCase; + const pending = handleResponses(request("chat"), config("chat"), { model: "", provider: "" }); + const settled = pending.then(() => {}, () => {}); + try { + await Promise.race([entered.promise, pending.then(() => { throw new Error("request ended before fixture gate"); })]); + let finished = false; + const closing = owner.close(); + void closing.then(() => { finished = true; }); + expect(owner.close()).toBe(closing); + expect(owner.controller.signal.aborted).toBe(true); + expect(() => owner.assertClosed()).toThrow("previous case teardown did not settle"); + await Promise.resolve(); + expect(finished).toBe(false); + expect(globalThis.fetch).toBe(ownedFetch); expect(process.env.OPENCODEX_HOME).toBe(ownedHome); + release.resolve(); await closing; await settled; + expect(() => owner.assertClosed()).not.toThrow(); + const next = installFetch({ wire: "chat", statuses: [200], switchOn: "never" }); + await Promise.resolve(); + expect(next.dispatches).toEqual([]); + } finally { release.resolve(); await owner.close(); await settled; globalThis.fetch = originalFetch; } + }); + test.each(["chat", "responses", "image", "web-search"] as const)("%s API-key dispatch uses a selection committed during pacing", async path => { const cfg = config("chat"); cfg.defaultProvider = "fixture"; @@ -238,7 +323,7 @@ describe("GitHub Copilot bearer/origin snapshot atomicity", () => { model: "fixture/model", stream: path === "image" || path === "web-search", ...(path === "image" || path === "web-search" ? { tools: [{ type: path === "image" ? "image_generation" : "web_search" }] } : {}), }), cfg, { model: "", provider: "" }); - expect(await response.text()).toContain("ok"); + expect(await responseText(response)).toContain("ok"); expect(sent).toEqual(["Bearer synthetic-b"]); }); @@ -267,7 +352,7 @@ describe("GitHub Copilot bearer/origin snapshot atomicity", () => { { type: "input_image", image_url: "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==" }, ] }], }), cfg, { model: "", provider: "" }); - expect(await response.text()).toContain("ok"); + expect(await responseText(response)).toContain("ok"); expect(sent).toEqual(["Bearer synthetic-anthropic-b", "Bearer synthetic-anthropic-b"]); }); @@ -278,7 +363,7 @@ describe("GitHub Copilot bearer/origin snapshot atomicity", () => { const cfg = config("chat"); cfg.emptyCompletionRetry = true; const response = await handleResponses(request("chat"), cfg, { model: "", provider: "" }); - expect(await response.text()).toContain("ok"); + expect(await responseText(response)).toContain("ok"); expect(observed.dispatches).toEqual([ { origin: ACCOUNT_B_ORIGIN, authorization: bearer("copilot-access-b") }, { origin: ACCOUNT_B_ORIGIN, authorization: bearer("copilot-access-b") }, @@ -299,7 +384,7 @@ describe("GitHub Copilot bearer/origin snapshot atomicity", () => { const response = await handleResponses(request("chat", { stream: true, tools: [{ type: path === "image" ? "image_generation" : "web_search" }], }), cfg, { model: "", provider: "" }); - expect(await response.text()).toContain("ok"); + expect(await responseText(response)).toContain("ok"); expect(observed.dispatches).toEqual([{ origin: ACCOUNT_B_ORIGIN, authorization: bearer("copilot-access-b") }]); }); @@ -324,7 +409,7 @@ describe("GitHub Copilot bearer/origin snapshot atomicity", () => { const cfg = config("chat"); cfg.webSearchSidecar = { enabled: true, backend: "exa", exaApiKey: "synthetic-search-key" }; const response = await handleResponses(request("chat", { stream: true, tools: [{ type: "web_search" }] }), cfg, { model: "", provider: "" }); - expect(await response.text()).toContain("ok after search"); + expect(await responseText(response)).toContain("ok after search"); expect(sent).toEqual(["Bearer copilot-access-b", "Bearer copilot-access-b"]); expect(bodies[1]!.messages.some(message => message.role === "tool")).toBe(true); }); @@ -337,7 +422,7 @@ describe("GitHub Copilot bearer/origin snapshot atomicity", () => { let sends = 0; globalThis.fetch = (async () => { sends++; return successResponse("chat"); }) as typeof fetch; const response = await handleResponses(request("chat", { model: "fixture/model" }), cfg, { model: "", provider: "" }); - await response.text(); + await responseText(response); expect(response.status).not.toBe(200); expect(sends).toBe(0); }); @@ -348,7 +433,7 @@ describe("GitHub Copilot bearer/origin snapshot atomicity", () => { beforePacingReturns = async () => { await setActiveAccount("github-copilot", accounts.b); }; const observed = installFetch({ wire, statuses: [200], switchOn: "never" }); const response = await handleResponses(request(wire), config(wire), { model: "", provider: "" }); - await response.text(); + await responseText(response); expect(response.status).toBe(200); expect(observed.dispatches).toEqual([{ origin: ACCOUNT_B_ORIGIN, authorization: bearer("copilot-access-b") }]); }); @@ -357,7 +442,7 @@ describe("GitHub Copilot bearer/origin snapshot atomicity", () => { beforeBuildReturns = async () => { await setActiveAccount("github-copilot", accounts.b); }; const observed = installFetch({ wire, statuses: [200], switchOn: "never" }); const response = await handleResponses(request(wire), config(wire), { model: "", provider: "" }); - await response.text(); + await responseText(response); expect(response.status).toBe(200); expect(observed.dispatches).toEqual([{ origin: ACCOUNT_B_ORIGIN, authorization: bearer("copilot-access-b") }]); }); @@ -371,7 +456,7 @@ describe("GitHub Copilot bearer/origin snapshot atomicity", () => { }); const response = await handleResponses(request(wire), config(wire), { model: "", provider: "" }); - await response.text(); + await responseText(response); expect(response.status).toBe(200); expect(observed.dispatches).toEqual([{ @@ -390,7 +475,7 @@ describe("GitHub Copilot bearer/origin snapshot atomicity", () => { }); const response = await handleResponses(request(wire), config(wire), { model: "", provider: "" }); - await response.text(); + await responseText(response); expect(response.status).toBe(200); expect(observed.dispatches).toEqual([ @@ -405,7 +490,7 @@ describe("GitHub Copilot bearer/origin snapshot atomicity", () => { const observed = installFetch({ wire: "chat", statuses: [429, 200], switchOn: "never" }); const response = await handleResponses(request("chat"), config("chat"), { model: "", provider: "" }); - await response.text(); + await responseText(response); expect(response.status).toBe(200); expect(observed.dispatches).toEqual([ From bbaed36c388b723e5397845c3566435ad7c0397c Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Sat, 31 Oct 2026 04:54:09 +0900 Subject: [PATCH 3/3] test(copilot): preserve teardown failures and restore independent state --- .../github-copilot-account-origin.test.ts | 73 ++++++++++++++++--- 1 file changed, 63 insertions(+), 10 deletions(-) diff --git a/tests/providers/github-copilot/github-copilot-account-origin.test.ts b/tests/providers/github-copilot/github-copilot-account-origin.test.ts index 05894168fc7..e5d704cd9ed 100644 --- a/tests/providers/github-copilot/github-copilot-account-origin.test.ts +++ b/tests/providers/github-copilot/github-copilot-account-origin.test.ts @@ -40,16 +40,31 @@ function createCaseLifetime(cleanup: () => void) { return closing ??= (async () => { controller.abort(); while (pending.size) await Promise.allSettled([...pending]); + const failures: unknown[] = []; for (const response of responses) { - if (!response.bodyUsed) await response.body?.cancel(); + try { + if (!response.bodyUsed) await response.body?.cancel(); + responses.delete(response); + } catch (error) { failures.push(error); } } - responses.clear(); + // A failed cancellation may still own a reader/producer. Keep its home and fence; + // nevertheless attempt the other bodies so one failure does not strand all of them. + if (failures.length) throw failures[0]; cleanup(); closed = true; })(); }, }; } +function restoreCaseState(release: () => void, restore: Array<() => void>, removeHome: () => void): void { + const failures: unknown[] = []; + for (const step of [release, ...restore]) { + try { step(); } catch (error) { failures.push(error); } + } + // Restore independent globals after release failure, but never remove an uncertain live home. + if (failures.length) throw failures[0]; + removeHome(); +} function handleResponses(...args: Parameters): Promise { const owner = currentCase; if (owner.controller.signal.aborted) return Promise.reject(owner.controller.signal.reason); @@ -251,14 +266,13 @@ beforeEach(() => { home = mkdtempSync(join(tmpdir(), "ocx-copilot-origin-")); const ownedHome = home; let releaseSpendHome: (() => void) | undefined; - currentCase = createCaseLifetime(() => { - releaseSpendHome?.(); - globalThis.fetch = originalFetch; - clearGenericFailoverHealth(); - if (originalHome === undefined) delete process.env.OPENCODEX_HOME; - else process.env.OPENCODEX_HOME = originalHome; - removeTreeWithRetry(ownedHome); - }); + currentCase = createCaseLifetime(() => restoreCaseState( + () => { releaseSpendHome?.(); }, + [() => { globalThis.fetch = originalFetch; }, clearGenericFailoverHealth, + () => { if (originalHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = originalHome; }], + () => removeTreeWithRetry(ownedHome), + )); process.env.OPENCODEX_HOME = home; clearGenericFailoverHealth(); // Dispatches without starting a server, so it takes the spend-journal lease itself. Taken @@ -269,6 +283,45 @@ beforeEach(() => { afterEach(async () => { await currentCase?.close(); }); describe("GitHub Copilot bearer/origin snapshot atomicity", () => { + test.each(["locked", "reject"] as const)("teardown attempts all bodies and retains the fence after %s cancellation", async mode => { + const firstError = new Error("fixture cancel failed"); + let cleanupCalls = 0, otherCancels = 0; + const owner = createCaseLifetime(() => { cleanupCalls++; }); + const first = new Response(new ReadableStream({ cancel() { if (mode === "reject") throw firstError; } })); + const reader = mode === "locked" ? first.body!.getReader() : undefined; + const second = new Response(new ReadableStream({ cancel() { otherCancels++; } })); + owner.responses.add(first); owner.responses.add(second); + try { + const closing = owner.close(); + const failure = await closing.then(() => undefined, error => error); + if (mode === "reject") expect(failure).toBe(firstError); + else expect(failure).toBeInstanceOf(TypeError); + expect(owner.close()).toBe(closing); + expect(otherCancels).toBe(1); + expect(owner.responses.has(first)).toBe(true); + expect(owner.responses.has(second)).toBe(false); + expect(cleanupCalls).toBe(0); + expect(owner.closed).toBe(false); + expect(() => owner.assertClosed()).toThrow("did not settle"); + } finally { if (reader) { await reader.cancel(); reader.releaseLock(); } } + }); + + test("release failure restores every independent global but preserves the home and first error", async () => { + const firstError = new Error("fixture release failed"), calls: string[] = []; + const owner = createCaseLifetime(() => restoreCaseState( + () => { calls.push("release"); throw firstError; }, + [() => { calls.push("fetch"); }, () => { calls.push("health"); throw new Error("later failure"); }, + () => { calls.push("home-env"); }], + () => { calls.push("remove-home"); }, + )); + const closing = owner.close(); + expect(await closing.then(() => undefined, error => error)).toBe(firstError); + expect(calls).toEqual(["release", "fetch", "health", "home-env"]); + expect(owner.close()).toBe(closing); + expect(owner.closed).toBe(false); + expect(() => owner.assertClosed()).toThrow("did not settle"); + }); + test.each(["pacing", "build"] as const)("teardown joins a held %s gate before replacing the case fetch", async phase => { await seedAccounts(); const entered = Promise.withResolvers();