Add Kiro provider#139
Conversation
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Model: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a347d84985
ℹ️ 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".
| guard let result = try await group.next() else { | ||
| throw ProviderError.providerError("kiro-cli /usage task failed") | ||
| } | ||
|
|
||
| group.cancelAll() | ||
| if process.isRunning { | ||
| process.terminate() |
There was a problem hiding this comment.
When the timeout task wins, try await group.next() throws and execution skips the cleanup block that calls process.terminate(). In that path, the command task can remain suspended on its continuation while kiro-cli keeps running, so the fetch can hang past the timeout and leak subprocesses under slow/stuck CLI conditions. Move termination/cancellation into a defer (or equivalent error path) so process cleanup runs even when group.next() throws.
Useful? React with 👍 / 👎.
| process.executableURL = binaryPath | ||
| process.arguments = ["chat", "--classic"] | ||
|
|
There was a problem hiding this comment.
This launches Kiro as kiro-cli chat --classic, but Kiro’s CLI docs describe classic mode as a top-level flag (kiro-cli --classic) and list --classic in global UI-engine precedence. On CLI versions that enforce that contract, this argument order exits with an error, causing Kiro usage fetch to fail every time even when authentication is valid.
Useful? React with 👍 / 👎.
![op-gg-ai-devops[bot]](https://avatars.githubusercontent.com/in/2680015?s=60&v=4){kind=small}
There was a problem hiding this comment.
honestly this is a clean, well-scoped Kiro provider add. all the integration touch-points the codebase expects are wired up correctly — ProviderIdentifier, ProviderManager, CLIProviderManager, StatusBarController, both icon views, SubscriptionSettings, and ProviderMenuBuilder. test coverage is solid too — ANSI stripping, comma-separated numbers, plan-hint trimming, centicredit precision, and the "missing credits" error path are all exercised. nice work.
a few real things to look at though:
- SwiftLint is failing on
duplicate_imports—KiroProvider.swifthasimport Foundationtwice on lines 1–2. CI is red on this exact violation, so it has to be fixed before merge. kiro-cli chat --classic+ stdin piping is fragile. The cleaner non-interactive form iskiro-cli chat --no-interactive "/usage"(no stdin handshake, no/quitdance). This is what other Kiro-watching tools (e.g. CodexBar) use, and it removes a whole class of pipe-timing bugs.minimumFetchInterval = 60sspawns a full chat process every minute. Other CLI-spawning providers feel fine because HTTP is cheap, but spinning upkiro-cli chat ...every 60s is heavier and could trigger session-log spam on the Kiro side. Bumping to 5+ min would be friendlier.- Plan-name regex captures
KIRO PRO/KIRO POWERwhile presets are namedPro/Pro+/Power— name-based subscription matching may miss. Worth verifying or normalizing. - Output-format risk: the parser matches
Credits (X of Y covered in plan)andresets on YYYY-MM-DD. Newer Kiro CLI output has progress-bar style with███ X%andresets on MM/DDplus aBonus creditsblock (per the upstream Kiro issue #7752 and CodexBar'skiro.md). Test against the latestkiro-clito confirm the current format is still what we're parsing. - Minor:
k.circleSF symbol collides with Kimi (alsok.circle). Indistinguishable in alerts. Maybekeyboard.badge.eyeor another distinct symbol. - Docs nit:
docs/AI_USAGE_API_REFERENCE.mdhas a section per provider (Claude, Codex, Grok, ...). Kiro should get a short entry there too, and ascripts/query-kiro.shwould match the established pattern.
Nothing here is a correctness blocker — it all runs, parses, and ships data into the menu correctly. Just fix the lint failure and the rest can be follow-ups if you want.
CI Status
SwiftLint❌ failed (duplicate_importsonKiroProvider.swift:1-2)Build & Test✅Build🟡 in progressGitHub Actions Lint✅
| import Foundation | ||
| import Foundation |
There was a problem hiding this comment.
import Foundation appears twice. SwiftLint is failing on this exact line with duplicate_imports and that's blocking CI.
import Foundation
import os.log| static let kiro: [SubscriptionPreset] = [ | ||
| SubscriptionPreset(name: "Free", cost: 0), | ||
| SubscriptionPreset(name: "Pro", cost: 20), | ||
| SubscriptionPreset(name: "Pro+", cost: 40), | ||
| SubscriptionPreset(name: "Power", cost: 200) |
There was a problem hiding this comment.
The parser captures plan names like KIRO PRO, KIRO POWER (uppercase, with the KIRO prefix) from the CLI output, but the presets you added below name them Pro, Pro+, Power:
static let kiro: [SubscriptionPreset] = [
SubscriptionPreset(name: "Free", cost: 0),
SubscriptionPreset(name: "Pro", cost: 20),
SubscriptionPreset(name: "Pro+", cost: 40),
SubscriptionPreset(name: "Power", cost: 200)
]Depending on how preset matching is done (looks like name-based in ProviderSubscriptionPresets.presets(for:) consumers), this likely won't auto-resolve the right preset. Two clean options:
- Normalize the parsed plan name (strip
KIROprefix, lowercase, title-case) inparsePlanName. - Rename presets to match parsed output:
KIRO FREE,KIRO PRO,KIRO PRO+,KIRO POWER.
Worth picking whichever matches how other providers (Grok, Kimi) handle the same translation.
| let identifier: ProviderIdentifier = .kiro | ||
| let type: ProviderType = .quotaBased | ||
| let fetchTimeout: TimeInterval = 25 | ||
| let minimumFetchInterval: TimeInterval = 60 |
There was a problem hiding this comment.
60s is fine for HTTP-based quota providers but here we're spawning a whole kiro-cli chat process (plus a which/login-shell discovery if cached path expires) every minute. That's heavier than the other providers in this manager, and depending on how Kiro counts "sessions" on its side, it can show up as suspicious churn in their logs.
Suggest bumping to something more relaxed (Kiro's plan window is monthly anyway):
let minimumFetchInterval: TimeInterval = 300 // 5 minfetchTimeout = 25s is also pretty long — combined with the every-minute cadence, a hung CLI can keep a Process around for half the fetch interval. Worth trimming once the --no-interactive switch lands.
| return try await withThrowingTaskGroup(of: String.self) { group in | ||
| let process = Process() | ||
| process.executableURL = binaryPath | ||
| process.arguments = ["chat", "--classic"] |
There was a problem hiding this comment.
Spawning kiro-cli chat --classic and piping /usage\n/quit\n through stdin works but is brittle — any change in the chat REPL's prompt or input handling will silently break this. Kiro CLI also ships a non-interactive form:
process.arguments = ["chat", "--no-interactive", "/usage"]This is what CodexBar and similar tools use, and it removes the stdin pipe + /quit handshake entirely. The output is still ANSI-decorated so the existing stripANSI path keeps working.
Reference: Kiro issue kirodotdev/Kiro#7752 confirms there's no JSON usage endpoint yet, so a non-interactive chat invocation is the cleanest path.
| guard let creditsMatch = firstMatch( | ||
| in: normalized, | ||
| pattern: #"Credits\s*\(\s*([0-9][0-9,]*(?:\.[0-9]+)?)\s+of\s+([0-9][0-9,]*(?:\.[0-9]+)?)\s+covered\s+in\s+plan\s*\)"# | ||
| ), | ||
| creditsMatch.count >= 3, | ||
| let usedCredits = parseNumber(creditsMatch[1]), | ||
| let totalCredits = parseNumber(creditsMatch[2]), | ||
| totalCredits > 0 else { | ||
| throw ProviderError.decodingError("Kiro usage output did not include monthly credit usage") | ||
| } | ||
|
|
||
| let resetDate = firstMatch(in: normalized, pattern: #"resets\s+on\s+(\d{4}-\d{2}-\d{2})"#).flatMap { match in | ||
| match.count > 1 ? parseDate(match[1]) : nil | ||
| } | ||
| let planName = parsePlanName(from: normalized) | ||
| let overageStatus = firstMatch(in: normalized, pattern: #"Overages:\s*([A-Za-z]+)"#).flatMap { match in | ||
| match.count > 1 ? match[1] : nil | ||
| } |
There was a problem hiding this comment.
The regex chain is locked to a specific output format:
Credits (X of Y covered in plan)for creditsresets on YYYY-MM-DDfor reset dateOverages: <word>for overage status
Upstream Kiro (issue kirodotdev/Kiro#7752) and at least one third-party tool's docs describe a newer CLI output that uses a progress bar (██████ X%), a resets on MM/DD short date, and adds a Bonus credits: A/B credits used, expires in N days section. If you're on the older format the tests pass — but it'd be worth re-running kiro-cli chat --no-interactive "/usage" against the latest CLI build and:
- Confirming the regexes still hit.
- Adding a regression fixture for the newer format (with progress bar + MM/DD reset + bonus credits) so this provider doesn't silently zero out when Kiro updates the TUI.
If the new format is the only one in the wild, you'll also want to add a Bonus credits parse path so the secondary window shows up.
| return "moon.stars" | ||
| case .openCodeGo: | ||
| return "chevron.left.forwardslash.chevron.right" | ||
| case .kiro: |
There was a problem hiding this comment.
Kimi already uses k.circle (see the .kimi case right below in MultiProviderStatusBarIconView). Two providers with the same SF symbol means a user running both can't visually tell their status-bar alerts apart. Pick a Kiro-distinct symbol — e.g. keyboard.badge.eye, cpu, or a custom asset — and apply it consistently across ProviderIdentifier.iconName, MultiProviderStatusBarIconView, and ModernStatusBarIconView.
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Model: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbab362072
ℹ️ 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".
| var remainingCredits: Double { | ||
| max(totalCredits - usedCredits, 0) |
There was a problem hiding this comment.
When Kiro overages are enabled, usedCredits can exceed totalCredits, but this getter clamps remainingCredits to zero. That loses the over-limit amount before makeResult builds ProviderUsage.quotaBased, so the app reports at most 100% usage and cannot reflect how far over quota the account is. This underreports real usage specifically for paid plans with overage enabled.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
ngl, this is a really clean follow-up pass. you actually went through every single piece of feedback from the last round and fixed it — that's rare and worth calling out:
- ✅
duplicate_importslint failure → fixed, SwiftLint CI is green now. - ✅ Stdin pipe +
/quithandshake → replaced with the cleankiro-cli chat --no-interactive "/usage"invocation. - ✅
minimumFetchIntervalbumped from 60s → 300s, way friendlier to Kiro's side. - ✅ Plan name normalization (
KIRO PRO+→Pro+, etc.) now actually matches theSubscriptionPresetnames. Good catch on the precedence order innormalizePlanName—PRO+checked beforePROso it doesn't get swallowed. - ✅ New TUI format coverage: progress bar (
███ X%),MM/DDshort reset date, and theBonus credits:block all parsed. The table-style regression test (testUsageParserReadsTableOutputWithSlashReset) is exactly the kind of fixture I asked for. - ✅
k.circlecollision with Kimi resolved by pickingkeyboard.badge.eye, consistently applied acrossProviderIdentifier, both icon views, andStatusBarController. - ✅ Timeout path:
defer { group.cancelAll(); if process.isRunning { process.terminate() } }now handles the codex P1 about leaking subprocesses on the timeout branch. - ✅ Docs:
docs/AI_USAGE_API_REFERENCE.mdgot a proper Kiro section, andscripts/query-kiro.shmatches the establishedquery-*.shpattern.
Test coverage is solid for the parser surface — ANSI stripping, comma numbers, plan-hint trimming, centicredit scaling, the missing-credits error path, and the new TUI fallback are all exercised.
CI is fully green on cbab362 (Build, Build & Test, SwiftLint, GitHub Actions Lint).
A couple of small things still worth a look:
-
scripts/query-kiro.shis missing the shebang line. Every otherquery-*.shstarts with#!/usr/bin/env bashor#!/bin/bash. Without one, the kernel returnsENOEXECand the script falls through to/bin/sh(dash on Linux CI), which immediately dies onset -euo pipefailwithIllegal option -o pipefail. Reproduced locally:$ /bin/sh scripts/query-kiro.sh --help scripts/query-kiro.sh: 4: set: Illegal option -o pipefailOne-line fix, but it does block the script from being used outside of macOS bash.
-
Process.waitUntilExit()in the binary-discovery helpers. BothfindBinaryViaWhichandfindBinaryViaLoginShellblock the actor synchronously. The login-shell variant in particular can sit on a 100–500ms shell startup. Other providers (Antigravity, OpenCode Zen) moved to thewithCheckedThrowingContinuation+terminationHandlerpattern for exactly this reason — see the AGENTS.md "Process.waitUntilExit() Blocking Issue" note. Not a blocker since this only runs on cold fetch, but worth aligning eventually. -
Heads up on the open Codex P2 (
KiroUsageSnapshot.remainingCredits): clamping to>= 0discards overage information when the Kiro plan allows overages. The sharedTableFormatter.formatQuotaMetricsalready understands a negativeremainingas overage (e.g.,"3 overage (allowed)"), so passingentitlement - usedthrough unclamped would give a more honest read for paid plans. Not raising as a separate inline — Codex already has the right line on it.
None of these are correctness blockers for the feature, and the integration touch-points (ProviderManager, CLIProviderManager, StatusBarController, both icon views, SubscriptionSettings, ProviderMenuBuilder) are all wired up correctly. Nice work — happy to approve.
|
|
||
| do { | ||
| try process.run() | ||
| process.waitUntilExit() |
There was a problem hiding this comment.
both findBinaryViaWhich (line 115) and findBinaryViaLoginShell (line 143) use process.waitUntilExit() synchronously inside fetch(). per the AGENTS.md note on Process.waitUntilExit() Blocking Issue, this is a known anti-pattern in this codebase — it blocks the calling actor and can prevent other parallel provider fetches from completing while a slow shell startup (login-shell which can run 100–500ms easily) drags on.
other CLI-spawning providers (Antigravity, OpenCodeZen, and your own runKiroUsage below) already use the async withCheckedThrowingContinuation + terminationHandler pattern. consider migrating these helpers to the same style:
private func findBinaryViaWhich() async -> URL? {
try? await runProcessAsync(executable: "/usr/bin/which", args: ["kiro-cli"])
.flatMap { ... }
}not a blocker since cold-fetch discovery only happens once before the result gets cached upstream — just flagging for consistency with the established pattern.
| @@ -0,0 +1,203 @@ | |||
| # Query Kiro billing usage through the authenticated Kiro CLI. | |||
There was a problem hiding this comment.
this file is missing a shebang. without one, executing it falls through to /bin/sh (dash on Linux CI) which doesn't support set -o pipefail:
$ /bin/sh scripts/query-kiro.sh --help
scripts/query-kiro.sh: 4: set: Illegal option -o pipefail
every other scripts/query-*.sh starts with #!/usr/bin/env bash (e.g. query-grok.sh, query-opencode-go.sh). add the same line at the top so set -euo pipefail works regardless of how the script is invoked, and so scripts/query-all.sh (which exec's it directly) doesn't silently break in non-macOS bash environments.
#!/usr/bin/env bash
# Query Kiro billing usage through the authenticated Kiro CLI.
🤖 AI Code Review StatusReview started with |
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Model: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03fb2442b4
ℹ️ 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".
| # Query Kiro billing usage through the authenticated Kiro CLI. | ||
| #!/usr/bin/env bash |
There was a problem hiding this comment.
Putting a regular comment before #!/usr/bin/env bash prevents the kernel from recognizing the script interpreter, so direct invocations can run under /bin/sh (or fail with ENOEXEC) instead of Bash. In this script that breaks execution in non-Bash contexts because set -euo pipefail is not POSIX-sh compatible (e.g., dash errors on pipefail), which can make query-kiro.sh fail in automation even though it appears executable.
Useful? React with 👍 / 👎.
| "plan": plan, | ||
| "used_credits": used, | ||
| "total_credits": total, | ||
| "remaining_credits": max(total - used, 0), |
There was a problem hiding this comment.
Clamping remaining_credits with max(total - used, 0) hides overage when usage exceeds plan credits, so JSON/text output underreports real usage for paid plans with overages enabled. This makes the new Kiro query script report "0 left" instead of how far over quota the account is, which can mislead manual checks and any tooling consuming --json output.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
ngl this third pass is genuinely satisfying to review. you came back, hit every single point from the last round, and the diff actually shows it — overage preserved (remainingCredits = totalCredits - usedCredits, no clamp), shebang attempted, test fixture for the overage path, the works. that's rare and worth saying.
What landed cleanly this pass:
- ✅
KiroUsageSnapshot.remainingCreditsno longer clamps to>= 0. Negative remaining now flows intoProviderUsage.quotaBased, andtestMakeResultPreservesOverageAmountproves overage is reported asusagePercentage = 105,remainingQuota = -5_000. Exactly what the sharedTableFormatter.formatQuotaMetricsoverage path expects. - ✅ Integration touch-points are still all wired correctly:
ProviderIdentifier,ProviderManager,CLIProviderManager,StatusBarController(both the icon switch and theusagePercentCandidatesswitch),MultiProviderStatusBarIconView,ModernStatusBarIconView,SubscriptionSettings,ProviderMenuBuilder. Docs section 7 was added, sections 8–9 renumbered correctly. - ✅ SwiftLint and
Buildare green on03fb244.Build & Testis still pending at the time of review but didn't regress.
One thing actually broke though:
The Fix Kiro query script shebang commit (03fb244) put #!/usr/bin/env bash on line 2 of scripts/query-kiro.sh, with a # Query Kiro... comment on line 1. The kernel only treats the file as a shebang script if #! is the very first two bytes — line 2 doesn't count, so this is still the same ENOEXEC fallthrough the previous review flagged. Reproduced locally just now:
$ /bin/sh scripts/query-kiro.sh --help
scripts/query-kiro.sh: 6: set: Illegal option -o pipefail
Every other scripts/query-*.sh (e.g. query-grok.sh, query-opencode-go.sh) starts with the shebang on line 1. Swap the two lines and this is gone.
Smaller follow-up:
scripts/query-kiro.shstill clampsremaining_creditstomax(total - used, 0)(line 156), which contradicts the Swift change you just made on the same PR. Worth letting it go negative so the debug script matches what the app actually reports.
A few previously-raised items remain intentionally not re-raised: the Process.waitUntilExit() synchronous calls in findBinaryViaWhich / findBinaryViaLoginShell are a known not-a-blocker per the last review (cold-fetch only, cached upstream), and everything from rounds 1–2 (duplicate imports, --no-interactive, minimumFetchInterval=300, plan-name normalization, new TUI parser, keyboard.badge.eye, timeout-path cleanup, docs, query script existence) is fixed.
Approving once the shebang lands on line 1. Nice work overall.
| "plan": plan, | ||
| "used_credits": used, | ||
| "total_credits": total, | ||
| "remaining_credits": max(total - used, 0), |
There was a problem hiding this comment.
This still clamps remaining_credits to >= 0, but the Swift side in this same PR (KiroUsageSnapshot.remainingCredits, KiroProvider.makeResult) was specifically changed to let it go negative so overage on paid plans is reported honestly (e.g., usagePercentage = 105, remainingQuota = -5_000).
Either drop the clamp here too so the debug script matches the app, or at least surface the raw total - used alongside the clamped value:
result = {
...
"remaining_credits": total - used,
...
}Not a blocker — just so this script doesn't quietly mask the same overage information the menu now exposes.
| @@ -0,0 +1,205 @@ | |||
| # Query Kiro billing usage through the authenticated Kiro CLI. | |||
There was a problem hiding this comment.
The shebang line MUST be the first line of the file. Here it's on line 2 with a # Query Kiro billing usage... comment on line 1, so the kernel never sees #! at offset 0 — it returns ENOEXEC and the shell falls back to /bin/sh (dash on Linux CI), which immediately blows up on set -o pipefail:
$ /bin/sh scripts/query-kiro.sh --help
scripts/query-kiro.sh: 6: set: Illegal option -o pipefail
This is the exact failure mode the previous review flagged. Commit 03fb244 did add the shebang text, just placed it on the wrong line. Other scripts/query-*.sh files (grok, opencode-go) put it on line 1.
Fix:
#!/usr/bin/env bash
# Query Kiro billing usage through the authenticated Kiro CLI.Once that lands, scripts/query-all.sh (which exec's it directly) and any non-macOS bash invocation will stop silently dying.
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Model: |
There was a problem hiding this comment.
ngl this 4th pass is so close. you've got every Swift integration touch-point landing cleanly — KiroProvider itself reads fine, the parser fixture set actually covers both the classic line format and the new table-style TUI with bonus credits + MM/DD reset, and testMakeResultPreservesOverageAmount proves the overage path now flows through to ProviderUsage.quotaBased as usagePercentage = 105, remainingQuota = -5_000. integration touch-points still wired correctly: ProviderIdentifier, ProviderManager, CLIProviderManager, StatusBarController (icon switch + usagePercentCandidates weekly priority), MultiProviderStatusBarIconView, ModernStatusBarIconView, SubscriptionSettings, ProviderMenuBuilder. docs section 7 reads well, scripts/query-all.sh updated, and the keyboard.badge.eye icon picked specifically to avoid the Kimi k.circle collision is applied consistently across all 4 places.
also worth calling out: you actually went ahead and dropped the max(total - used, 0) clamp in scripts/query-kiro.sh line 156 between the last review and this one (commit 7b786a1), so the debug script now matches what the Swift provider reports. that's exactly what the previous round asked for, no nagging needed. good.
but one thing is genuinely still broken though:
scripts/query-kiro.sh STILL has the shebang on line 2, not line 1. this is now the third commit (4efebc2 had no shebang at all, 03fb244 put it on line 2, 7b786a1 left it on line 2) that's tried to fix this exact bug and the shebang is still in the wrong place. reproduced just now on this exact HEAD:
$ head -2 scripts/query-kiro.sh
# Query Kiro billing usage through the authenticated Kiro CLI.
#!/usr/bin/env bash
$ /bin/sh scripts/query-kiro.sh --help
scripts/query-kiro.sh: 6: set: Illegal option -o pipefail
the kernel only treats a file as a shebang script if #! is the very first two bytes of the file. a # comment on line 1 means the kernel returns ENOEXEC and the parent shell falls back to /bin/sh (dash on Linux CI / GitHub Actions), which then dies on set -o pipefail because that's bash-only. on macOS this happens to work because /bin/sh is bash3 in compat mode, but it breaks on every Linux invocation and breaks scripts/query-all.sh (which exec's it directly).
the fix is a one-line swap — just put #!/usr/bin/env bash as the very first line and move the comment below. every other scripts/query-*.sh does it this way. one shot, done.
everything else is approved as-is — Process.waitUntilExit() in findBinaryViaWhich / findBinaryViaLoginShell stays intentionally not re-raised per the last two reviews (cold-fetch only, cached upstream), and rounds 1–3 items (duplicate_imports, --no-interactive, minimumFetchInterval=300, plan-name normalization, new TUI parser, overage preservation in both Swift and the script, icon de-collision, timeout cleanup, docs section 7 + query-all.sh entry) are all properly landed.
approving on the strength of the Swift side. shebang swap is the only line that has to change to close out the round.
CI Status
SwiftLint✅ passGitHub Actions Lint✅ passBuild🟡 pending (no regression vs03fb244)Build & Test🟡 pending
| # Query Kiro billing usage through the authenticated Kiro CLI. | ||
| #!/usr/bin/env bash |
There was a problem hiding this comment.
this is the third commit aimed at fixing the shebang and it's still on line 2 instead of line 1. the kernel only recognizes #! as a shebang when it's at offset 0 — a regular # comment on line 1 makes the kernel return ENOEXEC, the parent shell falls back to /bin/sh (dash on Linux CI / GitHub Actions), and set -o pipefail on line 6 immediately blows up because that's not POSIX-sh:
$ head -2 scripts/query-kiro.sh
# Query Kiro billing usage through the authenticated Kiro CLI.
#!/usr/bin/env bash
$ /bin/sh scripts/query-kiro.sh --help
scripts/query-kiro.sh: 6: set: Illegal option -o pipefail
on macOS this happens to work because /bin/sh is bash3 in compat mode, but it breaks on every Linux exec path and breaks scripts/query-all.sh which exec's this file directly. every other scripts/query-*.sh (grok, opencode-go, ...) has the shebang as the literal first line — just swap line 1 and line 2:
#!/usr/bin/env bash
# Query Kiro billing usage through the authenticated Kiro CLI.not a Swift-side blocker so not failing the review on it, but this is the same line that's been called out twice already in rounds 2 and 3 — please get it on line 1 this time.
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Model: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 801c3b1342
ℹ️ 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".
|
|
||
| do { | ||
| try process.run() | ||
| process.waitUntilExit() |
There was a problem hiding this comment.
findBinaryViaLoginShell() uses process.waitUntilExit(), which blocks the task thread and does not cooperate with cancellation. Because ProviderManager.fetchWithTimeout relies on task-group cancellation for timeouts, a hung login shell startup (e.g., slow/blocked shell init scripts) can keep KiroProvider.fetch() stuck indefinitely and prevent the timeout path from completing, stalling provider refreshes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
ngl this is the round where everything actually lands. 5th pass and the one outstanding blocker from rounds 3 and 4 — the scripts/query-kiro.sh shebang on line 2 — is finally on line 1. verified on this exact HEAD (801c3b1):
$ head -2 scripts/query-kiro.sh
#!/usr/bin/env bash
$ scripts/query-kiro.sh --help
Usage: scripts/query-kiro.sh [options]
...
kernel no longer falls back to dash, set -o pipefail works, scripts/query-all.sh can exec it cleanly on Linux CI too. done.
What's still landed correctly across all 5 rounds:
- ✅
KiroProvideritself reads cleanly —withThrowingTaskGroupwith timeout +defer { group.cancelAll(); if process.isRunning { process.terminate() } }handles the subprocess leak path,nonisolated(unsafe) var outputData = Data()matches the repo's documented async Process pattern, andkiro-cli chat --no-interactive "/usage"is the clean invocation (no stdin handshake). - ✅ Overage preservation in both directions: Swift
KiroUsageSnapshot.remainingCredits = totalCredits - usedCredits(no clamp),makeResultpasses the negativeremainingstraight intoProviderUsage.quotaBased, andtestMakeResultPreservesOverageAmountprovesusagePercentage = 105,remainingQuota = -5_000. Python script also matches now (remaining_credits: total - used, no clamp on line 156). - ✅ Parser fixture set actually covers both the classic line format (
Credits (X of Y covered in plan)) and the new TUI table format with progress bar (███ X%),MM/DDshort reset date, and theBonus credits:block.testUsageParserReadsTableOutputWithSlashResetis exactly the regression coverage the bonus credits path needed. - ✅ Plan name normalization order (
PRO+checked beforePROso it doesn't get swallowed),keyboard.badge.eyechosen specifically to avoid the Kimik.circlecollision,minimumFetchInterval = 300friendly to Kiro's side. - ✅ Every integration touch-point this codebase expects:
ProviderIdentifier(rawValue/displayName/shortName/iconName),ProviderManager,CLIProviderManager, both icon views (MultiProviderStatusBarIconView+ModernStatusBarIconView),StatusBarController(icon switch +usagePercentCandidatesmonthly priority),ProviderMenuBuilderKiro submenu with Monthly Credits + Bonus Credits + Plan rows,SubscriptionSettingspresets matching the normalized plan names. - ✅ Docs:
docs/AI_USAGE_API_REFERENCE.mdgot a proper Kiro section 7 with renumbering of Grok→8 and Antigravity→9, plus the test script table entry.
Two tiny FYI items, neither blocking:
- The
Free 50 / Pro 1000 / Pro+ 2000 / Power 10000plan-totals fallback is duplicated acrossKiroProvider.planCreditTotal(Swift) andPLAN_TOTALSinscripts/query-kiro.sh(Python). If Kiro ever changes a plan limit, both need to be updated. There's no clean cross-language share for this so leaving it as-is is fine, just keep them in sync. - If
kiro-cliis installed but not signed in, the parser throwsKiro usage output did not include monthly credit usageinstead of surfacing an auth-style message. Not a blocker — the throw is correct, just a bit cryptic for that specific failure mode. Future improvement could sniff for "sign in" / "login" patterns in the merged stdout+stderr and convert toProviderError.authenticationFailedwith the same string the cold-fetch path uses.
Carried over intentionally not re-raised (consistent with rounds 1–4 conclusions): Process.waitUntilExit() in findBinaryViaWhich / findBinaryViaLoginShell is cold-fetch only and cached upstream; everything from rounds 1–3 (duplicate_imports, --no-interactive, minimumFetchInterval=300, plan-name normalization, TUI parser, overage preservation, icon de-collision, timeout cleanup, docs, query-all.sh entry) is properly landed.
5 rounds of iteration on a single provider is real work, and you actually shipped it without giving up on the shebang fix even after the kernel rules tripped you up three times in a row. nice.
CI Status
SwiftLint✅ passGitHub Actions Lint✅ passBuild🟡 in progress (no regression vs prior HEAD)Build & Test🟡 in progress
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Model: |
There was a problem hiding this comment.
ngl 6th round and this is just a vibes pass — the only thing that changed since the last review is the Kiro status-bar icon swap from the SF symbol keyboard.badge.eye to a custom KiroIcon asset (the Kiro ghost mascot). nothing else moved.
The icon swap itself is wired up cleanly. all four touch-points the codebase expects are updated consistently:
ProviderIdentifier.iconName→"KiroIcon"StatusBarController.providerImage→NSImage(named: "KiroIcon")MultiProviderStatusBarIconView.drawProviderAlert→iconName = "KiroIcon"(thenicon.isTemplate = truefor red-tint alert path)ModernStatusBarIconView.iconView→Image("KiroIcon").renderingMode(.template)with a dedicated SwiftUI branch, andsystemIconName(for: .kiro)correctly returnsnilnow that the custom asset path owns the rendering
Asset catalog is configured right too: Contents.json has template-rendering-intent: template + preserves-vector-representation: true, and the SVG is single-color (fill="#000000") on a transparent background, so template tinting will work in both light/dark mode and on the red-alert tint path in MultiProviderStatusBarIconView. matches exactly how GrokIcon and the other custom brand assets are wired.
no Kimi k.circle collision risk anymore — KiroIcon is its own asset and doesn't overlap with any existing custom icon (ClaudeIcon, CodexIcon, GrokIcon, KimiIcon/k.circle, etc.). the original reason for picking keyboard.badge.eye (collision avoidance) is moot once the custom asset is in.
Everything from rounds 1–5 is still landed correctly:
KiroProviderasync pattern withwithThrowingTaskGroup+ timeout +defer { group.cancelAll(); if process.isRunning { process.terminate() } }for the subprocess leak pathkiro-cli chat --no-interactive "/usage"clean invocation (no stdin handshake)- Overage preserved in both Swift (
KiroUsageSnapshot.remainingCredits = totalCredits - usedCredits, no clamp) and the Python script (remaining_credits: total - usedatscripts/query-kiro.sh:156) - Parser fixture set covers classic line format + new TUI table format with progress bar (
███ X%),MM/DDshort reset date, andBonus credits:block minimumFetchInterval = 300, plan-name normalization order (PRO+beforePRO), shebang on line 1 ofscripts/query-kiro.sh- Docs
AI_USAGE_API_REFERENCE.mdsection 7 +scripts/query-all.shentry
carried over intentionally not re-raised (consistent with rounds 1–5): Process.waitUntilExit() in findBinaryViaWhich / findBinaryViaLoginShell is cold-fetch only per prior reviews; the PLAN_TOTALS duplication between Swift and the Python script stays as documented sync.
6 rounds of iteration on one provider, and you closed every loop without giving up — the shebang odyssey alone was 3 commits worth of kernel-rules learning. clean shipping. approving.
CI Status (HEAD 56864e3)
SwiftLint✅ passGitHub Actions Lint✅ passBuild🟡 in progress (no regression expected — pure asset + icon-name change)Build & Test🟡 in progress
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56864e376b
ℹ️ 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".
| return try await withThrowingTaskGroup(of: String.self) { group in | ||
| let process = Process() | ||
| process.executableURL = binaryPath | ||
| process.arguments = ["chat", "--no-interactive", "/usage"] |
There was a problem hiding this comment.
This command calls kiro-cli chat --no-interactive "/usage", but Kiro documents slash commands as interactive-chat features while --no-interactive is a single-response non-interactive mode. In that mode, /usage can be treated as plain prompt text instead of emitting the structured usage panel output this parser expects, which causes parseUsageOutput to fail and the Kiro provider to surface an error even for authenticated users.
Useful? React with 👍 / 👎.
| found = shutil.which("kiro-cli") | ||
| if found: | ||
| return found |
There was a problem hiding this comment.
Binary discovery here only checks shutil.which in the current process environment plus hardcoded fallback paths, so it misses installs that are only exposed by login-shell startup files (e.g., PATH exported in .zprofile/.bash_profile). In those setups this script incorrectly exits with “kiro-cli not found” despite a valid installation, which breaks local diagnostics and automation using query-kiro.sh.
Useful? React with 👍 / 👎.
✅ AI Code Review CompletedReview finished. Check the PR for inline comments. 📋 View Logs | 🤖 Model: |
There was a problem hiding this comment.
ngl, 7th round on this PR and the one thing the last review explicitly carried over as "not re-raised" — Process.waitUntilExit() blocking in findBinaryViaWhich / findBinaryViaLoginShell — you just went and fixed anyway in commit 29cc25e. that's the kind of follow-through that's actually rare. respect.
What 29cc25e did right:
- Both lookup paths now go through a shared
runLookupProcess(executableURL:arguments:timeout:)helper that mirrors the exact async pattern the codebase already uses forrunKiroUsage:withThrowingTaskGroup+ child task withwithCheckedThrowingContinuation+terminationHandler+ a sibling timeout task that throws after 5s +defer { group.cancelAll(); if process.isRunning { process.terminate() } }for the subprocess leak path. matches the documentedAGENTS.mdrule againstProcess.waitUntilExit()in async contexts. - The Python
scripts/query-kiro.shgot the same login-shell PATH discovery ($SHELL -lc 'command -v kiro-cli') ahead of the hardcoded fallback list, with a 5s timeout and a silentexcept Exception. parity with the Swift discovery order — good. validatedBinaryPath(from:)extracted as a small helper so both lookup methods share the trim + executability check. clean dedup.
Everything from rounds 1–6 is still landed and not regressed:
kiro-cli chat --no-interactive "/usage"invocation (no stdin handshake),nonisolated(unsafe) var outputDatafor the Process readability handler, ANSI strip + non-breaking-space normalization inparseUsageOutput.- Overage preserved end-to-end:
KiroUsageSnapshot.remainingCredits = totalCredits - usedCredits(no clamp),makeResultpasses the negativeremainingstraight intoProviderUsage.quotaBased,testMakeResultPreservesOverageAmountprovesusagePercentage == 105andremainingQuota == -5_000. Python script matches (remaining_credits: total - usedon line 170, no clamp). - Parser fixtures still cover classic line format (
Credits (X of Y covered in plan)) + new TUI table format with progress bar (███ X%) +MM/DDshort reset (rolls year forward when in the past) +Bonus credits:block. - Every integration touch-point:
ProviderIdentifier(rawValue/displayName/shortName/iconName),ProviderManager,CLIProviderManager,StatusBarController(icon switch +usagePercentCandidatesmonthly priority + visible providers list),ProviderMenuBuilderKiro submenu (Monthly Credits + bonus + plan rows),MultiProviderStatusBarIconView+ModernStatusBarIconViewwith the customKiroIcontemplate asset,SubscriptionSettingspresets (Free 0 / Pro 20 / Pro+ 40 / Power 200). - Shebang on line 1 of
scripts/query-kiro.sh,query-all.shentry, docs section 7 with proper renumbering.
Carried over intentionally, still not blocking:
PLAN_TOTALSduplicated betweenKiroProvider.planCreditTotal(Swift) and the Python script — no clean cross-language share. keep them in sync when Kiro changes plan limits.- The "signed-in but no usage line" failure mode still surfaces
Kiro usage output did not include monthly credit usagerather than an auth-style message. cryptic but correct; future improvement could sniffsign in/loginpatterns and remap toProviderError.authenticationFailed. ProviderMenuBuilderKiro case showsCredits Left: -50.00when in overage — accurate per the design ("overage allowed"), but a future polish could flip toCredits Over: 50.00when negative, matching howTableFormatter.formatQuotaMetricsrenders the overage suffix.
7 rounds of iteration, every loop closed, and the optional carry-over got fixed for free. nice work.
CI Status (HEAD 29cc25e)
Build✅ passBuild & Test✅ passSwiftLint✅ passGitHub Actions Lint✅ pass
2 participants
Overview
This PR adds Kiro as a first-class quota provider in OpenCode Bar.
Kiro exposes usage through the authenticated local
kiro-cliexperience rather than a stable public usage API. The provider therefore uses the officially available CLI flow, runs/usagethroughkiro-cli chat --classic, and parses the same billing summary that users can see in Kiro itself. This keeps the integration aligned with Kiro's current product surface while avoiding direct reads of local auth tokens or private account databases.Changes
KiroProvideras a quota-based provider.kiro-clithrough PATH, login shell PATH, common install locations, and the macOS app bundle.kiro-cli chat --classicwith/usage.Validation
KiroProviderTestspass locally.ENABLE_DEBUG_DYLIB=NO.ProviderManagerincludes Kiro in the fetch cycle.KiroProviderfindskiro-clion this Mac./usage.Notes
The provider intentionally does not parse Kiro's local auth database. It uses
kiro-clias the integration boundary so authentication remains owned by Kiro.