Conversation
properly structure UI updates
Contributor
|
This is gonna be such a fun review tmr 😈 |
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.
Premise
The core architectural principle: the rendered screen is a pure function of session state. Nobody imperatively pushes screens around. Business logic sets state through store setters, and the router derives which screen should be active. If a screen appears at the wrong time, the bug is in a predicate – not in a scattered clack call somewhere.
Session: source of truth
Everything the wizard needs to render lives in one typed object:
WizardSession. It gets populated in layers – CLI args fill what they can, auto-detection fills more, TUI screens collect the rest, OAuth provides credentials. By the time the agent starts, the session is populated.Business logic reads session fields and writes them through the
WizardUIinterface (getUI().setCredentials(),getUI().startRun(), etc.), which bridges to store setters.Session fields that affect screen resolution –
cloudRegion,runPhase,credentials,frameworkConfig,outroData,serviceStatus,loginUrl,detectedFrameworkLabel,frameworkContext— are only mutated through explicit store setters likestore.setRunPhase(),store.setCredentials(), etc. Direct session mutation outside the store is a bug.Router: reactive resolution plus overlays
The
WizardRouterowns two things: flow definitions (declarative pipelines) and overlay interrupts (outage, auth-expired, etc.).Flows are arrays of entries, each with a screen and an
isCompletepredicate:The router's
resolve(session)method walks the array, skipping entries whereshow()returns false orisComplete()returns true, and returns the first incomplete screen. There is no cursor. There is noadvance(). The active screen is resolved fresh on every React render cycle from the current session state. Bugs live in the completion predicate, not buried in some surprising crevice.Overlays
Overlays are a separate stack for interrupts.
store.pushOverlay(Overlay.Outage)puts the outage screen on top of whatever flow screen is active.store.popOverlay()dismisses it and the flow resumes. Overlays don't break the active flow.The visible screen is: top of overlay stack if non-empty, otherwise the resolved flow screen.
Store: setters + change notification
WizardStoreis an EventEmitter that React components subscribe to viauseSyncExternalStore. It holds the router, the session, and observable agent state (tasks, status messages).Every session mutation that affects screen resolution goes through an explicit setter:
setCloudRegion(),setRunPhase(),setCredentials(),setFrameworkConfig(),setDetectedFramework(),setLoginUrl(),setServiceStatus(),setOutroData(),setFrameworkContext(). Each setter mutates the session field and callsemitChange(), which increments the version counter and triggers React re-renders. On the next render,store.currentScreencallsrouter.resolve(session)and returns the correct screen.Screens never call navigation methods. They set the session fields they own through store setters, and the router calculates the rest.
Screen registry + flow
Screens are registered in
screen-registry.tsx, a factory function that maps screen names to React components and wires up services. App.tsx is a thin shell that calls the factory. Adding a new screen means:screens/screen-registry.tsxFlowEntrywith anisCompletepredicate to the flow inrouter.tsApp.tsx never changes. If a new screen needs new reactive session state, add a setter to the store.
Screen names, overlay names, flow names, run phases, and outro kinds are all TypeScript enums — no string comparisons anywhere in the navigation or session layer.
Is there a skill file for this?
Come on.
Thoughts?
What do you think? It's working pretty well and I like how tidy changes are, but it's a big shift, so lmk how this lands.