Repository navigation
Add exact turn steering and API-key attention waits - #5499
Conversation
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. |
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved at 95d38b9.
Reviewed exact steering ownership, stale-target races, per-input identities, mode mixing, passive attention waits, sparse rate-limit parsing, sensitive-header filtering, and HTTP/SSE/WebSocket/error propagation. Required native helpers exist in the pinned dependency; no dependency on later layers found. No actionable findings. Hosted lint/docs and non-Rust CodeQL passed; native Linux/macOS/Windows and Rust CodeQL remain pending.
Static review only; no candidate code or tests run. Leave unmerged as requested.
95d38b9 to
1863681
Compare
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved at 1863681.
Reviewed the exact tree changes with an independent reviewer. The new steer_exact method requires and validates the native turn ID before submission; legacy steer retains its original signature and behavior. Updated regression cases and the public API fixture agree. The macOS native CI timeout change is sound. No actionable findings.
Static review only; no candidate code or tests run. Fresh hosted CI remains pending. Leave unmerged as requested.
1863681 to
f52af83
Compare
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-reviewed the control layer, including interrupted-child attention and repair rate-limit propagation. No remaining actionable finding in this layer.
Static review only; no candidate code or tests executed. Hosted CI: pending: native-linux / Native package (ubuntu-24.04), native-macos / Native package (macos-latest); no failed checks observed.
f52af83 to
f23f470
Compare
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-reviewed the exact updated commit. The API-contract snapshot now matches NativeError._interrupted. No new actionable findings in the exact change since the prior review.
Static review only; no candidate code or tests executed locally. Hosted CI: pending checks remain; no failures observed.
c50f179 to
eb24907
Compare
f23f470 to
5cde9ce
Compare
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-reviewed the exact change from the previously reviewed commit: the only change raises the macOS native test step from 10 to 20 minutes. This addresses the observed hosted timeout without changing test assertions or execution behavior. No new findings.
Reviewed commit: 5cde9cecfb990b363bd2e2937cee1d61b9417759. Static review only; no candidate code or tests executed locally. Hosted CI is still running: native-linux / Native package (ubuntu-24.04), native-macos / Native package (macos-latest). No current hosted failures observed.
eb24907 to
c81aec7
Compare
5cde9ce to
607c06b
Compare
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved at 607c06b. Reviewed the incremental Linux/Windows runner routing and matching change-detection assertions against the previously reviewed commit; no new correctness or security findings. Runtime code is unchanged. Hosted CI is still pending. Static review only; no candidate code or local tests executed.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
The executor fixture now lets the child select port zero and reads its bound address, removing the parent-to-child port selection race. Startup failure diagnostics and the competing-bind regression cover the relevant boundary.
Coordinator and independent static reviews of actual old/new file contents found no actionable issue. Hosted CI on this new head remains pending; prior-head results do not qualify it. No candidate code or local tests executed.
Review stack: #5504 (CI) > #5499 (turn controls) > #5500 (receipts) > #5501 (executor) > #5502 (recovery) > #5503 (example) > #5495 (docs). Combined reference: #5494.
Requirements ·
REQ733,REQ735.Summary
A parent session now receives the rate-limit snapshot from a repair run. Child attention reports an interruption only when the native abort reason was an interruption.
Supervisors need to target a live native turn without sending a late correction to its successor. Add
Turn.steer_exact(..., expected_turn_id=...)andSteeringReceipt: stale targets fail, and accepted input carries the native turn and input IDs. Acceptance does not prove the model sampled the input.Turn.steer(input)keeps its existing signature and logical-submission behavior.Add
Turn.wait_for_attention()over the existing request/result wait, returning known interruption, connection-loss, rate-limit and blocking-quota outcomes.Session.rate_limits()andApiRateLimitsUpdatedexpose recognized HTTP/WebSocket headers and retry advice separately from token usage. Missing fields stay unknown. Model authentication remains API-key-only; managed login, plan credits and billing verification are excluded underREQ731.Layer 1 of 5; base #5504 (
chore/worker-native-ci-timeouts). This carries Core patch 0018; the CI budget changes are owned by #5504.Acceptance dependencies enabled here:
REQ781needs exact steering and supervisory outcomes for its combined workflow. This layer supplies those controls; the example layer exercises them.Test plan
The child-capability test fixture now lets the executor reserve its own port. It reads the listening address from the process before connecting. A competing listener reproduced the same startup assertion as the failed Python 3.12 job. The original job discarded the process diagnostic, so its underlying cause is unconfirmed. A regression that occupies the requested port immediately before startup failed on the old fixture. The regression and the previously failing typed-role test both passed with the updated fixture and the same hosted native binary.
The reviewed API snapshot now includes the interruption flag used by the native error type. All 30 API contract tests pass after the update. The previous snapshot failed the same four checks on Python 3.11 through 3.14.
The model-only child cancellation regression passed over both HTTP and WebSocket. Three focused native cases passed with the precise abort-reason check. The updated stack is running its broad checks.
Issue number
None linked. Owning requirements database IDs are listed above.
Checks
AGENTS.md, including.agents/skills/code-change-verification/scripts/run.shwhen requiredAGENTS.md