Repository navigation
refactor: give each program a tui entry and end runs through the store - #1382
Conversation
🧙 Wizard CIRun the Wizard CI and test your changes against wizard-workbench example apps by replying with a GitHub comment using one of the following commands: Test all apps:
Test all apps in a directory:
Test an individual app:
Show more apps
Test against a Context Mill branch:
Add Results will be posted here when complete. |
3521f0c to
e5138a9
Compare
4c97205 to
cc1f5dd
Compare
e5138a9 to
fd7ec0e
Compare
cc1f5dd to
e321c28
Compare
e321c28 to
9ce4be1
Compare
fd7ec0e to
2b4fdc6
Compare
9ce4be1 to
38c058c
Compare
2b4fdc6 to
62de363
Compare
johncwaters
left a comment
johncwaters
left a comment
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
src/tui/tools/doctor/screens/DoctorIntroScreen.tsx:48: Cancel still callsprocess.exit(0), which the description says every screen stopped doing. Usestore.requestExit(0), unless a later PR in the stack handles it.
| // After a mint failure run-wizard owns the exit (status 1, analytics). | ||
| if (!store?.session.mintHandoff) process.exit(0); | ||
| // After a mint failure the host owns the end (status 1, analytics). | ||
| if (!store.mintHandoff) store.requestExit(0); |
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
At this head no real host (start-tui.ts, run-wizard.ts, the slack and mcp commands) subscribes to store.exitRequest, and nothing calls store.launch(). This exit and the other converted ones (AuthError, PortConflict, ManagedSettings, intro Cancels, KeepSkills, the health-check outage abort) would leave the TUI hanging, and screens reading store.mcpFeatures, store.programLabel or integrate would see null. Fine if a later PR in the stack wires the hosts before the phase lands. Which PR does that?
There was a problem hiding this comment.
Expected at this PR: the hosts that subscribe to exitRequest and call store.launch() land in #1383 (src/tui/run.ts, src/tui/run-tool.ts).
| unmount(); | ||
| releaseTerminal(); | ||
| process.exit(0); | ||
| return new Promise((resolve) => { |
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
This dropped its process.exit(0) and now returns a promise, but runPlayground in src/cli/commands/basic-integration/playground.ts still ignores it. Any handle left open keeps wizard playground from returning to the shell. Could runPlayground await it and set process.exitCode?
There was a problem hiding this comment.
Handled in #1385: src/cli/commands/basic-integration/playground.ts runs it through exitWith, which exits with the resolved code, so open handles can't hold the shell.
38c058c to
fabac48
Compare
62de363 to
0ecdbe0
Compare
fabac48 to
f8b95db
Compare
0ecdbe0 to
7597ed2
Compare
Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
7597ed2 to
581e05a
Compare
f8b95db to
11b9caa
Compare
edwinyjlim
left a comment
edwinyjlim
left a comment
There was a problem hiding this comment.
Review of #1382 at 11b9caab. Comments inline.
Drafted with Claude Code from a review of the whole Release C stack, re-verified at the stack tip 2e31e438. Line numbers are at this PR's head.
| ? () => void handleDownloadAndExit() | ||
| : () => | ||
| void wizardAbort({ | ||
| void abortOnScreens(store, { |
There was a problem hiding this comment.
[P1] The health-check outage "Exit" never ends the run: the abort's outro cannot render, so abortOnScreens waits forever
The Exit handlers call abortOnScreens(store, { code: EnvServiceOutage }). The presenter sets the error outro and waits for outroDismissed, but the router walk stops at the health-check step because its readiness predicate stays false, and the router's only abort special case is the auth entry (src/tui/router.ts:82). So the screen stays health-check, the outro never renders, and nothing ends the exit. A probe at the stack tip (2e31e43) after the abort reads currentScreen = health-check, runPhase = error, exitRequest = null. The only way out is Ctrl-C, which reports 130 and "cancelled" instead of the outage error. Main had the same shape; this PR rewrites the handler and says screens now end runs through the store, but this path still can't. Same at lines 198 and 204.
Suggested fix: Let the walk reach the outro once an abort is recorded: in router.resolve generalize the special case to if (session.runPhase === RunPhase.Error && session.outroData) return ScreenId.Outro before return entry.id; auth and health-check both need it, and real run failures are already diverted earlier. Or have the Exit handler call store.requestExit(1) after analytics.shutdown('error') instead of the presenter. Add a screen-exits case for the outage Exit asserting exitRequest === 1.
Phase 2 of Release C (#1320), PR 6 of 16, after #1381. Review PR. PRs 1 to 14 of this phase split one change into reviewable pieces and compile only as a stack: review them one by one, then land the whole phase together.
Problem
Screens on main call
process.exitand read TUI answers fromstore.session, and a program's screens, deck and tips hang off itsProgramConfig. So the TUI core names every program, and a screen can end the process under its host.Changes
src/tui/programs/<id>/index.tsx: itsflow,screens,deckandtips(TuiPrograminprograms/types.ts).getTuiProgramandflowOwnerfind them, so the core names no program.src/tui/screen-ids.tsfor the core (its code moved in refactor: move every file into its layer #1375),programs/<id>/screen-ids.tsfor each program.process.exitin a screen becomesstore.requestExit(code)with the same code, for example inAuthErrorScreen,ExitScreen,ManagedSettingsScreen,PortConflictScreenandHealthCheckScreen.store.slackConnected,store.settingsConflictsandstore.mintHandoff.abortOnScreens(abort.ts) shows an abort on the outro screen,renderFamilyPicker(family-picker.tsx) brings the CLI's subcommand picker into the TUI,services/slack-app-card.ts(moved in refactor: move every file into its layer #1375) now holds the Slack card copy itself instead of importing it from the MCP prompts, andstartPlaygroundresolves an exit code.harness.tsxholds what the e2e harness takes from the TUI in its own process:createTuiStore(a store with no terminal),mountScreens(the real screens on a test terminal),readTuiStateandtuiScreenIds. The TUI entry loads it on first use in refactor: add the tui and headless hosts and let only the cli exit #1383.process.exitswaps and each program'sindex.tsx. Most other edits read a field from its new home.Test plan
pnpm typecheck0 errors,pnpm lint0 errors, 236 test files (3,515 tests) pass,pnpm buildok.Created with PostHog Desktop