fix(tui): patch nanostores map.setKey to mutate $session in place#762
Merged
Conversation
PR #760 added the pendingEnvSelection flag and gated every post-Auth show: predicate on it, but the live bug still reproduced. Root cause: bin.ts holds a session ref that it mutates in-place during checkpoint hydration, resolveCredentials, and applyEnvSelectionDeferral. The WizardStore exposes that same ref via store.session and re-syncs it at the end of the credential block via tui.store.session = session. Nanostores' map.setKey reassigns $session.value to a fresh { ...prev, [key]: value } object on every key change. The first in-store setter (typically concludeIntro fired when the user picks Resume on the checkpoint prompt while resolveCredentials is still awaiting) detaches $session.value from the bin.ts session ref. After that point bin.ts's later in-place mutations — including the deferral that sets pendingEnvSelection = true and clears credentials — never reach the store. The final tui.store.session = session then OVERWRITES the store's accumulated state with the stale bin.ts ref, wiping the user's introConcluded = true progress and producing either a bump back to Intro or a stall with no env-picker surface (the bug PR #760 was supposed to fix). Patching setKey to mutate $session.value in place keeps it and the bin.ts session ref pointing at the same object for the entire run, so external mutations propagate transparently and store setters don't clobber them. notify() still fires per key for the React useSyncExternalStore bridge (which reads $version); listenKeys subscribers would see the new value via both args of their callback, but the wizard's UI doesn't use listenKeys anywhere. Adds two regression tests in flow-invariants.test.ts that pin both halves of the regression: ref stability across in-store setters AND propagation of in-place external mutations after a setter has fired. Both tests fail on origin/main and pass with this change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
8 tasks
kelsonpw
added a commit
that referenced
this pull request
May 14, 2026
…n when pendingEnvSelection gets clobbered (#775) Three prior PRs claimed to fix the "stuck on Setup with no env-picker" bug — #747 cleared session.credentials when resolveCredentials returned needs_user_choice/environment_selection, #760 added the pendingEnvSelection flag and gated every post-Auth show: predicate on it, and #762 patched nanostores' map.setKey to mutate in-place so the bin.ts session reference stays aligned with $session.value. Each ship passed its unit tests. The bug kept reproducing in user-reported runs (pnpm try:prod --install-dir=~/excalidraw → successful run → git reset --hard → rerun → wizard navigates the TUI, stops at "✓ Welcome ─ ✓ Auth ─ ● Setup ←", and hangs). Per-run logs end at "[intro] no framework matched — falling back to Generic" and nothing fires after. Static analysis of every write to pendingEnvSelection across src/ found exactly one direct set-true path (applyEnvSelectionDeferral) and four explicit set-false paths (all inside AuthScreen, gated on the user picking an env). None of them explain how the flag flips back to false before the user has interacted with the picker — but the journey stepper's "● Setup ←" state requires both Auth.isComplete=true AND pendingEnvSelection=false, so something is clobbering the flag through a path we don't yet understand. Rather than ship a fourth speculative root-cause fix, this commit adds a structural fallback gate to Auth.isComplete: when pendingOrgs is populated AND the resolved project has ≥ 2 environments with API keys AND selectedEnvName is null, Auth.isComplete returns false regardless of pendingEnvSelection's value. That predicate reads observable session shape — pendingOrgs is set by resolveCredentials in the deferred path, selectedEnvName is cleared by applyEnvSelectionDeferral — so there's no mutable flag for a future regression to clobber. The flag stays in place as the primary gate; the structural check is a second, redundant safety net. Scoped carve-outs: - pendingOrgs===null short-circuits to false so the manual-API-key path (apiKeyNotice resolver outcome) passes through unchanged. - Single-env projects don't trigger the gate (no picker needed). - Stale checkpoint selectedProjectId not in pendingOrgs short-circuits to false; the existing stale-org useEffect in AuthScreen handles that case. Regression coverage in src/ui/tui/__tests__/env-picker-repro.test.ts includes the smoking-gun scenario (multi-env + selectedEnvName=null + pendingEnvSelection=false → must still land on Auth) plus four carve-out cases (env picked, manual key, single-env, stale checkpoint) to guard the fix's scope. All six fail without the change and pass with it. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 14, 2026
kelsonpw
added a commit
that referenced
this pull request
May 15, 2026
…stale selectedOrgId/ProjectId (#797) Prior PRs #747 / #760 / #762 / #775 / #778 / #780 each tried to fix the recurring "stuck on Setup with no env-picker" hang after `git reset --hard` on a previously-instrumented project. Each landed a gate that worked at the unit-test level, but the live repro kept reproducing. This pins the actual gap PR #780 left behind. PR #780's structural fallback (`needsEnvPickStillRequired` → `pendingOrgs[0].projects[0]`) only fires when BOTH `selectedOrgId` AND `selectedProjectId` are null. In the real restart-after-reset sequence: 1. `resolveCredentials` populates `pendingOrgs` (first fetch) and defers on multi-env. 2. `applyEnvSelectionDeferral` sets `pendingEnvSelection=true`. 3. AuthScreen mounts and its single-org/single-project auto-resolve effect writes `selectedOrgId='org-1'`, `selectedProjectId='proj-1'` from that first snapshot. 4. `authTask` runs OAuth, `fetchAmplitudeUser` fetches a SECOND time, and `setOAuthComplete` REPLACES `pendingOrgs` with the fresh data. If the two snapshots' IDs don't match (re-fetch race, ordering, account changes, stale in-memory IDs), the existing `pendingOrgs.find(o => o.id === selectedOrgId)` lookup returns `undefined`. 5. The structural gate short-circuits to `false`. If anything clobbers `pendingEnvSelection=false` (the recurring failure-mode the 6 prior PRs chased — every fix relied on the flag staying true), Auth.isComplete returns true and the router walks past Auth into the Setup-bucket screens. User sees `✓ Auth ─ ● Setup ←` with no env picker on screen. Fix: when stale `selectedOrgId/ProjectId` don't resolve a project in `pendingOrgs`, fall through to the same `pendingOrgs[0].projects[0]` heuristic instead of bailing to `false`. The structural gate is now load-bearing across all three states: (a) no IDs picked yet (PR #780) (b) IDs picked and valid in pendingOrgs (existing happy path) (c) IDs picked but stale relative to fresh pendingOrgs (this PR) Regression test: src/ui/tui/__tests__/env-picker-restart-after-reset-hang.test.ts drives the complete bin.ts startup sequence — buildSession → store.session → resolveCredentials multi-env defer → applyEnvSelectionDeferral → concludeIntro → setOAuthComplete → AuthScreen's setOrgAndProject — and asserts the router parks on Auth at every step. The new "stale IDs" test fails without the fix (router resolves to data-setup) and passes with it. Pre-existing test in env-picker-repro.test.ts that asserted the OPPOSITE behavior ("does not block when the resolved project is missing from pendingOrgs") was inverted to match the new (correct) behavior — that test was inadvertently encoding the bug as expected behavior. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 16, 2026
Merged
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
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.
Summary
PR #760 added the
pendingEnvSelectionflag and gated every post-Authshow:predicate on it, but the live bug still reproduced. Real root cause is a stale-reference bug between bin.ts and the WizardStore:sessionref that it mutates in-place during checkpoint hydration,resolveCredentials, andapplyEnvSelectionDeferral, then callstui.store.session = sessionto re-emit.$session = map<WizardSession>(...). Nanostores'map.setKeyreassigns$session.valueto a fresh{ ...prev, [key]: value }object on every key change.concludeIntrofired when the user clicks Resume on the checkpoint prompt whileresolveCredentialsis still awaiting) detaches$session.valuefrom the bin.tssessionref.pendingEnvSelection = trueand clears credentials — never reach the store.tui.store.session = sessionthen OVERWRITES the store's accumulated state with the stale bin.ts ref, wiping the user'sintroConcluded = trueprogress and producing either a bump back to Intro or a stall with no env-picker surface.PR #760's flag write site (
session.pendingEnvSelection = true) is correct in isolation — it just lands on a session object the store already stopped reading from.The fix
Patch
$session.setKeyto mutate$session.valuein place instead of allocating a fresh spread.notify()still fires per key, so the ReactuseSyncExternalStorebridge (which reads$version) is unaffected. The wizard's UI doesn't uselistenKeysanywhere, so the per-listeneroldValuearg semantics change is non-observable.With the patch,
$session.valueand the bin.tssessionref point at the same object for the entire run. External mutations propagate transparently; store setters don't clobber them.Regression test
Added two tests in
src/ui/tui/__tests__/flow-invariants.test.ts:preserves the bin.ts session reference after an in-store setKey— pins ref stability acrossconcludeIntro.propagates in-place external mutations after an in-store setKey, including pendingEnvSelection— replays the exact bin.ts sequence: build session → startTUI → concludeIntro → in-place deferral mutations → finalstore.session = session. AssertscurrentScreen === Auth(env picker surface) and thatintroConcludedsurvives the resync.Both tests fail on
origin/main(verified by stashing the store change) and pass with this commit.Test plan
pnpm exec tsc --noEmitcleanpnpm exec eslint .clean (one pre-existing warning unrelated to this change)pnpm buildsucceeds (smoke test passes)pnpm exec vitest run --pool=forks --maxWorkers=1 --testTimeout=30000— 3955/3955 passingorigin/mainwithout the store change🤖 Generated with Claude Code
Note
Medium Risk
Changes core TUI session state update semantics by overriding nanostores
map.setKey, which could affect any code relying on object identity orlistenKeysold/new value behavior; however the change is scoped to the Wizard session store and covered by new regression tests.Overview
Fixes a TUI race where
WizardStoresession updates could detach from thebin.tssession reference, causing in-place mutations (notablypendingEnvSelection) to be missed and laterstore.session = sessionre-emits to clobber progress.WizardStorenow builds$sessionvia a customcreateSessionStore()that patches nanostoresmap.setKeyto mutate the existing session object in-place while still callingnotify().Adds regression tests in
flow-invariants.test.tsto assert (1) session reference stability acrossconcludeIntro()and (2) propagation of external in-place mutations plus preservation ofintroConcludedand correct routing back toScreen.Auth.Reviewed by Cursor Bugbot for commit d8f31a7. Bugbot is set up for automated code reviews on this repo. Configure here.