diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index 05c5dc2e70..6c29f6657a 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -10,6 +10,9 @@ ### Fixed +- Adding a second account to a provider whose stored credential predates credential pools (a flat entry with no `accounts` array, which is what `openai-codex` OAuth login writes) no longer overwrites the first one. `appendLoginSlot` now promotes that legacy credential into the pool as the `default` slot and stores the new login beside it as `login-2`, so both accounts remain usable and the flat top-level fields still authenticate a build predating pools. First login (no stored credential) still writes the flat credential as-is, and a provider that returns its own populated `accounts` array is still written through untouched. +- Removing the account whose material the flat top-level credential fields projected no longer leaves the pool authenticating as the deleted account. `removeSlot` now re-projects those fields from the first surviving slot, so a pool left with a single account (which does not enter credential rotation and therefore resolves through the flat projection) immediately uses the account that remains. `accounts` is kept, removing a non-projected slot still leaves the flat fields untouched, and removing the last slot still drops the credential entirely. + ### Removed ## [2026.9.3-2] - 2026-09-03 diff --git a/packages/ai/src/auth/pool/slots.ts b/packages/ai/src/auth/pool/slots.ts index 32e07280c2..d1bd00567f 100644 --- a/packages/ai/src/auth/pool/slots.ts +++ b/packages/ai/src/auth/pool/slots.ts @@ -82,16 +82,42 @@ export function upsertSlot(credential: PooledCredential | undefined, slot: Crede return { ...base, accounts }; } +/** Whether the flat top-level fields are this slot's material rather than a sibling's. */ +function slotMirrorsFlat(credential: PooledCredential, slot: CredentialSlot): boolean { + if (credential.type === "oauth") return slot.access === credential.access || slot.refresh === credential.refresh; + return slot.key === credential.key; +} + +/** Rewrites the flat top-level projection to carry the given slot's material. */ +function projectFlatFields(credential: PooledCredential, slot: CredentialSlot): PooledCredential { + if (credential.type === "oauth") { + if (slot.access === undefined || slot.refresh === undefined || slot.expires === undefined) return credential; + return { ...credential, access: slot.access, refresh: slot.refresh, expires: slot.expires }; + } + return { ...credential, key: slot.key }; +} + /** * Removes one slot. The credential is dropped entirely once its last slot is gone, * and a pin naming the removed slot is cleared so selection never points at a slot * that no longer exists. + * + * When the removed slot was the one the flat top-level fields projected, those + * fields are re-projected from the first survivor. Without that the pool keeps + * authenticating with the deleted account's material: a credential with a single + * remaining slot does not enter the rotation path, so ordinary requests resolve + * the flat projection and would keep using exactly the account the user removed. + * `accounts` is preserved either way, so the entry stays a pool. */ export function removeSlot(credential: PooledCredential | undefined, name: string): PooledCredential | undefined { if (!credential) return undefined; - const accounts = listSlots(credential).filter((slot) => slot.name !== name); + const existing = listSlots(credential); + const removed = existing.find((slot) => slot.name === name); + const accounts = existing.filter((slot) => slot.name !== name); if (accounts.length === 0) return undefined; - const next: PooledCredential = { ...credential, accounts }; + const reprojected = + removed && slotMirrorsFlat(credential, removed) ? projectFlatFields(credential, accounts[0]) : credential; + const next: PooledCredential = { ...reprojected, accounts }; if (next.pinned === name) delete next.pinned; return next; } @@ -141,9 +167,10 @@ function nextLoginSlotName(credential: PooledCredential): string { } /** - * Appends an unnamed flat credential to a pool as a generated `login-N` slot. A - * flat or absent current entry keeps today's whole-write shape so no existing - * user's stored bytes change until a second credential actually exists. + * Appends an unnamed flat credential to a pool as a generated `login-N` slot. + * An absent current entry keeps today's whole-write shape; a flat current entry + * is promoted to a pool so the legacy credential stays reachable as `default` + * instead of being overwritten by the second login. * * A login result that already carries its own populated `accounts` array is a * provider-owned pool: it IS the complete post-login credential, so it is @@ -154,7 +181,7 @@ export function appendLoginSlot(current: PooledCredential | undefined, flat: Cre if ("accounts" in flat && Array.isArray(flat.accounts) && flat.accounts.length > 0) { return flat; } - if (!current || !Array.isArray(current.accounts) || current.accounts.length === 0) { + if (!current) { return flat; } return upsertSlot(current, slotFromFlatCredentialNamed(flat, nextLoginSlotName(current))); diff --git a/packages/ai/src/changes.md b/packages/ai/src/changes.md index 79cce9714a..08649e75fa 100644 --- a/packages/ai/src/changes.md +++ b/packages/ai/src/changes.md @@ -1,3 +1,51 @@ +## A legacy flat credential is promoted, not overwritten, by a second login (2026-09-03) + +### What changed + +- `packages/ai/src/auth/pool/slots.ts`: `appendLoginSlot` whole-writes the login result only when there is no stored + credential at all (`if (!current)`), instead of also whole-writing whenever the stored credential is flat. A flat + `current` now takes the `upsertSlot` path, so `listSlots` synthesizes its `default` slot from the flat fields and the + fresh login is appended as the next generated `login-N`. The provider-owned pool guard added for senpi#1279 keeps its + place ahead of both branches and is unchanged, as is the pooled-`current` append. +- `packages/ai/src/auth/pool/slots.ts`: `removeSlot` re-projects the flat top-level fields from the first surviving slot + when the removed slot was the one those fields mirrored (matched by `access`/`refresh` for OAuth, by `key` for an API + key). Removing a slot whose material the flat fields never carried still leaves them byte-identical, the last-slot + removal still returns `undefined`, and a pin naming the removed slot is still cleared. `accounts` is preserved in every + surviving case, so a one-slot pool stays a pool rather than collapsing to a bare flat credential; only its projection + moves to the survivor. + +### Why + +- `openai-codex` OAuth `login` returns a plain flat `OAuthCredential` with no `accounts` array, so the #1279 guard never + fires for it and the old flat-current disjunct did. A second `/login openai-codex` (or the coding-agent `AuthStorage.set` + RPC path) therefore replaced the first account's tokens outright: the user lost the credential they were already using + and the pool they were trying to build never came into existence (senpi LAB-109). Promotion is the same transition + `setSlot` already performs, and `upsertSlot` keeps the pre-existing flat fields as the top-level projection, so a build + that ignores `accounts` still authenticates with exactly the bytes it authenticated with before. +- Promotion alone made removal unsafe. After promotion the flat fields are the legacy `default`'s material, so removing + `default` used to leave the pool listing only `login-2` while the flat projection still held the deleted account's + tokens. That is not cosmetic: `mightHoldCredentialPool` in the coding-agent model runtime only routes through + credential rotation when `accounts.length > 1`, so a pool with one slot left resolves through `resolveProviderAuth`'s + flat branch and kept authenticating as exactly the account the user had just removed, with no way to pin around it. + Re-projecting from the survivor makes the remaining account the effective credential the moment the removal lands. +- This supersedes the sentence in the 2026-09-03 senpi#1279 entry below that says a flat `current` still stores the flat + credential as-is; that branch is what this pass changes. Every other branch it describes is still accurate. + +### Why an extension could not handle it + +- `appendLoginSlot` is the shared write step inside `ModelsImpl.login` and the coding-agent auth storage `set`, running + after the provider's `login` resolves and before the credential is persisted. No provider or extension seam exists + between producing the credential and the write that was discarding the previous account. +- `removeSlot` is the shared slot algebra behind `ModelsImpl.logout({ slotId })`, `AuthStorage.removeSlot` and + `removeCredentialAccount`. Every removal caller reaches the flat projection only through it, so nothing above it can + keep the projection and the surviving slot in agreement. + +### Expected merge conflict zones + +- LOW: the second condition of `appendLoginSlot` and its JSDoc in `auth/pool/slots.ts`, immediately below the senpi#1279 + guard that the open PRs #1304 and #1196 also touch. +- LOW: the `removeSlot` body and the two projection helpers added directly above it in `auth/pool/slots.ts`. + ## OAuth prompt types carry the provider's cancellation signal (2026-09-03) ### What changed diff --git a/packages/ai/test/credential-pool-mutations.test.ts b/packages/ai/test/credential-pool-mutations.test.ts index 449325d6a2..ec3be44243 100644 --- a/packages/ai/test/credential-pool-mutations.test.ts +++ b/packages/ai/test/credential-pool-mutations.test.ts @@ -1,4 +1,6 @@ import { describe, expect, test } from "vitest"; +import { InMemoryCredentialStore } from "../src/auth/credential-store.ts"; +import { envApiKeyAuth } from "../src/auth/helpers.ts"; import { appendLoginSlot, type Credential, @@ -8,6 +10,8 @@ import { removeSlot, upsertSlot, } from "../src/auth/pool/slots.ts"; +import { resolveProviderAuth } from "../src/auth/resolve.ts"; +import { createProvider, type Provider } from "../src/models.ts"; describe("credential pool slot algebra", () => { function pooledApiKey(): PooledCredential { @@ -60,6 +64,24 @@ describe("credential pool slot algebra", () => { expect(names(next)).toEqual(["default", "second"]); }); + test("appendLoginSlot promotes a legacy flat credential before adding a login", () => { + const current: Credential = { type: "oauth", access: "first-access", refresh: "first-refresh", expires: 1 }; + const next = appendLoginSlot(current, { + type: "oauth", + access: "second-access", + refresh: "second-refresh", + expires: 2, + }); + + expect(next).toMatchObject({ type: "oauth", access: "first-access", refresh: "first-refresh", expires: 1 }); + expect(names(next)).toEqual(["default", "login-2"]); + expect(listSlots(next).find((slot) => slot.name === "login-2")).toMatchObject({ + access: "second-access", + refresh: "second-refresh", + expires: 2, + }); + }); + test("removeSlot deletes only the named slot", () => { expect(names(removeSlot(pooledApiKey(), "default"))).toEqual(["work"]); }); @@ -77,6 +99,49 @@ describe("credential pool slot algebra", () => { expect(removeSlot(single, "default")).toBeUndefined(); }); + test("removeSlot re-projects the flat fields from the survivor when the projected slot is removed", () => { + const promoted = appendLoginSlot( + { type: "oauth", access: "legacy-access", refresh: "legacy-refresh", expires: 1 }, + { type: "oauth", access: "second-access", refresh: "second-refresh", expires: 2 }, + ) as PooledCredential; + + const next = removeSlot(promoted, "default"); + + expect(names(next)).toEqual(["login-2"]); + expect(next).toMatchObject({ + type: "oauth", + access: "second-access", + refresh: "second-refresh", + expires: 2, + }); + }); + + test("removeSlot leaves the flat projection alone when a non-projected slot is removed", () => { + const promoted = appendLoginSlot( + { type: "oauth", access: "legacy-access", refresh: "legacy-refresh", expires: 1 }, + { type: "oauth", access: "second-access", refresh: "second-refresh", expires: 2 }, + ) as PooledCredential; + + const next = removeSlot(promoted, "login-2"); + + expect(names(next)).toEqual(["default"]); + expect(next).toMatchObject({ + type: "oauth", + access: "legacy-access", + refresh: "legacy-refresh", + expires: 1, + }); + }); + + test("removeSlot re-projects an api_key survivor when the projected slot is removed", () => { + const promoted = appendLoginSlot({ type: "api_key", key: "legacy-key" }, { type: "api_key", key: "second-key" }); + + const next = removeSlot(promoted, "default"); + + expect(names(next)).toEqual(["login-2"]); + expect(next).toMatchObject({ type: "api_key", key: "second-key" }); + }); + test("upsertSlot rejects a slot name that could collide with path syntax", () => { const slot = { name: "../escape", key: "k", source: "login" } as CredentialSlot; expect(() => upsertSlot(pooledApiKey(), slot)).toThrow(/Invalid account name/); @@ -131,3 +196,54 @@ describe("credential pool slot algebra", () => { expect(appendLoginSlot(undefined, flat)).toBe(flat); }); }); + +describe("ordinary auth resolution after removing a promoted account", () => { + const authContext = { env: async () => undefined, fileExists: async () => false }; + const FUTURE = 4_102_444_800_000; + + const provider: Provider = createProvider({ + id: "removeoauth", + name: "Remove OAuth", + baseUrl: "https://removeoauth.example", + auth: { + apiKey: envApiKeyAuth("Remove OAuth API key", ["REMOVEOAUTH_API_KEY"]), + oauth: { + name: "Remove OAuth", + login: async () => ({ type: "oauth", access: "a", refresh: "r", expires: FUTURE }), + refresh: async (credential) => credential, + toAuth: async (credential) => ({ apiKey: credential.access }), + }, + }, + models: [], + api: "openai-responses" as never, + }); + + function promotedPool(): PooledCredential { + return appendLoginSlot( + { type: "oauth", access: "legacy-access", refresh: "legacy-refresh", expires: FUTURE }, + { type: "oauth", access: "second-access", refresh: "second-refresh", expires: FUTURE }, + ) as PooledCredential; + } + + test("removing the promoted default makes login-2 the credential ordinary requests use", async () => { + const store = new InMemoryCredentialStore(); + const remaining = removeSlot(promotedPool(), "default"); + if (!remaining) throw new Error("removeSlot dropped a pool that still had a slot"); + await store.modify("removeoauth", async () => remaining); + + const resolved = await resolveProviderAuth(provider, store, authContext); + + expect(resolved?.auth.apiKey).toBe("second-access"); + }); + + test("removing login-2 keeps the promoted default the credential ordinary requests use", async () => { + const store = new InMemoryCredentialStore(); + const remaining = removeSlot(promotedPool(), "login-2"); + if (!remaining) throw new Error("removeSlot dropped a pool that still had a slot"); + await store.modify("removeoauth", async () => remaining); + + const resolved = await resolveProviderAuth(provider, store, authContext); + + expect(resolved?.auth.apiKey).toBe("legacy-access"); + }); +}); diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index eb56644550..63185f6e7f 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,10 +4,13 @@ ### Added +- `/gpt-account` manages OpenAI Codex OAuth accounts the way `/claude-account` manages claude-sdk-oauth ones: `add` runs an interactive Codex login and stores the new account beside the existing ones, `remove `, `pin ` and `unpin` select which account is used, and the bare command lists every stored account with its source, availability and pin state without printing token material. + ### Changed ### Fixed +- `/gpt-account remove` (and every other account-removal path) now leaves the remaining account actually in use. Removing the account the stored credential's top-level fields projected used to keep those fields pointing at the deleted account's tokens, and a provider left with one account does not enter credential rotation, so requests kept authenticating as the removed account; the surviving account's material is now projected onto those fields. - Claude SDK OAuth classifies Fable-style "requires usage credits" failures as a non-retryable entitlement instead of a rate limit, so the account is not blocked for 60s and AgentSession can fall through to the next model ([#709](https://github.com/code-yeongyu/senpi/issues/709)). - Print mode (`-p` / `--mode json`) now writes a stderr notice when retry fallback substitutes a model, so silent wrong-model answers are visible without corrupting JSON stdout ([oh-my-openagent#7626](https://github.com/code-yeongyu/oh-my-openagent/issues/7626)). ### New Features diff --git a/packages/coding-agent/src/core/extensions/builtin/changes.md b/packages/coding-agent/src/core/extensions/builtin/changes.md index c331fdafc8..d0eb665f6e 100644 --- a/packages/coding-agent/src/core/extensions/builtin/changes.md +++ b/packages/coding-agent/src/core/extensions/builtin/changes.md @@ -1,5 +1,38 @@ # Builtin extensions changes +## OpenAI Codex OAuth account command (2026-09-03) + +### What changed + +- `gpt-account.ts` (new): `/gpt-account` is the dedicated OpenAI Codex OAuth account manager, mirroring the + `/claude-account` action set. `add` runs an interactive `openai-codex` oauth login through + `ctx.modelRegistry.modelRuntime.login` and emits `emitProviderAccountsChanged` so subscribed clients re-read the pool; + `remove `, `pin ` and `unpin` go through `credential-accounts.ts` (which emits on its own); the + no-argument form lists every stored slot as `name | source | available|blocked` with the pin marked. Only names, + sources and health are rendered, never key or token material. +- `index.ts`: registers `{ id: "gpt-account", factory: gptAccountExtension }` immediately after the provider-neutral + `account` builtin, so the Codex lane keeps its own command name the way `claude-sdk-oauth` and `cursor-cli-oauth` do. + +### Why + +- The provider-neutral `/account` command lists, pins, unpins and removes accounts for any provider but has no `add`, so + the only way to put a second `openai-codex` account into the pool was `/login openai-codex` - the shared write path + that this same pass fixes for LAB-109. Codex users need the add/remove/pin surface that claude-sdk-oauth users already + have from `/claude-account`, and keeping it in its own command leaves the Codex-specific login wiring (interactive + prompt relay, auth-url notices) out of the provider-neutral command. + +### Why an extension could not handle it + +- The command has to exist for every session, which means being present in the `builtinExtensions` registry in + `index.ts`; a user extension cannot insert itself there. It also drives `modelRuntime.login` and the coding-agent auth + storage pool directly, and that login/persist seam is core state with no extension-visible hook between producing a + credential and writing it. + +### Expected merge conflict zones + +- LOW: the import block and the `builtinExtensions` array in `index.ts`, where every new provider lane adds a line. + `gpt-account.ts` itself is new and fork-only. + ## Shared eval-only routing predicate for prompt surfaces (2026-09-03) ### What changed diff --git a/packages/coding-agent/src/core/extensions/builtin/gpt-account.ts b/packages/coding-agent/src/core/extensions/builtin/gpt-account.ts new file mode 100644 index 0000000000..54ef0c8374 --- /dev/null +++ b/packages/coding-agent/src/core/extensions/builtin/gpt-account.ts @@ -0,0 +1,121 @@ +import { + getCredentialAccounts, + pinCredentialAccount, + removeCredentialAccount, +} from "../../../core/credential-accounts.ts"; +import type { ExtensionAPI, ExtensionCommandContext } from "../types.ts"; +import { emitProviderAccountsChanged } from "./claude-sdk-oauth/account-events.ts"; + +const OPENAI_CODEX_PROVIDER_ID = "openai-codex"; +const LOGIN_CANCELLED_MESSAGE = "Login cancelled"; + +function parseArgs(rawArgs: string): string[] { + return rawArgs.trim().split(/\s+/).filter(Boolean); +} + +function usage(ctx: ExtensionCommandContext): void { + ctx.ui.notify("Usage: /gpt-account [add | remove | pin | unpin]", "error"); +} + +function authEventMessage(event: unknown): string { + if (event === null || typeof event !== "object") return "OpenAI Codex OAuth authentication update."; + const value = event as Record; + if (value.type === "auth_url" && typeof value.url === "string") { + return `Open this URL to authorize OpenAI Codex OAuth:\n${value.url}`; + } + if (value.type === "device_code" && typeof value.verificationUri === "string") { + return `Open this URL to authorize OpenAI Codex OAuth:\n${value.verificationUri}`; + } + return typeof value.message === "string" ? value.message : "OpenAI Codex OAuth authentication update."; +} + +async function showAccounts(ctx: ExtensionCommandContext): Promise { + const accounts = await getCredentialAccounts(ctx.modelRegistry.authStorage, OPENAI_CODEX_PROVIDER_ID); + const lines = ["OpenAI Codex OAuth accounts:"]; + if (accounts.length === 0) lines.push(" (none)"); + for (const account of accounts) { + const states = [account.name, account.source, account.blocked ? "blocked" : "available"]; + if (account.pinned) states.push("pinned"); + lines.push(` ${states.join(" | ")}`); + } + ctx.ui.notify(lines.join("\n"), "info"); +} + +async function addAccount(ctx: ExtensionCommandContext): Promise { + if (!ctx.hasUI) { + ctx.ui.notify("/gpt-account add requires an interactive UI.", "error"); + return; + } + try { + await ctx.modelRegistry.modelRuntime.login(OPENAI_CODEX_PROVIDER_ID, "oauth", { + signal: ctx.signal, + prompt: async (prompt) => { + const answer = await ctx.ui.input(prompt.message); + if (answer === undefined) throw new Error(LOGIN_CANCELLED_MESSAGE); + return answer; + }, + notify: (event) => ctx.ui.notify(authEventMessage(event), "info"), + }); + emitProviderAccountsChanged(OPENAI_CODEX_PROVIDER_ID); + ctx.ui.notify("OpenAI Codex OAuth account added.", "info"); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + if (message === LOGIN_CANCELLED_MESSAGE) return; + ctx.ui.notify(message, "error"); + } +} + +async function removeAccount(ctx: ExtensionCommandContext, name: string | undefined): Promise { + if (!name) { + usage(ctx); + return; + } + await removeCredentialAccount(ctx.modelRegistry.authStorage, OPENAI_CODEX_PROVIDER_ID, name); + ctx.ui.notify(`Removed OpenAI Codex OAuth account '${name}'.`, "info"); +} + +async function pinAccount(ctx: ExtensionCommandContext, name: string | undefined): Promise { + if (!name) { + usage(ctx); + return; + } + await pinCredentialAccount(ctx.modelRegistry.authStorage, OPENAI_CODEX_PROVIDER_ID, name); + ctx.ui.notify(`Pinned OpenAI Codex OAuth account '${name}'.`, "info"); +} + +export default function gptAccountExtension(pi: ExtensionAPI): void { + pi.registerCommand("gpt-account", { + description: "List and manage OpenAI Codex OAuth accounts.", + argumentHint: "[add | remove | pin | unpin]", + handler: async (rawArgs, ctx) => { + const args = parseArgs(rawArgs); + const action = args[0] ?? "list"; + try { + if (action === "list") { + await showAccounts(ctx); + return; + } + if (action === "add") { + await addAccount(ctx); + return; + } + if (action === "remove") { + await removeAccount(ctx, args[1]); + return; + } + if (action === "pin" && args[1] !== "unpin") { + await pinAccount(ctx, args[1]); + return; + } + if (action === "unpin" || (action === "pin" && args[1] === "unpin")) { + await pinCredentialAccount(ctx.modelRegistry.authStorage, OPENAI_CODEX_PROVIDER_ID, null); + ctx.ui.notify("Unpinned OpenAI Codex OAuth account.", "info"); + return; + } + usage(ctx); + } catch (error) { + ctx.ui.notify(error instanceof Error ? error.message : String(error), "error"); + } + }, + }); +} diff --git a/packages/coding-agent/src/core/extensions/builtin/index.ts b/packages/coding-agent/src/core/extensions/builtin/index.ts index 0c02f8b7c9..f71093528b 100644 --- a/packages/coding-agent/src/core/extensions/builtin/index.ts +++ b/packages/coding-agent/src/core/extensions/builtin/index.ts @@ -12,6 +12,7 @@ import cursorCliOauthExtension from "./cursor-cli-oauth/index.ts"; import diffExtension from "./diff.ts"; import filesExtension from "./files.ts"; import goalExtension from "./goal/index.ts"; +import gptAccountExtension from "./gpt-account.ts"; import gptApplyPatchExtension from "./gpt-apply-patch/index.ts"; import helpExtension from "./help/index.ts"; import historySearchExtension from "./history-search/index.ts"; @@ -102,6 +103,7 @@ export const builtinExtensions: BuiltinExtensionFactory[] = [ // Provider-neutral account listing; sits before the provider lanes so their // dedicated commands (claude-account, cursor accounts) keep their own names. { id: "account", factory: accountExtension }, + { id: "gpt-account", factory: gptAccountExtension }, { id: "claude-sdk-oauth", factory: claudeSdkOauthExtension }, // Registers unconditionally and reports executable/auth state through its oauth check, so it stays beside the other provider lane. { id: "cursor-cli-oauth", factory: cursorCliOauthExtension }, diff --git a/packages/coding-agent/test/auth-storage-slot-writes.test.ts b/packages/coding-agent/test/auth-storage-slot-writes.test.ts index 108f6782a0..81db2e19c4 100644 --- a/packages/coding-agent/test/auth-storage-slot-writes.test.ts +++ b/packages/coding-agent/test/auth-storage-slot-writes.test.ts @@ -130,13 +130,26 @@ describe("AuthStorage slot-preserving writes", () => { expect(entry?.pinned).toBe("work"); }); - test("set on a flat provider keeps today's whole-write shape", () => { + test("set on a flat provider promotes the legacy credential instead of overwriting it", () => { writeAuthJson({ openai: { type: "api_key", key: "legacy-key" } }); const storage = AuthStorage.create(authJsonPath); - storage.set("openai", { type: "api_key", key: "replaced-key" }); + storage.set("openai", { type: "api_key", key: "second-key" }); - expect(readAuthJson().openai).toEqual({ type: "api_key", key: "replaced-key" }); + const entry = readAuthJson().openai; + expect(entry?.accounts?.map((slot) => slot.name)).toEqual(["default", "login-2"]); + expect(entry?.accounts?.find((slot) => slot.name === "default")).toMatchObject({ key: "legacy-key" }); + expect(entry?.accounts?.find((slot) => slot.name === "login-2")).toMatchObject({ key: "second-key" }); + expect(entry).toMatchObject({ type: "api_key", key: "legacy-key" }); + }); + + test("set without a stored credential still writes the flat credential as-is", () => { + writeAuthJson({}); + const storage = AuthStorage.create(authJsonPath); + + storage.set("openai", { type: "api_key", key: "first-key" }); + + expect(readAuthJson().openai).toEqual({ type: "api_key", key: "first-key" }); }); test("reading a flat credential never rewrites auth.json", () => { diff --git a/packages/coding-agent/test/suite/account-extension.test.ts b/packages/coding-agent/test/suite/account-extension.test.ts index 11d429ad5f..ed3ce8709e 100644 --- a/packages/coding-agent/test/suite/account-extension.test.ts +++ b/packages/coding-agent/test/suite/account-extension.test.ts @@ -4,6 +4,8 @@ import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { AuthStorage } from "../../src/core/auth-storage.ts"; import accountExtension from "../../src/core/extensions/builtin/account/index.ts"; +import { subscribeProviderAccountEvents } from "../../src/core/extensions/builtin/claude-sdk-oauth/account-events.ts"; +import gptAccountExtension from "../../src/core/extensions/builtin/gpt-account.ts"; import type { ExtensionAPI, ExtensionCommandContext, RegisteredCommand } from "../../src/core/extensions/types.ts"; type Command = Pick; @@ -120,3 +122,180 @@ describe("/account command", () => { expect(notices.at(-1)?.message).toContain("Usage: /account"); }); }); + +describe("/gpt-account command", () => { + function registeredGptCommand(): Command { + const commands = new Map(); + const pi = { + registerCommand: (name: string, command: Command) => commands.set(name, command), + } as unknown as ExtensionAPI; + gptAccountExtension(pi); + const registered = commands.get("gpt-account"); + if (!registered) throw new Error("/gpt-account was not registered"); + return registered; + } + + it("lists OpenAI Codex OAuth accounts without leaking tokens", async () => { + await storage.modify("openai-codex", async () => ({ + type: "oauth", + access: "access-secret", + refresh: "refresh-secret", + expires: 1, + accounts: [ + { name: "default", access: "access-secret", refresh: "refresh-secret", expires: 1, source: "login" }, + { name: "work", access: "work-access", refresh: "work-refresh", expires: 1, source: "login" }, + ], + })); + const { ctx, notices } = createContext(); + + await registeredGptCommand().handler("", ctx); + + const output = notices.map((notice) => notice.message).join("\n"); + expect(output).toContain("OpenAI Codex OAuth accounts:"); + expect(output).toContain("default | login | available"); + expect(output).toContain("work | login | available"); + expect(output).not.toContain("access-secret"); + expect(output).not.toContain("work-access"); + }); + + it("pins and unpins an OpenAI Codex OAuth account", async () => { + await storage.modify("openai-codex", async () => ({ + type: "oauth", + access: "access-secret", + refresh: "refresh-secret", + expires: 1, + accounts: [ + { name: "default", access: "access-secret", refresh: "refresh-secret", expires: 1, source: "login" }, + { name: "work", access: "work-access", refresh: "work-refresh", expires: 1, source: "login" }, + ], + })); + const { ctx, notices } = createContext(); + const command = registeredGptCommand(); + + await command.handler("pin work", ctx); + await command.handler("", ctx); + expect(notices[notices.length - 1]?.message).toContain("work | login | available | pinned"); + + await command.handler("unpin", ctx); + expect(storage.get("openai-codex")).not.toHaveProperty("pinned"); + }); + + function createLoginContext(login: (provider: string, method: string) => Promise): { + ctx: ExtensionCommandContext; + notices: Notice[]; + logins: string[]; + } { + const { ctx, notices } = createContext(); + const logins: string[] = []; + const runtime = { + login: async (provider: string, method: string) => { + logins.push(`${provider}:${method}`); + await login(provider, method); + }, + }; + Object.assign(ctx.modelRegistry, { modelRuntime: runtime }); + return { ctx, notices, logins }; + } + + it("add runs an openai-codex oauth login and announces the new account", async () => { + const changed: string[] = []; + const unsubscribe = subscribeProviderAccountEvents((event) => { + if (event.type === "accounts_changed") changed.push(event.provider); + }); + const { ctx, notices, logins } = createLoginContext(async () => {}); + + try { + await registeredGptCommand().handler("add", ctx); + } finally { + unsubscribe(); + } + + expect(logins).toEqual(["openai-codex:oauth"]); + expect(notices.at(-1)).toMatchObject({ message: "OpenAI Codex OAuth account added.", type: "info" }); + expect(changed).toEqual(["openai-codex"]); + }); + + it("add stays silent when the user cancels the login prompt", async () => { + const changed: string[] = []; + const unsubscribe = subscribeProviderAccountEvents((event) => { + if (event.type === "accounts_changed") changed.push(event.provider); + }); + const { ctx, notices } = createLoginContext(async () => { + throw new Error("Login cancelled"); + }); + + try { + await registeredGptCommand().handler("add", ctx); + } finally { + unsubscribe(); + } + + expect(notices).toEqual([]); + expect(changed).toEqual([]); + }); + + it("add surfaces a real login failure as an error notice", async () => { + const { ctx, notices } = createLoginContext(async () => { + throw new Error("authorization server rejected the code"); + }); + + await registeredGptCommand().handler("add", ctx); + + expect(notices.at(-1)).toMatchObject({ message: "authorization server rejected the code", type: "error" }); + }); + + it("remove deletes exactly the named account", async () => { + await storage.modify("openai-codex", async () => ({ + type: "oauth", + access: "access-secret", + refresh: "refresh-secret", + expires: 1, + accounts: [ + { name: "default", access: "access-secret", refresh: "refresh-secret", expires: 1, source: "login" }, + { name: "work", access: "work-access", refresh: "work-refresh", expires: 1, source: "login" }, + ], + })); + const { ctx, notices } = createContext(); + + await registeredGptCommand().handler("remove work", ctx); + + expect(notices.at(-1)?.message).toContain("Removed OpenAI Codex OAuth account 'work'"); + expect(storage.listSlots("openai-codex").map((slot) => slot.name)).toEqual(["default"]); + }); + + it("remove default on a promoted pool leaves the survivor as the stored top-level credential", async () => { + // The shape appendLoginSlot writes when a legacy flat openai-codex credential + // gains a second login: the flat fields still project the legacy `default`. + await storage.modify("openai-codex", async () => ({ + type: "oauth", + access: "legacy-access", + refresh: "legacy-refresh", + expires: 1, + accounts: [ + { name: "default", access: "legacy-access", refresh: "legacy-refresh", expires: 1, source: "login" }, + { name: "login-2", access: "second-access", refresh: "second-refresh", expires: 2, source: "login" }, + ], + })); + const { ctx, notices } = createContext(); + + await registeredGptCommand().handler("remove default", ctx); + + expect(notices.at(-1)?.message).toContain("Removed OpenAI Codex OAuth account 'default'"); + expect(storage.listSlots("openai-codex").map((slot) => slot.name)).toEqual(["login-2"]); + expect(storage.get("openai-codex")).toMatchObject({ + type: "oauth", + access: "second-access", + refresh: "second-refresh", + expires: 2, + }); + }); + + it("remove without a name reports usage instead of removing anything", async () => { + const { ctx, notices } = createContext(); + + await registeredGptCommand().handler("remove", ctx); + + expect(notices.at(-1)?.type).toBe("error"); + expect(notices.at(-1)?.message).toContain("Usage: /gpt-account"); + }); +});