Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (6)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughCodeBuddy tool bridging now applies strict indexed-block matching, validates buffered arguments before implicit closure, and rejects unattributable fragments. Turn limits are enforced when blocks open. Tests and documentation cover these cases. ChangesCodeBuddy tool-block capture
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Possibly related PRs
Merge Risk: ⚪ Minimal · up to The reviewed tool-block handling has no established merge-blocking defect; it is ready for normal merge checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The reviewed paths reject ambiguous or incomplete tool calls earlier and apply the call limit before buffered calls are forwarded. No newly introduced security concern was established, but the assessment cannot cover every downstream consumer or independently compare the full previous implementation. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 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. |
리뷰 · 우선순위 62 / 80이 PR은 CodeBuddy가 도구를 여러 개 부를 때, 인자가 다 쓰이기 전에 그 호출을 끝내지 못하게 막습니다. #5945 다음에 오는 보강입니다. 베이스는 같은 번호로 다음 도구가 시작되면, 앞에 열린 도구의 인자가 완전한 JSON 객체일 때만 그 도구를 닫습니다. 글자가 없거나 한 턴의 도구 상한은 16개입니다. 세는 시점은 도구가 열리는 순간입니다. 17번째가 열리면 이 검사는 도구 다리가 켜진 CodeBuddy 턴에만 붙습니다. Qoder와 Claude CLI는 그 다리를 넘기지 않습니다. 라인 - 라인 - 메인테이너의 판단이 필요한 지점 번호 없는 종료는 턴을 실패로 끝냅니다. 번호 없는 인자 조각은 조용히 사라집니다. 둘을 같은 실패로 볼지 정해야 합니다. 인자가 한 조각도 없는 종료는 빈 도구 호출로 남습니다. 주석은 이것을 CLI의 기존 표현이라고 합니다. 처음부터 인자가 없는 도구와, 번호가 없어서 조각이 사라진 도구를 어떻게 가를지 정해야 합니다. 너의 추천 엄격 모드에서는 번호 없는 이 댓글은 grok-bot이 작성했습니다 |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
|
Early draft blocker on exact head Please fail the capture on an unresolvable argument delta and add this exact regression: INIT → START(index 2/id A) → nonempty |
|
Re-review on exact head |
|
Landed on |
Carried from lidge-jun#6022 into merge train round 3. Co-authored-by: mdwsk88 <924038395@qq.com>
…ge-jun#6022 dev's lidge-jun#6022 hardened the coding-agent capture path and moved the per-turn tool-call limit from the emitted tool_call_start to the block open, because the parser buffers a block until its stop. This adapter reads the same parse state through its own capture-only bridge, so both halves had to follow. The state now sets strictToolBlockCapture for a turn with a bridge, which is what makes the parser refuse incomplete JSON arguments, refuse a delta that belongs to no open block, and treat a same-index start as an implicit stop only once the previous arguments are complete. The cap is checked when toolBlockStarts grows instead of when the buffered start is finally emitted, so a stream that only opens blocks is bounded at the open rather than after it parks. The added test opens more blocks than the cap allows without closing one: without the move it ran on to message_stop and failed there for a different reason.
…ge-jun#6022 dev's lidge-jun#6022 hardened the coding-agent capture path and moved the per-turn tool-call limit from the emitted tool_call_start to the block open, because the parser buffers a block until its stop. This adapter reads the same parse state through its own capture-only bridge, so both halves had to follow. The state now sets strictToolBlockCapture for a turn with a bridge, which is what makes the parser refuse incomplete JSON arguments, refuse a delta that belongs to no open block, and treat a same-index start as an implicit stop only once the previous arguments are complete. The cap is checked when toolBlockStarts grows instead of when the buffered start is finally emitted, so a stream that only opens blocks is bounded at the open rather than after it parks. The added test opens more blocks than the cap allows without closing one: without the move it ran on to message_stop and failed there for a different reason.
…ge-jun#6022 dev's lidge-jun#6022 hardened the coding-agent capture path and moved the per-turn tool-call limit from the emitted tool_call_start to the block open, because the parser buffers a block until its stop. This adapter reads the same parse state through its own capture-only bridge, so both halves had to follow. The state now sets strictToolBlockCapture for a turn with a bridge, which is what makes the parser refuse incomplete JSON arguments, refuse a delta that belongs to no open block, and treat a same-index start as an implicit stop only once the previous arguments are complete. The cap is checked when toolBlockStarts grows instead of when the buffered start is finally emitted, so a stream that only opens blocks is bounded at the open rather than after it parks. The added test opens more blocks than the cap allows without closing one: without the move it ran on to message_stop and failed there for a different reason.
…ge-jun#6022 dev's lidge-jun#6022 hardened the coding-agent capture path and moved the per-turn tool-call limit from the emitted tool_call_start to the block open, because the parser buffers a block until its stop. This adapter reads the same parse state through its own capture-only bridge, so both halves had to follow. The state now sets strictToolBlockCapture for a turn with a bridge, which is what makes the parser refuse incomplete JSON arguments, refuse a delta that belongs to no open block, and treat a same-index start as an implicit stop only once the previous arguments are complete. The cap is checked when toolBlockStarts grows instead of when the buffered start is finally emitted, so a stream that only opens blocks is bounded at the open rather than after it parks. The added test opens more blocks than the cap allows without closing one: without the move it ran on to message_stop and failed there for a different reason.
dev's lidge-jun#6081/lidge-jun#6083 moved the ceiling and the budget for coding-agent tool state into the shared parser, which is the same state this adapter reads through its capture-only bridge. Two halves had to follow, the same class as lidge-jun#6022 and lidge-jun#5945 before them. The parser now enforces its ceiling (16 starts, or a bridge's tighter limit) before it allocates the block and reports the refusal through `toolCallLimitExceeded`; the parse state carries the bridge's limit for that. This adapter only compared `toolBlockStarts` after the fact, so a start the parser refused before allocation left no trace: the dropped call never entered `toolBlockStarts`, the completeness invariants therefore saw starts and completions as equal, and a turn could end as a successful completion with a call missing. The parser also charges retained tool identity and argument fragments to the request's translator budget and releases them on close, EOF, protocol error and abort. The state now carries `incoming.translatorBudget`, `releaseOpenToolBlocks(state)` runs on every exit path, and a budget overflow keeps its own `translation_buffer_limit` code instead of being flattened into the generic SDK error the surrounding branch reports. Two tests cover it: a turn without a bridge that passes the shared ceiling fails closed, and a retained argument past a small per-call budget surfaces the budget's code. Reverting the adapter fails four tests, those two and the two that pinned the cap before, and the existing cap tests now pass through the parser's own admission rather than a parallel count.
3 participants
Summary
input_json_deltacannot be attributed to an open tool block, so a later indexed stop cannot normalize the loss into an empty{}call.tool_call_start.Verification
bun test tests/providers/codebuddy-protocol.test.ts tests/providers/codebuddy-tool-bridge-turn.test.ts— 68 pass, 0 fail.bun run typecheck— pass.cd docs-site && bun install --frozen-lockfile && bun run build— pass, 537 pages built.git diff --check upstream/dev..HEAD— pass.bun run testin a clean checkout outside~/.codex— exit 1. 32134 pass / 43 skip / 26 fail / 3 errors in the parallel lane, plus 1 serial-lane failure. Failures are 20 DNS fake-IP environment failures, 3 failures because an existing OpenCodex service is already listening on port 10100, 1 macOS concurrency timeout, and 3 load/order flakes that pass when their files are rerun alone. None are attributable to this PR.Checklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
Required local validation passed; commands, results, and any full-suite exception are documented.
I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit