From 8fb462f84ce9254bebbbdb0ee826652dcb1f7b8d Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 27 Jul 2026 16:18:43 -0400 Subject: [PATCH] fix: gateway restart no longer takes the service down into a known-fatal auth migration (#114715) * fix(gateway): refuse to restart the service into a known-fatal auth migration * refactor(cli): keep service action preflight types module-local --- .../legacy-source-diagnostic.test.ts | 39 ++++++++ .../auth-profiles/legacy-source-diagnostic.ts | 46 +++++++-- .../lifecycle-action-preflight.test.ts | 65 +++++++++++++ .../daemon-cli/lifecycle-action-preflight.ts | 96 +++++++++++++++++++ .../daemon-cli/lifecycle-config-preflight.ts | 46 --------- src/cli/daemon-cli/lifecycle-core.ts | 10 +- 6 files changed, 241 insertions(+), 61 deletions(-) create mode 100644 src/agents/auth-profiles/legacy-source-diagnostic.test.ts create mode 100644 src/cli/daemon-cli/lifecycle-action-preflight.test.ts create mode 100644 src/cli/daemon-cli/lifecycle-action-preflight.ts delete mode 100644 src/cli/daemon-cli/lifecycle-config-preflight.ts diff --git a/src/agents/auth-profiles/legacy-source-diagnostic.test.ts b/src/agents/auth-profiles/legacy-source-diagnostic.test.ts new file mode 100644 index 000000000000..d59248a5dc45 --- /dev/null +++ b/src/agents/auth-profiles/legacy-source-diagnostic.test.ts @@ -0,0 +1,39 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { withTempDir } from "../../test-helpers/temp-dir.js"; +import { + assertAuthProfileMigrationReady, + clearAuthProfileMigrationDiagnostics, + listAuthProfileStoresRequiringMigration, +} from "./legacy-source-diagnostic.js"; +import { resolveAuthProfileDatabasePath } from "./sqlite.js"; + +afterEach(() => { + clearAuthProfileMigrationDiagnostics(); +}); + +describe("listAuthProfileStoresRequiringMigration", () => { + it("reports only credential sources without marking runtime migration state", async () => { + await withTempDir({ prefix: "openclaw-auth-migration-diagnostic-" }, async (root) => { + const credentialAgentDir = path.join(root, "credential-agent"); + const authStateAgentDir = path.join(root, "auth-state-agent"); + const env = { OPENCLAW_STATE_DIR: path.join(root, "state") }; + await fs.mkdir(credentialAgentDir, { recursive: true }); + await fs.mkdir(authStateAgentDir, { recursive: true }); + const credentialPath = path.join(credentialAgentDir, "auth-profiles.json"); + await fs.writeFile(credentialPath, "{}\n"); + await fs.writeFile(path.join(authStateAgentDir, "auth-state.json"), "{}\n"); + + expect( + listAuthProfileStoresRequiringMigration({ + agentDirs: [authStateAgentDir, credentialAgentDir, credentialAgentDir], + env, + }), + ).toEqual([resolveAuthProfileDatabasePath(credentialAgentDir)]); + + await fs.rm(credentialPath); + expect(() => assertAuthProfileMigrationReady(credentialAgentDir)).not.toThrow(); + }); + }); +}); diff --git a/src/agents/auth-profiles/legacy-source-diagnostic.ts b/src/agents/auth-profiles/legacy-source-diagnostic.ts index 92673e6d5f2f..2a4905493450 100644 --- a/src/agents/auth-profiles/legacy-source-diagnostic.ts +++ b/src/agents/auth-profiles/legacy-source-diagnostic.ts @@ -7,7 +7,7 @@ import { resolveSharedMainAuthAgentDir } from "./shared-main-dir.js"; import { resolveAuthProfileDatabasePath } from "./sqlite.js"; const AUTH_PROFILE_MIGRATION_REQUIRED_CODE = "AUTH_PROFILE_MIGRATION_REQUIRED" as const; -const AUTH_PROFILE_MIGRATION_COMMAND = "openclaw doctor --fix" as const; +export const AUTH_PROFILE_MIGRATION_COMMAND = "openclaw doctor --fix" as const; const log = createSubsystemLogger("auth-profiles/persistence"); type LegacyAuthProfileSourceKind = "auth-profiles" | "auth-state" | "legacy-auth" | "legacy-oauth"; @@ -17,6 +17,10 @@ type LegacyAuthProfileSource = { path: string; }; +function isCredentialSource(source: LegacyAuthProfileSource): boolean { + return source.kind !== "auth-state"; +} + export function resolveLegacyOAuthPath(env: NodeJS.ProcessEnv = process.env): string { return path.join(resolveOAuthDir(env), "oauth.json"); } @@ -79,20 +83,33 @@ export function listLegacyAuthProfileArchives(params: { } export function hasLegacyAuthProfileCredentialSource(agentDir?: string): boolean { - return listLegacyAuthProfileSources({ agentDir }).some((source) => source.kind !== "auth-state"); + return listLegacyAuthProfileSources({ agentDir }).some(isCredentialSource); +} + +function listStartupLegacyAuthProfileSources(params: { + agentDirs: readonly string[]; + env?: NodeJS.ProcessEnv; +}): Array<{ + agentDir: string; + sources: LegacyAuthProfileSource[]; + credentialSources: LegacyAuthProfileSource[]; +}> { + const sharedMainDir = resolveSharedMainAuthAgentDir(params.env); + return [...new Set([...params.agentDirs, sharedMainDir])].map((agentDir) => { + const sources = listLegacyAuthProfileSources({ agentDir, env: params.env }); + return { agentDir, sources, credentialSources: sources.filter(isCredentialSource) }; + }); } export function hasLegacyAuthProfileSourcesForStartup(params: { agentDirs: readonly string[]; env?: NodeJS.ProcessEnv; }): boolean { - const sharedMainDir = resolveSharedMainAuthAgentDir(params.env); - const candidates = new Set([...params.agentDirs, sharedMainDir]); let detected = false; - for (const agentDir of candidates) { - const sources = listLegacyAuthProfileSources({ agentDir, env: params.env }); + for (const { agentDir, sources, credentialSources } of listStartupLegacyAuthProfileSources( + params, + )) { detected ||= sources.length > 0; - const credentialSources = sources.filter((source) => source.kind !== "auth-state"); if (credentialSources.length > 0) { markAuthProfileMigrationRequired( agentDir, @@ -103,6 +120,17 @@ export function hasLegacyAuthProfileSourcesForStartup(params: { return detected; } +/** Agent auth stores whose retired credential files make gateway startup fail until Doctor migrates them. */ +export function listAuthProfileStoresRequiringMigration(params: { + agentDirs: readonly string[]; + env?: NodeJS.ProcessEnv; +}): string[] { + const owners = listStartupLegacyAuthProfileSources(params) + .filter(({ credentialSources }) => credentialSources.length > 0) + .map(({ agentDir }) => shortenHomePath(resolveAuthProfileDatabasePath(agentDir))); + return [...new Set(owners)].toSorted(); +} + export class AuthProfileMigrationRequiredError extends Error { readonly code = AUTH_PROFILE_MIGRATION_REQUIRED_CODE; readonly action = AUTH_PROFILE_MIGRATION_COMMAND; @@ -179,9 +207,7 @@ export function assertAuthProfileMigrationReady(agentDir?: string): void { } // Older shipped processes and restores can recreate these three fixed files // after startup, so this credential boundary deliberately rechecks their names. - const sources = listLegacyAuthProfileSources({ agentDir }).filter( - (source) => source.kind !== "auth-state", - ); + const sources = listLegacyAuthProfileSources({ agentDir }).filter(isCredentialSource); if (sources.length > 0) { const migrationError = new AuthProfileMigrationRequiredError({ agentDir, sources }); markAuthProfileMigrationRequired(agentDir, migrationError); diff --git a/src/cli/daemon-cli/lifecycle-action-preflight.test.ts b/src/cli/daemon-cli/lifecycle-action-preflight.test.ts new file mode 100644 index 000000000000..ff75ce3632c5 --- /dev/null +++ b/src/cli/daemon-cli/lifecycle-action-preflight.test.ts @@ -0,0 +1,65 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { resetConfigRuntimeState } from "../../config/config.js"; +import { withTempDir } from "../../test-helpers/temp-dir.js"; +import { getServiceActionPreflightFailure } from "./lifecycle-action-preflight.js"; + +afterEach(() => { + resetConfigRuntimeState(); + vi.unstubAllEnvs(); +}); + +async function withIsolatedLifecycleState( + run: (params: { agentDir: string }) => Promise, +): Promise { + await withTempDir({ prefix: "openclaw-lifecycle-action-preflight-" }, async (root) => { + const stateDir = path.join(root, "state"); + const configPath = path.join(root, "openclaw.json"); + const agentDir = path.join(stateDir, "agents", "main", "agent"); + await fs.mkdir(agentDir, { recursive: true }); + await fs.writeFile(configPath, "{}\n"); + vi.stubEnv("HOME", root); + vi.stubEnv("OPENCLAW_STATE_DIR", stateDir); + vi.stubEnv("OPENCLAW_CONFIG_PATH", configPath); + resetConfigRuntimeState(); + await run({ agentDir }); + }); +} + +describe("getServiceActionPreflightFailure", () => { + it.each(["start", "restart"] as const)( + "blocks %s when a legacy credential file exists", + async (action) => { + await withIsolatedLifecycleState(async ({ agentDir }) => { + await fs.writeFile(path.join(agentDir, "auth-profiles.json"), "{}\n"); + + await expect(getServiceActionPreflightFailure(action)).resolves.toEqual({ + message: + "Auth profile store ~/state/agents/main/agent/openclaw-agent.sqlite requires legacy credential migration.", + hints: ["Run `openclaw doctor --fix`, then retry this command."], + }); + }); + }, + ); + + it.each(["stop", "uninstall"] as const)( + "allows %s with the same pending migration", + async (action) => { + await withIsolatedLifecycleState(async ({ agentDir }) => { + await fs.writeFile(path.join(agentDir, "auth-profiles.json"), "{}\n"); + + await expect(getServiceActionPreflightFailure(action)).resolves.toBeNull(); + }); + }, + ); + + it.each(["start", "restart"] as const)( + "allows %s when no legacy credential files exist", + async (action) => { + await withIsolatedLifecycleState(async () => { + await expect(getServiceActionPreflightFailure(action)).resolves.toBeNull(); + }); + }, + ); +}); diff --git a/src/cli/daemon-cli/lifecycle-action-preflight.ts b/src/cli/daemon-cli/lifecycle-action-preflight.ts new file mode 100644 index 000000000000..5d6fdc26b637 --- /dev/null +++ b/src/cli/daemon-cli/lifecycle-action-preflight.ts @@ -0,0 +1,96 @@ +import { + AUTH_PROFILE_MIGRATION_COMMAND, + listAuthProfileStoresRequiringMigration, +} from "../../agents/auth-profiles/legacy-source-diagnostic.js"; +import { readConfigFileSnapshot } from "../../config/config.js"; +import { resolveFutureConfigActionBlock } from "../../config/future-version-guard.js"; +import { formatConfigIssueLines } from "../../config/issue-format.js"; +import { isPluginPackagingRuntimeOutputInvalidConfigSnapshot } from "../../config/recovery-policy.js"; +import type { ConfigFileSnapshot } from "../../config/types.openclaw.js"; +import { collectCandidateAgentDirs } from "../../secrets/runtime-fast-path.js"; +import { formatPluginPackagingRuntimeOutputRecoveryHint } from "../config-recovery-hints.js"; + +/** Service lifecycle actions; only start/restart bring the gateway up. */ +type DaemonServiceAction = "start" | "restart" | "stop" | "uninstall"; + +type ServiceActionPreflightFailure = { + message: string; + hints?: string[]; +}; + +const ACTION_PROSE: Record = { + start: "start the gateway service", + restart: "restart the gateway service", + stop: "stop the gateway service", + uninstall: "uninstall the gateway service", +}; + +const GATEWAY_LAUNCHING_ACTIONS = new Set(["start", "restart"]); + +function formatPluginPackagingRuntimeOutputRecoveryHints(): string[] { + return formatPluginPackagingRuntimeOutputRecoveryHint().split("\n"); +} + +/** + * Retired credential files make the gateway throw AuthProfileMigrationRequiredError during boot. + * Blocking here keeps the running service up instead of taking it down into a known-fatal state. + * Only launching actions are gated; stop/uninstall never read the auth store. + */ +function resolveAuthProfileMigrationBlock( + action: DaemonServiceAction, + snapshot: ConfigFileSnapshot, +): ServiceActionPreflightFailure | null { + if (!GATEWAY_LAUNCHING_ACTIONS.has(action) || !snapshot.valid) { + return null; + } + let stores: string[]; + try { + stores = listAuthProfileStoresRequiringMigration({ + agentDirs: collectCandidateAgentDirs(snapshot.runtimeConfig, process.env), + env: process.env, + }); + } catch { + // A preflight must never be the reason a lifecycle command fails. + return null; + } + if (stores.length === 0) { + return null; + } + return { + message: + stores.length === 1 + ? `Auth profile store ${stores[0]} requires legacy credential migration.` + : `Auth profile stores ${stores.join(", ")} require legacy credential migration.`, + hints: [`Run \`${AUTH_PROFILE_MIGRATION_COMMAND}\`, then retry this command.`], + }; +} + +/** Best-effort validation before a service action mutates runtime state. */ +export async function getServiceActionPreflightFailure( + action: DaemonServiceAction, +): Promise { + let snapshot: ConfigFileSnapshot; + try { + snapshot = await readConfigFileSnapshot(); + if (snapshot.exists && !snapshot.valid) { + const message = + snapshot.issues.length > 0 + ? formatConfigIssueLines(snapshot.issues, "", { normalizeRoot: true }).join("\n") + : "Unknown validation issue."; + return { + message, + ...(isPluginPackagingRuntimeOutputInvalidConfigSnapshot(snapshot) + ? { hints: formatPluginPackagingRuntimeOutputRecoveryHints() } + : {}), + }; + } + } catch { + return null; + } + + const futureBlock = resolveFutureConfigActionBlock({ action: ACTION_PROSE[action], snapshot }); + if (futureBlock) { + return { message: futureBlock.message, hints: futureBlock.hints }; + } + return resolveAuthProfileMigrationBlock(action, snapshot); +} diff --git a/src/cli/daemon-cli/lifecycle-config-preflight.ts b/src/cli/daemon-cli/lifecycle-config-preflight.ts deleted file mode 100644 index e95daf3b9ac6..000000000000 --- a/src/cli/daemon-cli/lifecycle-config-preflight.ts +++ /dev/null @@ -1,46 +0,0 @@ -import { readConfigFileSnapshot } from "../../config/config.js"; -import { resolveFutureConfigActionBlock } from "../../config/future-version-guard.js"; -import { formatConfigIssueLines } from "../../config/issue-format.js"; -import { isPluginPackagingRuntimeOutputInvalidConfigSnapshot } from "../../config/recovery-policy.js"; -import { formatPluginPackagingRuntimeOutputRecoveryHint } from "../config-recovery-hints.js"; - -type ConfigActionPreflightFailure = { - message: string; - hints?: string[]; -}; - -function formatPluginPackagingRuntimeOutputRecoveryHints(): string[] { - return formatPluginPackagingRuntimeOutputRecoveryHint().split("\n"); -} - -/** Best-effort validation before a service action mutates runtime state. */ -export async function getConfigActionPreflightFailure( - action: string, -): Promise { - let snapshot: Awaited>; - try { - snapshot = await readConfigFileSnapshot(); - if (snapshot.exists && !snapshot.valid) { - const message = - snapshot.issues.length > 0 - ? formatConfigIssueLines(snapshot.issues, "", { normalizeRoot: true }).join("\n") - : "Unknown validation issue."; - return { - message, - ...(isPluginPackagingRuntimeOutputInvalidConfigSnapshot(snapshot) - ? { hints: formatPluginPackagingRuntimeOutputRecoveryHints() } - : {}), - }; - } - } catch { - return null; - } - - const futureBlock = resolveFutureConfigActionBlock({ action, snapshot }); - return futureBlock - ? { - message: futureBlock.message, - hints: futureBlock.hints, - } - : null; -} diff --git a/src/cli/daemon-cli/lifecycle-core.ts b/src/cli/daemon-cli/lifecycle-core.ts index c559fba547c2..2d8cb9e81679 100644 --- a/src/cli/daemon-cli/lifecycle-core.ts +++ b/src/cli/daemon-cli/lifecycle-core.ts @@ -24,11 +24,11 @@ import { defaultRuntime } from "../../runtime.js"; import { formatCliCommand } from "../command-format.js"; import { formatInvalidConfigRecoveryHint } from "../config-recovery-hints.js"; import { resolveGatewayTokenForDriftCheck } from "./gateway-token-drift.js"; +import { getServiceActionPreflightFailure } from "./lifecycle-action-preflight.js"; import { appendServiceLifecycleRepairAudit, createServiceLifecycleMutationAudit, } from "./lifecycle-audit.js"; -import { getConfigActionPreflightFailure } from "./lifecycle-config-preflight.js"; import { buildDaemonServiceSnapshot, createDaemonActionContext, @@ -150,7 +150,7 @@ export async function runServiceUninstall(params: { } { - const preflight = await getConfigActionPreflightFailure("uninstall the gateway service"); + const preflight = await getServiceActionPreflightFailure("uninstall"); if (preflight) { fail(`${params.serviceNoun} uninstall blocked: ${preflight.message}`, preflight.hints); return; @@ -218,7 +218,7 @@ export async function runServiceStart(params: { // Pre-flight config validation (#35862) — run for both loaded and not-loaded // to prevent launching from invalid config in any start path. { - const preflight = await getConfigActionPreflightFailure("start the gateway service"); + const preflight = await getServiceActionPreflightFailure("start"); if (preflight) { fail( preflight.hints @@ -371,7 +371,7 @@ export async function runServiceStop(params: { return; } { - const preflight = await getConfigActionPreflightFailure("stop the gateway service"); + const preflight = await getServiceActionPreflightFailure("stop"); if (preflight) { fail(`${params.serviceNoun} stop blocked: ${preflight.message}`, preflight.hints); return; @@ -521,7 +521,7 @@ export async function runServiceRestart(params: { // Pre-flight config validation: check before any restart action (including // onNotLoaded which may send SIGUSR1 to an unmanaged process). (#35862) { - const preflight = await getConfigActionPreflightFailure("restart the gateway service"); + const preflight = await getServiceActionPreflightFailure("restart"); if (preflight) { fail( preflight.hints