Repository navigation
BARC-01.1f: admit coordinator batches atomically with provenance - #1
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48acc6f4b7
ℹ️ 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".
| if (!batch.has_client_command_id() || batch.client_command_id() == 0) { | ||
| return {CommandBatchAdmissionStatus::kInvalidCorrelation, 0}; |
There was a problem hiding this comment.
When clients omit the proto-optional client_command_id, this now drops the batch after the coordinator has already acknowledged it as forwarded. The checked coordinator validates only batch_seq, commands, and finite values (specs/002-live-headless-e2e/examples/coordinator.py:151-162), while bundled callers such as _make_move_batch omit the correlation field entirely (command_channel_repro.py:42-53); the normal gateway validator also requires it only when strict validation is configured. Consequently, otherwise-valid coordinator-mode commands report success to the caller but never reach the engine queue. Either preserve compatibility here or make the coordinator populate/reject the field before acknowledging submission.
Useful? React with 👍 / 👎.
The coordinator previously discarded batch, client, index, and full-width sequence provenance while enqueueing children individually, and queue overflow could leave a partially admitted parent.
This change validates each 1–64 command batch before queue mutation, preserves session, batch sequence, client correlation, command index, and authoritative target on every queued child, and admits the complete batch through one atomic
TryPushBatchcall. Typed accepted, invalid, and queue-full outcomes feed coordinator counters and trace/error logging. Focused coverage exercises exact three-child provenance, values aboveUInt32, atomic overflow, invalid and oversized batches, and concurrent non-interleaving.Validation:
command_queue_testpassedcoordinator_command_admission_testpassed, including 100 consecutive executionsCoordinatorClient.cpppassed a standalone C++20 syntax compile against generated protobuf/gRPC headersThis focused source evidence does not claim a full Recoil engine build, live game session, end-to-end broker acknowledgement, or deployed native qualification.