-
Notifications
You must be signed in to change notification settings - Fork 3
[Experiment] orchestrator-v2 hardening: telemetry, resilience, capability routing #43
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
Open
matt-wright86
wants to merge
4
commits into
main
Choose a base branch
from
refactor/orchestrator-v2-hardening
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
e5fe563
harden orchestrator-v2: telemetry, resilience, capability routing
matt-wright86 e6856ec
add claude-engine example + hardening-experiment docs
matt-wright86 9020d17
address PR review: fail-closed capabilities, sendRpcError unwrap, tes…
matt-wright86 72fac71
format docs and example
matt-wright86 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
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,38 @@ | ||
| # Orchestrator v2 — hardening experiment | ||
|
|
||
| This directory captures an exploratory session that **audited, hardened, and dogfooded** the | ||
| Orchestrator v2 rebuild. It lives on the `refactor/orchestrator-v2-hardening` branch; it is not a | ||
| merge-ready change set but a documented investigation with working code behind it. | ||
|
|
||
| ## What's in the branch | ||
|
|
||
| **Code (fixes + tests, all with green build/lint and 43/43 tests):** | ||
|
|
||
| | | What | Where | | ||
| | ------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------ | | ||
| | **Route A** | Telemetry propagates engine → `ScriptResult` → RPC → `TaskSource` as a terminal outcome; `ExecutionStats` unified in `interfaces-task-source`. | `runner`, `orchestrator`, `interfaces-task*`, `agent-3-task` | | ||
| | **G1** | A failed run persists its `sessionId` (and telemetry), so retries are resumable. | `agent-3-task/run-task-agent.ts` | | ||
| | **I3** | A throwing task-source callback can no longer wedge a peer or escape as an unhandled rejection (`settle` / `recordBestEffort` guards + a `route().catch` net). | `orchestrator/{result-handler,rpc-router,dispatch-ack-handler,best-effort}.ts` | | ||
| | **I1** | Capability-aware routing: runners advertise their agents in the heartbeat; the orchestrator only dispatches to a capable peer. | `protocol`, `runner`, `orchestrator/peer-registry.ts` | | ||
| | **Packaging** | `@bifrost-ai/orchestrator`'s `./test-helpers` export fixed via `publishConfig.exports`. | `orchestrator/package.json`, `publish.js` | | ||
|
|
||
| Two `/simplify` passes and a runnable example (`examples/claude-engine/`) accompany the fixes. | ||
|
|
||
| ## The documents | ||
|
|
||
| - **[lessons.md](./lessons.md)** — the retrospective: what shipped, what we learned, recommended next steps. Start here. | ||
| - **[gap-report.md](./gap-report.md)** — the detailed doc↔code + integration audit (findings G1–G26, I1–I3, E1–E3), with fixed items marked. | ||
| - **[dogfood-runs.md](./dogfood-runs.md)** — three escalating real-`claude`-engine runs with telemetry receipts (incl. the G1 resume payoff). | ||
|
|
||
| ## The headline lesson | ||
|
|
||
| > Orchestrator v2 is a solid dispatch skeleton whose terminal boundary dropped everything but | ||
| > `taskId`. Telemetry (E1), crash-resilience (I3), and capability (I1) all had to be threaded | ||
| > deliberately — and the task's **output** (E3) still isn't. The layer that would _consume_ rich | ||
| > outcomes (`agent-4-workflow`) doesn't exist yet. | ||
|
|
||
| ## Top follow-ups (from the lessons) | ||
|
|
||
| 1. **E3** — propagate task output (a Route A-style change, or a documented `setState` contract). | ||
| 2. Reconcile the 3 correctness-level **doc gaps** (README `Task` shape, `ScriptContext` omissions). | ||
| 3. Build **`agent-4-workflow`** + a `TaskSource` read-side, so the DAG/retry we hand-rolled has a home. |
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,126 @@ | ||
| # Dogfood runs | ||
|
|
||
| Three escalating end-to-end runs of Orchestrator v2 driven by a **real engine** — the `claude` CLI, | ||
| whose JSON output (`result`, `usage`, `total_cost_usd`, `num_turns`, `session_id`) maps straight onto | ||
| `EngineResult`. Each run exercised the whole stack the way an operator would, with **real** tokens, | ||
| cost, and telemetry flowing through the [Route A](./lessons.md) pipeline. | ||
|
|
||
| > Reproduce the basic run: `examples/claude-engine/` (the harder/hardest runs were throwaway | ||
| > harnesses; their setup is described below). | ||
|
|
||
| --- | ||
|
|
||
| ## Run 1 — basic real deployment | ||
|
|
||
| **Setup:** generated ed25519 keys, wrote a real `runner.yaml` loaded via `new Runner({ configPath })`, | ||
| enrolled a Task Agent, registered a `claude`/haiku engine, dispatched 2 tasks. | ||
|
|
||
| **Exercised:** the documented `runner.yaml` onboarding path (`config-loader`, PEM parsing) — **never | ||
| covered by any test or prior harness** — plus `enrollTaskAgent`, a real engine, Route A telemetry, and | ||
| the session-id round-trip. | ||
|
|
||
| **Result:** both tasks completed; real telemetry reached the task source; real Claude `session_id`s | ||
| round-tripped via `setState`. | ||
|
|
||
| | task | out tokens | turns | cost | | ||
| | ------------------------------------ | ---------: | ----: | ------: | | ||
| | dog-1 (`"reply DOGFOOD"`) | 61 | 1 | $0.0227 | | ||
| | dog-2 (`"describe an orchestrator"`) | 128 | 1 | $0.0153 | | ||
|
|
||
| **Total: $0.038.** Lesson: the untested onboarding path worked first try. | ||
|
|
||
| --- | ||
|
|
||
| ## Run 2 — a hand-orchestrated DAG (fan-out → aggregate) | ||
|
|
||
| **Setup:** two `summarize` tasks fanned out to a capability-routed **summarizer** runner (concurrent, | ||
| `maxInFlight=2`); an aggregate `release-notes` task then routed to a separate **reviewer** runner. The | ||
| task source owned the dependency gate. Engines persisted their output via `ctx.setState` so the | ||
| aggregate step could read the summaries. | ||
|
|
||
| **Exercised:** I1 capability routing across two specialized runners; concurrency; Route A telemetry | ||
| **roll-up** across the graph (exactly a workflow "verify-step aggregate stats"). | ||
|
|
||
| **Result — the system wrote release notes for its own new features:** | ||
|
|
||
| > _"Task completion now sends execution telemetry to the backend, providing detailed performance | ||
| > metrics and better observability. Runners can validate required capabilities, ensuring tasks are | ||
| > only assigned to runners equipped to handle them."_ | ||
|
|
||
| | task | out tokens | cost | | ||
| | ------------- | ---------: | ----------: | | ||
| | sum-1 | 235 | $0.0237 | | ||
| | sum-2 | 316 | $0.0241 | | ||
| | release-notes | 384 | $0.0167 | | ||
| | **total** | **935** | **$0.0645** | | ||
|
|
||
| **Lesson (new gap E3):** chaining only worked because the **engine persisted each output via | ||
| `setState`**. `task.complete` carries telemetry, not the task's `message` — there is no built-in | ||
| channel for a task's output to reach the source or a downstream step. The DAG gate was hand-rolled in | ||
| the task source because the Workflow Agent (`agent-4-workflow`) doesn't exist yet. | ||
|
|
||
| --- | ||
|
|
||
| ## Run 3 — a 3-stage pipeline with retry + session-resume (hardest) | ||
|
|
||
| **Setup:** three `analyze` tasks fanned out to an **analyzer** runner (haiku, concurrent); a | ||
| **synthesizer** runner (a **sonnet** engine) combined them; a **critic** runner (haiku) reviewed the | ||
| result. Capability-routed across **three** specialized runners with **heterogeneous engines**. One | ||
| analyze task (`an-routing`) was flaky — it failed _after_ doing its work, and the task source | ||
| **retried it with `--resume`**. | ||
|
|
||
| **Exercised, in one run:** Route A (the telemetry below), I1 (3-way capability routing), G1 (failed-run | ||
| session persistence → resumable retry), heterogeneous per-task engine selection, concurrency, and a | ||
| task-source-owned DAG + retry policy. | ||
|
|
||
| ### 🧾 Telemetry receipts (per attempt) | ||
|
|
||
| ``` | ||
| task att model outcome in out cacheR cost$ ms | ||
| ------------------------------------------------------------------------ | ||
| an-telemetry 1 haiku completed 10 473 21220 0.0171 6931 | ||
| an-routing 1 haiku failed 10 531 17157 0.0253 6980 | ||
| an-resilience 1 haiku completed 10 609 17157 0.0255 7265 | ||
| an-routing 2 haiku completed 10 402 27639 0.0059 5037 | ||
| synthesize 1 sonnet completed 11202 169 23131 0.0780 3334 | ||
| critique 1 haiku completed 10 538 21220 0.0175 7613 | ||
| ------------------------------------------------------------------------ | ||
| TOTAL 6 calls 11252 2722 127524 0.1693 37160 | ||
| ``` | ||
|
|
||
| By model: haiku — 5 calls, $0.0913 · sonnet — 1 call, $0.0780. | ||
|
|
||
| ### ⭐ The receipt that matters — the G1 payoff | ||
|
|
||
| `an-routing` **failed** on attempt 1 ($0.0253, building 10,482 cache-creation tokens of context), then | ||
| resumed on retry: | ||
|
|
||
| > attempt 2 → **$0.0059** (cacheR: 27,639) — **4.3× cheaper**, reusing the session instead of | ||
| > re-paying to rebuild context. | ||
|
|
||
| Without the G1 fix (persisting `sessionId` on failure), the retry would re-do the expensive work from | ||
| scratch. The receipts make the value concrete. | ||
|
|
||
| ### The pipeline's output (it critiqued its own design) | ||
|
|
||
| **Synthesis (sonnet):** _"The orchestrator matches tasks to runners by required agent capability, | ||
| avoiding wasted attempts on incapable runners, while tracking per-task execution costs and resource | ||
| usage to support operator budgeting… It isolates task-source callback failures so a hung or erroring | ||
| callback cannot crash or stall the orchestrator itself."_ | ||
|
|
||
| **Critique (haiku):** _"The three concerns aren't truly independent — callback isolation is | ||
| foundational reliability that enables cost tracking and matching to work correctly, and cost data | ||
| should feed back into matching decisions…"_ — a genuinely sharp observation about the architecture. | ||
|
|
||
| --- | ||
|
|
||
| ## What the runs proved, together | ||
|
|
||
| - **Route A telemetry** carries real numbers end-to-end (every receipt above). | ||
| - **I1 capability routing** correctly places tasks across 1, 2, and 3 specialized runners. | ||
| - **G1** turns a failed run into a cheap resumable retry ($0.0059 vs. $0.0253). | ||
| - **The onboarding path** (`runner.yaml`) and a **real engine** both work first try. | ||
| - **E3** (task output isn't propagated) is the next thing to fix — and the hand-rolled DAG/retry is the | ||
| argument for building `agent-4-workflow`. | ||
|
|
||
| Total spend across all three runs: **~$0.27**. | ||
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Label the telemetry receipt fence.
Markdownlint flags this bare fence. Add a language tag such as
textso the doc stays lint-clean.Fix
📝 Committable suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 78-78: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Source: Linters/SAST tools