Skip to content

feat: add Kiro reauthentication action - #2980

Merged
steipete merged 10 commits into
mainfrom
fix/2340-reauth-actions
Aug 16, 2026
Merged

feat: add Kiro reauthentication action#2980
steipete merged 10 commits into
mainfrom
fix/2340-reauth-actions

Conversation

@steipete

Copy link
Copy Markdown
Owner

Summary

  • Supersedes Add re-authenticate actions for Codex/Kiro, and a background-repair toggle for Claude #2340 while preserving @Vit129's contributor history and the safe Kiro reauthentication work.
  • Adds an explicit Kiro Re-authenticate action backed by kiro-cli login, with bounded process teardown, device-flow progress, failure presentation, and refresh only after success.
  • Adds complete locale-catalog fallbacks, binary-locator coverage, action-boundary coverage, and the shifted provider gatekeeper anchor.

Auth and Keychain review

  • Claude prompt-policy verdict: safe. The original boolean background-repair toggle from Add re-authenticate actions for Codex/Kiro, and a background-repair toggle for Claude #2340 is not retained because it duplicated the current three-mode policy and could turn an explicit never into always. The existing policy defaults to onlyOnUserAction; background delegated refresh crosses the opaque CLI/Keychain boundary only for stored mode always; never remains fail-closed. This branch adds no Claude background-prompt path.
  • Codex account scope: the global Codex action from Add re-authenticate actions for Codex/Kiro, and a background-repair toggle for Claude #2340 is intentionally removed. It used the ambient Codex home and could report success while the selected managed account remained expired. Existing account-row reauthentication remains account-scoped.
  • Kiro user action: the new runner is reachable through explicit Settings/menu login actions only. Descriptor construction is inert, and a focused test proves the settings closure runs only when the action is performed.
  • Shared launch helpers: ProcessPipeCapture.currentSnapshot() is a synchronized additive read, and resolveKiroCLIBinary is a provider-specific additive resolver. Existing process/path suites pass. New UI and alert strings contain only Kiro data; no cross-provider identity fields are used.

No live Keychain access or real login flow was executed.

Verification

  • make check
  • CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 swift test --skip-build --no-parallel --filter 'KiroLogin|CodexLogin|ClaudeOAuth|Keychain' — 360 macOS tests and 5 Linux tests passed
  • CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 swift test --skip-build --no-parallel --filter 'KiroBinaryLocator|PathBuilder|SubprocessRunner|ProviderSettingsDescriptor|ProviderArchitectureGatekeeper|SettingsStoreCoverage|CodexAccountsSettingsSection' — 203 tests passed
  • Structured pre-commit autoreview: clean, no accepted/actionable findings

Thanks @Vit129 for the original implementation and follow-up fixes in #2340.

Vit129 and others added 9 commits August 5, 2026 15:13
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 #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 #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
Co-authored-by: Vit129 <vitosk129@gmail.com>
@clawsweeper

clawsweeper Bot commented Aug 16, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6352ab7694

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

{
self.resolveBinary(
name: "kiro-cli",
overrideKey: "KIRO_CLI_PATH",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Use the Kiro binary override during usage refresh

When KIRO_CLI_PATH points to a binary outside the effective PATH, this resolver lets reauthentication succeed, but the post-login refresh still constructs KiroStatusProbe, whose resolver is TTYCommandRunner.which("kiro-cli"); the Kiro descriptor is named kiro and does not register this binary locator, so which cannot see the override and the successful login is immediately followed by a CLI-unavailable refresh. Wire the same locator into the Kiro provider/probe, or ensure the override is added to the refresh environment.

Useful? React with 👍 / 👎.

}
}

let timedOut = await self.wait(timeout: timeout, termination: termination)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Terminate the login subprocess when its task is cancelled

If CodexBar shuts down while Kiro login is polling, cancelShutdownTasks() cancels loginTask, but this continuation wait never observes task cancellation and only calls terminate after the 120-second timeout. Since application termination destroys that timeout task as well, the independently launched kiro-cli process group can remain orphaned after CodexBar exits; make cancellation resolve the wait and run the existing process-group teardown.

Useful? React with 👍 / 👎.

@clawsweeper clawsweeper Bot added merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Aug 16, 2026
@clawsweeper

clawsweeper Bot commented Aug 16, 2026

Copy link
Copy Markdown

Codex review: needs changes before merge. Reviewed August 16, 2026, 11:09 AM ET / 15:09 UTC.

ClawSweeper review

What this changes

Adds an explicit Kiro Settings/menu re-authentication action that runs kiro-cli login, shows device-flow progress and errors, then refreshes usage after success.

Merge readiness

Blocked by patch quality or review findings - 6 items remain

Keep open: both prior P2 blockers remain on the supplied latest head—an override-selected Kiro CLI cannot refresh usage, and cancelling the login task does not terminate its subprocess. Likely related people: steipete (medium confidence).

Priority: P2
Reviewed head: 15e39ba4bf3bfb7b878c266bb08fe86087bc373b

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The proposed action is narrowly scoped, but two source-confirmed P2 reliability defects remain before it is ready to merge.
Proof confidence 🌊 off-meta tidepool Not applicable: This owner-authored PR is not subject to the external-contributor proof gate; its stated tests are supplemental and no live Keychain or login run was requested.
Patch quality 🦪 silver shellfish (2/6) 2 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This owner-authored PR is not subject to the external-contributor proof gate; its stated tests are supplemental and no live Keychain or login run was requested.
Evidence reviewed 5 items Refresh still bypasses the new resolver: Current main’s Kiro fetch strategy checks TTYCommandRunner.which("kiro-cli") and constructs the default probe, while the PR’s new login runner separately accepts KIRO_CLI_PATH; a successful login through an override outside PATH therefore reaches a refresh that reports no CLI.
Default probe has the same PATH-only assumption: The default Kiro status probe resolves only through TTYCommandRunner.which("kiro-cli"), confirming that the post-login refresh does not inherit the PR’s provider-specific binary resolution.
Cancellation reaches the login task but not the spawned process: Shutdown cancels loginTask; the previously reviewed Kiro runner waits on a continuation that has no cancellation path, so its process-group teardown only occurs on timeout. The supplied latest commit is localization-only and does not address that earlier finding.
Findings 2 actionable findings [P2] Use the resolved Kiro CLI for post-login refresh
[P2] Terminate Kiro login when its task is cancelled
Security None None.

How this fits together

CodexBar’s Kiro provider locates kiro-cli, runs it to collect usage, and exposes provider-specific login actions. The new action launches Kiro’s interactive CLI flow and feeds a successful result into the normal provider refresh pipeline.

flowchart LR
  A[Settings or menu action] --> B[Kiro login flow]
  B --> C[Resolve Kiro CLI]
  C --> D[Browser device login]
  D --> E[Login result]
  E --> F[Provider usage refresh]
  F --> G[Menu and Settings usage]
Loading

Before merge

  • Use the resolved Kiro CLI for post-login refresh (P2) - KIRO_CLI_PATH can make this login succeed outside PATH, but the refresh then uses the current Kiro strategy and default probe, both of which resolve only which("kiro-cli"). Route those paths through the same resolver and add override coverage so a successful login does not immediately become a CLI-unavailable refresh.
  • Terminate Kiro login when its task is cancelled (P2) - This runner’s completion wait is only resolved by process termination or timeout. Cancelling loginTask during shutdown therefore leaves the interactive kiro-cli process group alive until its timeout; make the wait cancellation-aware and invoke the existing group teardown.
  • Resolve merge risk (P1) - Existing users who set KIRO_CLI_PATH outside PATH can complete re-authentication but immediately see a failed Kiro refresh.
  • Resolve merge risk (P2) - Quitting CodexBar during an interactive Kiro login can leave the launched CLI process group running until its timeout.
  • Improve patch quality - Use the override-aware resolver for the Kiro provider’s availability and probe paths, with a regression test.
  • Improve patch quality - Make login-task cancellation terminate the Kiro process group and add an interrupted-login regression test.

Findings

  • [P2] Use the resolved Kiro CLI for post-login refresh — Sources/CodexBar/Providers/Kiro/KiroLoginRunner.swift:37
  • [P2] Terminate Kiro login when its task is cancelled — Sources/CodexBar/Providers/Kiro/KiroLoginRunner.swift:76
Agent review details

Security

None.

Review metrics

None.

Root-cause cluster

Relationship: canonical
Canonical: #2980
Summary: This PR is the active narrowed replacement for the earlier combined re-authentication proposal.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Repair the shared Kiro resolution and teardown paths (recommended)
    Route normal Kiro probing through the same override-aware resolver and terminate the login process group when the login task is cancelled before merging.
  2. Pause the re-authentication action
    Do not merge the action until the refresh and shutdown outcomes are reliable for configured binaries and interrupted logins.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Route Kiro availability, probing, and version detection through the override-aware resolver; add override-path coverage; make the login wait cancellation-aware and terminate the process group; run focused Kiro tests and make check.

Technical review

Best possible solution:

Use one provider-scoped Kiro CLI resolver for login, availability, version detection, and status probing, and make the login wait cancellation-aware so teardown always reaches the process group.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: set KIRO_CLI_PATH to an executable outside PATH, run the action successfully, and the normal refresh follows the current PATH-only Kiro probe. Cancellation is likewise traceable from shutdown’s loginTask.cancel() to the runner’s non-cancellable continuation wait.

Is this the best way to solve the issue?

Yes for the repair direction: sharing the Kiro resolver across login and probing, then tying cancellation to the existing process-group teardown, preserves the intended explicit user action without adding parallel behavior.

Full review comments:

  • [P2] Use the resolved Kiro CLI for post-login refresh — Sources/CodexBar/Providers/Kiro/KiroLoginRunner.swift:37
    KIRO_CLI_PATH can make this login succeed outside PATH, but the refresh then uses the current Kiro strategy and default probe, both of which resolve only which("kiro-cli"). Route those paths through the same resolver and add override coverage so a successful login does not immediately become a CLI-unavailable refresh.
    Confidence: 0.96
  • [P2] Terminate Kiro login when its task is cancelled — Sources/CodexBar/Providers/Kiro/KiroLoginRunner.swift:76
    This runner’s completion wait is only resolved by process termination or timeout. Cancelling loginTask during shutdown therefore leaves the interactive kiro-cli process group alive until its timeout; make the wait cancellation-aware and invoke the existing group teardown.
    Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against f0824413c72a.

Labels

Label justifications:

  • P2: The new recovery action can misreport a successful authentication as unavailable and can leave a local login process running, but the impact is limited to Kiro users.
  • merge-risk: 🚨 auth-provider: The PR introduces a Kiro authentication path whose successful override-based login is not compatible with the existing refresh resolver.
  • merge-risk: 🚨 availability: The interactive subprocess can outlive CodexBar shutdown because task cancellation is not connected to process teardown.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🌊 off-meta tidepool and patch quality is 🦪 silver shellfish.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: This owner-authored PR is not subject to the external-contributor proof gate; its stated tests are supplemental and no live Keychain or login run was requested.

Evidence

Acceptance criteria:

  • [P1] CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 swift test --filter KiroBinaryLocator.
  • [P1] CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 swift test --filter KiroLoginRunner.
  • [P1] CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 swift test --filter KiroStatusProbe.
  • [P1] make check.
  • [P1] make test.

What I checked:

  • Refresh still bypasses the new resolver: Current main’s Kiro fetch strategy checks TTYCommandRunner.which("kiro-cli") and constructs the default probe, while the PR’s new login runner separately accepts KIRO_CLI_PATH; a successful login through an override outside PATH therefore reaches a refresh that reports no CLI. (Sources/CodexBarCore/Providers/Kiro/KiroProviderDescriptor.swift:56, f0824413c72a)
  • Default probe has the same PATH-only assumption: The default Kiro status probe resolves only through TTYCommandRunner.which("kiro-cli"), confirming that the post-login refresh does not inherit the PR’s provider-specific binary resolution. (Sources/CodexBarCore/Providers/Kiro/KiroStatusProbe.swift:242, f0824413c72a)
  • Cancellation reaches the login task but not the spawned process: Shutdown cancels loginTask; the previously reviewed Kiro runner waits on a continuation that has no cancellation path, so its process-group teardown only occurs on timeout. The supplied latest commit is localization-only and does not address that earlier finding. (Sources/CodexBar/StatusItemController+Shutdown.swift:30, f0824413c72a)
  • Current Kiro ownership trail: Blame attributes the current Kiro fetch strategy’s PATH-only availability and probe construction to Peter Steinberger; the feature history is concentrated in this current provider surface. (Sources/CodexBarCore/Providers/Kiro/KiroProviderDescriptor.swift:56, 0e453c4a5b2a)
  • PR-head inspection limitation: The local partial clone could inspect current main but could not fetch the supplied PR head because the sandbox could not resolve github.com; latest-head continuity is therefore based on the provided PR commit and review context. (15e39ba4bf3b)

Likely related people:

  • steipete: Current Kiro provider availability and fetch lines are attributed to Peter Steinberger, and the owner authored the integration PR. (role: recent area contributor; confidence: medium; commits: 0e453c4a5b2a; files: Sources/CodexBarCore/Providers/Kiro/KiroProviderDescriptor.swift, Sources/CodexBarCore/Providers/Kiro/KiroStatusProbe.swift)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)
  • reviewed 2026-08-16T14:36:50.509Z sha 6352ab7 :: needs changes before merge. :: [P2] Honor the configured Kiro binary during refresh | [P2] Terminate Kiro login when its task is cancelled

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Aug 16, 2026
@steipete
steipete merged commit 355f844 into main Aug 16, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants