diff --git a/src/agents/auth-profiles/portability.test.ts b/src/agents/auth-profiles/portability.test.ts index 85623170073d..2efa4d31939f 100644 --- a/src/agents/auth-profiles/portability.test.ts +++ b/src/agents/auth-profiles/portability.test.ts @@ -33,6 +33,12 @@ describe("auth profile portability", () => { expires: Date.now() + 60_000, }, }, + order: { + openai: ["openai:default", "openai:api-key"], + "github-copilot": ["github-copilot:default"], + }, + lastGood: { openai: "openai:api-key" }, + usageStats: { "openai:api-key": { lastUsed: 1_000 } }, }; const portable = buildPortableAuthProfileStoreForAgentCopy(store); @@ -43,6 +49,12 @@ describe("auth profile portability", () => { "openai:api-key": store.profiles["openai:api-key"], "github-copilot:default": store.profiles["github-copilot:default"], }); + expect(portable.store.order).toEqual({ + openai: ["openai:api-key"], + "github-copilot": ["github-copilot:default"], + }); + expect(portable.store.lastGood).toBeUndefined(); + expect(portable.store.usageStats).toBeUndefined(); }); it("allows provider-owned OAuth profiles to opt in explicitly", () => { @@ -67,7 +79,12 @@ describe("auth profile portability", () => { provider: "openai", expires: Date.now() + 60_000, copyToAgents: true, - } as AuthProfileCredential; + oauthRef: { + source: "openclaw-credentials", + provider: "openai", + id: "0123456789abcdef0123456789abcdef", + }, + } as unknown as AuthProfileCredential; expect(resolveAuthProfilePortability(credential)).toEqual({ portable: false, diff --git a/src/commands/agents.add.test.ts b/src/commands/agents.add.test.ts index f11ff3008983..ed39ec7965f3 100644 --- a/src/commands/agents.add.test.ts +++ b/src/commands/agents.add.test.ts @@ -1,10 +1,11 @@ // Agents add tests cover agent creation, workspace setup, channel binding, and onboarding integration. import fs from "node:fs/promises"; import path from "node:path"; +import { DatabaseSync } from "node:sqlite"; import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { AUTH_STORE_VERSION } from "../agents/auth-profiles/constants.js"; -import { resolveAuthProfileOrder } from "../agents/auth-profiles/order.js"; import { loadPersistedAuthProfileStore } from "../agents/auth-profiles/persisted.js"; +import { resolveAuthProfileDatabasePath } from "../agents/auth-profiles/sqlite.js"; import { saveAuthProfileStore } from "../agents/auth-profiles/store.js"; import { formatCliCommand } from "../cli/command-format.js"; import { closeOpenClawAgentDatabasesForTest } from "../state/openclaw-agent-db.js"; @@ -115,7 +116,7 @@ vi.mock("./onboard-helpers.js", () => ({ })); import { WizardCancelledError } from "../wizard/prompts.js"; -import { agentsAddCommand, testing } from "./agents.commands.add.js"; +import { agentsAddCommand } from "./agents.commands.add.js"; const runtime = createTestRuntime(); const RESERVED_SYSTEM_AGENT_IDS_FOR_TEST = ["openclaw", "crestodian"] as const; // reserved ids @@ -304,166 +305,90 @@ describe("agents add command", () => { ); }); - it("copies only portable auth profiles when seeding a new agent store", async () => { + it("reports only auth profiles persisted to the new agent store", async () => { await withAgentsAddStateRoot("openclaw-agents-add-auth-copy-", async (root) => { - const sourceAgentDir = path.join(root, "main", "agent"); - const destAgentDir = path.join(root, "work", "agent"); + const sourceAgentDir = path.join(root, "agents", "main", "agent"); + const destAgentDir = path.join(root, "agents", "work", "agent"); + const workspaceDir = path.join(root, "workspace-work"); await fs.mkdir(sourceAgentDir, { recursive: true }); saveAuthProfileStore( { version: AUTH_STORE_VERSION, profiles: { - "openai:default": { + "openai:api-key": { type: "api_key", provider: "openai", key: "sk-test", }, - "openai:backup": { - type: "api_key", - provider: "openai", - key: "sk-backup", - }, - "github-copilot:default": { - type: "token", - provider: "github-copilot", - token: "gho-test", - }, - "openai:oauth": { - type: "oauth", - provider: "openai", - access: "codex-access", - refresh: "codex-refresh", - expires: Date.now() + 60_000, - }, - }, - order: { - openai: ["openai:oauth", "openai:backup", "openai:default"], - "github-copilot": ["github-copilot:default"], - }, - lastGood: { openai: "openai:default" }, - usageStats: { "openai:default": { lastUsed: 1_000 } }, - }, - sourceAgentDir, - ); - - const result = await testing.copyPortableAuthProfiles({ - sourceAgentDir, - destAgentDir, - }); - - expect(result).toEqual({ copied: 3, skipped: 1 }); - const copied = loadPersistedAuthProfileStore(destAgentDir); - expect(Object.keys(copied?.profiles ?? {}).toSorted()).toEqual([ - "github-copilot:default", - "openai:backup", - "openai:default", - ]); - expect(copied?.order).toEqual({ - openai: ["openai:backup", "openai:default"], - "github-copilot": ["github-copilot:default"], - }); - expect(copied?.lastGood).toBeUndefined(); - expect(copied?.usageStats).toBeUndefined(); - expect(resolveAuthProfileOrder({ store: copied!, provider: "openai" })).toEqual([ - "openai:backup", - "openai:default", - ]); - }); - }); - - it("copies portable Codex OAuth profiles inline", async () => { - await withAgentsAddStateRoot("openclaw-agents-add-oauth-copy-", async (root) => { - const sourceAgentDir = path.join(root, "main", "agent"); - const destAgentDir = path.join(root, "work", "agent"); - const expires = Date.now() + 60_000; - await fs.mkdir(sourceAgentDir, { recursive: true }); - saveAuthProfileStore( - { - version: AUTH_STORE_VERSION, - profiles: { "openai:oauth": { type: "oauth", provider: "openai", access: "codex-copy-access-token", refresh: "codex-copy-refresh-token", - expires, + expires: Date.now() + 60_000, copyToAgents: true, }, }, }, sourceAgentDir, ); - - const result = await testing.copyPortableAuthProfiles({ - sourceAgentDir, - destAgentDir, + readConfigFileSnapshotMock.mockResolvedValue({ + ...baseConfigSnapshot, + config: { agents: { list: [{ id: "main", default: true }] } }, + sourceConfig: { agents: { list: [{ id: "main", default: true }] } }, }); - - expect(result).toEqual({ copied: 1, skipped: 0 }); - const copied = loadPersistedAuthProfileStore(destAgentDir); - const credential = copied?.profiles["openai:oauth"]; - expect(credential).toStrictEqual({ - type: "oauth", - provider: "openai", - access: "codex-copy-access-token", - refresh: "codex-copy-refresh-token", - expires, - copyToAgents: true, - }); - }); - }); - - it("skips unresolved OAuth profiles when seeding a new agent store", async () => { - await withAgentsAddStateRoot("openclaw-agents-add-oauth-ref-skip-", async (root) => { - const sourceAgentDir = path.join(root, "main", "agent"); - const destAgentDir = path.join(root, "work", "agent"); - const profileId = "openai:oauth"; - const ref = { - source: "openclaw-credentials" as const, - provider: "openai" as const, - id: "0123456789abcdef0123456789abcdef", + const prompter = { + intro: vi.fn(), + text: vi.fn().mockResolvedValueOnce("work").mockResolvedValueOnce(workspaceDir), + confirm: vi.fn().mockResolvedValueOnce(true).mockResolvedValueOnce(false), + note: vi.fn(), + outro: vi.fn(), }; - await fs.mkdir(sourceAgentDir, { recursive: true }); - saveAuthProfileStore( - { - version: AUTH_STORE_VERSION, - profiles: { - [profileId]: { - type: "oauth", - provider: "openai", - copyToAgents: true, - expires: Date.now() + 60_000, - oauthRef: ref, - }, - }, - } as never, - sourceAgentDir, - ); - const result = await testing.copyPortableAuthProfiles({ - sourceAgentDir, - destAgentDir, - }); + wizardMocks.createClackPrompter.mockReturnValue(prompter); - expect(result).toEqual({ copied: 0, skipped: 1 }); - expect(loadPersistedAuthProfileStore(destAgentDir)).toBeNull(); + await agentsAddCommand({}, runtime); + + expect(Object.keys(loadPersistedAuthProfileStore(destAgentDir)?.profiles ?? {})).toEqual([ + "openai:api-key", + ]); + expect(prompter.note).toHaveBeenCalledWith( + 'Copied 1 portable auth profile from "main". OAuth profiles stay shared from "main" unless this agent signs in separately.', + "Auth profiles", + ); }); }); - it("does not claim skipped OAuth profiles stay shared from a non-main source agent", () => { - expect( - testing.formatSkippedOAuthProfilesMessage({ - sourceAgentId: "default-work", - sourceIsInheritedMain: false, - }), - ).toBe( - 'OAuth profiles were not copied from "default-work"; sign in separately for this agent.', - ); - expect( - testing.formatSkippedOAuthProfilesMessage({ - sourceAgentId: "main", - sourceIsInheritedMain: true, - }), - ).toBe('OAuth profiles stay shared from "main" unless this agent signs in separately.'); + it("fails before config mutation when the source auth store is unreadable", async () => { + await withAgentsAddStateRoot("openclaw-agents-add-auth-unreadable-", async (root) => { + const sourceAgentDir = path.join(root, "agents", "main", "agent"); + const workspaceDir = path.join(root, "workspace-work"); + await fs.mkdir(sourceAgentDir, { recursive: true }); + const database = new DatabaseSync(resolveAuthProfileDatabasePath(sourceAgentDir)); + database.exec( + "CREATE VIEW auth_profile_store AS SELECT 'primary' AS store_key, '{}' AS store_json;", + ); + database.close(); + readConfigFileSnapshotMock.mockResolvedValue({ + ...baseConfigSnapshot, + config: { agents: { list: [{ id: "main", default: true }] } }, + sourceConfig: { agents: { list: [{ id: "main", default: true }] } }, + }); + const prompter = { + intro: vi.fn(), + text: vi.fn().mockResolvedValueOnce("work").mockResolvedValueOnce(workspaceDir), + confirm: vi.fn().mockResolvedValue(false), + note: vi.fn(), + outro: vi.fn(), + }; + wizardMocks.createClackPrompter.mockReturnValue(prompter); + + await expect(agentsAddCommand({}, runtime)).rejects.toThrow( + /auth profile store .* is unreadable; run .*doctor --fix/i, + ); + + expect(writeConfigFileMock).not.toHaveBeenCalled(); + expect(prompter.outro).not.toHaveBeenCalled(); + }); }); describe("non-interactive config mutation", () => { diff --git a/src/commands/agents.commands.add.ts b/src/commands/agents.commands.add.ts index c41ba09a8ee6..2450a807dd89 100644 --- a/src/commands/agents.commands.add.ts +++ b/src/commands/agents.commands.add.ts @@ -14,9 +14,14 @@ import { import { buildPortableAuthProfileStoreForAgentCopy, ensureAuthProfileStore, + type AuthProfileStore, } from "../agents/auth-profiles.js"; +import { AuthProfileStoreUnreadableError } from "../agents/auth-profiles/legacy-source-diagnostic.js"; import { loadPersistedAuthProfileStore } from "../agents/auth-profiles/persisted.js"; -import { resolveAuthProfileDatabasePath } from "../agents/auth-profiles/sqlite.js"; +import { + inspectPersistedAuthProfileStoreRaw, + resolveAuthProfileDatabasePath, +} from "../agents/auth-profiles/sqlite.js"; import { saveAuthProfileStore } from "../agents/auth-profiles/store.js"; import { formatCliCommand } from "../cli/command-format.js"; import { logConfigUpdated } from "../config/logging.js"; @@ -60,36 +65,25 @@ function emptyBindingResult(config: Parameters[0]): A return { config, added: [], updated: [], skipped: [], conflicts: [] }; } -async function copyPortableAuthProfiles(params: { - destAgentDir: string; - sourceAgentDir: string; -}): Promise<{ copied: number; skipped: number }> { - const sourceStore = loadPersistedAuthProfileStore(params.sourceAgentDir); - if (!sourceStore || Object.keys(sourceStore.profiles).length === 0) { - return { copied: 0, skipped: 0 }; +function loadReadablePersistedAuthProfileStore(agentDir: string): AuthProfileStore | null { + const store = loadPersistedAuthProfileStore(agentDir); + if (!store && inspectPersistedAuthProfileStoreRaw(agentDir).status !== "missing") { + throw new AuthProfileStoreUnreadableError(agentDir); } - const portable = buildPortableAuthProfileStoreForAgentCopy(sourceStore); - if (portable.copiedProfileIds.length === 0) { - return { copied: 0, skipped: portable.skippedProfileIds.length }; - } - await fs.mkdir(params.destAgentDir, { recursive: true }); - saveAuthProfileStore(portable.store, params.destAgentDir, { - filterExternalAuthProfiles: false, - syncExternalCli: false, - }); - return { - copied: portable.copiedProfileIds.length, - skipped: portable.skippedProfileIds.length, - }; + return store; } -function formatSkippedOAuthProfilesMessage(params: { - sourceAgentId: string; - sourceIsInheritedMain: boolean; -}): string { - return params.sourceIsInheritedMain - ? `OAuth profiles stay shared from "${params.sourceAgentId}" unless this agent signs in separately.` - : `OAuth profiles were not copied from "${params.sourceAgentId}"; sign in separately for this agent.`; +function hasOAuthProfiles(store: AuthProfileStore, profileIds: readonly string[]): boolean { + return profileIds.some((profileId) => store.profiles[profileId]?.type === "oauth"); +} + +function formatSkippedOAuthProfilesMessage( + sourceAgentId: string, + sourceIsInheritedMain: boolean, +): string { + return sourceIsInheritedMain + ? `OAuth profiles stay shared from "${sourceAgentId}" unless this agent signs in separately.` + : `OAuth profiles were not copied from "${sourceAgentId}"; sign in separately for this agent.`; } /** Create or update an agent through the non-interactive path or guided wizard. */ @@ -270,12 +264,13 @@ export async function agentsAddCommand( normalizeLowercaseStringOrEmpty(path.resolve(sourceAuthPath)) === normalizeLowercaseStringOrEmpty(path.resolve(mainAuthPath)); if (!sameAuthPath) { - const sourceStore = loadPersistedAuthProfileStore(sourceAgentDir); - const destStore = loadPersistedAuthProfileStore(agentDir); + const sourceStore = loadReadablePersistedAuthProfileStore(sourceAgentDir); + const destStore = loadReadablePersistedAuthProfileStore(agentDir); const portable = sourceStore ? buildPortableAuthProfileStoreForAgentCopy(sourceStore) : undefined; if ( + sourceStore && portable && portable.copiedProfileIds.length > 0 && Object.keys(destStore?.profiles ?? {}).length === 0 @@ -290,24 +285,33 @@ export async function agentsAddCommand( filterExternalAuthProfiles: false, syncExternalCli: false, }); - const skippedText = - portable.skippedProfileIds.length > 0 - ? ` ${formatSkippedOAuthProfilesMessage({ - sourceAgentId: defaultAgentId, - sourceIsInheritedMain, - })}` + const persistedDestStore = loadPersistedAuthProfileStore(agentDir); + const copiedCount = portable.copiedProfileIds.filter( + (profileId) => persistedDestStore?.profiles[profileId] !== undefined, + ).length; + const skippedOAuthProfiles = + hasOAuthProfiles(sourceStore, portable.skippedProfileIds) || + portable.copiedProfileIds.some( + (profileId) => + sourceStore.profiles[profileId]?.type === "oauth" && + persistedDestStore?.profiles[profileId] === undefined, + ); + const copiedText = + copiedCount > 0 + ? `Copied ${copiedCount} portable auth profile${copiedCount === 1 ? "" : "s"} from "${defaultAgentId}".` : ""; - await prompter.note( - `Copied ${portable.copiedProfileIds.length} portable auth profile${portable.copiedProfileIds.length === 1 ? "" : "s"} from "${defaultAgentId}".${skippedText}`, - "Auth profiles", - ); + const skippedText = skippedOAuthProfiles + ? ` ${formatSkippedOAuthProfilesMessage(defaultAgentId, sourceIsInheritedMain)}` + : ""; + await prompter.note(`${copiedText}${skippedText}`.trim(), "Auth profiles"); } - } else if ((portable?.skippedProfileIds.length ?? 0) > 0) { + } else if ( + sourceStore && + portable && + hasOAuthProfiles(sourceStore, portable.skippedProfileIds) + ) { await prompter.note( - formatSkippedOAuthProfilesMessage({ - sourceAgentId: defaultAgentId, - sourceIsInheritedMain, - }), + formatSkippedOAuthProfilesMessage(defaultAgentId, sourceIsInheritedMain), "Auth profiles", ); } @@ -440,8 +444,3 @@ export async function agentsAddCommand( throw err; } } - -export const testing = { - copyPortableAuthProfiles, - formatSkippedOAuthProfilesMessage, -};