Skip to content

refactor(app): type event fan-out with sealed Event union (#101) - #103

Merged
ezynda3 merged 2 commits into
masterfrom
feat/101-typed-event-union
Jul 30, 2026
Merged

refactor(app): type event fan-out with sealed Event union (#101)#103
ezynda3 merged 2 commits into
masterfrom
feat/101-typed-event-union

Conversation

@ezynda3

@ezynda3 ezynda3 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Step (a) of the #101 decoupling: give the app layer a typed vocabulary for the events it emits toward a display, so internal/app no longer has to speak Bubble Tea's untyped tea.Msg to describe its own events.

This is behaviour-preserving and the second of the sequenced PRs for #101 (after #102). It does not close the issue.

What changed

  • New sealed app.Event union (internal/app/events.go): an interface with an unexported isAppEvent() marker, implemented by every event type in the file. The set is closed — only app-owned types can satisfy it, and a new event type is a compile error until it is added to the marker block.
  • Typed fan-out: retyped the app-internal event paths from func(tea.Msg) to func(Event)RunOnceWithDisplay/RunOnceWithDisplayAndFiles, executeStep, executeBatch, runQueueBatch, subscribeSDKEvents, handleTurnEnd, recordStepUsage, and the sendEvent helper. Only the concrete transport (prog.Send) adapts Event at the boundary now.
  • CLI handler (internal/ui/event_handler.go): Handle now takes app.Event, which lets it drop its charm.land/bubbletea/v2 import entirely.
  • Renamed the UI-transport escape hatch: AppController.SendEvent(tea.Msg)SendUIMessage(tea.Msg) (and the App method to match). This method is not an app event — it re-injects UI-internal messages (async goroutine results like beforeForkResultMsg, extensionCmdResultMsg) into the Bubble Tea Update loop and is opaque to the app layer. Renaming disambiguates it from the typed Event fan-out.

Why

SendEvent(tea.Msg) and the func(tea.Msg) fan-out meant internal/app depended on the TUI's message type to model its own events, and any value could flow through those paths unchecked. A sealed Event union makes the event surface transport-agnostic (a future non-Bubble Tea transport only adapts at prog.Send), self-documenting, and type-checked.

Scope / deferred

  • Response-channel events (PasswordPromptEvent, PromptRequestEvent, OverlayRequestEvent, NewSessionRequestEvent) keep their channels. Converting them to a request-id correlation scheme (and error fields to strings) only matters for a real out-of-process boundary, which does not exist yet — deferred to avoid speculative churn. Noted in the Event doc comment.
  • The *tea.Program field and SendUIMessage's tea.Msg transport remain: fully removing Bubble Tea from internal/app is out of scope for this step.

Testing

  • go build ./..., go vet ./... — clean (the pre-existing examples/extensions "function main is undeclared" error is unrelated and present on master).
  • go test -race ./... — all pass.
  • gofmt -l clean; golangci-lint run — 0 issues.
  • Manual tmux smoke test: TUI launches, /help renders, empty stderr — event round-trip through real Bubble Tea confirmed.

Refs #101

Summary by CodeRabbit

  • New Features
    • Added a sealed, app-layer event system for consistent event handling across the app and UI.
    • Introduced a dedicated UI message channel for delivering asynchronous UI updates safely.
  • Improvements
    • Updated non-interactive “display events” and UI event handling to use the new app event type end-to-end.
    • Kept existing warning, streaming, spinner, and async result behaviors the same.
  • API Changes
    • Replaced the generic event-sending method with SendUIMessage for UI-only message reinjection.

The app layer described its own events with Bubble Tea's untyped tea.Msg,
forcing internal/app to speak the TUI's message vocabulary and letting any
value flow through the event-producing paths unchecked.

Introduce a sealed app.Event interface (unexported isAppEvent marker)
implemented by every event type in events.go, and retype all app-internal
fan-out — RunOnceWithDisplay(AndFiles), executeStep, executeBatch,
runQueueBatch, subscribeSDKEvents, handleTurnEnd, recordStepUsage and the
sendEvent helper — from func(tea.Msg) to func(Event). The CLI handler's
Handle now takes app.Event, dropping its bubbletea import entirely. Only
the concrete transport (prog.Send) adapts Event at the boundary, so the
event-producing code is now transport-agnostic.

Rename the UI-transport escape hatch AppController.SendEvent(tea.Msg) to
SendUIMessage(tea.Msg) so it is clearly distinct from app events: it
re-injects UI-internal messages (async goroutine results) into the Update
loop and stays opaque to the app layer, which never interprets it.

Response-channel events (password/prompt/overlay/new-session) keep their
channels for now; a request-id correlation scheme for a real out-of-process
transport is deferred until such a transport exists.

Refs #101
@mark-iii-labs-huly

Copy link
Copy Markdown

Connected to Huly®: KIT-104

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The app now uses a sealed Event type for execution and SDK event delivery, while Bubble Tea messages use the separate SendUIMessage API for asynchronous UI reinjection.

Changes

Event and UI transport separation

Layer / File(s) Summary
Sealed app event contract
internal/app/events.go
Defines the sealed Event interface and registers all concrete app events with the marker method.
App event execution pipeline
internal/app/app.go, internal/app/app_test.go
Updates execution callbacks, SDK subscriptions, turn handling, usage recording, and display methods to use Event, with corresponding test updates.
UI message delivery boundary
internal/app/app.go, internal/ui/event_handler.go, internal/ui/model.go, internal/ui/model_test.go
Replaces SendEvent with SendUIMessage, routes asynchronous UI results through it, and updates event handling to accept app.Event.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: refactoring app event fan-out to a sealed Event union.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/101-typed-event-union

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ezynda3

ezynda3 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@CodeRabbit review

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/app/events.go`:
- Around line 5-24: Revise the Event interface documentation to remove the claim
that type switches over Event are compiler-checked for exhaustiveness. Keep the
sealed-union and app-owned-type explanation, but accurately state that Go’s
unexported marker prevents external implementations without implying unhandled
variants produce compile errors.

In `@internal/ui/model.go`:
- Line 3943: Update the concurrency rationale comment immediately above the
extension command result goroutine to reference SendUIMessage instead of the
stale SendEvent API, matching the call in the goroutine.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: bbac234d-1115-417b-8687-53f08c07becf

📥 Commits

Reviewing files that changed from the base of the PR and between de50e9c and 79226e9.

📒 Files selected for processing (6)
  • internal/app/app.go
  • internal/app/app_test.go
  • internal/app/events.go
  • internal/ui/event_handler.go
  • internal/ui/model.go
  • internal/ui/model_test.go

Comment thread internal/app/events.go
Comment thread internal/ui/model.go
- events.go: drop the inaccurate claim that Go compiler-checks type-switch
  exhaustiveness over the sealed Event interface; state that the unexported
  marker only closes the union to external implementations and that
  downstream switches should keep a default case.
- model.go / model_test.go: update stale SendEvent references in the
  extension-command goroutine rationale and the session-switch test comment
  to the renamed SendUIMessage.

Refs #101
@ezynda3

ezynda3 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@CodeRabbit review

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@ezynda3
ezynda3 merged commit b03a41b into master Jul 30, 2026
3 checks passed
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.

1 participant