From 9bac0dc683c751415292ac6a0d2d46e5ea2115f2 Mon Sep 17 00:00:00 2001 From: yanyihan Date: Mon, 19 Oct 2026 18:08:49 +0800 Subject: [PATCH 1/2] fix(tips): stop recommending /login when providers already have credentials MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Home tips forced the /login tip whenever auth.json was empty, even if env or mimocode.jsonc API keys were already present. Gate on known credential fields (key, source=env, options.apiKey) across computed providers instead. Bare list membership does not count — mimo is always listed. --- .../spec/tips-login-credential-gate.md | 17 ++++++++ .../tui/feature-plugins/home/tips-view.tsx | 35 ++++++++++++--- .../cli/test/cli/cmd/tui/tips-view.test.ts | 43 ++++++++++++------- 3 files changed, 74 insertions(+), 21 deletions(-) create mode 100644 docs/compose/spec/tips-login-credential-gate.md diff --git a/docs/compose/spec/tips-login-credential-gate.md b/docs/compose/spec/tips-login-credential-gate.md new file mode 100644 index 000000000..ed35150b3 --- /dev/null +++ b/docs/compose/spec/tips-login-credential-gate.md @@ -0,0 +1,17 @@ +--- +feature: tips-login-credential-gate +status: delivered +updated: 2026-02-14 +branch: fix/tips-login-credential-gate +commits: 67fecf88..HEAD +--- + +# Tips /login Credential Gate + +## Report + +**What was built** — Home tips no longer force the `/login` tip just because the auth store is empty. The tip is shown only when no computed provider carries known credentials. A provider counts as credential-bearing when it has `key`, `source === "env"`, or a non-empty `options.apiKey`. That covers env keys, `mimocode.jsonc` API keys, and auth-store API keys through one rule. Bare membership in the computed provider list does not count — mimo is always listed and is not a credential. Keyless local-only `provider` blocks still recommend `/login` (accepted gap). + +**Verification** — From `packages/cli`: `bun test test/cli/cmd/tui/tips-view.test.ts` → 11 pass / 0 fail. From worktree root: `bun run typecheck` PASS. + +**Journey log** — `provider_next.authenticated` is auth-store only and missed config/env keys. Counting “providers in the list” false-positives on mimo. The gate is per-provider credential fields, not list membership or auth-store presence. diff --git a/packages/cli/src/cli/cmd/tui/feature-plugins/home/tips-view.tsx b/packages/cli/src/cli/cmd/tui/feature-plugins/home/tips-view.tsx index ec667dcef..ba770e11f 100644 --- a/packages/cli/src/cli/cmd/tui/feature-plugins/home/tips-view.tsx +++ b/packages/cli/src/cli/cmd/tui/feature-plugins/home/tips-view.tsx @@ -135,15 +135,39 @@ export function tipWeight(key: string) { return PRIORITY_WEIGHTS[key] ?? 1 } +/** Credential fields already resolved onto a computed provider. */ +export type CredentialProvider = { + key?: string | undefined + source: string + options?: Record | undefined +} + +/** + * Show `/login` only when no computed provider carries credentials. + * + * One rule, any source: env keys land as `key` / `source === "env"`, config and + * auth-store API keys as `options.apiKey` / `key`. Bare list membership does + * not count — mimo is always listed. Keyless local-only config is the accepted + * gap (still recommends `/login`). + */ +export function hasUsableCredentials(providers: readonly CredentialProvider[]): boolean { + return providers.some((provider) => { + if (provider.key) return true + if (provider.source === "env") return true + const apiKey = provider.options?.apiKey + return typeof apiKey === "string" && apiKey.length > 0 + }) +} + /** * Display override for the rotating tip. A credential-less install shows * /login (default model needs auth); otherwise the weighted rotation wins. */ export function pickDisplayKey(input: { - authenticatedCount: number + hasCredentials: boolean rotationKey: string }): string { - if (!input.authenticatedCount) return LOGIN_TIP + if (!input.hasCredentials) return LOGIN_TIP return input.rotationKey } @@ -203,12 +227,11 @@ export function Tips() { }) const interval = setInterval(() => setKey(pickWeighted(allKeys())), TIP_ROTATION_MS) onCleanup(() => clearInterval(interval)) - // With no authenticated provider, surface the /login tip instead so a - // credential-less install knows how to proceed (the default model requires - // login). Otherwise the weighted rotation is shown as-is. + // Only known credentials on computed providers suppress /login. mimo's mere + // presence in that list is not a credential. const displayKey = createMemo(() => pickDisplayKey({ - authenticatedCount: sync.data.provider_next.authenticated.length, + hasCredentials: hasUsableCredentials(sync.data.provider), rotationKey: key(), }), ) diff --git a/packages/cli/test/cli/cmd/tui/tips-view.test.ts b/packages/cli/test/cli/cmd/tui/tips-view.test.ts index 0cdf47e1e..b2c974491 100644 --- a/packages/cli/test/cli/cmd/tui/tips-view.test.ts +++ b/packages/cli/test/cli/cmd/tui/tips-view.test.ts @@ -1,5 +1,10 @@ import { describe, expect, test } from "bun:test" -import { buildTipKeys, pickDisplayKey, tipWeight } from "../../../../src/cli/cmd/tui/feature-plugins/home/tips-view" +import { + buildTipKeys, + hasUsableCredentials, + pickDisplayKey, + tipWeight, +} from "../../../../src/cli/cmd/tui/feature-plugins/home/tips-view" import { dict as en } from "../../../../src/cli/cmd/tui/i18n/en" import { dict as es } from "../../../../src/cli/cmd/tui/i18n/es" import { dict as fr } from "../../../../src/cli/cmd/tui/i18n/fr" @@ -52,25 +57,33 @@ describe("buildTipKeys", () => { }) }) +describe("hasUsableCredentials", () => { + test("is false when no computed provider carries credentials", () => { + // mimo is always in the computed list — presence alone must not count. + expect(hasUsableCredentials([])).toBe(false) + expect(hasUsableCredentials([{ source: "custom", options: {} }])).toBe(false) + expect(hasUsableCredentials([{ source: "config", options: { apiKey: "" } }])).toBe(false) + }) + + test("is true when any computed provider carries known credentials", () => { + expect(hasUsableCredentials([{ source: "env", key: "sk-env" }])).toBe(true) + expect(hasUsableCredentials([{ source: "env", options: {} }])).toBe(true) + expect(hasUsableCredentials([{ source: "config", options: { apiKey: "sk-config" } }])).toBe(true) + expect(hasUsableCredentials([{ source: "custom", options: {} }, { source: "api", key: "sk-api" }])).toBe(true) + }) +}) + describe("pickDisplayKey", () => { - test("forces the login tip when no provider is authenticated", () => { - expect( - pickDisplayKey({ authenticatedCount: 0, rotationKey: "tui.tips.theme" }), - ).toBe("tui.tips.login") + test("forces the login tip when the install has no credentials", () => { + expect(pickDisplayKey({ hasCredentials: false, rotationKey: "tui.tips.theme" })).toBe("tui.tips.login") }) - test("keeps the rotation tip once any provider is authenticated", () => { - expect( - pickDisplayKey({ authenticatedCount: 1, rotationKey: "tui.tips.theme" }), - ).toBe("tui.tips.theme") + test("keeps the rotation tip once credentials exist", () => { + expect(pickDisplayKey({ hasCredentials: true, rotationKey: "tui.tips.theme" })).toBe("tui.tips.theme") }) test("does not lock compose-next over the rotation", () => { - expect( - pickDisplayKey({ authenticatedCount: 1, rotationKey: "tui.tips.compose_next" }), - ).toBe("tui.tips.compose_next") - expect( - pickDisplayKey({ authenticatedCount: 1, rotationKey: "tui.tips.theme" }), - ).toBe("tui.tips.theme") + expect(pickDisplayKey({ hasCredentials: true, rotationKey: "tui.tips.compose_next" })).toBe("tui.tips.compose_next") + expect(pickDisplayKey({ hasCredentials: true, rotationKey: "tui.tips.theme" })).toBe("tui.tips.theme") }) }) From ec4c1a6132e7436bc5351664a88879647fd6e405 Mon Sep 17 00:00:00 2001 From: yanyihan Date: Mon, 19 Oct 2026 18:10:14 +0800 Subject: [PATCH 2/2] docs: record tips-login-credential-gate delivery range --- .../spec/tips-login-credential-gate.md | 40 +++++++++++++++++-- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/docs/compose/spec/tips-login-credential-gate.md b/docs/compose/spec/tips-login-credential-gate.md index ed35150b3..a5425f0d9 100644 --- a/docs/compose/spec/tips-login-credential-gate.md +++ b/docs/compose/spec/tips-login-credential-gate.md @@ -3,15 +3,49 @@ feature: tips-login-credential-gate status: delivered updated: 2026-02-14 branch: fix/tips-login-credential-gate -commits: 67fecf88..HEAD +commits: 67fecf88..9bac0dc6 --- # Tips /login Credential Gate ## Report -**What was built** — Home tips no longer force the `/login` tip just because the auth store is empty. The tip is shown only when no computed provider carries known credentials. A provider counts as credential-bearing when it has `key`, `source === "env"`, or a non-empty `options.apiKey`. That covers env keys, `mimocode.jsonc` API keys, and auth-store API keys through one rule. Bare membership in the computed provider list does not count — mimo is always listed and is not a credential. Keyless local-only `provider` blocks still recommend `/login` (accepted gap). +**What was built** — Home tips force the `/login` tip only when no computed provider carries known credentials. Credential-bearing means `key`, `source === "env"`, or a non-empty `options.apiKey`. This covers env keys, `mimocode.jsonc` API keys, and auth-store API keys through one rule. Bare list membership does not count (mimo is always listed). Keyless local-only providers still recommend `/login`. **Verification** — From `packages/cli`: `bun test test/cli/cmd/tui/tips-view.test.ts` → 11 pass / 0 fail. From worktree root: `bun run typecheck` PASS. -**Journey log** — `provider_next.authenticated` is auth-store only and missed config/env keys. Counting “providers in the list” false-positives on mimo. The gate is per-provider credential fields, not list membership or auth-store presence. +## [S1] Problem + +Home tips forced the `/login` tip whenever `provider_next.authenticated` was empty. That field is auth-store only (`auth.json` / `/login`). Users who already had API keys in env or `mimocode.jsonc` still saw “run /login”. + +## [S2] Design + +Show the forced `/login` tip only when **no computed provider carries known credentials**; otherwise use the normal weighted tip rotation. + +A provider is credential-bearing iff any of: + +- `key` is truthy — single-env keys and auth-store API keys +- `source === "env"` — env-resolved credentials (including multi-env, which omits `key`) +- `options.apiKey` is a non-empty string — config and auth-store keys + +Rules: + +- Gate solely on these credential fields of `sync.data.provider`. Do not use auth-store presence, config-`provider` presence, or “list is non-empty”. +- mimo is always in the computed provider list and is not a credential; bare membership must not suppress the tip. +- Keyless local-only `provider` blocks still recommend `/login` (accepted gap). +- Do not change provider load/write path. + +API: `hasUsableCredentials(providers) => boolean`; `pickDisplayKey({ hasCredentials, rotationKey })` returns `tui.tips.login` when `hasCredentials` is false. + +## [S3] Out of Scope + +- Config-`provider`-presence or auth-store-presence as independent gates. +- Model dialog’s always-on `tui.dialog.model.login_hint`. +- Changing `provider_next.authenticated` semantics or redaction. +- Plugin auth loaders that leave no `key`/`options.apiKey` (e.g. Copilot `apiKey: ""`). +- Non-TUI / CLI “not logged in” messages. + +## Tasks +- [x] T1: `hasUsableCredentials` + `pickDisplayKey` gate — acceptance: true iff any provider has `key`, `source==="env"`, or non-empty `options.apiKey`; false for bare list membership and empty `apiKey` (covers: S2) +- [x] T2: Wire tips from computed provider credentials only — acceptance: force `/login` exactly when `hasUsableCredentials` is false (covers: S2; depends: T1) +- [x] T3: Regression tests — acceptance: `bun test packages/cli/test/cli/cmd/tui/tips-view.test.ts` covers no-credentials and each credential field (covers: S2; depends: T1)