From fd1fd2f802157ff5f50f11ffe387855defc5c13f Mon Sep 17 00:00:00 2001 From: mptyl Date: Wed, 5 Aug 2026 06:30:25 +0200 Subject: [PATCH] fix: refresh Pi credential readiness --- backend/src/pi/management.ts | 2 +- backend/src/pi/provider-credentials.ts | 11 ++++- backend/test/provider-credentials.test.ts | 59 ++++++++++++++++++++++- frontend/src/shell/PiManagement.test.tsx | 53 ++++++++++++++++++-- frontend/src/shell/PiManagement.tsx | 7 +-- 5 files changed, 117 insertions(+), 15 deletions(-) diff --git a/backend/src/pi/management.ts b/backend/src/pi/management.ts index ccba2e5b..114db34f 100644 --- a/backend/src/pi/management.ts +++ b/backend/src/pi/management.ts @@ -117,7 +117,7 @@ export function createPiManagement(config: AppConfig, deps: PiManagementDeps): P return piProviderCredentialStatus({ provider, authProviders: loadPiAuthProviders(), - credentialValue: secretValue(config, "THT_MODEL_API_KEY"), + resolveCredentialValue: () => secretValue(config, "THT_MODEL_API_KEY"), credentialFile: config.modelApiKeyFile, }); } catch { diff --git a/backend/src/pi/provider-credentials.ts b/backend/src/pi/provider-credentials.ts index 6e2aab5b..a01d6669 100644 --- a/backend/src/pi/provider-credentials.ts +++ b/backend/src/pi/provider-credentials.ts @@ -179,7 +179,7 @@ export function buildPiChildEnv(opts: { export function piProviderCredentialStatus(opts: { provider?: string; credentialFile?: string; - credentialValue?: string; + resolveCredentialValue?: () => string | undefined; authProviders?: ReadonlySet; fsOps?: CredentialFsOps; }): PiCredentialStatus { @@ -187,7 +187,14 @@ export function piProviderCredentialStatus(opts: { if (!provider || LOCAL_PROVIDERS.has(provider)) return "missing"; if (opts.authProviders?.has(provider)) return "present"; try { - buildPiChildEnv({ ...opts, ambient: {} }); + buildPiChildEnv({ + ambient: {}, + provider, + credentialFile: opts.credentialFile, + credentialValue: opts.resolveCredentialValue?.(), + authProviders: opts.authProviders, + fsOps: opts.fsOps, + }); return "present"; } catch { return "missing"; diff --git a/backend/test/provider-credentials.test.ts b/backend/test/provider-credentials.test.ts index 98911e29..a3b06cb9 100644 --- a/backend/test/provider-credentials.test.ts +++ b/backend/test/provider-credentials.test.ts @@ -135,15 +135,70 @@ test("credential status reports only present or missing without treating local p expect(piProviderCredentialStatus({ provider: "deepseek", authProviders: new Set(["deepseek"]), - credentialValue: "must-not-be-returned", + resolveCredentialValue: () => "must-not-be-returned", })).toBe("present"); expect(piProviderCredentialStatus({ provider: "deepseek" })).toBe("missing"); expect(piProviderCredentialStatus({ provider: "local-qwen", - credentialValue: "must-not-be-returned", + resolveCredentialValue: () => "must-not-be-returned", })).toBe("missing"); }); +// Catches the generic managed secret being read before providers that self-authenticate or need +// no credential have been classified. +test("credential status never resolves the generic secret for auth-store or local providers", () => { + let secretReads = 0; + const unreadableSecret = () => { + secretReads += 1; + throw new Error("unrelated generic secret is unreadable"); + }; + + expect(piProviderCredentialStatus({ + provider: "deepseek", + authProviders: new Set(["deepseek"]), + resolveCredentialValue: unreadableSecret, + })).toBe("present"); + expect(piProviderCredentialStatus({ + provider: "local-qwen", + resolveCredentialValue: unreadableSecret, + })).toBe("missing"); + expect(secretReads).toBe(0); +}); + +// Catches generic hosted providers skipping their managed-secret source or treating an absent or +// unreadable source as credentialed. +test("credential status resolves the generic secret only for providers that require it", () => { + let presentReads = 0; + expect(piProviderCredentialStatus({ + provider: "openai", + resolveCredentialValue: () => { + presentReads += 1; + return "managed-openai-key"; + }, + })).toBe("present"); + expect(presentReads).toBe(1); + + let missingReads = 0; + expect(piProviderCredentialStatus({ + provider: "openai", + resolveCredentialValue: () => { + missingReads += 1; + return undefined; + }, + })).toBe("missing"); + expect(missingReads).toBe(1); + + let unreadableReads = 0; + expect(piProviderCredentialStatus({ + provider: "openai", + resolveCredentialValue: () => { + unreadableReads += 1; + throw new Error("generic secret is unreadable"); + }, + })).toBe("missing"); + expect(unreadableReads).toBe(1); +}); + test("bundle value is injected without exposing bundle metadata to Pi", () => { const env = buildPiChildEnv({ ambient: { diff --git a/frontend/src/shell/PiManagement.test.tsx b/frontend/src/shell/PiManagement.test.tsx index 4bf01e3c..34d22e1c 100644 --- a/frontend/src/shell/PiManagement.test.tsx +++ b/frontend/src/shell/PiManagement.test.tsx @@ -81,6 +81,38 @@ test("saves only a selected non-secret configuration", async () => { expect(screen.getByRole("status", { name: "Pi management feedback" })).toHaveTextContent("Defaults saved"); }); +// Catches a provider switch updating only the saved config while leaving the credential rail +// attached to the previously selected provider. +test("shows authoritative credential presence after saving a different provider", async () => { + const user = userEvent.setup(); + let saved = false; + server.use( + http.get("/api/pi-management/status", () => HttpResponse.json(saved ? { + ...readyStatus, + credentials: "missing", + config: { provider: "deepseek", model: "deepseek-v4", reasoning: "medium" }, + checkedAt: "2026-08-05T10:02:00.000Z", + } : readyStatus)), + http.put("/api/pi-management/config", async ({ request }) => { + saved = true; + return HttpResponse.json({ + ...await request.json() as object, + updatedAt: "2026-08-05T10:01:00.000Z", + }); + }), + ); + renderManagement(); + + expect(await screen.findByText("Credentials present")).toBeVisible(); + await user.selectOptions(screen.getByLabelText("Provider"), "deepseek"); + await user.click(screen.getByRole("button", { name: "Save defaults" })); + + expect(await screen.findByText("Credentials missing")).toBeVisible(); + expect(screen.queryByText("Credentials present")).not.toBeInTheDocument(); + expect(screen.getByLabelText("Provider")).toHaveValue("deepseek"); + expect(screen.getByRole("status", { name: "Pi management feedback" })).toHaveTextContent("Defaults saved"); +}); + test("runs the saved-configuration test without changing credential presence", async () => { const user = userEvent.setup(); let tests = 0; @@ -196,17 +228,28 @@ test("shows an explicit recoverable incomplete state when no provider model is a }); test("keeps suggested draft choices distinct from invalid persisted defaults until save succeeds", async () => { + let configured = false; const persisted = { ...readyStatus, credentials: "missing", config: { provider: "retired", model: "old-model", reasoning: "medium" }, }; server.use( - http.get("/api/pi-management/status", () => HttpResponse.json(persisted)), - http.put("/api/pi-management/config", async ({ request }) => HttpResponse.json({ - ...await request.json() as object, - updatedAt: "2026-08-05T10:05:00.000Z", - })), + http.get("/api/pi-management/status", () => HttpResponse.json(configured ? { + ...readyStatus, + credentials: "missing", + config: { provider: "zai", model: "glm-5.2", reasoning: "medium" }, + checkedAt: "2026-08-05T10:05:00.000Z", + } : persisted)), + http.put("/api/pi-management/config", () => { + configured = true; + return HttpResponse.json({ + provider: "zai", + model: "glm-5.2", + reasoning: "medium", + updatedAt: "2026-08-05T10:05:00.000Z", + }); + }), ); const user = userEvent.setup(); renderManagement(); diff --git a/frontend/src/shell/PiManagement.tsx b/frontend/src/shell/PiManagement.tsx index ea3f1da8..dd5a7816 100644 --- a/frontend/src/shell/PiManagement.tsx +++ b/frontend/src/shell/PiManagement.tsx @@ -10,7 +10,6 @@ import { savePiManagementConfig, type PiInstallationConfig, type PiManagementOptions, - type PiManagementStatus, } from "../api/pi-management"; import { Button } from "../components/ui/button"; import { Dialog, DialogContent, DialogDescription, DialogHeader, DialogTitle } from "../components/ui/dialog"; @@ -143,13 +142,11 @@ export function PiManagement({ open, onClose }: { open: boolean; onClose: () => const saveMutation = useMutation({ mutationFn: savePiManagementConfig, - onSuccess: (saved) => { + onSuccess: async (saved) => { const config = { provider: saved.provider, model: saved.model, reasoning: saved.reasoning }; - queryClient.setQueryData(["pi-management", "status"], (current) => ( - current ? { ...current, config } : current - )); setDraft(config); setSmokeState(undefined); + await queryClient.invalidateQueries({ queryKey: ["pi-management", "status"] }); setFeedback({ tone: "success", message: "Defaults saved." }); }, onError: (error) => setFeedback({ tone: "error", message: errorMessage(error, "Could not save Pi defaults.") }),