Skip to content

fix: restore Kiro usage refresh across CLI transports#2012

Merged
steipete merged 3 commits into
steipete:mainfrom
txarly89:agent/fix-kiro-pty-timeout
Jul 12, 2026
Merged

fix: restore Kiro usage refresh across CLI transports#2012
steipete merged 3 commits into
steipete:mainfrom
txarly89:agent/fix-kiro-pty-timeout

Conversation

@txarly89

@txarly89 txarly89 commented Jul 9, 2026 •
edited by steipete
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Prefer semantically validated pipe-backed Kiro CLI probes for current releases.
  • Retain a same-deadline PTY fallback for older or terminal-required Kiro CLIs.
  • Make stdout/stderr, cancellation, process cleanup, and app-shutdown ownership deterministic.
  • Add focused regressions for pipe, PTY, partial output, auth, deadlines, cancellation, and process cleanup.

Closes #1883.

Root cause

Current Kiro CLI releases can return normally through pipes but stall when launched directly under a PTY. Older releases had the inverse behavior and required a terminal. The original candidate retried PTY only after a thrown pipe timeout, so incomplete or noisy pipe output could bypass the fallback, and each transport could consume a fresh timeout budget.

Implementation

  • Validate pipe results per command before accepting them; incomplete output falls through to PTY.
  • Keep authentication failures authoritative while preventing failed transports from preempting a viable peer.
  • Use one monotonic deadline across a staggered pipe/PTY race, cleanup, and result selection.
  • Give pipe launches the same login-shell-enriched environment used by PTY launches so GUI refreshes resolve the same CLI.
  • Combine stdout and stderr deterministically and drain both concurrently.
  • Fully terminate and reap pipe roots, descendants, and output holders before starting PTY.
  • Register pipe PIDs and process groups with the app-shutdown registry.
  • Preserve cancellation before launch, between transports, during cleanup, and before return.

Validation

  • Exact head: 7846f59bc3ebebd87ff8432b8d016ca1e61f8b6f, rebased onto current main after fix: isolate interactive PATH probes from caller terminal #2074.
  • Exact-head focused Kiro matrix: 63/63 passed across four suites, including pipe-only current CLI, PTY-only legacy CLI, slow-pipe/PTY races, failed-transport authority, authentication rejection, shared deadlines, process cleanup, app shutdown, and cancellation.
  • Exact-head make check: SwiftFormat and SwiftLint clean.
  • Exact-head full local suite: 615 selections in 52 groups; every group passed first attempt, with zero retries, recoveries, or timeouts.
  • Final autoreview: clean, no accepted/actionable findings; 0.86 correctness confidence.
  • Controlled live-process fixtures exercised actual pipes, PTYs, child processes, cancellation, signals, and deadline cleanup. No fixture process survived its test.
  • No dependency changes.

Live-provider limitation

This machine does not have kiro-cli installed or an approved logged-in Kiro account, so a real authenticated provider request was intentionally not attempted. The transport state machine and OS-process behavior are covered by the exact-head integration matrix; contributor and maintainer authorization to land this repair waives the unavailable account-dependent proof.

@clawsweeper

clawsweeper Bot commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed July 11, 2026, 10:26 PM ET / July 12, 2026, 02:26 UTC.

Summary
The PR changes Kiro probing to use semantically validated pipe execution with a shared-deadline PTY fallback, deterministic process cleanup, and extensive transport regressions.

Reproducibility: yes. The canonical report shows a repeatable CodexBar timeout with successful standalone commands, and exact-head real-process fixtures reproduce both conflicting transport requirements; authenticated live confirmation is a proof gap rather than a reproduction gap.

Review metrics: 3 noteworthy metrics.

  • Patch surface: 6 files, +1,344/-61. The provider-scoped repair includes a substantial process-lifecycle rewrite.
  • Focused validation: 63/63 Kiro cases reported passing. The matrix covers pipe-only and PTY-only CLIs, result authority, deadlines, cancellation, shutdown, and cleanup.
  • Reported CLI coverage: Kiro CLI 2.10–2.12. Independent reports and the real logged-out check span multiple released versions affected by the transport mismatch.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #1883
Summary: This PR is the active candidate fix for the canonical Kiro timeout issue; the closed timeout report shares the same transport root cause, while the PATH-isolation PR is adjacent infrastructure.

Members:

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

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦐 gold shrimp
Patch quality: 🦞 diamond lobster
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Post redacted authenticated exact-head packaged-helper refresh evidence, or obtain an explicit maintainer proof waiver.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR has strong real-process fixtures and a logged-out real Kiro CLI check, but no authenticated after-fix quota refresh through the exact packaged helper; added proof should redact accounts, credentials, endpoints, and other private details. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Authenticated Kiro quota and context refresh through the exact packaged helper remains unverified unless a maintainer explicitly waives it.
  • [P2] The compatibility repair must retain coverage for both pipe-compatible current CLIs and terminal-required older CLIs; removing either path could restore the timeout for an existing user cohort.

Maintainer options:

  1. Prove authenticated compatibility (recommended)
    Run exact head through the packaged helper with an authenticated Kiro CLI and provide redacted successful refresh, parsing, timing, and cleanup evidence.
  2. Accept process-backed proof
    A maintainer may explicitly waive the authenticated run and own the residual account-dependent compatibility uncertainty.

Next step before merge

  • [P1] The implementation review is clean; a maintainer must require or explicitly waive the unavailable authenticated-provider proof before merge.

Maintainer decision needed

  • Question: Is the controlled real-process pipe/PTY matrix plus logged-out real Kiro CLI validation sufficient to waive an authenticated packaged-helper refresh for exact head 7846f59?
  • Rationale: The implementation and OS-process behavior are strongly validated, but the available environment has no approved logged-in Kiro account, so only a maintainer can accept the remaining account-dependent uncertainty.
  • Likely owner: steipete — He owns the relevant current-main history, authored the final repair, and explicitly identified this remaining merge gate.
  • Options:
    • Require authenticated proof (recommended): Obtain a redacted successful quota and context refresh through the exact packaged helper on a logged-in current Kiro CLI.
    • Waive provider proof: Explicitly accept the real-process compatibility matrix and logged-out real-CLI validation as sufficient for merge.

Security
Cleared: No dependency, permission, secret, or supply-chain changes were found; execution remains limited to the existing locally resolved Kiro CLI with stricter shutdown ownership.

Review details

Best possible solution:

Land the dual-transport repair after a redacted authenticated packaged-helper run or an explicit maintainer waiver, while retaining both transport-mode process fixtures as regression coverage.

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

Yes. The canonical report shows a repeatable CodexBar timeout with successful standalone commands, and exact-head real-process fixtures reproduce both conflicting transport requirements; authenticated live confirmation is a proof gap rather than a reproduction gap.

Is this the best way to solve the issue?

Yes. The final design gives a viable pipe the full deadline, starts PTY opportunistically, semantically selects results, uses the enriched environment, and drains structured cancellation and cleanup without removing legacy compatibility.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: Affected users cannot refresh Kiro usage even though the Kiro CLI succeeds standalone.
  • merge-risk: 🚨 compatibility: The rewrite must preserve both current pipe-compatible and older terminal-required Kiro CLI behavior.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦞 diamond lobster.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR has strong real-process fixtures and a logged-out real Kiro CLI check, but no authenticated after-fix quota refresh through the exact packaged helper; added proof should redact accounts, credentials, endpoints, and other private details. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

What I checked:

Likely related people:

  • steipete: Current-main history connects him to the Kiro probe, and he authored both final hardening commits, performed the exact-head validation, and identified the remaining proof gate. (role: feature-history owner and recent repair author; confidence: high; commits: 7ddcf28b0442, 86d3f2e1ff2b, 7846f59bc3eb; files: Sources/CodexBarCore/Providers/Kiro/KiroStatusProbe.swift, Tests/CodexBarTests/KiroStatusProbeTests.swift, Tests/CodexBarTests/KiroTransportRaceTests.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • 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.
Review history (4 earlier review cycles)
  • reviewed 2026-07-09T19:57:04.387Z sha 83dc40b :: needs real behavior proof before merge. :: [P1] Retry PTY after unusable pipe output
  • reviewed 2026-07-12T01:37:44.142Z sha 86d3f2e :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-12T01:44:14.638Z sha 86d3f2e :: needs real behavior proof before merge. :: [P1] Let pipe-only CLIs use the documented timeout | [P1] Launch pipe probes with the enriched PATH
  • reviewed 2026-07-12T02:19:45.128Z sha 7846f59 :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 9, 2026
@steipete steipete force-pushed the agent/fix-kiro-pty-timeout branch from 83dc40b to 5787a81 Compare July 9, 2026 23:13
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Maintainer repair pushed at exact head 5787a81abee53158cb5c5ef1a0c5fe3d4a5c4e0c.

Proof completed:

  • focused Kiro suite: 54/54 passed
  • make check: passed
  • full local suite: all 50 shards passed
  • exact-diff review: clean
  • signed packaged helper: strict signature + Gatekeeper accepted; bundle commit 5787a81a
  • source-blind matrix: 3 pipe-only + 3 terminal-required runs with varied values, noisy stderr, auth rejection, one-deadline hang cleanup; all expected behaviors passed and no process remained
  • real Kiro CLI 2.12.0: version and logged-out whoami behavior verified without starting login or touching credentials
  • Public Model Identifier Gate: PASS; dependencies unchanged

Remaining merge gate: this machine has no approved logged-in Kiro account. @txarly89, if you have an authenticated Kiro setup, please run this exact head through the freshly packaged helper and share a redacted positive result (helper/bundle commit, Kiro CLI version, three refresh timings, parsed quota/context presence, and confirmation that no Kiro process remains). Do not include account or credential values. Otherwise a maintainer must explicitly waive that final provider-access proof.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

txarly89 and others added 2 commits July 12, 2026 02:29
@steipete steipete force-pushed the agent/fix-kiro-pty-timeout branch from 5787a81 to 86d3f2e Compare July 12, 2026 01:32
@steipete steipete changed the title Fix Kiro CLI timeout fallback fix: restore Kiro usage refresh across CLI transports Jul 12, 2026
@steipete steipete marked this pull request as ready for review July 12, 2026 01:32
@steipete

steipete commented Jul 12, 2026

Copy link
Copy Markdown
Owner

@clawsweeper re-review

Maintainer repair is complete on exact head 86d3f2e1ff2b0e5ce642ddaedcddf8314ae83da6: pipe-first semantic validation, same-deadline PTY fallback, deterministic stream draining and process cleanup, 54 focused transport/deadline/cancellation regressions green, and make check clean. Exact-head full local and hosted suites are running.

@clawsweeper

clawsweeper Bot commented Jul 12, 2026 •
edited
Loading

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Jul 12, 2026
View reviewed changes

@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: 86d3f2e1ff

ℹ️ 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".

Comment thread Sources/CodexBarCore/Providers/Kiro/KiroStatusProbe.swift Outdated
{
let clock = ContinuousClock()
let deadline = clock.now.advanced(by: .seconds(max(0, timeout)))
let pipeBudget = min(max(0, self.pipeTimeoutCap), max(0, timeout / 2))

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 12, 2026

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 Allow pipe-only CLIs to use the full timeout

When a current Kiro CLI only works through pipes, this caps the only usable transport to 5 seconds of the 20-second usage deadline. If kiro-cli chat --no-interactive /usage has a cold start or network delay and produces its first output after 5 seconds but before the documented 20-second timeout, runViaPipe kills it and the PTY fallback is exactly the transport that stalls, so refresh still fails even though the command would have completed within the configured deadline.

Useful? React with 👍 / 👎.

Comment thread Sources/CodexBarCore/Providers/Kiro/KiroStatusProbe.swift Outdated
let stdoutPipe = Pipe()
let stderrPipe = Pipe()

var env = ProcessInfo.processInfo.environment

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 12, 2026

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 enriched PATH for pipe launches

In the macOS app environment, PATH often lacks Homebrew/npm locations; cliBinaryResolver can still find kiro-cli via the login-shell PATH, but launching the resolved shim with raw ProcessInfo means interpreters such as #!/usr/bin/env node cannot be found. For current Kiro versions that need the pipe transport, the subsequent PTY fallback may stall, so these installs never refresh usage. Build this environment with the same enriched PATH used by TTYCommandRunner.

Useful? React with 👍 / 👎.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. P1 Urgent regression or broken agent/channel workflow affecting real users now. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. P2 Normal priority bug or improvement with limited blast radius. labels Jul 12, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 12, 2026
@steipete

steipete commented Jul 12, 2026

Copy link
Copy Markdown
Owner

Maintainer verification at exact head 7846f59bc3ebebd87ff8432b8d016ca1e61f8b6f:

  • swift test --filter Kiro: 63 selections across 4 suites passed.
  • make test: 615 selections across 52 groups passed on the first attempt, with zero retries, recoveries, or timeouts.
  • make check: passed.
  • Maintainer autoreview: clean.
  • Controlled live-process validation exercised the pipe/PTY race, deadline handling, failed-PTY fallback, and cancellation behavior against real child processes.

No real Kiro account was available, so account-authenticated refresh was not exercised. The transport behavior and parser outcomes are covered by the focused regressions and the process-level smoke proof above.

Hosted CI passed lint, Linux x64/arm64, and macOS shard 1. macOS shard 0's only failure was the unrelated Kimi timing assertion measuring 253.38 ms against a strict 250 ms threshold; the failed exact-head shard was rerun before landing.

@steipete steipete merged commit 237fa70 into steipete:main Jul 12, 2026
12 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

Assignees

No one assigned

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Kiro timeout

2 participants

@txarly89 @steipete