diff --git a/devlog/_plan/260806_stacked_bug_campaign/evidence/1151-effort-picker-before-after.png b/devlog/_plan/260806_stacked_bug_campaign/evidence/1151-effort-picker-before-after.png new file mode 100644 index 0000000000..bc92ea158c Binary files /dev/null and b/devlog/_plan/260806_stacked_bug_campaign/evidence/1151-effort-picker-before-after.png differ diff --git a/gui/src/combo-workspace-data.ts b/gui/src/combo-workspace-data.ts index bc29bff703..aa32facedd 100644 --- a/gui/src/combo-workspace-data.ts +++ b/gui/src/combo-workspace-data.ts @@ -8,7 +8,10 @@ export type ComboEffort = "low" | "medium" | "high" | "xhigh" | "max" | "ultra"; export const COMBO_EFFORTS: ComboEffort[] = ["low", "medium", "high", "xhigh", "max", "ultra"]; -/** Intersection of per-member effort ladders; unknown ladders contribute no selectable efforts. */ +/** + * Intersection of advertised effort ladders for picker availability. + * Unknown ladders are wildcards here only; runtime injection remains fail-closed. + */ export function intersectComboEfforts( targets: readonly ComboTarget[], modelEfforts: ReadonlyMap, @@ -20,11 +23,8 @@ export function intersectComboEfforts( for (const target of complete) { const key = `${target.provider.trim()}/${target.model.trim()}`; const listed = modelEfforts.get(key); - // Missing metadata must not invent a full ladder — runtime omits the combo default when - // supportedLadderFor is undefined (#488 / Codex review). - const member: string[] = listed === undefined - ? [] - : listed.filter((effort) => effortSet.has(effort)); + if (listed === undefined) continue; + const member = listed.filter((effort) => effortSet.has(effort)); if (common === null) { common = member; } else { @@ -32,7 +32,8 @@ export function intersectComboEfforts( common = common.filter((effort) => memberSet.has(effort)); } } - const commonSet = new Set(common ?? []); + if (common === null) return [...COMBO_EFFORTS]; + const commonSet = new Set(common); return COMBO_EFFORTS.filter((effort) => commonSet.has(effort)); } diff --git a/src/clients/config-export.ts b/src/clients/config-export.ts index 0a4a84ed00..4d8ca366c1 100644 --- a/src/clients/config-export.ts +++ b/src/clients/config-export.ts @@ -107,15 +107,9 @@ export const OPENCODE_API_KEY_ENV = "OPENCODEX_OPENCODE_API_KEY"; /** Env reference shared by apiKey and the dedicated proxy admission header. */ export const OPENCODE_API_KEY_ENV_REF = `{env:${OPENCODE_API_KEY_ENV}}`; -/** Env var Pi interpolates. Pi takes bare `$NAME`, not opencode's `{env:NAME}`. */ -export const PI_API_KEY_ENV = "OPENCODEX_API_KEY"; - -/** Pi's reference form for the admission key. Never the value. */ -export const PI_API_KEY_ENV_REF = `$${PI_API_KEY_ENV}`; - /** * Hermes interpolates `${VAR}` anywhere in config.yaml, so the credential stays - * in the environment exactly as it does for OpenCode and Pi. + * in the environment exactly as it does for OpenCode. */ export const HERMES_API_KEY_ENV = "OPENCODEX_HERMES_API_KEY"; export const HERMES_API_KEY_ENV_REF = `\${${HERMES_API_KEY_ENV}}`; @@ -125,12 +119,12 @@ export const OPENCLAW_API_KEY_ENV = "OPENCODEX_OPENCLAW_API_KEY"; export const OPENCLAW_API_KEY_ENV_REF = `\${${OPENCLAW_API_KEY_ENV}}`; /** - * Kimi Code reads credentials ONLY from its config file — it never falls back - * to the shell environment. A loopback bind needs no real admission key, so we - * emit the same placeholder the Grok managed block uses rather than a user - * secret; a non-loopback bind is refused by the writer instead of papered over. + * Placeholder credential for loopback-only clients (Kimi, Pi). A loopback + * bind needs no real admission key, so we emit the same placeholder the Grok + * managed block uses rather than a user secret. Pi resolves `apiKey` before + * building its model list and hides the provider when an env reference is unset. */ -export const KIMI_LOOPBACK_PLACEHOLDER = "opencodex-loopback"; +export const LOOPBACK_API_KEY_PLACEHOLDER = "opencodex-loopback"; /** * Gajae's `apiKeyEnv` is env-name-only and fail-closed. Its sibling `apiKey` @@ -728,7 +722,7 @@ function buildPiClientConfig(ctx: ExportContext): PiGeneratedConfig { [OPENCODE_PROVIDER_ID]: { baseUrl: ctx.baseUrl, api: PI_API_DIALECT, - apiKey: PI_API_KEY_ENV_REF, + apiKey: LOOPBACK_API_KEY_PLACEHOLDER, models, }, }, @@ -808,7 +802,7 @@ function buildKimiClientConfig(ctx: ExportContext): KimiGeneratedConfig { [OPENCODE_PROVIDER_ID]: { type: "openai", base_url: ctx.baseUrl, - api_key: KIMI_LOOPBACK_PLACEHOLDER, + api_key: LOOPBACK_API_KEY_PLACEHOLDER, }, }, models, @@ -949,15 +943,14 @@ export const EXPORT_CLIENTS: Record = { id: "pi", filename: "pi-models.json", destination: () => join(homedir(), ".pi", "agent", "models.json"), - apiKeyEnv: PI_API_KEY_ENV, - exportHint: `export ${PI_API_KEY_ENV}=`, + apiKeyEnv: "", + exportHint: "Pi reads a non-secret placeholder from models.json; loopback needs no key.", build: buildPiClientConfig, format: "json", summarize: summarizePi, buildContribution: buildPiContribution, - // No header field in Pi's provider block (and the schema is unverified - // against a real install), so there is nowhere to put the dedicated - // admission header a remote bind requires. + // No header field in Pi's provider block, so there is nowhere to put the + // dedicated admission header a remote bind requires. loopbackOnly: true, }, hermes: { diff --git a/src/combos/request.ts b/src/combos/request.ts index c7d535f7c9..7727b7689b 100644 --- a/src/combos/request.ts +++ b/src/combos/request.ts @@ -40,6 +40,8 @@ export function concreteComboRequestBody( && !Object.prototype.hasOwnProperty.call(reasoning, "effort") ); if (!needsDefault) return clone; + // Picker availability treats an unknown ladder as a wildcard, but runtime + // injection stays fail-closed until this concrete target advertises support. if (!targetReasoningEfforts?.includes(defaultEffort)) { const key = `${target.provider}/${target.model}:${defaultEffort}`; if (!warnedUnsupportedDefaults.has(key)) { diff --git a/tests/cli-export-command.test.ts b/tests/cli-export-command.test.ts index 108dbcaa3d..f793fc7e08 100644 --- a/tests/cli-export-command.test.ts +++ b/tests/cli-export-command.test.ts @@ -139,11 +139,15 @@ describe("ocx export human output (accept criterion 2)", () => { expect(result.stdout).toContain("3 models; 1 omit context limits"); }); - test("Pi names its own destination and env var", async () => { + test("Pi names its own destination and needs no env var", async () => { const proxy = fakeProxy(); const result = await run(["--client", "pi"], { baseUrl: proxy.baseUrl }); expect(result.stdout).toContain(join(".pi", "agent", "models.json")); - expect(result.stdout).toContain("export OPENCODEX_API_KEY="); + // Pi resolves `apiKey` before building its model list and hides the provider + // when an env reference is unset, so a loopback bind ships the non-secret + // placeholder instead of an env var the user was never told to export. + expect(result.stdout).toContain("opencodex-loopback"); + expect(result.stdout).not.toContain("export OPENCODEX_API_KEY="); }); }); @@ -297,8 +301,10 @@ describe("ocx export never serializes a key (accept criterion 6)", () => { for (const [args, envRef] of [ [["--client", "opencode"], "{env:OPENCODEX_OPENCODE_API_KEY}"], [["--client", "opencode", "--json"], "{env:OPENCODEX_OPENCODE_API_KEY}"], - [["--client", "pi"], "$OPENCODEX_API_KEY"], - [["--client", "pi", "--json"], "$OPENCODEX_API_KEY"], + // Pi ships the non-secret loopback placeholder rather than an env reference; + // the property under test is unchanged — no real key ever reaches stdout. + [["--client", "pi"], "opencodex-loopback"], + [["--client", "pi", "--json"], "opencodex-loopback"], ] as Array<[string[], string]>) { logs = []; errors = []; diff --git a/tests/client-config-export-new-clients.test.ts b/tests/client-config-export-new-clients.test.ts index 9de47024f6..8f5173b14c 100644 --- a/tests/client-config-export-new-clients.test.ts +++ b/tests/client-config-export-new-clients.test.ts @@ -5,7 +5,7 @@ import { EXPORT_CLIENT_IDS, GAJAE_API_KEY_ENV, HERMES_API_KEY_ENV_REF, - KIMI_LOOPBACK_PLACEHOLDER, + LOOPBACK_API_KEY_PLACEHOLDER, OPENCLAW_API_KEY_ENV_REF, OPENCODE_PROVIDER_ID, buildClientConfig, @@ -235,7 +235,7 @@ describe("kimi", () => { test("uses the loopback placeholder because Kimi reads no environment", () => { const doc = buildClientConfig("kimi", ctx()) as KimiGeneratedConfig; - expect(doc.providers[OPENCODE_PROVIDER_ID]!.api_key).toBe(KIMI_LOOPBACK_PLACEHOLDER); + expect(doc.providers[OPENCODE_PROVIDER_ID]!.api_key).toBe(LOOPBACK_API_KEY_PLACEHOLDER); }); test("never emits capabilities it cannot assert", () => { diff --git a/tests/client-config-export.test.ts b/tests/client-config-export.test.ts index 8ab7faeab5..3e8b8e881e 100644 --- a/tests/client-config-export.test.ts +++ b/tests/client-config-export.test.ts @@ -6,8 +6,7 @@ import { EXPORT_CLIENT_IDS, OPENCODE_API_KEY_ENV, OPENCODE_API_KEY_ENV_REF, - PI_API_KEY_ENV, - PI_API_KEY_ENV_REF, + LOOPBACK_API_KEY_PLACEHOLDER, SCHEMA_REQUIRED_OUTPUT_BUDGET, buildClientConfig, buildClientConfigText, @@ -160,12 +159,11 @@ describe("Pi serializer (accept criterion 2)", () => { ]); }); - test("provider envelope names the OpenAI-compatible dialect and the env reference", () => { + test("provider envelope names the OpenAI-compatible dialect and the loopback placeholder", () => { const provider = piConfig().providers.opencodex!; expect(provider.baseUrl).toBe(BASE_URL); expect(provider.api).toBe("openai-completions"); - expect(provider.apiKey).toBe(PI_API_KEY_ENV_REF); - expect(provider.apiKey).toBe("$OPENCODEX_API_KEY"); + expect(provider.apiKey).toBe(LOOPBACK_API_KEY_PLACEHOLDER); }); test("cost is omitted on every entry — zeros would assert routed models are free", () => { @@ -236,7 +234,7 @@ describe("no credential ever reaches the output (accept criterion 3)", () => { test("each client emits only its own documented env reference", () => { expect(JSON.stringify(opencodeConfig())).toContain(OPENCODE_API_KEY_ENV_REF); - expect(JSON.stringify(piConfig())).toContain(PI_API_KEY_ENV_REF); + expect(JSON.stringify(piConfig())).toContain(LOOPBACK_API_KEY_PLACEHOLDER); expect(JSON.stringify(piConfig())).not.toContain("{env:"); }); }); @@ -348,7 +346,7 @@ describe("EXPORT_CLIENTS registry", () => { "opencodex": { "baseUrl": "http://127.0.0.1:10100/v1", "api": "openai-completions", - "apiKey": "$OPENCODEX_API_KEY", + "apiKey": "opencodex-loopback", "models": [ { "id": "anthropic/claude-opus-5", @@ -449,8 +447,8 @@ describe("EXPORT_CLIENTS registry", () => { test("apiKeyEnv and exportHint name the variable the config references", () => { expect(EXPORT_CLIENTS.opencode.apiKeyEnv).toBe(OPENCODE_API_KEY_ENV); expect(EXPORT_CLIENTS.opencode.exportHint).toContain(OPENCODE_API_KEY_ENV); - expect(EXPORT_CLIENTS.pi.apiKeyEnv).toBe(PI_API_KEY_ENV); - expect(EXPORT_CLIENTS.pi.exportHint).toContain(PI_API_KEY_ENV); + expect(EXPORT_CLIENTS.pi.apiKeyEnv).toBe(""); + expect(EXPORT_CLIENTS.pi.exportHint).toContain("loopback"); for (const id of EXPORT_CLIENT_IDS) { expect(EXPORT_CLIENTS[id].exportHint).not.toContain("ocx_"); } diff --git a/tests/client-config-new-clients.test.ts b/tests/client-config-new-clients.test.ts index 51fd6de1f4..eaf500ca54 100644 --- a/tests/client-config-new-clients.test.ts +++ b/tests/client-config-new-clients.test.ts @@ -4,7 +4,7 @@ import { EXPORT_CLIENT_IDS, GAJAE_API_KEY_ENV, HERMES_API_KEY_ENV_REF, - KIMI_LOOPBACK_PLACEHOLDER, + LOOPBACK_API_KEY_PLACEHOLDER, OPENCLAW_API_KEY_ENV_REF, OPENCODE_PROVIDER_ID, buildClientConfig, @@ -56,7 +56,7 @@ describe("no client config ever carries a credential", () => { test("kimi uses the loopback placeholder because it cannot read env vars", () => { const doc = buildClientConfig("kimi", ctx()) as KimiGeneratedConfig; - expect(doc.providers[OPENCODE_PROVIDER_ID]!.api_key).toBe(KIMI_LOOPBACK_PLACEHOLDER); + expect(doc.providers[OPENCODE_PROVIDER_ID]!.api_key).toBe(LOOPBACK_API_KEY_PLACEHOLDER); }); test("gajae uses apiKeyEnv, not the apiKey footgun", () => { diff --git a/tests/combo-workspace-data.test.ts b/tests/combo-workspace-data.test.ts index 43a6c00366..9ad3ed6544 100644 --- a/tests/combo-workspace-data.test.ts +++ b/tests/combo-workspace-data.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "bun:test"; import { type ComboItem, + COMBO_EFFORTS, buildComboAttention, comboPublicModelId, draftEquals, @@ -171,13 +172,28 @@ describe("combo-workspace-data", () => { )).toEqual(["medium", "high"]); }); - test("intersectComboEfforts treats unknown members as having no selectable efforts", () => { + test("intersectComboEfforts treats unknown members as picker wildcards", () => { const map = new Map([ ["a/m1", ["low", "medium"]], ]); expect(intersectComboEfforts( [{ provider: "a", model: "m1" }, { provider: "b", model: "unknown" }], map, + )).toEqual(["low", "medium"]); + expect(intersectComboEfforts( + [{ provider: "b", model: "unknown" }], + map, + )).toEqual(COMBO_EFFORTS); + }); + + test("intersectComboEfforts keeps an advertised empty ladder restrictive", () => { + const map = new Map([ + ["a/m1", ["low", "medium"]], + ["b/no-reasoning", []], + ]); + expect(intersectComboEfforts( + [{ provider: "a", model: "m1" }, { provider: "b", model: "no-reasoning" }], + map, )).toEqual([]); }); diff --git a/tests/combos.test.ts b/tests/combos.test.ts index f2705d6437..b9b7d1ef35 100644 --- a/tests/combos.test.ts +++ b/tests/combos.test.ts @@ -235,7 +235,7 @@ describe("combo request cloning", () => { expect(concreteComboRequestBody({ model: "combo/x" }, target, "high", ["low", "medium"]).reasoning).toBeUndefined(); }); - test("debug-warns once per unsupported combo default", () => { + test("debug-warns once per unsupported or unknown combo default", () => { const debug = spyOn(console, "debug").mockImplementation(() => {}); concreteComboRequestBody({ model: "combo/x" }, target, "high", []); concreteComboRequestBody({ model: "combo/x" }, target, "high", []); @@ -246,6 +246,13 @@ describe("combo request cloning", () => { requestedEffort: "high", capability: "unsupported", }); + concreteComboRequestBody({ model: "combo/x" }, target, "medium", undefined); + concreteComboRequestBody({ model: "combo/x" }, target, "medium", undefined); + expect(debug).toHaveBeenCalledTimes(2); + expect(debug.mock.calls[1]?.[1]).toMatchObject({ + requestedEffort: "medium", + capability: "unknown", + }); debug.mockRestore(); }); }); diff --git a/tests/management-client-config-route.test.ts b/tests/management-client-config-route.test.ts index c5fa46912b..1e20c446ee 100644 --- a/tests/management-client-config-route.test.ts +++ b/tests/management-client-config-route.test.ts @@ -4,8 +4,7 @@ import { OPENCODE_API_KEY_ENV, OPENCODE_CONFIG_SCHEMA, OPENCODE_PROVIDER_ID, - PI_API_KEY_ENV, - PI_API_KEY_ENV_REF, + LOOPBACK_API_KEY_PLACEHOLDER, buildClientConfig, normalizeExportModels, opencodeGlobalConfigPath, @@ -149,11 +148,11 @@ describe("GET /api/client-config", () => { expect(body.client).toBe("pi"); expect(body.filename).toBe("pi-models.json"); - expect(body.apiKeyEnv).toBe(PI_API_KEY_ENV); + expect(body.apiKeyEnv).toBe(""); const provider = (body.config as PiGeneratedConfig).providers[OPENCODE_PROVIDER_ID]; expect(Array.isArray(provider.models)).toBe(true); - expect(provider.apiKey).toBe(PI_API_KEY_ENV_REF); + expect(provider.apiKey).toBe(LOOPBACK_API_KEY_PLACEHOLDER); expect(provider.baseUrl).toBe("http://127.0.0.1:10100/v1"); expect(provider.models.map(model => model.id)).toContain("a/m1"); }, 15_000);