From 1cbca36d2c18975144df0d0b44770ca50543c704 Mon Sep 17 00:00:00 2001 From: Colin Francis Date: Thu, 16 Jul 2026 19:06:34 -0700 Subject: [PATCH 1/9] rework openwiki --init checklist status and layout --- src/credentials.tsx | 480 +++++++++++++++++++++------------------ test/credentials.test.ts | 35 ++- 2 files changed, 291 insertions(+), 224 deletions(-) diff --git a/src/credentials.tsx b/src/credentials.tsx index 77718adc..8afef35b 100644 --- a/src/credentials.tsx +++ b/src/credentials.tsx @@ -59,7 +59,6 @@ import { isOpenWikiOnboardingCompleteSync, isOnboardingComplete, isRepositoryCodeOnboardingCompleteSync, - openWikiOnboardingPath, readOpenWikiOnboardingConfig, readRepositoryWikiInstructions, saveRepositoryWikiInstructions, @@ -554,6 +553,9 @@ export function InitSetup({ const [gcpLocation, setGcpLocation] = useState(null); const [modelId, setModelId] = useState(null); const [langSmithKey, setLangSmithKey] = useState(null); + // True once the user confirms a provider this session. Provider always holds a + // default value, so a null-check cannot detect the in-session choice. + const [providerConfirmed, setProviderConfirmed] = useState(false); const [input, setInput] = useState(""); const [onboardingConfig, setOnboardingConfig] = useState(() => createEmptyOnboardingConfig()); @@ -1157,6 +1159,7 @@ export function InitSetup({ DEFAULT_PROVIDER; setProvider(selectedProvider); + setProviderConfirmed(true); setProviderSelectionIndex(getProviderSelectionIndex(selectedProvider)); setModelSelectionIndex( getModelSelectionIndex( @@ -2254,220 +2257,221 @@ export function InitSetup({ - - - {providerUsesOAuth(provider) || apiKeyEnvKey ? ( - - ) : null} - {providerRequiresSecretKey(provider) ? ( - - ) : null} - {projectEnvKey ? ( - - ) : null} - {projectEnvKey && locationEnvKey ? ( + + Detected from your command + - ) : null} - {providerRequiresBaseUrl(provider) ? ( - - ) : null} - {providerRequiresRegion(provider) ? ( + {selectedMode === "code" ? ( + + ) : null} + + + + + Set up + - ) : null} - - - - {selectedMode === "personal" ? ( + {providerUsesOAuth(provider) || apiKeyEnvKey ? ( + + ) : null} + {providerRequiresSecretKey(provider) ? ( + + ) : null} + {projectEnvKey ? ( + + ) : null} + {projectEnvKey && locationEnvKey ? ( + + ) : null} + {providerRequiresBaseUrl(provider) ? ( + + ) : null} + {providerRequiresRegion(provider) ? ( + + ) : null} - ) : null} - - {selectedMode === "personal" ? ( - ) : null} - {selectedMode === "personal" ? ( - 0 - ? "done" - : isSourceStep(step) - ? "current" - : "pending" + langSmithKey !== null + ? langSmithKey.length > 0 + ? "configured" + : "skipped" + : process.env.LANGSMITH_API_KEY !== undefined + ? "configured" + : "not set" } - detail={`${getConnectedSourceCount( - onboardingConfig, - activeSourceOptions, - )} setup(s) configured`} /> - ) : null} + {selectedMode === "personal" ? ( + + ) : null} + {selectedMode === "personal" ? ( + + ) : null} + {selectedMode === "personal" ? ( + + ) : null} + {selectedMode === "personal" ? ( + + 0 + ? "done" + : isSourceStep(step) + ? "current" + : "pending" + } + detail={`${getConnectedSourceCount( + onboardingConfig, + activeSourceOptions, + )} configured`} + /> + ) : null} + {step === "oauth-login" ? ( @@ -3216,6 +3220,48 @@ function SetupHeader() { ); } +type SetupStepState = "current" | "done" | "optional" | "pending"; + +/** + * Resolve a checklist row's status. The active step wins, so navigating back to + * an already-done step shows the current-row cursor rather than a check; a done + * step reads done; anything else falls to its resting status. + */ +export function resolveStepStatus( + id: PromptStep, + activeStep: PromptStep | null, + done: boolean, + resting: "optional" | "pending" = "pending", +): SetupStepState { + if (id === activeStep) { + return "current"; + } + if (done) { + return "done"; + } + return resting; +} + +/** + * Progress glyph per status: a check for done, an arrow for the active row, a + * hollow circle for not-started (and optional). Single cell wide so every row's + * label column lines up without padding the marker. + */ +const STEP_GLYPH: Record = { + done: "✓", + current: "❯", + optional: "○", + pending: "○", +}; + +/** Color per status. Optionality is conveyed by the detail text, not the glyph. */ +const STEP_COLOR: Record = { + done: "green", + current: "cyan", + optional: "gray", + pending: "gray", +}; + function SetupStep({ detail, label, @@ -3223,21 +3269,15 @@ function SetupStep({ }: { detail: string; label: string; - state: "current" | "done" | "optional" | "pending"; + state: SetupStepState; }) { - const color = - state === "done" - ? "green" - : state === "current" - ? "yellow" - : state === "optional" - ? "cyan" - : "gray"; - return ( - [{state.toUpperCase()}]{" "} - {label.padEnd(16)} {detail} + {STEP_GLYPH[state]}{" "} + + {label.padEnd(16)} + {" "} + {detail} ); } @@ -3487,7 +3527,9 @@ function InputValueWithCursor({ } function formatSecretInputDisplay(value: string): string { - return value.length === 0 ? "empty" : `hidden (${value.length} chars)`; + // Empty renders as nothing (just the cursor); dots for the entered length, + // matching the non-secret inputs rather than printing a literal "empty". + return "•".repeat(value.length); } function formatTerminalHyperlink(url: string, label: string): string { @@ -3918,14 +3960,6 @@ function isScheduleStep(step: PromptStep | null): boolean { return Boolean(step?.startsWith("global-")); } -function getProviderSetupDetail(provider: OpenWikiProvider): string { - if (hasValidConfiguredProvider()) { - return getProviderLabel(provider); - } - - return `default ${getProviderLabel(DEFAULT_PROVIDER)}`; -} - /** * Label for the provider's primary credential input. Bedrock authenticates * with an IAM access key ID (paired with a secret access key), not a single diff --git a/test/credentials.test.ts b/test/credentials.test.ts index d5ef68df..f6147967 100644 --- a/test/credentials.test.ts +++ b/test/credentials.test.ts @@ -1,5 +1,8 @@ import { afterEach, describe, expect, test } from "vitest"; -import { needsCredentialSetup } from "../src/credentials.tsx"; +import { + needsCredentialSetup, + resolveStepStatus, +} from "../src/credentials.tsx"; const ENV_KEYS = [ "LANGSMITH_API_KEY", @@ -34,3 +37,33 @@ describe("needsCredentialSetup", () => { expect(needsCredentialSetup()).toBe(true); }); }); + +describe("resolveStepStatus", () => { + test("the active step is current, even when it is also done", () => { + expect(resolveStepStatus("model", "model", true)).toBe("current"); + expect(resolveStepStatus("model", "model", false)).toBe("current"); + }); + + test("a completed, non-active step reads done", () => { + expect(resolveStepStatus("model", "provider", true)).toBe("done"); + expect(resolveStepStatus("model", null, true)).toBe("done"); + }); + + test("an unstarted, non-active step falls to its resting status", () => { + expect(resolveStepStatus("model", "provider", false)).toBe("pending"); + expect(resolveStepStatus("model", null, false)).toBe("pending"); + expect(resolveStepStatus("langsmith", "provider", false, "optional")).toBe( + "optional", + ); + }); + + test("ordering: active beats done, done beats resting", () => { + // Active wins even over a done step (the cursor shows where you are when + // you step back onto a completed row). + expect(resolveStepStatus("model", "model", true)).toBe("current"); + // Done wins over an optional resting status. + expect(resolveStepStatus("langsmith", "model", true, "optional")).toBe( + "done", + ); + }); +}); From f07c1dc9841171e369871aa61f87d63c42b32e6e Mon Sep 17 00:00:00 2001 From: Colin Francis Date: Thu, 16 Jul 2026 21:00:31 -0700 Subject: [PATCH 2/9] let init setup navigate backward with Esc and repopulate fields --- src/credentials.tsx | 400 +++++++++++++++++++++++++++++++-------- test/credentials.test.ts | 57 ++++++ 2 files changed, 383 insertions(+), 74 deletions(-) diff --git a/src/credentials.tsx b/src/credentials.tsx index 8afef35b..0262ceea 100644 --- a/src/credentials.tsx +++ b/src/credentials.tsx @@ -413,6 +413,96 @@ function credentialStep(provider: OpenWikiProvider): PromptStep { return providerRequiresApiKey(provider) ? "api-key" : "gcp-project"; } +/** + * The setup steps that apply to a provider and run mode, in the order the wizard + * walks them. Unlike the skip-based waterfall in {@link getInitialStep}, this + * includes steps already satisfied by the environment, so navigation can reach + * and re-edit an auto-skipped step. The provider's primary credential step + * ({@link credentialStep}) is emitted once; for keyless providers that step is + * the GCP project, so it is not appended again below. + */ +export function orderedSetupSteps( + provider: OpenWikiProvider, + mode: OpenWikiRunMode, + allowModeSelection: boolean, +): PromptStep[] { + const steps: PromptStep[] = []; + + if (allowModeSelection) { + steps.push("run-mode"); + } + + steps.push("provider"); + + const primary = credentialStep(provider); + steps.push(primary); + + if (providerRequiresSecretKey(provider)) { + steps.push("secret-key"); + } + if (getProviderProjectEnvKey(provider) && primary !== "gcp-project") { + steps.push("gcp-project"); + } + if ( + getProviderProjectEnvKey(provider) && + getProviderLocationEnvKey(provider) + ) { + steps.push("gcp-location"); + } + if (providerRequiresBaseUrl(provider)) { + steps.push("base-url"); + } + if (providerRequiresRegion(provider)) { + steps.push("region"); + } + + steps.push("model"); + steps.push("langsmith"); + + steps.push(mode === "code" ? "code-repo-confirm" : "template"); + + return steps; +} + +/** + * The step before `step` in the applicable spine, or null when `step` is the + * first step or is outside the spine (for example the personal-mode source + * sub-flow, which manages its own navigation). + */ +export function previousSpineStep( + step: PromptStep | null, + provider: OpenWikiProvider, + mode: OpenWikiRunMode, + allowModeSelection: boolean, +): PromptStep | null { + if (step === null) { + return null; + } + const spine = orderedSetupSteps(provider, mode, allowModeSelection); + const index = spine.indexOf(step); + return index > 0 ? spine[index - 1] : null; +} + +/** + * The step after `step` in the applicable spine, or null when `step` is the last + * spine step or outside it. Drives forward navigation: Enter advances to the + * next applicable step in order rather than skipping ones already satisfied by + * the environment, so setup reads as a sequential walk. + */ +export function nextSetupStep( + step: PromptStep | null, + provider: OpenWikiProvider, + mode: OpenWikiRunMode, + allowModeSelection: boolean, +): PromptStep | null { + if (step === null) { + return null; + } + const spine = orderedSetupSteps(provider, mode, allowModeSelection); + const index = spine.indexOf(step); + return index >= 0 && index + 1 < spine.length ? spine[index + 1] : null; +} + function hasValidStoredToken(env: NodeJS.ProcessEnv = process.env): boolean { const tokens = readCodexTokensFromEnv(env); @@ -759,18 +849,26 @@ export function InitSetup({ setOauthTokens(tokens); setIsLoggingIn(false); - const nextStep = getNextStepAfterApiKey( - provider, - modelIdOverride, - onboardingConfig, - selectedMode, - forceModelStep, - ); + const nextStep = + nextSetupStep( + "oauth-login", + provider, + selectedMode, + allowModeSelection, + ) ?? + getNextStepAfterApiKey( + provider, + modelIdOverride, + onboardingConfig, + selectedMode, + forceModelStep, + ); if (nextStep) { setIsCustomModelInput( nextStep === "model" && shouldStartWithCustomModelInput(provider), ); + seedInputForStep(nextStep); setStep(nextStep); return; } @@ -803,6 +901,97 @@ export function InitSetup({ }; }, [step, loginAttempt]); + /** + * Pre-fill the input or selection for a step reached via navigation, so a done + * step opens ready to edit. Secret steps are pre-filled with the stored key, + * which renders as dots (formatSecretInputDisplay), never raw. + */ + function seedInputForStep(target: PromptStep): void { + switch (target) { + case "provider": + setProviderSelectionIndex(getProviderSelectionIndex(provider)); + break; + case "run-mode": + setRunModeSelectionIndex(getRunModeSelectionIndex(selectedMode)); + break; + case "model": + setModelSelectionIndex( + getModelSelectionIndex( + provider, + modelId ?? getDefaultModelId(provider), + ), + ); + // Preset-less providers (e.g. Bedrock) take the model as free text, so + // restore the saved id into the field; selection-based providers drive + // off the index and keep the input empty. + setInput( + shouldStartWithCustomModelInput(provider) ? (modelId ?? "") : "", + ); + break; + case "api-key": { + const envKey = getProviderApiKeyEnvKey(provider); + setInput(apiKey ?? (envKey ? (process.env[envKey] ?? "") : "")); + break; + } + case "secret-key": { + const envKey = getProviderSecretKeyEnvKey(provider); + setInput(secretKey ?? (envKey ? (process.env[envKey] ?? "") : "")); + break; + } + case "base-url": + setInput(baseUrl ?? ""); + break; + case "region": + setInput(region ?? ""); + break; + case "gcp-project": + setInput(gcpProject ?? ""); + break; + case "gcp-location": + setInput(gcpLocation ?? ""); + break; + case "langsmith": + setInput(langSmithKey ?? ""); + break; + default: + setInput(""); + } + } + + /** + * Commit the current step's typed value into state so stepping back with Esc + * preserves it rather than discarding an unsubmitted edit. Only text-input + * steps carry a value here; selection steps commit on their own submit. + */ + function captureInputForStep(from: PromptStep): void { + const trimmed = input.trim(); + switch (from) { + case "api-key": + if (trimmed) setApiKey(trimmed); + break; + case "secret-key": + if (trimmed) setSecretKey(trimmed); + break; + case "base-url": + if (trimmed) setBaseUrl(trimmed); + break; + case "region": + if (trimmed) setRegion(trimmed); + break; + case "gcp-project": + if (trimmed) setGcpProject(trimmed); + break; + case "gcp-location": + if (trimmed) setGcpLocation(trimmed); + break; + case "langsmith": + setLangSmithKey(trimmed); + break; + default: + break; + } + } + useInput((inputValue, key) => { if ( isSaving || @@ -813,6 +1002,26 @@ export function InitSetup({ return; } + // Esc returns to the previous applicable step so no choice is a dead end and + // an auto-skipped step stays reachable. It commits the current field first + // (so an unsubmitted edit is kept) and is a no-op on the first step. + if (key.escape) { + const target = previousSpineStep( + step, + provider, + selectedMode, + allowModeSelection, + ); + if (target !== null) { + captureInputForStep(step); + setStep(target); + seedInputForStep(target); + setError(null); + setNotice(null); + } + return; + } + if (step === "oauth-login") { if ( input.length === 0 && @@ -1105,6 +1314,7 @@ export function InitSetup({ ); if (nextStep) { + seedInputForStep(nextStep); setStep(nextStep); return; } @@ -1171,19 +1381,27 @@ export function InitSetup({ const providerChanged = process.env[OPENWIKI_PROVIDER_ENV_KEY] !== selectedProvider; setForceModelStep(providerChanged); - const nextStep = getNextStepAfterProvider( - selectedProvider, - modelIdOverride, - onboardingConfig, - selectedMode, - providerChanged, - ); + const nextStep = + nextSetupStep( + "provider", + selectedProvider, + selectedMode, + allowModeSelection, + ) ?? + getNextStepAfterProvider( + selectedProvider, + modelIdOverride, + onboardingConfig, + selectedMode, + providerChanged, + ); if (nextStep) { setIsCustomModelInput( nextStep === "model" && shouldStartWithCustomModelInput(selectedProvider), ); + seedInputForStep(nextStep); setStep(nextStep); return; } @@ -1206,34 +1424,42 @@ export function InitSetup({ if (step === "api-key") { const trimmedInput = input.trim(); + // Empty submit keeps an existing key (session or env); only a genuinely + // missing key is an error. + const nextApiKey = trimmedInput.length > 0 ? trimmedInput : apiKey; - if (trimmedInput.length === 0) { + if (nextApiKey === null && !isCredentialConfigured(provider)) { setError( `${getProviderApiKeyEnvKey(provider) ?? "API key"} is required.`, ); return; } - setApiKey(trimmedInput); + if (trimmedInput.length > 0) { + setApiKey(trimmedInput); + } setInput(""); - const nextStep = getNextStepAfterApiKey( - provider, - modelIdOverride, - onboardingConfig, - selectedMode, - forceModelStep, - ); + const nextStep = + nextSetupStep("api-key", provider, selectedMode, allowModeSelection) ?? + getNextStepAfterApiKey( + provider, + modelIdOverride, + onboardingConfig, + selectedMode, + forceModelStep, + ); if (nextStep) { setIsCustomModelInput( nextStep === "model" && shouldStartWithCustomModelInput(provider), ); + seedInputForStep(nextStep); setStep(nextStep); return; } await completeSetup({ - nextApiKey: trimmedInput, + nextApiKey, nextBaseUrl: baseUrl, nextSecretKey: secretKey, nextRegion: region, @@ -1250,28 +1476,40 @@ export function InitSetup({ if (step === "secret-key") { const trimmedInput = input.trim(); + // Empty submit keeps an existing secret key (see the api-key step). + const nextSecretKey = trimmedInput.length > 0 ? trimmedInput : secretKey; - if (trimmedInput.length === 0) { + if (nextSecretKey === null && !isSecretKeyConfigured(provider)) { setError( `${getProviderSecretKeyEnvKey(provider) ?? "Secret key"} is required.`, ); return; } - setSecretKey(trimmedInput); + if (trimmedInput.length > 0) { + setSecretKey(trimmedInput); + } setInput(""); - const nextStep = getNextStepAfterSecretKey( - provider, - modelIdOverride, - onboardingConfig, - selectedMode, - forceModelStep, - ); + const nextStep = + nextSetupStep( + "secret-key", + provider, + selectedMode, + allowModeSelection, + ) ?? + getNextStepAfterSecretKey( + provider, + modelIdOverride, + onboardingConfig, + selectedMode, + forceModelStep, + ); if (nextStep) { setIsCustomModelInput( nextStep === "model" && shouldStartWithCustomModelInput(provider), ); + seedInputForStep(nextStep); setStep(nextStep); return; } @@ -1279,7 +1517,7 @@ export function InitSetup({ await completeSetup({ nextApiKey: apiKey, nextBaseUrl: baseUrl, - nextSecretKey: trimmedInput, + nextSecretKey, nextRegion: region, nextGcpLocation: gcpLocation, nextGcpProject: gcpProject, @@ -1304,18 +1542,21 @@ export function InitSetup({ setRegion(trimmedInput); setInput(""); - const nextStep = getNextStepAfterRegion( - provider, - modelIdOverride, - onboardingConfig, - selectedMode, - forceModelStep, - ); + const nextStep = + nextSetupStep("region", provider, selectedMode, allowModeSelection) ?? + getNextStepAfterRegion( + provider, + modelIdOverride, + onboardingConfig, + selectedMode, + forceModelStep, + ); if (nextStep) { setIsCustomModelInput( nextStep === "model" && shouldStartWithCustomModelInput(provider), ); + seedInputForStep(nextStep); setStep(nextStep); return; } @@ -1353,6 +1594,9 @@ export function InitSetup({ setGcpProject(trimmedInput); setInput(""); + // gcp-location always follows gcp-project (gemini-enterprise); seed it so a + // previously entered location is restored instead of arriving blank. + seedInputForStep("gcp-location"); setStep("gcp-location"); return; } @@ -1371,18 +1615,26 @@ export function InitSetup({ setGcpLocation(nextGcpLocation); setInput(""); - const nextStep = getNextStepAfterGcpLocation( - provider, - modelIdOverride, - onboardingConfig, - selectedMode, - forceModelStep, - ); + const nextStep = + nextSetupStep( + "gcp-location", + provider, + selectedMode, + allowModeSelection, + ) ?? + getNextStepAfterGcpLocation( + provider, + modelIdOverride, + onboardingConfig, + selectedMode, + forceModelStep, + ); if (nextStep) { setIsCustomModelInput( nextStep === "model" && shouldStartWithCustomModelInput(provider), ); + seedInputForStep(nextStep); setStep(nextStep); return; } @@ -1420,18 +1672,21 @@ export function InitSetup({ setBaseUrl(trimmedInput); setInput(""); - const nextStep = getNextStepAfterBaseUrl( - provider, - modelIdOverride, - onboardingConfig, - selectedMode, - forceModelStep, - ); + const nextStep = + nextSetupStep("base-url", provider, selectedMode, allowModeSelection) ?? + getNextStepAfterBaseUrl( + provider, + modelIdOverride, + onboardingConfig, + selectedMode, + forceModelStep, + ); if (nextStep) { setIsCustomModelInput( nextStep === "model" && shouldStartWithCustomModelInput(provider), ); + seedInputForStep(nextStep); setStep(nextStep); return; } @@ -1475,24 +1730,10 @@ export function InitSetup({ setInput(""); setIsCustomModelInput(false); - if (process.env.LANGSMITH_API_KEY === undefined) { - setStep("langsmith"); - return; - } - - await continueAfterCredentials({ - nextApiKey: apiKey, - nextBaseUrl: baseUrl, - nextSecretKey: secretKey, - nextRegion: region, - nextGcpLocation: gcpLocation, - nextGcpProject: gcpProject, - nextLangSmithKey: langSmithKey, - nextModelId: selectedModelId, - nextOAuthTokens: oauthTokens, - nextProvider: provider, - runMode: selectedMode, - }); + // Sequential: always visit LangSmith next (the next spine step). Seed it + // from state so a key entered earlier and stepped past is not dropped. + seedInputForStep("langsmith"); + setStep("langsmith"); return; } @@ -2519,8 +2760,19 @@ export function InitSetup({ )} + {previousSpineStep(step, provider, selectedMode, allowModeSelection) !== + null ? ( + + esc to go back + + ) : null} + {needsCredentialPrompt ? ( - Secrets are masked and saved only after setup. + + + Secrets are masked and saved only after setup. + + ) : null} {notice ? ( diff --git a/test/credentials.test.ts b/test/credentials.test.ts index f6147967..2a8cf050 100644 --- a/test/credentials.test.ts +++ b/test/credentials.test.ts @@ -1,6 +1,9 @@ import { afterEach, describe, expect, test } from "vitest"; import { needsCredentialSetup, + nextSetupStep, + orderedSetupSteps, + previousSpineStep, resolveStepStatus, } from "../src/credentials.tsx"; @@ -67,3 +70,57 @@ describe("resolveStepStatus", () => { ); }); }); + +describe("orderedSetupSteps", () => { + test("openai (code mode): provider, key, model, langsmith, then the tail", () => { + expect(orderedSetupSteps("openai", "code", false)).toEqual([ + "provider", + "api-key", + "model", + "langsmith", + "code-repo-confirm", + ]); + }); + + test("run-mode is first only when mode selection is allowed", () => { + expect(orderedSetupSteps("openai", "code", true)[0]).toBe("run-mode"); + expect(orderedSetupSteps("openai", "code", false)).not.toContain( + "run-mode", + ); + }); + + test("bedrock adds secret-key and region before model", () => { + const spine = orderedSetupSteps("bedrock", "code", false); + expect(spine).toContain("secret-key"); + expect(spine).toContain("region"); + expect(spine.indexOf("secret-key")).toBeLessThan(spine.indexOf("model")); + expect(spine.indexOf("region")).toBeLessThan(spine.indexOf("model")); + }); + + test("personal mode ends at the template tail, not code-repo-confirm", () => { + const spine = orderedSetupSteps("openai", "personal", false); + expect(spine[spine.length - 1]).toBe("template"); + }); + + test("the spine includes applicable steps regardless of env (reachability)", () => { + // api-key is present even when a key is already set, so navigation can + // still reach and re-edit it. + expect(orderedSetupSteps("openai", "code", false)).toContain("api-key"); + }); +}); + +describe("previousSpineStep and nextSetupStep", () => { + test("walk backward and forward through the spine", () => { + expect(previousSpineStep("model", "openai", "code", false)).toBe("api-key"); + expect(nextSetupStep("api-key", "openai", "code", false)).toBe("model"); + }); + + test("no-op at the ends and for null", () => { + expect(previousSpineStep("provider", "openai", "code", false)).toBe(null); + expect(nextSetupStep("code-repo-confirm", "openai", "code", false)).toBe( + null, + ); + expect(previousSpineStep(null, "openai", "code", false)).toBe(null); + expect(nextSetupStep(null, "openai", "code", false)).toBe(null); + }); +}); From f7cebd8a2914b6160adc57544b671eb850a7d4f9 Mon Sep 17 00:00:00 2001 From: Colin Francis Date: Thu, 16 Jul 2026 21:34:59 -0700 Subject: [PATCH 3/9] surface shell-env precedence over saved config in init --- src/credentials.tsx | 57 ++++++++++++++++++++++++++++++++++++--- src/env.ts | 53 +++++++++++++++++++++++++++++++++++- test/env-behavior.test.ts | 36 +++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 5 deletions(-) diff --git a/src/credentials.tsx b/src/credentials.tsx index 0262ceea..af0eb772 100644 --- a/src/credentials.tsx +++ b/src/credentials.tsx @@ -53,7 +53,7 @@ import type { AuthProviderId } from "./auth/types.js"; import type { OpenWikiRunMode } from "./commands.js"; import type { ConnectorId } from "./connectors/types.js"; import { getConnectorConfigPath } from "./openwiki-home.js"; -import { openWikiEnvPath, saveOpenWikiEnv } from "./env.js"; +import { getShellEnvValue, openWikiEnvPath, saveOpenWikiEnv } from "./env.js"; import { createEmptyOnboardingConfig, isOpenWikiOnboardingCompleteSync, @@ -413,6 +413,27 @@ function credentialStep(provider: OpenWikiProvider): PromptStep { return providerRequiresApiKey(provider) ? "api-key" : "gcp-project"; } +/** + * Every managed env key the wizard lets you set for a provider, in checklist + * order: the provider selection, its credential keys, the model, and the + * LangSmith tracing key. Used to detect which of them a shell export is + * currently shadowing (a shell var wins at runtime and would silently override + * the choice made here). Returns key names only, never values. + */ +function getWizardManagedEnvKeys(provider: OpenWikiProvider): string[] { + return [ + OPENWIKI_PROVIDER_ENV_KEY, + getProviderApiKeyEnvKey(provider), + getProviderSecretKeyEnvKey(provider), + getProviderProjectEnvKey(provider), + getProviderLocationEnvKey(provider), + getProviderBaseUrlEnvKey(provider), + getProviderRegionEnvKey(provider), + OPENWIKI_MODEL_ID_ENV_KEY, + "LANGSMITH_API_KEY", + ].filter((key): key is string => key !== undefined); +} + /** * The setup steps that apply to a provider and run mode, in the order the wizard * walks them. Unlike the skip-based waterfall in {@link getInitialStep}, this @@ -2494,10 +2515,36 @@ export function InitSetup({ const projectEnvKey = getProviderProjectEnvKey(provider); const locationEnvKey = getProviderLocationEnvKey(provider); + // A shell export wins over saved config at runtime. List any wizard-managed + // keys present in the shell so their precedence is not a surprise and the + // "from shell" rows below are explained. Presence only, not a value compare; + // key names only, never values. + const shadowedShellKeys = getWizardManagedEnvKeys(provider).filter( + (key) => getShellEnvValue(key) !== undefined, + ); + const isSingleShadow = shadowedShellKeys.length === 1; + const shadowedShellWarning = + shadowedShellKeys.length === 0 + ? null + : `${ + isSingleShadow ? "This key was" : "These keys were" + } detected in your shell and ${ + isSingleShadow ? "overrides" : "override" + } saved config: ${shadowedShellKeys.join(", ")}. Runs use the shell ` + + `value${isSingleShadow ? "" : "s"}; unset ${ + isSingleShadow ? "it" : "them" + } to use your saved config.`; + return ( + {shadowedShellWarning ? ( + + ⚠ {shadowedShellWarning} + + ) : null} + Detected from your command @@ -2545,9 +2592,11 @@ export function InitSetup({ detail={ providerUsesOAuth(provider) ? getCredentialSetupDetail(provider, oauthTokens) - : apiKey !== null || isCredentialConfigured(provider) - ? "configured" - : "not set" + : apiKeyEnvKey && getShellEnvValue(apiKeyEnvKey) !== undefined + ? "from shell" + : apiKey !== null || isCredentialConfigured(provider) + ? "configured" + : "not set" } /> ) : null} diff --git a/src/env.ts b/src/env.ts index c38b422d..5e9b5901 100644 --- a/src/env.ts +++ b/src/env.ts @@ -166,7 +166,52 @@ const managedEnvKeys: readonly string[] = MANAGED_ENV_KEYS; const deprecatedEnvKeys = ["OPENAI_ORG_ID", "OPENAI_PROJECT"]; +/** + * The shell's values for managed credential keys, captured once before any load + * or save wrote to `process.env`. A shell export keeps precedence over + * `~/.openwiki/.env` at runtime, so this snapshot lets the wizard tell the user + * when a value they save will be shadowed, and keeps {@link saveOpenWikiEnv} + * from masking a shell var in-process. Held in memory only; never persisted or + * logged. + */ +let shellEnvAtStartup: Record | undefined; + +/** + * Snapshot the shell's values for managed credential keys. Idempotent: the + * first call wins, so a later load or save cannot capture values OpenWiki + * itself seeded into `process.env`. + */ +function captureShellEnv(): void { + if (shellEnvAtStartup !== undefined) { + return; + } + + const snapshot: Record = {}; + + for (const key of CREDENTIAL_DIAGNOSTIC_ENV_KEYS) { + const value = process.env[key]; + + if (value !== undefined) { + snapshot[key] = value; + } + } + + shellEnvAtStartup = snapshot; +} + +/** + * The shell value for a managed key as captured at startup, or `undefined` when + * the shell did not set it. Reflects the pre-load snapshot, so it stays stable + * even after {@link loadOpenWikiEnv} / {@link saveOpenWikiEnv} mutate + * `process.env`. + */ +export function getShellEnvValue(key: string): string | undefined { + return shellEnvAtStartup?.[key]; +} + export async function loadOpenWikiEnv(): Promise { + captureShellEnv(); + const env = await readOpenWikiEnv(); for (const [key, value] of Object.entries(env)) { @@ -193,6 +238,8 @@ export async function getCredentialDiagnostics(): Promise< } export async function saveOpenWikiEnv(updates: EnvMap): Promise { + captureShellEnv(); + const currentEnv = await readOpenWikiEnv(); const nextEnv = { ...currentEnv, @@ -216,7 +263,11 @@ export async function saveOpenWikiEnv(updates: EnvMap): Promise { await chmod(openWikiEnvPath, 0o600); for (const [key, value] of Object.entries(updates)) { - process.env[key] = value; + // A shell export wins at runtime, so don't mask it in process.env; the + // saved value is only the fallback for when that shell var is unset. + if (shellEnvAtStartup?.[key] === undefined) { + process.env[key] = value; + } } } diff --git a/test/env-behavior.test.ts b/test/env-behavior.test.ts index f4594945..7cb1e0a8 100644 --- a/test/env-behavior.test.ts +++ b/test/env-behavior.test.ts @@ -190,6 +190,42 @@ describe("saveOpenWikiEnv", () => { expect(process.env[OPENAI_API_KEY_ENV_KEY]).toBe("sk-immediate"); }); + + test("does not mask a shell var in process.env, but still writes the file", async () => { + // A shell export present before any load/save wins at runtime, so the save + // must not overwrite it in-process; the saved value is only the fallback. + process.env[OPENROUTER_API_KEY_ENV_KEY] = "from-shell"; + + await env.saveOpenWikiEnv({ [OPENROUTER_API_KEY_ENV_KEY]: "from-wizard" }); + + expect(process.env[OPENROUTER_API_KEY_ENV_KEY]).toBe("from-shell"); + + const contents = await readFile(env.openWikiEnvPath, "utf8"); + expect(contents).toContain('OPENROUTER_API_KEY="from-wizard"'); + }); +}); + +describe("getShellEnvValue", () => { + test("reflects the pre-load shell snapshot, stable across later writes", async () => { + process.env[OPENAI_API_KEY_ENV_KEY] = "shell-key"; + + // The first load/save captures the snapshot. + await env.loadOpenWikiEnv(); + + expect(env.getShellEnvValue(OPENAI_API_KEY_ENV_KEY)).toBe("shell-key"); + + // A key the shell did not set is not in the snapshot, and saving it later + // does not retroactively add it (the snapshot is taken once, up front). + await env.saveOpenWikiEnv({ [OPENROUTER_API_KEY_ENV_KEY]: "saved" }); + + expect(env.getShellEnvValue(OPENROUTER_API_KEY_ENV_KEY)).toBeUndefined(); + }); + + test("is undefined for a key absent from the shell at startup", async () => { + await env.loadOpenWikiEnv(); + + expect(env.getShellEnvValue(OPENAI_API_KEY_ENV_KEY)).toBeUndefined(); + }); }); describe("getCredentialDiagnostics", () => { From fdb5487c76e653a8af168cb0cd6b4c85f1240cd4 Mon Sep 17 00:00:00 2001 From: Colin Francis Date: Thu, 16 Jul 2026 21:45:30 -0700 Subject: [PATCH 4/9] codeql --- src/credentials.tsx | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/credentials.tsx b/src/credentials.tsx index af0eb772..cc6aacc5 100644 --- a/src/credentials.tsx +++ b/src/credentials.tsx @@ -705,6 +705,10 @@ export function InitSetup({ const [codeRepoRoot, setCodeRepoRoot] = useState(() => getDefaultCodeRepoRootPath(), ); + // Dedicated buffer for the code-repo-path field, kept separate from the shared + // `input` (which seedInputForStep prefills with credentials on other steps) so + // a secret never shares the buffer that feeds the thread-id path hash. + const [codeRepoPathInput, setCodeRepoPathInput] = useState(""); const [codeRepoConfirmed, setCodeRepoConfirmed] = useState(false); const [isCustomModelInput, setIsCustomModelInput] = useState(false); const [error, setError] = useState(null); @@ -1263,7 +1267,7 @@ export function InitSetup({ } if (key.backspace || key.delete) { - setInput((value) => value.slice(0, -1)); + setCodeRepoPathInput((value) => value.slice(0, -1)); return; } @@ -1271,7 +1275,7 @@ export function InitSetup({ if (sanitizedInput && !key.ctrl && !key.meta) { setError(null); - setInput((value) => value + sanitizedInput); + setCodeRepoPathInput((value) => value + sanitizedInput); } return; @@ -1361,7 +1365,7 @@ export function InitSetup({ CODE_REPO_OPTIONS[codeRepoSelectionIndex] ?? CODE_REPO_OPTIONS[0]; if (selectedOption === "Edit path") { - setInput(codeRepoRoot); + setCodeRepoPathInput(codeRepoRoot); setStep("code-repo-path"); return; } @@ -1373,10 +1377,10 @@ export function InitSetup({ if (step === "code-repo-path") { try { - const repoRoot = await validateLocalDirectoryPath(input); + const repoRoot = await validateLocalDirectoryPath(codeRepoPathInput); setCodeRepoRoot(repoRoot); setCodeRepoConfirmed(true); - setInput(""); + setCodeRepoPathInput(""); continueAfterCodeRepoConfirmed(repoRoot); } catch (pathError) { setError(getErrorMessage(pathError)); @@ -2776,6 +2780,7 @@ export function InitSetup({ {step ? ( Press Enter to confirm this path. From 1ad3ac8a83610e18389c84ed9d928c46a0364261 Mon Sep 17 00:00:00 2001 From: Colin Francis Date: Thu, 16 Jul 2026 22:19:44 -0700 Subject: [PATCH 5/9] replace the credential dump on failure with a how-to-fix panel --- src/cli.tsx | 135 ++++++++++++++++++++++++++++++++++++++- src/commands.ts | 12 ++++ src/diagnostics.ts | 24 +++++++ test/commands.test.ts | 22 +++++++ test/diagnostics.test.ts | 43 +++++++++++++ 5 files changed, 235 insertions(+), 1 deletion(-) create mode 100644 test/diagnostics.test.ts diff --git a/src/cli.tsx b/src/cli.tsx index 6d03d72f..b0fe061c 100644 --- a/src/cli.tsx +++ b/src/cli.tsx @@ -28,6 +28,7 @@ import { } from "./credentials.js"; import { getCredentialDiagnostics, + getShellEnvValue, loadOpenWikiEnv, saveOpenWikiEnv, type CredentialDiagnostic, @@ -36,6 +37,7 @@ import { createOpenWikiThreadId, runOpenWikiAgent } from "./agent/index.js"; import { formatChatGptAccountFromEnv } from "./agent/openai-chatgpt-oauth.js"; import { getErrorMessage, + isAuthError, isSecretLikeKey, sanitizeDiagnosticText, } from "./diagnostics.js"; @@ -121,6 +123,7 @@ type RunState = message: string; credentialDiagnostics?: CredentialDiagnostic[]; errorDiagnostics?: ErrorDiagnostic[]; + authFix?: AuthFix; }; type RunLogItem = { @@ -152,6 +155,17 @@ type ErrorDiagnostic = { value: string; }; +/** + * What the "How to fix" panel needs after an auth failure. Key names only, no + * secret values: {@link AuthFix.keyFromShell} reflects only whether the failing + * provider's API key was sourced from a shell export (which shadows saved + * config), so the panel can tell the user to unset it. + */ +type AuthFix = { + apiKeyEnvKey: string | undefined; + keyFromShell: boolean; +}; + type AppProps = { command: CliCommand; }; @@ -390,6 +404,19 @@ function App({ command }: AppProps) { const errorDiagnostics = getErrorDiagnostics(error); const message = getErrorMessage(error); + const authFix = getAuthFix(error, message, sessionProvider); + + // The full credential dump is opt-in (--debug); by default show only the + // concise message, allowlisted error fields, and the how-to-fix panel. + if (!shouldShowCredentialDiagnostics()) { + setRunState({ + status: "error", + message, + errorDiagnostics, + authFix, + }); + return; + } void getCredentialDiagnostics() .catch(() => undefined) @@ -403,6 +430,7 @@ function App({ command }: AppProps) { message, credentialDiagnostics, errorDiagnostics, + authFix, }); }); }); @@ -610,6 +638,19 @@ function App({ command }: AppProps) { const errorDiagnostics = getErrorDiagnostics(error); const message = getErrorMessage(error); + const authFix = getAuthFix(error, message, sessionProvider); + + // The full credential dump is opt-in (--debug); by default show only the + // concise message, allowlisted error fields, and the how-to-fix panel. + if (!shouldShowCredentialDiagnostics()) { + setRunState({ + status: "error", + message, + errorDiagnostics, + authFix, + }); + return; + } void getCredentialDiagnostics() .catch(() => undefined) @@ -623,6 +664,7 @@ function App({ command }: AppProps) { message, credentialDiagnostics, errorDiagnostics, + authFix, }); }); }); @@ -904,6 +946,7 @@ function App({ command }: AppProps) {
+ {runState.authFix ? : null} {runState.credentialDiagnostics ? ( + Your provider rejected the credentials for this run. + {steps.map((step, index) => ( + + {index + 1}. + {step} + + ))} + For full detail, re-run with --debug. + + ); +} + function ErrorDiagnosticsPanel({ diagnostics, }: { @@ -3187,6 +3269,31 @@ function getDisplayModelId(modelId: string | null): string { ); } +/** + * The auth "how to fix" context for a failure, or undefined when it does not + * look like an auth error. Names the failing provider's API key env var and + * flags whether it came from the shell (a shell export shadows saved config, so + * the fix is to unset it). Existence check only, never reads the value. + */ +function getAuthFix( + error: unknown, + message: string, + provider: OpenWikiProvider, +): AuthFix | undefined { + if (!isAuthError(error, message)) { + return undefined; + } + + const apiKeyEnvKey = getProviderApiKeyEnvKey(provider); + + return { + apiKeyEnvKey, + keyFromShell: + apiKeyEnvKey !== undefined && + getShellEnvValue(apiKeyEnvKey) !== undefined, + }; +} + function getErrorDiagnostics(error: unknown): ErrorDiagnostic[] { const diagnostics: ErrorDiagnostic[] = []; const debugMode = isDebugMode(); @@ -3998,12 +4105,38 @@ async function runPrintCommand( process.exitCode = 0; } catch (error) { - process.stderr.write(`${getErrorMessage(error)}\n`); + const message = getErrorMessage(error); + process.stderr.write(`${message}\n`); + writePrintAuthFix(error, message); writePrintErrorDiagnostics(error); process.exitCode = 1; } } +/** + * Write the concise auth "how to fix" guidance to stderr on a non-interactive + * failure, mirroring the interactive panel so CI/print runs get the same help. + * No-op unless the failure looks like an auth error. Key names only. + */ +function writePrintAuthFix(error: unknown, message: string): void { + const authFix = getAuthFix(error, message, resolveConfiguredProvider()); + + if (!authFix) { + return; + } + + process.stderr.write("\nHow to fix\n"); + process.stderr.write( + "Your provider rejected the credentials for this run.\n", + ); + + getAuthFixSteps(authFix).forEach((step, index) => { + process.stderr.write(`${index + 1}. ${step}\n`); + }); + + process.stderr.write("For full detail, re-run with --debug.\n"); +} + function writePrintErrorDiagnostics(error: unknown): void { const diagnostics = getErrorDiagnostics(error); diff --git a/src/commands.ts b/src/commands.ts index eda369b3..791b5669 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -369,6 +369,13 @@ function parseRunCommand( continue; } + if (arg === "--debug") { + // isDebugMode() reads OPENWIKI_DEBUG; setting it at parse time is the + // least-invasive way to opt into full credential/error diagnostics. + process.env.OPENWIKI_DEBUG = "1"; + continue; + } + if (arg === "--init" || arg === "--update") { const nextCommand = arg === "--init" ? "init" : "update"; @@ -727,6 +734,11 @@ export const helpContent: HelpContent = { label: "-p, --print", description: "Run once and print the final assistant output.", }, + { + label: "--debug", + description: + "Show full credential and error diagnostics when a run fails.", + }, { label: "--modelId ", description: "Use a model ID for this run.", diff --git a/src/diagnostics.ts b/src/diagnostics.ts index dc51d479..e68498f4 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -116,6 +116,30 @@ export function getErrorMessage(error: unknown): string { return sanitizeDiagnosticText(message); } +/** + * Whether a run failure looks like a credential rejection, judged from the HTTP + * status (401/403, number or string) and the already-redacted message. Drives + * whether the CLI shows the auth "how to fix" panel. + */ +export function isAuthError(error: unknown, message: string): boolean { + const status = isRecord(error) + ? (error.statusCode ?? error.status) + : undefined; + + if ( + status === 401 || + status === 403 || + status === "401" || + status === "403" + ) { + return true; + } + + return /\b40[13]\b|incorrect api key|invalid api key|unauthorized|forbidden|authentication|permission denied|not authorized/iu.test( + message, + ); +} + function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null; } diff --git a/test/commands.test.ts b/test/commands.test.ts index 3ff5d722..a77f666d 100644 --- a/test/commands.test.ts +++ b/test/commands.test.ts @@ -6,10 +6,13 @@ import { parseCommand, shouldRunNonInteractively } from "../src/commands.ts"; // restore afterward. const originalNodeEnv = process.env.NODE_ENV; const originalDevFlag = process.env.OPENWIKI_DEV; +const originalDebug = process.env.OPENWIKI_DEBUG; beforeEach(() => { delete process.env.NODE_ENV; delete process.env.OPENWIKI_DEV; + // parseCommand sets OPENWIKI_DEBUG as a side effect of --debug; start clean. + delete process.env.OPENWIKI_DEBUG; }); afterEach(() => { @@ -17,6 +20,25 @@ afterEach(() => { else process.env.NODE_ENV = originalNodeEnv; if (originalDevFlag === undefined) delete process.env.OPENWIKI_DEV; else process.env.OPENWIKI_DEV = originalDevFlag; + if (originalDebug === undefined) delete process.env.OPENWIKI_DEBUG; + else process.env.OPENWIKI_DEBUG = originalDebug; +}); + +describe("parseCommand — --debug", () => { + test("--debug sets OPENWIKI_DEBUG and still parses the run", () => { + expect(process.env.OPENWIKI_DEBUG).toBeUndefined(); + + const result = parseCommand(["--debug", "--init"]); + + expect(process.env.OPENWIKI_DEBUG).toBe("1"); + expect(result).toMatchObject({ kind: "run", command: "init" }); + }); + + test("without --debug, OPENWIKI_DEBUG stays unset", () => { + parseCommand(["--init"]); + + expect(process.env.OPENWIKI_DEBUG).toBeUndefined(); + }); }); describe("parseCommand — help", () => { diff --git a/test/diagnostics.test.ts b/test/diagnostics.test.ts new file mode 100644 index 00000000..9abf0db6 --- /dev/null +++ b/test/diagnostics.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, test } from "vitest"; +import { isAuthError } from "../src/diagnostics.ts"; + +describe("isAuthError", () => { + test("classifies 401/403 status codes (number or string) as auth errors", () => { + expect(isAuthError({ statusCode: 401 }, "boom")).toBe(true); + expect(isAuthError({ statusCode: 403 }, "boom")).toBe(true); + expect(isAuthError({ status: "401" }, "boom")).toBe(true); + expect(isAuthError({ status: "403" }, "boom")).toBe(true); + }); + + test("classifies auth-shaped messages regardless of status", () => { + for (const message of [ + "Incorrect API key provided", + "invalid api key", + "401 Unauthorized", + "authentication failed", + "permission denied", + "you are not authorized", + // 403/forbidden and a bare status code in the message (the provider does + // not always expose statusCode on the error object). + '403 "Forbidden"', + "Forbidden", + "request failed with status 401", + ]) { + expect(isAuthError(undefined, message)).toBe(true); + } + }); + + test("does not flag unrelated failures", () => { + expect(isAuthError({ statusCode: 500 }, "internal server error")).toBe( + false, + ); + expect(isAuthError(new Error("timeout"), "timeout")).toBe(false); + expect(isAuthError(undefined, "rate limit exceeded")).toBe(false); + expect(isAuthError(undefined, "404 not found")).toBe(false); + }); + + test("matches the message case-insensitively", () => { + expect(isAuthError(undefined, "UNAUTHORIZED")).toBe(true); + expect(isAuthError(undefined, "Invalid API Key")).toBe(true); + }); +}); From 33149986aa4a1b82049a2171d1ffc41f4e89c86a Mon Sep 17 00:00:00 2001 From: Colin Francis Date: Thu, 16 Jul 2026 23:58:11 -0700 Subject: [PATCH 6/9] reset api key on provider switch --- src/credentials.tsx | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/credentials.tsx b/src/credentials.tsx index cc6aacc5..3f4b8e82 100644 --- a/src/credentials.tsx +++ b/src/credentials.tsx @@ -1392,9 +1392,25 @@ export function InitSetup({ const selectedProvider = SELECTABLE_OPENWIKI_PROVIDERS[providerSelectionIndex] ?? DEFAULT_PROVIDER; + // Credentials are provider-specific, so switching providers must not carry + // the previous provider's key/secret/etc. across (otherwise seedInputForStep + // prefills it and empty-submit-keeps would save it under the new provider). + const switchedProvider = selectedProvider !== provider; setProvider(selectedProvider); setProviderConfirmed(true); + + if (switchedProvider) { + setApiKey(null); + setSecretKey(null); + setBaseUrl(null); + setRegion(null); + setGcpProject(null); + setGcpLocation(null); + setOauthTokens(null); + setModelId(null); + } + setProviderSelectionIndex(getProviderSelectionIndex(selectedProvider)); setModelSelectionIndex( getModelSelectionIndex( @@ -1426,7 +1442,14 @@ export function InitSetup({ nextStep === "model" && shouldStartWithCustomModelInput(selectedProvider), ); - seedInputForStep(nextStep); + // On a switch the closure still holds the old provider/apiKey, so + // seedInputForStep would re-seed stale values; leave the field empty and + // let a later visit seed from the new provider's own env. + if (switchedProvider) { + setInput(""); + } else { + seedInputForStep(nextStep); + } setStep(nextStep); return; } From a297bcd30229efce498469a9ee99046fe6c53522 Mon Sep 17 00:00:00 2001 From: Colin Francis Date: Fri, 17 Jul 2026 10:07:41 -0700 Subject: [PATCH 7/9] idempotent walk --- src/cli.tsx | 10 +- src/credentials.tsx | 269 ++++++++++++++++++++++++++++---------- src/env.ts | 21 +++ test/credentials.test.ts | 40 ++++-- test/env-behavior.test.ts | 27 ++++ 5 files changed, 286 insertions(+), 81 deletions(-) diff --git a/src/cli.tsx b/src/cli.tsx index b0fe061c..e47a6553 100644 --- a/src/cli.tsx +++ b/src/cli.tsx @@ -308,13 +308,19 @@ function App({ command }: AppProps) { ? command.command : null, ); + // `--init` always opens the full setup walk, even when everything is already + // configured, so you can review or change any step. Consumed once the walk + // finishes so it does not re-open when the run later returns to idle. + const [initWizardConsumed, setInitWizardConsumed] = useState(false); + const isInitCommand = command.kind === "run" && command.command === "init"; const shouldRunInteractiveCredentialSetup = command.kind === "run" && resolvedCommand !== null && !command.dryRun && process.stdin.isTTY && runState.status === "idle" && - needsCredentialSetup(sessionModelId, runMode); + (needsCredentialSetup(sessionModelId, runMode) || + (isInitCommand && !initWizardConsumed)); const displayModelId = sessionModelId ?? startupModelId; function submitChatMessage(message: string) { @@ -737,7 +743,9 @@ function App({ command }: AppProps) { allowModeSelection={false} mode={command.mode} modelIdOverride={command.modelId} + walkAllSteps={isInitCommand} onComplete={(result) => { + setInitWizardConsumed(true); const nextCodeRuntimeCwd = result.repoRoot ?? codeRuntimeCwd; if (result.repoRoot) { diff --git a/src/credentials.tsx b/src/credentials.tsx index 3f4b8e82..cf53844b 100644 --- a/src/credentials.tsx +++ b/src/credentials.tsx @@ -53,7 +53,12 @@ import type { AuthProviderId } from "./auth/types.js"; import type { OpenWikiRunMode } from "./commands.js"; import type { ConnectorId } from "./connectors/types.js"; import { getConnectorConfigPath } from "./openwiki-home.js"; -import { getShellEnvValue, openWikiEnvPath, saveOpenWikiEnv } from "./env.js"; +import { + getSavedEnvValue, + getShellEnvValue, + openWikiEnvPath, + saveOpenWikiEnv, +} from "./env.js"; import { createEmptyOnboardingConfig, isOpenWikiOnboardingCompleteSync, @@ -97,6 +102,12 @@ type InitSetupProps = { modelIdOverride?: string | null; onComplete: (result: InitSetupResult) => void; onError: (message: string) => void; + /** + * When true (explicit `--init`), walk every applicable step even when it is + * already configured, so the run can review/change any of them. When false + * the wizard skips satisfied steps and collects only what is missing. + */ + walkAllSteps?: boolean; }; type PromptStep = @@ -480,30 +491,16 @@ export function orderedSetupSteps( steps.push("model"); steps.push("langsmith"); - steps.push(mode === "code" ? "code-repo-confirm" : "template"); + // Personal mode's template is fixed by the run mode, so it skips the + // Code/Personal chooser and walks straight into the wiki brief. Only code + // mode needs a spine step after langsmith (repo confirmation). + if (mode === "code") { + steps.push("code-repo-confirm"); + } return steps; } -/** - * The step before `step` in the applicable spine, or null when `step` is the - * first step or is outside the spine (for example the personal-mode source - * sub-flow, which manages its own navigation). - */ -export function previousSpineStep( - step: PromptStep | null, - provider: OpenWikiProvider, - mode: OpenWikiRunMode, - allowModeSelection: boolean, -): PromptStep | null { - if (step === null) { - return null; - } - const spine = orderedSetupSteps(provider, mode, allowModeSelection); - const index = spine.indexOf(step); - return index > 0 ? spine[index - 1] : null; -} - /** * The step after `step` in the applicable spine, or null when `step` is the last * spine step or outside it. Drives forward navigation: Enter advances to the @@ -650,10 +647,27 @@ export function InitSetup({ modelIdOverride = null, onComplete, onError, + walkAllSteps = false, }: InitSetupProps) { const { stdout } = useStdout(); const initialProvider = resolveConfiguredProvider(); - const [step, setStep] = useState(null); + const [step, setStepRaw] = useState(null); + const navHistory = useRef([]); + // Guards the mount effect so the initial step is seeded once per mount, not + // re-seeded when the effect re-fires on parent re-renders. + const didInitializeRef = useRef(false); + /** + * Advance to a step, recording the current step on the back-navigation + * history unless this is a back move. A ref-backed stack so Esc can retrace + * the actual path taken (including the branchy source sub-flow), which a + * linear spine cannot model. + */ + function setStep(next: PromptStep | null, opts?: { back?: boolean }): void { + if (!opts?.back && step !== null && next !== null && next !== step) { + navHistory.current.push(step); + } + setStepRaw(next); + } const [selectedMode, setSelectedMode] = useState(mode); const [provider, setProvider] = useState(initialProvider); const [apiKey, setApiKey] = useState(null); @@ -743,9 +757,14 @@ export function InitSetup({ readOpenWikiOnboardingConfig() .then(async (config) => { - if (cancelled) { + // Seed the initial step exactly once per mount. onComplete/onError are + // inline parent closures in the deps, so this effect re-fires on parent + // re-renders; without this guard a re-fire would reset step back to the + // first step (getInitialStep with walkAll always returns it). + if (cancelled || didInitializeRef.current) { return; } + didInitializeRef.current = true; const defaultRepoRoot = getDefaultCodeRepoRootPath(); const configForMode = allowModeSelection @@ -768,6 +787,7 @@ export function InitSetup({ configForMode, mode, allowModeSelection, + walkAllSteps, ); if (initialStep === null) { @@ -955,28 +975,97 @@ export function InitSetup({ break; case "api-key": { const envKey = getProviderApiKeyEnvKey(provider); - setInput(apiKey ?? (envKey ? (process.env[envKey] ?? "") : "")); + setInput(apiKey ?? (envKey ? (getSavedEnvValue(envKey) ?? "") : "")); break; } case "secret-key": { const envKey = getProviderSecretKeyEnvKey(provider); - setInput(secretKey ?? (envKey ? (process.env[envKey] ?? "") : "")); + setInput(secretKey ?? (envKey ? (getSavedEnvValue(envKey) ?? "") : "")); break; } - case "base-url": - setInput(baseUrl ?? ""); + case "base-url": { + const envKey = getProviderBaseUrlEnvKey(provider); + setInput(baseUrl ?? (envKey ? (getSavedEnvValue(envKey) ?? "") : "")); break; - case "region": - setInput(region ?? ""); + } + case "region": { + const envKey = getProviderRegionEnvKey(provider); + setInput(region ?? (envKey ? (getSavedEnvValue(envKey) ?? "") : "")); break; - case "gcp-project": - setInput(gcpProject ?? ""); + } + case "gcp-project": { + const envKey = getProviderProjectEnvKey(provider); + setInput( + gcpProject ?? (envKey ? (getSavedEnvValue(envKey) ?? "") : ""), + ); break; - case "gcp-location": - setInput(gcpLocation ?? ""); + } + case "gcp-location": { + const envKey = getProviderLocationEnvKey(provider); + setInput( + gcpLocation ?? (envKey ? (getSavedEnvValue(envKey) ?? "") : ""), + ); break; + } case "langsmith": - setInput(langSmithKey ?? ""); + // Prefill from state or the saved config (masked as dots), matching the + // api-key/secret-key steps, so a walk-through Enter keeps the existing + // key instead of submitting empty and clearing it. + setInput(langSmithKey ?? getSavedEnvValue("LANGSMITH_API_KEY") ?? ""); + break; + case "template": + setTemplateSelectionIndex( + Math.max( + 0, + ONBOARDING_TEMPLATES.findIndex( + (template) => template.id === getConfigModeId(onboardingConfig), + ), + ), + ); + setInput(""); + break; + case "wiki-goal": + setInput(onboardingConfig.wikiGoal ?? ""); + break; + case "global-cron-mode": + setCronModeSelectionIndex(0); + setInput(""); + break; + case "global-cron-custom": + setInput( + onboardingConfig.ingestionSchedule?.expression ?? + suggestedCronExpression, + ); + setCronFieldSelectionIndex(0); + setCronReplaceCurrentField(true); + break; + case "global-power-mode": + setPowerModeSelectionIndex(0); + setInput(""); + break; + case "source-menu": + // Park the cursor on the "Continue" row so Enter keeps sources as-is. + setSourceSelectionIndex(activeSourceOptions.length); + setInput(""); + break; + case "source-description": + setSourceDescriptionSelectionIndex(0); + setInput(""); + break; + case "source-confirm-continue": + setSourceContinueSelectionIndex(0); + setInput(""); + break; + case "final": + setFinalSelectionIndex(0); + setInput(""); + break; + case "code-repo-confirm": + setCodeRepoSelectionIndex(0); + setInput(""); + break; + case "code-repo-path": + setCodeRepoPathInput(codeRepoRoot); break; default: setInput(""); @@ -1012,6 +1101,13 @@ export function InitSetup({ case "langsmith": setLangSmithKey(trimmed); break; + case "wiki-goal": + // Keep an unsubmitted goal edit in-session (not yet persisted) so + // stepping back and forward does not lose it. + if (trimmed) { + setOnboardingConfig((config) => ({ ...config, wikiGoal: trimmed })); + } + break; default: break; } @@ -1027,19 +1123,15 @@ export function InitSetup({ return; } - // Esc returns to the previous applicable step so no choice is a dead end and - // an auto-skipped step stays reachable. It commits the current field first - // (so an unsubmitted edit is kept) and is a no-op on the first step. + // Esc retraces the actual path taken via the navigation history stack, so + // it works through the branchy source sub-flow too. It commits the current + // field first (so an unsubmitted edit is kept) and is a no-op at the start. if (key.escape) { - const target = previousSpineStep( - step, - provider, - selectedMode, - allowModeSelection, - ); - if (target !== null) { + const target = navHistory.current[navHistory.current.length - 1]; + if (target !== undefined) { captureInputForStep(step); - setStep(target); + navHistory.current.pop(); + setStep(target, { back: true }); seedInputForStep(target); setError(null); setNotice(null); @@ -1845,7 +1937,17 @@ export function InitSetup({ templateName: selectedTemplate.name, }; await saveConfig(nextConfig); - setInput(selectedTemplate.suggestedGoal); + // Keep the existing goal when the template is unchanged (so re-walking is + // idempotent); use the template's suggested goal when it actually changed. + const keepExistingGoal = + selectedTemplate.id === getConfigModeId(onboardingConfig) && + onboardingConfig.wikiGoal !== undefined && + onboardingConfig.wikiGoal.length > 0; + setInput( + keepExistingGoal + ? (onboardingConfig.wikiGoal ?? "") + : selectedTemplate.suggestedGoal, + ); setStep("wiki-goal"); return; } @@ -2120,6 +2222,28 @@ export function InitSetup({ async function continueAfterCredentials(options: CompleteSetupOptions) { await saveCredentialUpdates(options); + // Explicit --init walks the whole tail; enter at its first step rather than + // skipping steps that are already configured. + if (walkAllSteps) { + if (options.runMode === "code") { + setCodeRepoRoot(getDefaultCodeRepoRootPath()); + setCodeRepoSelectionIndex(0); + setStep("code-repo-confirm"); + return; + } + + // Personal mode fixes the template from the run mode, so skip the + // redundant Code/Personal chooser and walk straight into the wiki brief. + // Seed the existing goal so Enter keeps it (idempotent re-walk), else the + // template's suggested goal. + setInput( + onboardingConfig.wikiGoal ?? + getTemplateGoal(getConfigModeId(onboardingConfig)), + ); + setStep("wiki-goal"); + return; + } + if (options.runMode === "code" && !isOnboardingComplete(onboardingConfig)) { setCodeRepoRoot(getDefaultCodeRepoRootPath()); setCodeRepoSelectionIndex(0); @@ -2153,13 +2277,19 @@ export function InitSetup({ } function continueAfterCodeRepoConfirmed(repoRoot: string) { - if (!onboardingConfig.wikiGoal) { - setInput(getTemplateGoal(getConfigModeId(onboardingConfig))); + setCodeRepoRoot(repoRoot); + + // Walk the wiki-goal step on --init even when set; otherwise only when + // unset. Seed the existing goal so Enter keeps it (idempotent). + if (walkAllSteps || !onboardingConfig.wikiGoal) { + setInput( + onboardingConfig.wikiGoal ?? + getTemplateGoal(getConfigModeId(onboardingConfig)), + ); setStep("wiki-goal"); return; } - setCodeRepoRoot(repoRoot); setStep("final"); } @@ -2732,17 +2862,6 @@ export function InitSetup({ : "not set" } /> - {selectedMode === "personal" ? ( - - ) : null} {selectedMode === "personal" ? ( - 0 - ? "done" - : isSourceStep(step) - ? "current" + isSourceStep(step) + ? "current" + : getConnectedSourceCount( + onboardingConfig, + activeSourceOptions, + ) > 0 + ? "done" : "pending" } detail={`${getConnectedSourceCount( @@ -2837,8 +2958,7 @@ export function InitSetup({ )} - {previousSpineStep(step, provider, selectedMode, allowModeSelection) !== - null ? ( + {navHistory.current.length > 0 ? ( esc to go back @@ -3933,7 +4053,14 @@ export function getInitialStep( onboardingConfig: OpenWikiOnboardingConfig = createEmptyOnboardingConfig(), mode: OpenWikiRunMode = "code", allowModeSelection = false, + walkAll = false, ): PromptStep | null { + if (walkAll) { + // Explicit --init: always start at the top and walk every applicable step, + // even ones already configured, instead of skipping to the first unset one. + return orderedSetupSteps(provider, mode, allowModeSelection)[0] ?? null; + } + if (allowModeSelection) { return "run-mode"; } diff --git a/src/env.ts b/src/env.ts index 5e9b5901..8809798c 100644 --- a/src/env.ts +++ b/src/env.ts @@ -209,11 +209,32 @@ export function getShellEnvValue(key: string): string | undefined { return shellEnvAtStartup?.[key]; } +/** + * The values saved in `~/.openwiki/.env` as of the first load, before shell + * exports win in `process.env`. Lets the setup wizard pre-fill fields from the + * saved config rather than `process.env` (which a shell var may shadow), so + * editing config never captures a shell override. In memory only. + */ +let savedEnvAtStartup: Record | undefined; + +/** + * The saved `~/.openwiki/.env` value for a key as of startup, or `undefined`. + * Distinct from {@link getShellEnvValue} (the shell snapshot) and from + * `process.env` (shell-over-file at runtime). + */ +export function getSavedEnvValue(key: string): string | undefined { + return savedEnvAtStartup?.[key]; +} + export async function loadOpenWikiEnv(): Promise { captureShellEnv(); const env = await readOpenWikiEnv(); + if (savedEnvAtStartup === undefined) { + savedEnvAtStartup = { ...env }; + } + for (const [key, value] of Object.entries(env)) { if (deprecatedEnvKeys.includes(key)) { continue; diff --git a/test/credentials.test.ts b/test/credentials.test.ts index 2a8cf050..b4bb126f 100644 --- a/test/credentials.test.ts +++ b/test/credentials.test.ts @@ -1,9 +1,9 @@ import { afterEach, describe, expect, test } from "vitest"; import { + getInitialStep, needsCredentialSetup, nextSetupStep, orderedSetupSteps, - previousSpineStep, resolveStepStatus, } from "../src/credentials.tsx"; @@ -97,9 +97,14 @@ describe("orderedSetupSteps", () => { expect(spine.indexOf("region")).toBeLessThan(spine.indexOf("model")); }); - test("personal mode ends at the template tail, not code-repo-confirm", () => { + test("personal mode ends at langsmith with no template chooser or code tail", () => { + // The template is fixed by the run mode in personal mode, so the spine + // skips the redundant Code/Personal chooser and walks straight into the + // wiki brief after langsmith. const spine = orderedSetupSteps("openai", "personal", false); - expect(spine[spine.length - 1]).toBe("template"); + expect(spine[spine.length - 1]).toBe("langsmith"); + expect(spine).not.toContain("template"); + expect(spine).not.toContain("code-repo-confirm"); }); test("the spine includes applicable steps regardless of env (reachability)", () => { @@ -109,18 +114,35 @@ describe("orderedSetupSteps", () => { }); }); -describe("previousSpineStep and nextSetupStep", () => { - test("walk backward and forward through the spine", () => { - expect(previousSpineStep("model", "openai", "code", false)).toBe("api-key"); +describe("getInitialStep", () => { + test("walkAll starts at the first spine step regardless of configuration", () => { + // walkAll short-circuits the skip-waterfall, so it never returns null even + // when everything would otherwise be satisfied. + expect(getInitialStep(null, "openai", undefined, "code", false, true)).toBe( + "provider", + ); + expect( + getInitialStep(null, "bedrock", undefined, "code", false, true), + ).toBe("provider"); + }); + + test("walkAll returns run-mode first when mode selection is allowed", () => { + expect(getInitialStep(null, "openai", undefined, "code", true, true)).toBe( + "run-mode", + ); + }); +}); + +describe("nextSetupStep", () => { + test("walks forward through the spine", () => { + expect(nextSetupStep("provider", "openai", "code", false)).toBe("api-key"); expect(nextSetupStep("api-key", "openai", "code", false)).toBe("model"); }); - test("no-op at the ends and for null", () => { - expect(previousSpineStep("provider", "openai", "code", false)).toBe(null); + test("no-op at the end and for null", () => { expect(nextSetupStep("code-repo-confirm", "openai", "code", false)).toBe( null, ); - expect(previousSpineStep(null, "openai", "code", false)).toBe(null); expect(nextSetupStep(null, "openai", "code", false)).toBe(null); }); }); diff --git a/test/env-behavior.test.ts b/test/env-behavior.test.ts index 7cb1e0a8..2607ddd1 100644 --- a/test/env-behavior.test.ts +++ b/test/env-behavior.test.ts @@ -228,6 +228,33 @@ describe("getShellEnvValue", () => { }); }); +describe("getSavedEnvValue", () => { + test("returns the saved file value, not a shadowing shell value", async () => { + // The shell shadows the key at runtime... + process.env[OPENROUTER_API_KEY_ENV_KEY] = "from-shell"; + // ...but a different value is saved in the file. + await mkdir(path.dirname(env.openWikiEnvPath), { recursive: true }); + await writeFile( + env.openWikiEnvPath, + `${OPENROUTER_API_KEY_ENV_KEY}="from-file"\n`, + "utf8", + ); + + await env.loadOpenWikiEnv(); + + // process.env keeps the shell value (shell wins)... + expect(process.env[OPENROUTER_API_KEY_ENV_KEY]).toBe("from-shell"); + // ...but the saved snapshot reflects the file, so the wizard seeds config. + expect(env.getSavedEnvValue(OPENROUTER_API_KEY_ENV_KEY)).toBe("from-file"); + }); + + test("is undefined for a key absent from the saved file", async () => { + await env.loadOpenWikiEnv(); + + expect(env.getSavedEnvValue(OPENROUTER_API_KEY_ENV_KEY)).toBeUndefined(); + }); +}); + describe("getCredentialDiagnostics", () => { test("includes the provider and each credential key in display order", async () => { const diagnostics = await env.getCredentialDiagnostics(); From 4fbf198803c0bbfcabb5e8f85217ab40a3cd71b6 Mon Sep 17 00:00:00 2001 From: Colin Francis Date: Fri, 17 Jul 2026 11:53:35 -0700 Subject: [PATCH 8/9] align arrow with selected value on init --- src/credentials.tsx | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/credentials.tsx b/src/credentials.tsx index cf53844b..61447ac5 100644 --- a/src/credentials.tsx +++ b/src/credentials.tsx @@ -959,20 +959,28 @@ export function InitSetup({ case "run-mode": setRunModeSelectionIndex(getRunModeSelectionIndex(selectedMode)); break; - case "model": - setModelSelectionIndex( - getModelSelectionIndex( - provider, - modelId ?? getDefaultModelId(provider), - ), - ); + case "model": { + // Point the cursor at the saved model (or the --modelId override), not + // the provider default, so it matches the checklist on a re-walk. + const seededModelId = + modelId ?? + modelIdOverride ?? + getSavedEnvValue(OPENWIKI_MODEL_ID_ENV_KEY) ?? + getDefaultModelId(provider); + setModelSelectionIndex(getModelSelectionIndex(provider, seededModelId)); // Preset-less providers (e.g. Bedrock) take the model as free text, so // restore the saved id into the field; selection-based providers drive // off the index and keep the input empty. setInput( - shouldStartWithCustomModelInput(provider) ? (modelId ?? "") : "", + shouldStartWithCustomModelInput(provider) + ? (modelId ?? + modelIdOverride ?? + getSavedEnvValue(OPENWIKI_MODEL_ID_ENV_KEY) ?? + "") + : "", ); break; + } case "api-key": { const envKey = getProviderApiKeyEnvKey(provider); setInput(apiKey ?? (envKey ? (getSavedEnvValue(envKey) ?? "") : "")); From de10ce177d1889d9511f17292aef96c364a25a93 Mon Sep 17 00:00:00 2001 From: Colin Francis Date: Fri, 17 Jul 2026 12:06:04 -0700 Subject: [PATCH 9/9] skipped langsmith key should not be set as an empty string in .env --- src/credentials.tsx | 13 ++++++------- src/env.ts | 19 ++++++++++++++++++- test/env-behavior.test.ts | 11 +++++++++++ test/openai-chatgpt-credentials.test.ts | 6 +++--- 4 files changed, 38 insertions(+), 11 deletions(-) diff --git a/src/credentials.tsx b/src/credentials.tsx index 61447ac5..f2128e88 100644 --- a/src/credentials.tsx +++ b/src/credentials.tsx @@ -392,7 +392,7 @@ export function needsCredentialSetup( needsRegionStep(provider) || (modelIdOverride === null && process.env[OPENWIKI_MODEL_ID_ENV_KEY] === undefined) || - process.env.LANGSMITH_API_KEY === undefined; + !process.env.LANGSMITH_API_KEY; if (needsCredentials) { return true; @@ -2675,7 +2675,7 @@ export function InitSetup({ needsRegionStep(provider) || (modelIdOverride === null && process.env[OPENWIKI_MODEL_ID_ENV_KEY] === undefined) || - process.env.LANGSMITH_API_KEY === undefined; + !process.env.LANGSMITH_API_KEY; const apiKeyEnvKey = getProviderApiKeyEnvKey(provider); const projectEnvKey = getProviderProjectEnvKey(provider); const locationEnvKey = getProviderLocationEnvKey(provider); @@ -2856,8 +2856,7 @@ export function InitSetup({ state={resolveStepStatus( "langsmith", step, - langSmithKey !== null || - process.env.LANGSMITH_API_KEY !== undefined, + langSmithKey !== null || Boolean(process.env.LANGSMITH_API_KEY), "optional", )} detail={ @@ -2865,7 +2864,7 @@ export function InitSetup({ ? langSmithKey.length > 0 ? "configured" : "skipped" - : process.env.LANGSMITH_API_KEY !== undefined + : process.env.LANGSMITH_API_KEY ? "configured" : "not set" } @@ -4104,7 +4103,7 @@ export function getInitialStep( return "model"; } - if (process.env.LANGSMITH_API_KEY === undefined) { + if (!process.env.LANGSMITH_API_KEY) { return "langsmith"; } @@ -4245,7 +4244,7 @@ function getNextStepAfterRegion( return "model"; } - if (process.env.LANGSMITH_API_KEY === undefined) { + if (!process.env.LANGSMITH_API_KEY) { return "langsmith"; } diff --git a/src/env.ts b/src/env.ts index 8809798c..d1b399a5 100644 --- a/src/env.ts +++ b/src/env.ts @@ -271,6 +271,15 @@ export async function saveOpenWikiEnv(updates: EnvMap): Promise { delete nextEnv[key]; } + // An empty value means "not set" (e.g. skipping the optional LangSmith key), + // so drop the key rather than persisting KEY="" which would later read back + // as configured. Also self-heals any empty values left by earlier writes. + for (const key of Object.keys(nextEnv)) { + if (nextEnv[key] === "") { + delete nextEnv[key]; + } + } + await mkdir(openWikiEnvDir, { recursive: true, mode: 0o700, @@ -286,7 +295,15 @@ export async function saveOpenWikiEnv(updates: EnvMap): Promise { for (const [key, value] of Object.entries(updates)) { // A shell export wins at runtime, so don't mask it in process.env; the // saved value is only the fallback for when that shell var is unset. - if (shellEnvAtStartup?.[key] === undefined) { + if (shellEnvAtStartup?.[key] !== undefined) { + continue; + } + + // Mirror the file: an empty value means "not set", so clear it from + // process.env rather than leaving KEY="" (which reads back as configured). + if (value === "") { + delete process.env[key]; + } else { process.env[key] = value; } } diff --git a/test/env-behavior.test.ts b/test/env-behavior.test.ts index 2607ddd1..e427a37b 100644 --- a/test/env-behavior.test.ts +++ b/test/env-behavior.test.ts @@ -203,6 +203,17 @@ describe("saveOpenWikiEnv", () => { const contents = await readFile(env.openWikiEnvPath, "utf8"); expect(contents).toContain('OPENROUTER_API_KEY="from-wizard"'); }); + + test('drops an empty value instead of persisting KEY=""', async () => { + // Skipping an optional key (empty value) must clear it, not save KEY="" + // (which would later read back as configured). + await env.saveOpenWikiEnv({ [OPENROUTER_API_KEY_ENV_KEY]: "sk-real" }); + await env.saveOpenWikiEnv({ [OPENROUTER_API_KEY_ENV_KEY]: "" }); + + const contents = await readFile(env.openWikiEnvPath, "utf8"); + expect(contents).not.toContain(OPENROUTER_API_KEY_ENV_KEY); + expect(process.env[OPENROUTER_API_KEY_ENV_KEY]).toBeUndefined(); + }); }); describe("getShellEnvValue", () => { diff --git a/test/openai-chatgpt-credentials.test.ts b/test/openai-chatgpt-credentials.test.ts index 666caec3..0f0bb3d4 100644 --- a/test/openai-chatgpt-credentials.test.ts +++ b/test/openai-chatgpt-credentials.test.ts @@ -58,7 +58,7 @@ function configureValidChatGptSession(): void { set("OPENWIKI_PROVIDER", "openai-chatgpt"); storeChatGptTokens(); set("OPENWIKI_MODEL_ID", "gpt-5.5"); - set("LANGSMITH_API_KEY", ""); + set("LANGSMITH_API_KEY", "lsv2_test-key"); } beforeEach(() => { @@ -178,7 +178,7 @@ describe("forceModel re-asks the model after a provider change", () => { set("OPENWIKI_PROVIDER", "openai-chatgpt"); storeChatGptTokens(); set("OPENWIKI_MODEL_ID", "gpt-5.4-mini"); - set("LANGSMITH_API_KEY", ""); + set("LANGSMITH_API_KEY", "lsv2_test-key"); // Without force, a stored model is kept (no model step). expect( @@ -193,7 +193,7 @@ describe("forceModel re-asks the model after a provider change", () => { test("a per-run model override still suppresses the model step", () => { set("OPENWIKI_PROVIDER", "openai-chatgpt"); storeChatGptTokens(); - set("LANGSMITH_API_KEY", ""); + set("LANGSMITH_API_KEY", "lsv2_test-key"); expect( getNextStepAfterProvider(