Add re-authenticate actions for Codex/Kiro, and a background-repair toggle for Claude - #2340
Add re-authenticate actions for Codex/Kiro, and a background-repair toggle for Claude#2340Vit129 wants to merge 8 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 883624f090
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| stdoutCapture.start() | ||
| stderrCapture.start() | ||
|
|
||
| let timedOut = await self.wait(timeout: timeout, termination: termination) |
There was a problem hiding this comment.
Surface Kiro device-flow output before waiting
When kiro-cli login uses device flow (the documented fallback for remote/browser-doesn't-open cases, where it prints a device code/URL and then polls), this waits for the subprocess to exit or hit the 120s timeout before combinedOutput is ever shown to the user. In that scenario the user never sees the code while the CLI is still polling, so the new Re-authenticate action times out and kills the login instead of letting them complete it; stream/detect the URL as it appears or run the login in a terminal/PTY.
Useful? React with 👍 / 👎.
|
Codex review: needs real behavior proof before merge. Reviewed August 16, 2026, 7:37 AM ET / 11:37 UTC. ClawSweeper reviewWhat this changesThe branch adds a Kiro CLI reauthentication flow, a global Codex settings reauthentication action, supporting process helpers, localized copy, and Kiro runner tests. Merge readiness⛔ Blocked until stronger real behavior proof is added - 5 items remain Keep open: the branch still adds a global Codex reauthentication action that bypasses the existing account-scoped flow, and its manual-test descriptions are not inspectable real-behavior proof. Priority: P2 Review scores
Verification
How this fits togetherProvider settings invoke provider-specific login flows and refresh usage afterward. Codex has managed and ambient accounts with separate authentication ownership, while Kiro depends on its locally installed CLI for Builder ID authentication. flowchart LR
A[Provider settings] --> B[Login action]
B --> C[Provider-specific runner]
C --> D[CLI and browser authentication]
D --> E[Credential storage]
E --> F[Usage refresh]
G[Managed Codex account] --> H[Account-scoped login]
H --> E
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Keep Codex reauthentication on its existing per-account controls, then land the Kiro flow only after redacted proof shows a successful live login and refreshed Kiro usage. Do we have a high-confidence way to reproduce the issue? Yes—source provides a high-confidence path: configure a managed Codex account and invoke the proposed global settings action; it reaches ambient login rather than the selected account’s scoped runner. This review did not execute the path because the checkout is read-only. Is this the best way to solve the issue? No—the Codex portion is not the best solution because current account controls already preserve managed versus ambient ownership; remove the global action and retain the Kiro-specific addition. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 010a352b0591. LabelsLabel justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (14 earlier review cycles; latest 8 shown)
|
Addresses clawsweeper P2 finding on steipete#2340: KiroLoginRunner only returned its captured output once the kiro-cli subprocess exited, but a device-flow login prints its URL and one-time code while still polling for browser approval, so the Settings action never showed it before the 120s timeout killed the login. Adds a live poll of the still-running subprocess's pipes for a device-flow URL and surfaces it as soon as it appears via an onProgress callback, instead of waiting for process completion.
Addresses clawsweeper P2 finding on steipete#2340: KiroProviderImplementation returned true unconditionally, so the shared login contract refreshed usage even after a missing CLI, timeout, failed login, or cancellation. runKiroLoginFlow now returns the actual outcome.
Addresses clawsweeper P2 finding on steipete#2340: KiroLoginRunner only returned its captured output once the kiro-cli subprocess exited, but a device-flow login prints its URL and one-time code while still polling for browser approval, so the Settings action never showed it before the 120s timeout killed the login. Adds a live poll of the still-running subprocess's pipes for a device-flow URL and surfaces it as soon as it appears via an onProgress callback, instead of waiting for process completion.
Addresses clawsweeper P2 finding on steipete#2340: KiroProviderImplementation returned true unconditionally, so the shared login contract refreshed usage even after a missing CLI, timeout, failed login, or cancellation. runKiroLoginFlow now returns the actual outcome.
|
Thanks for the re-authenticate actions and the background-repair toggle — both address real friction. The branch has had failing checks since mid-July and the review still asks for real-behavior proof of the new auth flows. Could you rebase on current |
Previously the only way to change claudeOAuthKeychainPromptMode away from onlyOnUserAction was a defaults write — users hitting "background repair is suppressed... Click Refresh" had no in-app way to opt into always-on background repair. Adds a Privacy-section toggle in Advanced settings wrapping the existing mode as a simple on/off (always vs onlyOnUserAction), leaving the never/prompt-free path untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HzWPve8BJ9KqMcSBiGXzPN
Codex had no settings action to recover from an expired OAuth token
short of running \`codex login\` manually in a terminal. Adds a
settingsActions entry (same pattern already used by Antigravity's
"Login with Google" button) that calls the existing runLoginFlow ->
CodexLoginRunner -> \`codex login\` subprocess path.
Antigravity already exposes an equivalent button ("Login with Google" /
"Add Google Account" in Settings > Antigravity > Connection) wired to a
real browser OAuth flow via AntigravityLoginRunner — no change needed
there. Kiro has no CLI login/auth surface in its \`kiro\` binary
(confirmed via \`kiro --help\`: file/diff/chat/tunnel commands only, no
login or auth subcommand), so no button was added for it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HzWPve8BJ9KqMcSBiGXzPN
Earlier claim that Kiro has no CLI login was wrong — it was tested against \`kiro\` (the editor launcher), not \`kiro-cli\` (a separate binary at ~/.local/bin/kiro-cli). \`kiro-cli login\` opens a real browser OAuth flow and blocks until it completes, same shape as \`codex login\`. Adds BinaryLocator.resolveKiroCLIBinary (mirrors resolveAntigravityBinary) and KiroLoginRunner (mirrors CodexLoginRunner's subprocess-lifecycle handling) to wire a settingsActions "Re-authenticate" button through the existing runLoginFlow -> StatusItemController path already used by Codex and Antigravity. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HzWPve8BJ9KqMcSBiGXzPN
Addresses clawsweeper P2 finding on steipete#2340: KiroLoginRunner only returned its captured output once the kiro-cli subprocess exited, but a device-flow login prints its URL and one-time code while still polling for browser approval, so the Settings action never showed it before the 120s timeout killed the login. Adds a live poll of the still-running subprocess's pipes for a device-flow URL and surfaces it as soon as it appears via an onProgress callback, instead of waiting for process completion.
Addresses clawsweeper P2 finding on steipete#2340: KiroProviderImplementation returned true unconditionally, so the shared login contract refreshed usage even after a missing CLI, timeout, failed login, or cancellation. runKiroLoginFlow now returns the actual outcome.
Mirrors CodexLoginRunnerTests' process-lifecycle coverage for the new subprocess runner, which previously shipped with no test changes.
The Advanced pane's Boolean toggle wrote the same claudeOAuthKeychainPromptMode preference as the existing three-mode picker in Provider settings, but couldn't represent "Never prompt" - two controls, conflicting semantics. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018gG6Sz5aaJw2vbT2DSgtJU
532d45d to
76c12ef
Compare
|
Rebased onto current Ran the tests covering the three new flows locally ( Pushed the rebase ( Still working through the two remaining manual checklist items (Settings toggle flip + live Codex/Kiro re-authenticate click) and will follow up with results. |
|
Manual test plan results:
CI is still sitting on |
|
Closing this out for now — not planning to continue addressing the review feedback (the account-scoped Codex reauth fix and fresh proof) in the near term. Thanks for the review time either way; may reopen a focused follow-up later. |
|
Thanks @Vit129! This landed via #2980, which preserves your Kiro re-authenticate work (device-flow progress, bounded teardown, failure presentation) with full locale coverage. Two pieces were intentionally not carried over after review: the Claude background auto-repair boolean (it duplicated the existing three-mode Keychain prompt policy and could effectively turn an explicit Never into Always — the policy modes remain the way to opt into background prompting) and the global Codex re-auth button (it used the ambient Codex home and could report success while the selected managed account stayed expired; the account-scoped re-auth rows already cover this). Closing as superseded, with credit in the changelog. |
* Port upstream 0.50.1: Cursor rename + Ollama cookie stripping (steipete#2951, steipete#2949) * Port upstream 0.50.1: OpenCode Go session+weekly pace in CLI (steipete#2957) * Port upstream 0.50.1: chart axis label centering (steipete#2974) * Port upstream 0.50.1: Kiro re-authenticate via kiro-cli login (steipete#2340) * Port upstream 0.50.1: serve identity follows app redaction per-request (steipete#2960) * Port upstream 0.50.1: Codex routed pricing, auth.json read-only, known-zero history, Antigravity dashboard lanes, Claude OAuth revoked detection steipete#2946 Codex routed models pricing — codex_routed_provider() + strip_route_prefix() in cost_pricing.rs; deepseek/, kimi/, opencode/ routes price against matching models.dev provider; unknown provider/ prefixes return None. steipete#2944 Codex auth.json read-only during refresh — codex_external_oauth_sources_allowed setting (default OFF); is_external_oauth + last_refresh fields on CodexCredentials; enforce_external_oauth_gate (8-day staleness window). steipete#2932 Codex known-zero history — known_zero field on CostSummary; set in both cache-debounce and full-scan paths (history_coverage_established && sessions_count == 0); knownZero JSON field + CLI text update. steipete#2963 Antigravity dashboard lanes — quota-bucket dedup in parse_user_status; models sharing the same (remaining_fraction, reset_time) collapse to one lane. steipete#2516 Claude revoked vs missing OAuth — ProviderError::OAuthRevoked variant; revocation detection (401/403 with revoked/invalid_grant/token_revoked); 15-min CLI result cache (LazyLock<Mutex<Option<CachedCliResult>>>); fetch_via_auto returns cached CLI result when OAuth revoked, and stale cache when all live sources fail. * Port upstream 0.50.1: Mistral PAYG spend, cost-summary display style, per-provider accent color Item 1 — Mistral PAYG current-month API spend (steipete#2821, steipete#2947): - Add MonthlyPlan variant to MetricPreference enum - Add currency_symbol field to CostSnapshot with with_currency_symbol builder - Set currency_symbol from Mistral billing API response - Add MonthlyPlan to bridge label/parse, tray selected_metric_percent (None = no bar) - Show formatted cost amount in provider_status_label for MonthlyPlan - Add format_cost_amount helper in bridge.rs - Add Mistral monthly spend row in MenuCardDetails - Add monthlyPlan option to MenuBarMetricSection for Mistral Item 2 — Menu cost-summary display style per provider (steipete#2976): - Add CostSummaryDisplayStyle enum (Compact/Detailed/Hidden) in types.rs - Add cost_summary_display_style field to Settings + Default + RawSettings round-trip - Add to SettingsSnapshot bridge + SettingsUpdate + apply in Tauri commands - Add bridge label/parse functions - Apply in MenuCardDetails: hidden hides cost section, compact shows used/limit only - Update describeCard to filter hasCost for hidden style - Add Select control in UsageSpendTab Item 3 — Per-provider accent color override (steipete#2972): - Add accent_color field to ProviderConfig - Add brand_color(ProviderId) function in provider.rs (mirrors frontend registry) - Add accent_color/set_accent_color/effective_accent_color accessors in Settings - Add set/get/get_effective Tauri commands + normalize_hex_accent_color validator - Add providerAccentColors map to SettingsSnapshot bridge - Add setProviderAccentColor/getProviderAccentColor/getProviderEffectiveAccentColor to tauri.ts - Create AccentColorSection component (hex input, color picker, reset button) - Inject --provider-accent CSS variable on MenuCard article and ChartsSection - Update menu-metric__bar-fill and chart colors to use --provider-accent fallback - Update chartPalette providerCostColor/providerCreditsColor fallback chain - Pass providerAccentColors through TrayPanel/PopOutPanel/ProvidersTab/ProviderDetailPane Also: - Add 11 new locale keys to all 7 .ftl files + keys.ts - Add costSummaryDisplayStyle + providerAccentColors to all test mock SettingsSnapshot - Update chartPalette tests for --provider-accent fallback * Resolve merge conflicts and fix gate checks * Thermo-nuclear: extract ollama cookies, codex pricing, simplify accent color * fix: correct serde attribute placement on currency_symbol after rebase
Summary
claudeOAuthKeychainPromptModeisonlyOnUserAction(the default) and the token expires.runLoginFlow->CodexLoginRunner->codex loginpath already used elsewhere (e.g. Switch Account). Previously the only recovery from an expired Codex OAuth token was runningcodex loginmanually in a terminal.KiroLoginRunnerthat spawnskiro-cli login(mirrorsCodexLoginRunner's subprocess-lifecycle handling) and a newBinaryLocator.resolveKiroCLIBinary(mirrorsresolveAntigravityBinary). Antigravity already has an equivalent flow ("Login with Google"); this brings Codex and Kiro to parity with it.Test plan
swift build --product CodexBar -c debugsucceedsclaudeOAuthKeychainPromptModebetweenonlyOnUserActionandalways