Repository navigation
[codex] add external worker execution - #407
Merged
Merged
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 33 minutes and 30 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis PR implements a complete external-worker execution mode that separates durable control-plane logic (session/run management on the server) from tool execution (running locally via CLI). It introduces an ChangesExternal Worker Execution Control Plane
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
delete-cloud
force-pushed
the
external-worker-execution
branch
from
May 31, 2026 11:48
269c0d3 to
96a1d27
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@coding-agent/src/coding_agent/cli/remote_commands.py`:
- Around line 410-414: The --poll-interval option currently allows 0 or negative
values which will cause run_worker_loop() to busy-wait; add validation that
poll_interval > 0 before entering the loop. Fix by validating the option value
(either add a click callback on the existing --poll-interval click.option or
perform an explicit check in the command handler immediately before calling
run_worker_loop(poll_interval=...)) and raise click.BadParameter (or exit with a
clear error) if the value is <= 0. Apply the same validation to the other
--poll-interval definition mentioned (lines 433-441) so both command paths
reject non-positive intervals.
In `@coding-agent/src/coding_agent/remote/worker.py`:
- Around line 344-370: The await heartbeat_task in the finally currently
re-raises non-Cancelled exceptions and prevents the subsequent TurnEnd/complete
POST; change the finally to catch and swallow heartbeat errors so finalization
always runs: wrap the await heartbeat_task in a try/except Exception as e (still
allow asyncio.CancelledError to be ignored) and either append or fold e (e.g.,
str(e)) into the existing error variable before continuing, then always execute
the TurnEnd emit and the client.post to /worker/runs/{run_id}/complete; refer to
_heartbeat_until_complete, heartbeat_task, TurnEnd emission, and the
client.post("/worker/runs/{run_id}/complete") call to locate where to make this
change.
- Around line 123-125: The control-plane HTTP client currently uses timeout=None
in run_local_worker_once and run_worker_loop which lets non-SSE POSTs
(claim/heartbeat/events/complete) hang indefinitely; change those AsyncClient
calls to use a bounded httpx.Timeout(...) for normal control requests and only
disable timeouts for the SSE prompt-stream request. Also update
_execute_claimed_run so that in the finally block you wait for the
heartbeat_task with errors captured (e.g., asyncio.gather(heartbeat_task,
return_exceptions=True) or catching exceptions from await heartbeat_task)
instead of letting an exception from _heartbeat_until_complete/client.post abort
before posting to /worker/runs/{run_id}/complete, ensuring the finalization POST
always runs even if heartbeat fails.
In `@coding-agent/src/coding_agent/runtime_store.py`:
- Around line 223-241: Add a DB index to speed up the external-worker claim
query: create a partial/expression index on agent_runs covering the filtered
expression columns and ordering keys used by _CLAIM_EXTERNAL_WORKER_RUN_SQL —
e.g. an index on (started_at, run_id) with expression columns
(metadata->>'execution_binding_kind', metadata->>'executor_kind', session_id) in
the predicate and a WHERE clause matching the claimable subset (status IN
('requested','expired') AND metadata->>'execution_binding_kind' =
'external_worker' AND metadata->>'executor_kind' IS NOT NULL). Put this in a
migration (CREATE INDEX CONCURRENTLY ...) so the agent_runs SELECT ... FOR
UPDATE SKIP LOCKED query in runtime_store.py benefits from the index without
full table scans.
In `@coding-agent/src/coding_agent/server/http_server.py`:
- Around line 2630-2644: The current mapping sets completion_status to
CompletionStatus.ERROR for any non-"completed" status, making cancellations
indistinguishable from failures; update the logic that computes
completion_status (the variable set before calling _broadcast_event) to
explicitly map body.status == "cancelled" to CompletionStatus.BLOCKED.value,
keep body.status == "completed" mapping to CompletionStatus.COMPLETED.value, and
only use CompletionStatus.ERROR.value for other unexpected statuses so that
cancelled runs are broadcast as BLOCKED rather than ERROR.
- Around line 2611-2649: The try block currently wraps the durable state change
finalize_external_worker_run and the subsequent non-critical calls
get_session_async and _broadcast_event, so failures in session fetch or
broadcasting can raise after the run is already finalized; move
finalize_external_worker_run(run_id=..., ...) out of the broader try so it
executes and returns success independently, then perform
get_session_async(run.session_id) and _broadcast_event(...) in a separate
try/except that catches and logs exceptions (including run_id and session_id)
but does not re-raise them to clients; ensure any casting/serialization for
tape_entries/result remains unchanged and only the non-idempotent durable call
is guaranteed before returning success.
In `@coding-agent/src/coding_agent/server/session_manager.py`:
- Around line 1150-1169: The _load_and_authorize_external_worker_run helper only
validates worker_id and claim_token; add a guard that rejects expired or
inactive claims by checking the run.metadata lease fields (e.g.
"claim_expires_at" timestamp and/or a "worker_active" flag) before returning the
run; parse the timestamp (use datetime.utcnow() for comparison) and if the lease
has expired or worker_active is false/absent raise PermissionError("external
worker claim is expired or inactive") so stale workers cannot heartbeat, append
events, or finalize runs.
- Around line 2664-2695: The code moves an unclaimed ExternalWorker run into
"cancelling", which makes it unclaimable; instead, after loading the run via
load_runtime_run(session.current_turn_id) check the run's current status and if
it is still "requested" or "expired" update the run in _runtime_store to a
terminal "cancelled" (and add cancel_requested_at metadata) and set the session
(session.turn_status, session.turn_in_progress, last_activity) to idle; only set
the run and session to "cancelling" when the loaded run indicates it has been
claimed/started (i.e., not "requested"/"expired"). Ensure you update the same
call site that currently uses _runtime_store.update_agent_run and adjust the
returned CancelTurnResult status accordingly.
In `@coding-agent/tests/ui/test_http_server.py`:
- Around line 7034-7055: The claim_external_worker_run test double only claims
runs with status "requested", but production allows claiming recovered "expired"
runs; update claim_external_worker_run to treat runs with status in
{"requested","expired"} as claimable (i.e., change the status check in
claim_external_worker_run to allow both "requested" and "expired"), preserving
the existing metadata merge and replace call that sets status="claimed" and
updates self.runs[run.run_id].
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 744f0850-ccd4-4f16-90ed-5872fb985946
📒 Files selected for processing (13)
coding-agent/.opencode/prompts/tasks/external-worker-execution-task-packet.mdcoding-agent/docs/adr/0051-external-worker-execution-control-plane.mdcoding-agent/src/coding_agent/cli/remote_commands.pycoding-agent/src/coding_agent/remote/worker.pycoding-agent/src/coding_agent/runtime_store.pycoding-agent/src/coding_agent/server/binding_resolver.pycoding-agent/src/coding_agent/server/execution_binding.pycoding-agent/src/coding_agent/server/http_server.pycoding-agent/src/coding_agent/server/schemas.pycoding-agent/src/coding_agent/server/session_manager.pycoding-agent/tests/cli/test_remote_client.pycoding-agent/tests/coding_agent/test_pg_runtime_store.pycoding-agent/tests/ui/test_http_server.py
delete-cloud
force-pushed
the
external-worker-execution
branch
from
May 31, 2026 12:03
96a1d27 to
3ba47a1
Compare
delete-cloud
force-pushed
the
external-worker-execution
branch
from
May 31, 2026 12:07
3ba47a1 to
55e4a6b
Compare
This was referenced May 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
Summary by CodeRabbit
Release Notes
New Features
local-run) or continuously (worker)Documentation
Tests