feat(ui): restyle status prefixes per output mode; warnings as CI annotations - #287
Merged
Conversation
readline v1.5.x changed prompt teardown (stdin wrapping, ioloop/Close semantics) in ways promptui v0.9.0 was never adapted to: every interactive picker leaked readers parked on stdin/channel sends, which could wedge the process after the prompt returned - terminal stopped echoing, Ctrl+C was swallowed, the command never exited (seen on 'openframe cluster status' with interactive selection). Pin readline to the commit promptui declares and exclude the broken v1.5.0/v1.5.1 so 'go get -u' cannot silently re-upgrade onto them.
promptui is unmaintained (last release Oct 2021, master == v0.9.0) and its readline dependency is the root of the interactive-prompt teardown bugs the previous commit pinned around. Replace it with charmbracelet/huh, which is actively maintained and built on bubbletea (already a dependency for the app-status TUI). - shared/ui: SelectFromList keeps its (int, string, error) contract; new SelectOption covers short fixed menus; new PromptInput replaces promptui.Prompt (defaults pre-fill the field, validation on submit). - Long lists scroll with a 'type / to filter' hint; filtering is fuzzy. - Prompt aborts (Ctrl+C/Esc) surface as ui.ErrPromptInterrupted; the error handler matches it structurally (errors.Is) so %w-wrapped aborts still print the friendly 'cancelled by user' notice instead of an error panel. - Wizard steps, cluster/chart mode selects and the typed-name delete confirmation now go through the shared prompt helpers; promptui and chzyer/readline (with its version excludes) drop out of go.mod entirely. Verified under a PTY harness: picker select/arrows//-filter, Ctrl+C abort, and the full create wizard in --dry-run all complete and exit cleanly.
promptui is gone from the module graph; interactive prompts now run on charmbracelet/huh over bubbletea (which also powers the app status TUI).
…t height Interactive prompts now refuse to start when the session is non-interactive (CI env or non-TTY stdin), matching the RequireConfirmation contract. On Unix CI this was already the effective behavior - bubbletea cannot open /dev/tty there - but Windows runners DO have a console, so a prompt reached in tests blocked on ReadConsole until the 10-minute job timeout (TestWizardSteps_ConfirmConfiguration on windows-amd64). Also size long-list selects correctly: huh's Height includes the title and description rows, so selectPageSize+2 - not +3 - shows exactly ten option rows (CodeRabbit flagged the mismatch; its literal suggestion of +0 would have shown eight).
…otations One central hook (ApplyStatusPrefixTheme, wired into ApplyGlobalOutputFlags) re-themes the package-level pterm printers every call site prints through: - Interactive terminals drop the block badges for quiet glyph prefixes from the shared GlyphSet (dim bullet for info, triangle/cross/check for warning/error/success), inheriting its ASCII fallback. - Non-interactive output (CI, pipes, --plain) keeps word tags for grep-ability but lowercase, column-aligned and foreground-colored: info/warning/error/success/debug, no background blocks. - Inside GitHub Actions, warnings are teed into ::warning:: annotations so they surface on the PR; errors already got richer annotations from the shared error handler, which now also stops prepending a manual failure glyph the themed prefix carries. --silent still wins (theme runs after the flag handlers), NO_COLOR degrades to plain glyph/tag text via the existing color contract.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe change adds configurable status prefixes and GitHub Actions annotations, applies timestamp and theme settings during global output setup, updates generic error rendering, and changes several status and progress messages to avoid repeated labels and indentation. ChangesStatus output handling
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ApplyGlobalOutputFlags
participant ptermStatusPrinters
participant waitNote
participant annotationWriter
participant GitHubActions
CLI->>ApplyGlobalOutputFlags: apply verbose and output flags
ApplyGlobalOutputFlags->>ptermStatusPrinters: enable timestamps and apply status theme
waitNote->>ptermStatusPrinters: route formatted status message
ptermStatusPrinters->>annotationWriter: write warning output
annotationWriter->>GitHubActions: emit sanitized warning annotation
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
ivan-flamingo
previously approved these changes
Aug 5, 2026
oleg-flamingocx
enabled auto-merge (squash)
August 5, 2026 17:05
The Actions runner echoes every ::warning:: workflow command inline in the log, so the ArgoCD wait's recurring stuck-app summary produced a doubled, unstyled 'Warning:' line next to each themed one, every cycle - and GitHub keeps only 10 annotations per step, so repeats crowded out new warnings. The annotation tee now emits each distinct message once per process. Also drop the manual two-space indent from the wait loop's pending/stuck/ hint messages: it predates the aligned prefix tags, which now hold the column on their own - the indent just made rows ragged. Deliberately nested verbose detail (per-app drill-down) keeps its deeper indentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/shared/ui/status_theme.go`:
- Around line 71-73: Update the waitNote warning path to use the warning-aware
annotation writer configured by ApplyStatusPrefixTheme(), rather than dash.Note
or pterm.DefaultBasicText.Println(). Ensure ArgoCD wait warnings emit a
deduplicated WarningAnnotation once per distinct waiting issue while preserving
existing behavior for non-warning messages.
🪄 Autofix
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: 36a04794-67f0-43d3-8080-ff9408fb9fd1
📒 Files selected for processing (6)
internal/chart/providers/argocd/wait.gointernal/shared/errors/errors.gointernal/shared/ui/ghactions.gointernal/shared/ui/silent.gointernal/shared/ui/status_theme.gointernal/shared/ui/status_theme_test.go
The verbose ArgoCD wait intro printed its explanation as debug lines with a manual ' - ' indent, and several list sites (orphan GKE disks / EBS volumes, verbose cluster list, out-of-sync apps, unreachable-cluster cause) stamped a status tag on every item. With the aligned prefix tags the manual indents read as ragged columns, and a tag per list item is noise. Intro lines lose the dash indent (each timestamped debug line stands on its own); list items under a tagged header now go through DefaultBasicText — the repo's existing header+items pattern (see ShowCleanupSummary) — and the unreachable-cluster cause row mirrors the shared error panel's dim-key style. Deliberately nested verbose drill-downs keep their indentation.
waitNote took a pre-styled string and printed it via DefaultBasicText, so the ArgoCD wait's warnings (repo-server not recovering, apps stuck in Unknown, stall-sync failures) looked like warnings but bypassed pterm.Warning's writer — where the CI ::warning:: annotation tee lives — and never surfaced as PR annotations. waitNote now takes the status printer plus the raw message: the non-dashboard path prints through that printer (annotation tee included); the dashboard path still pins the styled line under the live area and never annotates, but it only runs on interactive terminals — CI always takes the printer path.
Two defects in the annotation tee's prefix stripping:
- Under NO_COLOR pterm's RawOutput mode renders the tag as 'warning: msg';
the strip list only knew the bare word, so a leading colon leaked into
the annotation text.
- Prefixes were stripped sequentially without a boundary, so after
removing 'warning' the loop would also eat a message-initial 'error'
('warning errors found' -> 's found'). No current call site hits this,
but it was latent.
Strip exactly one marker (break on first match) and drop RawOutput's
trailing colon after the matched prefix.
yaroslavmokflmg
previously approved these changes
Aug 5, 2026
--verbose exists to correlate CLI actions with cluster events, but only Debug lines carried the wall clock: info/warning rows in the same stream were clock-less and their message column sat 13 characters to the left of every debug row. All five status printers now share one timestamp writer (single line-start state), so a verbose run reads as one aligned timeline. The ArgoCD wait heartbeat keeps its own embedded [clock] ONLY when the writer is not stamping lines (plain CI logs, where it is the sole time reference) — under --verbose it drops it instead of printing two clocks on one row (ui.TimestampsActive). CI annotations are unaffected: the tee sees rendered text before the clock is prepended.
yaroslavmokflmg
self-requested a review
August 5, 2026 21:05
yaroslavmokflmg
approved these changes
Aug 5, 2026
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
3 participants
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.
Summary by CodeRabbit