Explain Kiro CLI requirement when no fetch strategy is available - #2465
Explain Kiro CLI requirement when no fetch strategy is available#2465hxy91819 wants to merge 1 commit into
Conversation
7cbdd58 to
a96e849
Compare
![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: 7cbdd58b06
ℹ️ 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".
| try await fetcher.fetchResolvedCookie(logger: logger) | ||
| }, | ||
| clearCached: { cached in | ||
| _ = CookieHeaderCache.clearIfCurrent(provider: .ollama, expected: cached) |
There was a problem hiding this comment.
When the cached Ollama cookie expires while another refresh or login has already replaced the cache, clearIfCurrent returns false, but this path discards that result and fetchAutomatic continues to import and store a browser cookie unconditionally. In that race the older in-flight refresh can overwrite the newer cache entry, switching the app back to a stale or different browser session; treat a failed conditional clear as a retry/abort before allowing storeResolved to run.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Codex review: needs real behavior proof before merge. Reviewed July 26, 2026, 9:48 PM ET / July 27, 2026, 01:48 UTC. ClawSweeper reviewWhat this changesThe PR gives Kiro a provider-specific unavailable-strategy message that tells users to install and log in to the Kiro CLI, and adds a focused regression test. Merge readiness⛔ Blocked until real behavior proof from a real setup is added - 3 items remain Keep this PR open. Current main still emits the generic unavailable-strategy diagnostic, while the project’s Kiro documentation confirms the local CLI is the supported collection path; the patch is focused and appears correct, but it needs redacted after-fix behavior proof from a real Kiro setup before merge. Priority: P3 Review scores
Verification
How this fits togetherCodexBar gathers usage through provider-specific fetch strategies and turns probe failures into diagnostics shown by the menu-bar app. Kiro’s usage provider relies on the locally installed Kiro CLI, so an unavailable strategy currently stops the usage probe before it can return account data. flowchart LR
A[Enabled Kiro provider] --> B[Provider fetch plan]
B --> C{Kiro CLI resolved?}
C -->|Yes| D[Run Kiro usage probe]
C -->|No| E[Unavailable strategy error]
E --> F[Menu bar diagnostic]
D --> G[Usage status]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Preserve the generic message for other providers, land the Kiro-specific prerequisite guidance after a redacted real missing-CLI or logged-out-path capture confirms the exact diagnostic, and retain the focused regression test. Do we have a high-confidence way to reproduce the issue? Yes, from source. Configure Kiro and make Is this the best way to solve the issue? Yes, conditionally. A Kiro-only message preserves the generic contract for other providers and aligns with the documented CLI requirement; the remaining need is real behavior proof that this is the message shown for the intended failure path. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against cc8da27cec92. LabelsLabel justifications:
EvidenceWhat 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
|
1 participant
Problem
When Kiro is enabled but
kiro-cliis not installed or is not discoverable, CodexBar only reports:No available fetch strategy for kiro.This is not actionable for users. Kiro usage is currently collected exclusively through the local Kiro CLI, which also requires the user to complete
kiro-cli login(including enterprise/IAM login flows).Cause
The Kiro fetch strategy advertises itself as available only when
kiro-clican be resolved. If it is unavailable, the genericProviderFetchError.noAvailableStrategymessage is surfaced without provider-specific guidance.Solution
Add a Kiro-specific error message that explains the CLI requirement, links to the Kiro CLI documentation, and tells the user to run
kiro-cli login.Other providers retain the existing generic error message.
Validation
swift test --filter KiroStatusProbeTests(56 tests passed)make check(passed; SwiftFormat and SwiftLint reported no violations)