pam/adapter: rewrite nativeClient as a sequential function#1453
Draft
pam/adapter: rewrite nativeClient as a sequential function#1453
Conversation
The previous nativeModel used bubbletea's reactive Update/Cmd pattern to drive a fundamentally sequential flow: prompt the user, call an RPC, inspect the result, repeat. This mismatch caused significant accidental complexity: - A 'busy' boolean flag to reject duplicate events that bubbletea could deliver while a blocking operation was in progress. - A stage-mismatch retry loop (tea.Sequence(ChangeStage, repeatEvent)) because stage changes are asynchronous. - startAsyncOp() wrapping every blocking call in a goroutine just to work around the single-threaded Update model. - Internal event types (nativeStageChangeRequest, nativeUserSelection, nativeBrokerSelection, nativeChallengeRequested, nativeGoBack, ...) that existed only to shuttle control between Update calls. Replace nativeModel with nativeClient, a plain Go struct whose run() method executes the entire flow synchronously in one goroutine. It integrates with bubbletea at exactly two points: - Init() returns a tea.Cmd that starts run() in a goroutine. - run() returns a PamReturnStatus, which the main Update loop sees as the final event and uses to quit. All RPCs are now plain blocking calls instead of tea.Cmds wrapped in goroutines. All the internal event types, the busy flag, and the startAsyncOp() helper are gone. Error handling is straightforward Go: return an error, check at the call site. Net change: -716 / +599 lines for equivalent functionality.
Contributor
|
While this was my idea too at the beginning I decided that it was still better to follow the bubbletea model in the native case for the main reason that it acted only as the UI, while all the logic was kept the same (thus, in the actual main model). So if we want to take this road, I'd first extract and factorize all the logic into places that can be used in both the bubbletea TUI and this, and only then re-implement this. Even though the module should just be a simple UI for the server, the logic that we have it's not as simple at this point, and thus the main goal was to ensure that all the logic was done in a single place and shared between all the three UIs. |
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.
The previous
nativeModelusedbubbletea's reactiveUpdate/Cmdpattern to drive a fundamentally sequential flow: prompt the user, call an RPC, inspect the result, repeat. This mismatch caused significant accidental complexity:busyboolean flag to reject duplicate events thatbubbleteacould deliver while a blocking operation was in progress.tea.Sequence(ChangeStage, repeatEvent)) because stage changes are asynchronous.startAsyncOp()wrapping every blocking call in a goroutine just to work around the single-threadedUpdatemodel.nativeStageChangeRequest,nativeUserSelection,nativeBrokerSelection,nativeChallengeRequested,nativeGoBack, ...) that existed only to shuttle control betweenUpdatecalls.Replace
nativeModelwithnativeClient, a plain Go struct whoserun()method executes the entire flow synchronously in one goroutine. It integrates withbubbleteaat exactly two points:Init()returns atea.Cmdthat startsrun()in a goroutine.run()returns aPamReturnStatus, which the mainUpdateloop sees as the final event and uses to quit.All RPCs are now plain blocking calls instead of
tea.Cmds wrapped in goroutines. All the internal event types, thebusyflag, and thestartAsyncOp()helper are gone. Error handling is straightforward Go: return an error, check at the call site.Net change:
-716 / +599lines for equivalent functionality.