fix(kiro): run kiro-cli through a PTY so usage loads (fixes #1619)#1744
fix(kiro): run kiro-cli through a PTY so usage loads (fixes #1619)#1744sf-jin-ku wants to merge 2 commits into
Conversation
kiro-cli 2.8.1 (an Amazon Q Developer CLI fork) performs terminal setup on startup and emits no output at all when launched without a controlling terminal. The probe launched `whoami`, `chat --no-interactive /usage`, and `/context` over plain pipes, so every command hung until the 20s deadline and surfaced as "Kiro CLI timed out." — even though the same commands return in ~2-4s from a real terminal. Route every kiro-cli invocation through TTYCommandRunner (PTY), matching the Codex/Claude status probes. The existing parser handles the PTY output unchanged. The now-orphaned `isUsageOutputComplete` helper is removed; the pipe-based `runCommand` is left in place (still covered by its own tests). steipete#1627 only made /usage independent of the slow account probe and was never validated against real kiro-cli, so the timeout persisted on 2.8.1. Fixes steipete#1619. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Codex review: needs maintainer review before merge. Reviewed June 24, 2026, 3:46 PM ET / 19:46 UTC. Summary Reproducibility: yes. from source and contributor live proof. Current main uses pipe-backed Kiro Review metrics: 2 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Merge the focused PTY transport fix if maintainers accept the small Kiro failure-classification tradeoff, and handle exact exit-status preservation later if support diagnostics need it. Do we have a high-confidence way to reproduce the issue? Yes from source and contributor live proof. Current main uses pipe-backed Kiro Is this the best way to solve the issue? Likely yes. Reusing the existing PTY runner is the narrowest maintainable fix for a terminal-presence bug; preserving exact nonzero exit-status classification is a diagnostic refinement rather than a clear blocker. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against ada3660e9d61. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
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. How this review workflow works
|
![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: 4ac29c3751
ℹ️ 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".
clawsweeper
Bot
added
proof: sufficient
The PTY runner cannot surface the child exit status, so a non-zero `kiro-cli whoami` that prints a non-login error (config/network) would be parsed as a logged-in account with empty fields. Reject output carrying no account markers so the best-effort account probe reports it as unavailable. Addresses Codex review feedback on steipete#1744. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Summary
Kiro usage fails with "Kiro CLI timed out." on
kiro-cli2.8.1, even though the same commands return in seconds from a terminal. Fixes #1619.Root cause:
kiro-cli(an Amazon Q Developer CLI fork) performs terminal setup on startup and emits no output at all when launched without a controlling terminal. The probe ranwhoami,chat --no-interactive /usage, andchat --no-interactive /contextover plain pipes (SpawnedProcessGroup). In a GUI process there is no TTY, so every command blocks producing zero bytes and the probe hits its 20s deadline.This is purely a TTY-presence problem — independent of
PATH/HOME/env. Reproduced viaposix_spawnwith a minimal environment; the binary only produces output when a controlling terminal is present.Fix: route every
kiro-cliinvocation throughTTYCommandRunner(PTY), exactly like the Codex (/status) and Claude status probes already do. The existing parser handles the PTY output unchanged.Why #1619 was still broken after #1627
#1627 only made
/usagereturn independently of the slow optionalwhoamiaccount probe, on the theory that the account probe was blocking the fetch. As its author noted on the PR, it could not be validated because that machine had nokiro-cliinstalled. The real cause is that all kiro-cli commands (includingwhoamiand/usagethemselves) emit nothing over a bare pipe — so the timeout persisted on 2.8.1 onmain.Changes
KiroStatusProbe: addrunViaPTY(...)and pointensureLoggedIn/runUsageCommand/fetchContextUsageat it; mapTTYCommandRunnererrors to the existingKiroStatusProbeErrorcases.isUsageOutputCompletehelper.runCommand/SpawnedProcessGroupare left untouched (still covered by their ownrun command*tests). They are now production-unused and can be removed in a separate cleanup PR.KiroStatusProbeTests: thefetch ... inherited pipes opentest asserted pipe-holder cleanup that the PTY runner does not perform (and does not need — realkiro-clicleans up under the PTY). Reframed it as "returns promptly when usage helper spawns a detached child"; the pipe-holder-kill capability is still directly covered byrun command kills a pipe holder that escapes the process group.Verification
Commands run:
swift test --filter KiroStatusProbeTests— 40 passedmake check— 0 violations, 0 serious (1180 files)KiroStatusProbe().fetch()(real kiro-cli 2.8.1): returns in ~3-4s with full data, 0 residual processes across repeated runs.usedPercent: 100,resetsAt: 2026-07-01) instead of the timeout error.🤖 Generated with Claude Code