Skip to content

pam/adapter: rewrite nativeClient as a sequential function#1453

Draft
adombeck wants to merge 1 commit intomainfrom
pam-native-model-rewrite
Draft

pam/adapter: rewrite nativeClient as a sequential function#1453
adombeck wants to merge 1 commit intomainfrom
pam-native-model-rewrite

Conversation

@adombeck
Copy link
Copy Markdown
Contributor

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.

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.
@3v1n0
Copy link
Copy Markdown
Contributor

3v1n0 commented Apr 17, 2026

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants