fix(onboarding): keep the first run on Stage 0 and make its routing legible - #611
Open
kateebonner wants to merge 2 commits into
Open
fix(onboarding): keep the first run on Stage 0 and make its routing legible#611kateebonner wants to merge 2 commits into
kateebonner wants to merge 2 commits into
Conversation
…egible Three defects found while testing a genuine first run in an isolated sandbox (clean HOME, no model configured, no profile, no completion marker). The overture and its walkthrough were spliced correctly every time; the user never reached them. - Pasqal python provisioning fired `amicode.restartServer` while the Stage 0 webview was live. On a first run that provisioning always lands mid-onboarding, and the restart strands the panel on its "Getting Amico ready..." splash. The restart is now deferred while an onboarding panel is open — the panel already issues its own restart on submit, which is where the fresh AMICO_PYTHON gets picked up. - The onReady gate re-derived its decision inline from `isModelConfigured()` alone, so it ignored the completion marker and provider env vars. It now calls `resolveOnboardingAction`, which was written and unit-tested for exactly this and never wired up — the module was reachable only from its own tests. - The routing was entirely silent. A first run that lands on an empty chat left nothing to explain why, which is what made this hard to diagnose. Both the inputs and the chosen action are now logged to the opencode channel, and a failure to open the webview is reported instead of swallowed by a floating promise. Also drops the dead `OnboardingLauncher` and the `welcome_shown` helpers: nothing read or wrote them, `resolveOnboardingAction` never consulted the flag, and extension.ts owns launching via `serverManager.onReady`. Pre-existing and untouched: 4 failures in test/ops/skill_freshness_orchestrator.test.ts, identical on the base.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The actual reason a first run never reached Stage 0.
`respawnForVault()` runs on every genuine first run — `ensureDefaultPersonalVault()`
provisions a vault, then respawns — and it CONSTRUCTS A NEW ServerManager with
its own onReady:
serverManager.onReady((url) => {
opencodeReadyUrl = url;
statusBar?.setServerReady(true);
sseClient?.connect(url);
});
No onboarding gate, no chat open. It replaces the boot manager, so the boot
handler — the only one that routes onboarding — is orphaned, and the server that
actually comes up is owned by a handler that just wires SSE. Stage 0 never opens
and neither does the chat: the user lands on whatever surface happens to exist.
Confirmed from a virgin-HOME run: `[sse] connecting` was logged (respawn's
handler) while the routing log never appeared (boot's handler, orphaned).
The post-ready routing is now a named `routePostReady`, assigned at boot and
invoked by the respawn handler too. Verified on a clean first run:
[onboarding] action=show-webview modelConfigured=false providerEnv=false completed=false autoOpen=true
[onboarding] Stage 0 webview opened
[pasqal] python provisioned: ... — restart deferred (onboarding in progress)
The solver-mode switch in watchSolverMode() replaces serverManager the same way
and is left alone here: it fires post-onboarding, where re-running the routing
would re-open chat mid-switch. Noted in the PR as its own decision.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
Problem: On a genuine first run — clean
HOME, no model configured, no profile, no completion marker — the user never reaches the overture or its studio walkthrough. The score is spliced correctly every time (AGENTS.mdcarries all 9Tour ·stops,score_manifest.jsonis written, the onboarding branch is taken); the failure is entirely in surfacing Stage 0.Approach: Stop tearing the Stage 0 panel down mid-flow, route through the predicate that was already written and tested for this, and make the decision observable so the next regression is one log line instead of an afternoon.
Scope: Extension-side onboarding routing only. No score changes — the
Tour · Xheader sync contract with opencode#258 is untouched.What was wrong
Found while driving a real first run in an isolated sandbox (separate
HOME,--user-data-dir,--extensions-dir).1. Provisioning restarts strand the Stage 0 panel.
provisionPasqalPython()firesamicode.restartServerthe moment it finishes. On a first run that always lands while the Stage 0 webview is waiting on the server, so the panel sits on itsGetting Amico ready...splash forever. Observed directly:The restart is now deferred while an onboarding panel is live. Nothing is lost: the panel issues its own
amicode.restartServeron submit, which is where the freshAMICO_PYTHONis picked up.2. The gate ignored two of its own inputs. The
onReadyhandler branched onisModelConfigured()alone, so it never consulted the completion marker or provider env vars.resolveOnboardingActionexists for exactly this decision, is unit-tested, and was reachable only from its own test file —extension.tsimported justisModelConfiguredfrom the module. It is now wired, fedisModelConfigured() || hasProviderEnvVar()andhasOnboardingCompleted(onboardingDir()).3. The routing was silent. Every branch was side-effect-only, so a first run that lands on an empty chat left nothing behind to explain it — which is what made this expensive to diagnose. Inputs and the chosen action now go to the opencode channel, and a webview that fails to open is reported rather than swallowed by a floating
voidpromise.Also removed
OnboardingLauncherand thewelcome_shownread/write helpers. Nothing read or wrote them,resolveOnboardingActionnever consulted thewelcomeShownflag, andextension.tsowns launching throughserverManager.onReady. Their tests went with them; three cases were added covering the newly-wired inputs.Verification
pnpm run typecheck— cleantest/onboarding_routing.test.ts— 18 passedtest/ops/skill_freshness_orchestrator.test.tsand identical on the base with these changes stashed. Pre-existing, untouched here.Not changed, deliberately
Stage 0 writes
"github-copilot": {}for OAuth providers, andisModelConfigured()counts any provider key, so that credential-free stub satisfies the model gate on every later boot — the same shape as the Bedrock placeholder retired in #602. I did not tighten the check:{}is legitimate for an OAuth provider once the user has signed in, and making the gate stricter risks pushing users who have working configs back into Stage 0. Worth a decision of its own.