Repository navigation
[codex] add external worker session inspection - #408
Conversation
|
Warning Review limit reached
More reviews will be available in 47 minutes and 16 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 (8)
📝 WalkthroughWalkthroughThis PR implements an external-worker usable control plane by adding HTTP endpoints for listing session runs and worker status, remote client helper functions for fetching and streaming that data, CLI commands for inspection and interaction, and supporting tests. Worker health status is derived from existing durable run metadata without requiring new persisted state. ChangesExternal Worker Usable Control Plane
Possibly Related PRs
Poem
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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 |
ef64b07 to
b7f1ad2
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
coding-agent/src/coding_agent/server/http_server.py (1)
3335-3347: ⚡ Quick winGroup runs by worker once before computing statuses.
_worker_statuses_from_runs()collects worker IDs, then_worker_status_from_runs()refilters and resorts the full run list for each worker. On/workers, that turns into repeated O(total_runs) scans. Bucket byworker_idfirst and compute each status from its own slice.♻️ Proposed refactor
def _worker_statuses_from_runs( runs: Iterable[AgentRunRecord], ) -> list[WorkerStatusResponse]: - run_list = list(runs) - worker_ids = { - worker_id - for run in run_list - if isinstance(worker_id := run.metadata.get("worker_id"), str) and worker_id - } + runs_by_worker: dict[str, list[AgentRunRecord]] = {} + for run in runs: + worker_id = run.metadata.get("worker_id") + if isinstance(worker_id, str) and worker_id: + runs_by_worker.setdefault(worker_id, []).append(run) return sorted( - (_worker_status_from_runs(worker_id, run_list) for worker_id in worker_ids), + ( + _worker_status_from_runs(worker_id, worker_runs) + for worker_id, worker_runs in runs_by_worker.items() + ), key=lambda worker: worker.worker_id, )🤖 Prompt for 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. In `@coding-agent/src/coding_agent/server/http_server.py` around lines 3335 - 3347, The function _worker_statuses_from_runs currently builds worker_ids then calls _worker_status_from_runs(worker_id, run_list) which causes repeated full-list scans; instead group runs by their metadata worker_id once (e.g., iterate run_list and append each run to a dict bucket keyed by the string worker_id extracted the same way you do now), then produce statuses by iterating the buckets and calling _worker_status_from_runs(worker_id, bucket_runs) (or add a new helper that computes status from a slice). This eliminates O(total_runs^2) behavior by operating on each worker's slice only and preserves sorting by worker_id when returning the final list.
🤖 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/docs/adr/0052-external-worker-usable-control-plane.md`:
- Around line 24-29: Update the ADR to document the full WorkerStatusResponse
contract exactly as defined in src/coding_agent/server/schemas.py: include the
missing top-level status field and enumerate all allowed status values
(including the "offline" value), and add the omitted fields current_session_id,
last_run_id, and last_session_id to the response shape; ensure the paragraph
that currently lists worker_id, executor_kind, worker_pool, workspace_ref,
current_run_id, last_seen_at, and lease_expires_at is expanded to show the
complete JSON/contract shape and the canonical set of status enum values so
readers don't need to consult the implementation or schema.
In `@coding-agent/src/coding_agent/cli/remote_commands.py`:
- Around line 826-845: The legacy top-level attach flow still calls
stream_prompt(), which breaks when the server closes the SSE stream after a
RunRequested event; update the attach command (the function that currently calls
stream_prompt) to use the same logic as remote_prompt by calling
stream_prompt_or_run_request(base_url=..., session_id=..., prompt=...,
headers=auth_headers(...)) or add a conditional that switches from stream_prompt
to stream_prompt_or_run_request when the remote session is an external-worker /
will emit RunRequested; ensure you reference auth_headers and get_remote to
obtain headers and endpoint just like remote_prompt so both entry points behave
identically.
In `@coding-agent/src/coding_agent/server/http_server.py`:
- Around line 3297-3310: The status logic currently treats any active run
(current) without lease_expires_at as "running"; change it to consider
last_seen_at so old active records can become "stale" or "offline". In the block
that sets status (where symbols are lease_expires_at, last_seen_at, current,
now, status), if lease_expires_at is None compute age = now - last_seen_at
(guarding for last_seen_at being None) and compare against your existing
stale/offline thresholds (or add reasonable STALE_THRESHOLD and
OFFLINE_THRESHOLD constants) to set status to "running", "stale", or "offline"
accordingly instead of always "running".
In `@coding-agent/tests/ui/test_http_server.py`:
- Around line 2485-2541: The test
test_external_worker_workers_endpoint_reports_running_and_stale_workers uses a
hardcoded "/tmp/repo" display_path which triggers Ruff S108; replace the literal
string with the pytest tmp_path fixture (e.g., str(tmp_path) or
tmp_path.as_posix()) wherever workspace_ref "display_path" is set in the session
create JSON and where the expected response is asserted, updating the two
occurrences that reference "/tmp/repo" (in the creation payload and the expected
workspace_ref dict) so the test uses a temporary secure path instead of a
hardcoded one.
---
Nitpick comments:
In `@coding-agent/src/coding_agent/server/http_server.py`:
- Around line 3335-3347: The function _worker_statuses_from_runs currently
builds worker_ids then calls _worker_status_from_runs(worker_id, run_list) which
causes repeated full-list scans; instead group runs by their metadata worker_id
once (e.g., iterate run_list and append each run to a dict bucket keyed by the
string worker_id extracted the same way you do now), then produce statuses by
iterating the buckets and calling _worker_status_from_runs(worker_id,
bucket_runs) (or add a new helper that computes status from a slice). This
eliminates O(total_runs^2) behavior by operating on each worker's slice only and
preserves sorting by worker_id when returning the final list.
🪄 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: 2ec5a658-d295-4b2d-baca-ed073186124e
📒 Files selected for processing (8)
coding-agent/.opencode/prompts/tasks/external-worker-usable-control-plane-task-packet.mdcoding-agent/docs/adr/0052-external-worker-usable-control-plane.mdcoding-agent/src/coding_agent/cli/remote_commands.pycoding-agent/src/coding_agent/remote/client.pycoding-agent/src/coding_agent/server/http_server.pycoding-agent/src/coding_agent/server/schemas.pycoding-agent/tests/cli/test_remote_client.pycoding-agent/tests/ui/test_http_server.py
b7f1ad2 to
9c22572
Compare
Summary
Tests
Summary by CodeRabbit
New Features
Documentation