Repository navigation
arch(ws-11): add loop input port - #3646
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a host-owned input queue system for the Reborn loop, including the HostInputQueue trait and the HostQueueLoopInputPort adapter. It also updates the RebornLoopDriverHostFactory to integrate this new input mechanism and adds comprehensive tests for steering and follow-up message handling. Feedback suggests implementing a maximum batch size limit in poll_inputs to ensure consistency and prevent performance issues from excessively large batches.
SummaryReviewed WS11 PR #3646 only. Loop input port run-binding looks good, but ack identity is cursor-through rather than per-input token, breaking mixed-batch replay/idempotency invariant. Validation:
Findings
Security/data-flow notes
Correctness/invariant notes
Missing tests
|
|
Follow-up from the ws-6a review pass on #3596: #3596 now defers user-facing input ack until after the checkpoint containing the advanced cursor is durable. This WS11 input-port PR still owns the more precise production input contract. Please make sure this branch preserves the same ordering with the real input implementation: durable cursor/checkpoint first, physical ack second. If the real port supports mixed batches, exact per-input ack tokens should avoid acknowledging an entire batch just because one user-facing item was consumed; advance only the contiguous durable prefix that the checkpoint can recover from. |
109d2a3 to
0615a4e
Compare
…factory composition test - Add TODO on issued_cursors unbounded growth - Document intentional deviation from brief spec (input_queue on factory not config) - Add input_queue_wired_through_factory_drains_steering_message integration test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ct factory pattern Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00f8dfad20
ℹ️ 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".
* arch: ws-11-loop-input-port (iter 1, approved) * fix(ws-11): document cursor growth + PlannedDriverConfig deviation + factory composition test - Add TODO on issued_cursors unbounded growth - Document intentional deviation from brief spec (input_queue on factory not config) - Add input_queue_wired_through_factory_drains_steering_message integration test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ws-11): capture ack_inputs tokens in mock + update brief to reflect factory pattern Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ws-11): handle queued loop input controls * fix(ws-11): reject unissued input cursors --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: serrrfirat <f@nuff.tech>
Context
Input-drain host port branch. It gives the executor a durable/caller-visible way to drain steering and follow-up inputs without giving strategies direct queue access.
Master spec:
docs/reborn/agent-loop-skeleton.mdWorkstream brief:
docs/reborn/agent-loop-briefs/loop-input-port.mdStack base:
arch/ws-8Latest stack maintenance on 2026-05-14:
origin/reborn-integrationthrough WS17 before publishing the PR descriptions.What landed
InputDrainStrategydecisions through the host port.Reviewer focus
Non-goals / deferred work
Validation
cargo check -p ironclaw_reborncargo test -p ironclaw_loop_support --lib input_portgit diff --checkarch/ws-14-parentvalidation: multi-cratecargo checkwithironclaw_agent_loop/test-support.Stack position