From aa4645109a1628c20e967ffbaf7e81628972bd22 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Thu, 17 Sep 2026 18:34:41 -0700 Subject: [PATCH 1/7] docs(onboarding): decompose the fresh-install first-run fixes Refs #995 --- docs/decomposition/onboarding-first-run.md | 126 +++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 docs/decomposition/onboarding-first-run.md diff --git a/docs/decomposition/onboarding-first-run.md b/docs/decomposition/onboarding-first-run.md new file mode 100644 index 000000000..1f2dc129b --- /dev/null +++ b/docs/decomposition/onboarding-first-run.md @@ -0,0 +1,126 @@ +# Decomposition: usable first run without pre-installed CLIs + +Refs #995 (audit + acceptance criteria), #994 (opencode bundling, in flight), #993 (state-lock incident) + +## A — what exists and is trusted (named) + +- `src/main/setup/runtimeTools.ts` — bundled-artifact resolution (tmux/mitmproxy/cloudflared), + manifest-pinned, noexec-probed, asar-unpack aware. Trusted; do not redesign. +- `src/main/setup/binaryResolver.ts` + `prerequisites.ts` — PATH/login-shell/well-known-dir + probing with manual-override precedence and persisted cache (`setupState.ts`). +- `src/main/sessionManager.ts:2556-2587` — spawn-time absolute-path-or-fail + late re-resolve. +- `src/renderer/src/features/setup/ui/SetupGate.tsx` — the gate overlay as it exists. +- `src/renderer/src/workspace/hook/persistence/useBootstrap.ts` — fresh-install bootstrap. +- The three read-only audits of 2026-09-18 (issue #995) — evidence base. + +## D — end state in observable behavior + +A clean machine (no brew, no git CLT, no provider CLIs, fresh `~/.config/agent-code`) can: +1. Open the app and reach a usable workspace **without ever installing claude or codex** — + via a terminal pane, or via bundled opencode once #994 lands. +2. See, per missing provider, a copyable install command and a link in the gate. +3. Always have "Continue anyway" (with an explicit acknowledgment) instead of a lockout. +4. Keep autosave/persistence on even when the first spawn fails or never happens. +5. Get a first-project cwd of the home directory, never `/`. +6. Reopen Setup from the menu/command palette; the "open Setup to locate it" toast opens it. +7. See provider pickers mark missing providers instead of failing after cwd selection. + +## Stage 1 — Fresh-run recorder (instrumentation; no visible product change) + +- **Produces**: `testing/first-run/fixtures/` — recorded outputs of the REAL + `checkPrerequisites()` and `workspace:default-cwd` handler and the bootstrap decision + (`useBootstrap` state transitions), captured by a harness that runs the real main-process + code against (a) a temp STATE_DIR with stripped PATH (simulated clean machine) and + (b) the current dev machine (rich PATH), on current `main` BEFORE any policy change. +- **Verified by**: harness runs green in `vitest --project system` and the fixture files + exist and show the clean-machine WALL (gate not ready) and cwd=`/` — the recorded + baseline that every later test asserts against. +- **Why separate**: stages 2–6 must be tested against recorded reality, not typed-in + literals. Merging the recorder forward = imagined fixtures = vanity tests. +- **Reality check**: it records the actual code paths the audits cited + (prerequisites.ts:85-165, ipc/workspace.ts:64-66, useBootstrap.ts:87-121). + +## Stage 2 — Readiness policy (the wall) + +- **Produces**: `src/main/setup/readiness.ts` — single module emitting one readiness + object (`{ usableProviders, blocking: never, warnings, installHints }`); gate becomes + dismissible-with-acknowledgment whenever zero providers are usable; per-provider + copyable install commands (from the same strings `cliUpdateOrchestrator.ts:589,622` + already uses) + links in `SetupGate.tsx`. `required:true` flags die for providers. +- **Verified by**: tests from Stage-1 fixtures — clean-machine fixture now yields + "continue allowed (terminal-only)" + warning instead of `ready:false`; claude-only + machine yields continue; both-providers machine unchanged. +- **Why separate**: policy semantics must exist in exactly one place before UI, bootstrap + and toasts all consume it; otherwise each consumer re-derives requiredness (the exact + ownership bug class the audits found). +- **Reality check**: built directly on recorded fixture #1 (the wall) and #2 (dev machine). +- **Isolation**: ONLY the gate, bootstrap (stage 4), and spawn toast (stage 3) may import + `readiness.ts`. Forbidden: any other module re-computing required/blocked. + +## Stage 3 — Reopenable gate + actionable toast + Settings tool paths + +- **Produces**: "Open Setup…" menu entry (`appMenu.ts`) + command palette command; + spawn-error toast action that opens the gate; Settings → Setup rows for manual tool + paths reusing the existing `setup:set-tool-path` IPC. +- **Verified by**: unit/system tests that the command dispatches to the gate surface; + recorded-fixture test that the sessionManager toast string resolves to a real action. +- **Why separate**: pure surface wiring over stage-2 policy; independent of bootstrap. +- **Reality check**: audit F2/dead-end #2 and #6 (gate-only override). + +## Stage 4 — Bootstrap de-race + persistence latch + +- **Produces**: gate-aware bootstrap in `useBootstrap.ts`: never spawn while the gate + blocks; on continue-without-providers open a terminal-pane default project; + `bootstrapComplete` no longer hostage to first-spawn failure (persistence stays on). +- **Verified by**: Stage-1 recorded bootstrap transitions replayed as system tests: + missing-CLI run must end `bootstrapComplete === true`, zero-spawn, persistence active. +- **Why separate**: touching persistence latches while also changing policy hides which + change fixed what; the recorded baseline pins the current broken behavior first. +- **Reality check**: audit F2/F4 with exact file:line evidence. + +## Stage 5 — cwd + picker availability + +- **Produces**: `workspace:default-cwd` falls back to homedir for Finder launches; + provider pickers (`providerChoices.ts`, `PathPickerModal.tsx`, `NewAgentPlacementOverlay`) + render missing providers as disabled with "not installed — open Setup" hint from + readiness state. +- **Verified by**: fixture from Stage 1 (cwd=`/`) inverted to homedir; picker snapshot + tests with readiness injected. +- **Why separate**: small, user-visible polish isolated from policy machinery. + +## Stage 6 — First-run integration sweep + +- **Produces**: one system test simulating the entire fresh run (temp STATE_DIR, stripped + PATH, no provider CLIs): launch → gate informs → continue → terminal workspace → + persistence writes → reopen Setup from palette. Runs in CI. +- **Verified by**: it is the verification — green against real code, red on the + pre-change baseline (demonstrated once by running it against a stash). +- **Why separate**: the sweep depends on all stages; it is the acceptance test for #995. + +## What is being isolated + +`readiness.ts` (stage 2) is the hard part: reconciling tool presence, bundling (#994), +and user acknowledgment into one object. Consumers: gate UI, bootstrap, spawn toast, +pickers. Nothing else may import it or recompute its inputs. + +## Unknowns (explicit) + +1. **#994 overlap**: the bundling branch touches `prerequisites.ts`/`toolchain.ts`/ + `runtimeTools.ts` concurrently. Sequencing: rebase this branch after #994 merges; + readiness must treat bundled opencode as a usable provider via its `source:'bundled'`. +2. **Install-command copy per provider**: must match reality (npm specs in + `cliUpdateOrchestrator.ts:589,622`); verify opencode's canonical install string once + #994 lands (bundled = no command needed). +3. **Terminal-only default project shape**: what exactly bootstrap should create on + continue-without-providers (single tmux/direct-PTY pane project in Dispatch?) — needs + one UX decision from the user before stage 4. +4. **Login detection** (provider-not-ready forever-panes) — explicitly OUT of scope here; + follow-up issue after this lands. +5. **App self-update** — separate issue, not in this decomposition. + +## Fixture plan + +All behavioral fixtures come from Stage 1's recorder: two environments × three code paths +(prerequisites check, default-cwd, bootstrap transitions), stored as JSON under +`testing/first-run/fixtures/` with the harness committed beside them. No hand-typed +plausible-looking inputs anywhere in this feature's tests. From e4202cf3bba1e33d0f0e62361b40d22c47e81d4a Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 19 Sep 2026 02:19:36 -0700 Subject: [PATCH 2/7] test(first-run): record the real prerequisite check on a simulated clean Mac (#995 stage 1) The recorder runs the REAL checkPrerequisites with only the machine's edges simulated: - a fresh HOME, where every provider CLI installs on macOS; - launchd's PATH; - /bin/sh; - a temp Electron app path, which for the packaged case holds the OpenCode runtime tree #994 ships. The recordings were taken on main 82babd21, before any policy change, and show the wall. Both clean machines report ready:false with blocking [claude, codex], even the packaged one where OpenCode is bundled and usable, and even though Grok was found. Without RECORD_FIRST_RUN=1 the test re-runs both clean simulations live and fails if a provider row no longer matches its recording, so the fixtures cannot drift from what the probes report. Rows found at a machine-wide path (this Mac's npm-global Grok under /opt/homebrew) are recording-machine facts and are not compared. The README explains this. Co-Authored-By: Claude Opus 5 (1M context) --- testing/fixtures/first-run/README.md | 39 ++++ .../first-run/clean-machine-packaged.json | 55 +++++ testing/fixtures/first-run/clean-machine.json | 54 +++++ .../fixtures/first-run/developer-machine.json | 54 +++++ .../first-run/prerequisites.firstRun.test.ts | 195 ++++++++++++++++++ 5 files changed, 397 insertions(+) create mode 100644 testing/fixtures/first-run/README.md create mode 100644 testing/fixtures/first-run/clean-machine-packaged.json create mode 100644 testing/fixtures/first-run/clean-machine.json create mode 100644 testing/fixtures/first-run/developer-machine.json create mode 100644 testing/system/first-run/prerequisites.firstRun.test.ts diff --git a/testing/fixtures/first-run/README.md b/testing/fixtures/first-run/README.md new file mode 100644 index 000000000..1b77b18f1 --- /dev/null +++ b/testing/fixtures/first-run/README.md @@ -0,0 +1,39 @@ +# First-run recordings (#995) + +Recorded by `testing/system/first-run/prerequisites.firstRun.test.ts`: the REAL +`checkPrerequisites()` from `src/main/setup/prerequisites.ts`, run with only the +machine's edges simulated (HOME, PATH, SHELL, and Electron's app path). Nothing +in these files was typed by hand. + +Re-record on a Mac with: + +```sh +RECORD_FIRST_RUN=1 npx vitest run --project system testing/system/first-run +``` + +| File | What it simulates | +|---|---| +| `clean-machine.json` | A fresh HOME, launchd's minimal PATH and `/bin/sh`: a Mac that never ran a provider installer, running a build without bundled OpenCode. | +| `clean-machine-packaged.json` | The same Mac running the packaged app, which ships OpenCode under `out/main/runtime/opencode` (#994). The binary is a stub at the path the real resolver computes, because the check only asks whether it exists. | +| `developer-machine.json` | The recording developer's real environment, with every provider CLI installed. It is machine-specific, so it is used only as policy input and never replayed live. | + +## What the recordings show + +- **The wall (baseline, recorded on main `82babd21` before any #995 change).** Both + clean recordings report `ready: false, blocking: ["claude", "codex"]`. That + includes the packaged one, where OpenCode is bundled and usable, and the Grok + row, which is found. A machine with a working provider was locked out because + it lacked two particular ones. `verdictAtRecording` keeps that verdict + verbatim. +- **Machine-wide residue.** System locations outside HOME stay visible to the + probes: `/usr/bin/git`, and Homebrew in `/opt/homebrew`. On the recording + machine that includes `/opt/homebrew/bin/grok`, an npm-global install of + `@xai-official/grok` under Homebrew's node. A factory-fresh Mac has none of + them. They are recorded as-is: + - The live drift check skips rows found at a machine-wide path. + - The policy tests treat the clean recording as a "Grok is the only provider" + machine. That is a real shape: before #995 it was also locked out. + - The zero-provider case is covered by the macOS CI runner, which has no + provider CLI and runs the same simulation live on every push. + +Paths under the recording HOME are written as `~`, so the files carry no username. diff --git a/testing/fixtures/first-run/clean-machine-packaged.json b/testing/fixtures/first-run/clean-machine-packaged.json new file mode 100644 index 000000000..b76b8c4a6 --- /dev/null +++ b/testing/fixtures/first-run/clean-machine-packaged.json @@ -0,0 +1,55 @@ +{ + "environment": "clean-machine-packaged", + "description": "The same clean Mac running the packaged app, which ships OpenCode under out/main/runtime/opencode (#994).", + "platform": "darwin", + "arch": "arm64", + "tools": { + "brew": { + "id": "brew", + "found": true, + "path": "/opt/homebrew/bin/brew", + "source": "system" + }, + "claude": { + "id": "claude", + "found": false, + "path": null + }, + "codex": { + "id": "codex", + "found": false, + "path": null + }, + "opencode": { + "id": "opencode", + "found": true, + "path": null, + "source": "bundled" + }, + "grok": { + "id": "grok", + "found": true, + "path": "/opt/homebrew/bin/grok", + "source": "system" + }, + "git": { + "id": "git", + "found": true, + "path": "/usr/bin/git", + "source": "system" + }, + "mitmdump": { + "id": "mitmdump", + "found": true, + "path": "/opt/homebrew/bin/mitmdump", + "source": "system" + } + }, + "verdictAtRecording": { + "ready": false, + "blocking": [ + "claude", + "codex" + ] + } +} diff --git a/testing/fixtures/first-run/clean-machine.json b/testing/fixtures/first-run/clean-machine.json new file mode 100644 index 000000000..c5402688f --- /dev/null +++ b/testing/fixtures/first-run/clean-machine.json @@ -0,0 +1,54 @@ +{ + "environment": "clean-machine", + "description": "Fresh HOME, launchd PATH, /bin/sh, no staged runtimes: a Mac that never ran a provider installer, on a build without bundled OpenCode.", + "platform": "darwin", + "arch": "arm64", + "tools": { + "brew": { + "id": "brew", + "found": true, + "path": "/opt/homebrew/bin/brew", + "source": "system" + }, + "claude": { + "id": "claude", + "found": false, + "path": null + }, + "codex": { + "id": "codex", + "found": false, + "path": null + }, + "opencode": { + "id": "opencode", + "found": false, + "path": null + }, + "grok": { + "id": "grok", + "found": true, + "path": "/opt/homebrew/bin/grok", + "source": "system" + }, + "git": { + "id": "git", + "found": true, + "path": "/usr/bin/git", + "source": "system" + }, + "mitmdump": { + "id": "mitmdump", + "found": true, + "path": "/opt/homebrew/bin/mitmdump", + "source": "system" + } + }, + "verdictAtRecording": { + "ready": false, + "blocking": [ + "claude", + "codex" + ] + } +} diff --git a/testing/fixtures/first-run/developer-machine.json b/testing/fixtures/first-run/developer-machine.json new file mode 100644 index 000000000..e246aca23 --- /dev/null +++ b/testing/fixtures/first-run/developer-machine.json @@ -0,0 +1,54 @@ +{ + "environment": "developer-machine", + "description": "The recording developer's real HOME, PATH and SHELL, with every provider CLI installed. Machine-specific; replayed only as policy input.", + "platform": "darwin", + "arch": "arm64", + "tools": { + "brew": { + "id": "brew", + "found": true, + "path": "/opt/homebrew/bin/brew", + "source": "system" + }, + "claude": { + "id": "claude", + "found": true, + "path": "~/.local/bin/claude", + "source": "system" + }, + "codex": { + "id": "codex", + "found": true, + "path": "~/.local/bin/codex", + "source": "system" + }, + "opencode": { + "id": "opencode", + "found": true, + "path": "~/.opencode/bin/opencode", + "source": "system" + }, + "grok": { + "id": "grok", + "found": true, + "path": "~/.local/bin/grok", + "source": "system" + }, + "git": { + "id": "git", + "found": true, + "path": "/opt/homebrew/bin/git", + "source": "system" + }, + "mitmdump": { + "id": "mitmdump", + "found": true, + "path": "/opt/homebrew/bin/mitmdump", + "source": "system" + } + }, + "verdictAtRecording": { + "ready": true, + "blocking": [] + } +} diff --git a/testing/system/first-run/prerequisites.firstRun.test.ts b/testing/system/first-run/prerequisites.firstRun.test.ts new file mode 100644 index 000000000..7d2adb39f --- /dev/null +++ b/testing/system/first-run/prerequisites.firstRun.test.ts @@ -0,0 +1,195 @@ +import { chmod, copyFile, mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { homedir, tmpdir } from 'node:os' +import { join, resolve } from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' + +import type { SetupCheckResult, SetupToolId } from '@shared/types/setup' + +// Stage 1 of docs/decomposition/onboarding-first-run.md (#995): the fresh-run +// recorder. +// +// WHY this runs the REAL checkPrerequisites instead of feeding a typed-in +// SetupCheckResult to the policy: every layer that decides "is Claude +// installed?" (login-shell `command -v`, the PATH + well-known-dir scan, the +// persisted-path fallback, bundled-archive detection) reads the machine. A +// fixture someone typed would encode what they THINK a clean Mac looks like. +// Here the machine is simulated at its edges only, and every probe runs: +// +// - HOME is a fresh temp dir. On macOS every provider CLI installs under +// HOME (~/.local/bin for Claude's native installer, Codex's and Grok's; +// ~/.opencode/bin for OpenCode), so a fresh HOME is a machine that never +// ran an installer. STATE_DIR (~/.config/agent-code) follows HOME, which is +// also "first launch ever". +// - PATH is launchd's minimal PATH, which is what a Finder/Dock launch gets. +// - SHELL is /bin/sh, so the login-shell probe sources only the system +// profile (path_helper), never this developer's rc files. +// - Electron's app path is a temp dir. `clean-machine` has nothing there +// (a dev build that never staged runtimes, or any pre-#994 build). +// `clean-machine-packaged` stages exactly the tree the packaged app ships +// for OpenCode: the manifest and the binary at the path the real resolver +// computes (#994, runtimeTools.ts opencodeBinaryPath). +// +// System-level tools (/usr/bin/git, Homebrew in /opt/homebrew) stay visible +// because they are outside HOME. That is recorded, not hidden: the policy +// treats them as optional, and the live check below compares provider rows +// only, which are the rows that are deterministic on every macOS machine +// including the CI runner. +// +// RECORD_FIRST_RUN=1 rewrites testing/fixtures/first-run/*.json from this +// machine. Without it, the test re-runs the clean simulations live and fails if +// the committed recordings no longer match what the real code reports, so a +// fixture cannot silently drift away from reality. + +const appRoot = { path: '' } +vi.mock('electron', () => ({ + app: { + getAppPath: () => appRoot.path, + getPath: (name: string) => join(appRoot.path, name), + isPackaged: false, + }, +})) + +const REPO = resolve(__dirname, '../../..') +const FIXTURES = join(REPO, 'testing/fixtures/first-run') +const RECORD = process.env.RECORD_FIRST_RUN === '1' +const PROVIDER_ROWS: SetupToolId[] = ['claude', 'codex', 'opencode', 'grok'] + +type Environment = 'clean-machine' | 'clean-machine-packaged' | 'developer-machine' + +/** One recorded tool row, with machine paths reduced to `~` so the fixture + * holds no username. `path` stays: it is the evidence of WHERE a tool was + * found (bundled vs ~/.local/bin vs /opt/homebrew), which the policy and a + * future reader both need. */ +export type RecordedTool = Pick +export type FirstRunRecording = { + environment: Environment + /** Why this environment exists and what it simulates. */ + description: string + platform: string + arch: string + tools: Record + /** The verdict fields exactly as the code at recording time returned them. + * Kept so the baseline (the pre-#995 wall) stays legible after the policy + * changes; nothing asserts against them. */ + verdictAtRecording: Record +} + +const saved = { HOME: process.env.HOME, PATH: process.env.PATH, SHELL: process.env.SHELL } +const temps: string[] = [] +afterEach(async () => { + for (const [key, value] of Object.entries(saved)) { + if (value === undefined) delete process.env[key] + else process.env[key] = value + } + for (const dir of temps.splice(0)) await rm(dir, { recursive: true, force: true }) +}) + +async function temp(prefix: string): Promise { + const dir = await mkdtemp(join(tmpdir(), prefix)) + temps.push(dir) + return dir +} + +/** Stages the packaged OpenCode runtime exactly where runtimeTools.ts looks. */ +async function stageBundledOpencode(root: string): Promise { + const manifestPath = join(REPO, 'third_party/opencode/manifest.json') + const manifest = JSON.parse(await readFile(manifestPath, 'utf8')) as { executableInsideArchive: string } + const arch = process.arch === 'x64' ? 'x86_64' : process.arch + const runtime = join(root, 'out/main/runtime/opencode') + await mkdir(join(runtime, `darwin-${arch}`), { recursive: true }) + await copyFile(manifestPath, join(runtime, 'manifest.json')) + const binary = join(runtime, `darwin-${arch}`, manifest.executableInsideArchive) + // A stub, not the real 100 MB binary: the setup check asks only whether the + // file exists (isBundledArchiveAvailable). The toolchain's separate + // `--version` probe may reject the stub, which affects only its spawn + // override cache, never the check result recorded here. + await writeFile(binary, '#!/bin/sh\nexit 0\n') + await chmod(binary, 0o755) +} + +async function runCheck(environment: Environment): Promise { + appRoot.path = await temp('first-run-app-') + if (environment !== 'developer-machine') { + process.env.HOME = await temp('first-run-home-') + process.env.PATH = '/usr/bin:/bin:/usr/sbin:/sbin' + process.env.SHELL = '/bin/sh' + } + if (environment === 'clean-machine-packaged') await stageBundledOpencode(appRoot.path) + // Fresh modules per environment: STATE_DIR and the well-known bin dirs are + // computed from HOME at module load. + vi.resetModules() + const { checkPrerequisites } = await import('@main/setup/prerequisites.js') + return await checkPrerequisites() +} + +function sanitize(path: string | null, home: string): string | null { + if (!path) return path + return path.startsWith(home) ? `~${path.slice(home.length)}` : path +} + +function record(environment: Environment, description: string, result: SetupCheckResult, home: string): FirstRunRecording { + const { tools, checkedAt: _checkedAt, ...verdict } = result + return { + environment, + description, + platform: process.platform, + arch: process.arch, + tools: Object.fromEntries( + Object.entries(tools).map(([id, tool]) => [ + id, + { id: tool.id, found: tool.found, path: sanitize(tool.path, home), source: tool.source }, + ]), + ) as Record, + verdictAtRecording: verdict, + } +} + +async function load(environment: Environment): Promise { + return JSON.parse(await readFile(join(FIXTURES, `${environment}.json`), 'utf8')) as FirstRunRecording +} + +const DESCRIPTIONS: Record = { + 'clean-machine': + 'Fresh HOME, launchd PATH, /bin/sh, no staged runtimes: a Mac that never ran a provider installer, on a build without bundled OpenCode.', + 'clean-machine-packaged': + 'The same clean Mac running the packaged app, which ships OpenCode under out/main/runtime/opencode (#994).', + 'developer-machine': + "The recording developer's real HOME, PATH and SHELL, with every provider CLI installed. Machine-specific; replayed only as policy input.", +} + +describe.skipIf(process.platform !== 'darwin')('first-run prerequisites on a simulated clean Mac (#995)', () => { + it.skipIf(!RECORD)('records every environment', async () => { + await mkdir(FIXTURES, { recursive: true }) + for (const environment of Object.keys(DESCRIPTIONS) as Environment[]) { + const result = await runCheck(environment) + const home = process.env.HOME ?? homedir() + const recording = record(environment, DESCRIPTIONS[environment], result, home) + await writeFile(join(FIXTURES, `${environment}.json`), `${JSON.stringify(recording, null, 2)}\n`) + for (const [key, value] of Object.entries(saved)) { + if (value === undefined) delete process.env[key] + else process.env[key] = value + } + } + }, 120_000) + + it.each(['clean-machine', 'clean-machine-packaged'] as const)( + '%s: the committed recording still matches the real probes for every provider row', + async environment => { + const recording = await load(environment) + const live = await runCheck(environment) + // A row the recording found at an absolute, machine-wide path (outside + // the simulated HOME) is a fact about the recording machine, not about + // a clean Mac: here, Grok is an npm-global install under Homebrew's + // node (/opt/homebrew/bin/grok). The CI runner has no such install, so + // comparing that row would fail for a reason unrelated to the code. + // Rows found under HOME or bundled, and rows not found, are + // deterministic on any machine and are always compared. + const machineWide = (id: SetupToolId) => recording.tools[id].path?.startsWith('/') === true + for (const id of PROVIDER_ROWS.filter(id => !machineWide(id))) { + expect({ id, found: live.tools[id].found, source: live.tools[id].source }) + .toEqual({ id, found: recording.tools[id].found, source: recording.tools[id].source }) + } + }, + 60_000, + ) +}) From 0011d9693621d3549f786de5bb2b99b203b8e6fb Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 19 Sep 2026 02:47:09 -0700 Subject: [PATCH 3/7] feat(setup): no tool blocks launch; one readiness policy decides the first project (#995 stage 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude Code and Codex were `required`, so a Mac without both met a SetupGate with no Continue button. The stage-1 recordings show the wall: - the packaged app on a clean Mac, with OpenCode bundled and usable, was ready:false and blocked on [claude, codex]; - so was a Grok-only machine. Changes: - `required` is removed from provider descriptors. SetupToolStatus.provider marks what the gate must treat as an agent CLI, without the lockout. - src/shared/setup/readiness.ts is the one policy: - usableProviders: found, bundled included; - firstSessionKind: the default provider if usable, else the first usable one in registry order, else a terminal. checkPrerequisites stamps both onto SetupCheckResult, so no consumer re-derives them. ready/blocking are gone; nothing blocks. - Each provider carries a copyable install command and docs URL. These are the providers' native installers, since a fresh Mac has no npm or brew. Each URL was checked on 2026-09-19. Grok's only published distribution is npm. Tests: - The recorder now saves the whole result as `check`. The probe rows are identical to the baseline recording, and the pre-#995 verdicts are kept in baseline-main-82babd21.json. - readiness.test.ts runs the policy over the recorded machines: - packaged clean Mac → opencode; - Grok-only → grok; - zero providers → terminal, via the one documented edit that unsets machine-wide installs; - developer machine → claude. - The live system check asserts the policy on the real probe result. On the macOS CI runner that is the genuine zero-provider Mac. Co-Authored-By: Claude Opus 5 (1M context) --- src/main/setup/prerequisites.ts | 25 +++-- src/providers/registry.setup.ts | 61 ++++++++---- src/shared/commands/nativeMenuCommandIds.ts | 3 + .../setup/firstRunRecordings.testSupport.ts | 51 ++++++++++ src/shared/setup/readiness.test.ts | 55 +++++++++++ src/shared/setup/readiness.ts | 60 +++++++++++ src/shared/types/setup.ts | 31 +++++- testing/fixtures/first-run/README.md | 14 ++- .../first-run/baseline-main-82babd21.json | 47 +++++++++ .../first-run/clean-machine-packaged.json | 98 ++++++++++++++++-- testing/fixtures/first-run/clean-machine.json | 96 ++++++++++++++++-- .../fixtures/first-run/developer-machine.json | 99 ++++++++++++++++++- .../first-run/prerequisites.firstRun.test.ts | 30 ++++-- 13 files changed, 613 insertions(+), 57 deletions(-) create mode 100644 src/shared/setup/firstRunRecordings.testSupport.ts create mode 100644 src/shared/setup/readiness.test.ts create mode 100644 src/shared/setup/readiness.ts create mode 100644 testing/fixtures/first-run/baseline-main-82babd21.json diff --git a/src/main/setup/prerequisites.ts b/src/main/setup/prerequisites.ts index 8c5bbc06d..1d9b03414 100644 --- a/src/main/setup/prerequisites.ts +++ b/src/main/setup/prerequisites.ts @@ -10,6 +10,7 @@ import { loadSetupState, updateToolPaths } from '@main/setup/setupState.js' import { listProviderSetupDescriptors } from '@providers/registry.setup.js' import { AGENT_PROVIDER_KINDS } from '@shared/types/providerKind.js' import { refreshToolchainFromState } from '@main/setup/toolchain.js' +import { deriveReadiness } from '@shared/setup/readiness.js' // WHY this map exists: not every SetupToolId has a bundled artifact, // and the `tool === 'X'` shape doesn't compose well when more tools @@ -28,18 +29,21 @@ const BUNDLED_TOOL_IDS: ReadonlySet = new Set([ // Provider SetupGate rows derived from the plain-data setup registry // (cycle-safe — see registry.setup.ts's header for why it isn't -// registry.main.ts). `installable: false` for all providers today: -// the CLIs have their own install/sign-in stories the gate can't -// automate. +// registry.main.ts). `installable: false` for all providers: that flag means +// "the gate can run the install for you" (Homebrew), and provider CLIs have +// their own installers and sign-in the gate cannot automate. What the gate +// CAN do is show the user the exact command, which is `installCommand`. const PROVIDER_TOOL_META = Object.fromEntries( listProviderSetupDescriptors().map(([kind, d]) => [ kind, { id: kind, label: d.label, - required: d.required, + provider: true, installable: false, detail: d.detail, + installCommand: d.install.command, + docsUrl: d.install.docsUrl, }, ]), ) as Record> @@ -58,14 +62,14 @@ const TOOL_META: Record> = brew: { id: 'brew', label: 'Homebrew', - required: false, + provider: false, installable: false, detail: 'Used in dev to install optional tools. Not required to launch.', }, git: { id: 'git', label: 'Git', - required: false, + provider: false, installable: false, detail: 'Used by Git Bar, worktree badges, and repository metadata.', }, @@ -78,7 +82,7 @@ const TOOL_META: Record> = mitmdump: { id: 'mitmdump', label: 'Claude Proxy Helper', - required: false, + provider: false, installable: true, detail: 'Installed by Homebrew package mitmproxy; enables Claude proxy streaming.', }, @@ -162,11 +166,12 @@ export async function checkPrerequisites(): Promise { ) await refreshToolchainFromState() - const blocking = CHECK_ORDER.filter(tool => tools[tool].required && !tools[tool].found) + // No `ready`/`blocking` any more (#995): nothing blocks launch. The policy + // that replaced them is in readiness.ts, and it runs here so every + // consumer receives the same verdict instead of re-deriving it. return { checkedAt: Date.now(), - ready: blocking.length === 0, - blocking, tools, + ...deriveReadiness(tools), } } diff --git a/src/providers/registry.setup.ts b/src/providers/registry.setup.ts index 344834068..240511792 100644 --- a/src/providers/registry.setup.ts +++ b/src/providers/registry.setup.ts @@ -27,48 +27,69 @@ export type ProviderSetupDescriptor = { binaryName: string /** Human label shown in the SetupGate row. */ label: string - /** Whether a missing binary blocks app launch. Both shipped - * providers are launch-blocking today; plug-and-play likely turns - * this into a user-level "installed providers" concept later - * (#394 §13.5) — the flag is here so that change is one edit per - * provider. */ - required: boolean + // `required: boolean` lived here until #995. Claude and Codex were + // launch-blocking, so a Mac without both CLIs met a gate it could not pass, + // even when another provider was installed or bundled. No provider is + // required now; readiness.ts decides what a first run opens with. /** SetupGate detail line. */ detail: string + /** + * How a user installs this CLI on a Mac that has nothing, shown copyable in + * the SetupGate. + * + * WHY these commands and not `npm install -g`: a fresh Mac has no Node and + * no Homebrew, so an npm or brew command fails before it starts. Each + * command is the provider's own native installer, checked on 2026-09-19 by + * fetching the URL (every one resolves to a real script). Grok is the one + * exception: its only published distribution found is the npm package + * `@xai-official/grok` (registry.npmjs.org, and the recording machine's own + * install), so its hint says npm. + */ + install: { command: string; docsUrl: string } } const claudeSetup: ProviderSetupDescriptor = { binaryName: 'claude', label: 'Claude Code', - required: true, - detail: 'Install and sign in to Claude Code before using Claude panes.', + detail: 'Install and sign in to Claude Code to use Claude panes.', + install: { + command: 'curl -fsSL https://claude.ai/install.sh | bash', + docsUrl: 'https://code.claude.com/docs/en/setup', + }, } const codexSetup: ProviderSetupDescriptor = { binaryName: 'codex', label: 'Codex', - required: true, - detail: 'Install and sign in to Codex before using Codex panes.', + detail: 'Install and sign in to Codex to use Codex panes.', + install: { + // The same installer cliUpdateOrchestrator runs for native Codex updates. + command: 'curl -fsSL https://chatgpt.com/codex/install.sh | sh', + docsUrl: 'https://github.com/openai/codex', + }, } const opencodeSetup: ProviderSetupDescriptor = { binaryName: 'opencode', label: 'OpenCode', - // FIRST required:false provider (#406 blocker 5): opencode must not - // block app launch for the vast majority of installs that don't - // have the binary. Verify the SetupGate soft-handles this before - // the branch merges. - required: false, - detail: 'Install the opencode CLI to use OpenCode panes. Optional.', + // The packaged app ships this CLI (#994), so on a release build this row + // reads "Bundled" and the install command is never needed. It matters in + // dev builds and if a user removed the bundled runtime. + detail: 'Install the opencode CLI to use OpenCode panes.', + install: { + command: 'curl -fsSL https://opencode.ai/install | bash', + docsUrl: 'https://opencode.ai/docs', + }, } const grokSetup: ProviderSetupDescriptor = { binaryName: 'grok', label: 'Grok', - // Optional like OpenCode: most installs do not have the Grok CLI, and its - // absence must not block app launch. - required: false, - detail: 'Install the Grok CLI to use Grok panes. Optional.', + detail: 'Install the Grok CLI to use Grok panes.', + install: { + command: 'npm install -g @xai-official/grok', + docsUrl: 'https://www.npmjs.com/package/@xai-official/grok', + }, } const providerSetupDescriptors: Record = { diff --git a/src/shared/commands/nativeMenuCommandIds.ts b/src/shared/commands/nativeMenuCommandIds.ts index c17878039..f876a7365 100644 --- a/src/shared/commands/nativeMenuCommandIds.ts +++ b/src/shared/commands/nativeMenuCommandIds.ts @@ -26,6 +26,9 @@ export const NATIVE_MENU_COMMAND_IDS = [ 'save-all-editor-files', 'reorder-tabs', 'close-tab', + // #995: the spawn error says "open Setup"; the File menu is where a user + // who never opened the command palette looks for it. + 'open-setup', ] as const export type NativeMenuCommandId = (typeof NATIVE_MENU_COMMAND_IDS)[number] diff --git a/src/shared/setup/firstRunRecordings.testSupport.ts b/src/shared/setup/firstRunRecordings.testSupport.ts new file mode 100644 index 000000000..f4e298c33 --- /dev/null +++ b/src/shared/setup/firstRunRecordings.testSupport.ts @@ -0,0 +1,51 @@ +import { readFileSync } from 'node:fs' +import { resolve } from 'node:path' + +import { deriveReadiness } from '@shared/setup/readiness.js' +import type { SetupCheckResult, SetupToolId } from '@shared/types/setup.js' + +/** + * Loads a first-run recording (testing/fixtures/first-run, #995) as the exact + * SetupCheckResult main sent over `setup:check`. Tests feed THIS to the gate, + * the bootstrap and the pickers instead of building a check by hand. + */ +export type FirstRunEnvironment = 'clean-machine' | 'clean-machine-packaged' | 'developer-machine' + +const FIXTURES = resolve(__dirname, '../../../testing/fixtures/first-run') + +export function loadFirstRunCheck(environment: FirstRunEnvironment): SetupCheckResult { + const recording = JSON.parse(readFileSync(resolve(FIXTURES, `${environment}.json`), 'utf8')) as { check: SetupCheckResult } + return recording.check +} + +/** + * The recording with every tool found at a MACHINE-WIDE path removed: the + * recording machine's own installs outside the simulated HOME (its npm-global + * Grok in /opt/homebrew). The result is the factory-fresh Mac with no provider + * at all. + * + * WHY an edit, stated in the one place it happens: that Mac cannot be recorded + * on the recording machine, because the probes rightly look in /opt/homebrew. + * The edit is only the `found`/`path`/`source` of those rows; readiness is then + * re-derived by the real policy, never typed. The unedited zero-provider case + * runs live on the macOS CI runner, which has no provider CLI + * (prerequisites.firstRun.test.ts). + */ +export function withoutMachineWideInstalls(check: SetupCheckResult): SetupCheckResult { + const tools = Object.fromEntries( + Object.entries(check.tools).map(([id, tool]) => + tool.provider && tool.source === 'system' && tool.path?.startsWith('/') + ? [id, { ...tool, found: false, path: null, source: undefined }] + : [id, tool], + ), + ) as Record + return { ...check, tools, ...deriveReadiness(tools) } +} + +/** The pre-#995 verdicts, recorded on main 82babd21. */ +export function loadFirstRunBaseline(): Record { + const baseline = JSON.parse(readFileSync(resolve(FIXTURES, 'baseline-main-82babd21.json'), 'utf8')) as { + environments: Record + } + return baseline.environments +} diff --git a/src/shared/setup/readiness.test.ts b/src/shared/setup/readiness.test.ts new file mode 100644 index 000000000..3d15b0158 --- /dev/null +++ b/src/shared/setup/readiness.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from 'vitest' + +import { deriveReadiness } from '@shared/setup/readiness.js' +import { + loadFirstRunBaseline, + loadFirstRunCheck, + withoutMachineWideInstalls, +} from '@shared/setup/firstRunRecordings.testSupport.js' + +// The readiness policy (#995) over the RECORDED first-run probes +// (testing/fixtures/first-run). Every input here is what the real +// checkPrerequisites reported on a simulated clean Mac, the packaged app on +// it, and a developer machine; see that folder's README. + +describe('first-run readiness over recorded machines (#995)', () => { + it('the packaged app on a clean Mac opens its first project with the bundled OpenCode', () => { + const check = loadFirstRunCheck('clean-machine-packaged') + expect(check.tools.opencode).toMatchObject({ found: true, source: 'bundled' }) + expect(deriveReadiness(check.tools).firstSessionKind).toBe('opencode') + }) + + it('a Mac whose only provider is Grok opens with Grok', () => { + // The recording machine's npm-global Grok is outside the simulated HOME, + // so the clean recording is a real "Grok only" machine. + expect(deriveReadiness(loadFirstRunCheck('clean-machine').tools)).toEqual({ usableProviders: ['grok'], firstSessionKind: 'grok' }) + }) + + it('a Mac with no provider at all opens a terminal, never a lockout', () => { + const check = withoutMachineWideInstalls(loadFirstRunCheck('clean-machine')) + expect(check.usableProviders).toEqual([]) + expect(check.firstSessionKind).toBe('terminal') + }) + + it('a machine with the default provider behaves exactly as before: Claude first', () => { + expect(deriveReadiness(loadFirstRunCheck('developer-machine').tools).firstSessionKind).toBe('claude') + }) + + it('every machine the old gate locked out had something to run', () => { + // The baseline is the pre-#995 verdict on the same recordings. Both clean + // machines were `ready: false`, blocked on Claude and Codex, while a + // provider was usable. That is the wall this policy removes. + const baseline = loadFirstRunBaseline() + for (const environment of ['clean-machine', 'clean-machine-packaged'] as const) { + expect(baseline[environment].verdict).toEqual({ ready: false, blocking: ['claude', 'codex'] }) + expect(deriveReadiness(loadFirstRunCheck(environment).tools).usableProviders.length).toBeGreaterThan(0) + } + }) + + it('what main recorded is what the policy derives, so no consumer can disagree with it', () => { + for (const environment of ['clean-machine', 'clean-machine-packaged', 'developer-machine'] as const) { + const check = loadFirstRunCheck(environment) + expect(deriveReadiness(check.tools)).toEqual({ usableProviders: check.usableProviders, firstSessionKind: check.firstSessionKind }) + } + }) +}) diff --git a/src/shared/setup/readiness.ts b/src/shared/setup/readiness.ts new file mode 100644 index 000000000..ad5437c29 --- /dev/null +++ b/src/shared/setup/readiness.ts @@ -0,0 +1,60 @@ +import { + AGENT_PROVIDER_KINDS, + DEFAULT_PROVIDER, + type AgentProviderKind, +} from '@shared/types/providerKind.js' +import type { SetupCheckResult, SetupToolId, SetupToolStatus } from '@shared/types/setup.js' + +/** + * First-run readiness: the ONE place that turns probed tool rows into "what + * can this machine run, and what does a fresh install open with" (#995). + * + * WHY a single module: before #995 the answer was spread across three + * consumers that each re-derived it. The gate read `required` flags, the + * bootstrap assumed Claude, and the spawn path discovered the truth last, by + * throwing. They disagreed exactly when it mattered: on a clean Mac the gate + * blocked, the bootstrap spawned Claude underneath it anyway, the spawn failed, + * and the run was left with no tabs and autosave off. + * + * ISOLATION: production code reaches this only through checkPrerequisites, + * which stamps the result onto SetupCheckResult. The gate, bootstrap and + * pickers read those fields and must never recompute them from `tools`. It + * lives in `shared` (not `main`) only so tests on both sides can run the same + * policy over the recorded fixtures in testing/fixtures/first-run. + * + * WHAT IS DELIBERATELY NOT HERE: + * - Any "required" tool. No single tool blocks launch: a terminal pane needs + * none, and one provider is enough for agents. The probes can also be wrong + * (#495 A1: exotic shells, Finder PATH), so a hard block on a probe result + * is a lockout with a false-negative trigger. + * - Login state. A found CLI may still be signed out; detecting that is a + * separate problem (#995 finding 7) and is out of scope here. + */ +export function deriveReadiness( + tools: Record, +): Pick { + // A bundled provider counts: `found` is already true for it (the probe + // sets found = bundled || system path). That is the whole point of #994: + // the packaged app's OpenCode makes a clean Mac usable. + const usableProviders = AGENT_PROVIDER_KINDS.filter(kind => tools[kind]?.found === true) + return { usableProviders, firstSessionKind: firstSessionKindFor(usableProviders) } +} + +/** + * The first project's session kind. + * + * WHY the default provider first, then registry order, then a terminal: + * - The default provider is what every other spawn path uses when nothing + * is chosen, so a machine that has it behaves exactly as before #995. + * - Otherwise any usable provider beats a terminal, because agents are the + * product. Registry order (AGENT_PROVIDER_KINDS) is the one ordering every + * picker already shows, so the choice is predictable, not arbitrary. + * - With no provider, a terminal is the only surface that needs nothing + * installed, and it is where a user runs the install commands the gate shows. + */ +export function firstSessionKindFor( + usableProviders: readonly AgentProviderKind[], +): AgentProviderKind | 'terminal' { + if (usableProviders.includes(DEFAULT_PROVIDER)) return DEFAULT_PROVIDER + return usableProviders[0] ?? 'terminal' +} diff --git a/src/shared/types/setup.ts b/src/shared/types/setup.ts index 91d7b6bfc..e5c1daf6b 100644 --- a/src/shared/types/setup.ts +++ b/src/shared/types/setup.ts @@ -34,20 +34,45 @@ export type SetupToolSource = 'bundled' | 'system' export type SetupToolStatus = { id: SetupToolId label: string - required: boolean + /** + * An agent provider CLI rather than a helper tool. + * + * WHY this replaced `required` (#995): `required` meant "a missing binary + * blocks app launch", and Claude Code and Codex were both required. A fresh + * Mac therefore met a gate with no Continue button, even in the packaged app + * where OpenCode ships bundled and works (testing/fixtures/first-run records + * that wall). No single tool is a launch prerequisite any more: a terminal + * pane needs none, and any ONE provider is enough for agents. The gate still + * needs to tell providers from helpers, to show install commands and the + * manual path override, and this is that distinction without the lockout. + */ + provider: boolean found: boolean path: string | null installable: boolean source?: SetupToolSource skipped?: boolean detail?: string + /** A copyable install command for a provider CLI (registry.setup.ts). */ + installCommand?: string + /** The provider's own install documentation. */ + docsUrl?: string } export type SetupCheckResult = { checkedAt: number - ready: boolean - blocking: SetupToolId[] tools: Record + /** + * Providers whose CLI resolved, in AGENT_PROVIDER_KINDS order. Computed once, + * in src/main/setup/readiness.ts. The gate, the fresh-install bootstrap and + * the provider pickers all read it; none of them re-derives it from `tools`. + */ + usableProviders: AgentProviderKind[] + /** + * What a fresh install opens its first project with: the default provider + * when it is usable, otherwise the first usable one, otherwise a terminal. + */ + firstSessionKind: AgentProviderKind | 'terminal' } // Targets the SetupGate's "Install via Homebrew" button can hand to diff --git a/testing/fixtures/first-run/README.md b/testing/fixtures/first-run/README.md index 1b77b18f1..8e1ede8c4 100644 --- a/testing/fixtures/first-run/README.md +++ b/testing/fixtures/first-run/README.md @@ -23,8 +23,14 @@ RECORD_FIRST_RUN=1 npx vitest run --project system testing/system/first-run clean recordings report `ready: false, blocking: ["claude", "codex"]`. That includes the packaged one, where OpenCode is bundled and usable, and the Grok row, which is found. A machine with a working provider was locked out because - it lacked two particular ones. `verdictAtRecording` keeps that verdict - verbatim. + it lacked two particular ones. `baseline-main-82babd21.json` keeps those + verdicts verbatim. +- **The current result.** Each recording's `check` is the whole + `SetupCheckResult`, with paths sanitized and `checkedAt` zeroed: exactly what + the renderer receives. Renderer tests load it through + `src/shared/setup/firstRunRecordings.testSupport.ts` instead of building a + check by hand. The probe rows (`tools`) were re-recorded for this field and + are identical to the baseline recording. - **Machine-wide residue.** System locations outside HOME stay visible to the probes: `/usr/bin/git`, and Homebrew in `/opt/homebrew`. On the recording machine that includes `/opt/homebrew/bin/grok`, an npm-global install of @@ -34,6 +40,8 @@ RECORD_FIRST_RUN=1 npx vitest run --project system testing/system/first-run - The policy tests treat the clean recording as a "Grok is the only provider" machine. That is a real shape: before #995 it was also locked out. - The zero-provider case is covered by the macOS CI runner, which has no - provider CLI and runs the same simulation live on every push. + provider CLI and runs the same simulation live on every push. Tests that + need it here use `withoutMachineWideInstalls()`, the one stated edit: it + unsets those rows and re-derives readiness with the real policy. Paths under the recording HOME are written as `~`, so the files carry no username. diff --git a/testing/fixtures/first-run/baseline-main-82babd21.json b/testing/fixtures/first-run/baseline-main-82babd21.json new file mode 100644 index 000000000..8fb9a50d4 --- /dev/null +++ b/testing/fixtures/first-run/baseline-main-82babd21.json @@ -0,0 +1,47 @@ +{ + "provenance": "The verdict the REAL checkPrerequisites returned on main 82babd21, before #995, recorded by testing/system/first-run/prerequisites.firstRun.test.ts. Kept verbatim as the baseline: both clean machines were locked out (ready:false) although OpenCode was usable in the packaged one and Grok was found in both.", + "environments": { + "clean-machine": { + "verdict": { + "ready": false, + "blocking": [ + "claude", + "codex" + ] + }, + "providersFound": { + "claude": false, + "codex": false, + "opencode": false, + "grok": true + } + }, + "clean-machine-packaged": { + "verdict": { + "ready": false, + "blocking": [ + "claude", + "codex" + ] + }, + "providersFound": { + "claude": false, + "codex": false, + "opencode": true, + "grok": true + } + }, + "developer-machine": { + "verdict": { + "ready": true, + "blocking": [] + }, + "providersFound": { + "claude": true, + "codex": true, + "opencode": true, + "grok": true + } + } + } +} diff --git a/testing/fixtures/first-run/clean-machine-packaged.json b/testing/fixtures/first-run/clean-machine-packaged.json index b76b8c4a6..f21796ff8 100644 --- a/testing/fixtures/first-run/clean-machine-packaged.json +++ b/testing/fixtures/first-run/clean-machine-packaged.json @@ -45,11 +45,97 @@ "source": "system" } }, - "verdictAtRecording": { - "ready": false, - "blocking": [ - "claude", - "codex" - ] + "check": { + "checkedAt": 0, + "tools": { + "brew": { + "id": "brew", + "label": "Homebrew", + "provider": false, + "installable": false, + "detail": "Used in dev to install optional tools. Not required to launch.", + "found": true, + "path": "/opt/homebrew/bin/brew", + "source": "system", + "skipped": false + }, + "claude": { + "id": "claude", + "label": "Claude Code", + "provider": true, + "installable": false, + "detail": "Install and sign in to Claude Code to use Claude panes.", + "installCommand": "curl -fsSL https://claude.ai/install.sh | bash", + "docsUrl": "https://code.claude.com/docs/en/setup", + "found": false, + "path": null, + "skipped": false + }, + "codex": { + "id": "codex", + "label": "Codex", + "provider": true, + "installable": false, + "detail": "Install and sign in to Codex to use Codex panes.", + "installCommand": "curl -fsSL https://chatgpt.com/codex/install.sh | sh", + "docsUrl": "https://github.com/openai/codex", + "found": false, + "path": null, + "skipped": false + }, + "opencode": { + "id": "opencode", + "label": "OpenCode", + "provider": true, + "installable": false, + "detail": "Install the opencode CLI to use OpenCode panes.", + "installCommand": "curl -fsSL https://opencode.ai/install | bash", + "docsUrl": "https://opencode.ai/docs", + "found": true, + "path": null, + "source": "bundled", + "skipped": false + }, + "grok": { + "id": "grok", + "label": "Grok", + "provider": true, + "installable": false, + "detail": "Install the Grok CLI to use Grok panes.", + "installCommand": "npm install -g @xai-official/grok", + "docsUrl": "https://www.npmjs.com/package/@xai-official/grok", + "found": true, + "path": "/opt/homebrew/bin/grok", + "source": "system", + "skipped": false + }, + "git": { + "id": "git", + "label": "Git", + "provider": false, + "installable": false, + "detail": "Used by Git Bar, worktree badges, and repository metadata.", + "found": true, + "path": "/usr/bin/git", + "source": "system", + "skipped": false + }, + "mitmdump": { + "id": "mitmdump", + "label": "Claude Proxy Helper", + "provider": false, + "installable": true, + "detail": "Installed by Homebrew package mitmproxy; enables Claude proxy streaming.", + "found": true, + "path": "/opt/homebrew/bin/mitmdump", + "source": "system", + "skipped": false + } + }, + "usableProviders": [ + "opencode", + "grok" + ], + "firstSessionKind": "opencode" } } diff --git a/testing/fixtures/first-run/clean-machine.json b/testing/fixtures/first-run/clean-machine.json index c5402688f..6fa4e2c40 100644 --- a/testing/fixtures/first-run/clean-machine.json +++ b/testing/fixtures/first-run/clean-machine.json @@ -44,11 +44,95 @@ "source": "system" } }, - "verdictAtRecording": { - "ready": false, - "blocking": [ - "claude", - "codex" - ] + "check": { + "checkedAt": 0, + "tools": { + "brew": { + "id": "brew", + "label": "Homebrew", + "provider": false, + "installable": false, + "detail": "Used in dev to install optional tools. Not required to launch.", + "found": true, + "path": "/opt/homebrew/bin/brew", + "source": "system", + "skipped": false + }, + "claude": { + "id": "claude", + "label": "Claude Code", + "provider": true, + "installable": false, + "detail": "Install and sign in to Claude Code to use Claude panes.", + "installCommand": "curl -fsSL https://claude.ai/install.sh | bash", + "docsUrl": "https://code.claude.com/docs/en/setup", + "found": false, + "path": null, + "skipped": false + }, + "codex": { + "id": "codex", + "label": "Codex", + "provider": true, + "installable": false, + "detail": "Install and sign in to Codex to use Codex panes.", + "installCommand": "curl -fsSL https://chatgpt.com/codex/install.sh | sh", + "docsUrl": "https://github.com/openai/codex", + "found": false, + "path": null, + "skipped": false + }, + "opencode": { + "id": "opencode", + "label": "OpenCode", + "provider": true, + "installable": false, + "detail": "Install the opencode CLI to use OpenCode panes.", + "installCommand": "curl -fsSL https://opencode.ai/install | bash", + "docsUrl": "https://opencode.ai/docs", + "found": false, + "path": null, + "skipped": false + }, + "grok": { + "id": "grok", + "label": "Grok", + "provider": true, + "installable": false, + "detail": "Install the Grok CLI to use Grok panes.", + "installCommand": "npm install -g @xai-official/grok", + "docsUrl": "https://www.npmjs.com/package/@xai-official/grok", + "found": true, + "path": "/opt/homebrew/bin/grok", + "source": "system", + "skipped": false + }, + "git": { + "id": "git", + "label": "Git", + "provider": false, + "installable": false, + "detail": "Used by Git Bar, worktree badges, and repository metadata.", + "found": true, + "path": "/usr/bin/git", + "source": "system", + "skipped": false + }, + "mitmdump": { + "id": "mitmdump", + "label": "Claude Proxy Helper", + "provider": false, + "installable": true, + "detail": "Installed by Homebrew package mitmproxy; enables Claude proxy streaming.", + "found": true, + "path": "/opt/homebrew/bin/mitmdump", + "source": "system", + "skipped": false + } + }, + "usableProviders": [ + "grok" + ], + "firstSessionKind": "grok" } } diff --git a/testing/fixtures/first-run/developer-machine.json b/testing/fixtures/first-run/developer-machine.json index e246aca23..79ce40e71 100644 --- a/testing/fixtures/first-run/developer-machine.json +++ b/testing/fixtures/first-run/developer-machine.json @@ -47,8 +47,101 @@ "source": "system" } }, - "verdictAtRecording": { - "ready": true, - "blocking": [] + "check": { + "checkedAt": 0, + "tools": { + "brew": { + "id": "brew", + "label": "Homebrew", + "provider": false, + "installable": false, + "detail": "Used in dev to install optional tools. Not required to launch.", + "found": true, + "path": "/opt/homebrew/bin/brew", + "source": "system", + "skipped": false + }, + "claude": { + "id": "claude", + "label": "Claude Code", + "provider": true, + "installable": false, + "detail": "Install and sign in to Claude Code to use Claude panes.", + "installCommand": "curl -fsSL https://claude.ai/install.sh | bash", + "docsUrl": "https://code.claude.com/docs/en/setup", + "found": true, + "path": "~/.local/bin/claude", + "source": "system", + "skipped": false + }, + "codex": { + "id": "codex", + "label": "Codex", + "provider": true, + "installable": false, + "detail": "Install and sign in to Codex to use Codex panes.", + "installCommand": "curl -fsSL https://chatgpt.com/codex/install.sh | sh", + "docsUrl": "https://github.com/openai/codex", + "found": true, + "path": "~/.local/bin/codex", + "source": "system", + "skipped": false + }, + "opencode": { + "id": "opencode", + "label": "OpenCode", + "provider": true, + "installable": false, + "detail": "Install the opencode CLI to use OpenCode panes.", + "installCommand": "curl -fsSL https://opencode.ai/install | bash", + "docsUrl": "https://opencode.ai/docs", + "found": true, + "path": "~/.opencode/bin/opencode", + "source": "system", + "skipped": false + }, + "grok": { + "id": "grok", + "label": "Grok", + "provider": true, + "installable": false, + "detail": "Install the Grok CLI to use Grok panes.", + "installCommand": "npm install -g @xai-official/grok", + "docsUrl": "https://www.npmjs.com/package/@xai-official/grok", + "found": true, + "path": "~/.local/bin/grok", + "source": "system", + "skipped": false + }, + "git": { + "id": "git", + "label": "Git", + "provider": false, + "installable": false, + "detail": "Used by Git Bar, worktree badges, and repository metadata.", + "found": true, + "path": "/opt/homebrew/bin/git", + "source": "system", + "skipped": false + }, + "mitmdump": { + "id": "mitmdump", + "label": "Claude Proxy Helper", + "provider": false, + "installable": true, + "detail": "Installed by Homebrew package mitmproxy; enables Claude proxy streaming.", + "found": true, + "path": "/opt/homebrew/bin/mitmdump", + "source": "system", + "skipped": false + } + }, + "usableProviders": [ + "claude", + "codex", + "opencode", + "grok" + ], + "firstSessionKind": "claude" } } diff --git a/testing/system/first-run/prerequisites.firstRun.test.ts b/testing/system/first-run/prerequisites.firstRun.test.ts index 7d2adb39f..a23baf0ec 100644 --- a/testing/system/first-run/prerequisites.firstRun.test.ts +++ b/testing/system/first-run/prerequisites.firstRun.test.ts @@ -68,10 +68,11 @@ export type FirstRunRecording = { platform: string arch: string tools: Record - /** The verdict fields exactly as the code at recording time returned them. - * Kept so the baseline (the pre-#995 wall) stays legible after the policy - * changes; nothing asserts against them. */ - verdictAtRecording: Record + /** The whole result, paths sanitized and `checkedAt` zeroed: exactly what + * the renderer receives over `setup:check`, so renderer tests feed the gate + * and bootstrap main's real output instead of a hand-built object. The + * pre-#995 verdicts are kept separately in baseline-main-82babd21.json. */ + check: SetupCheckResult } const saved = { HOME: process.env.HOME, PATH: process.env.PATH, SHELL: process.env.SHELL } @@ -128,7 +129,7 @@ function sanitize(path: string | null, home: string): string | null { } function record(environment: Environment, description: string, result: SetupCheckResult, home: string): FirstRunRecording { - const { tools, checkedAt: _checkedAt, ...verdict } = result + const { tools } = result return { environment, description, @@ -140,7 +141,13 @@ function record(environment: Environment, description: string, result: SetupChec { id: tool.id, found: tool.found, path: sanitize(tool.path, home), source: tool.source }, ]), ) as Record, - verdictAtRecording: verdict, + check: { + ...result, + checkedAt: 0, + tools: Object.fromEntries( + Object.entries(tools).map(([id, tool]) => [id, { ...tool, path: sanitize(tool.path, home) }]), + ) as SetupCheckResult['tools'], + }, } } @@ -189,6 +196,17 @@ describe.skipIf(process.platform !== 'darwin')('first-run prerequisites on a sim expect({ id, found: live.tools[id].found, source: live.tools[id].source }) .toEqual({ id, found: recording.tools[id].found, source: recording.tools[id].source }) } + // The #995 policy on the LIVE result. Nothing blocks launch: the first + // project is always something this machine can run. On the macOS CI + // runner, which has no provider CLI, the unbundled case is the genuine + // zero-provider Mac and must come out as a terminal. + const usable = live.usableProviders + expect(live.firstSessionKind).toBe(usable.includes('claude') ? 'claude' : usable[0] ?? 'terminal') + expect(live).not.toHaveProperty('blocking') + if (environment === 'clean-machine-packaged') { + expect(usable).toContain('opencode') + expect(live.firstSessionKind).not.toBe('terminal') + } }, 60_000, ) From 45fd8b2a966156b7af6d12c6a5a59b10033c735c Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 19 Sep 2026 02:47:10 -0700 Subject: [PATCH 4/7] feat(onboarding): a fresh install always reaches a workspace, and Setup can be reopened (#995) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stages 3–6 of docs/decomposition/onboarding-first-run.md. - Setup store (features/setup/store.ts): one shared check for the gate, the bootstrap and the pickers, with one in-flight probe. - SetupGate: - It opens by itself only when no provider is usable, or when a Homebrew-installable helper is missing. - "Continue with a terminal" is the explicit acknowledgment. Escape does not decide it. - Missing providers show a copyable install command and a docs link. The manual path override now applies to any missing provider. - Reopenable: the `open-setup` command, and File › Setup… (added to NATIVE_MENU_COMMAND_IDS). The spawn error names that place instead of "open Setup", which did not exist after launch. - Bootstrap: - A fresh install waits for the first-run decision instead of spawning Claude under the gate. - It opens firstSessionKind, and falls back to a terminal if that spawn fails, so the run always has a project and autosave. - The persisted-fallback recovery shell uses the same verdict without waiting. - The first project's cwd is home when launchd started the app in /. - Pickers (path picker, new agent, new agent in, provider switch): - Missing providers get a "Not installed · File › Setup…" hint but stay selectable, since a probe can be wrong. - The path picker preselects the machine's first usable provider. Tests: firstRun.renderer.test.tsx runs the real useWorkspace and bootstrap and the real SetupGate against the RECORDED checks. The cases: - no provider: panel, install commands, no spawn until answered; - a terminal on Continue, and restoreStatus fresh; - Retry after installing gives an agent; - the packaged app opens the bundled OpenCode; - a failed spawn falls back to a terminal; - Open Setup reopens and re-probes. Fail-first: 5 of 6 fail against main's bootstrap. Also added: - PathPickerModal preselect and hint tests on the packaged recording; - defaultWorkspaceCwd tests; - catalog governance: 124 commands, 43 approved additions, and the File menu plus Setup. Closes #995 Co-Authored-By: Claude Opus 5 (1M context) --- docs/decomposition/onboarding-first-run.md | 44 ++++ src/main/ipc/workspace.test.ts | 16 +- src/main/ipc/workspace.ts | 26 +- src/main/menu/appMenu.ts | 9 + src/main/sessionManager.ts | 4 +- .../features/command-palette/catalog.test.ts | 32 ++- .../src/features/command-palette/catalog.ts | 3 + .../ui/PathPickerModal.renderer.test.tsx | 25 ++ .../path-picker/ui/PathPickerModal.tsx | 18 +- .../features/setup/commands/setupCommands.ts | 28 ++ .../src/features/setup/controlReference.ts | 18 +- .../features/setup/firstRun.renderer.test.tsx | 159 ++++++++++++ src/renderer/src/features/setup/store.ts | 142 ++++++++++ .../src/features/setup/ui/SetupGate.tsx | 244 +++++++++++++----- .../workspace/ui/NewAgentInDialog.tsx | 4 +- .../workspace/ui/NewAgentPlacementOverlay.tsx | 6 +- .../ui/ProviderSwitchPickerModal.tsx | 4 +- .../hook/persistence/useBootstrap.ts | 50 +++- 18 files changed, 725 insertions(+), 107 deletions(-) create mode 100644 src/renderer/src/features/setup/commands/setupCommands.ts create mode 100644 src/renderer/src/features/setup/firstRun.renderer.test.tsx create mode 100644 src/renderer/src/features/setup/store.ts diff --git a/docs/decomposition/onboarding-first-run.md b/docs/decomposition/onboarding-first-run.md index 1f2dc129b..57d49ed7a 100644 --- a/docs/decomposition/onboarding-first-run.md +++ b/docs/decomposition/onboarding-first-run.md @@ -124,3 +124,47 @@ All behavioral fixtures come from Stage 1's recorder: two environments × three (prerequisites check, default-cwd, bootstrap transitions), stored as JSON under `testing/first-run/fixtures/` with the harness committed beside them. No hand-typed plausible-looking inputs anywhere in this feature's tests. + +## Status (2026-09-19, branch `fix/onboarding-first-run`) + +All six stages are built in one PR. They are coupled: the policy's output shape +is what the gate, the bootstrap and the pickers read. + +| Stage | Artifact | Verified by | +|---|---|---| +| 1 Recorder | `testing/system/first-run/prerequisites.firstRun.test.ts` and `testing/fixtures/first-run/` (3 environments, plus `baseline-main-82babd21.json`) | The live drift check re-runs both clean simulations on every push and compares provider rows. The baseline shows the wall: `ready:false, blocking:[claude,codex]` on both clean machines. | +| 2 Policy | `src/shared/setup/readiness.ts`; `required` removed from providers; `SetupCheckResult` = `usableProviders` + `firstSessionKind`; per-provider install command and docs URL in `registry.setup.ts` | `readiness.test.ts` over the recordings; the live verdict in the system test | +| 3 Reopenable | `open-setup` command, File › Setup…, and the new spawn error text | `firstRun.renderer.test.tsx` (reopen and re-probe), catalog native-menu contract | +| 4 Bootstrap | `awaitFirstRunDecision` plus `openFirstProject` (terminal fallback) in `useBootstrap.ts` | `firstRun.renderer.test.tsx`: 5 of 6 fail against main's bootstrap | +| 5 cwd and pickers | `defaultWorkspaceCwd` (home for a launchd `/`); `useMissingProviders` hint and `preferredPickerProvider` | `workspace.test.ts`, `PathPickerModal.renderer.test.tsx` | +| 6 Sweep | `firstRun.renderer.test.tsx`: the real `useWorkspace` plus the real SetupGate fed the recorded checks | Fail-first as above | + +### Deviations from the plan, and why + +- **`readiness.ts` lives in `src/shared/setup`, not `src/main/setup`.** Main still + runs it, inside checkPrerequisites. It is in `shared` so renderer tests can run + the same policy over the recordings. The isolation rule is kept: production + consumers read the stamped fields and never import it. +- **No toast action.** The global toast is text-only, and adding actions to it + is its own change. The spawn error now names a place that exists: "Open Setup + (File › Setup…)", which is also a palette command. +- **Pickers hint and never disable.** A probe can be wrong (#495 A1), and the + spawn re-resolves the CLI itself. A disabled row would turn a false negative + back into a lockout. +- **No Settings → Setup rows for tool paths.** The reopenable Setup panel + already hosts the manual path override, available at any time. A second copy + in Settings would be a second owner for the same state. +- **Unknown 3 (the terminal-only default project)** took the ledger's + recommended default: a single terminal project, opened only after the user + presses "Continue with a terminal". Pressing Retry after installing a CLI + opens an agent instead. +- **Unknown 1 (#994)** had already merged as #1002: a bundled OpenCode is + `found` with `source:'bundled'` and counts as a usable provider. + +### Still open + +- Login detection (#995 finding 7) is out of scope here, as planned. +- App self-update (finding 5) is a separate issue. +- Nothing tells a packaged-clean user whose first project opened in the bundled + OpenCode that Claude Code and Codex exist, other than File › Setup…. A + first-run notice would need an owner decision. diff --git a/src/main/ipc/workspace.test.ts b/src/main/ipc/workspace.test.ts index 938f67daf..7dba87ae7 100644 --- a/src/main/ipc/workspace.test.ts +++ b/src/main/ipc/workspace.test.ts @@ -12,7 +12,21 @@ vi.mock('electron', () => ({ })) vi.mock('@main/window/windowRegistry.js', () => ({ windowIdFor, getBrowserWindow })) -const { registerWorkspaceIpc } = await import('@main/ipc/workspace.js') +const { registerWorkspaceIpc, defaultWorkspaceCwd } = await import('@main/ipc/workspace.js') + +// #995: launchd starts a Finder/Dock launch with cwd `/`, and every fresh +// install's first project used to be the filesystem root. +describe('the first project directory', () => { + it('is home when the app was launched from Finder or the Dock (cwd /)', () => { + expect(defaultWorkspaceCwd({}, '/', '/Users/someone')).toBe('/Users/someone') + }) + it('keeps a real working directory, as a terminal launch or npm run dev has', () => { + expect(defaultWorkspaceCwd({}, '/Users/someone/project', '/Users/someone')).toBe('/Users/someone/project') + }) + it('honours AGENT_CODE_CWD over both', () => { + expect(defaultWorkspaceCwd({ AGENT_CODE_CWD: '/work' }, '/', '/Users/someone')).toBe('/work') + }) +}) // The addressing layer: which window a workspace payload belongs to, and what // main tells SessionManager afterwards. diff --git a/src/main/ipc/workspace.ts b/src/main/ipc/workspace.ts index 5a97531ac..74b73ed31 100644 --- a/src/main/ipc/workspace.ts +++ b/src/main/ipc/workspace.ts @@ -1,3 +1,5 @@ +import { homedir } from 'node:os' + import { ipcMain } from 'electron' import type { SessionManager } from '@main/sessionManager.js' @@ -61,7 +63,25 @@ export function registerWorkspaceIpc( // Renderer calls this on first launch when there's no saved state // and no user-picked cwd yet. AGENT_CODE_CWD overrides — useful in // dev for launching the app pointed at a specific test project. - ipcMain.handle('workspace:default-cwd', () => { - return process.env.AGENT_CODE_CWD || process.cwd() - }) + ipcMain.handle('workspace:default-cwd', () => defaultWorkspaceCwd()) +} + +/** + * The first project's directory. + * + * WHY home instead of `/` (#995): an app launched from Finder or the Dock is + * started by launchd with cwd `/`, so every fresh install's first project was + * the filesystem root. There an agent's first `ls` lists system folders, the + * project title reads "/", and anything the agent writes is refused or lands + * somewhere nobody meant. `process.cwd()` is kept when it is anything else: + * `npm run dev` from a checkout, or a terminal `open -a` that passes a real + * directory, still starts where the developer was. + */ +export function defaultWorkspaceCwd( + env: NodeJS.ProcessEnv = process.env, + cwd: string = process.cwd(), + home: string = homedir(), +): string { + if (env.AGENT_CODE_CWD) return env.AGENT_CODE_CWD + return cwd === '/' ? home : cwd } diff --git a/src/main/menu/appMenu.ts b/src/main/menu/appMenu.ts index 7345eab8c..c644d469f 100644 --- a/src/main/menu/appMenu.ts +++ b/src/main/menu/appMenu.ts @@ -115,6 +115,15 @@ export function buildAppMenu(): Menu { click: () => dispatchCommand('reorder-tabs'), }, { type: 'separator' }, + { + label: 'Setup…', + // → renderer command `open-setup` (#995). The spawn error for a + // missing CLI tells the user to "open Setup"; a user who has never + // opened the command palette looks in the menu bar, so it has to be + // here too. + click: () => dispatchCommand('open-setup'), + }, + { type: 'separator' }, { label: 'Close Tab', // → renderer command `close-tab`. No accelerator (renderer binds ⌘⇧W). diff --git a/src/main/sessionManager.ts b/src/main/sessionManager.ts index afebaccff..baad7af16 100644 --- a/src/main/sessionManager.ts +++ b/src/main/sessionManager.ts @@ -2586,7 +2586,9 @@ export class SessionManager extends EventEmitter { } } if (!binary) { - throw new Error(`${kind} CLI not found — open Setup to locate it`) + // Names a real place (#995): Setup opens from File › Setup… or the + // "Open Setup" command, and shows the install command for this CLI. + throw new Error(`${kind} CLI not found. Open Setup (File › Setup…) to install it or enter its path.`) } const initialSize = { cols: options.cols ?? 120, diff --git a/src/renderer/src/features/command-palette/catalog.test.ts b/src/renderer/src/features/command-palette/catalog.test.ts index 9d35b54d0..bc0a220f5 100644 --- a/src/renderer/src/features/command-palette/catalog.test.ts +++ b/src/renderer/src/features/command-palette/catalog.test.ts @@ -25,6 +25,7 @@ import { RETIRED_BUILT_IN_COMMAND_IDS } from '@renderer/app-state/settings/persi // 16 retirements took it to 118, Clear Lane (stage 4) to 119 and the lane keyboard // grammar (stage 5) to 123. (#992 was written against 130 and read 119 at the end; // merging main added Goal Loop's two commands and the two generated Grok splits.) +// 124 with Open Setup (#995). // Keeping ONE snapshot that moved — rather // than a "baseline" file and an "after" file — is what makes the plan's // headline count an assertion anyone can check against running code instead of @@ -168,6 +169,8 @@ const BASELINE_COMMAND_IDS: readonly string[] = [ 'open-keyboard-shortcuts', 'toggle-aggressive-debug-persistence', 'toggle-worktrees-bar', + // setupCommands (1, #995) + 'open-setup', // copy-assistant / copy-code-block (2) 'copy-assistant-message', 'copy-code-block', @@ -233,12 +236,12 @@ const NAVIGATION_COMMAND_GROUP: readonly string[] = [ const ids = (): string[] => builtInCommandCatalog.map(c => c.id) describe('built-in command catalog — baseline characterization', () => { - it('contains exactly the 123 governed commands in registration order', () => { + it('contains exactly the 124 governed commands in registration order', () => { // Order matters: this is the palette's empty-query browse order. expect(ids()).toEqual([...BASELINE_COMMAND_IDS]) }) - it('has exactly 123 commands', () => { + it('has exactly 124 commands', () => { // Stated separately from the order assertion because this number is the // thing that moves, and a bare count failure is a clearer signal than a // 99-line array diff. @@ -260,11 +263,12 @@ describe('built-in command catalog — baseline characterization', () => { // −normalize×3 → 114 with stage 3a: −tiled-tabs, −bury/revive/kill-buried, // −attach×2, −detach → 115 with Clear Lane (#992 stage 4) → 119 with the // lane keyboard grammar (#992 stage 5) → 123 once main's Goal Loop preview - // and stop (#1001) and the two generated Grok splits (#844) merged in. + // and stop (#1001) and the two generated Grok splits (#844) merged in → 124 + // with Open Setup (#995). // Each step of that arithmetic was a deliberate edit to this line, which is the entire point of pinning it. (The two test // titles above had drifted to "115" while this line said 116; they now // track it again.) - expect(builtInCommandCatalog).toHaveLength(123) + expect(builtInCommandCatalog).toHaveLength(124) }) it('reports no structural defects', () => { @@ -298,13 +302,14 @@ describe('generated per-provider split commands', () => { }) it('accounts for the difference between literal and total command count', () => { - // 123 total - 6 generated = 117 literal `id:` fields across the command + // 124 total - 6 generated = 118 literal `id:` fields across the command // modules. At the original baseline this read 102 - 4 = 98; it moved down by // the five retirements, then back up by the nine additions, Grid Dispatch's // six row commands, New Window, and the later additions recorded in the // count test above (through the lane keyboard grammar, #992 stage 5, and - // Goal Loop, #1001). Grok (#844) grew only the GENERATED term, 4 → 6. - expect(builtInCommandCatalog.length - nonDefaultProviders.length * 2).toBe(117) + // Goal Loop, #1001, and Open Setup, #995). Grok (#844) grew only the + // GENERATED term, 4 → 6. + expect(builtInCommandCatalog.length - nonDefaultProviders.length * 2).toBe(118) }) it('emits both directions for every non-default provider', () => { @@ -365,7 +370,7 @@ describe('native menu contract', () => { expect(missing).toEqual([]) }) - it('covers the six File-menu actions recorded in the audit', () => { + it('covers the six File-menu actions recorded in the audit, plus Setup', () => { expect([...NATIVE_MENU_COMMAND_IDS]).toEqual([ 'new-tab', 'resume-session', @@ -373,6 +378,9 @@ describe('native menu contract', () => { 'save-all-editor-files', 'reorder-tabs', 'close-tab', + // #995: the spawn error for a missing CLI says "Open Setup", so the menu + // bar has to offer it. + 'open-setup', ]) }) }) @@ -411,7 +419,7 @@ describe('governance targets', () => { }) it('lands on the arithmetic the plan predicted', () => { - // 102 baseline - 21 retirements + 42 additions = 123, checked against the + // 102 baseline - 21 retirements + 43 additions = 124, checked against the // real catalog rather than trusted as prose. (5 governance retirements + // 16 unified-layout retirements, all recorded in RETIRED_COMMAND_IDS.) // @@ -441,9 +449,9 @@ describe('governance targets', () => { // `agent-analytics.open` (#964), `goal-loop-preview` and `goal-loop-stop` // (#1001), `grok-vertical` and `grok-horizontal` (#844, generated from // AGENT_PROVIDER_KINDS), `clear-focused-lane` (#992 stage 4), and the four - // lane-grammar commands (#992 stage 5). - expect(builtInCommandCatalog.length + RETIRED_COMMAND_IDS.length - 42).toBe(102) - expect(builtInCommandCatalog).toHaveLength(123) + // lane-grammar commands (#992 stage 5), and `open-setup` (#995). + expect(builtInCommandCatalog.length + RETIRED_COMMAND_IDS.length - 43).toBe(102) + expect(builtInCommandCatalog).toHaveLength(124) }) }) diff --git a/src/renderer/src/features/command-palette/catalog.ts b/src/renderer/src/features/command-palette/catalog.ts index 65c5b29c8..d83f283a3 100644 --- a/src/renderer/src/features/command-palette/catalog.ts +++ b/src/renderer/src/features/command-palette/catalog.ts @@ -7,6 +7,7 @@ import { sessionCommands } from '@renderer/features/workspace/commands/sessionCo import { tabCommands } from '@renderer/features/workspace/commands/tabCommands' import { windowCommands } from '@renderer/features/workspace/commands/windowCommands' import { settingsCommands } from '@renderer/features/settings/commands/settingsCommands' +import { setupCommands } from '@renderer/features/setup/commands/setupCommands' import { spotlightCommands } from '@renderer/features/spotlight/commands/spotlightCommands' import { readerCommands } from '@renderer/features/reader/commands/readerCommands' import { copyAssistantCommands } from '@renderer/features/copy-assistant/commands/copyAssistantCommands' @@ -79,6 +80,8 @@ export const builtInCommandCatalog: readonly CommandDef[] = Object.freeze([ ...goalLoopCommands, ...readerCommands, ...settingsCommands, + // Beside Settings: Setup is the other configuration surface (#995). + ...setupCommands, ...copyAssistantCommands, ...copyCodeBlockCommands, ...promptTemplateCommands, diff --git a/src/renderer/src/features/path-picker/ui/PathPickerModal.renderer.test.tsx b/src/renderer/src/features/path-picker/ui/PathPickerModal.renderer.test.tsx index 0aa56723f..e9cb78c70 100644 --- a/src/renderer/src/features/path-picker/ui/PathPickerModal.renderer.test.tsx +++ b/src/renderer/src/features/path-picker/ui/PathPickerModal.renderer.test.tsx @@ -4,6 +4,8 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import type { Conversation, ConversationListRequest, ConversationListResponse } from '@shared/conversations/types' import { PathPickerModal } from './PathPickerModal' +import { MISSING_PROVIDER_HINT, resetSetupStoreForTests, useSetupStore } from '@renderer/features/setup/store' +import { loadFirstRunCheck } from '@shared/setup/firstRunRecordings.testSupport' const originalApiDescriptor = Object.getOwnPropertyDescriptor(window, 'api') @@ -222,3 +224,26 @@ describe('PathPickerModal reuse of an open tab (#913)', () => { expect(onActivateTab).not.toHaveBeenCalled() }) }) + +describe('PathPickerModal on a machine without the default provider (#995)', () => { + afterEach(() => resetSetupStoreForTests()) + + it('preselects the provider the machine has and marks the missing ones, without disabling them', async () => { + // The packaged app on a clean Mac, as main recorded it: only the bundled + // OpenCode (and this recording machine's npm-global Grok) resolve. The + // picker used to preselect Claude, so ⌘T failed only after the user had + // chosen a directory. + useSetupStore.getState().setCheck(loadFirstRunCheck('clean-machine-packaged')) + const list = vi.fn(async () => response([])) + installApi(list) + render() + await waitFor(() => expect(list).toHaveBeenCalledWith(expect.objectContaining({ providers: ['opencode'] }))) + const claude = screen.getByRole('button', { name: /^claude$/i }) + expect(claude).toHaveAttribute('data-provider-missing', 'true') + expect(claude).toHaveAttribute('title', MISSING_PROVIDER_HINT) + expect(screen.getByRole('button', { name: /^opencode$/i })).not.toHaveAttribute('data-provider-missing') + // Still selectable: a probe can be wrong, and the spawn re-resolves. + fireEvent.click(claude) + await waitFor(() => expect(list).toHaveBeenCalledWith(expect.objectContaining({ providers: ['claude'] }))) + }) +}) diff --git a/src/renderer/src/features/path-picker/ui/PathPickerModal.tsx b/src/renderer/src/features/path-picker/ui/PathPickerModal.tsx index 430aaecfb..7bfd1686b 100644 --- a/src/renderer/src/features/path-picker/ui/PathPickerModal.tsx +++ b/src/renderer/src/features/path-picker/ui/PathPickerModal.tsx @@ -1,6 +1,7 @@ -import { AGENT_PROVIDER_KINDS, DEFAULT_PROVIDER } from '@shared/types/providerKind' +import { AGENT_PROVIDER_KINDS } from '@shared/types/providerKind' import type { AgentProviderKind } from '@shared/types/providerKind' import { getRendererProviderCapabilities } from '@providers/registry.renderer.capabilities' +import { MISSING_PROVIDER_HINT, preferredPickerProvider, useMissingProviders } from '@renderer/features/setup/store' import { useEffect, useRef, useState } from 'react' import { Button } from '@renderer/components/ui/button' @@ -89,8 +90,12 @@ export function PathPickerModal({ const [value, setValue] = useState(defaultValue) const [error, setError] = useState(null) const [busy, setBusy] = useState(false) - // Provider toggle: Claude (default) or Codex. Resets on modal open. - const [provider, setProvider] = useState(DEFAULT_PROVIDER) + // Provider toggle. Resets on modal open. Preselects the provider a fresh + // install would use (#995): the default when it is installed, otherwise the + // first one that is. It used to be Claude unconditionally, so on a Mac + // without Claude, ⌘T failed only after the user had chosen a directory. + const [provider, setProvider] = useState(() => preferredPickerProvider()) + const missingProviders = useMissingProviders() // Resume list state. We eagerly refresh the list whenever the path // changes and resolves to a valid directory — gives the user live @@ -139,7 +144,7 @@ export function PathPickerModal({ setListingTarget(null) setResolvedPath(null) setPendingCreatePath(null) - setProvider(DEFAULT_PROVIDER) + setProvider(preferredPickerProvider()) }, [open, defaultValue]) // Refresh the sessions list whenever the typed path changes. Run @@ -321,12 +326,17 @@ export function PathPickerModal({ if (p !== provider) invalidateResumeListing() setProvider(p) }} + // Still selectable when missing: the probe can be wrong, and + // the spawn re-resolves the CLI itself (see useMissingProviders). + title={missingProviders.has(p) ? MISSING_PROVIDER_HINT : undefined} + data-provider-missing={missingProviders.has(p) || undefined} className={`rounded-control px-3 py-1 text-[11px] font-semibold uppercase tracking-wider border transition-colors duration-120 ${provider === p ? 'bg-accent text-accent-fg border-accent' : 'bg-transparent text-muted border-border hover:border-border-hi hover:text-ink'} + ${missingProviders.has(p) ? 'line-through decoration-muted' : ''} `} > {getRendererProviderCapabilities(p).shortLabel} diff --git a/src/renderer/src/features/setup/commands/setupCommands.ts b/src/renderer/src/features/setup/commands/setupCommands.ts new file mode 100644 index 000000000..d65d84666 --- /dev/null +++ b/src/renderer/src/features/setup/commands/setupCommands.ts @@ -0,0 +1,28 @@ +import type { CommandDef } from '@renderer/features/command-palette/types' +import { panel } from '@renderer/features/command-palette/commandState' +import { useSetupStore } from '@renderer/features/setup/store' + +export const setupCommands: CommandDef[] = [{ + // WHY this command exists (#995): the spawn error for a missing CLI says + // "open Setup to locate it", and until now there was no Setup to open after + // launch. The gate appeared once, from a local flag, and nothing could + // bring it back. The File menu dispatches this same id + // (NATIVE_MENU_COMMAND_IDS), so both roads lead to one surface. + // + // 'preferences' beside Open Settings: installing a provider or pointing + // Agent Code at a CLI is configuration, and that is the drawer people open + // when they are thinking about it. + id: 'open-setup', + category: 'preferences', + surface: 'app', + title: 'Open Setup', + description: '**What it does:** Opens **Setup**: which agent CLIs and helper tools this Mac has, a copyable install command for each missing provider, and a manual path override.\n\n**Use when:** A provider is "not installed", a spawn says to open Setup, or you just installed a CLI and want Agent Code to find it.\n\n**Notes:** Opening it re-checks every tool. Escape closes it.', + keywords: ['setup', 'install', 'provider', 'cli', 'claude', 'codex', 'opencode', 'grok', 'path', 'prerequisites', 'onboarding'], + getState: () => panel(useSetupStore.getState().requested), + run: ({ ui }) => { + ui.closePalette() + const store = useSetupStore.getState() + if (store.requested) store.close() + else store.open() + }, +}] diff --git a/src/renderer/src/features/setup/controlReference.ts b/src/renderer/src/features/setup/controlReference.ts index 5f4e15e63..f2d5cc1dd 100644 --- a/src/renderer/src/features/setup/controlReference.ts +++ b/src/renderer/src/features/setup/controlReference.ts @@ -6,16 +6,16 @@ export const controlReference = [ { "id": "setup", "title": "First-run provider and toolchain setup", - "purpose": "Establish the native tools Agent Code needs.", - "ui": "Setup gate and setup settings.", - "prerequisites": "Supported platform and required provider/tool availability.", + "purpose": "Show which agent CLIs and helper tools this Mac has, and how to add the missing ones.", + "ui": "The Setup panel: opens by itself when no provider is usable, or on request from File › Setup… or Open Setup.", + "prerequisites": "None. No single tool blocks launch: any one provider is enough for agents, and a terminal pane needs none.", "workflow": [ - "Review missing tools", - "complete installation/authentication through supported flows", - "recheck readiness." + "Open Setup", + "copy a missing provider's install command and run it in a terminal pane", + "press Retry, or enter the CLI's path manually if the probe misses it." ], - "outcome": "The chosen provider/runtime becomes available for session creation.", - "cautions": "Provider login and permission rules remain provider-owned.", - "commandIds": [] + "outcome": "The provider becomes available for new agents. A fresh install opens its first project with the default provider when usable, otherwise the first usable one, otherwise a terminal.", + "cautions": "A found CLI may still be signed out; provider login and permission rules remain provider-owned.", + "commandIds": ["open-setup"] } ] satisfies FeatureReference[] diff --git a/src/renderer/src/features/setup/firstRun.renderer.test.tsx b/src/renderer/src/features/setup/firstRun.renderer.test.tsx new file mode 100644 index 000000000..32e452e00 --- /dev/null +++ b/src/renderer/src/features/setup/firstRun.renderer.test.tsx @@ -0,0 +1,159 @@ +import { act, cleanup, fireEvent, render, renderHook, screen, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { useAppStore } from '@renderer/app-state/hooks' +import { useWorkspace } from '@renderer/workspace/hook' +import { SetupGate } from '@renderer/features/setup/ui/SetupGate' +import { setupCommands } from '@renderer/features/setup/commands/setupCommands' +import { resetSetupStoreForTests } from '@renderer/features/setup/store' +import { + loadFirstRunCheck, + withoutMachineWideInstalls, +} from '@shared/setup/firstRunRecordings.testSupport' +import type { SessionSpawnOptions } from '@preload/api/types' +import type { SetupCheckResult } from '@shared/types/setup' +import type { CommandContext } from '@renderer/features/command-palette/types' + +// The first run, end to end in the renderer (#995 stage 6): the REAL +// workspace hook with its REAL bootstrap, the REAL SetupGate and the setup +// store they share. Main is represented by the check it RECORDED on a +// simulated clean Mac (testing/fixtures/first-run) and by a spawn that +// behaves like SessionManager's: a provider the check did not find fails +// with SessionManager's own error, and a terminal always starts. +// +// Only process and IPC ingress are suppressed, as in the other whole-hook +// suites. +vi.mock('@renderer/workspace/hook/ipc/useIpcSubscriptions', () => ({ useIpcSubscriptions: () => undefined })) +vi.mock('@renderer/workspace/hook/ipc/useWorkspaceAdoption', () => ({ useWorkspaceAdoption: () => undefined })) +vi.mock('@renderer/features/sessionFeed/SessionFeedContext', () => ({ useSessionFeed: () => ({}) })) +vi.mock('@renderer/performance/client', () => ({ + mark: vi.fn(), + span: () => ({ end: vi.fn(), fail: vi.fn() }), + measure: (_name: string, fn: () => T | Promise) => fn(), +})) + +const originalStore = useAppStore.getState() +const originalApi = Object.getOwnPropertyDescriptor(window, 'api') +beforeEach(() => resetSetupStoreForTests()) +afterEach(() => { + cleanup() + resetSetupStoreForTests() + useAppStore.setState(originalStore, true) + if (originalApi) Object.defineProperty(window, 'api', originalApi) + else Reflect.deleteProperty(window, 'api') +}) + +/** The machine as main sees it. `checks` are returned in order, the last one + * repeating, so a test can install a CLI between two Retry presses. */ +function mountMachine(checks: SetupCheckResult[], options: { failKinds?: string[] } = {}) { + let sequence = 0 + let current = checks[0]! + const setupCheck = vi.fn(async () => { + current = checks[Math.min(sequence, checks.length - 1)]! + sequence += 1 + return current + }) + const spawnSession = vi.fn(async (spawn: SessionSpawnOptions) => { + const kind = spawn.kind ?? 'claude' + const installed = kind === 'terminal' || current.usableProviders.includes(kind as never) + if (!installed || options.failKinds?.includes(kind)) { + // SessionManager's own message for a CLI it cannot resolve. + throw new Error(`${kind} CLI not found. Open Setup (File › Setup…) to install it or enter its path.`) + } + return { sessionId: `session-${spawnSession.mock.calls.length}` } + }) + Object.defineProperty(window, 'api', { configurable: true, value: { + loadWorkspace: async () => null, + defaultCwd: async () => '/Users/someone', + setupCheck, + setupSkipOptional: vi.fn(async () => current), + spawnSession, + onOrchestrationRequest: () => () => undefined, + onAgentManagementRequest: () => () => undefined, + ghostRead: async () => [], + reportSessionLifecycle: vi.fn(), + appendFeedDebugLog: async () => undefined, + } }) + const hook = renderHook(() => useWorkspace()) + render() + return { hook, spawnSession, setupCheck } +} + +const spawnedKinds = (spawnSession: ReturnType['spawnSession']) => + spawnSession.mock.calls.map(([spawn]) => spawn.kind ?? 'claude') +const projects = () => useAppStore.getState().workspaceState.tabs.length + +describe('first run on a Mac with no provider (#995)', () => { + const noProvider = () => withoutMachineWideInstalls(loadFirstRunCheck('clean-machine')) + + it('explains, offers each install command, and spawns nothing until the user answers', async () => { + const { spawnSession } = mountMachine([noProvider()]) + expect(await screen.findByText('No agent provider is installed yet')).toBeTruthy() + expect(screen.getByText('curl -fsSL https://claude.ai/install.sh | bash')).toBeTruthy() + expect(screen.getByText('curl -fsSL https://chatgpt.com/codex/install.sh | sh')).toBeTruthy() + // Before #995 bootstrap spawned Claude here, underneath the gate, and + // the failure left no project and autosave off. + expect(spawnSession).not.toHaveBeenCalled() + // The acknowledgment is the button; a stray Escape decides nothing. + fireEvent.keyDown(window, { key: 'Escape' }) + expect(screen.getByRole('dialog')).toBeTruthy() + expect(spawnSession).not.toHaveBeenCalled() + // Answer it before the test ends: the waiting bootstrap is subscribed to + // the module-wide setup store, and left waiting it would be released by + // the NEXT test's answer and add a second project there. + fireEvent.click(screen.getByRole('button', { name: 'Continue with a terminal' })) + await waitFor(() => expect(projects()).toBe(1)) + }) + + it('"Continue with a terminal" opens a terminal project, and the run can save', async () => { + const { hook, spawnSession } = mountMachine([noProvider()]) + fireEvent.click(await screen.findByRole('button', { name: 'Continue with a terminal' })) + await waitFor(() => expect(projects()).toBe(1)) + expect(spawnedKinds(spawnSession)).toEqual(['terminal']) + // `fresh` is the one status that unlocks autosave on an empty disk. + await waitFor(() => expect(hook.result.current.restoreStatus).toBe('fresh')) + expect(screen.queryByRole('dialog')).toBeNull() + }) + + it('installing a CLI and pressing Retry makes the first project an agent', async () => { + // The second probe is the developer machine's: Claude now resolves. + const { spawnSession } = mountMachine([noProvider(), loadFirstRunCheck('developer-machine')]) + fireEvent.click(await screen.findByRole('button', { name: 'Retry' })) + await waitFor(() => expect(projects()).toBe(1)) + expect(spawnedKinds(spawnSession)).toEqual(['claude']) + expect(screen.queryByRole('dialog')).toBeNull() + }) +}) + +describe('first run on the packaged app (#995, bundled OpenCode #994)', () => { + it('opens straight into the bundled OpenCode, with no panel in the way', async () => { + const { spawnSession } = mountMachine([loadFirstRunCheck('clean-machine-packaged')]) + await waitFor(() => expect(projects()).toBe(1)) + expect(spawnedKinds(spawnSession)).toEqual(['opencode']) + expect(screen.queryByRole('dialog')).toBeNull() + }) + + it('falls back to a terminal when the chosen provider still fails to start', async () => { + // A probe can say "found" for a binary that then fails to spawn. The run + // must still end with a project, or autosave stays off for all of it. + const { spawnSession } = mountMachine([loadFirstRunCheck('clean-machine-packaged')], { failKinds: ['opencode'] }) + await waitFor(() => expect(projects()).toBe(1)) + expect(spawnedKinds(spawnSession)).toEqual(['opencode', 'terminal']) + }) +}) + +describe('Setup can be reopened (#995 finding 2)', () => { + it('Open Setup shows the panel on a machine with everything installed, re-probes, and Escape closes it', async () => { + const { setupCheck } = mountMachine([loadFirstRunCheck('developer-machine')]) + await waitFor(() => expect(projects()).toBe(1)) + expect(screen.queryByRole('dialog')).toBeNull() + const probesBefore = setupCheck.mock.calls.length + await act(async () => { + setupCommands.find(command => command.id === 'open-setup')!.run({ ui: { closePalette: vi.fn() } } as unknown as CommandContext) + }) + expect(await screen.findByText('Agent Code Setup')).toBeTruthy() + await waitFor(() => expect(setupCheck.mock.calls.length).toBeGreaterThan(probesBefore)) + fireEvent.keyDown(window, { key: 'Escape' }) + expect(screen.queryByRole('dialog')).toBeNull() + }) +}) diff --git a/src/renderer/src/features/setup/store.ts b/src/renderer/src/features/setup/store.ts new file mode 100644 index 000000000..719e4fca5 --- /dev/null +++ b/src/renderer/src/features/setup/store.ts @@ -0,0 +1,142 @@ +import { useMemo } from 'react' +import { create } from 'zustand' + +import { AGENT_PROVIDER_KINDS, DEFAULT_PROVIDER, type AgentProviderKind } from '@shared/types/providerKind' +import type { SetupCheckResult } from '@shared/types/setup' + +/** + * The renderer's one copy of the setup check (#995). + * + * WHY a store instead of the gate's local state (which is what it was): + * - Three surfaces need the same answer. The gate shows it, the fresh-install + * bootstrap picks the first project's kind from it, and the provider pickers + * mark missing CLIs with it. Each calling `setupCheck()` itself would run + * three rounds of login-shell probes at launch, and could get three + * different answers if the user installs a CLI mid-launch. + * - The gate must be REOPENABLE. The spawn error said "open Setup to locate + * it", but Setup was a local `dismissed` flag inside a component mounted + * once, with no way back after launch. `requested` is the reopen. + */ +type SetupStore = { + check: SetupCheckResult | null + error: string | null + /** The user opened Setup (command or File menu). Shows the panel even + * when nothing is missing, and closes on Escape. */ + requested: boolean + /** The automatic first-run panel was answered for this run. */ + dismissed: boolean + setCheck: (check: SetupCheckResult) => void + setError: (error: string | null) => void + open: () => void + close: () => void +} + +export const useSetupStore = create(set => ({ + check: null, + error: null, + requested: false, + dismissed: false, + setCheck: check => set({ check, error: null }), + setError: error => set({ error }), + open: () => set({ requested: true }), + // Closing a requested panel also answers the automatic one: a user who + // opened Setup, read it and closed it has seen everything the automatic + // panel would say. + close: () => set({ requested: false, dismissed: true }), +})) + +let inFlight: Promise | null = null + +/** + * Runs the check, sharing one in-flight probe among all callers. + * + * Resolves null when the check itself failed (IPC error). That is + * "unknown", never "nothing installed": callers fall back to the behavior + * before #995 rather than treat a broken probe as an empty machine. + */ +export function refreshSetupCheck(): Promise { + if (inFlight) return inFlight + const store = useSetupStore.getState() + // Through a resolved promise so a missing or throwing IPC surface (an older + // preload, a test stub) lands in the catch below instead of throwing out of + // bootstrap synchronously. + inFlight = Promise.resolve() + .then(() => window.api.setupCheck()) + .then(check => { store.setCheck(check); return check }) + .catch((err: unknown) => { + store.setError(err instanceof Error ? err.message : String(err)) + return null + }) + .finally(() => { inFlight = null }) + return inFlight +} + +/** The cached check, or the in-flight one, or a new one. */ +export function ensureSetupCheck(): Promise { + const { check } = useSetupStore.getState() + return check ? Promise.resolve(check) : refreshSetupCheck() +} + +/** + * What the fresh-install bootstrap may open its first project with. + * + * WHY bootstrap waits here (#995 finding 3): it used to spawn Claude the + * moment it mounted, underneath the gate that was telling the user Claude + * was missing. The spawn failed and left no tabs, which also kept autosave + * off for the run. Now: + * - With a usable provider, this resolves at once and nothing waits. + * - With none, the automatic panel is up, and this resolves when the user + * answers it. Either they install a CLI and press Retry (the check then + * has a provider, so the first project is an agent), or they choose + * "Continue with a terminal". + * - A failed check resolves null at once. Never hold the workspace hostage + * to a probe. + */ +export async function awaitFirstRunDecision(): Promise { + const first = await ensureSetupCheck() + if (!first) return null + if (first.usableProviders.length > 0 || useSetupStore.getState().dismissed) return first + return await new Promise(resolve => { + const unsubscribe = useSetupStore.subscribe(state => { + const decided = state.dismissed || (state.check?.usableProviders.length ?? 0) > 0 + if (!decided) return + unsubscribe() + resolve(state.check) + }) + }) +} + +/** + * Providers the last check did not find, for the pickers' "not installed" + * hint (#995 finding 6: the failure used to surface only after the user had + * already chosen a directory). + * + * WHY a hint and never a disabled row: the probe can be wrong (#495 A1, an + * exotic shell or a Finder PATH), and a spawn does its own late re-resolve. + * A disabled row would turn a probe false-negative back into a lockout, which + * is the thing #995 removes. Empty until a check exists, so nothing is ever + * marked missing on a guess. + */ +export function useMissingProviders(): ReadonlySet { + const usable = useSetupStore(state => state.check?.usableProviders) + return useMemo( + () => new Set(usable ? AGENT_PROVIDER_KINDS.filter(kind => !usable.includes(kind)) : []), + [usable], + ) +} + +/** The provider a picker should preselect: the first-run choice when it is a + * provider, otherwise the default. Read once when a picker opens. */ +export function preferredPickerProvider(): AgentProviderKind { + const kind = useSetupStore.getState().check?.firstSessionKind + return kind && kind !== 'terminal' ? kind : DEFAULT_PROVIDER +} + +/** Hint shown beside a provider the last check did not find. */ +export const MISSING_PROVIDER_HINT = 'Not installed · File › Setup…' + +/** Test seam: the module-level in-flight promise outlives a store reset. */ +export function resetSetupStoreForTests(): void { + inFlight = null + useSetupStore.setState({ check: null, error: null, requested: false, dismissed: false }) +} diff --git a/src/renderer/src/features/setup/ui/SetupGate.tsx b/src/renderer/src/features/setup/ui/SetupGate.tsx index a8c6ef8dc..81544c353 100644 --- a/src/renderer/src/features/setup/ui/SetupGate.tsx +++ b/src/renderer/src/features/setup/ui/SetupGate.tsx @@ -1,11 +1,11 @@ import { useCallback, useEffect, useMemo, useState } from 'react' import type { - SetupCheckResult, SetupInstallTarget, SetupToolId, SetupToolStatus, } from '@shared/types/setup' +import { refreshSetupCheck, useSetupStore } from '@renderer/features/setup/store' // tmux is no longer listed in the SetupGate because it ships as a // bundled runtime artifact (#120). mitmdump will follow when its @@ -15,28 +15,49 @@ const OPTIONAL_INSTALL_TARGET: Partial> mitmdump: 'mitmproxy', } +/** + * The setup panel: which provider CLIs and helper tools this Mac has, and how + * to get the missing ones. + * + * WHY it can never lock the app (#995): it used to block until BOTH Claude + * Code and Codex were installed, with no Continue button, no install + * instructions and no way back once dismissed. A fresh Mac met a wall, even + * in the packaged app where OpenCode ships bundled and works. Now: + * - It opens by itself only when there is nothing to run agents with, or + * when a helper Homebrew can install is missing (the behavior that was + * already here). "Continue with a terminal" always answers it. + * - It opens on request from the File menu or the "Open Setup" command, and + * then Escape or Close closes it. + * - Missing providers show a copyable install command and a docs link. The + * manual path override stays, because the probes can be wrong (#495 A1). + */ export function SetupGate() { - const [check, setCheck] = useState(null) + const check = useSetupStore(state => state.check) + const error = useSetupStore(state => state.error) + const requested = useSetupStore(state => state.requested) + const dismissed = useSetupStore(state => state.dismissed) const [busy, setBusy] = useState('check') - const [error, setError] = useState(null) - const [dismissed, setDismissed] = useState(false) + const [actionError, setActionError] = useState(null) const refresh = useCallback(async () => { setBusy('check') - setError(null) - try { - setCheck(await window.api.setupCheck()) - } catch (err) { - setError(err instanceof Error ? err.message : String(err)) - } finally { - setBusy(null) - } + setActionError(null) + await refreshSetupCheck() + setBusy(null) }, []) useEffect(() => { void refresh() }, [refresh]) + // Opening on request re-probes: the user may have just run an installer + // in a terminal pane, and a stale "Not installed" would contradict them. + useEffect(() => { + if (requested) void refresh() + }, [requested, refresh]) + + const noProvider = check !== null && check.usableProviders.length === 0 + const missingOptional = useMemo(() => { if (!check) return [] // Bundled tools never appear here even when not yet extracted — @@ -44,42 +65,43 @@ export function SetupGate() { // tmux/mitmproxy" via Homebrew would be misleading. The bundled // archive resolves on first session spawn instead. return Object.values(check.tools).filter( - tool => !tool.required && !tool.found && tool.source !== 'bundled', + tool => !tool.provider && !tool.found && tool.source !== 'bundled', ) }, [check]) - const shouldShow = Boolean( + const automatic = Boolean( !dismissed && check && - (!check.ready || missingOptional.some(tool => tool.installable && !tool.skipped)), + (noProvider || missingOptional.some(tool => tool.installable && !tool.skipped)), ) + const shouldShow = Boolean(check && (requested || automatic)) const install = useCallback(async (target: SetupInstallTarget) => { setBusy(target) - setError(null) + setActionError(null) try { const result = await window.api.setupInstall(target) - setCheck(result.check) - if (!result.ok) setError(result.output || `Failed to install ${target}`) + useSetupStore.getState().setCheck(result.check) + if (!result.ok) setActionError(result.output || `Failed to install ${target}`) } catch (err) { - setError(err instanceof Error ? err.message : String(err)) + setActionError(err instanceof Error ? err.message : String(err)) } finally { setBusy(null) } }, []) // Manual path override (#495 A1). Automatic resolution is a probe and - // can be wrong (exotic $SHELL, rc-file breakage, Finder-launch PATH) — - // if a *required* tool is falsely reported missing there must be a way - // through that isn't "retry the same failing probe". ok:true hands back - // a fresh check, so a valid path unlocks Continue in one round-trip. + // can be wrong (exotic $SHELL, rc-file breakage, Finder-launch PATH), so + // there must be a way through that isn't "retry the same failing probe". + // ok:true hands back a fresh check, so a valid path takes effect in one + // round-trip. const setToolPath = useCallback(async (tool: SetupToolId, path: string): Promise => { setBusy('check') - setError(null) + setActionError(null) try { const result = await window.api.setupSetToolPath(tool, path) if (!result.ok) return result.reason - setCheck(result.check) + useSetupStore.getState().setCheck(result.check) return null } catch (err) { return err instanceof Error ? err.message : String(err) @@ -88,61 +110,93 @@ export function SetupGate() { } }, []) - const continueWithOptionalSkipped = useCallback(async () => { + // Answers the automatic panel. Optional installable helpers the user did + // not install are recorded as skipped (the pre-#995 behavior), so they + // stop reopening it on every launch. With no provider, this is the + // explicit "yes, just a terminal for now" acknowledgment. + const continueOn = useCallback(async () => { const skippedTools = missingOptional.filter(tool => tool.installable && !tool.skipped) setBusy('check') try { - let next = check for (const tool of skippedTools) { - next = await window.api.setupSkipOptional(tool.id) + useSetupStore.getState().setCheck(await window.api.setupSkipOptional(tool.id)) } - setCheck(next) - setDismissed(true) + useSetupStore.getState().close() } catch (err) { - setError(err instanceof Error ? err.message : String(err)) + setActionError(err instanceof Error ? err.message : String(err)) } finally { setBusy(null) } - }, [check, missingOptional]) + }, [missingOptional]) + + // Escape closes a panel the user opened. The automatic zero-provider panel + // is answered only by its button: the acknowledgment is the point of it, + // and an Escape pressed for something else must not silently decide that + // the first project is a terminal. + useEffect(() => { + if (!shouldShow || !requested) return + const onKeyDown = (event: KeyboardEvent) => { + if (event.key !== 'Escape') return + event.preventDefault() + event.stopPropagation() + useSetupStore.getState().close() + } + window.addEventListener('keydown', onKeyDown, true) + return () => window.removeEventListener('keydown', onKeyDown, true) + }, [shouldShow, requested]) if (!shouldShow || !check) return null + const providers = Object.values(check.tools).filter(tool => tool.provider) + const helpers = Object.values(check.tools).filter(tool => !tool.provider) + const shownError = actionError ?? error + return (
-
+
-
Agent Code Setup
+
+ {noProvider ? 'No agent provider is installed yet' : 'Agent Code Setup'} +
- Required tools must be available before agent sessions can start. + {noProvider + ? 'Install one of the CLIs below in a terminal, then press Retry. Or continue with a terminal now and install from there. Setup stays available from the File menu and the command palette.' + : 'The agent CLIs and helper tools on this Mac, and how to add the ones that are missing.'}
-
- {Object.values(check.tools).map(tool => ( - - ))} +
+ Agent providers +
+ {providers.map(tool => ( + + ))} +
+ Helper tools +
+ {helpers.map(tool => ( + + ))} +
- {error ? ( + {shownError ? (
- {error} + {shownError}
) : null} -
+
- Homebrew, Claude Code, and Codex are external prerequisites. + Any one provider is enough. A terminal pane needs none.
-
+
- {check.ready ? ( + {requested && !automatic ? ( + ) : ( + - ) : null} + )}
@@ -167,6 +230,14 @@ export function SetupGate() { ) } +function SectionLabel({ children }: { children: string }) { + return ( +
+ {children} +
+ ) +} + function SetupRow({ tool, busy, @@ -181,6 +252,7 @@ function SetupRow({ const [overrideOpen, setOverrideOpen] = useState(false) const [overridePath, setOverridePath] = useState('') const [overrideError, setOverrideError] = useState(null) + const [copied, setCopied] = useState(false) const submitOverride = async () => { const reason = await onSetPath(tool.id, overridePath) @@ -193,36 +265,41 @@ function SetupRow({ const target = OPTIONAL_INSTALL_TARGET[tool.id] const installing = target ? busy === target : false const isBundled = tool.source === 'bundled' - // Bundled tools always show as "Bundled" regardless of their - // required/optional metadata — that's the whole point of shipping - // them with the app, and the install button must never appear for + // Bundled tools always show as "Bundled": that is the whole point of + // shipping them with the app, and the install button must never appear for // them even if Homebrew is also present on the machine. const statusLabel = isBundled ? 'Bundled' : tool.found ? 'Found' - : tool.required - ? 'Required' + : tool.provider + ? 'Not installed' : tool.skipped ? 'Skipped' : 'Optional' - const statusBorder = isBundled - ? 'border-accent text-accent' - : tool.found - ? 'border-accent text-accent' - : tool.required - ? 'border-danger text-danger' - : 'border-border text-muted' + const statusBorder = tool.found || isBundled ? 'border-accent text-accent' : 'border-border text-muted' const detail = isBundled ? 'Shipped with Agent Code; no install required.' : (tool.path ?? tool.detail ?? 'Not found') - // The manual override renders only for missing REQUIRED tools: those - // are the ones whose false-negative locks the whole app (#495 A1). - // Optional tools already have Install/Skip affordances and can't block. - const canOverride = !isBundled && !tool.found && tool.required + const missingProvider = tool.provider && !tool.found && !isBundled + // The manual override is for providers: a false "Not installed" is the one + // probe error that costs the user an agent (#495 A1). Helpers have + // Install/Skip and never gate anything. + const canOverride = missingProvider + + const copy = async (command: string) => { + try { + await navigator.clipboard.writeText(command) + setCopied(true) + setTimeout(() => setCopied(false), 1500) + } catch { + // Clipboard denied: the command is selectable text right beside the + // button, so the user can still copy it by hand. + } + } return ( -
+
@@ -258,6 +335,31 @@ function SetupRow({
+ {missingProvider && tool.installCommand ? ( +
+ + {tool.installCommand} + + + {tool.docsUrl ? ( + + Docs + + ) : null} +
+ ) : null} + {canOverride && overrideOpen ? (
diff --git a/src/renderer/src/features/workspace/ui/NewAgentInDialog.tsx b/src/renderer/src/features/workspace/ui/NewAgentInDialog.tsx index 164378229..a7e696657 100644 --- a/src/renderer/src/features/workspace/ui/NewAgentInDialog.tsx +++ b/src/renderer/src/features/workspace/ui/NewAgentInDialog.tsx @@ -18,6 +18,7 @@ import { import { AGENT_PROVIDER_CHOICES } from '@renderer/workspace/providerChoices' import type { TabId } from '@renderer/workspace/types' import type { Workspace } from '@renderer/workspace/workspaceStore' +import { MISSING_PROVIDER_HINT, useMissingProviders } from '@renderer/features/setup/store' type Props = { open: boolean @@ -56,6 +57,7 @@ export function NewAgentInDialog({ open, workspace, onClose }: Props) { // A ref, not state: it must gate the synchronous key handler, not re-render. const committingRef = useRef(false) const [step, setStep] = useState('agent') + const missingProviders = useMissingProviders() const [agentIndex, setAgentIndex] = useState(0) // The highlighted project is held by TAB ID, not list index: the model is // live while the dialog is open (an MCP operator can close an agent or a @@ -239,7 +241,7 @@ export function NewAgentInDialog({ open, workspace, onClose }: Props) { `} >
{option.label}
-
{option.description}
+
{missingProviders.has(option.kind) ? MISSING_PROVIDER_HINT : option.description}
) }) diff --git a/src/renderer/src/features/workspace/ui/NewAgentPlacementOverlay.tsx b/src/renderer/src/features/workspace/ui/NewAgentPlacementOverlay.tsx index d61a5c999..0a711abcb 100644 --- a/src/renderer/src/features/workspace/ui/NewAgentPlacementOverlay.tsx +++ b/src/renderer/src/features/workspace/ui/NewAgentPlacementOverlay.tsx @@ -8,6 +8,7 @@ import type { TabId, } from '@renderer/workspace/types' import type { Workspace } from '@renderer/workspace/workspaceStore' +import { MISSING_PROVIDER_HINT, useMissingProviders } from '@renderer/features/setup/store' import { SESSION_SPAWN_CHOICES, type AgentProviderChoice, @@ -64,6 +65,7 @@ export function NewAgentPlacementOverlay({ }: Props) { const linkedMode = linkedAgentParentId !== null const [selectedIndex, setSelectedIndex] = useState(0) + const missingProviders = useMissingProviders() // One-shot latch around the spawn. Creation is async (spawns a session, // awaits an IPC round-trip, then closes the overlay). Until the close fires, // this overlay keeps its `open` prop true and its keydown listener @@ -202,7 +204,9 @@ export function NewAgentPlacementOverlay({ > {option.label} - {option.description} + {isAgentProviderKind(option.kind) && missingProviders.has(option.kind) + ? MISSING_PROVIDER_HINT + : option.description} ) diff --git a/src/renderer/src/features/workspace/ui/ProviderSwitchPickerModal.tsx b/src/renderer/src/features/workspace/ui/ProviderSwitchPickerModal.tsx index 038ca3936..d72110cb5 100644 --- a/src/renderer/src/features/workspace/ui/ProviderSwitchPickerModal.tsx +++ b/src/renderer/src/features/workspace/ui/ProviderSwitchPickerModal.tsx @@ -18,6 +18,7 @@ import { import type { Workspace } from '@renderer/workspace/workspaceStore' import type { SessionId } from '@renderer/workspace/types' import { isAgentProviderKind } from '@shared/types/providerKind' +import { MISSING_PROVIDER_HINT, useMissingProviders } from '@renderer/features/setup/store' type Props = { open: boolean @@ -42,6 +43,7 @@ export function ProviderSwitchPickerModal({ [sourceKind], ) const [selectedIndex, setSelectedIndex] = useState(0) + const missingProviders = useMissingProviders() useEffect(() => { if (!open) return @@ -157,7 +159,7 @@ export function ProviderSwitchPickerModal({ `} >
{choice.label}
-
{choice.description}
+
{missingProviders.has(choice.kind) ? MISSING_PROVIDER_HINT : choice.description}
) })} diff --git a/src/renderer/src/workspace/hook/persistence/useBootstrap.ts b/src/renderer/src/workspace/hook/persistence/useBootstrap.ts index f53c85468..bd69fa62d 100644 --- a/src/renderer/src/workspace/hook/persistence/useBootstrap.ts +++ b/src/renderer/src/workspace/hook/persistence/useBootstrap.ts @@ -8,6 +8,9 @@ import type { } from '@renderer/workspace/hook/context' import type { WorkspaceRefs } from '@renderer/workspace/hook/refs' +import type { SessionKind } from '@renderer/workspace/types' +import type { SetupCheckResult } from '@shared/types/setup' +import { awaitFirstRunDecision, ensureSetupCheck } from '@renderer/features/setup/store' import { rehydrateWorkspace } from '@renderer/workspace/hook/persistence/rehydrate' import { reconcileStuckTranscriptLoads } from '@renderer/workspace/hook/actions/initialHistory' import * as perf from '@renderer/performance/client' @@ -50,7 +53,7 @@ export function useBootstrap( refs: WorkspaceRefs, setState: WorkspaceSetState, setRuntimes: WorkspaceSetRuntimes, - newTab: (cwd: string) => Promise, + newTab: (cwd: string, resumeSessionId?: string, kind?: SessionKind) => Promise, setBootstrapComplete: (complete: boolean) => void, // Mirrors setBootstrapComplete in lifetime — set once at the end of // bootstrap to one of the WorkspaceRestoreStatus values. The composer @@ -86,8 +89,15 @@ export function useBootstrap( const cwd = await perf.measure('workspace.bootstrap.defaultCwd', () => window.api.defaultCwd(), ) + // #995: wait for the setup verdict before choosing what to spawn. + // This returns at once when any provider is usable; with none, it + // waits for the user to answer the setup panel (see + // awaitFirstRunDecision for why spawning underneath it was wrong). + const setup = await perf.measure('workspace.bootstrap.firstRunDecision', () => + awaitFirstRunDecision(), + ) try { - await perf.measure('workspace.bootstrap.initialNewTab', () => newTab(cwd)) + await perf.measure('workspace.bootstrap.initialNewTab', () => openFirstProject(newTab, cwd, setup)) canAutosaveBootState = refs.latestStateRef.current.tabs.length > 0 finalStatus = 'fresh' // A fresh install lands on ONE row × ONE lane showing its one @@ -170,7 +180,12 @@ export function useBootstrap( window.api.defaultCwd(), ) try { - await perf.measure('workspace.bootstrap.fallbackNewTab', () => newTab(cwd)) + // The recovery shell must come up on a machine without the + // default provider too, so it follows the same readiness verdict. + // It does not WAIT for the setup panel: a returning user with a + // broken file needs a surface now, not a first-run question. + const setup = await ensureSetupCheck() + await perf.measure('workspace.bootstrap.fallbackNewTab', () => openFirstProject(newTab, cwd, setup)) finalStatus = 'persisted-fallback' // The recovery shell gets the minimal [1] stage by the same route // as the fresh path: this is not the user's real workspace, just @@ -223,3 +238,32 @@ export function useBootstrap( // eslint-disable-next-line react-hooks/exhaustive-deps }, []) } + +/** + * Opens the first project of a run that has nothing to restore (#995). + * + * WHY a terminal is the last resort, after the chosen kind fails: a run with + * no tabs is the worst outcome bootstrap can produce. It keeps autosave off + * for the whole run (the no-tabs guard above), and the user faces an empty + * window with nothing to type into. That is what a clean Mac got before + * #995: Claude was spawned unconditionally, failed because it was not + * installed, and left nothing. A terminal needs no provider, and it is where + * the user runs the install commands the setup panel shows. + * + * `setup` null (the check itself failed) keeps the pre-#995 choice, the + * default provider: an unknown machine is not an empty one. + */ +async function openFirstProject( + newTab: (cwd: string, resumeSessionId?: string, kind?: SessionKind) => Promise, + cwd: string, + setup: SetupCheckResult | null, +): Promise { + const kind = setup?.firstSessionKind + try { + await newTab(cwd, undefined, kind) + } catch (err) { + if (kind === 'terminal') throw err + console.warn('[workspace] first project spawn failed; opening a terminal instead:', err) + await newTab(cwd, undefined, 'terminal') + } +} From 159176d32f22c9418f04419e4c7b950acef84735 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 19 Sep 2026 18:28:15 -0700 Subject: [PATCH 5/7] fix(setup): the panel owns focus, always answers, and says what its button does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #1047 (CHANGES REQUESTED): 1. The panel stamped the interaction-owner marker and said aria-modal but never moved focus. The keyboard router's ownership branch does not stop propagation and a terminal pane forwards keys straight to its PTY, so with focus left in an agent pane a user could type — and press Enter — into the live shell underneath a panel that looked modal, and Tab walked out into the background UI. It now composes the shared Dialog primitive, which owns focus containment, the inert background, the marker and Escape (components/ui/README.md makes that the primitive's job), and the hand-rolled window listener is gone. 2. "Continue with a terminal" recorded the skipped optional helpers BEFORE closing, inside the same try. A setup.json write failure (a full disk, a read-only state dir) therefore left the panel up with no Escape and the fresh-install bootstrap waiting on an answer that could never arrive — the lockout class #995 removes. The close moved to finally: recording a skip is best effort, the acknowledgment is not. 3. The button promised a terminal project in cases where none would be opened (a returning user with a restored workspace; a panel the user opened whose re-probe found no provider). The store now tracks whether a fresh-install bootstrap is parked on the answer, and only then does the button read "Continue with a terminal"; otherwise it is Continue or Close. Escape and click-outside are refused only in that same state, instead of for every automatic panel. 4. A failed install's output is capped and scrollable: unbounded, it pushed the footer and its only button past the viewport. Tests: focus containment and the inert background, the skip failure still answering the panel, and the Close wording when nothing waits. All three fail against the previous gate. Co-Authored-By: Claude Opus 5 (1M context) --- .../features/setup/firstRun.renderer.test.tsx | 69 +++++++++- src/renderer/src/features/setup/store.ts | 14 +- .../src/features/setup/ui/SetupGate.tsx | 125 +++++++++++------- 3 files changed, 152 insertions(+), 56 deletions(-) diff --git a/src/renderer/src/features/setup/firstRun.renderer.test.tsx b/src/renderer/src/features/setup/firstRun.renderer.test.tsx index 32e452e00..652afecca 100644 --- a/src/renderer/src/features/setup/firstRun.renderer.test.tsx +++ b/src/renderer/src/features/setup/firstRun.renderer.test.tsx @@ -94,8 +94,9 @@ describe('first run on a Mac with no provider (#995)', () => { // Before #995 bootstrap spawned Claude here, underneath the gate, and // the failure left no project and autosave off. expect(spawnSession).not.toHaveBeenCalled() - // The acknowledgment is the button; a stray Escape decides nothing. - fireEvent.keyDown(window, { key: 'Escape' }) + // The acknowledgment is the button; a stray Escape decides nothing while + // the bootstrap is parked on this answer. + fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Escape' }) expect(screen.getByRole('dialog')).toBeTruthy() expect(spawnSession).not.toHaveBeenCalled() // Answer it before the test ends: the waiting bootstrap is subscribed to @@ -153,7 +154,67 @@ describe('Setup can be reopened (#995 finding 2)', () => { }) expect(await screen.findByText('Agent Code Setup')).toBeTruthy() await waitFor(() => expect(setupCheck.mock.calls.length).toBeGreaterThan(probesBefore)) - fireEvent.keyDown(window, { key: 'Escape' }) - expect(screen.queryByRole('dialog')).toBeNull() + fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Escape' }) + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()) + }) + + it('takes focus, so keystrokes cannot reach the agent pane underneath (#1047 review)', async () => { + // The first version stamped the interaction-owner marker and said + // aria-modal but never moved focus. The keyboard router's ownership branch + // does not stop propagation and a terminal pane forwards keys to its PTY, + // so typing — and Enter — reached the live shell under a panel that looked + // modal. + const { setupCheck } = mountMachine([loadFirstRunCheck('developer-machine')]) + await waitFor(() => expect(projects()).toBe(1)) + const outside = document.createElement('textarea') + document.body.appendChild(outside) + outside.focus() + expect(setupCheck).toHaveBeenCalled() + await act(async () => { + setupCommands.find(command => command.id === 'open-setup')!.run({ ui: { closePalette: vi.fn() } } as unknown as CommandContext) + }) + const dialog = await screen.findByRole('dialog') + await waitFor(() => expect(dialog.contains(document.activeElement)).toBe(true)) + expect(document.activeElement).not.toBe(outside) + // Radix marks the rest of the document inert while the dialog is open. + expect(outside.closest('[aria-hidden="true"]') ?? document.body.getAttribute('aria-hidden')).toBeTruthy() + outside.remove() + }) +}) + +describe('the setup panel never strands the first run (#1047 review)', () => { + it('answers the panel even when recording the skipped helper fails', async () => { + // setup.json is best effort (a full disk, a read-only state dir). The + // acknowledgment is not: before this, the failure left the panel up with + // no Escape and the bootstrap waiting on a decision that could not arrive. + const check = withoutMachineWideInstalls(loadFirstRunCheck('clean-machine')) + const withMissingHelper: SetupCheckResult = { + ...check, + tools: { ...check.tools, mitmdump: { ...check.tools.mitmdump, found: false, path: null, source: undefined, installable: true, skipped: false } }, + } + const { spawnSession } = mountMachine([withMissingHelper]) + window.api.setupSkipOptional = vi.fn(async () => { throw new Error('ENOSPC: no space left on device') }) + fireEvent.click(await screen.findByRole('button', { name: 'Continue with a terminal' })) + await waitFor(() => expect(projects()).toBe(1)) + expect(spawnedKinds(spawnSession)).toEqual(['terminal']) + }) + + it('says Close, not "Continue with a terminal", when no bootstrap is waiting', async () => { + // A returning user whose workspace restored, on a machine with no + // provider: the panel still explains, but pressing the button dismisses + // it — it does not open a terminal project. + const check = withoutMachineWideInstalls(loadFirstRunCheck('clean-machine')) + const { spawnSession } = mountMachine([check]) + await screen.findByRole('dialog') + fireEvent.click(screen.getByRole('button', { name: 'Continue with a terminal' })) + await waitFor(() => expect(projects()).toBe(1)) + // Now nothing is waiting: reopen it and the button reads Close. + await act(async () => { + setupCommands.find(command => command.id === 'open-setup')!.run({ ui: { closePalette: vi.fn() } } as unknown as CommandContext) + }) + await screen.findByRole('dialog') + expect(screen.getByRole('button', { name: 'Close' })).toBeTruthy() + expect(screen.queryByRole('button', { name: 'Continue with a terminal' })).toBeNull() + expect(spawnedKinds(spawnSession)).toEqual(['terminal']) }) }) diff --git a/src/renderer/src/features/setup/store.ts b/src/renderer/src/features/setup/store.ts index 719e4fca5..34924c046 100644 --- a/src/renderer/src/features/setup/store.ts +++ b/src/renderer/src/features/setup/store.ts @@ -25,6 +25,15 @@ type SetupStore = { requested: boolean /** The automatic first-run panel was answered for this run. */ dismissed: boolean + /** + * A fresh-install bootstrap is parked on this panel's answer. + * + * WHY the panel needs to know (#1047 review): "Continue with a terminal" + * promises something only the waiting bootstrap can deliver. A returning + * user with a restored workspace, or a panel the user opened whose re-probe + * happens to find no provider, would read that label and get a dismissal. + */ + firstRunWaiting: boolean setCheck: (check: SetupCheckResult) => void setError: (error: string | null) => void open: () => void @@ -36,6 +45,7 @@ export const useSetupStore = create(set => ({ error: null, requested: false, dismissed: false, + firstRunWaiting: false, setCheck: check => set({ check, error: null }), setError: error => set({ error }), open: () => set({ requested: true }), @@ -96,11 +106,13 @@ export async function awaitFirstRunDecision(): Promise const first = await ensureSetupCheck() if (!first) return null if (first.usableProviders.length > 0 || useSetupStore.getState().dismissed) return first + useSetupStore.setState({ firstRunWaiting: true }) return await new Promise(resolve => { const unsubscribe = useSetupStore.subscribe(state => { const decided = state.dismissed || (state.check?.usableProviders.length ?? 0) > 0 if (!decided) return unsubscribe() + useSetupStore.setState({ firstRunWaiting: false }) resolve(state.check) }) }) @@ -138,5 +150,5 @@ export const MISSING_PROVIDER_HINT = 'Not installed · File › Setup…' /** Test seam: the module-level in-flight promise outlives a store reset. */ export function resetSetupStoreForTests(): void { inFlight = null - useSetupStore.setState({ check: null, error: null, requested: false, dismissed: false }) + useSetupStore.setState({ check: null, error: null, requested: false, dismissed: false, firstRunWaiting: false }) } diff --git a/src/renderer/src/features/setup/ui/SetupGate.tsx b/src/renderer/src/features/setup/ui/SetupGate.tsx index 81544c353..6a7a9f060 100644 --- a/src/renderer/src/features/setup/ui/SetupGate.tsx +++ b/src/renderer/src/features/setup/ui/SetupGate.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useState } from 'react' +import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import type { SetupInstallTarget, @@ -6,6 +6,12 @@ import type { SetupToolStatus, } from '@shared/types/setup' import { refreshSetupCheck, useSetupStore } from '@renderer/features/setup/store' +import { + Dialog, + DialogContent, + DialogDescription, + DialogTitle, +} from '@renderer/components/ui/dialog' // tmux is no longer listed in the SetupGate because it ships as a // bundled runtime artifact (#120). mitmdump will follow when its @@ -36,6 +42,8 @@ export function SetupGate() { const error = useSetupStore(state => state.error) const requested = useSetupStore(state => state.requested) const dismissed = useSetupStore(state => state.dismissed) + const firstRunWaiting = useSetupStore(state => state.firstRunWaiting) + const panelRef = useRef(null) const [busy, setBusy] = useState('check') const [actionError, setActionError] = useState(null) @@ -75,6 +83,11 @@ export function SetupGate() { (noProvider || missingOptional.some(tool => tool.installable && !tool.skipped)), ) const shouldShow = Boolean(check && (requested || automatic)) + // The one state that may not be dismissed by a stray key or click: a fresh + // install with nothing to run, whose bootstrap is parked on this answer. + // Everything else — a returning user, an optional helper, a panel the user + // opened — closes like any other dialog. + const mustAnswer = automatic && noProvider && firstRunWaiting const install = useCallback(async (target: SetupInstallTarget) => { setBusy(target) @@ -114,6 +127,14 @@ export function SetupGate() { // not install are recorded as skipped (the pre-#995 behavior), so they // stop reopening it on every launch. With no provider, this is the // explicit "yes, just a terminal for now" acknowledgment. + // + // WHY the close is in `finally` (#1047 review): it used to sit after the + // skip loop inside the `try`, so a setup.json write failure (a full disk, + // a read-only state dir) left the panel up with no way to answer it — the + // automatic panel takes no Escape — and the fresh-install bootstrap waited + // on a decision that could never arrive, which is the lockout class #995 + // exists to remove. Recording a skip is best effort; the user's + // acknowledgment is not. const continueOn = useCallback(async () => { const skippedTools = missingOptional.filter(tool => tool.installable && !tool.skipped) setBusy('check') @@ -121,29 +142,13 @@ export function SetupGate() { for (const tool of skippedTools) { useSetupStore.getState().setCheck(await window.api.setupSkipOptional(tool.id)) } - useSetupStore.getState().close() } catch (err) { setActionError(err instanceof Error ? err.message : String(err)) } finally { setBusy(null) - } - }, [missingOptional]) - - // Escape closes a panel the user opened. The automatic zero-provider panel - // is answered only by its button: the acknowledgment is the point of it, - // and an Escape pressed for something else must not silently decide that - // the first project is a terminal. - useEffect(() => { - if (!shouldShow || !requested) return - const onKeyDown = (event: KeyboardEvent) => { - if (event.key !== 'Escape') return - event.preventDefault() - event.stopPropagation() useSetupStore.getState().close() } - window.addEventListener('keydown', onKeyDown, true) - return () => window.removeEventListener('keydown', onKeyDown, true) - }, [shouldShow, requested]) + }, [missingOptional]) if (!shouldShow || !check) return null @@ -151,24 +156,45 @@ export function SetupGate() { const helpers = Object.values(check.tools).filter(tool => !tool.provider) const shownError = actionError ?? error + // WHY the shared Dialog primitive and not a bare overlay div (#1047 + // review): the first version stamped the interaction-owner marker and said + // aria-modal, but never moved focus. The router's ownership branch + // deliberately does not stop propagation, and a terminal pane forwards + // keystrokes straight to its PTY, so with focus left in an agent pane the + // user could type — and press Enter — into the live shell underneath a + // panel that looked modal. Tab walked out into the background UI too. + // DialogContent owns focus containment, the inert background, the marker + // and Escape; components/ui/README.md makes that the primitive's job. return ( -
-
+ { if (!next) useSetupStore.getState().close() }}> + { + // The panel itself, not the first row: the first focusable control + // is a provider's "Enter path manually…", and landing there reads + // as if that is what Setup is for. + event.preventDefault() + panelRef.current?.focus() + }} + // The automatic panel is answered only by its button: the + // acknowledgment is the point of it, and an Escape or a stray click + // pressed for something else must not silently decide that the first + // project is a terminal. A panel the user OPENED closes either way. + onEscapeKeyDown={event => { if (mustAnswer) event.preventDefault() }} + onInteractOutside={event => { if (mustAnswer) event.preventDefault() }} + >
-
+ {noProvider ? 'No agent provider is installed yet' : 'Agent Code Setup'} -
-
+ + {noProvider ? 'Install one of the CLIs below in a terminal, then press Retry. Or continue with a terminal now and install from there. Setup stays available from the File menu and the command palette.' : 'The agent CLIs and helper tools on this Mac, and how to add the ones that are missing.'} -
+
@@ -187,7 +213,11 @@ export function SetupGate() {
{shownError ? ( -
+ // Capped and scrollable: this can be the whole stdout+stderr of a + // failed `brew install` (homebrewInstaller's 8 MiB buffer). Unbounded, + // it pushed the footer — and the only button that answers the panel — + // past the bottom of the viewport (#1047 review). +
{shownError}
) : null} @@ -205,28 +235,21 @@ export function SetupGate() { > Retry - {requested && !automatic ? ( - - ) : ( - - )} + {/* One button, whose word is what pressing it actually does: + it opens the first project only while a fresh-install + bootstrap is waiting on this answer (#1047 review). */} +
-
-
+ + ) } From 14377943e9397b5e46f2969a40bc2ceb150ffff9 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 19 Sep 2026 18:45:40 -0700 Subject: [PATCH 6/7] fix(setup): find a provider where its own installer puts it, and remember the answer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review of #1047 (policy/bootstrap lens), CHANGES REQUESTED: 1. BLOCKING. The panel tells users to run OpenCode's installer, which writes ~/.opencode/bin and appends its PATH export to ~/.zshrc. The login-shell probe runs `$SHELL -lc`, and `zsh -lc` never sources .zshrc, so the directory was invisible to both resolution layers: the gate said "Not installed", the user ran the command it gave them, pressed Retry, and it still said "Not installed" — the one loop this feature exists to close. WELL_KNOWN_BIN_DIRS now includes it, and ~/.grok/bin for the same reason. A system test installs a stub there and fails without the change. 2. The provider-less answer is persisted in setup.json beside the skipped helpers. In memory only, it reopened the modal on every launch AND in every new window, each being its own renderer process. 3. Only the panel that ASKED records an answer. Closing a panel opened from the menu used to durably skip mitmproxy, while Escape in the same panel recorded nothing. 4. open-setup no longer closes a panel a parked bootstrap is waiting on: a menu item called Setup… must not silently decide "continue with a terminal" when Escape and click-outside are refused on purpose. 5. resetSetupStoreForTests settles a parked waiter, so one test's bootstrap can no longer resolve into the next one. 6. The live policy assertion is concrete per environment instead of restating firstSessionKindFor, which could only fail for a stamping mismatch. 7. The fixtures README says that recording rewrites the developer's real setup.json, and the plan records the accepted probe-deadline residual. Co-Authored-By: Claude Opus 5 (1M context) --- docs/decomposition/onboarding-first-run.md | 30 ++++++++++ src/main/ipc/setup.ts | 10 +++- src/main/setup/binaryResolver.ts | 12 ++++ src/main/setup/prerequisites.ts | 1 + src/main/setup/setupState.ts | 20 +++++++ .../setup/toolchain.bundledOverride.test.ts | 1 + src/preload/api/setup.ts | 2 + .../features/setup/commands/setupCommands.ts | 6 +- .../features/setup/firstRun.renderer.test.tsx | 56 ++++++++++++++++++- src/renderer/src/features/setup/store.ts | 15 ++++- .../src/features/setup/ui/SetupGate.tsx | 21 ++++++- src/shared/types/setup.ts | 7 +++ testing/fixtures/first-run/README.md | 7 +++ .../first-run/prerequisites.firstRun.test.ts | 43 ++++++++++++-- 14 files changed, 219 insertions(+), 12 deletions(-) diff --git a/docs/decomposition/onboarding-first-run.md b/docs/decomposition/onboarding-first-run.md index 57d49ed7a..718da54ff 100644 --- a/docs/decomposition/onboarding-first-run.md +++ b/docs/decomposition/onboarding-first-run.md @@ -161,6 +161,36 @@ is what the gate, the bootstrap and the pickers read. - **Unknown 1 (#994)** had already merged as #1002: a bundled OpenCode is `found` with `source:'bundled'` and counts as a usable provider. +### Review findings resolved after the first implementation + +Two adversarial reviews (UI/keyboard, then policy/bootstrap) found these, all +fixed on the branch: + +- **The install command pointed where the resolver never looked.** OpenCode's + installer writes `~/.opencode/bin` and exports it from `~/.zshrc`, which + `zsh -lc` does not source — so "install, press Retry" still said "Not + installed". `WELL_KNOWN_BIN_DIRS` now includes that directory and `~/.grok/bin`, + with a system test that installs a stub there and fails without the change. +- **The panel claimed modality without focus.** It is a real `Dialog` now. +- **The acknowledgment could be lost or repeated.** It closes in `finally`, and + "continue without a provider" is persisted in `setup.json` beside the skipped + helpers, so it does not reopen every launch and in every window. +- **Only the panel that asked records an answer**, so Close and Escape no longer + differ in durable effect. +- **`open-setup` can no longer answer a parked panel.** +- **The test seam settles a parked waiter**, so one test's bootstrap cannot + resolve into the next. +- **The live policy assertion is concrete per environment** instead of restating + the policy function. + +### Accepted, with reasons + +- The first project waits on the prerequisites probe with no independent + deadline (worst case two login-shell layers at 10 s each; measured 0.02–0.03 s + here). A deadline would have to invent a verdict for a slow machine, which is + what the lockout did; if this ever bites, bound the PROBE rather than the + wait. + ### Still open - Login detection (#995 finding 7) is out of scope here, as planned. diff --git a/src/main/ipc/setup.ts b/src/main/ipc/setup.ts index aa3957499..a77f0ce47 100644 --- a/src/main/ipc/setup.ts +++ b/src/main/ipc/setup.ts @@ -8,7 +8,7 @@ import type { import { classifyExecutable } from '@main/setup/binaryResolver.js' import { installWithHomebrew } from '@main/setup/homebrewInstaller.js' import { checkPrerequisites } from '@main/setup/prerequisites.js' -import { markOptionalSkipped, setManualToolPath } from '@main/setup/setupState.js' +import { markNoProvidersAcknowledged, markOptionalSkipped, setManualToolPath } from '@main/setup/setupState.js' import { refreshToolchainFromState } from '@main/setup/toolchain.js' export function registerSetupIpc(): void { @@ -38,6 +38,14 @@ export function registerSetupIpc(): void { return await checkPrerequisites() }) + // The first-run answer "continue without a provider" (#995). Durable for the + // same reason a skipped helper is: otherwise the panel reopens on every + // launch, and in every new window, for a user who has already answered it. + ipcMain.handle('setup:acknowledge-no-providers', async () => { + await markNoProvidersAcknowledged() + return await checkPrerequisites() + }) + // Escape hatch for #495 A1: automatic resolution is a probe, and a // probe false-negative must never be the *sole* gate on the whole // product. The user pastes an absolute path; we accept it iff it is an diff --git a/src/main/setup/binaryResolver.ts b/src/main/setup/binaryResolver.ts index f5456a5b2..877345bc5 100644 --- a/src/main/setup/binaryResolver.ts +++ b/src/main/setup/binaryResolver.ts @@ -77,6 +77,18 @@ export async function isExecutable(path: string): Promise { // dirs earn their keep. const WELL_KNOWN_BIN_DIRS = [ join(homedir(), '.local', 'bin'), + // WHY ~/.opencode/bin (#995 Codex review, reproduced): OpenCode's own + // installer — the command the setup panel now tells users to run — writes + // the binary here and appends its PATH export to ~/.zshrc. The login-shell + // probe runs `$SHELL -lc`, and `zsh -lc` sources .zprofile/.zshenv but NOT + // .zshrc, so the probe cannot see it either. Without this entry the loop the + // feature exists to close dead-ended: the gate says "Not installed", the + // user runs the command it gave them, presses Retry, and it still says + // "Not installed". + join(homedir(), '.opencode', 'bin'), + // The Grok CLI's own layout, for the same reason. Its npm install lands in a + // prefix already scanned below, but a native install puts the binary here. + join(homedir(), '.grok', 'bin'), join(homedir(), '.volta', 'bin'), join(homedir(), '.asdf', 'shims'), join(homedir(), '.bun', 'bin'), diff --git a/src/main/setup/prerequisites.ts b/src/main/setup/prerequisites.ts index 1d9b03414..814db09cb 100644 --- a/src/main/setup/prerequisites.ts +++ b/src/main/setup/prerequisites.ts @@ -172,6 +172,7 @@ export async function checkPrerequisites(): Promise { return { checkedAt: Date.now(), tools, + noProvidersAcknowledged: state.acknowledgedNoProviders, ...deriveReadiness(tools), } } diff --git a/src/main/setup/setupState.ts b/src/main/setup/setupState.ts index b7b52b167..289720856 100644 --- a/src/main/setup/setupState.ts +++ b/src/main/setup/setupState.ts @@ -45,6 +45,18 @@ export type PersistedSetupState = { // version bump / migration is needed. manualToolPaths: Partial> skippedOptionalTools: Partial> + /** + * The user answered "continue without an agent provider" (#995). + * + * WHY this is persisted rather than a per-run flag: a deliberate + * terminal-only user answered the first-run panel once, and an in-memory + * flag made it reopen on every launch AND in every new window — each window + * is its own renderer process with its own store (#995 Codex review). The + * skipped-helper answer above is durable for exactly the same reason. + * Absent from setup.json files written before this field existed; + * loadSetupState defaults it, so no migration is needed. + */ + acknowledgedNoProviders: boolean // Auto-updater behavior + cache. Same "additive, no version bump" // rationale as manualToolPaths: absent from older setup.json blobs, // loadSetupState defaults it, no migration path required. The @@ -63,6 +75,7 @@ const DEFAULT_SETUP_STATE: PersistedSetupState = { toolPaths: {}, manualToolPaths: {}, skippedOptionalTools: {}, + acknowledgedNoProviders: false, cliUpdateBehavior: 'automatic', cliUpdateCache: {}, updatedAt: 0, @@ -81,6 +94,7 @@ export async function loadSetupState(): Promise { toolPaths: parsed.toolPaths ?? {}, manualToolPaths: parsed.manualToolPaths ?? {}, skippedOptionalTools: parsed.skippedOptionalTools ?? {}, + acknowledgedNoProviders: parsed.acknowledgedNoProviders === true, // Coerce the CLI-update fields defensively: a hand-edited setup.json // with a stray string for cliUpdateBehavior must not throw at load — // fall back to 'automatic'. Same discipline as customAppearance in @@ -179,6 +193,12 @@ export async function markOptionalSkipped( }) } +/** Records that the user chose to continue with no provider installed. */ +export async function markNoProvidersAcknowledged(): Promise { + const state = await loadSetupState() + return await saveSetupState({ ...state, acknowledgedNoProviders: true }) +} + /** Persist the user's CLI auto-update preference. Written by the setting * row in the renderer via IPC — same shape as markOptionalSkipped: * pure bookkeeping over the persisted state. */ diff --git a/src/main/setup/toolchain.bundledOverride.test.ts b/src/main/setup/toolchain.bundledOverride.test.ts index b94c673e6..135e91e55 100644 --- a/src/main/setup/toolchain.bundledOverride.test.ts +++ b/src/main/setup/toolchain.bundledOverride.test.ts @@ -44,6 +44,7 @@ function setupState(overrides: { toolPaths: overrides.toolPaths ?? {}, manualToolPaths: overrides.manualToolPaths ?? {}, skippedOptionalTools: {}, + acknowledgedNoProviders: false, cliUpdateBehavior: 'automatic' as const, cliUpdateCache: {}, updatedAt: 0, diff --git a/src/preload/api/setup.ts b/src/preload/api/setup.ts index 94509f6c1..be6a9a3f8 100644 --- a/src/preload/api/setup.ts +++ b/src/preload/api/setup.ts @@ -15,6 +15,8 @@ export const setupApi = { ipcRenderer.invoke('setup:install', target), setupSkipOptional: (tool: SetupToolId): Promise => ipcRenderer.invoke('setup:skip-optional', tool), + setupAcknowledgeNoProviders: (): Promise => + ipcRenderer.invoke('setup:acknowledge-no-providers'), setupSetToolPath: (tool: SetupToolId, path: string): Promise => ipcRenderer.invoke('setup:set-tool-path', tool, path), } diff --git a/src/renderer/src/features/setup/commands/setupCommands.ts b/src/renderer/src/features/setup/commands/setupCommands.ts index d65d84666..0c4ac3f49 100644 --- a/src/renderer/src/features/setup/commands/setupCommands.ts +++ b/src/renderer/src/features/setup/commands/setupCommands.ts @@ -22,7 +22,11 @@ export const setupCommands: CommandDef[] = [{ run: ({ ui }) => { ui.closePalette() const store = useSetupStore.getState() - if (store.requested) store.close() + // Never let the command ANSWER the panel (#995 Codex review): while a + // fresh-install bootstrap is parked on it, Escape and click-outside are + // refused on purpose, and closing through this toggle would silently + // decide "continue with a terminal" from a menu item called Setup…. + if (store.requested && !store.firstRunWaiting) store.close() else store.open() }, }] diff --git a/src/renderer/src/features/setup/firstRun.renderer.test.tsx b/src/renderer/src/features/setup/firstRun.renderer.test.tsx index 652afecca..ecdaf4aef 100644 --- a/src/renderer/src/features/setup/firstRun.renderer.test.tsx +++ b/src/renderer/src/features/setup/firstRun.renderer.test.tsx @@ -67,6 +67,10 @@ function mountMachine(checks: SetupCheckResult[], options: { failKinds?: string[ defaultCwd: async () => '/Users/someone', setupCheck, setupSkipOptional: vi.fn(async () => current), + setupAcknowledgeNoProviders: vi.fn(async () => { + current = { ...current, noProvidersAcknowledged: true } + return current + }), spawnSession, onOrchestrationRequest: () => () => undefined, onAgentManagementRequest: () => () => undefined, @@ -76,7 +80,7 @@ function mountMachine(checks: SetupCheckResult[], options: { failKinds?: string[ } }) const hook = renderHook(() => useWorkspace()) render() - return { hook, spawnSession, setupCheck } + return { hook, spawnSession, setupCheck, api: window.api } } const spawnedKinds = (spawnSession: ReturnType['spawnSession']) => @@ -199,6 +203,56 @@ describe('the setup panel never strands the first run (#1047 review)', () => { expect(spawnedKinds(spawnSession)).toEqual(['terminal']) }) + it('records the provider-less answer, so the panel stops opening by itself', async () => { + // A deliberate terminal-only install answered this once. An in-memory + // flag made the modal reappear on every launch and in every window, each + // of which is its own renderer process with its own store. + const { api } = mountMachine([withoutMachineWideInstalls(loadFirstRunCheck('clean-machine'))]) + fireEvent.click(await screen.findByRole('button', { name: 'Continue with a terminal' })) + await waitFor(() => expect(projects()).toBe(1)) + expect(api.setupAcknowledgeNoProviders).toHaveBeenCalledOnce() + }) + + it('does not record an answer for a panel the user merely opened', async () => { + // Close on a panel opened from the menu used to durably skip mitmproxy, + // while Escape in the same panel recorded nothing. + const check = loadFirstRunCheck('developer-machine') + const { api } = mountMachine([{ + ...check, + tools: { ...check.tools, mitmdump: { ...check.tools.mitmdump, found: false, path: null, source: undefined, installable: true, skipped: false } }, + }]) + await waitFor(() => expect(projects()).toBe(1)) + // The automatic panel is up for the missing helper: answering it records. + fireEvent.click(await screen.findByRole('button', { name: 'Continue' })) + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()) + expect(api.setupSkipOptional).toHaveBeenCalled() + vi.mocked(api.setupSkipOptional).mockClear() + // Reopening it by hand and closing records nothing. + await act(async () => { + setupCommands.find(command => command.id === 'open-setup')!.run({ ui: { closePalette: vi.fn() } } as unknown as CommandContext) + }) + fireEvent.click(await screen.findByRole('button', { name: 'Close' })) + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()) + expect(api.setupSkipOptional).not.toHaveBeenCalled() + }) + + it('Open Setup cannot answer the panel a parked bootstrap is waiting on', async () => { + // Running the command twice used to close the panel, which resolved the + // parked bootstrap into a terminal project — from a menu item called + // Setup…, while Escape and click-outside are refused on purpose. + const { spawnSession } = mountMachine([withoutMachineWideInstalls(loadFirstRunCheck('clean-machine'))]) + await screen.findByRole('dialog') + const run = () => act(async () => { + setupCommands.find(command => command.id === 'open-setup')!.run({ ui: { closePalette: vi.fn() } } as unknown as CommandContext) + }) + await run() + await run() + expect(screen.getByRole('dialog')).toBeTruthy() + expect(spawnSession).not.toHaveBeenCalled() + fireEvent.click(screen.getByRole('button', { name: 'Continue with a terminal' })) + await waitFor(() => expect(projects()).toBe(1)) + }) + it('says Close, not "Continue with a terminal", when no bootstrap is waiting', async () => { // A returning user whose workspace restored, on a machine with no // provider: the panel still explains, but pressing the button dismisses diff --git a/src/renderer/src/features/setup/store.ts b/src/renderer/src/features/setup/store.ts index 34924c046..9750b9edb 100644 --- a/src/renderer/src/features/setup/store.ts +++ b/src/renderer/src/features/setup/store.ts @@ -111,13 +111,23 @@ export async function awaitFirstRunDecision(): Promise const unsubscribe = useSetupStore.subscribe(state => { const decided = state.dismissed || (state.check?.usableProviders.length ?? 0) > 0 if (!decided) return + settleFirstRunWaiter(state.check) + }) + // Held at module scope so resetSetupStoreForTests can settle a parked + // waiter (#995 Codex review). Without it a waiter from one test outlived + // the reset, unsubscribed from nothing, and resolved on the NEXT test's + // first check — spawning a second project there. + settleFirstRunWaiter = check => { unsubscribe() + settleFirstRunWaiter = () => undefined useSetupStore.setState({ firstRunWaiting: false }) - resolve(state.check) - }) + resolve(check ?? null) + } }) } +let settleFirstRunWaiter: (check: SetupCheckResult | null | undefined) => void = () => undefined + /** * Providers the last check did not find, for the pickers' "not installed" * hint (#995 finding 6: the failure used to surface only after the user had @@ -150,5 +160,6 @@ export const MISSING_PROVIDER_HINT = 'Not installed · File › Setup…' /** Test seam: the module-level in-flight promise outlives a store reset. */ export function resetSetupStoreForTests(): void { inFlight = null + settleFirstRunWaiter(null) useSetupStore.setState({ check: null, error: null, requested: false, dismissed: false, firstRunWaiting: false }) } diff --git a/src/renderer/src/features/setup/ui/SetupGate.tsx b/src/renderer/src/features/setup/ui/SetupGate.tsx index 6a7a9f060..2c0280a8a 100644 --- a/src/renderer/src/features/setup/ui/SetupGate.tsx +++ b/src/renderer/src/features/setup/ui/SetupGate.tsx @@ -80,7 +80,10 @@ export function SetupGate() { const automatic = Boolean( !dismissed && check && - (noProvider || missingOptional.some(tool => tool.installable && !tool.skipped)), + // A provider-less machine whose owner already answered is a DELIBERATE + // terminal-only install, not an unfinished setup (#995 Codex review). + ((noProvider && !check.noProvidersAcknowledged) + || missingOptional.some(tool => tool.installable && !tool.skipped)), ) const shouldShow = Boolean(check && (requested || automatic)) // The one state that may not be dismissed by a stray key or click: a fresh @@ -136,19 +139,31 @@ export function SetupGate() { // exists to remove. Recording a skip is best effort; the user's // acknowledgment is not. const continueOn = useCallback(async () => { - const skippedTools = missingOptional.filter(tool => tool.installable && !tool.skipped) + // Only the panel that ASKED records an answer (#995 Codex review). A user + // who opened Setup from the menu to look around and pressed Close used to + // durably skip mitmproxy on their way out, while pressing Escape in the + // same panel recorded nothing — two exits from one dialog with different + // lasting effects. + const skippedTools = automatic + ? missingOptional.filter(tool => tool.installable && !tool.skipped) + : [] setBusy('check') try { for (const tool of skippedTools) { useSetupStore.getState().setCheck(await window.api.setupSkipOptional(tool.id)) } + // The provider-less answer is durable too, or the panel reopens on every + // launch and in every window for someone who has already answered it. + if (automatic && noProvider) { + useSetupStore.getState().setCheck(await window.api.setupAcknowledgeNoProviders()) + } } catch (err) { setActionError(err instanceof Error ? err.message : String(err)) } finally { setBusy(null) useSetupStore.getState().close() } - }, [missingOptional]) + }, [automatic, missingOptional, noProvider]) if (!shouldShow || !check) return null diff --git a/src/shared/types/setup.ts b/src/shared/types/setup.ts index e5c1daf6b..b98cac008 100644 --- a/src/shared/types/setup.ts +++ b/src/shared/types/setup.ts @@ -73,6 +73,13 @@ export type SetupCheckResult = { * when it is usable, otherwise the first usable one, otherwise a terminal. */ firstSessionKind: AgentProviderKind | 'terminal' + /** + * The user has already answered "continue without a provider" on this + * machine. The panel stops opening by itself for that reason; it is still + * one command away. Without this, a terminal-only user met the modal on + * every launch and in every new window. + */ + noProvidersAcknowledged: boolean } // Targets the SetupGate's "Install via Homebrew" button can hand to diff --git a/testing/fixtures/first-run/README.md b/testing/fixtures/first-run/README.md index 8e1ede8c4..e0de14739 100644 --- a/testing/fixtures/first-run/README.md +++ b/testing/fixtures/first-run/README.md @@ -45,3 +45,10 @@ RECORD_FIRST_RUN=1 npx vitest run --project system testing/system/first-run unsets those rows and re-derives readiness with the real policy. Paths under the recording HOME are written as `~`, so the files carry no username. + +**Recording touches your real setup state.** The `developer-machine` environment +deliberately keeps the real `HOME`, and `STATE_DIR` follows it, so +`checkPrerequisites` writes its resolved paths back to your own +`~/.config/agent-code/setup.json` (and a probe that transiently misses clears +that tool's cached path). The two clean environments use a temp `HOME` and +cannot touch it. diff --git a/testing/system/first-run/prerequisites.firstRun.test.ts b/testing/system/first-run/prerequisites.firstRun.test.ts index a23baf0ec..9662afd44 100644 --- a/testing/system/first-run/prerequisites.firstRun.test.ts +++ b/testing/system/first-run/prerequisites.firstRun.test.ts @@ -164,6 +164,33 @@ const DESCRIPTIONS: Record = { "The recording developer's real HOME, PATH and SHELL, with every provider CLI installed. Machine-specific; replayed only as policy input.", } +describe.skipIf(process.platform !== 'darwin')('a provider installed by the command the panel shows is found (#995)', () => { + // The loop this feature exists to close: the panel says "Not installed", + // the user runs the command it gave them, presses Retry. OpenCode's + // installer writes ~/.opencode/bin and appends its PATH export to ~/.zshrc, + // which `zsh -lc` never sources — so the probe can only see it if the + // resolver knows that directory. Reproduced by Codex review of this PR. + it.each([ + { provider: 'opencode', dir: '.opencode' }, + { provider: 'grok', dir: '.grok' }, + ] as const)('$provider installed under ~/$dir/bin', async ({ provider, dir }) => { + appRoot.path = await temp('first-run-app-') + const home = await temp('first-run-home-') + process.env.HOME = home + process.env.PATH = '/usr/bin:/bin:/usr/sbin:/sbin' + process.env.SHELL = '/bin/sh' + const bin = join(home, dir, 'bin') + await mkdir(bin, { recursive: true }) + await writeFile(join(bin, provider), '#!/bin/sh\nexit 0\n') + await chmod(join(bin, provider), 0o755) + vi.resetModules() + const { checkPrerequisites } = await import('@main/setup/prerequisites.js') + const result = await checkPrerequisites() + expect(result.tools[provider]).toMatchObject({ found: true, source: 'system', path: join(bin, provider) }) + expect(result.usableProviders).toContain(provider) + }, 60_000) +}) + describe.skipIf(process.platform !== 'darwin')('first-run prerequisites on a simulated clean Mac (#995)', () => { it.skipIf(!RECORD)('records every environment', async () => { await mkdir(FIXTURES, { recursive: true }) @@ -200,12 +227,20 @@ describe.skipIf(process.platform !== 'darwin')('first-run prerequisites on a sim // project is always something this machine can run. On the macOS CI // runner, which has no provider CLI, the unbundled case is the genuine // zero-provider Mac and must come out as a terminal. - const usable = live.usableProviders - expect(live.firstSessionKind).toBe(usable.includes('claude') ? 'claude' : usable[0] ?? 'terminal') expect(live).not.toHaveProperty('blocking') + // Concrete per environment, not a re-implementation of the policy + // (#995 Codex review): restating `firstSessionKindFor` here could only + // ever fail for a stamping mismatch, never for a policy change. if (environment === 'clean-machine-packaged') { - expect(usable).toContain('opencode') - expect(live.firstSessionKind).not.toBe('terminal') + // The packaged app ships OpenCode, so this holds on any machine. + expect(live.usableProviders).toContain('opencode') + expect(live.firstSessionKind).toBe('opencode') + } else if (live.usableProviders.length === 0) { + // The CI runner: a genuinely provider-less Mac. + expect(live.firstSessionKind).toBe('terminal') + } else { + // This developer's machine keeps a machine-wide Grok outside HOME. + expect(live.firstSessionKind).toBe(live.usableProviders[0]) } }, 60_000, From cf33fcd0c1192deefea5ec4f47324e90d2b480f8 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 19 Sep 2026 18:56:25 -0700 Subject: [PATCH 7/7] fix(setup): an acknowledged terminal-only machine never parks a new window's bootstrap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review of #1047, P1. awaitFirstRunDecision treated only a live provider or this run's dismissal as an answer. A machine that had already chosen "continue with a terminal" therefore parked forever when a SECOND window opened: its workspace slice is absent, so bootstrap takes the fresh path, while the panel correctly refused to show itself because the answer was persisted — leaving nothing to release the wait. One `decided()` predicate now covers all three ways the question is already answered, and it is used both before parking and inside the subscription. The new test opens such a window and expects a terminal project with no dialog. Co-Authored-By: Claude Opus 5 (1M context) --- .../features/setup/firstRun.renderer.test.tsx | 12 ++++++++++++ src/renderer/src/features/setup/store.ts | 18 +++++++++++++++--- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/renderer/src/features/setup/firstRun.renderer.test.tsx b/src/renderer/src/features/setup/firstRun.renderer.test.tsx index ecdaf4aef..cef989661 100644 --- a/src/renderer/src/features/setup/firstRun.renderer.test.tsx +++ b/src/renderer/src/features/setup/firstRun.renderer.test.tsx @@ -213,6 +213,18 @@ describe('the setup panel never strands the first run (#1047 review)', () => { expect(api.setupAcknowledgeNoProviders).toHaveBeenCalledOnce() }) + it('a second window on an acknowledged terminal-only machine opens its project without waiting', async () => { + // A new window's workspace slice is absent, so its bootstrap takes the + // fresh path. The panel correctly stays hidden — the machine already + // answered — so nothing would ever have released a bootstrap that waited + // for that answer again (#995 Codex review). + const acknowledged = { ...withoutMachineWideInstalls(loadFirstRunCheck('clean-machine')), noProvidersAcknowledged: true } + const { spawnSession } = mountMachine([acknowledged]) + await waitFor(() => expect(projects()).toBe(1)) + expect(spawnedKinds(spawnSession)).toEqual(['terminal']) + expect(screen.queryByRole('dialog')).toBeNull() + }) + it('does not record an answer for a panel the user merely opened', async () => { // Close on a panel opened from the menu used to durably skip mitmproxy, // while Escape in the same panel recorded nothing. diff --git a/src/renderer/src/features/setup/store.ts b/src/renderer/src/features/setup/store.ts index 9750b9edb..efab5b3d5 100644 --- a/src/renderer/src/features/setup/store.ts +++ b/src/renderer/src/features/setup/store.ts @@ -105,12 +105,15 @@ export function ensureSetupCheck(): Promise { export async function awaitFirstRunDecision(): Promise { const first = await ensureSetupCheck() if (!first) return null - if (first.usableProviders.length > 0 || useSetupStore.getState().dismissed) return first + // The persisted answer counts as an answer (#995 Codex review). Without it, + // a terminal-only install that already acknowledged this opened a SECOND + // window — whose workspace slice is absent, so bootstrap runs again — and + // parked forever on a panel that correctly refused to show itself. + if (decided(first)) return first useSetupStore.setState({ firstRunWaiting: true }) return await new Promise(resolve => { const unsubscribe = useSetupStore.subscribe(state => { - const decided = state.dismissed || (state.check?.usableProviders.length ?? 0) > 0 - if (!decided) return + if (!decided(state.check)) return settleFirstRunWaiter(state.check) }) // Held at module scope so resetSetupStoreForTests can settle a parked @@ -128,6 +131,15 @@ export async function awaitFirstRunDecision(): Promise let settleFirstRunWaiter: (check: SetupCheckResult | null | undefined) => void = () => undefined +/** The three ways the first-run question is already answered: a provider + * exists, this run dismissed the panel, or the machine acknowledged running + * without one on an earlier launch. */ +function decided(check: SetupCheckResult | null | undefined): boolean { + return useSetupStore.getState().dismissed + || (check?.usableProviders.length ?? 0) > 0 + || check?.noProvidersAcknowledged === true +} + /** * Providers the last check did not find, for the pickers' "not installed" * hint (#995 finding 6: the failure used to surface only after the user had