From cd75f44f9cd3fb2476beb313f87d8b8acd50f2f5 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 29 Sep 2026 13:40:46 -0700 Subject: [PATCH] fix(setup): restore credentials before recovery reads auth A failed sign-in activation can publish restored config before rolling back its credential. Recovery may then capture auth rows that the rollback invalidates, leaving the old connection restored on disk but unavailable in the Gateway. Restore the credential during the recovery application's existing guarded preparation, before auth capture. Consume its receipt once so the fallback after config undo cannot invalidate that recovery read again. Preserve config supersession, credential ownership, and stale-read rejection without retries. Strengthen the existing integration case with a real auth-read/rollback barrier. The unmodified original seven-file replay passed 79 tests; the deterministic regression then reproduced the exact stale-auth recovery failure before the fix. The final original-group replay passed 79 tests in 52.70s with eight workers, and 136 adjacent wizard/setup/auth tests passed in 215.39s wrapper time. Local proof used Node 24.21.0; original CI used Node 24.19.0. No passing CI outcome is inferred from these local results. Core and focused test types, graph boundaries, lint, formatting, diff checks, size/env/assertion ratchets, and independent P2 review passed. No schema, configuration, permission, dependency, or installed-updater contract changes. --- docs/auth-credential-semantics.md | 8 ++-- ...nfig-reload.activation.integration.test.ts | 38 ++++++++++++++++++- .../setup-inference-transition.ts | 9 ++++- 3 files changed, 49 insertions(+), 6 deletions(-) diff --git a/docs/auth-credential-semantics.md b/docs/auth-credential-semantics.md index 86ac7b911fd4..ef9bbc28d142 100644 --- a/docs/auth-credential-semantics.md +++ b/docs/auth-credential-semantics.md @@ -65,9 +65,11 @@ another agent. Only the owning setup operation can test the selected credential. After one successful tool-free turn, setup asks whether to activate it. Declining or failing the test keeps the saved credential inactive and preserves the current connection. Model Setup offers the same saved sign-in for a fresh test without -another login. Gateway activation waits for config application; a required restart -keeps the replacement inactive until setup is retried. Ordinary login remains -immediate. The descriptor retains the selected model and connection settings for retry after +another login. If applying a replacement fails, recovery restores its inactive +state before rebuilding the previous connection. Gateway activation waits for +config application; a required restart keeps the replacement inactive until setup +is retried. Ordinary login remains immediate. The descriptor retains the selected +model and connection settings for retry after restart, without caching a verification result. This adds no database schema or migration; older runtimes do not enforce the inactive state. Before downgrading, remove saved inactive replacements or restore the state from before setup. diff --git a/src/gateway/config-reload.activation.integration.test.ts b/src/gateway/config-reload.activation.integration.test.ts index 3ece81868f99..5d66f02cdd2e 100644 --- a/src/gateway/config-reload.activation.integration.test.ts +++ b/src/gateway/config-reload.activation.integration.test.ts @@ -5,7 +5,9 @@ import fs from "node:fs/promises"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { createDeferred } from "../../test/helpers/promise.js"; import { resolveApiKeyForProfile } from "../agents/auth-profiles/oauth.js"; +import { runtimeAuthProfileRowsCache } from "../agents/auth-profiles/runtime-snapshots.js"; import { loadAuthProfileStoreWithoutExternalProfiles } from "../agents/auth-profiles/store-runtime.js"; +import * as authProfileStore from "../agents/auth-profiles/store.js"; import type { AuthProfileCredential } from "../agents/auth-profiles/types.js"; import { prepareModelRuntimeSnapshot, @@ -220,6 +222,32 @@ describe("setup activation reload ownership", () => { const completion = createDeferred<() => Promise>(); const applied = createDeferred>(); let recoveryApplication: ReturnType | undefined; + // Recovery can capture auth before the config write returns to its rollback caller. + const prepareRows = runtimeAuthProfileRowsCache.prepare.bind(runtimeAuthProfileRowsCache); + const rowRead = vi + .spyOn(runtimeAuthProfileRowsCache, "prepare") + .mockImplementation((...args) => { + const reader = prepareRows(...args); + if (outcome !== "runtime-failed" || !recoveryApplication?.claimed) { + return reader; + } + return { + ...reader, + async read() { + const rows = await reader.read(); + captureEntered.resolve(); + await releaseCapture.promise; + return rows; + }, + }; + }); + const restoreAuth = authProfileStore.restoreAuthProfileStorePersistenceSnapshot; + const rollback = vi + .spyOn(authProfileStore, "restoreAuthProfileStorePersistenceSnapshot") + .mockImplementation((...args) => { + restoreAuth(...args); + releaseCapture.resolve(); + }); try { await reloader.ready; if (scenario === "superseded") { @@ -261,9 +289,13 @@ describe("setup activation reload ownership", () => { writeOptions, transform: (_current, context) => { const undo = captureSetupInferenceFileUndo(context.snapshot, candidate); - captureUndo((options) => { + captureUndo(async (options) => { recoveryApplication = getRuntimeConfigWriteApplication(options); - return undo(options); + const restored = await undo(options); + if (outcome === "runtime-failed") { + await captureEntered.promise; + } + return restored; }); return { nextConfig: candidate }; }, @@ -419,6 +451,8 @@ describe("setup activation reload ownership", () => { try { await reloader.stop(); } finally { + rowRead.mockRestore(); + rollback.mockRestore(); configFileAdapter.mockRestore(); } } diff --git a/src/system-agent/setup-inference-transition.ts b/src/system-agent/setup-inference-transition.ts index 5d35c96c66d7..71fdd07a8eb0 100644 --- a/src/system-agent/setup-inference-transition.ts +++ b/src/system-agent/setup-inference-transition.ts @@ -134,10 +134,17 @@ export async function commitSetupInferenceActivation(params: { }) : undefined; const restore = async () => { + const rollbackCredential = () => { + credential?.rollback(); + credential = undefined; + }; const restoredApplication = application ? createRuntimeConfigWriteApplication(continuation, { prepare: async (assertCurrent) => { assertApplicationCurrent = assertCurrent; + assertCurrent(); + // Recovery must capture auth only after restoring this activation's credential. + rollbackCredential(); }, }) : undefined; @@ -145,7 +152,7 @@ export async function commitSetupInferenceActivation(params: { configCommitted && undoConfig ? await undoConfig(attachRuntimeConfigWriteApplication({}, restoredApplication)) : { config: (await params.configTarget.read()).config, written: false }; - credential?.rollback(); + rollbackCredential(); if (restoredApplication && restored.written) { if (!restoredApplication.claimed || (await restoredApplication.result) !== "applied") { throw new Error(