fix(kiro): accept Codex parallel tool permission - #2309
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughKiro now accepts ChangesKiro parallel-tool hint handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change is localized to accepting a client hint while preserving serialized behavior, and the reported tests and checks pass; no actionable merge-blocking risk remains beyond normal review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
|
Exact-head CI is fully green on |
리뷰 · 우선순위 70 / 80지금 핵심. 현재 테스트. 기존 루프에서 구멍. (1) 카탈로그를 true로 올리지 말 것. 올리면 Codex가 진짜 병렬 배치를 기대함. Kiro는 직렬. (2)
해결방안: 이 패치로 가라. throw 삭제만. 카탈로그 비트와 Kiro 와이어는 손대지 말 것. 이 댓글은 grok-bot이 작성했습니다 |
|
Thanks for the fast turnaround on this. The adapter change and the removed validation-loop entry match what I had locally. The regression test here is stronger: explicitly asserting the absence of any For reference, the fix I was running locally before this PR was narrower: function validateKiroCapabilities(parsed: OcxParsedRequest): void {
const choice = parsed.options.toolChoice;
if (choice !== undefined && choice !== "auto" && choice !== "none") {
throw new Error("Kiro supports only automatic tool choice or tool_choice:none");
}
- if (parsed.options.parallelToolCalls === true) {
- throw new Error("Kiro does not support parallel tool calls");
- }
+ // Responses defines parallel_tool_calls as permission, not a requirement. Kiro's wire has no
+ // corresponding request field and remains serialized, so clients may safely send `true` here.
if (parsed.options.serviceTier !== undefined) {Same deletion, inline comment instead of the architecture-doc section + decision log. Your version is cleaner — the adapter stays comment-free and the reasoning lives where maintainers actually look for contract decisions. One additional data point from our investigation: upstream Codex commit openai/codex@86b1123 went further by removing Applied this PR's exact changes locally (all four files), focused tests pass (55/0, 263 expects), and confirmed the fix works end-to-end with Codex CLI 0.149.0 against a live Kiro endpoint — tool turns stream normally with no regression. |
|
Exact-head CI remains fully green on |
|
Updated onto the current dev tip 69907dd (including #2312) at exact head 1d5d935. The merge was conflict-free. The Grok-reviewed scope is unchanged: remove only the local rejection of the permissive parallel_tool_calls hint; do not advertise parallel capability and do not add a Kiro wire field. Exact-head verification passed 55/55 Kiro tests, typecheck, privacy scan, diff check, and the 393-page docs build. Fresh CI is running. This remains pending independent maintainer approval; I will not self-approve or self-merge. |
|
Exact-head CI is fully green on 1d5d935 after the current-dev update, including Cross-platform CI, macOS, all Linux shards, npm-global smokes, React Doctor, hygiene, privacy/type gates, and target enforcement. There are no unresolved review threads. @lidge-jun @Wibias, this remains ready for independent review; I will not self-approve or self-merge. |
011 records work-phase 1: four green PRs merged (lidge-jun#2309, lidge-jun#2339, lidge-jun#2335, lidge-jun#2313), lidge-jun#2359 held on a reproduced test failure, a correction to 001 (dev IS protected, by rulesets rather than classic branch protection), and an honest incident record of a hard reset that dropped an unpushed commit and how it was recovered. 090 records work-phase 9, the four PRs that arrived mid-loop. lidge-jun#2361 merged; lidge-jun#2362, lidge-jun#2363 and lidge-jun#2364 left open with their blockers restated. Two of those verdicts rest on falsification rather than diff reading: lidge-jun#2363's tests still pass with its real call site deleted, and lidge-jun#2364's second commit deleted the management validation its first commit added. It also records a CodeRabbit finding that was dismissed as wrong on the evidence.
3 participants
Summary
parallel_tool_calls: trueas a permissive client hint instead of rejecting the Kiro requestFixes #2308
Validation
bun test tests/kiro-adapter.test.ts: 55 passed, 0 failedThe repository-wide monolithic run also exercised the patch broadly, but produced unrelated environment/flaky failures (missing isolated GUI dependencies before they were linked and one request-pacing timing miss). Those exact files passed on the clean rerun above.
Exact-head CI is green. The PR is ready for independent maintainer review and remains unmerged until that approval.
Review readiness checklist
Summary by CodeRabbit
Bug Fixes
Documentation