Skip to content

feat: TUI v2 engine — theme system, rich dashboard, enhanced select#517

Draft
ndycode wants to merge 20 commits intoNoeFabris:devfrom
ndycode:feature/tui-v2
Draft

feat: TUI v2 engine — theme system, rich dashboard, enhanced select#517
ndycode wants to merge 20 commits intoNoeFabris:devfrom
ndycode:feature/tui-v2

Conversation

@ndycode
Copy link
Contributor

@ndycode ndycode commented Feb 28, 2026

Summary

Port the portable TUI engine from codex-multi-auth into opencode-antigravity-auth, adding a theme system, rich account dashboard with quota visualization, enhanced interactive select component, settings hub, action panel, and UI preference persistence.

What Changed

Phase 1 — TUI v2 Engine

New Files (8)

File Lines Description
src/plugin/ui/theme.ts 187 Theme system with ansi16/ansi256/truecolor color profiles, unicode/ascii glyph sets, palette & accent support
src/plugin/ui/runtime.ts 72 Global UI runtime options singleton with theme rebuilding on config change + initUiFromConfig
src/plugin/ui/format.ts 164 Format utilities: paintUiText, formatUiBadge, formatUiHeader/Section/Item/KeyValue, quotaToneFromLeftPercent
src/plugin/ui/copy.ts 79 Centralized UI strings for all menu text + settings labels + return flow prompts
src/plugin/ui/theme.test.ts Tests for theme creation across all profiles/palettes/accents
src/plugin/ui/format.test.ts Tests for all format utilities with v2 enabled/disabled
src/plugin/ui/runtime.test.ts Tests for runtime singleton set/get/reset lifecycle
src/plugin/ui/copy.test.ts Tests for UI_COPY structure and formatCheckFlaggedLabel

Modified Files (7)

File Change Description
src/plugin/ui/select.ts 316→491 Generic SelectOptions<T>, dynamicSubtitle, theme, focusStyle, selectedEmphasis, showHintsForUnselected, refreshIntervalMs, initialCursor, onCursorChange, onInput hotkey callbacks, Home/End keys, input guard, stdin drain
src/plugin/ui/ansi.ts 57→68 Added altScreenOn/Off, black, white, bg colors, Home/End key parsing, dual stdin+stdout TTY check
src/plugin/ui/confirm.ts 16→18 Passes runtime theme to select
src/plugin/ui/auth-menu.ts 143→688 Rich dashboard: search (/), quick-switch (1-9), quota bars (█▒), theme-aware badges, focus tracking, hotkeys (? help toggle, Q back), statusBadge(), formatQuotaSummary(), formatAccountHint(), extended AccountInfo with quota fields, AuthMenuOptions, new actions
src/plugin/ui/auth-menu.test.ts 101→265 Rewritten: 30 tests covering all new helpers (sanitize, quota, duration, status, accountTitle)
src/plugin/cli.ts 162→200+ Wires new actions: set-current-account, refresh-account, toggle-account, delete-account, settings, search; adds quota data passthrough, settings menu wiring, action panel for configure-models
package-lock.json Lockfile sync

Phase 2 — Enhancements

New Files (4)

File Lines Description
src/plugin/ui/settings-menu.ts 187 Interactive settings hub with sub-menus for color profile, glyph mode, palette, and accent
src/plugin/ui/settings-menu.test.ts 187 10 tests covering menu flows, persistence, same-value detection, multi-change sessions
src/plugin/ui/action-panel.ts 236 runActionPanel() with alt-screen spinner, console capture, auto-return countdown; waitForMenuReturn()
src/plugin/ui/action-panel.test.ts 184 8 tests covering TTY/non-TTY paths, console restore on error, cleanup verification

Modified Files (5)

File Change Description
src/plugin/config/schema.ts +25 Added optional ui section to AntigravityConfigSchema with color_profile, glyph_mode, palette, accent
src/plugin/config/loader.ts +26 Added saveUserConfig() for persisting UI preferences to antigravity.json
src/plugin/ui/copy.ts +13 Added settings section labels/hints and returnFlow prompt strings
src/plugin/ui/runtime.ts +15 Added initUiFromConfig() to bootstrap runtime options from saved config
src/plugin/cli.ts +39 Wired settings menu, action panel for configure-models, quota data passthrough from ExistingAccountInfoAccountInfo

Key Features

Phase 1:

  • Theme System — 3 color profiles (ansi16/ansi256/truecolor), 2 palettes (green/blue), 4 accents (green/cyan/blue/yellow), 2 glyph modes (ascii/unicode)
  • Quota Visualization█▒ bars with 5h/7d windows, cooldown timers, percentage display, theme-aware coloring
  • Search/ key opens search prompt, filters accounts by email/label/id/number
  • Quick-Switch — 1-9 number keys instantly set current account
  • Focus Tracking — cursor position preserved across menu re-renders
  • Enhanced Select — generic type support, hotkey callbacks, dynamic subtitle refresh, Home/End navigation
  • Centralized Copy — all UI strings in copy.ts for easy localization/modification

Phase 2:

  • Settings Hub — interactive menu to customize color profile, glyph mode, palette, and accent color
  • Action Panel — alt-screen spinner with console capture for long-running operations (e.g., configure-models)
  • Live Quota Passthrough — quota data flows from ExistingAccountInfo to AccountInfo for dashboard display
  • UI Preference Persistence — save/load theme choices to/from antigravity.json via Zod schema extension

What Was Skipped (Codex-specific, no backend)

  • forecast / fix / deep-check / verify-flagged actions
  • codex-manager patterns (5,687-line mixed-concern file)

Verification

  • npm run typecheck — zero errors
  • npm run build — clean compilation
  • npm test — 36 test files, 990 tests passed (88 new tests added)
  • No as any, @ts-ignore, or @ts-expect-error used
  • Backward compatible — existing promptLoginMode API unchanged
  • CodeRabbit CI check — SUCCESS

Stats

  • +2,898 lines added, 256 removed across 21 files
  • 12 new files, 9 modified files
  • 88 new tests across 7 test files
  • 2 commits on feature/tui-v2

…ed select

Port portable UI engine from codex-multi-auth TUI:
- Theme system (ansi16/ansi256/truecolor profiles, unicode/ascii glyphs)
- Format utilities (paintUiText, formatUiBadge, quotaToneFromLeftPercent)
- Runtime options singleton with theme rebuilding
- Centralized UI copy strings for all menu text
- Enhanced select with focus tracking, hotkeys, dynamic subtitle, refresh
- Rich auth-menu dashboard with search, quota bars, quick-switch 1-9
- Account details with set-current action and keyboard shortcuts
- cli.ts wiring for new action types (set-current, refresh, toggle, delete, settings, search)
- Colocated Vitest tests for all new modules (70 new tests)
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 28, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds a feature-complete terminal UI subsystem and settings persistence. New modules provide UI runtime options, theming, ANSI utilities, text formatting, localized UI copy, selectable menus, an action-panel runner, and a settings menu that persists choices via saveUserConfig. The auth-menu is expanded with richer AccountInfo (quota fields, IDs, labels), additional actions (set-current-account, refresh, toggle, delete, settings, search), and per-account detail views. CLI maps and propagates quota fields and uses the action panel for long-running tasks. Many unit tests were added for the new UI pieces.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.59% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: TUI v2 engine — theme system, rich dashboard, enhanced select' accurately summarizes the main changes: introduction of a TUI v2 engine with theme system, rich dashboard features, and enhanced select component.
Description check ✅ Passed The description comprehensively relates to the changeset, detailing Phase 1 and Phase 2 implementations, file-by-file modifications, key features, verification steps, and statistics—all aligned with the actual changes across 21 files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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 and usage tips.

…stence

- Add interactive settings menu with color profile, glyph mode, palette, and accent sub-menus
- Add action panel with alt-screen spinner, console capture, and auto-return countdown
- Wire quota data passthrough from ExistingAccountInfo to AccountInfo in CLI
- Add UI preferences section to config schema with persistence via saveUserConfig
- Add initUiFromConfig to bootstrap runtime options from saved config
- Add 18 tests covering settings menu flows and action panel TTY/non-TTY paths
@ndycode ndycode marked this pull request as ready for review February 28, 2026 18:27
@kilo-code-bot
Copy link
Contributor

kilo-code-bot bot commented Feb 28, 2026

Code Review Summary

Status: No New Issues Found | Recommendation: Merge

Overview

This is a follow-up review confirming no new issues were found in the PR diff. All previously flagged issues remain present in the modified code.

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 0

Previously Flagged Issues (Noted in Summary)

The following issues from earlier reviews were verified in this diff:

File Line Issue
src/plugin/ui/select.ts 67, 78 ANSI reset sequence removed - may cause color bleeding
src/plugin/ui/runtime.ts 71 Returns direct reference to internal state instead of clone
src/plugin/cli.ts 211 delete-account returns mode: "add" — mismatched routing
src/plugin/cli.ts N/A initUiFromConfig imported but never called
src/plugin/ui/settings-menu.ts 61, 107, 119, 147, 174, 203 Console.log between select calls; always returns true regardless of save status

Changes Reviewed (23 files)

  • package-lock.json - Version bump (1.3.3-beta.2 → 1.6.5-beta.0)
  • src/plugin.ts - Added runActionPanel and promptSignInMethod; changed constlet for useManualMode
  • src/plugin/ui/select.ts - Major rewrite with theme support (ANSI reset issue persists)
  • src/plugin/ui/settings-menu.ts - New file
  • src/plugin/ui/theme.ts - New file
  • src/plugin/ui/runtime.ts - New file
  • src/plugin/ui/action-panel.ts - New file
  • src/plugin/ui/copy.ts - New file
  • Plus 15 other modified files

Note

The change from const useManualMode to let useManualMode in plugin.ts line 2577 is intentional and correct - the variable is reassigned at line 3241 when user selects "manual" sign-in method.

Files Reviewed (23 files)
  • package-lock.json
  • src/plugin.ts
  • src/plugin/cli.ts
  • src/plugin/config/loader.ts
  • src/plugin/config/schema.ts
  • src/plugin/quota.ts
  • src/plugin/ui/action-panel.test.ts
  • src/plugin/ui/action-panel.ts
  • src/plugin/ui/ansi.ts
  • src/plugin/ui/auth-menu.test.ts
  • src/plugin/ui/auth-menu.ts
  • src/plugin/ui/confirm.ts
  • src/plugin/ui/copy.test.ts
  • src/plugin/ui/copy.ts
  • src/plugin/ui/format.test.ts
  • src/plugin/ui/format.ts
  • src/plugin/ui/runtime.test.ts
  • src/plugin/ui/runtime.ts
  • src/plugin/ui/select.ts
  • src/plugin/ui/settings-menu.test.ts
  • src/plugin/ui/settings-menu.ts
  • src/plugin/ui/theme.test.ts
  • src/plugin/ui/theme.ts

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/plugin/ui/auth-menu.test.ts (1)

4-103: ⚠️ Potential issue | 🟠 Major

These tests currently validate copied logic, not the production helper implementations.

Re-implementing helpers in the test file means regressions in src/plugin/ui/auth-menu.ts can slip through undetected while tests still pass.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/ui/auth-menu.test.ts` around lines 4 - 103, The tests duplicate
helper implementations (sanitizeTerminalText, formatRelativeTime, formatDate,
normalizeQuotaPercent, parseLeftPercentFromSummary, formatDurationCompact,
formatLimitCooldown, statusTone, statusText, accountTitle) causing false
positives; remove these in-test re-implementations and instead import the real
helpers from the production module (auth-menu.ts), or export the needed
functions from that module if they are not exported, then update the test file
to reference the imported symbols so tests exercise the actual production logic.
🧹 Nitpick comments (5)
src/plugin/config/loader.ts (1)

172-172: Use atomic write for config persistence safety.

Line 172 writes directly to the final file; interruption can leave a truncated JSON file. Prefer temp-file + rename for atomic replacement.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/config/loader.ts` at line 172, The current direct
writeFileSync(path, JSON.stringify(merged, null, 2) + "\n", "utf-8") can produce
truncated config files on interruption; change to an atomic write: serialize
merged to the content string, write that content to a temporary sibling file
(e.g., `${path}.tmp`) using a safe sequence (open -> write -> fsync -> close or
writeFileSync to temp then fsync/close), then rename the temp to the final path
with fs.renameSync so the replacement is atomic. Update the code around the
writeFileSync call in loader.ts (referencing path and merged) to perform the
temp-write + fsync + rename sequence and handle cleanup on errors.
src/plugin/cli.ts (1)

107-121: Consider extracting account mapping to reduce field-drift risk.

This manual copy block will keep growing as AccountInfo evolves. A single mapper helper will reduce maintenance misses.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/cli.ts` around lines 107 - 121, Extract the manual
property-by-property mapping into a dedicated mapper function (e.g.,
mapToAccountInfo or mapAccount) and use it inside the existingAccounts.map call
to produce accounts; the mapper should accept the original account object (the
type used in existingAccounts) and return an AccountInfo by
selecting/transforming fields (email, index, addedAt, lastUsed, status,
isCurrentAccount, enabled, quota5hLeftPercent, quota7dLeftPercent,
quota5hResetAtMs, quota7dResetAtMs, quotaRateLimited, quotaSummary); replace the
inline object literal in the accounts = existingAccounts.map(...) with
existingAccounts.map(mapToAccountInfo) so future AccountInfo additions only
require updating the single mapper.
src/plugin/ui/settings-menu.ts (1)

96-99: Centralize submenu option copy to keep text management consistent.

These hints are currently inline literals while the rest of the UI uses UI_COPY. Moving them into src/plugin/ui/copy.ts will make future edits/localization safer.

Also applies to: 120-123, 144-146, 167-170

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/ui/settings-menu.ts` around lines 96 - 99, Replace inline hint
literals in the settings-menu submenu option objects with centralized keys in
the UI_COPY map: add new entries in src/plugin/ui/copy.ts (e.g. ANSI16_HINT,
ANSI256_HINT, TRUECOLOR_HINT and the analogous keys for the other sets
referenced at the comment: the groups at lines ~120-123, ~144-146, ~167-170),
then import UI_COPY in src/plugin/ui/settings-menu.ts and replace each hint:
'...' with hint: UI_COPY.ANSI16_HINT (or the corresponding key names you add).
Ensure key names are descriptive and used consistently across all submenu
objects so all hint text comes from UI_COPY.
src/plugin/ui/auth-menu.ts (2)

82-181: Consider extracting common switch logic in statusBadge.

The statusBadge function has nearly identical switch statements for v2Enabled and legacy paths. The only difference is 'verification-required' renders as 'verify' in v2 mode vs 'needs verification' in legacy mode.

This is a minor duplication that could be simplified, but the current implementation is functionally correct.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/ui/auth-menu.ts` around lines 82 - 181, statusBadge duplicates the
same switch logic for ui.v2Enabled and legacy paths; extract the common mapping
of AccountStatus -> {label, tone} (or compute label with a small conditional
only for 'verification-required') and then call the single withTone(label, tone)
once. Update statusBadge to call getUiRuntimeOptions() and withTone as now,
compute label = status === 'verification-required' ? (ui.v2Enabled ? 'verify' :
'needs verification') : <default label from status>, and tone = <map status to
'success'|'warning'|'danger'|'muted'> (you can reuse the existing mappings),
then return withTone(label, tone) from a single switch/mapping instead of
duplicating the whole switch for v2Enabled.

498-500: Simplify redundant ternary expression.

The hintText assignment has identical branches:

const hintText = ui.v2Enabled
  ? hasHint ? hint : undefined
  : hasHint ? hint : undefined;

This can be simplified since both v2Enabled paths produce the same result.

♻️ Suggested simplification
-            const hintText = ui.v2Enabled
-              ? hasHint ? hint : undefined
-              : hasHint ? hint : undefined;
+            const hintText = hasHint ? hint : undefined;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/ui/auth-menu.ts` around lines 498 - 500, The ternary that assigns
hintText is redundant: both ui.v2Enabled branches return the same hasHint ? hint
: undefined. Edit the assignment for hintText in src/plugin/ui/auth-menu.ts (the
expression using ui.v2Enabled, hasHint, and hint) to remove the ui.v2Enabled
check and directly return the hasHint ? hint : undefined result so the code is
simpler and equivalent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/plugin/config/loader.ts`:
- Around line 171-172: The current shallow merge (const merged = { ...existing,
...update }) drops keys in nested objects; replace it with a deep merge so
nested fields (e.g., ui) are merged rather than overwritten. Implement or import
a deepMerge helper and compute merged = deepMerge(existing, update) (or use
lodash.merge(existingCopy, update)), then call writeFileSync(path,
JSON.stringify(merged, null, 2) + "\n", "utf-8"); ensure deepMerge recursively
merges objects and preserves existing nested keys not present in update.

In `@src/plugin/ui/action-panel.test.ts`:
- Around line 73-75: The test cleanup currently only restores
process.stdout.rows when originalRows !== undefined, leaving a mocked value when
the original was undefined; change the teardown so it always removes the mocked
property first and then re-define it only if originalRows was defined—e.g.,
delete process.stdout.rows unconditionally, then if originalRows !== undefined
use Object.defineProperty(process.stdout, 'rows', { value: originalRows,
configurable: true }); this ensures process.stdout.rows is restored to the
original (including non-existence) in action-panel.test.ts.

In `@src/plugin/ui/auth-menu.test.ts`:
- Around line 8-9: The two .replace(...) calls in auth-menu.test.ts use regex
literals containing control-character escape sequences (\x1b, \u0000, \u007f)
which Biome flags; change them to avoid literal control escapes by constructing
the patterns with the RegExp constructor or by building the character classes
from string values (e.g., use new RegExp('\\x1b\\[[0-?]*[ -/]*[`@-`~]', 'g') for
the ANSI CSI sequence and new RegExp('[\\u0000-\\u001f\\u007f]', 'g') or a
String.fromCharCode()-based class for control chars) and then pass those RegExp
instances into the existing .replace(...) calls so the logic stays the same but
the regex literals no longer contain raw control-character escapes.

In `@src/plugin/ui/runtime.ts`:
- Around line 47-52: getUiRuntimeOptions currently returns the global
runtimeOptions object by reference, allowing external mutation; update
getUiRuntimeOptions to return a copy instead (e.g., a shallow copy via object
spread or a deep copy via structuredClone/JSON round-trip) so callers can’t
mutate the global state, and apply the same change to any other getters that
return runtimeOptions directly (e.g., the similar getter around lines 69-71);
keep setUiRuntimeOptions as the only mutator for the global state.

In `@src/plugin/ui/settings-menu.ts`:
- Around line 55-60: The current logic logs UI_COPY.settings.saved when the
local state changed (variable changed / result) but saveUserConfig may swallow
write errors; update the flow so that saveUserConfig returns or propagates a
boolean/Promise<boolean> indicating persistence success (or throw on failure)
and only call console.log(UI_COPY.settings.saved) when that persistence result
is true. Modify callers in settings-menu (the blocks around result handling at
the shown diff and the other similar blocks at lines 66-76, 112-114, 136-138,
159-161, 184-186) to await the saveUserConfig outcome and log
UI_COPY.settings.unchanged on no change and UI_COPY.settings.saved only on
confirmed persistence success; handle and log persistence failures explicitly.

In `@src/plugin/ui/theme.test.ts`:
- Line 21: Replace the control-character escape in the test assertion to satisfy
the linter: in src/plugin/ui/theme.test.ts update the regex used in the expect
call that asserts ansi16.colors.primary (the line with
expect(ansi16.colors.primary).toMatch(...)) to use the Unicode escape \u001b
instead of the hex escape \x1b so the pattern becomes /^\u001b\[\d+m$/; make no
other behavioral changes.

---

Outside diff comments:
In `@src/plugin/ui/auth-menu.test.ts`:
- Around line 4-103: The tests duplicate helper implementations
(sanitizeTerminalText, formatRelativeTime, formatDate, normalizeQuotaPercent,
parseLeftPercentFromSummary, formatDurationCompact, formatLimitCooldown,
statusTone, statusText, accountTitle) causing false positives; remove these
in-test re-implementations and instead import the real helpers from the
production module (auth-menu.ts), or export the needed functions from that
module if they are not exported, then update the test file to reference the
imported symbols so tests exercise the actual production logic.

---

Nitpick comments:
In `@src/plugin/cli.ts`:
- Around line 107-121: Extract the manual property-by-property mapping into a
dedicated mapper function (e.g., mapToAccountInfo or mapAccount) and use it
inside the existingAccounts.map call to produce accounts; the mapper should
accept the original account object (the type used in existingAccounts) and
return an AccountInfo by selecting/transforming fields (email, index, addedAt,
lastUsed, status, isCurrentAccount, enabled, quota5hLeftPercent,
quota7dLeftPercent, quota5hResetAtMs, quota7dResetAtMs, quotaRateLimited,
quotaSummary); replace the inline object literal in the accounts =
existingAccounts.map(...) with existingAccounts.map(mapToAccountInfo) so future
AccountInfo additions only require updating the single mapper.

In `@src/plugin/config/loader.ts`:
- Line 172: The current direct writeFileSync(path, JSON.stringify(merged, null,
2) + "\n", "utf-8") can produce truncated config files on interruption; change
to an atomic write: serialize merged to the content string, write that content
to a temporary sibling file (e.g., `${path}.tmp`) using a safe sequence (open ->
write -> fsync -> close or writeFileSync to temp then fsync/close), then rename
the temp to the final path with fs.renameSync so the replacement is atomic.
Update the code around the writeFileSync call in loader.ts (referencing path and
merged) to perform the temp-write + fsync + rename sequence and handle cleanup
on errors.

In `@src/plugin/ui/auth-menu.ts`:
- Around line 82-181: statusBadge duplicates the same switch logic for
ui.v2Enabled and legacy paths; extract the common mapping of AccountStatus ->
{label, tone} (or compute label with a small conditional only for
'verification-required') and then call the single withTone(label, tone) once.
Update statusBadge to call getUiRuntimeOptions() and withTone as now, compute
label = status === 'verification-required' ? (ui.v2Enabled ? 'verify' : 'needs
verification') : <default label from status>, and tone = <map status to
'success'|'warning'|'danger'|'muted'> (you can reuse the existing mappings),
then return withTone(label, tone) from a single switch/mapping instead of
duplicating the whole switch for v2Enabled.
- Around line 498-500: The ternary that assigns hintText is redundant: both
ui.v2Enabled branches return the same hasHint ? hint : undefined. Edit the
assignment for hintText in src/plugin/ui/auth-menu.ts (the expression using
ui.v2Enabled, hasHint, and hint) to remove the ui.v2Enabled check and directly
return the hasHint ? hint : undefined result so the code is simpler and
equivalent.

In `@src/plugin/ui/settings-menu.ts`:
- Around line 96-99: Replace inline hint literals in the settings-menu submenu
option objects with centralized keys in the UI_COPY map: add new entries in
src/plugin/ui/copy.ts (e.g. ANSI16_HINT, ANSI256_HINT, TRUECOLOR_HINT and the
analogous keys for the other sets referenced at the comment: the groups at lines
~120-123, ~144-146, ~167-170), then import UI_COPY in
src/plugin/ui/settings-menu.ts and replace each hint: '...' with hint:
UI_COPY.ANSI16_HINT (or the corresponding key names you add). Ensure key names
are descriptive and used consistently across all submenu objects so all hint
text comes from UI_COPY.

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between f0ee206 and 8baf2ae.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (20)
  • src/plugin/cli.ts
  • src/plugin/config/loader.ts
  • src/plugin/config/schema.ts
  • src/plugin/ui/action-panel.test.ts
  • src/plugin/ui/action-panel.ts
  • src/plugin/ui/ansi.ts
  • src/plugin/ui/auth-menu.test.ts
  • src/plugin/ui/auth-menu.ts
  • src/plugin/ui/confirm.ts
  • src/plugin/ui/copy.test.ts
  • src/plugin/ui/copy.ts
  • src/plugin/ui/format.test.ts
  • src/plugin/ui/format.ts
  • src/plugin/ui/runtime.test.ts
  • src/plugin/ui/runtime.ts
  • src/plugin/ui/select.ts
  • src/plugin/ui/settings-menu.test.ts
  • src/plugin/ui/settings-menu.ts
  • src/plugin/ui/theme.test.ts
  • src/plugin/ui/theme.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Greptile Review
🧰 Additional context used
🧬 Code graph analysis (14)
src/plugin/ui/confirm.ts (1)
src/plugin/ui/runtime.ts (1)
  • getUiRuntimeOptions (50-52)
src/plugin/ui/runtime.ts (1)
src/plugin/ui/theme.ts (6)
  • UiColorProfile (5-5)
  • UiGlyphMode (6-6)
  • UiPalette (7-7)
  • UiAccent (8-8)
  • UiTheme (33-38)
  • createUiTheme (169-187)
src/plugin/config/loader.ts (2)
src/plugin/config/schema.ts (1)
  • AntigravityConfig (462-462)
src/plugin/config/index.ts (2)
  • AntigravityConfig (19-19)
  • getUserConfigPath (25-25)
src/plugin/cli.ts (3)
src/plugin/ui/settings-menu.ts (1)
  • showSettingsMenu (9-64)
src/plugin/ui/action-panel.ts (1)
  • runActionPanel (42-158)
src/plugin/config/updater.ts (1)
  • updateOpencodeConfig (107-177)
src/plugin/ui/settings-menu.ts (5)
src/plugin/ui/runtime.ts (2)
  • getUiRuntimeOptions (50-52)
  • setUiRuntimeOptions (29-48)
src/plugin/ui/select.ts (2)
  • MenuItem (4-14)
  • select (126-491)
src/plugin/ui/copy.ts (1)
  • UI_COPY (1-72)
src/plugin/config/loader.ts (1)
  • saveUserConfig (160-176)
src/plugin/ui/theme.ts (4)
  • UiColorProfile (5-5)
  • UiGlyphMode (6-6)
  • UiPalette (7-7)
  • UiAccent (8-8)
src/plugin/ui/theme.test.ts (1)
src/plugin/ui/theme.ts (1)
  • createUiTheme (169-187)
src/plugin/ui/auth-menu.ts (6)
src/plugin/ui/runtime.ts (2)
  • getUiRuntimeOptions (50-52)
  • UiRuntimeOptions (4-11)
src/plugin/ui/format.ts (2)
  • formatUiBadge (147-156)
  • quotaToneFromLeftPercent (158-164)
src/plugin/ui/ansi.ts (1)
  • ANSI (6-34)
src/plugin/ui/select.ts (2)
  • MenuItem (4-14)
  • select (126-491)
src/plugin/ui/copy.ts (2)
  • formatCheckFlaggedLabel (74-79)
  • UI_COPY (1-72)
src/plugin/ui/confirm.ts (1)
  • confirm (4-18)
src/plugin/ui/format.ts (1)
src/plugin/ui/runtime.ts (1)
  • UiRuntimeOptions (4-11)
src/plugin/ui/copy.test.ts (1)
src/plugin/ui/copy.ts (2)
  • UI_COPY (1-72)
  • formatCheckFlaggedLabel (74-79)
src/plugin/ui/action-panel.test.ts (1)
src/plugin/ui/action-panel.ts (2)
  • runActionPanel (42-158)
  • waitForMenuReturn (160-236)
src/plugin/ui/runtime.test.ts (1)
src/plugin/ui/runtime.ts (3)
  • resetUiRuntimeOptions (69-72)
  • getUiRuntimeOptions (50-52)
  • setUiRuntimeOptions (29-48)
src/plugin/ui/select.ts (2)
src/plugin/ui/theme.ts (1)
  • UiTheme (33-38)
src/plugin/ui/ansi.ts (3)
  • ANSI (6-34)
  • isTTY (66-68)
  • parseKey (42-61)
src/plugin/ui/action-panel.ts (5)
src/plugin/ui/runtime.ts (1)
  • getUiRuntimeOptions (50-52)
src/plugin/cli.ts (1)
  • isTTY (212-212)
src/plugin/ui/copy.ts (1)
  • UI_COPY (1-72)
src/plugin/ui/format.ts (1)
  • paintUiText (107-112)
src/plugin/ui/ansi.ts (1)
  • ANSI (6-34)
src/plugin/ui/auth-menu.test.ts (1)
src/plugin/ui/auth-menu.ts (2)
  • AccountStatus (11-21)
  • AccountInfo (23-49)
🪛 Biome (2.4.4)
src/plugin/ui/theme.test.ts

[error] 21-21: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)

src/plugin/ui/auth-menu.ts

[error] 77-77: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)


[error] 78-78: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)


[error] 78-78: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)

src/plugin/ui/select.ts

[error] 45-45: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)


[error] 46-46: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)

src/plugin/ui/auth-menu.test.ts

[error] 8-8: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)


[error] 9-9: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)


[error] 9-9: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)

🔇 Additional comments (30)
src/plugin/ui/ansi.ts (1)

10-11: Solid ANSI/key handling expansion.

Home/End sequence support and stricter TTY gating look correct for interactive flows, and the new ANSI constants align with the v2 UI needs.

Also applies to: 19-29, 36-37, 50-51, 67-67

src/plugin/ui/copy.test.ts (1)

4-54: Good contract coverage for UI copy + label formatter.

The tests hit the key sections and verify the formatCheckFlaggedLabel behavior for no arg/zero/positive paths.

src/plugin/ui/confirm.ts (1)

1-1: Theme propagation in confirm flow is correct.

Using runtime theme in select keeps confirm rendering consistent with the rest of the TUI.

Also applies to: 5-5, 16-16

src/plugin/ui/settings-menu.test.ts (1)

63-187: Comprehensive settings-flow test coverage.

The suite cleanly validates change detection, runtime updates, and persisted payloads across single and multi-step sessions.

src/plugin/ui/runtime.test.ts (1)

4-67: Runtime options tests look solid.

Default values, partial updates, theme regeneration, and reset behavior are all validated well.

src/plugin/ui/copy.ts (1)

1-79: Centralized copy + formatter implementation is clean.

Good separation of user-facing text and small helper for flagged-count label composition.

src/plugin/ui/settings-menu.ts (1)

12-64: Main settings loop and action dispatch are clean and readable.

The current/selection flow is straightforward and easy to reason about.

src/plugin/config/schema.ts (1)

441-458: UI preference schema/default alignment looks good.

Enums and defaults for color_profile, glyph_mode, palette, and accent are consistent and match runtime/theme expectations.

Also applies to: 501-506

src/plugin/ui/action-panel.test.ts (1)

78-185: Good coverage on action-panel behavior across execution modes.

The suite exercises non-TTY, TTY alt-screen, logging capture, error cleanup, and return-path behavior well.

src/plugin/ui/format.test.ts (1)

22-140: Formatting test matrix is comprehensive and well-structured.

Both v2-enabled and fallback behavior are validated across all core formatting helpers.

src/plugin/cli.ts (1)

164-204: New action routing is wired cleanly.

set-current, refresh/toggle/delete account, settings, and configure-models panel flow are integrated coherently.

src/plugin/ui/auth-menu.test.ts (1)

160-266: Nice edge-case coverage for quota/status/title formatting scenarios.

The added cases improve confidence in the display semantics around account metadata.

src/plugin/ui/theme.ts (1)

46-187: Theme factory and profile/palette/accent mapping are well structured.

The API surface is clear and the composition path (resolveGlyphModegetGlyphs/getColors) is easy to maintain.

src/plugin/ui/runtime.ts (1)

54-67: Config-to-runtime mapping flow is straightforward.

initUiFromConfig cleanly translates persisted snake_case values into runtime options.

src/plugin/ui/action-panel.ts (3)

1-41: LGTM!

The imports, interfaces, constants, and helper functions are well-structured. The stringifyLogArgs function properly handles various input types with a fallback to String() when JSON serialization fails.


42-158: LGTM!

The runActionPanel function correctly:

  • Bypasses the panel UI for non-TTY environments
  • Captures console output with bounded buffer
  • Manages alt-screen and cursor state
  • Ensures cleanup in both success and error paths via nested try/finally blocks

The result as T assertion on line 157 is safe since the only path reaching it is when failed is false, meaning result was successfully assigned.


160-236: LGTM!

The waitForMenuReturn function correctly implements both auto-return (with countdown and pause) and simple prompt paths. The cleanup logic properly removes event listeners and restores stdin state. The terminal row calculations use Math.max(1, rows - 1) to handle small terminal sizes gracefully.

src/plugin/ui/format.ts (3)

1-23: LGTM!

The UiTextTone type and TONE_TO_COLOR mapping provide a clean abstraction for theme-based text styling. The null value for 'normal' correctly indicates no color transformation should be applied.


24-105: LGTM!

The badgeStyleForTone function provides comprehensive badge styling across all three color profiles (truecolor, ansi256, basic ANSI). The nested accent/palette handling ensures consistent visual appearance regardless of user configuration. TypeScript's exhaustive checking on the constrained tone parameter ensures all cases are handled.


107-164: LGTM!

The public formatting functions provide clean abstractions with consistent v1/v2 mode handling. The quotaToneFromLeftPercent thresholds (≤15% danger, ≤35% warning) provide reasonable visual indicators for quota status.

src/plugin/ui/select.ts (5)

45-46: Static analysis false positive - control characters are intentional.

The Biome lint warning about control characters in these regex patterns is a false positive. The \x1b (ESC, 0x1B) character is the standard prefix for ANSI escape sequences, and using it in regex is the correct approach for parsing terminal color codes.


48-124: LGTM!

The helper functions handle ANSI sequences correctly:

  • truncateAnsi preserves escape sequences while truncating visible characters
  • decodeHotkeyInput properly maps numpad escape sequences to their character equivalents

126-185: LGTM!

The select function validation and setup are robust:

  • Proper TTY check with clear error message
  • Edge case handling for empty/all-disabled items
  • Single-item shortcut for better UX
  • Bounds-checked initial cursor with fallback

187-340: LGTM!

The rendering logic is well-structured:

  • Theme-aware color mapping with proper fallbacks
  • Two focus styles (row-invert, chip) for different visual preferences
  • Scrolling window calculation for long item lists
  • Proper handling of separators, headings, and disabled items

342-491: LGTM!

The event handling is well-designed:

  • Input guard (120ms) prevents accidental selections on menu entry
  • Home/End navigation correctly finds boundary selectable items
  • allowEscape option provides control over escape behavior
  • Dynamic subtitle refresh respects minimum interval (80ms)
  • Cleanup properly removes all listeners and restores terminal state
src/plugin/ui/auth-menu.ts (5)

74-80: Static analysis false positive - control characters are intentional.

The Biome lint warnings about control characters in these regex patterns are false positives. The regex intentionally targets:

  • \x1b (ESC) to strip ANSI escape sequences
  • \u0000-\u001f and \u007f to remove C0 control characters and DEL

This is the standard approach for sanitizing terminal text to prevent control character injection.


11-73: LGTM!

The type definitions are comprehensive and well-structured. The AccountInfo interface supports flexible rendering with optional fields for status badges, quota display, and various UI customizations.


183-223: LGTM!

The account helper functions provide consistent behavior:

  • accountTitle generates numbered labels with fallback display names
  • accountSearchText enables filtering across multiple account fields
  • accountRowColor maps account state to appropriate visual indicators

225-400: LGTM!

The quota formatting functions provide rich visualization:

  • formatQuotaBar renders Unicode block characters for visual quota representation
  • formatDurationCompact handles all duration ranges (seconds to days)
  • formatAccountHint respects statuslineFields for customizable info display

619-687: LGTM!

The showAccountDetails function provides a comprehensive account management interface with keyboard shortcuts and confirmation dialogs for destructive actions. Focus preservation ensures smooth navigation.

Comment on lines 73 to 75
if (originalRows !== undefined) {
Object.defineProperty(process.stdout, 'rows', { value: originalRows, configurable: true })
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Restore process.stdout.rows even when the original value is undefined.

Current cleanup leaves the mocked rows value in place when the original was undefined, which can leak state to later tests.

✅ Minimal fix
-    if (originalRows !== undefined) {
-      Object.defineProperty(process.stdout, 'rows', { value: originalRows, configurable: true })
-    }
+    Object.defineProperty(process.stdout, 'rows', { value: originalRows, configurable: true })
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (originalRows !== undefined) {
Object.defineProperty(process.stdout, 'rows', { value: originalRows, configurable: true })
}
Object.defineProperty(process.stdout, 'rows', { value: originalRows, configurable: true })
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/ui/action-panel.test.ts` around lines 73 - 75, The test cleanup
currently only restores process.stdout.rows when originalRows !== undefined,
leaving a mocked value when the original was undefined; change the teardown so
it always removes the mocked property first and then re-define it only if
originalRows was defined—e.g., delete process.stdout.rows unconditionally, then
if originalRows !== undefined use Object.defineProperty(process.stdout, 'rows',
{ value: originalRows, configurable: true }); this ensures process.stdout.rows
is restored to the original (including non-existence) in action-panel.test.ts.

Comment on lines 8 to 9
.replace(/\x1b\[[0-?]*[ -/]*[@-~]/g, '')
.replace(/[\u0000-\u001f\u007f]/g, '')
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
python - <<'PY'
from pathlib import Path
p = Path("src/plugin/ui/auth-menu.test.ts")
lines = p.read_text(encoding="utf-8").splitlines()
for n in (8, 9):
    line = lines[n-1]
    ctrls = [hex(ord(ch)) for ch in line if ord(ch) < 32 and ch not in (9, 10, 13)]
    print(f"Line {n}: {line!r}")
    print(f"  control_chars: {ctrls if ctrls else 'none'}")
PY

rg -n 'replace\(' src/plugin/ui/auth-menu.test.ts

Repository: NoeFabris/opencode-antigravity-auth

Length of output: 401


🏁 Script executed:

#!/bin/bash
# Look for Biome configuration files
fd -e json -e jsonc -e toml | grep -i biome

# Check for biome config in common locations
ls -la biome.json biome.jsonc biome.toml .biomerc* 2>/dev/null || echo "No biome config found in root"

# Search for any mentions of noControlCharactersInRegex in the codebase
rg -i "noControlCharactersInRegex" --type json --type toml

Repository: NoeFabris/opencode-antigravity-auth

Length of output: 171


🏁 Script executed:

cat biome.json

Repository: NoeFabris/opencode-antigravity-auth

Length of output: 1853


🏁 Script executed:

#!/bin/bash
# Check the actual file to see the context around lines 8-9
head -20 src/plugin/ui/auth-menu.test.ts

# Try to run biome check if available (read-only)
if command -v biome &> /dev/null; then
  biome lint src/plugin/ui/auth-menu.test.ts 2>&1 | head -50
else
  echo "Biome not available in sandbox"
fi

Repository: NoeFabris/opencode-antigravity-auth

Length of output: 2466


Normalize regex patterns to fix Biome noControlCharactersInRegex lint errors.

Lines 8–9 contain escape sequences (\x1b, \u0000, \u007f) that Biome flags as control characters in regex. Refactor these patterns to avoid the representation of control characters within the regex literals—for example, by restructuring the character classes or moving the escapes outside the regex context.

Current code triggering lint errors:
    .replace(/\x1b\[[0-?]*[ -/]*[`@-`~]/g, '')
    .replace(/[\u0000-\u001f\u007f]/g, '')
🧰 Tools
🪛 Biome (2.4.4)

[error] 8-8: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)


[error] 9-9: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)


[error] 9-9: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/ui/auth-menu.test.ts` around lines 8 - 9, The two .replace(...)
calls in auth-menu.test.ts use regex literals containing control-character
escape sequences (\x1b, \u0000, \u007f) which Biome flags; change them to avoid
literal control escapes by constructing the patterns with the RegExp constructor
or by building the character classes from string values (e.g., use new
RegExp('\\x1b\\[[0-?]*[ -/]*[`@-`~]', 'g') for the ANSI CSI sequence and new
RegExp('[\\u0000-\\u001f\\u007f]', 'g') or a String.fromCharCode()-based class
for control chars) and then pass those RegExp instances into the existing
.replace(...) calls so the logic stays the same but the regex literals no longer
contain raw control-character escapes.

Comment on lines +55 to +60
if (result === true) {
changed = true
console.log(UI_COPY.settings.saved)
} else if (result === false) {
console.log(UI_COPY.settings.unchanged)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid showing “saved” when persistence may silently fail.

Line 57 reports success based only on local state change, but persistence goes through saveUserConfig, which swallows write errors. This can mislead users into thinking preferences were persisted when they were not.

💡 Suggested direction
- function persistUiConfig(): void {
+ function persistUiConfig(): boolean {
   const current = getUiRuntimeOptions()
-  saveUserConfig({
+  return saveUserConfig({
     ui: {
       color_profile: current.colorProfile,
       glyph_mode: current.glyphMode,
       palette: current.palette,
       accent: current.accent,
     },
   })
 }
-  persistUiConfig()
-  return true
+  const persisted = persistUiConfig()
+  return persisted

Then only print UI_COPY.settings.saved when persistence confirms success.
(Requires saveUserConfig to return success/failure instead of swallowing internally.)

Also applies to: 66-76, 112-114, 136-138, 159-161, 184-186

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/ui/settings-menu.ts` around lines 55 - 60, The current logic logs
UI_COPY.settings.saved when the local state changed (variable changed / result)
but saveUserConfig may swallow write errors; update the flow so that
saveUserConfig returns or propagates a boolean/Promise<boolean> indicating
persistence success (or throw on failure) and only call
console.log(UI_COPY.settings.saved) when that persistence result is true. Modify
callers in settings-menu (the blocks around result handling at the shown diff
and the other similar blocks at lines 66-76, 112-114, 136-138, 159-161, 184-186)
to await the saveUserConfig outcome and log UI_COPY.settings.unchanged on no
change and UI_COPY.settings.saved only on confirmed persistence success; handle
and log persistence failures explicitly.


expect(truecolor.colors.primary).toContain(';2;');
expect(ansi256.colors.primary).toContain(';5;');
expect(ansi16.colors.primary).toMatch(/^\x1b\[\d+m$/);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n src/plugin/ui/theme.test.ts | sed -n '15,30p'

Repository: NoeFabris/opencode-antigravity-auth

Length of output: 876


🏁 Script executed:

# Find linting configuration files
fd -e json -e js -e ts | grep -E "(eslint|biome|lint)" | head -20

Repository: NoeFabris/opencode-antigravity-auth

Length of output: 89


🏁 Script executed:

# Check for biome or eslint config in common locations
ls -la | grep -E "(eslint|biome)"
cat biome.json 2>/dev/null || cat eslintrc.json 2>/dev/null || cat .eslintrc.json 2>/dev/null || echo "No config found in root"

Repository: NoeFabris/opencode-antigravity-auth

Length of output: 1915


🏁 Script executed:

# Search for other \x1b patterns in regex literals across the codebase
rg '\\x1b' --type ts --type js -n

Repository: NoeFabris/opencode-antigravity-auth

Length of output: 10420


Replace control-character regex escape to satisfy linting.

Line 21 uses \x1b inside a regex literal and triggers lint/suspicious/noControlCharactersInRegex. Use \u001b instead.

Minimal fix
-      expect(ansi16.colors.primary).toMatch(/^\x1b\[\d+m$/);
+      expect(ansi16.colors.primary).toMatch(/^\u001b\[\d+m$/);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
expect(ansi16.colors.primary).toMatch(/^\x1b\[\d+m$/);
expect(ansi16.colors.primary).toMatch(/^\u001b\[\d+m$/);
🧰 Tools
🪛 Biome (2.4.4)

[error] 21-21: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/ui/theme.test.ts` at line 21, Replace the control-character escape
in the test assertion to satisfy the linter: in src/plugin/ui/theme.test.ts
update the regex used in the expect call that asserts ansi16.colors.primary (the
line with expect(ansi16.colors.primary).toMatch(...)) to use the Unicode escape
\u001b instead of the hex escape \x1b so the pattern becomes /^\u001b\[\d+m$/;
make no other behavioral changes.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 28, 2026

Greptile Summary

This PR introduces a comprehensive TUI v2 engine with a theme system, rich account dashboard, enhanced interactive select component, settings hub, action panel, and UI preference persistence. The implementation is well-architected with clean separation of concerns across 12 new files and 9 modified files.

Key achievements:

  • Theme system with 3 color profiles, 2 palettes, 4 accents, and 2 glyph modes provides excellent terminal compatibility
  • Rich dashboard with quota visualization (█▒ bars), search (/), quick-switch (1-9), and focus tracking significantly improves UX
  • Enhanced select component now supports generic types, hotkey callbacks, dynamic subtitle refresh, and Home/End navigation
  • Settings hub enables interactive theme customization with persistence to antigravity.json
  • Action panel provides polished alt-screen spinner with console capture for long-running operations
  • Comprehensive test coverage with 88 new tests across 7 test files (139 total test cases)

Issues identified (already noted in previous comments):
The existing review comments correctly identify the critical bugs that need to be addressed before merging. All issues are already documented in the previous review thread.

Confidence Score: 3/5

  • This PR has solid architecture and comprehensive test coverage, but contains several critical bugs that must be fixed before merging
  • Score reflects excellent design and testing (88 new tests), but multiple bugs exist: color bleed in truncateAnsi, incorrect delete-account routing, settings save error handling issues, missing initUiFromConfig call, and runtime state management bug. All issues are already documented in previous comments and are fixable without architectural changes.
  • Pay special attention to src/plugin/ui/select.ts (color bleed), src/plugin/cli.ts (routing + missing init), and src/plugin/ui/settings-menu.ts (save error handling)

Important Files Changed

Filename Overview
src/plugin/ui/theme.ts New theme system with comprehensive color profile support (ansi16/ansi256/truecolor), glyph modes, palettes, and accent colors. Clean implementation with proper type safety.
src/plugin/ui/runtime.ts Runtime options singleton with theme rebuilding. Has existing issue with returning direct reference instead of clone when config is undefined (line 71).
src/plugin/ui/select.ts Enhanced interactive select with generic types, hotkeys, dynamic subtitle refresh, and Home/End navigation. Has color bleed bug in truncateAnsi (ANSI reset removed before suffix).
src/plugin/ui/auth-menu.ts Rich account dashboard with search, quick-switch (1-9), quota bars, theme-aware badges. Well-implemented with comprehensive account display and focus tracking.
src/plugin/ui/settings-menu.ts Interactive settings hub for customizing color profile, glyph mode, palette, and accent. Has issues with save error handling (always returns true) and console.log causing screen flash.
src/plugin/ui/action-panel.ts Alt-screen spinner with console capture and auto-return countdown for long-running operations. Clean implementation with proper cleanup and error handling.
src/plugin/cli.ts CLI wiring for new actions and settings menu. Has delete-account routing bug (mode: "add" instead of "manage") and missing initUiFromConfig call at startup.
src/plugin/config/loader.ts Added saveUserConfig() for persisting UI preferences with deep merge and atomic write (tmp + rename). Clean implementation with proper error handling.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    Start([User launches TUI]) --> LoadConfig[loadConfig loads antigravity.json]
    LoadConfig --> InitUI{initUiFromConfig called?}
    InitUI -->|No - Bug| DefaultTheme[Uses default theme]
    InitUI -->|Yes| CustomTheme[Loads saved UI preferences]
    
    DefaultTheme --> MainMenu[showAuthMenu displays account dashboard]
    CustomTheme --> MainMenu
    
    MainMenu --> MenuAction{User selects action}
    
    MenuAction -->|1-9 key| QuickSwitch[Set current account instantly]
    MenuAction -->|/ key| Search[Filter accounts by search query]
    MenuAction -->|Settings| SettingsMenu[showSettingsMenu]
    MenuAction -->|Configure Models| ActionPanel[runActionPanel with spinner]
    MenuAction -->|Select Account| AccountDetails[showAccountDetails]
    MenuAction -->|Add Account| OAuth[OAuth flow]
    
    QuickSwitch --> MainMenu
    Search --> MainMenu
    
    SettingsMenu --> SubMenu{Select preference}
    SubMenu -->|Color/Glyph/Palette/Accent| UpdateRuntime[setUiRuntimeOptions]
    UpdateRuntime --> PersistConfig[persistUiConfig saves to antigravity.json]
    PersistConfig -->|Success| SaveSuccess[Show 'Settings saved']
    PersistConfig -->|Failure - Bug| SaveSuccess
    SaveSuccess --> SettingsMenu
    SubMenu -->|Back| MainMenu
    
    ActionPanel --> RunAction[Execute action with console capture]
    RunAction --> ShowLogs[Display logs in alt-screen]
    ShowLogs --> AutoReturn[Auto-return countdown]
    AutoReturn --> MainMenu
    
    AccountDetails --> DetailAction{Action}
    DetailAction -->|Delete| DeleteFlow[mode: add - Bug should be manage]
    DetailAction -->|Refresh| RefreshFlow[mode: add for re-auth]
    DetailAction -->|Toggle/Set Current| ManageFlow[mode: manage]
    DetailAction -->|Back| MainMenu
    
    DeleteFlow --> ProcessAction[Process in plugin.ts]
    RefreshFlow --> ProcessAction
    ManageFlow --> ProcessAction
    
    style InitUI fill:#fee
    style SaveSuccess fill:#fee
    style DeleteFlow fill:#fee
Loading

Last reviewed commit: 749f11f

- loader.ts: deep merge nested config objects, atomic write via
  temp+rename, return boolean success/failure
- runtime.ts: clone options in getters to prevent external mutation
- auth-menu.ts: export helper functions for test reuse, deduplicate
  statusBadge switch via mapping table, simplify redundant hintText
  ternary
- auth-menu.test.ts: import helpers from source instead of duplicating,
  removes Biome noControlCharactersInRegex violations
- settings-menu.ts: use saveUserConfig return value for persistence
  confirmation, centralize inline hint strings in UI_COPY
- copy.ts: add settings submenu hint constants and saveFailed message
- action-panel.test.ts: always restore process.stdout.rows in cleanup
- theme.test.ts: use RegExp constructor to avoid Biome control-char lint
- cli.ts: extract mapToAccountInfo helper from inline mapping
@ndycode ndycode changed the base branch from main to dev February 28, 2026 19:05
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (2)
src/plugin/ui/auth-menu.ts (1)

74-79: ⚠️ Potential issue | 🟠 Major

Regex literals here still trigger Biome noControlCharactersInRegex.

Line 77 and Line 78 use control-character escapes in regex literals; this is currently flagged by static analysis and can block linted CI.

💡 Proposed fix
+const ANSI_CSI_REGEX = new RegExp(String.raw`\x1b\[[0-?]*[ -/]*[`@-`~]`, 'g');
+const CONTROL_CHAR_REGEX = new RegExp(String.raw`[\u0000-\u001f\u007f]`, 'g');
+
 export function sanitizeTerminalText(value: string | undefined): string | undefined {
   if (!value) return undefined;
   return value
-    .replace(/\x1b\[[0-?]*[ -/]*[`@-`~]/g, '')
-    .replace(/[\u0000-\u001f\u007f]/g, '')
+    .replace(ANSI_CSI_REGEX, '')
+    .replace(CONTROL_CHAR_REGEX, '')
     .trim();
 }
#!/bin/bash
# Verify remaining control-char regex literals and (if available) run Biome on this file.
rg -nP "\\.replace\\(/.*(\\\\x1b|\\\\u0000|\\\\u007f)" src/plugin/ui/auth-menu.ts src/plugin/ui/auth-menu.test.ts
if command -v biome >/dev/null 2>&1; then
  biome lint src/plugin/ui/auth-menu.ts
fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/ui/auth-menu.ts` around lines 74 - 79, sanitizeTerminalText
currently uses regex literals with control-character escapes which trigger
Biome's noControlCharactersInRegex; replace those literals in
sanitizeTerminalText with RegExp constructors built from escaped strings (e.g.,
create a RegExp for the ANSI sequence and another for control chars using string
escapes like "\\x1b" and "\\u0000") and use them with the global flag in the
.replace calls so the same behavior is preserved without inline control
characters; after changing, run the provided grep/biome check to verify the rule
no longer flags the file.
src/plugin/config/loader.ts (1)

94-108: ⚠️ Potential issue | 🟠 Major

Deep merge is still missing in load-time config precedence.

Line 99 and Line 100 use a shallow spread, so partial nested overrides (for example ui) can overwrite and drop existing nested keys from defaults/user config during loadConfig.

💡 Proposed fix
 function mergeConfigs(
   base: AntigravityConfig,
   override: Partial<AntigravityConfig>
 ): AntigravityConfig {
-  return {
-    ...base,
-    ...override,
-    // Deep merge signature_cache if both exist
-    signature_cache: override.signature_cache
-      ? {
-          ...base.signature_cache,
-          ...override.signature_cache,
-        }
-      : base.signature_cache,
-  };
+  return deepMergeConfig(
+    base as Record<string, unknown>,
+    override as Record<string, unknown>,
+  ) as AntigravityConfig;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/config/loader.ts` around lines 94 - 108, mergeConfigs currently
shallow-spreads base and override so nested objects (e.g., ui) in override will
replace entire nested defaults; update mergeConfigs to perform deep merge for
nested config keys instead of shallow spreading: either call a shared deepMerge
utility (or implement a small recursive merge) to merge base and override at top
level, then still preserve the special handling for signature_cache as currently
written; specifically ensure nested keys like ui are merged (not overwritten)
when override.ui exists, and keep function name mergeConfigs and symbol
signature_cache unchanged so loadConfig behavior remains correct.
🧹 Nitpick comments (2)
src/plugin/ui/action-panel.test.ts (1)

170-182: Consider adding TTY-path test for waitForMenuReturn.

The tests verify non-TTY behavior (immediate return), but there's no explicit test for the TTY path where waitForMenuReturn would actually wait for user input or auto-return countdown. This path is partially exercised through runActionPanel TTY tests, but a dedicated test could improve coverage.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/ui/action-panel.test.ts` around lines 170 - 182, Add a dedicated
TTY-path unit test for waitForMenuReturn: make mockIsTTY return true, use Jest
fake timers to control time, call waitForMenuReturn with autoReturnMs set (e.g.,
1000) and then advance timers to trigger the auto-return and assert the promise
resolves; also add a separate case that simulates a keypress on stdin (or emits
the expected input event) to verify immediate return on user input. Ensure you
restore mockIsTTY and real timers/stdin behavior after each test and reference
the waitForMenuReturn function, mockIsTTY mock, and the
autoReturnMs/pauseOnAnyKey options so the new test is easy to locate.
src/plugin/ui/auth-menu.test.ts (1)

40-57: Stabilize time-based tests with fake timers.

These assertions depend on live wall-clock timing. Using vi.useFakeTimers()/vi.setSystemTime() will make them deterministic and avoid intermittent failures.

💡 Example approach
-import { describe, it, expect } from 'vitest';
+import { describe, it, expect, vi } from 'vitest';

 describe('auth-menu', () => {
+  const FIXED_NOW = new Date('2026-01-15T12:00:00.000Z').getTime();
+  beforeEach(() => {
+    vi.useFakeTimers();
+    vi.setSystemTime(FIXED_NOW);
+  });
+  afterEach(() => {
+    vi.useRealTimers();
+  });

   // existing tests...

Also applies to: 127-134

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/ui/auth-menu.test.ts` around lines 40 - 57, Tests that call
formatRelativeTime are flaky because they rely on the real clock; wrap the test
suite or individual tests with fake timers: call vi.useFakeTimers(), set a fixed
time with vi.setSystemTime(<fixedTimestamp>) before invoking formatRelativeTime,
and restore timers with vi.useRealTimers() (or vi.clearAllTimers()) after; apply
the same change for the similar tests around lines 127-134 to ensure
deterministic assertions for "today", "yesterday", "3d ago", and "2w ago".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/plugin/ui/auth-menu.ts`:
- Around line 414-441: The computed verifyLabel (from verifyLabel and
flaggedCount) is never used; update the menu item entries in the items array
(the entries with value.type 'verify' and/or 'verify-all') to use verifyLabel
instead of the hardcoded "Verify All Accounts" (or "Verify One Account") so the
flaggedCount context appears in the UI; locate the items array in auth-menu.ts
and replace the hardcoded label(s) with verifyLabel where appropriate while
keeping the value/type and color fields unchanged.

---

Duplicate comments:
In `@src/plugin/config/loader.ts`:
- Around line 94-108: mergeConfigs currently shallow-spreads base and override
so nested objects (e.g., ui) in override will replace entire nested defaults;
update mergeConfigs to perform deep merge for nested config keys instead of
shallow spreading: either call a shared deepMerge utility (or implement a small
recursive merge) to merge base and override at top level, then still preserve
the special handling for signature_cache as currently written; specifically
ensure nested keys like ui are merged (not overwritten) when override.ui exists,
and keep function name mergeConfigs and symbol signature_cache unchanged so
loadConfig behavior remains correct.

In `@src/plugin/ui/auth-menu.ts`:
- Around line 74-79: sanitizeTerminalText currently uses regex literals with
control-character escapes which trigger Biome's noControlCharactersInRegex;
replace those literals in sanitizeTerminalText with RegExp constructors built
from escaped strings (e.g., create a RegExp for the ANSI sequence and another
for control chars using string escapes like "\\x1b" and "\\u0000") and use them
with the global flag in the .replace calls so the same behavior is preserved
without inline control characters; after changing, run the provided grep/biome
check to verify the rule no longer flags the file.

---

Nitpick comments:
In `@src/plugin/ui/action-panel.test.ts`:
- Around line 170-182: Add a dedicated TTY-path unit test for waitForMenuReturn:
make mockIsTTY return true, use Jest fake timers to control time, call
waitForMenuReturn with autoReturnMs set (e.g., 1000) and then advance timers to
trigger the auto-return and assert the promise resolves; also add a separate
case that simulates a keypress on stdin (or emits the expected input event) to
verify immediate return on user input. Ensure you restore mockIsTTY and real
timers/stdin behavior after each test and reference the waitForMenuReturn
function, mockIsTTY mock, and the autoReturnMs/pauseOnAnyKey options so the new
test is easy to locate.

In `@src/plugin/ui/auth-menu.test.ts`:
- Around line 40-57: Tests that call formatRelativeTime are flaky because they
rely on the real clock; wrap the test suite or individual tests with fake
timers: call vi.useFakeTimers(), set a fixed time with
vi.setSystemTime(<fixedTimestamp>) before invoking formatRelativeTime, and
restore timers with vi.useRealTimers() (or vi.clearAllTimers()) after; apply the
same change for the similar tests around lines 127-134 to ensure deterministic
assertions for "today", "yesterday", "3d ago", and "2w ago".

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 8baf2ae and a6cf62a.

📒 Files selected for processing (9)
  • src/plugin/cli.ts
  • src/plugin/config/loader.ts
  • src/plugin/ui/action-panel.test.ts
  • src/plugin/ui/auth-menu.test.ts
  • src/plugin/ui/auth-menu.ts
  • src/plugin/ui/copy.ts
  • src/plugin/ui/runtime.ts
  • src/plugin/ui/settings-menu.ts
  • src/plugin/ui/theme.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/plugin/ui/settings-menu.ts
  • src/plugin/ui/copy.ts
  • src/plugin/ui/runtime.ts
  • src/plugin/ui/theme.test.ts
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (4)
src/plugin/config/loader.ts (3)
src/plugin/config/schema.ts (1)
  • AntigravityConfig (462-462)
src/plugin/config/index.ts (2)
  • AntigravityConfig (19-19)
  • getUserConfigPath (25-25)
scripts/check-quota.mjs (1)
  • path (22-22)
src/plugin/ui/auth-menu.test.ts (1)
src/plugin/ui/auth-menu.ts (11)
  • sanitizeTerminalText (74-80)
  • formatRelativeTime (82-90)
  • formatDate (92-95)
  • normalizeQuotaPercent (196-199)
  • parseLeftPercentFromSummary (201-212)
  • formatDurationCompact (214-230)
  • formatLimitCooldown (232-237)
  • statusTone (97-114)
  • statusText (116-118)
  • AccountInfo (23-49)
  • accountTitle (154-162)
src/plugin/ui/auth-menu.ts (5)
src/plugin/cli.ts (1)
  • AccountStatus (217-217)
src/plugin/ui/runtime.ts (2)
  • getUiRuntimeOptions (61-63)
  • UiRuntimeOptions (4-11)
src/plugin/ui/format.ts (3)
  • formatUiBadge (147-156)
  • quotaToneFromLeftPercent (158-164)
  • paintUiText (107-112)
src/plugin/ui/select.ts (2)
  • MenuItem (4-14)
  • select (126-491)
src/plugin/ui/copy.ts (1)
  • UI_COPY (1-85)
src/plugin/cli.ts (5)
src/plugin/ui/auth-menu.ts (2)
  • AccountInfo (23-49)
  • isTTY (657-657)
src/plugin/ui/ansi.ts (1)
  • isTTY (66-68)
src/plugin/ui/settings-menu.ts (1)
  • showSettingsMenu (9-64)
src/plugin/ui/action-panel.ts (1)
  • runActionPanel (42-158)
src/plugin/config/updater.ts (1)
  • updateOpencodeConfig (107-177)
🪛 Biome (2.4.4)
src/plugin/ui/auth-menu.ts

[error] 77-77: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)


[error] 78-78: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)


[error] 78-78: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)

🔇 Additional comments (3)
src/plugin/ui/action-panel.test.ts (2)

70-74: LGTM!

The teardown now unconditionally restores process.stdout.rows to its original value (including undefined), addressing the previous review feedback about potential state leakage between tests.


76-168: LGTM!

The runActionPanel tests provide good coverage:

  • Non-TTY direct execution path
  • TTY alt-screen behavior with proper sequence verification
  • Console capture/restoration on both success and error paths
  • Error propagation after cleanup
  • Return value handling

The use of promise.catch(() => {}) at lines 135 and 151 correctly prevents unhandled rejection warnings during error testing.

src/plugin/cli.ts (1)

102-126: Nice extraction of account mapping into a dedicated helper.

mapToAccountInfo keeps the CLI/auth-menu boundary clean and avoids repeated field wiring as account metadata grows.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (3)
src/plugin/cli.ts (1)

196-215: Add a fallback error message in the configure-models catch path.

Right now catch {} suppresses unexpected exceptions silently. A minimal fallback print would improve debuggability if runActionPanel fails before rendering its own error state.

Proposed patch
-        } catch {
-          // Action panel handles error display
+        } catch (error) {
+          // Fallback in case action panel fails before rendering error state
+          const message = error instanceof Error ? error.message : String(error);
+          console.log(`
+✗ Failed to configure models: ${message}
+`);
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/cli.ts` around lines 196 - 215, The empty catch in the
"configure-models" case swallows unexpected exceptions from
runActionPanel/updateOpencodeConfig; change it to catch the error (e.g., catch
(err)) and print a minimal fallback error message using console.error (for
example "Failed to configure models" plus the error) so failures that occur
before the action panel renders are visible; update the catch block in the case
"configure-models" branch that surrounds runActionPanel/updateOpencodeConfig to
log the error and optionally set a non-zero exit/status if appropriate.
src/plugin/ui/auth-menu.ts (2)

506-506: Variable statusText shadows exported function.

The local variable statusText on line 506 shadows the exported statusText function defined on line 118. Consider renaming to statusMessage or resolvedStatusText for clarity.

🔧 Suggested fix
-      const statusText = resolveStatusMessage();
-      if (statusText) {
-        parts.push(statusText);
+      const resolvedStatus = resolveStatusMessage();
+      if (resolvedStatus) {
+        parts.push(resolvedStatus);
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/ui/auth-menu.ts` at line 506, The local variable named statusText
in the auth-menu code shadows the exported function statusText (defined as
exported function statusText), so rename the local binding (where you assign
const statusText = resolveStatusMessage()) to a non-conflicting name like
statusMessage or resolvedStatusText and update all subsequent references in the
same scope (e.g., where resolveStatusMessage() result is used) to the new
identifier to avoid the export/name collision.

554-559: Redundant conditional in onInput handler.

Lines 558-559 contain dead code—the condition on line 558 has no effect since line 559 unconditionally returns undefined regardless. This appears to be leftover from an incomplete implementation or refactoring.

🧹 Suggested cleanup
        const selected = context.items[context.cursor];
        if (!selected || selected.separator || selected.disabled || selected.kind === 'heading') {
          return undefined;
        }
-       if (selected.value.type !== 'select-account') return undefined;
        return undefined;

If additional per-account key handling was intended here, consider adding the implementation or removing this block entirely.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/ui/auth-menu.ts` around lines 554 - 559, In the onInput handler
for the auth menu (the block that reads context.items[context.cursor] and checks
selected.separator/disabled/kind), remove the redundant conditional that tests
selected.value.type !== 'select-account' followed by an unconditional return
undefined, or if per-account key handling was intended implement that logic
instead; specifically update the code around the selected variable check in the
onInput handler so either the select-account branch contains actual handling for
selected.value (type 'select-account') or the entire select-account conditional
is removed to avoid dead code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/plugin/cli.ts`:
- Around line 196-215: The empty catch in the "configure-models" case swallows
unexpected exceptions from runActionPanel/updateOpencodeConfig; change it to
catch the error (e.g., catch (err)) and print a minimal fallback error message
using console.error (for example "Failed to configure models" plus the error) so
failures that occur before the action panel renders are visible; update the
catch block in the case "configure-models" branch that surrounds
runActionPanel/updateOpencodeConfig to log the error and optionally set a
non-zero exit/status if appropriate.

In `@src/plugin/ui/auth-menu.ts`:
- Line 506: The local variable named statusText in the auth-menu code shadows
the exported function statusText (defined as exported function statusText), so
rename the local binding (where you assign const statusText =
resolveStatusMessage()) to a non-conflicting name like statusMessage or
resolvedStatusText and update all subsequent references in the same scope (e.g.,
where resolveStatusMessage() result is used) to the new identifier to avoid the
export/name collision.
- Around line 554-559: In the onInput handler for the auth menu (the block that
reads context.items[context.cursor] and checks
selected.separator/disabled/kind), remove the redundant conditional that tests
selected.value.type !== 'select-account' followed by an unconditional return
undefined, or if per-account key handling was intended implement that logic
instead; specifically update the code around the selected variable check in the
onInput handler so either the select-account branch contains actual handling for
selected.value (type 'select-account') or the entire select-account conditional
is removed to avoid dead code.

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between a6cf62a and a4dd97d.

📒 Files selected for processing (2)
  • src/plugin/cli.ts
  • src/plugin/ui/auth-menu.ts
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (1)
src/plugin/ui/auth-menu.ts (6)
src/plugin/ui/runtime.ts (2)
  • getUiRuntimeOptions (61-63)
  • UiRuntimeOptions (4-11)
src/plugin/ui/format.ts (3)
  • formatUiBadge (147-156)
  • quotaToneFromLeftPercent (158-164)
  • paintUiText (107-112)
src/plugin/ui/ansi.ts (1)
  • ANSI (6-34)
src/plugin/ui/select.ts (2)
  • MenuItem (4-14)
  • select (126-491)
src/plugin/ui/copy.ts (2)
  • formatCheckFlaggedLabel (87-92)
  • UI_COPY (1-85)
src/plugin/ui/confirm.ts (1)
  • confirm (4-18)
🪛 Biome (2.4.4)
src/plugin/ui/auth-menu.ts

[error] 79-79: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)


[error] 80-80: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)


[error] 80-80: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)

🔇 Additional comments (13)
src/plugin/cli.ts (5)

11-13: Clean UI module integration imports.

The new imports are coherent with the added settings/action-panel/menu runtime flow.


46-51: Quota field propagation is correctly wired end-to-end.

ExistingAccountInfo and mapToAccountInfo stay in sync, so quota data reaches the UI layer without loss.

Also applies to: 107-121


60-60: set-current action plumbing is consistent.

Nice alignment between LoginMenuResult and both action paths that return setCurrentAccountIndex.

Also applies to: 165-167, 177-179


126-131: TTY fallback and account mapping setup look good.

The non-TTY fallback remains intact, and mapped account shape is prepared once before menu loop entry.


189-194: Settings/search loop behavior is correct.

showSettingsMenu() then continue keeps the user in the dashboard flow as expected.

src/plugin/ui/auth-menu.ts (8)

1-74: LGTM!

The imports are well-organized and the type definitions are comprehensive, properly supporting the expanded TUI v2 features including new account statuses, quota fields, and menu actions.


76-97: LGTM!

The sanitization function correctly strips ANSI escape sequences and control characters using appropriate regex patterns. The static analysis warnings about control characters in the regex (lines 79-80) are false positives—the function's purpose is to match and remove these characters.

The time formatting helpers provide clean relative time display with sensible boundary handling.


99-196: LGTM!

The status helpers are well-structured with clean mapping logic. The statusBadge function properly handles both v2 and legacy rendering paths, and accountRowColor provides sensible color coding for all status types.


198-325: LGTM!

The quota formatting utilities are well-designed with proper null handling, clean modular decomposition, and consistent theming support for both v2 and legacy modes. The visual bar rendering using █▒ characters provides clear quota visualization.


327-373: LGTM!

The formatAccountHint function is well-structured with configurable field ordering via statuslineFields, proper handling of edge cases, and clean separation of concerns using a Map for intermediate storage.


375-410: LGTM!

The promptSearchQuery function properly handles TTY detection and ensures readline cleanup in the finally block. The authMenuFocusKey function provides exhaustive coverage of all action types for consistent focus management.


417-443: verifyLabel is computed but still not used in menu items.

Line 417 builds verifyLabel from flaggedCount, but line 443 still hardcodes "Verify All Accounts", so the flagged count context never appears in the menu.


591-660: LGTM!

The showAccountDetails function provides a clean account detail view with proper confirmation prompts for destructive actions (delete, refresh) and convenient keyboard shortcuts. The focus state management across iterations is correctly implemented.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
src/plugin/ui/auth-menu.ts (2)

506-506: Variable statusText shadows the exported function of the same name.

The local variable statusText on line 506 shadows the exported statusText(status) function defined at line 118. This could cause confusion during maintenance.

🔧 Proposed fix: rename local variable
-      const statusText = resolveStatusMessage();
-      if (statusText) {
-        parts.push(statusText);
+      const statusMessageText = resolveStatusMessage();
+      if (statusMessageText) {
+        parts.push(statusMessageText);
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/ui/auth-menu.ts` at line 506, The local variable named statusText
(assigned from resolveStatusMessage()) shadows the exported function
statusText(status); rename the local variable to a non-conflicting name (e.g.,
resolvedStatusText or statusMessage) and update all local references where that
variable is used so the exported function statusText(status) remains unshadowed
and readable.

554-559: Redundant condition check with identical return.

Lines 558-559 both return undefined regardless of the condition check. The if (selected.value.type !== 'select-account') check has no effect since both branches return the same value. This looks like incomplete logic or leftover from refactoring.

🔧 Proposed simplification

If no special handling is needed for 'select-account', simplify:

-        if (selected.value.type !== 'select-account') return undefined;
-        return undefined;
+        return undefined;

Or if 'select-account' should have special handling, implement it:

         if (selected.value.type !== 'select-account') return undefined;
-        return undefined;
+        // Handle select-account specific input here
+        return undefined;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/ui/auth-menu.ts` around lines 554 - 559, The early-return block in
the auth menu handler always returns undefined and contains a redundant branch:
after guarding for invalid items you check "if (selected.value.type !==
'select-account') return undefined;" and then immediately "return undefined"
again; either remove the useless type check entirely or implement the special
handling for the 'select-account' case inside the surrounding function (the code
around context.items, context.cursor and selected.value.type in auth-menu.ts).
Update the logic so only one clear code path returns undefined and, if
'select-account' needs custom behavior, add that handling where the second
return currently stands.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/plugin/ui/auth-menu.ts`:
- Line 506: The local variable named statusText (assigned from
resolveStatusMessage()) shadows the exported function statusText(status); rename
the local variable to a non-conflicting name (e.g., resolvedStatusText or
statusMessage) and update all local references where that variable is used so
the exported function statusText(status) remains unshadowed and readable.
- Around line 554-559: The early-return block in the auth menu handler always
returns undefined and contains a redundant branch: after guarding for invalid
items you check "if (selected.value.type !== 'select-account') return
undefined;" and then immediately "return undefined" again; either remove the
useless type check entirely or implement the special handling for the
'select-account' case inside the surrounding function (the code around
context.items, context.cursor and selected.value.type in auth-menu.ts). Update
the logic so only one clear code path returns undefined and, if 'select-account'
needs custom behavior, add that handling where the second return currently
stands.

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between a4dd97d and 892b8b9.

📒 Files selected for processing (1)
  • src/plugin/ui/auth-menu.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Greptile Review
🧰 Additional context used
🧬 Code graph analysis (1)
src/plugin/ui/auth-menu.ts (5)
src/plugin/ui/runtime.ts (2)
  • getUiRuntimeOptions (61-63)
  • UiRuntimeOptions (4-11)
src/plugin/ui/format.ts (3)
  • formatUiBadge (147-156)
  • quotaToneFromLeftPercent (158-164)
  • paintUiText (107-112)
src/plugin/ui/ansi.ts (1)
  • ANSI (6-34)
src/plugin/ui/select.ts (2)
  • MenuItem (4-14)
  • select (126-491)
src/plugin/ui/copy.ts (1)
  • UI_COPY (1-85)
🪛 Biome (2.4.4)
src/plugin/ui/auth-menu.ts

[error] 79-79: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)


[error] 80-80: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)


[error] 80-80: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)

🔇 Additional comments (7)
src/plugin/ui/auth-menu.ts (7)

1-74: LGTM!

The imports are properly structured, and the type definitions provide clear contracts for the auth menu system. The discriminated union types (AuthMenuAction, AccountAction) and the expanded AccountInfo interface with optional display flags allow flexible UI configuration.


76-82: LGTM! The static analysis warnings are false positives.

The regex patterns at lines 79-80 intentionally match control characters to sanitize terminal text—stripping ANSI escape sequences and C0 control characters is the correct behavior for preventing injection of terminal sequences from untrusted input. This is a necessary security measure.


84-120: LGTM!

The formatting and status mapping functions are well-implemented with appropriate fallbacks. The days <= 0 check in formatRelativeTime correctly handles same-day timestamps.


122-196: LGTM!

Good defensive implementation with sanitizeTerminalText for user-provided account fields in accountTitle. The statusBadge function properly handles the special case for 'verification-required' with a conditional label.


198-325: LGTM!

The quota formatting utilities are well-designed with proper null handling and theme-aware rendering. The formatDurationCompact function correctly handles all time ranges with appropriate fallbacks.


327-410: LGTM!

The promptSearchQuery function properly manages readline resources with try/finally cleanup. The authMenuFocusKey switch comprehensively covers all action types for focus preservation across menu re-renders.


591-658: LGTM!

The showAccountDetails function properly implements confirmation prompts for destructive actions (delete, refresh) and provides intuitive keyboard shortcuts. The focus state management via focusAction and onCursorChange ensures a smooth user experience.

- Fix duplicate 'Check Quotas' by introducing separate checkFlagged constant
- Reorganize menu: Quick Actions → Advanced Checks → Saved Accounts → Danger Zone
- Add missing UI_COPY constants (verifyOneAccount, configureModels, geminiCliLogin, checkFlagged)
- Replace all hardcoded menu strings with UI_COPY references
- Add Gemini CLI Login menu item to Advanced Checks section
- Color verify-all item red/yellow based on flagged count (matching codex-multi-auth)
- Update copy.test.ts to use checkFlagged instead of checkAccounts
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

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 the current code and only fix it if needed.

Inline comments:
In `@src/plugin/ui/auth-menu.ts`:
- Around line 301-321: formatQuotaSummary currently returns account.quotaSummary
directly in the fallback branch which allows terminal escape sequences to be
injected; fix by sanitizing the summary before any rendering: implement or call
a helper (e.g., sanitizeTerminalString) that strips ANSI/control characters and
non-printable bytes, then use that sanitized string when calling paintUiText(ui,
...) or when returning the plain fallback for non-v2 UIs; apply the sanitization
to summary wherever it's read (the fallback return and any place segments might
include the raw summary) so formatQuotaSummary, paintUiText usage, and the ANSI
fallback never emit untrusted terminal sequences.
- Around line 76-81: The regexes in sanitizeTerminalText contain literal control
characters which Biome flags; update both patterns to use hex escapes instead of
literal control characters: replace the ANSI escape remover regex with one using
hex ranges (e.g. /\x1b\[[\x30-\x3F]*[\x20-\x2F]*[\x40-\x7E]/g) and replace the
control-char stripper with /[\x00-\x1F\x7F]/g so sanitizeTerminalText uses
escaped \xNN ranges rather than literal control characters.

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 892b8b9 and 310a66b.

📒 Files selected for processing (3)
  • src/plugin/ui/auth-menu.ts
  • src/plugin/ui/copy.test.ts
  • src/plugin/ui/copy.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/plugin/ui/copy.test.ts
📜 Review details
🧰 Additional context used
🪛 Biome (2.4.4)
src/plugin/ui/auth-menu.ts

[error] 79-79: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)


[error] 80-80: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)


[error] 80-80: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)

🔇 Additional comments (1)
src/plugin/ui/copy.ts (1)

91-96: Nice utility extraction for flagged-label rendering.

formatCheckFlaggedLabel keeps the menu text logic centralized and avoids hardcoded string assembly at call sites.

Comment on lines +76 to +81
export function sanitizeTerminalText(value: string | undefined): string | undefined {
if (!value) return undefined;
return value
.replace(/\x1b\[[0-?]*[ -/]*[@-~]/g, '')
.replace(/[\u0000-\u001f\u007f]/g, '')
.trim();
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n src/plugin/ui/auth-menu.ts | sed -n '70,90p'

Repository: NoeFabris/opencode-antigravity-auth

Length of output: 986


🏁 Script executed:

# Check for Biome configuration and lint status
if command -v biome >/dev/null 2>&1; then
  echo "=== Biome version ===" 
  biome --version
  echo ""
  echo "=== Biome check on auth-menu.ts ==="
  biome check src/plugin/ui/auth-menu.ts 2>&1 | head -100
else
  echo "Biome not found in PATH"
fi

Repository: NoeFabris/opencode-antigravity-auth

Length of output: 3440


🏁 Script executed:

# Search for the function definition and context
rg -n 'sanitizeTerminalText' src/plugin/ui/auth-menu.ts -A 10 -B 2

Repository: NoeFabris/opencode-antigravity-auth

Length of output: 1639


Resolve Biome lint blockers in sanitizeTerminalText.

Lines 79–80 trigger lint/suspicious/noControlCharactersInRegex errors (3 instances total), which block lint-gated CI. The regex patterns use literal control characters that Biome flags as suspicious.

💡 Proposed fix
 export function sanitizeTerminalText(value: string | undefined): string | undefined {
   if (!value) return undefined;
-  return value
-    .replace(/\x1b\[[0-?]*[ -/]*[`@-`~]/g, '')
-    .replace(/[\u0000-\u001f\u007f]/g, '')
-    .trim();
+  let result = '';
+  let inAnsiSequence = false;
+  for (const char of value) {
+    const code = char.charCodeAt(0);
+    if (inAnsiSequence) {
+      if (code >= 0x40 && code <= 0x7e) inAnsiSequence = false;
+      continue;
+    }
+    if (code === 0x1b) {
+      inAnsiSequence = true;
+      continue;
+    }
+    if (code >= 0x20 && code !== 0x7f) {
+      result += char;
+    }
+  }
+  return result.trim();
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function sanitizeTerminalText(value: string | undefined): string | undefined {
if (!value) return undefined;
return value
.replace(/\x1b\[[0-?]*[ -/]*[@-~]/g, '')
.replace(/[\u0000-\u001f\u007f]/g, '')
.trim();
export function sanitizeTerminalText(value: string | undefined): string | undefined {
if (!value) return undefined;
let result = '';
let inAnsiSequence = false;
for (const char of value) {
const code = char.charCodeAt(0);
if (inAnsiSequence) {
if (code >= 0x40 && code <= 0x7e) inAnsiSequence = false;
continue;
}
if (code === 0x1b) {
inAnsiSequence = true;
continue;
}
if (code >= 0x20 && code !== 0x7f) {
result += char;
}
}
return result.trim();
}
🧰 Tools
🪛 Biome (2.4.4)

[error] 79-79: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)


[error] 80-80: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)


[error] 80-80: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/ui/auth-menu.ts` around lines 76 - 81, The regexes in
sanitizeTerminalText contain literal control characters which Biome flags;
update both patterns to use hex escapes instead of literal control characters:
replace the ANSI escape remover regex with one using hex ranges (e.g.
/\x1b\[[\x30-\x3F]*[\x20-\x2F]*[\x40-\x7E]/g) and replace the control-char
stripper with /[\x00-\x1F\x7F]/g so sanitizeTerminalText uses escaped \xNN
ranges rather than literal control characters.

Comment on lines +301 to +321
function formatQuotaSummary(account: AccountInfo, ui: UiRuntimeOptions): string {
const summary = account.quotaSummary ?? '';
const showCooldown = account.showQuotaCooldown !== false;
const left5h = normalizeQuotaPercent(account.quota5hLeftPercent) ?? parseLeftPercentFromSummary(summary, '5h');
const left7d = normalizeQuotaPercent(account.quota7dLeftPercent) ?? parseLeftPercentFromSummary(summary, '7d');
const segments: string[] = [];

if (left5h !== null || typeof account.quota5hResetAtMs === 'number') {
segments.push(formatQuotaWindow('5h', left5h, account.quota5hResetAtMs, showCooldown, ui));
}
if (left7d !== null || typeof account.quota7dResetAtMs === 'number') {
segments.push(formatQuotaWindow('7d', left7d, account.quota7dResetAtMs, showCooldown, ui));
}
if (account.quotaRateLimited || summary.toLowerCase().includes('rate-limited')) {
segments.push(ui.v2Enabled ? paintUiText(ui, 'rate-limited', 'danger') : `${ANSI.red}rate-limited${ANSI.reset}`);
}

if (segments.length === 0) {
if (!summary) return '';
return ui.v2Enabled ? paintUiText(ui, summary, 'muted') : summary;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Sanitize quotaSummary before rendering to prevent terminal-sequence injection.

account.quotaSummary is rendered directly in the fallback path, so escape/control sequences can still reach the terminal UI.

💡 Proposed fix
 function formatQuotaSummary(account: AccountInfo, ui: UiRuntimeOptions): string {
-  const summary = account.quotaSummary ?? '';
+  const summary = sanitizeTerminalText(account.quotaSummary) ?? '';
   const showCooldown = account.showQuotaCooldown !== false;
   const left5h = normalizeQuotaPercent(account.quota5hLeftPercent) ?? parseLeftPercentFromSummary(summary, '5h');
   const left7d = normalizeQuotaPercent(account.quota7dLeftPercent) ?? parseLeftPercentFromSummary(summary, '7d');
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/plugin/ui/auth-menu.ts` around lines 301 - 321, formatQuotaSummary
currently returns account.quotaSummary directly in the fallback branch which
allows terminal escape sequences to be injected; fix by sanitizing the summary
before any rendering: implement or call a helper (e.g., sanitizeTerminalString)
that strips ANSI/control characters and non-printable bytes, then use that
sanitized string when calling paintUiText(ui, ...) or when returning the plain
fallback for non-v2 UIs; apply the sanitization to summary wherever it's read
(the fallback return and any place segments might include the raw summary) so
formatQuotaSummary, paintUiText usage, and the ANSI fallback never emit
untrusted terminal sequences.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

23 files reviewed, 9 comments

Edit Code Review Agent Settings | Greptile

import { updateOpencodeConfig } from "./config/updater";
import { showSettingsMenu } from "./ui/settings-menu";
import { runActionPanel } from "./ui/action-panel";
import { initUiFromConfig } from "./ui/runtime";
Copy link
Contributor

Choose a reason for hiding this comment

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

initUiFromConfig is imported but never called in this file, so UI preferences from antigravity.json won't load on startup. Users can save preferences via settings menu, but they'll reset on restart.

Call initUiFromConfig after loading the config, likely in promptLoginMode or at module initialization:

const config = loadConfig(directory);
initUiFromConfig(config.ui);
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin/cli.ts
Line: 13

Comment:
`initUiFromConfig` is imported but never called in this file, so UI preferences from `antigravity.json` won't load on startup. Users can save preferences via settings menu, but they'll reset on restart.

Call `initUiFromConfig` after loading the config, likely in `promptLoginMode` or at module initialization:

```typescript
const config = loadConfig(directory);
initUiFromConfig(config.ui);
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Codex

- Rename shadowed statusText variable to resolvedStatus in buildSubtitle (auth-menu.ts)
- Remove redundant select-account conditional dead code (auth-menu.ts)
- Add fallback error message in configure-models catch block (cli.ts)
- Truncate action-panel log lines to terminal width to prevent wrapping
- Remove filter(Boolean) that dropped intentional blank separator lines
- Remove redundant clearScreen after altScreenOff in cleanupScreen
palette?: string;
accent?: string;
}): UiRuntimeOptions {
if (!config) return runtimeOptions;
Copy link
Contributor

Choose a reason for hiding this comment

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

returns direct reference to internal state instead of clone when config is undefined

Suggested change
if (!config) return runtimeOptions;
if (!config) return cloneRuntimeOptions(runtimeOptions);
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin/ui/runtime.ts
Line: 71

Comment:
returns direct reference to internal state instead of clone when config is undefined

```suggestion
  if (!config) return cloneRuntimeOptions(runtimeOptions);
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Codex

- Wrap Check Quotas handler in runActionPanel (alt-screen rendering)
- Wrap Verify All handler in runActionPanel, convert process.stdout.write to console.log
- Add console.clear() before Gemini CLI Login and Verify Single (interactive flows)
- Add clearScreen: true to all settings-menu select() calls
- Import runActionPanel in plugin.ts
}

return out + suffix;
return output + suffix;
Copy link
Contributor

Choose a reason for hiding this comment

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

WARNING: ANSI reset sequence removed - The previous code added an ANSI reset when truncating strings containing escape codes to prevent terminal display issues. This regression could cause colored text to leak into subsequent terminal output.

while (index < input.length && kept < keep) {
if (input[index] === '\x1b') {
const match = input.slice(index).match(ANSI_LEADING_REGEX);
if (match) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ANSI reset removed before truncation suffix — causes color bleed

The old truncateAnsi appended ANSI.reset before the ... suffix whenever the truncated string contained an ANSI escape sequence:

// Old behaviour
if (out.includes('\x1b[')) {
  return `${out}${ANSI.reset}${suffix}`;
}
return out + suffix;

The new implementation removes that reset entirely:

return output + suffix;

labelText for unselected items is built as ${itemColor}${item.label}${reset}. When truncateAnsi cuts this string (e.g. a long email address), the trailing ${reset} is discarded. The result is something like \x1b[32muser@very-long-domain... — without any escape close — so the open color sequence bleeds into the ... suffix and into subsequent writeLine calls (hint lines, the help bar, etc.).

The write-site for unselected items has no external reset either:

writeLine(
  ` ${muted}${unselectedGlyph}${reset} ${truncateAnsi(labelText, Math.max(1, columns - 4))}`,
  //                                                                                          ^ no reset here
);

Fix: restore the reset before the suffix:

Suggested change
if (match) {
return output.includes('\x1b[') ? `${output}${ANSI.reset}${suffix}` : output + suffix;
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin/ui/select.ts
Line: 67

Comment:
**ANSI reset removed before truncation suffix — causes color bleed**

The old `truncateAnsi` appended `ANSI.reset` before the `...` suffix whenever the truncated string contained an ANSI escape sequence:

```ts
// Old behaviour
if (out.includes('\x1b[')) {
  return `${out}${ANSI.reset}${suffix}`;
}
return out + suffix;
```

The new implementation removes that reset entirely:

```ts
return output + suffix;
```

`labelText` for unselected items is built as `${itemColor}${item.label}${reset}`. When `truncateAnsi` cuts this string (e.g. a long email address), the trailing `${reset}` is discarded. The result is something like `\x1b[32muser@very-long-domain...` — without any escape close — so the open color sequence bleeds into the `...` suffix and into subsequent `writeLine` calls (hint lines, the help bar, etc.).

The write-site for unselected items has no external reset either:
```ts
writeLine(
  ` ${muted}${unselectedGlyph}${reset} ${truncateAnsi(labelText, Math.max(1, columns - 4))}`,
  //                                                                                          ^ no reset here
);
```

Fix: restore the reset before the suffix:
```suggestion
  return output.includes('\x1b[') ? `${output}${ANSI.reset}${suffix}` : output + suffix;
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Codex

Comment on lines +180 to +181
case "refresh-account":
return { mode: "add", refreshAccountIndex: action.account.index };
Copy link
Contributor

Choose a reason for hiding this comment

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

delete-account returns mode: "add" — mismatched routing

The existing code path through showAccountDetails returns mode: "manage" for deletion:

if (accountAction === "delete") {
  return { mode: "manage", deleteAccountIndex: action.account.index };
}

The new direct-action path returns mode: "add":

case "delete-account":
  return { mode: "add", deleteAccountIndex: action.account.index };

If plugin.ts dispatches on menuResult.mode before inspecting deleteAccountIndex, this routes the delete action into the account-addition flow instead of account management/deletion. refresh-account using mode: "add" is arguably intentional (re-auth resembles a fresh login), but deletion should not trigger the add flow. The inconsistency with the showAccountDetails path suggests this is unintentional.

Suggested change
case "refresh-account":
return { mode: "add", refreshAccountIndex: action.account.index };
case "delete-account":
return { mode: "manage", deleteAccountIndex: action.account.index };
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin/cli.ts
Line: 180-181

Comment:
**`delete-account` returns `mode: "add"` — mismatched routing**

The existing code path through `showAccountDetails` returns `mode: "manage"` for deletion:

```ts
if (accountAction === "delete") {
  return { mode: "manage", deleteAccountIndex: action.account.index };
}
```

The new direct-action path returns `mode: "add"`:

```ts
case "delete-account":
  return { mode: "add", deleteAccountIndex: action.account.index };
```

If `plugin.ts` dispatches on `menuResult.mode` before inspecting `deleteAccountIndex`, this routes the delete action into the account-addition flow instead of account management/deletion. `refresh-account` using `mode: "add"` is arguably intentional (re-auth resembles a fresh login), but deletion should not trigger the add flow. The inconsistency with the `showAccountDetails` path suggests this is unintentional.

```suggestion
      case "delete-account":
        return { mode: "manage", deleteAccountIndex: action.account.index };
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Codex

Comment on lines +100 to +107
]

const selection = await select(options, {
message: UI_COPY.settings.colorProfile,
subtitle: `Current: ${ui.colorProfile}`,
help: UI_COPY.settings.help,
clearScreen: true,
theme: ui.theme,
Copy link
Contributor

Choose a reason for hiding this comment

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

console.log between select calls causes a momentary screen flash

After the sub-menu select returns and closes (cursor restored, cursor shown), console.log writes directly to the terminal before the next iteration clears the screen. Because clearScreen now only triggers on the first render of each select invocation, there is a brief moment between the console.log call and the next select render where the message appears inline against the stale menu output, producing a visible flicker.

Consider writing the status message into the main menu's subtitle (or a dynamicSubtitle) instead, or at minimum using stdout.write with a cursor-position sequence so the text appears in a controlled location that the next render will overwrite cleanly.

// Example: surface feedback through the subtitle option
const subtitle = lastResult === true
  ? UI_COPY.settings.saved
  : lastResult === false
  ? UI_COPY.settings.unchanged
  : UI_COPY.settings.subtitle;

await select(items, { ..., subtitle });
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin/ui/settings-menu.ts
Line: 100-107

Comment:
**`console.log` between `select` calls causes a momentary screen flash**

After the sub-menu `select` returns and closes (cursor restored, cursor shown), `console.log` writes directly to the terminal before the next iteration clears the screen. Because `clearScreen` now only triggers on the *first* render of each `select` invocation, there is a brief moment between the `console.log` call and the next `select` render where the message appears inline against the stale menu output, producing a visible flicker.

Consider writing the status message into the main menu's `subtitle` (or a `dynamicSubtitle`) instead, or at minimum using `stdout.write` with a cursor-position sequence so the text appears in a controlled location that the next render will overwrite cleanly.

```ts
// Example: surface feedback through the subtitle option
const subtitle = lastResult === true
  ? UI_COPY.settings.saved
  : lastResult === false
  ? UI_COPY.settings.unchanged
  : UI_COPY.settings.subtitle;

await select(items, { ..., subtitle });
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Codex

ndycode added 8 commits March 1, 2026 05:37
… title repeat spam

The render() function in select.ts only cleared the screen on the first
render (!hasRendered), then used moveTo(1,1) for subsequent re-renders.
When content exceeded terminal rows, each 200ms refresh cycle scrolled
the terminal down, causing the title to accumulate on screen.

Fix: always use clearScreen + moveTo(1,1) for clearScreen-enabled menus,
matching codex-multi-auth behavior.
…ck loading

refreshAccessToken and ensureProjectContext have no timeout protection.
If Google OAuth or the managed project onboarding hangs, the entire
Check Quotas flow blocks indefinitely. Wrapping each account's check
in Promise.race with a 30s timeout ensures the UI never gets stuck.
…creen clear on re-renders

The Phase 14 title-repeat fix went too far — clearing the entire screen every
200ms render cycle caused visible flicker. Now matches codex-multi-auth exactly:
- First render: clearScreen + moveTo(1,1) (full clear)
- Subsequent renders: up(previousRenderedLines) + clearLine per line (overwrite in-place)
This prevents both title-repeat AND flicker.
…peat

The cursor-up approach fails when content exceeds terminal height (11+ accounts),
causing title accumulation. The full-clear approach causes flicker every 200ms.

Solution: buffer all output for clearScreen menus, then flush as a single atomic
stdout.write(clearScreen + moveTo(1,1) + entireFrame). One write = no flicker,
and clearScreen in the same write = no stale content. Non-clearScreen menus
still use the cursor-up + clearLine overwrite pattern.
ndycode added 2 commits March 1, 2026 07:11
- Use moveTo(1,1) + clearBelow for re-renders instead of clearScreen
- clearScreen (\x1b[2J]) pushes to scrollback on Windows, causing title repeat
- clearBelow (\x1b[J]) erases leftover lines without scrollback side effects
- First render still uses clearScreen for clean slate, re-renders reposition only
- Fix broken newline string literals in writeLine()
Alt screen (\x1b[?1049h) is a separate terminal buffer with no scrollback.
moveTo(1,1) is always absolute — no scroll-related positioning bugs.
Entered on menu open, exited on cleanup. Replaces clearScreen approach entirely.
Comment on lines +114 to +119
setUiRuntimeOptions({ colorProfile: selection })
const saved = persistUiConfig()
if (!saved) {
console.log(UI_COPY.settings.saveFailed)
}
return true
Copy link
Contributor

Choose a reason for hiding this comment

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

returns true even when persistUiConfig() fails, showing both "Failed to save settings." and "Settings saved." messages

Suggested change
setUiRuntimeOptions({ colorProfile: selection })
const saved = persistUiConfig()
if (!saved) {
console.log(UI_COPY.settings.saveFailed)
}
return true
setUiRuntimeOptions({ colorProfile: selection })
const saved = persistUiConfig()
if (!saved) {
console.log(UI_COPY.settings.saveFailed)
}
return saved
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin/ui/settings-menu.ts
Line: 114-119

Comment:
returns true even when `persistUiConfig()` fails, showing both "Failed to save settings." and "Settings saved." messages

```suggestion
  setUiRuntimeOptions({ colorProfile: selection })
  const saved = persistUiConfig()
  if (!saved) {
    console.log(UI_COPY.settings.saveFailed)
  }
  return saved
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Codex

Comment on lines +142 to +147
setUiRuntimeOptions({ glyphMode: selection })
const saved = persistUiConfig()
if (!saved) {
console.log(UI_COPY.settings.saveFailed)
}
return true
Copy link
Contributor

Choose a reason for hiding this comment

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

same issue - always returns true regardless of save status

Suggested change
setUiRuntimeOptions({ glyphMode: selection })
const saved = persistUiConfig()
if (!saved) {
console.log(UI_COPY.settings.saveFailed)
}
return true
setUiRuntimeOptions({ glyphMode: selection })
const saved = persistUiConfig()
if (!saved) {
console.log(UI_COPY.settings.saveFailed)
}
return saved
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin/ui/settings-menu.ts
Line: 142-147

Comment:
same issue - always returns true regardless of save status

```suggestion
  setUiRuntimeOptions({ glyphMode: selection })
  const saved = persistUiConfig()
  if (!saved) {
    console.log(UI_COPY.settings.saveFailed)
  }
  return saved
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Codex

Comment on lines +169 to +174
setUiRuntimeOptions({ palette: selection })
const saved = persistUiConfig()
if (!saved) {
console.log(UI_COPY.settings.saveFailed)
}
return true
Copy link
Contributor

Choose a reason for hiding this comment

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

same issue - always returns true regardless of save status

Suggested change
setUiRuntimeOptions({ palette: selection })
const saved = persistUiConfig()
if (!saved) {
console.log(UI_COPY.settings.saveFailed)
}
return true
setUiRuntimeOptions({ palette: selection })
const saved = persistUiConfig()
if (!saved) {
console.log(UI_COPY.settings.saveFailed)
}
return saved
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin/ui/settings-menu.ts
Line: 169-174

Comment:
same issue - always returns true regardless of save status

```suggestion
  setUiRuntimeOptions({ palette: selection })
  const saved = persistUiConfig()
  if (!saved) {
    console.log(UI_COPY.settings.saveFailed)
  }
  return saved
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Codex

Comment on lines +198 to +203
setUiRuntimeOptions({ accent: selection })
const saved = persistUiConfig()
if (!saved) {
console.log(UI_COPY.settings.saveFailed)
}
return true
Copy link
Contributor

Choose a reason for hiding this comment

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

same issue - always returns true regardless of save status

Suggested change
setUiRuntimeOptions({ accent: selection })
const saved = persistUiConfig()
if (!saved) {
console.log(UI_COPY.settings.saveFailed)
}
return true
setUiRuntimeOptions({ accent: selection })
const saved = persistUiConfig()
if (!saved) {
console.log(UI_COPY.settings.saveFailed)
}
return saved
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin/ui/settings-menu.ts
Line: 198-203

Comment:
same issue - always returns true regardless of save status

```suggestion
  setUiRuntimeOptions({ accent: selection })
  const saved = persistUiConfig()
  if (!saved) {
    console.log(UI_COPY.settings.saveFailed)
  }
  return saved
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Codex

ndycode added 2 commits March 1, 2026 07:35
- Clamp rendered content to terminal height to prevent Windows scrollback repeating
- Add '!' and 'x' prefixes to console.warn/error in action-panel
- Fix log viewport calculation in action-panel
- Add new 'antigravity' palette with deep space/Google UI inspired colors
- Add 'magenta' and 'purple' accent colors
- Set default UI theme to 'antigravity' palette with 'cyan' accent
- Update UI copy and settings menu to support new colors
- Use Unicode braille spinner for loading screens in unicode mode
- Update Unicode box-drawing borders (╭ and ╰)
- Update default actions to use cyan accent
@ndycode ndycode marked this pull request as draft March 1, 2026 00:13
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