refactor(cli): fix bugs, remove dead code, and simplify logic#24
refactor(cli): fix bugs, remove dead code, and simplify logic#24
Conversation
- Use errors.Is for ErrRefreshTokenExpired instead of == to handle wrapped errors - Fix ShowUserInfo being a no-op in BubbleTea TUI mode - Remove unused pollForTokenWithProgress function and update tests to use pollForTokenWithUpdates - Remove unused tui/components package (ErrorView, HelpView, InfoBox, ProgressBar, StepIndicator, Timer) - Remove dead tea.Model interface methods from UnifiedFlowModel - Eliminate redundant token expiry re-check in run() by setting flow inline - Remove redundant getConfig call by storing TokenStoreMode in AppConfig - Remove unnecessary 500ms time.Sleep in non-interactive error display Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR refactors the CLI/TUI flow implementation by fixing a couple of correctness bugs (notably wrapped-error comparisons and a silent no-op UserInfo display), removing now-dead Bubble Tea component code, and simplifying token/config handling to reduce redundancy.
Changes:
- Fix refresh-token-expired detection by switching from
==toerrors.Isfor wrapped errors (main.go,auth.go). - Make TUI
ShowUserInfoactually render a visible “OIDC UserInfo” step instead of being a silent no-op (tui/bubbletea_manager.go). - Remove dead code paths (old polling function,
tui/components/*, Bubble Tea model methods) and simplify config/error-display behavior (device_flow.go,tui/*,config.go,go.mod/go.sum).
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tui/unified_flow_view.go | Removes Bubble Tea model/view code; leaves state model used by FlowRenderer. |
| tui/simple_manager.go | Removes non-interactive time.Sleep delay in error display. |
| tui/components/timer.go | Deleted unused TUI component. |
| tui/components/step_indicator.go | Deleted unused TUI component. |
| tui/components/progress_bar.go | Deleted unused TUI component and related Bubble Tea/Bubbles dependency usage. |
| tui/components/info_box.go | Deleted unused TUI component. |
| tui/components/help_view.go | Deleted unused TUI help component. |
| tui/components/error_view.go | Deleted unused rich error view component. |
| tui/components/components_test.go | Deletes tests for removed tui/components package. |
| tui/bubbletea_manager.go | Fixes ShowUserInfo behavior and ensures renderer-nil path returns early. |
| polling_test.go | Updates tests to use pollForTokenWithUpdates and adds update-draining helper. |
| main.go | Simplifies flow selection logic and uses errors.Is for refresh-token-expired handling. |
| go.sum | Removes checksums for removed Bubble Tea/Bubbles-related dependencies. |
| go.mod | Drops unused Bubble Tea/Bubbles/harmonica dependencies after refactor. |
| device_flow.go | Removes pollForTokenWithProgress and relies on update-driven polling function. |
| config.go | Adds TokenStoreMode to config and removes redundant token-store config read. |
| auth.go | Uses errors.Is to detect refresh-token-expired through wrapping. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Close the updates channel via t.Cleanup so the background goroutine exits when the test finishes instead of leaking until process exit. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Re-check terminal size in FlowRenderer.UpdateDisplay() so layout adapts to live resizes after removing the Bubble Tea WindowSizeMsg handler - Make addStep idempotent to prevent duplicate UI steps on repeated calls Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…t helper - Move getTerminalSize() call after spinner-only fast path so it only runs on content-dirty redraws, avoiding ~10 ioctl syscalls/sec - Replace channel-close with done channel in drainUpdates to avoid closing a producer-owned channel Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ntly Extract checkResize() helper that detects size changes and promotes to a full redraw. On resize, re-render the header so its width stays in sync with the rest of the content. The check now runs on every UpdateDisplay call, including spinner-only updates. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Throttle getTerminalSize() to at most once per 500ms to avoid unnecessary ioctl syscalls on the 100ms spinner tick - Track actual header line count from RenderHeader output instead of using a hardcoded formula, so cursor placement stays correct after terminal resizes that change line wrapping Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
errors.IsforErrRefreshTokenExpiredinstead of==to correctly handle wrapped errors (inmain.goandauth.go)BubbleTeaManager.ShowUserInfobeing a silent no-op in TUI mode — now renders an "OIDC UserInfo" steppollForTokenWithProgressfunction, entiretui/components/package (7 files), and deadtea.Modelinterface methods fromUnifiedFlowModeltime.Sleepin non-interactive error displayNet result: +55 / -870 lines
Test plan
go build ./...compiles successfullygo test ./...all tests passmake lintpasses with 0 issuesgo mod tidyremoves unusedcharm.land/bubbles/v2,charm.land/bubbletea/v2, andcharmbracelet/harmonicadependencies🤖 Generated with Claude Code