mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-26 20:35:39 -06:00
fix(agents): report copied auth profiles accurately (#122162)
This commit is contained in:
Peter Steinberger
committed by
GitHub
GitHub
parent
80435aa0de
commit
0855e491de
@@ -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,
|
||||
|
||||
+60
-135
@@ -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", () => {
|
||||
|
||||
@@ -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<typeof applyAgentBindings>[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,
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user