-
Notifications
You must be signed in to change notification settings - Fork 0
Centralize worker API routes, DI, and error URL handling #69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
31 commits
Select commit
Hold shift + click to select a range
0a47711
feat(worker): centralize API routes and refactor WorkerRuntime constr…
leynos 17f1e1f
refactor(worker): reduce api.rs size by extracting client methods
leynos 1326382
refactor(worker): derive ROUTE constants from PATH to prevent drift
leynos 0199548
refactor(worker): extract reporting logic to meet 400-line limit
leynos 440fbb6
fix(worker): address code review findings
leynos 3799776
fix(tests): update test files to use refactored WorkerRuntime::new API
leynos ae36562
fix(worker): use iteration 100 for pre-loop failure reporting
leynos b95241e
fix(tests): restore register_remote_tools call in test helper
leynos eff07b0
refactor(worker): address code review findings for error handling
leynos 095ca66
fix(rebase): resolve merge conflicts and update test files for new API
leynos 22ed684
style: apply cargo fmt fixes to worker_remote_tool_proxy tests
leynos 372871b
test: add regression tests for WorkerRuntime and WorkerHttpClient err…
leynos a189552
test: fix doc comments and convert async tests to sync
leynos fa6b0a6
refactor: address code review findings for error handling consistency
leynos 9977cf4
Fix worker API and container issues from code review
leynos 024ae4d
refactor(worker): extract report_stopped_outcome helper to reduce dup…
leynos cff9d94
Tighten hosted worker runtime error handling
leynos fc82e09
Normalise worker API job paths and test helpers
leynos 75f999f
Re-export job_scoped_path and fix remote_tools test context handling
leynos 255c447
Use anyhow::Context for remote_tools test error wrapping
leynos 81c8db9
Tighten worker runtime error handling and event flushing
leynos f0978a1
Apply cargo fmt to worker_job_url
leynos 9da84fe
Extract post_and_require_success helper in worker client
leynos 1f5a34b
Fix post_event docs and use full endpoint URLs in worker client errors
leynos b28633b
feat(worker-api): add detailed worker-orchestrator HTTP boundary cont…
leynos 1e8a0b4
refactor(worker/runtime): refactor job loop to use timeout within run…
leynos 9d84ce9
test(worker/api/tests): refactor client test setup and add generic ro…
leynos 4eecb94
test(worker/api/tests): simplify terminal result round-trip test
leynos b45bdc6
refactor(worker/api): split API types into proxy and remote tool modules
leynos b94696d
docs(worker/api): add module docs for proxy_types and remote_tool_types
leynos 61fe147
docs(api): correct doc comment for ProxyCompletionRequest struct
leynos File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,156 @@ | ||
| # Worker-orchestrator contract | ||
|
|
||
| This document is for maintainers who need to change the hosted worker path | ||
| without breaking the orchestrator boundary. It explains the current transport | ||
| contract, the dependency-injection seams used for tests and production, and the | ||
| event-reporting split between authoritative state and best-effort visibility. | ||
|
|
||
| ## 1. Scope and source of truth | ||
|
|
||
| The worker runtime runs inside a container and talks back to the orchestrator | ||
| over HTTP. The shared transport contract lives in | ||
| [the worker API module](../src/worker/api.rs) and | ||
| [its shared types](../src/worker/api/types.rs). The orchestrator side imports | ||
| the same route constants and payload types from that module instead of | ||
| re-declaring them. | ||
|
|
||
| This document is descriptive for the current implementation. The code remains | ||
| the authoritative source of truth for the wire format. | ||
|
|
||
| ## 2. Boundary model | ||
|
|
||
| The worker and orchestrator have distinct responsibilities: | ||
|
|
||
| - The orchestrator owns job lifecycle, credential issuance, event ingestion, | ||
| and proxied external access. | ||
| - The worker owns local reasoning, tool execution inside the sandbox, and | ||
| periodic reporting back to the orchestrator. | ||
| - The shared HTTP boundary exists so the worker can stay isolated from the | ||
| host process while still using the host's approved network, credential, and | ||
| observability surfaces. | ||
|
|
||
| Figure 1. Worker-orchestrator boundary and reporting channels. | ||
|
|
||
| ```mermaid | ||
| flowchart LR | ||
| WorkerRuntime[WorkerRuntime in container] | ||
| Delegate[ContainerDelegate] | ||
| Client[WorkerHttpClient] | ||
| Orchestrator[Orchestrator worker API] | ||
| Timeline[Job event timeline] | ||
| Status[Authoritative status store] | ||
| Completion[Authoritative completion store] | ||
|
|
||
| WorkerRuntime --> Delegate | ||
| WorkerRuntime --> Client | ||
| Delegate --> Client | ||
| Client --> Orchestrator | ||
| Orchestrator --> Timeline | ||
| Orchestrator --> Status | ||
| Orchestrator --> Completion | ||
| ``` | ||
|
|
||
| ## 3. Shared route constants | ||
|
|
||
| All worker endpoints are declared once in `src/worker/api/types.rs` as paired | ||
| `*_PATH` and `*_ROUTE` constants. The design intent is: | ||
|
|
||
| - `*_PATH` is the relative suffix used by `WorkerHttpClient`. | ||
| - `*_ROUTE` is the fully scoped Axum route used by the orchestrator router. | ||
| - Both sides derive their concrete URLs from the same source strings. | ||
|
|
||
| The current contract includes: | ||
|
|
||
| Worker-orchestrator HTTP endpoints and their purposes. | ||
|
|
||
| | Endpoint | Purpose | | ||
| | --- | --- | | ||
| | `GET /worker/{job_id}/job` | Fetch the sandboxed job description | | ||
| | `GET /worker/{job_id}/credentials` | Deliver job-scoped credentials for child-process injection | | ||
| | `POST /worker/{job_id}/status` | Persist authoritative progress state | | ||
| | `POST /worker/{job_id}/complete` | Persist authoritative terminal outcome | | ||
| | `POST /worker/{job_id}/event` | Append user-visible timeline events | | ||
| | `GET /worker/{job_id}/prompt` | Poll orchestrator-injected follow-up prompts | | ||
| | `POST /worker/{job_id}/llm/complete` | Proxy plain language model (LLM) completion | | ||
| | `POST /worker/{job_id}/llm/complete_with_tools` | Proxy tool-capable language model (LLM) completion | | ||
| | `GET /worker/{job_id}/tools/catalog` | Fetch hosted-visible remote tool definitions | | ||
| | `POST /worker/{job_id}/tools/execute` | Execute a hosted remote tool through the orchestrator | | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| Compile-time assertions in the worker API tests lock the canonical route values | ||
| so accidental path drift fails the build before runtime tests execute. | ||
|
|
||
| ## 4. Dependency injection and construction | ||
|
|
||
| `WorkerRuntime` uses two constructors with distinct roles: | ||
|
|
||
| - `WorkerRuntime::new(config, client)` is the primary constructor. It is used | ||
| by tests and by any caller that already owns a prepared `WorkerHttpClient`. | ||
| - `WorkerRuntime::from_env(config)` is the production convenience wrapper. It | ||
| reads `IRONCLAW_WORKER_TOKEN` and then delegates to `new`, which builds the | ||
| HTTP client with the shared timeout and error mapping. | ||
|
|
||
| This split exists so tests can validate runtime behaviour without relying on | ||
| ambient environment state. It also gives construction-time validation one | ||
| obvious home: `new` checks that `WorkerConfig` and `WorkerHttpClient` agree on | ||
| job identity and orchestrator base URL before the runtime starts. | ||
|
|
||
| `WorkerHttpClient::new(...)` follows the same pattern for tests, while | ||
| `WorkerHttpClient::from_env(...)` is reserved for production bootstrap. | ||
|
|
||
| ## 5. Authoritative reports versus best-effort events | ||
|
|
||
| The worker emits two classes of outbound signal: | ||
|
|
||
| - Authoritative reports: | ||
| - `report_status` | ||
| - `report_complete` | ||
| - Best-effort timeline events: | ||
| - `post_event` | ||
| - `report_status_lossy` | ||
|
|
||
| The distinction matters: | ||
|
|
||
| - Status and completion calls define the durable job record. If they fail at a | ||
| point where correctness depends on them, the worker treats that as a real | ||
| error. | ||
| - Event posting exists for operator visibility. It enriches the browser and | ||
| audit timeline, but it must not be allowed to block or invalidate terminal | ||
| completion reporting. | ||
|
|
||
| `ContainerDelegate` therefore, uses a background task and bounded queue for | ||
| event posting. `shutdown()` closes the queue and waits for the event worker, so | ||
| buffered events flush before the delegate disappears. | ||
|
|
||
| `WorkerRuntime::post_event(...)` also uses a bounded timeout around terminal | ||
| event publication, so the final `report_complete(...)` call remains the | ||
| authoritative acknowledgement path. | ||
|
|
||
| ## 6. Credential handling | ||
|
|
||
| Credentials are fetched through `GET /worker/{job_id}/credentials` and | ||
| deserialized into `CredentialResponse`. The worker runtime does not write them | ||
| into global process environment variables. Instead: | ||
|
|
||
| 1. `WorkerRuntime::hydrate_credentials()` fetches the granted credentials. | ||
| 2. The runtime stores them in `extra_env`. | ||
| 3. Tool execution passes `extra_env` through `JobContext` into child processes. | ||
|
|
||
| This keeps credential scope limited to the worker execution path and avoids | ||
| cross-test or cross-job global environment mutation. | ||
|
|
||
| ## 7. Prompt polling and hosted tool context | ||
|
|
||
| The worker loop polls `GET /worker/{job_id}/prompt` before LLM calls. The | ||
| orchestrator can use that channel to inject operator prompts or follow-up work | ||
| without restarting the worker process. | ||
|
|
||
| Hosted remote tools use a parallel mechanism: | ||
|
|
||
| 1. The worker fetches the hosted tool catalogue from the orchestrator. | ||
| 2. The worker registers local proxy wrappers using the orchestrator-provided | ||
| canonical `ToolDefinition` values. | ||
| 3. The runtime merges those definitions into the reasoning context alongside | ||
| container-local tools. | ||
|
|
||
| The shared route constants and transport types are what keep that hosted tool | ||
| surface consistent across the sandbox boundary. | ||
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.