fix(tui): flush analytics before a screen's exit request ends the run - #1417
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
🧙 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. |
…uest Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
gewenyu99
left a comment
gewenyu99
left a comment
There was a problem hiding this comment.
🥸 Reviewed by NotVincent, totally not Vincent. The real Vincent will review it separately. Probably slop, please disregard.
| if (code === null || handedOff || exitInProgress || signalled) return; | ||
| exitInProgress = true; | ||
| launch.signal.removeEventListener('abort', onAbort); | ||
| void (async () => { |
There was a problem hiding this comment.
suggested
Here's the potential issue: After Cancel, the intro stays on screen and keeps taking input until the analytics flush finishes. Nothing checks the exit request again.
analytics host blocked -> user picks Cancel -> menu still live -> user picks Continue -> run starts, then exit.end kills it mid-login
Suggested fix: Unmount the TUI before awaiting the shutdown and flush, the way the tool host does on its exit path.
There was a problem hiding this comment.
Hi, NotVincent here :disguise-emoji:, I will do the deeds
There was a problem hiding this comment.
It's done, NotVincent will tell Vincent to double check.
| const code = store.exitRequest; | ||
| if (code === null || handedOff || exitInProgress || signalled) return; | ||
| exitInProgress = true; | ||
| launch.signal.removeEventListener('abort', onAbort); |
There was a problem hiding this comment.
suggested
Here's the potential issue: Cancel used to exit at once. Now it waits on two analytics shutdowns with no time limit, and Ctrl-C is ignored for the whole wait.
network that drops PostHog requests -> user picks Cancel -> exit waits about 30 s or more -> Ctrl-C does nothing -> user kills the terminal
Suggested fix: Cap the shutdown and flush with a short timeout, like the 2 s cap on the stream shutdown.
There was a problem hiding this comment.
Hi, NotVincent here :disguise-emoji:, I will do the deeds
There was a problem hiding this comment.
It's done, NotVincent will tell Vincent to double check.
| const activeTui = tui; | ||
| const { store } = activeTui; | ||
| // A screen's exit request ends the run with no shutdown; start-tui's exit listener unmounts. | ||
| // A screen's exit request ends the run with no stream shutdown, once analytics report its end; start-tui's exit listener unmounts. |
There was a problem hiding this comment.
nit
Here's the potential issue: The TUI README still says runTui resolves a screen's exit request as is. The older test named for starting no end shutdown now runs one.
next reader adds an exit request on a hot path -> trusts the README -> doesn't expect a network wait before exit
Suggested fix: Update the README sentence to mention the report and flush, and rename the older test to say no stream shutdown.
There was a problem hiding this comment.
Hi, NotVincent here :disguise-emoji:, I will do the deeds
There was a problem hiding this comment.
It's done, NotVincent will tell Vincent to double check.
2 participants
Problem
When someone picks Cancel on the intro, or any other screen ends the run with
requestExit, the process exits before analytics flush, so the run sends onlywizard: started(12–20% of prod integrate runs since 2.75.0).Changes
runTuinow reports the end withanalytics.shutdownbefore it exits:cancelledfor code 0,errorotherwise. The first status still wins, so a run that already reported success keeps it.Test plan
run.test.tscases: an exit request before the run (codes 0 and 1) shuts analytics down with the right status and flushes before exiting. Both fail on main.run.test.ts21/21, plus full vitest, typecheck and lint.wizard: started. This branch also sendswizard: intro menu selectedandsetup wizard finished.Sources
process.exit(0). refactor: move the agent, programs, tui, hosts, tools and cli onto the session store #1378 moved it torequestExitwith no flush.🤖 Generated with Claude Code