fix(responses): repair native exec patch completion before Codex executes - #3701
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughNative Responses ChangesNative exec patch completion
Asynchronous client state probing
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to Native code-mode exec patch envelopes now resolve into executable apply_patch input at completion while unrelated custom payloads and ordinary JavaScript remain unchanged. No concrete merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant ResponsesStream
participant responsesCustomToolRepair
participant CodeModeHelper
ResponsesStream->>responsesCustomToolRepair: Send native exec input deltas
responsesCustomToolRepair->>responsesCustomToolRepair: Hold patch-like input
ResponsesStream->>responsesCustomToolRepair: Send completion event
responsesCustomToolRepair->>CodeModeHelper: Resolve and compile helper input
CodeModeHelper-->>responsesCustomToolRepair: Return executable tool input
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
…responses-patch-5598
|
✅ Deterministic PR hygiene checks passed. |
리뷰 · 우선순위 64 / 80이 PR은 스택 세 번째 층입니다. base는 #3699의 문제: 라우트된 모델이 완전한 apply_patch 봉투를 native code-mode 고친 핵심은 테스트가 lifecycle 경계를 촘촘히 잡습니다. raw/wrapped/pretty/escaped-key native exec, function apply_patch alias, input.done 없는 완성, terminal-only, 임의 JS·미완성 봉투·flat/foreign namespace 보존, failed/incomplete/dispose 시 버퍼 해제와 성공 합성 금지. docs와 방금 경로/심볼 - 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
…responses-patch-5598
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Summary
Repair a complete patch mistakenly emitted as native custom
execbefore Codex receives input/item completion events, matching the terminal snapshot and Chat bridge behavior.Hold helper-alias and ambiguous raw/JSON-wrapped patch previews until their executable form is known. Pretty-printed and escaped-key wrappers follow the same rule.
Preserve unrelated native custom payloads and arbitrary JavaScript byte-for-byte; block-leading JavaScript may wait until completion.
Stack layer 3, depends on fix(clients): converge owned Pi and Aside catalogs after selection changes #3699 and fix(clients): honor selected models in Pi and Aside catalogs #3698. A final layer will address the remaining ordinary-function and namespace parity gaps.
Replace a blocking test subprocess wait that twice stalled macOS CI with an asynchronous wait, a kill deadline, and bounded cleanup. The same state and exact-PID reaping assertions remain; this follow-up changes only the test helper.
Verification
Standalone lifecycle probe verifies held previews, identical input.done/item.done/terminal input, and exactly one canonical patch passed to a recording stub when generated code is executed. No real files were patched by the probe.
Added native raw/wrapped/pretty/escaped-key, alias, completion-variant, unrelated-payload, flat-catalog, foreign-namespace and cleanup regressions.
Remote focused regression verification and independent review are running. Hosted exact-head CI remains required before merge; no local suites or typecheck were executed.
Owner authorized
--no-verifypushes and admin merge after verification.The asynchronous probe file passed all 20 cases under pinned Bun 1.4 with
--isolateon macmini-cf, including the stalled-child kill/reap case; root typecheck and independent source review passed.Checklist