From c552b3be7242f84d7f26de44187b59cb4d8def0c Mon Sep 17 00:00:00 2001 From: kate bonner Date: Thu, 27 Aug 2026 15:04:19 -0400 Subject: [PATCH 1/2] fix(onboarding): keep the first run on Stage 0 and make its routing legible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects found while testing a genuine first run in an isolated sandbox (clean HOME, no model configured, no profile, no completion marker). The overture and its walkthrough were spliced correctly every time; the user never reached them. - Pasqal python provisioning fired `amicode.restartServer` while the Stage 0 webview was live. On a first run that provisioning always lands mid-onboarding, and the restart strands the panel on its "Getting Amico ready..." splash. The restart is now deferred while an onboarding panel is open — the panel already issues its own restart on submit, which is where the fresh AMICO_PYTHON gets picked up. - The onReady gate re-derived its decision inline from `isModelConfigured()` alone, so it ignored the completion marker and provider env vars. It now calls `resolveOnboardingAction`, which was written and unit-tested for exactly this and never wired up — the module was reachable only from its own tests. - The routing was entirely silent. A first run that lands on an empty chat left nothing to explain why, which is what made this hard to diagnose. Both the inputs and the chosen action are now logged to the opencode channel, and a failure to open the webview is reported instead of swallowed by a floating promise. Also drops the dead `OnboardingLauncher` and the `welcome_shown` helpers: nothing read or wrote them, `resolveOnboardingAction` never consulted the flag, and extension.ts owns launching via `serverManager.onReady`. Pre-existing and untouched: 4 failures in test/ops/skill_freshness_orchestrator.test.ts, identical on the base. --- packages/extension/src/extension.ts | 45 ++++- packages/extension/src/onboarding_routing.ts | 87 +-------- .../extension/test/onboarding_routing.test.ts | 173 +++++------------- 3 files changed, 79 insertions(+), 226 deletions(-) diff --git a/packages/extension/src/extension.ts b/packages/extension/src/extension.ts index d7e08e3c..bce0a2ca 100644 --- a/packages/extension/src/extension.ts +++ b/packages/extension/src/extension.ts @@ -38,10 +38,10 @@ import { stageDemoRun } from "./demo_replay"; import { writeStopFile, stopPlan, forceStop, runLogMtime } from "./run_controls"; import { watchSolverMode, applyEntitlementForMode, readSolverModeState } from "./solver_mode"; import { runSetCloudKeyCommand } from "./cloud_key"; -import { amicodeOpsDir } from "./substrate/vault_store"; +import { amicodeOpsDir, onboardingDir, hasOnboardingCompleted } from "./substrate/vault_store"; import { registerOnboardingPanel, onOnboardingCancelled, getOnboardingPanel, releaseOnboardingPanel } from "./onboarding_panel"; import { registerFleetPanel } from "./fleet_panel"; -import { isModelConfigured } from "./onboarding_routing"; +import { isModelConfigured, hasProviderEnvVar, resolveOnboardingAction } from "./onboarding_routing"; import { stagePasqalConnector } from "./pasqal_assets"; import { stageModCards } from "./mode_cards"; import { needsProvision, pasqalVenvDir, provisionPasqalPython } from "./pasqal_python"; @@ -276,10 +276,21 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { amicoPython = r.pythonPath; if (r.provisioned) { if (currentSpawnEnv) currentSpawnEnv.AMICO_PYTHON = r.pythonPath; - opencodeChannel.appendLine( - `[pasqal] python provisioned: ${r.pythonPath} — restarting server to pick it up`, - ); - void vscode.commands.executeCommand("amicode.restartServer"); + // Never restart out from under a live Stage 0 panel. On a first run + // this provisioning lands WHILE the onboarding webview is waiting on + // the server, and the restart strands it on the splash. The panel + // issues its own `amicode.restartServer` when the user submits the + // form, so the fresh AMICO_PYTHON is picked up there instead. + if (getOnboardingPanel()) { + opencodeChannel.appendLine( + `[pasqal] python provisioned: ${r.pythonPath} — restart deferred (onboarding in progress)`, + ); + } else { + opencodeChannel.appendLine( + `[pasqal] python provisioned: ${r.pythonPath} — restarting server to pick it up`, + ); + void vscode.commands.executeCommand("amicode.restartServer"); + } } } else { opencodeChannel.appendLine(`[pasqal] ${r.message}`); @@ -840,8 +851,24 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { // Onboarding gate: if no model is configured, open the Stage 0 webview // instead of chat. The webview will fire onOnboardingComplete when done, // which then opens chat. - if (!isModelConfigured() && vscode.workspace.getConfiguration("amicode").get("chat.autoOpen", true)) { - void vscode.commands.executeCommand("amicode.onboarding.open"); + const autoOpen = vscode.workspace.getConfiguration("amicode").get("chat.autoOpen", true); + const onboardingAction = resolveOnboardingAction({ + modelConfigured: isModelConfigured() || hasProviderEnvVar(), + onboardingCompleted: hasOnboardingCompleted(onboardingDir()), + partialStage: undefined, + }); + // First-run routing is otherwise invisible: every branch here is silent, + // so a first run that lands on an empty chat leaves nothing to explain + // why. Log the inputs AND the decision. + opencodeChannel.appendLine( + `[onboarding] action=${onboardingAction} modelConfigured=${isModelConfigured()} ` + + `providerEnv=${hasProviderEnvVar()} completed=${hasOnboardingCompleted(onboardingDir())} autoOpen=${autoOpen}`, + ); + if (onboardingAction === "show-webview" && autoOpen) { + void vscode.commands.executeCommand("amicode.onboarding.open").then( + () => opencodeChannel.appendLine("[onboarding] Stage 0 webview opened"), + (e) => opencodeChannel.appendLine(`[onboarding] Stage 0 webview FAILED to open: ${e}`), + ); // Wire: when onboarding completes, the server restarts and the // onReady handler (else-if branch below) opens the chat panel. // We do NOT open chat here — that would race the server restart @@ -850,7 +877,7 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { onOnboardingCancelled(() => { ChatPanel.openOrReveal(ctx, url, serverAuthToken(serverPassword), opencodeProject.projectDir); }); - } else if (vscode.workspace.getConfiguration("amicode").get("chat.autoOpen", true)) { + } else if (autoOpen) { // Normal path: model configured → open chat directly // Post-onboarding: adopt the onboarding panel as the chat panel (zero // tab switching — the splash overlay fades out revealing the chat). diff --git a/packages/extension/src/onboarding_routing.ts b/packages/extension/src/onboarding_routing.ts index fc238f01..879a8590 100644 --- a/packages/extension/src/onboarding_routing.ts +++ b/packages/extension/src/onboarding_routing.ts @@ -1,7 +1,7 @@ // Onboarding routing — session auto-launch and routing logic (#434) // // Pure routing predicate + launcher with at-most-once guard. -// Given (modelConfigured, welcomeShown, onboardingCompleted, partialStage), +// Given (modelConfigured, onboardingCompleted, partialStage), // determines the correct action for the session. import * as fs from "node:fs"; @@ -13,8 +13,6 @@ import * as os from "node:os"; export interface OnboardingFlags { /** True if the opencode config has at least one provider entry with credentials. */ modelConfigured: boolean; - /** True if the Stage 0 welcome animation has been played this install. */ - welcomeShown: boolean; /** True if the full onboarding flow (through Stage 8) has completed. */ onboardingCompleted: boolean; /** If partially completed, the last finished stage number (1-based). undefined = none. */ @@ -95,89 +93,6 @@ export function hasProviderEnvVar(): boolean { }); } -// ─── welcome_shown persistence ─────────────────────────────────────────────── - -const WELCOME_STATE_FILE = "onboarding_state.json"; - -/** Read whether the welcome animation has been shown (persisted across sessions). */ -export function readWelcomeShown( - statePath: string = path.join(os.homedir(), ".amico", "amicode", WELCOME_STATE_FILE), -): boolean { - try { - const data = JSON.parse(fs.readFileSync(statePath, "utf8")) as Record; - return data.welcome_shown === true; - } catch { - return false; - } -} - -/** Mark the welcome animation as shown. */ -export function writeWelcomeShown( - statePath: string = path.join(os.homedir(), ".amico", "amicode", WELCOME_STATE_FILE), -): void { - try { - fs.mkdirSync(path.dirname(statePath), { recursive: true }); - let existing: Record = {}; - try { - existing = JSON.parse(fs.readFileSync(statePath, "utf8")); - } catch { /* fresh file */ } - fs.writeFileSync(statePath, JSON.stringify({ ...existing, welcome_shown: true }, null, 2) + "\n"); - } catch { - // Non-critical — don't crash the extension - } -} - -// ─── Launcher (at-most-once guard) ────────────────────────────────────────── - -export interface LauncherCallbacks { - resolveFlags: () => OnboardingFlags; - showWebview: () => void; - openChat: () => void; - openChatAtStage: (stage: number) => void; -} - -/** Encapsulates the at-most-once launch logic for a VS Code window. - * Calling tryLaunch() multiple times fires the action only once. - * After webview success, onWebviewSuccess() opens chat. */ -export class OnboardingLauncher { - private launched = false; - private callbacks: LauncherCallbacks; - - constructor(callbacks: LauncherCallbacks) { - this.callbacks = callbacks; - } - - /** Attempt to launch the onboarding flow. Fires at most once per instance. */ - tryLaunch(): void { - if (this.launched) return; - - const flags = this.callbacks.resolveFlags(); - const action = resolveOnboardingAction(flags); - - if (action === "normal-session") return; // nothing to do - - this.launched = true; - - switch (action) { - case "show-webview": - this.callbacks.showWebview(); - break; - case "open-chat": - this.callbacks.openChat(); - break; - case "resume-chat-at-stage": - this.callbacks.openChatAtStage(flags.partialStage ?? 1); - break; - } - } - - /** Called when the Stage 0 webview completes successfully. - * Transitions to the chat panel. */ - onWebviewSuccess(): void { - this.callbacks.openChat(); - } -} - // ─── Helpers ───────────────────────────────────────────────────────────────── // (defaultConfigPath removed — isModelConfigured checks both .json and .jsonc) diff --git a/packages/extension/test/onboarding_routing.test.ts b/packages/extension/test/onboarding_routing.test.ts index ed400d2f..3e155ad6 100644 --- a/packages/extension/test/onboarding_routing.test.ts +++ b/packages/extension/test/onboarding_routing.test.ts @@ -14,9 +14,6 @@ import { resolveOnboardingAction, type OnboardingAction, isModelConfigured, - OnboardingLauncher, - readWelcomeShown, - writeWelcomeShown, } from "../src/onboarding_routing"; import { @@ -31,37 +28,37 @@ describe("resolveOnboardingAction — routing predicate (AC10)", () => { const cases: Array<{ name: string; flags: OnboardingFlags; expected: OnboardingAction }> = [ { name: "fresh install, no model → show-webview", - flags: { modelConfigured: false, welcomeShown: false, onboardingCompleted: false, partialStage: undefined }, + flags: { modelConfigured: false, onboardingCompleted: false, partialStage: undefined }, expected: "show-webview", }, { name: "no model, welcome already shown → show-webview (need model before chat)", - flags: { modelConfigured: false, welcomeShown: true, onboardingCompleted: false, partialStage: undefined }, + flags: { modelConfigured: false, onboardingCompleted: false, partialStage: undefined }, expected: "show-webview", }, { name: "model configured, no onboarding done → open-chat (overture will run inside)", - flags: { modelConfigured: true, welcomeShown: false, onboardingCompleted: false, partialStage: undefined }, + flags: { modelConfigured: true, onboardingCompleted: false, partialStage: undefined }, expected: "open-chat", }, { name: "model configured, partial at stage 2 → resume-chat-at-stage", - flags: { modelConfigured: true, welcomeShown: true, onboardingCompleted: false, partialStage: 2 }, + flags: { modelConfigured: true, onboardingCompleted: false, partialStage: 2 }, expected: "resume-chat-at-stage", }, { name: "model configured, onboarding completed → normal-session", - flags: { modelConfigured: true, welcomeShown: true, onboardingCompleted: true, partialStage: undefined }, + flags: { modelConfigured: true, onboardingCompleted: true, partialStage: undefined }, expected: "normal-session", }, { name: "onboarding completed (regardless of other flags) → normal-session", - flags: { modelConfigured: true, welcomeShown: false, onboardingCompleted: true, partialStage: undefined }, + flags: { modelConfigured: true, onboardingCompleted: true, partialStage: undefined }, expected: "normal-session", }, { name: "model configured, welcome shown, no partial stage → open-chat", - flags: { modelConfigured: true, welcomeShown: true, onboardingCompleted: false, partialStage: undefined }, + flags: { modelConfigured: true, onboardingCompleted: false, partialStage: undefined }, expected: "open-chat", }, ]; @@ -75,6 +72,41 @@ describe("resolveOnboardingAction — routing predicate (AC10)", () => { // ─── AC9: isModelConfigured ────────────────────────────────────────────────── +describe("first-run routing — env-var providers count as configured", () => { + // extension.ts feeds `isModelConfigured() || hasProviderEnvVar()` into the + // predicate. A machine whose only credential is an env var must NOT be sent + // to the Stage 0 model-setup webview. + it("a provider env var alone routes past the webview", () => { + expect( + resolveOnboardingAction({ + modelConfigured: true, // isModelConfigured() || hasProviderEnvVar() + onboardingCompleted: false, + partialStage: undefined, + }), + ).toBe("open-chat"); + }); + + it("no model and no completion marker is the Stage 0 case", () => { + expect( + resolveOnboardingAction({ + modelConfigured: false, + onboardingCompleted: false, + partialStage: undefined, + }), + ).toBe("show-webview"); + }); + + it("a completed onboarding never re-runs, even with no model", () => { + expect( + resolveOnboardingAction({ + modelConfigured: false, + onboardingCompleted: true, + partialStage: undefined, + }), + ).toBe("normal-session"); + }); +}); + describe("isModelConfigured — model-presence check (AC9)", () => { let tmpDir: string; @@ -119,131 +151,10 @@ describe("isModelConfigured — model-presence check (AC9)", () => { // ─── AC4: At-most-once guard ───────────────────────────────────────────────── -describe("OnboardingLauncher — at-most-once guard (AC4)", () => { - it("fires the launch callback at most once per instance", () => { - const launches: string[] = []; - const launcher = new OnboardingLauncher({ - resolveFlags: () => ({ - modelConfigured: false, - welcomeShown: false, - onboardingCompleted: false, - partialStage: undefined, - }), - showWebview: () => { launches.push("webview"); }, - openChat: () => { launches.push("chat"); }, - openChatAtStage: () => { launches.push("resume"); }, - }); - - launcher.tryLaunch(); - launcher.tryLaunch(); - launcher.tryLaunch(); - - expect(launches).toEqual(["webview"]); // only once - }); - - it("does not fire for normal-session action", () => { - const launches: string[] = []; - const launcher = new OnboardingLauncher({ - resolveFlags: () => ({ - modelConfigured: true, - welcomeShown: true, - onboardingCompleted: true, - partialStage: undefined, - }), - showWebview: () => { launches.push("webview"); }, - openChat: () => { launches.push("chat"); }, - openChatAtStage: () => { launches.push("resume"); }, - }); - - launcher.tryLaunch(); - expect(launches).toEqual([]); // normal session → no action - }); - - it("routes to openChat when model is configured", () => { - const launches: string[] = []; - const launcher = new OnboardingLauncher({ - resolveFlags: () => ({ - modelConfigured: true, - welcomeShown: false, - onboardingCompleted: false, - partialStage: undefined, - }), - showWebview: () => { launches.push("webview"); }, - openChat: () => { launches.push("chat"); }, - openChatAtStage: () => { launches.push("resume"); }, - }); - - launcher.tryLaunch(); - expect(launches).toEqual(["chat"]); - }); - - it("routes to openChatAtStage for partial state", () => { - const launches: string[] = []; - const launcher = new OnboardingLauncher({ - resolveFlags: () => ({ - modelConfigured: true, - welcomeShown: true, - onboardingCompleted: false, - partialStage: 3, - }), - showWebview: () => { launches.push("webview"); }, - openChat: () => { launches.push("chat"); }, - openChatAtStage: (_n) => { launches.push("resume"); }, - }); - - launcher.tryLaunch(); - expect(launches).toEqual(["resume"]); - }); -}); - // ─── AC5: Stage 0 success → chat auto-open ─────────────────────────────────── -describe("OnboardingLauncher — webview success triggers chat (AC5)", () => { - it("onWebviewSuccess opens chat", () => { - const launches: string[] = []; - const launcher = new OnboardingLauncher({ - resolveFlags: () => ({ - modelConfigured: false, - welcomeShown: false, - onboardingCompleted: false, - partialStage: undefined, - }), - showWebview: () => { launches.push("webview"); }, - openChat: () => { launches.push("chat"); }, - openChatAtStage: () => { launches.push("resume"); }, - }); - - launcher.tryLaunch(); // shows webview - expect(launches).toEqual(["webview"]); - - launcher.onWebviewSuccess(); // webview completed → open chat - expect(launches).toEqual(["webview", "chat"]); - }); -}); - // ─── AC6: welcome_shown persistence ────────────────────────────────────────── -describe("welcome_shown flag semantics (AC6)", () => { - let tmpDir: string; - - beforeEach(() => { - tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "welcome-")); - }); - afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); - }); - - it("reading from non-existent file returns false", () => { - expect(readWelcomeShown(path.join(tmpDir, "state.json"))).toBe(false); - }); - - it("writing and reading round-trips", () => { - const file = path.join(tmpDir, "state.json"); - writeWelcomeShown(file); - expect(readWelcomeShown(file)).toBe(true); - }); -}); - // ─── devtools restore marker — toggle-OFF guard ───────────────────────────── describe("devtools restore marker — toggle-OFF guard", () => { From e912ec9cfa7282bff3ff08948acfcba380383473 Mon Sep 17 00:00:00 2001 From: kate bonner Date: Thu, 27 Aug 2026 15:11:45 -0400 Subject: [PATCH 2/2] fix(onboarding): re-route post-ready after a serverManager replacement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The actual reason a first run never reached Stage 0. `respawnForVault()` runs on every genuine first run — `ensureDefaultPersonalVault()` provisions a vault, then respawns — and it CONSTRUCTS A NEW ServerManager with its own onReady: serverManager.onReady((url) => { opencodeReadyUrl = url; statusBar?.setServerReady(true); sseClient?.connect(url); }); No onboarding gate, no chat open. It replaces the boot manager, so the boot handler — the only one that routes onboarding — is orphaned, and the server that actually comes up is owned by a handler that just wires SSE. Stage 0 never opens and neither does the chat: the user lands on whatever surface happens to exist. Confirmed from a virgin-HOME run: `[sse] connecting` was logged (respawn's handler) while the routing log never appeared (boot's handler, orphaned). The post-ready routing is now a named `routePostReady`, assigned at boot and invoked by the respawn handler too. Verified on a clean first run: [onboarding] action=show-webview modelConfigured=false providerEnv=false completed=false autoOpen=true [onboarding] Stage 0 webview opened [pasqal] python provisioned: ... — restart deferred (onboarding in progress) The solver-mode switch in watchSolverMode() replaces serverManager the same way and is left alone here: it fires post-onboarding, where re-running the routing would re-open chat mid-switch. Noted in the PR as its own decision. --- packages/extension/src/extension.ts | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/extension/src/extension.ts b/packages/extension/src/extension.ts index bce0a2ca..ef89382c 100644 --- a/packages/extension/src/extension.ts +++ b/packages/extension/src/extension.ts @@ -95,6 +95,11 @@ let statusBar: StatusBarManager | undefined; let sseClient: OpencodeEventClient | undefined; let runsManager: RunsManager | undefined; let opencodeReadyUrl: URL | undefined; +// Post-ready routing (onboarding gate + provider signal). Assigned at boot, and +// re-invoked by every path that REPLACES serverManager — a replacement registers +// its own onReady, so without this the boot handler is orphaned and the first +// run silently loses its Stage 0 surface. +let routePostReady: ((url: URL) => void) | undefined; /** Set once the binary + vault are known; the watcher's onRunFinished closure * and the distillNow command read it lazily (undefined = distiller disabled). */ let distillerSetup: DistillerSetup | undefined; @@ -844,10 +849,7 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { }); ctx.subscriptions.push(sseClient); - serverManager.onReady((url) => { - opencodeReadyUrl = url; - statusBar?.setServerReady(true); - sseClient?.connect(url); + routePostReady = (url) => { // Onboarding gate: if no model is configured, open the Stage 0 webview // instead of chat. The webview will fire onOnboardingComplete when done, // which then opens chat. @@ -906,6 +908,13 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { : `[boot] LLM provider: ${sig.reason} → ${sig.fix}`, ); }); + }; + + serverManager.onReady((url) => { + opencodeReadyUrl = url; + statusBar?.setServerReady(true); + sseClient?.connect(url); + routePostReady?.(url); }); serverManager.start().catch((err) => { @@ -971,6 +980,7 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { opencodeReadyUrl = url; statusBar?.setServerReady(true); sseClient?.connect(url); + routePostReady?.(url); }); await serverManager.start(); if (project2.vaultDir) {