From d930f4c82d9cd4ec37bbffa30c1ab6ab3bd21ed4 Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 8 Jun 2026 01:16:37 +0200 Subject: [PATCH] feat(packages/ai): added credential origin to provider auth - Added `getCredentialOrigin` and `getEnvApiKeyName` to classify auth source. - Surfaced provenance tags in the `/login` and `/logout` provider picker. - Made the picker search filter match credential origin and env var name. - Added coverage for credential-origin precedence and env naming. --- packages/ai/CHANGELOG.md | 3 +- packages/ai/src/auth-storage.ts | 39 +++++++- packages/ai/src/stream.ts | 12 +++ .../auth-storage-credential-origin.test.ts | 94 +++++++++++++++++++ packages/coding-agent/CHANGELOG.md | 8 +- .../src/modes/components/oauth-selector.ts | 40 ++++++-- .../coding-agent/src/session/auth-storage.ts | 2 + .../coding-agent/test/cli-cwd-flag.test.ts | 4 +- .../modes/components/oauth-selector.test.ts | 3 + .../test/setup-wizard-sign-in.test.ts | 1 + 10 files changed, 193 insertions(+), 13 deletions(-) create mode 100644 packages/ai/test/auth-storage-credential-origin.test.ts diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index 192051eb2..44c9aeaac 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -5,10 +5,11 @@ ### Added - Added support for `impersonated_service_account` Application Default Credentials (ADC) in Vertex AI to enable chained impersonation without failing via 401 `invalid_client`. +- Added `AuthStorage.getCredentialOrigin(provider)` (returning a structured `CredentialOrigin` / `CredentialOriginKind`) and `getEnvApiKeyName(provider)`, so callers can render where a provider's auth comes from — runtime override, config, stored OAuth/api-key, env var (with the backing variable name), or fallback resolver — without parsing the prose of `describeCredentialSource`. ### Fixed -- Fixed duplicate upstream `tool_call_id` values collapsing distinct tool calls during message transformation, preserving one call/result pairing per emitted tool call before provider replay. ([#2055](https://github.com/can1357/oh-my-pi/issues/2055)) +- Fixed duplicate upstream `tool_call_id` values collapsing distinct tool calls during message transformation, preserving one call/result pairing per emitted tool call before provider replay and keeping generated duplicate IDs distinct after OpenAI/Mistral wire-length caps. ([#2055](https://github.com/can1357/oh-my-pi/issues/2055)) ## [15.10.1] - 2026-06-07 diff --git a/packages/ai/src/auth-storage.ts b/packages/ai/src/auth-storage.ts index b76c2bbd6..49a3843ba 100644 --- a/packages/ai/src/auth-storage.ts +++ b/packages/ai/src/auth-storage.ts @@ -13,7 +13,7 @@ import * as path from "node:path"; import { getAgentDbPath, logger } from "@oh-my-pi/pi-utils"; import type { ApiKeyResolver } from "./auth-retry"; import { isUsageLimitError } from "./rate-limit-utils"; -import { getEnvApiKey } from "./stream"; +import { getEnvApiKey, getEnvApiKeyName } from "./stream"; import type { Provider } from "./types"; import type { CredentialRankingStrategy, @@ -59,6 +59,23 @@ export type AuthCredentialEntry = AuthCredential | AuthCredential[]; export type AuthStorageData = Record; +/** + * Cascade leg that supplies a provider's active credential, highest precedence + * first — mirrors {@link AuthStorage.getApiKey}'s resolution order. + */ +export type CredentialOriginKind = "runtime" | "config" | "oauth" | "api_key" | "env" | "fallback"; + +/** + * Structured provenance for a provider's auth, for UI that needs a machine + * tag (the `/login` provider list) rather than the prose of + * {@link AuthStorage.describeCredentialSource}. + */ +export interface CredentialOrigin { + kind: CredentialOriginKind; + /** Env var name when `kind === "env"` and a single named variable backs it. */ + envVar?: string; +} + /** * Serialized representation of AuthStorage for passing to subagent workers. * Contains only the essential credential data, not runtime state. @@ -1430,6 +1447,26 @@ export class AuthStorage { return false; } + /** + * Classify where a provider's auth comes from, following the same precedence + * as {@link AuthStorage.getApiKey}: runtime override → config override → + * stored credential (api_key before oauth, matching getApiKey) → env var → + * fallback resolver. Returns undefined when no auth is configured. + * + * Compact, structured counterpart to {@link describeCredentialSource}. + */ + getCredentialOrigin(provider: string): CredentialOrigin | undefined { + if (this.#runtimeOverrides.has(provider)) return { kind: "runtime" }; + if (this.#configOverrides.has(provider)) return { kind: "config" }; + const stored = this.#getCredentialsForProvider(provider); + if (stored.length > 0) { + return { kind: stored.some(credential => credential.type === "api_key") ? "api_key" : "oauth" }; + } + if (getEnvApiKey(provider)) return { kind: "env", envVar: getEnvApiKeyName(provider) }; + if (this.#fallbackResolver?.(provider)) return { kind: "fallback" }; + return undefined; + } + /** * Check if OAuth credentials are configured for a provider. */ diff --git a/packages/ai/src/stream.ts b/packages/ai/src/stream.ts index f44c0a5b4..c081a80c8 100644 --- a/packages/ai/src/stream.ts +++ b/packages/ai/src/stream.ts @@ -285,6 +285,18 @@ export function getEnvApiKey(provider: string): string | undefined { return resolver?.(); } +/** + * Name of the environment variable that backs `getEnvApiKey` for a provider, + * when that provider maps to a single named variable (e.g. `github-copilot` → + * `COPILOT_GITHUB_TOKEN`). Returns undefined for providers whose env fallback + * is computed (multi-var pickers, Vertex ADC / Bedrock probes, …) since no + * single variable name describes the source. + */ +export function getEnvApiKeyName(provider: string): string | undefined { + const resolver = serviceProviderMap[provider]; + return typeof resolver === "string" ? resolver : undefined; +} + /** * Enumerate every provider that has an env-var fallback for `getEnvApiKey`. * Used by `omp auth-broker migrate --include-env` to discover env-sourced keys diff --git a/packages/ai/test/auth-storage-credential-origin.test.ts b/packages/ai/test/auth-storage-credential-origin.test.ts new file mode 100644 index 000000000..cbb50e26d --- /dev/null +++ b/packages/ai/test/auth-storage-credential-origin.test.ts @@ -0,0 +1,94 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { type AuthCredentialStore, AuthStorage, SqliteAuthCredentialStore } from "../src/auth-storage"; +import { withEnv } from "./helpers"; + +// Clear every env var the providers under test alias, so ambient shell / ~/.env +// state can't leak an env origin into precedence assertions. +const SUPPRESS_ENV = { + OPENAI_API_KEY: undefined, + ANTHROPIC_API_KEY: undefined, + ANTHROPIC_OAUTH_TOKEN: undefined, + COPILOT_GITHUB_TOKEN: undefined, +} as const; + +describe("AuthStorage.getCredentialOrigin", () => { + let tempDir = ""; + let store: AuthCredentialStore | null = null; + let auth: AuthStorage | null = null; + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "pi-ai-credential-origin-")); + store = await SqliteAuthCredentialStore.open(path.join(tempDir, "agent.db")); + auth = new AuthStorage(store); + }); + + afterEach(async () => { + store?.close(); + store = null; + auth = null; + if (tempDir) { + await fs.rm(tempDir, { recursive: true, force: true }); + tempDir = ""; + } + }); + + test("undefined when no auth is configured", async () => { + await withEnv(SUPPRESS_ENV, () => { + // Provider absent from the env map entirely — no env fallback can apply. + expect(auth?.getCredentialOrigin("no-such-provider")).toBeUndefined(); + }); + }); + + test("env origin carries the backing variable name for single-var providers", async () => { + await withEnv({ ...SUPPRESS_ENV, COPILOT_GITHUB_TOKEN: "ghp_fake" }, () => { + expect(auth?.getCredentialOrigin("github-copilot")).toEqual({ + kind: "env", + envVar: "COPILOT_GITHUB_TOKEN", + }); + }); + }); + + test("env origin omits the variable name for computed resolvers", async () => { + // anthropic resolves through $pickenv(...) — no single variable describes it. + await withEnv({ ...SUPPRESS_ENV, ANTHROPIC_API_KEY: "sk-fake" }, () => { + expect(auth?.getCredentialOrigin("anthropic")).toEqual({ kind: "env" }); + }); + }); + + test("a stored OAuth credential outranks an env var", async () => { + await withEnv({ ...SUPPRESS_ENV, COPILOT_GITHUB_TOKEN: "ghp_fake" }, async () => { + await auth?.set("github-copilot", [ + { type: "oauth", access: "a", refresh: "r", expires: Date.now() + 60_000 }, + ]); + expect(auth?.getCredentialOrigin("github-copilot")).toEqual({ kind: "oauth" }); + }); + }); + + test("a stored api key reports api_key and outranks a co-stored OAuth credential", async () => { + await withEnv(SUPPRESS_ENV, async () => { + // getApiKey() prefers api_key before oauth, so the origin must match. + await auth?.set("openai", [ + { type: "oauth", access: "a", refresh: "r", expires: Date.now() + 60_000 }, + { type: "api_key", key: "sk-stored" }, + ]); + expect(auth?.getCredentialOrigin("openai")).toEqual({ kind: "api_key" }); + }); + }); + + test("config then runtime overrides take precedence over stored credentials", async () => { + await withEnv(SUPPRESS_ENV, async () => { + if (!auth) throw new Error("test setup failed"); + await auth.set("openai", [{ type: "api_key", key: "sk-stored" }]); + expect(auth.getCredentialOrigin("openai")).toEqual({ kind: "api_key" }); + + auth.setConfigApiKey("openai", "gateway-bearer"); + expect(auth.getCredentialOrigin("openai")).toEqual({ kind: "config" }); + + auth.setRuntimeApiKey("openai", "cli-flag-bearer"); + expect(auth.getCredentialOrigin("openai")).toEqual({ kind: "runtime" }); + }); + }); +}); diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 1ee9d374f..73d2f1c4a 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +- Added credential provenance to the `/login` and `/logout` provider picker: each authenticated provider now shows where its credential comes from — `(login)`, `(api key)`, `(env: VAR_NAME)`, `(config)`, `(--api-key)`, or `(custom provider)` — so a real OAuth login is distinguishable from an env var that merely aliases the provider (e.g. `COPILOT_GITHUB_TOKEN`). The origin is also matched by the picker's type-to-search filter. + ### Fixed - Fixed the working spinner appearing to ignore Esc for 2-3 seconds when an interrupt lands mid-tool. Esc fires the abort synchronously, but the agent loop only stops the loader at `agent_end`, which it cannot reach until every in-flight tool settles in `executeToolCalls`' `await Promise.allSettled(...)` — and process/subagent/kernel-owning tools tear down gracefully (SIGTERM, 2-3s grace, SIGKILL), so the loader kept showing the unchanged "Working…/" line and read as a dropped keypress. The loader now switches to "Interrupting…" the instant Esc requests the abort and freezes intent-driven label updates until the turn unwinds (`EventController.notifyInterrupting`), so the interrupt is acknowledged immediately even while teardown completes. @@ -9,8 +13,8 @@ - Fixed reviewer-style subagent yields crashing the calling eval cell when a caller-supplied output schema declares `additionalProperties: false` without a `findings` property. `normalizeCompleteData` now consults the active validator before splicing collected `report_finding` entries onto the yielded payload, so injection is suppressed when the schema would reject it — keeping the executor's post-mortem validation in lockstep with the in-tool `yield` validation that already accepted the same raw payload ([#2070](https://github.com/can1357/oh-my-pi/issues/2070)) - Fixed Anthropic empty `toolUse` stops without tool calls corrupting session history by retrying them and removing orphaned turns even at the retry cap. - Fixed MCP tools hanging in non-yolo modes by declaring `approval = "write"` on `MCPTool` and `DeferredMCPTool`, and propagating the `approval` property through `customToolToDefinition()` in `sdk.ts` -- Fixed session resumption after a working directory is moved/renamed (e.g. `git worktree move`): `--continue` now re-roots the terminal's last session into the new directory when its original directory no longer exists, instead of silently starting a fresh empty session; cross-project `--resume ` offers to move (re-root) the session rather than only forking a duplicate copy when the source directory is gone -- Fixed Kitty OSC 5522 paste rejecting plain text as "no supported text or image data": the listing parser now decodes the `mime="."` DATA payload (whitespace-separated MIME list) Kitty actually sends, in addition to the per-type DATA packets described by the ancillary 5522-mode spec ([#2051](https://github.com/can1357/oh-my-pi/issues/2051)) +- Fixed session resumption after a working directory is moved/renamed (e.g. `git worktree move`): `--continue` now re-roots the terminal's last session into the new directory when its original directory no longer exists, explicit `--resume --session-dir ` local matches re-root instead of reopening with the stale cwd, and cross-project `--resume ` offers to move (re-root) the session rather than only forking a duplicate copy when the source directory is gone +- Fixed Kitty OSC 5522 paste rejecting plain text as "no supported text or image data": the listing parser now decodes the `mime="."` DATA payload (whitespace-separated MIME list) Kitty actually sends, in addition to the per-type DATA packets described by the ancillary 5522-mode spec, and per-type spec listings now request the selected payload with `type=read:mime=...` instead of Kitty's dot-payload request shape ([#2051](https://github.com/can1357/oh-my-pi/issues/2051)) - Fixed follow-up shortcut submission of builtin slash commands so `/goal set ...` applies goal mode instead of queueing as plain text. - Fixed Ctrl+Z crashing the agent on Windows with `TypeError: Unknown signal: SIGTSTP`. `InputController.handleCtrlZ` called `process.kill(0, "SIGTSTP")` unconditionally, but `SIGTSTP` is POSIX job-control and Bun/Node on Windows rejects the signal name from the JS side; the throw propagated out of the TUI input dispatcher as an uncaught exception. The handler now no-ops with a "Suspend (Ctrl+Z) is not supported on this platform" status on Windows, and on POSIX wraps `process.kill` in a try/catch that detaches the registered SIGCONT resume hook and re-`start()`s the TUI on failure so a rejected signal can never leave the UI stranded with a leaked listener ([#2036](https://github.com/can1357/oh-my-pi/issues/2036)). - Fixed a relative `--cwd` target (e.g. `omp --cwd repo` launched from `/tmp`) leaking the raw relative string into the session config. `applyStartupCwd` chdired into the resolved directory via `setProjectDir` but left `parsed.cwd` as `"repo"`, so `buildSessionOptions` (which prefers `parsed.cwd` over `getProjectDir()`) handed downstream settings/discovery/session creation a value that re-resolved against the new process cwd (`/tmp/repo/repo`) or persisted a relative session cwd. `parsed.cwd` is now re-synced to the resolved absolute project dir after the chdir. diff --git a/packages/coding-agent/src/modes/components/oauth-selector.ts b/packages/coding-agent/src/modes/components/oauth-selector.ts index e73e3f86e..b837b4ce2 100644 --- a/packages/coding-agent/src/modes/components/oauth-selector.ts +++ b/packages/coding-agent/src/modes/components/oauth-selector.ts @@ -11,10 +11,20 @@ import { } from "@oh-my-pi/pi-tui"; import { theme } from "../../modes/theme/theme"; import { matchesSelectCancel, matchesSelectDown, matchesSelectUp } from "../../modes/utils/keybinding-matchers"; -import type { AuthStorage } from "../../session/auth-storage"; +import type { AuthStorage, CredentialOriginKind } from "../../session/auth-storage"; import { DynamicBorder } from "./dynamic-border"; const OAUTH_SELECTOR_MAX_VISIBLE = 10; + +/** Compact, human-readable tag for each credential-origin leg. */ +const ORIGIN_LABELS: Record = { + runtime: "--api-key", + config: "config", + oauth: "login", + api_key: "api key", + env: "env", + fallback: "custom provider", +}; /** * Component that renders an OAuth provider selector. */ @@ -146,20 +156,34 @@ export class OAuthSelectorComponent extends Container { } } + /** + * Muted provenance suffix (" (env: COPILOT_GITHUB_TOKEN)", " (login)", …) so + * the list distinguishes a real login from an env var aliasing the provider. + */ + #getSourceLabel(providerId: string): string { + const origin = this.#authStorage.getCredentialOrigin(providerId); + if (!origin) return ""; + const detail = origin.kind === "env" && origin.envVar ? `env: ${origin.envVar}` : ORIGIN_LABELS[origin.kind]; + return theme.fg("muted", ` (${detail})`); + } + #getStatusIndicator(providerId: string): string { const state = this.#authState.get(providerId); + const source = this.#getSourceLabel(providerId); if (state === "checking") { const frameCount = theme.spinnerFrames.length; const spinner = frameCount > 0 ? theme.spinnerFrames[this.#spinnerFrame % frameCount] : theme.status.pending; - return theme.fg("warning", ` ${spinner} checking`); + return theme.fg("warning", ` ${spinner} checking`) + source; } if (state === "invalid") { - return theme.fg("error", ` ${theme.status.error} invalid`); + return theme.fg("error", ` ${theme.status.error} invalid`) + source; } if (state === "valid") { - return theme.fg("success", ` ${theme.status.success} logged in`); + return theme.fg("success", ` ${theme.status.success} logged in`) + source; } - return this.#hasSelectableAuth(providerId) ? theme.fg("success", ` ${theme.status.success} logged in`) : ""; + return this.#hasSelectableAuth(providerId) + ? theme.fg("success", ` ${theme.status.success} logged in`) + source + : ""; } #isSearchEnabled(): boolean { @@ -178,8 +202,10 @@ export class OAuthSelectorComponent extends Container { #getProviderSearchText(provider: OAuthProviderInfo): string { let text = `${provider.name} ${provider.id}`; - if (this.#hasSelectableAuth(provider.id)) { - text += " logged in authenticated"; + const origin = this.#authStorage.getCredentialOrigin(provider.id); + if (origin) { + text += ` logged in authenticated ${ORIGIN_LABELS[origin.kind]}`; + if (origin.envVar) text += ` ${origin.envVar}`; } if (!provider.available) { text += " unavailable"; diff --git a/packages/coding-agent/src/session/auth-storage.ts b/packages/coding-agent/src/session/auth-storage.ts index d3486d7e2..a0a8c0133 100644 --- a/packages/coding-agent/src/session/auth-storage.ts +++ b/packages/coding-agent/src/session/auth-storage.ts @@ -10,6 +10,8 @@ export type { AuthCredentialStore, AuthStorageData, AuthStorageOptions, + CredentialOrigin, + CredentialOriginKind, OAuthCredential, SerializedAuthStorage, SnapshotResponse, diff --git a/packages/coding-agent/test/cli-cwd-flag.test.ts b/packages/coding-agent/test/cli-cwd-flag.test.ts index 5fb734d7c..0f131f153 100644 --- a/packages/coding-agent/test/cli-cwd-flag.test.ts +++ b/packages/coding-agent/test/cli-cwd-flag.test.ts @@ -2,7 +2,7 @@ import { afterEach, describe, expect, it } from "bun:test"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; -import { getProjectDir, setProjectDir } from "@oh-my-pi/pi-utils"; +import { getProjectDir, normalizePathForComparison, setProjectDir } from "@oh-my-pi/pi-utils"; import { parseArgs } from "../src/cli/args"; import { applyStartupCwd } from "../src/cli/startup-cwd"; @@ -36,7 +36,7 @@ describe("parseArgs — --cwd flag", () => { expect(parsed.continue).toBe(true); expect(getProjectDir()).toBe(targetDir); - expect(process.cwd()).toBe(targetDir); + expect(normalizePathForComparison(process.cwd())).toBe(normalizePathForComparison(targetDir)); }); it("normalizes a relative --cwd target to the resolved absolute path", async () => { diff --git a/packages/coding-agent/test/modes/components/oauth-selector.test.ts b/packages/coding-agent/test/modes/components/oauth-selector.test.ts index 8267c5e09..b53b93ae3 100644 --- a/packages/coding-agent/test/modes/components/oauth-selector.test.ts +++ b/packages/coding-agent/test/modes/components/oauth-selector.test.ts @@ -11,6 +11,7 @@ beforeAll(async () => { const authStorage = { has: (_providerId: string) => false, hasAuth: (_providerId: string) => false, + getCredentialOrigin: (_providerId: string) => undefined, } as unknown as AuthStorage; describe("OAuthSelectorComponent", () => { @@ -54,6 +55,7 @@ describe("OAuthSelectorComponent", () => { { has: (_providerId: string) => false, hasAuth: (providerId: string) => providerId === "opencode-go" || providerId === "opencode-zen", + getCredentialOrigin: (_providerId: string) => undefined, } as unknown as AuthStorage, providerId => selected.push(providerId), () => {}, @@ -80,6 +82,7 @@ describe("OAuthSelectorComponent", () => { { has: (providerId: string) => providerId === "opencode-go", hasAuth: (providerId: string) => providerId === "opencode-go", + getCredentialOrigin: (_providerId: string) => undefined, } as unknown as AuthStorage, providerId => selected.push(providerId), () => {}, diff --git a/packages/coding-agent/test/setup-wizard-sign-in.test.ts b/packages/coding-agent/test/setup-wizard-sign-in.test.ts index f4c0c4864..ab234db51 100644 --- a/packages/coding-agent/test/setup-wizard-sign-in.test.ts +++ b/packages/coding-agent/test/setup-wizard-sign-in.test.ts @@ -18,6 +18,7 @@ describe("SignInTab", () => { const authStorage = { has: (_providerId: string) => false, hasAuth: (_providerId: string) => false, + getCredentialOrigin: (_providerId: string) => undefined, async login(_provider: OAuthProviderId, ctrl: OAuthLoginCallbacks): Promise { ctrl.onAuth({ url }); const prompt = ctrl.onManualCodeInput?.();