Skip to content

Commit d49d8e6

Browse files
akagifreeezclaude
andcommitted
fix(conductor): holistic-review fixes + CI + committed example artifacts (v0.3.3)
A whole-codebase review (6 dimensions, each finding independently verified) confirmed 17 issues; verdict "solid". This addresses the correctness ones plus the highest-leverage packaging gaps. Correctness: - Coordinator.run_all() is now safe to call repeatedly: a fresh per-batch token (not a per-Coordinator one) so trace files never collide/truncate, and a fresh owned ledger per batch so cost isn't double-counted across runs. (was HIGH) - Orchestrator owns its ledger PER run() (created in run(), like the Tracer), so re-running the same Orchestrator no longer writes to a closed ledger while stale in-memory rows leak into the next run's budget/summary. (was MEDIUM) - replay_trace now compares terminal status: a run that ended budget_exceeded can no longer replay as max_steps and still report match:true. (was MEDIUM) - CascadeBackend returns a new turn (dataclasses.replace) instead of mutating the strong backend's turn, so a reused turn object can't accumulate extra_usages. - replay.py docstring corrected (it never diffed per-provider cost). Packaging / positioning (the buried-lede gaps): - GitHub Actions CI (pytest on py3.9 + py3.12) + CI/python/license badges. - examples/ now COMMITS real outputs (a run trace, a per-provider ledger, a sandbox snapshot/rollback trace) so the signature artifact is visible on GitHub without cloning. README links them up top. - README "Known limitations (tracked)" section acknowledging the deferred low-severity items rather than hiding them. Regression tests for every correctness fix (second run_all, Orchestrator re-run, replay budget-status divergence, coordinator mid-job budget cutoff). 98 tests pass; wheel builds. Version 0.3.3. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 8187e69 commit d49d8e6

14 files changed

Lines changed: 296 additions & 37 deletions

.github/workflows/ci.yml

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
name: CI
2+
3+
on:
4+
push:
5+
branches: [master, main]
6+
pull_request:
7+
8+
jobs:
9+
test:
10+
runs-on: ubuntu-latest
11+
strategy:
12+
fail-fast: false
13+
matrix:
14+
python-version: ["3.9", "3.12"]
15+
steps:
16+
- uses: actions/checkout@v4
17+
- uses: actions/setup-python@v5
18+
with:
19+
python-version: ${{ matrix.python-version }}
20+
# token-router is the in-process accounting dependency (public repo).
21+
- name: Install token-router
22+
run: pip install "git+https://github.com/akagifreeez/token-router"
23+
- name: Install conductor (no deps; token-router already installed)
24+
run: pip install -e . --no-deps
25+
- name: Install pytest
26+
run: pip install pytest
27+
- name: Run tests
28+
run: pytest -q

.gitignore

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,17 @@ __pycache__/
77
*.py[cod]
88
*.egg-info/
99
.pytest_cache/
10+
.coverage
11+
.coverage.*
12+
htmlcov/
1013
build/
1114
dist/
1215

1316
# runtime artifacts
1417
traces/
1518
*.jsonl
1619
!tests/**/*.jsonl
20+
!examples/**/*.jsonl
1721

1822
# secrets — never commit keys
1923
*.key

README.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,16 @@
11
# 🎛️ Conductor
22

3+
[![CI](https://github.com/akagifreeez/conductor/actions/workflows/ci.yml/badge.svg)](https://github.com/akagifreeez/conductor/actions/workflows/ci.yml)
4+
[![Python](https://img.shields.io/badge/python-3.9%2B-blue.svg)](pyproject.toml)
5+
[![License: MIT](https://img.shields.io/badge/license-MIT-green.svg)](LICENSE)
6+
37
**A vendor-neutral, self-hosted control plane for LLM agents.**
48

9+
> 👀 **See it without running anything:** [`examples/`](examples/) holds real
10+
> committed outputs — a [run trace](examples/example-trace.jsonl), a
11+
> [per-provider cost ledger](examples/example-ledger.jsonl), and a
12+
> [sandbox snapshot/rollback trace](examples/example-sandbox-trace.jsonl).
13+
514
One self-built tool-use loop drives *any* provider — Claude (official Anthropic
615
SDK), any OpenAI-compatible API (OpenAI / OpenRouter / Groq / Mistral / Together
716
/ Fireworks), or a local model (Ollama / LM Studio / vLLM) — behind a single
@@ -318,6 +327,26 @@ conductor run --provider local --model qwen2.5:3b-instruct --task "..."
318327
- **Later (one of):** microVM (KVM/Firecracker) comparison · a lightweight web
319328
dashboard over traces/ledger.
320329

330+
## Known limitations (tracked, not hidden)
331+
332+
A whole-codebase review surfaced these; the correctness ones are fixed, the rest
333+
are acknowledged rather than papered over:
334+
335+
- **Replay does not re-apply a budget.** A run that ended `budget_exceeded`
336+
replays without the ceiling, so its status differs — the comparison now
337+
*reports this as a non-match* (`status_match: false`) instead of hiding it.
338+
- **The real-backend adapters' failure branches are lightly covered** by tests.
339+
The offline doubles exercise the happy path + injection/leak guards; live error
340+
paths (provider 5xx, a container that won't start) lean on the resilience layer
341+
rather than dedicated tests.
342+
- **`ProxmoxSandbox` (API) auto-adds unknown SSH host keys** (`AutoAddPolicy`) for
343+
the `pct exec` channel — fine on a trusted LAN/Tailscale, not for hostile nets.
344+
- **`DockerSandbox` containers are not resource/network-constrained by default**
345+
(no `--network none`/`--memory`/`--cap-drop`). Add them for untrusted workloads.
346+
- **Container isolation tier:** Docker and Proxmox LXC share the host kernel
347+
(contained + revertible, but not a VM-grade boundary). A microVM backend is the
348+
future option for that.
349+
321350
## Tech
322351

323352
Python · `requests` for the OpenAI-compatible path · official `anthropic` SDK

conductor/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
)
5252
from .tracer import Tracer
5353

54-
__version__ = "0.3.2"
54+
__version__ = "0.3.3"
5555

5656
__all__ = [
5757
"AgentBackend",

conductor/backends/cascade.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
"""
2828
from __future__ import annotations
2929

30+
from dataclasses import replace
3031
from typing import Callable, List, Optional
3132

3233
from .base import AgentBackend, AssistantTurn, Message, ToolSpec
@@ -100,5 +101,9 @@ def step(
100101
system=system, messages=messages, tools=tools,
101102
max_tokens=max_tokens, temperature=temperature,
102103
)
103-
strong_turn.extra_usages = list(strong_turn.extra_usages) + [cheap_turn.usage]
104-
return strong_turn
104+
# Return a NEW turn rather than mutating the strong backend's object: a
105+
# backend that reuses/returns a shared AssistantTurn would otherwise
106+
# accumulate extra_usages across steps and double-count the cheap leg.
107+
return replace(
108+
strong_turn, extra_usages=list(strong_turn.extra_usages) + [cheap_turn.usage]
109+
)

conductor/coordinator.py

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -91,34 +91,43 @@ def __init__(
9191
self.budget_usd = budget_usd
9292
self.trace_dir = trace_dir
9393
self.run_id_prefix = run_id_prefix
94-
# Unique per-coordinator token so repeated run_all() calls (or two
95-
# coordinators sharing a prefix) never collide on trace/ledger filenames
96-
# (a Tracer opens its file truncating - the CLI guards this the same way).
97-
self._uniq = f"{int(time.time() * 1000)}-{uuid.uuid4().hex[:6]}"
98-
self._owns_ledger = ledger is None
99-
if ledger is not None:
100-
self.ledger = ledger
101-
else:
102-
# Mirror the Orchestrator's owned-ledger crash-durability: stream to JSONL.
103-
path = os.path.join(trace_dir, f"ledger-{run_id_prefix}-{self._uniq}.jsonl")
104-
self.ledger = Ledger(pricing=make_pricing(), jsonl_path=path)
94+
# A caller-owned (shared) ledger persists across batches; an owned ledger
95+
# is created FRESH per run_all() (see below), never once in __init__ - so
96+
# repeated run_all() calls don't collide on filenames or double-count cost.
97+
self._injected_ledger = ledger
98+
self._batch = 0
99+
# Exposed after run_all so callers can inspect the most recent batch ledger.
100+
self.ledger = ledger if ledger is not None else Ledger(pricing=make_pricing())
105101

106102
def run_all(self, jobs: List[Job]) -> CoordinatorResult:
107103
result = CoordinatorResult(budget_usd=self.budget_usd)
104+
self._batch += 1
105+
# Fresh per-CALL token so repeated run_all() invocations never collide on
106+
# trace/ledger filenames (Tracer truncates) - a per-Coordinator token did.
107+
batch = f"{int(time.time() * 1000)}-{uuid.uuid4().hex[:6]}-b{self._batch}"
108+
owns_ledger = self._injected_ledger is None
109+
if owns_ledger:
110+
# Fresh owned ledger per batch: crash-durable JSONL, and no stale rows
111+
# from a prior batch leaking into this batch's budget/summary.
112+
path = os.path.join(self.trace_dir, f"ledger-{self.run_id_prefix}-{batch}.jsonl")
113+
ledger = Ledger(pricing=make_pricing(), jsonl_path=path)
114+
else:
115+
ledger = self._injected_ledger
116+
self.ledger = ledger
108117
try:
109118
for i, job in enumerate(jobs):
110119
# Skip before starting if the global budget is already spent.
111-
if self.budget_usd is not None and ledger_cost_usd(self.ledger) >= self.budget_usd:
120+
if self.budget_usd is not None and ledger_cost_usd(ledger) >= self.budget_usd:
112121
result.outcomes.append(JobOutcome(label=job.label, status="skipped_budget"))
113122
continue
114123
orch = Orchestrator(
115124
job.backend,
116125
job.registry,
117-
run_id=f"{self.run_id_prefix}-{self._uniq}-{i}-{job.label}",
126+
run_id=f"{self.run_id_prefix}-{batch}-{i}-{job.label}",
118127
system=job.system or DEFAULT_SYSTEM,
119128
max_steps=job.max_steps,
120129
trace_dir=self.trace_dir,
121-
ledger=self.ledger, # shared -> global budget
130+
ledger=ledger, # shared -> global budget for this batch
122131
budget_usd=self.budget_usd, # each step also checks the global spend
123132
)
124133
# Isolate per-job failures: one erroring agent must not abort the
@@ -133,9 +142,9 @@ def run_all(self, jobs: List[Job]) -> CoordinatorResult:
133142
error=f"{type(exc).__name__}: {exc}")
134143
)
135144
finally:
136-
if self._owns_ledger:
137-
self.ledger.close()
145+
if owns_ledger:
146+
ledger.close()
138147
# 6 dp to match ledger_summary["est_cost_usd"] (avoid two disagreeing totals).
139-
result.total_cost_usd = round(ledger_cost_usd(self.ledger), 6)
140-
result.ledger_summary = conductor_summary(self.ledger)
148+
result.total_cost_usd = round(ledger_cost_usd(ledger), 6)
149+
result.ledger_summary = conductor_summary(ledger)
141150
return result

conductor/orchestrator.py

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -116,17 +116,15 @@ def __init__(
116116
# run start, teardown in finally) and wires the registry's dangerous-tool
117117
# gate to dispatch through it. Without one, dangerous tools stay blocked.
118118
self.sandbox = sandbox
119-
# A fresh ledger per run unless the caller shares one across runs (e.g.
120-
# to aggregate the per-provider cost split of a two-provider demo). An
121-
# owned ledger is streamed to its own JSONL (crash-durable, mirroring the
122-
# trace) and closed by us; a shared/injected ledger is the caller's to
123-
# configure and close.
124-
self._owns_ledger = ledger is None
125-
if ledger is not None:
126-
self.ledger = ledger
127-
else:
128-
ledger_path = os.path.join(trace_dir, f"ledger-{run_id}.jsonl")
129-
self.ledger = Ledger(pricing=make_pricing(), jsonl_path=ledger_path)
119+
# Ledger policy: a shared/injected ledger is the caller's to configure and
120+
# close; an OWNED ledger is created FRESH inside run() (not here) - exactly
121+
# like the per-run Tracer - so that re-running the same Orchestrator doesn't
122+
# write to a closed ledger (silently dropping disk rows while in-memory
123+
# rows from the prior run leak into the next run's budget/summary).
124+
self._injected_ledger = ledger
125+
# self.ledger is (re)assigned at the start of each run(); seed it so the
126+
# attribute exists even before the first run.
127+
self.ledger = ledger if ledger is not None else Ledger(pricing=make_pricing())
130128

131129
def _trace_sandbox(self, tracer: Tracer, event: str, detail: str = "") -> None:
132130
"""Emit a sandbox lifecycle event, never letting a trace-write failure
@@ -141,6 +139,19 @@ def _trace_sandbox(self, tracer: Tracer, event: str, detail: str = "") -> None:
141139
pass
142140

143141
def run(self, task: str) -> RunResult:
142+
# Set up the ledger FRESH per run when we own it (single-use, like the
143+
# Tracer): this makes re-running the same Orchestrator safe and keeps the
144+
# on-disk JSONL and the in-memory rows in agreement.
145+
if self._injected_ledger is not None:
146+
self.ledger = self._injected_ledger
147+
owns_ledger = False
148+
else:
149+
self.ledger = Ledger(
150+
pricing=make_pricing(),
151+
jsonl_path=os.path.join(self.trace_dir, f"ledger-{self.run_id}.jsonl"),
152+
)
153+
owns_ledger = True
154+
144155
tools = self.registry.specs()
145156
messages: List[Message] = [Message(role="user", text=task)]
146157
final_text = ""
@@ -252,7 +263,7 @@ def run(self, task: str) -> RunResult:
252263
try:
253264
tracer.close()
254265
finally:
255-
if self._owns_ledger:
266+
if owns_ledger:
256267
self.ledger.close()
257268

258269
return RunResult(

conductor/replay.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,13 @@
1010
events) instead of re-executing tools.
1111
1212
So a replay produces a fresh trace that should match the original's tool I/O,
13-
final answer, and per-provider cost. This proves the trace is **complete and
13+
final answer, and terminal status. This proves the trace is **complete and
1414
sufficient** to reconstruct the run deterministically - the foundation for audit
1515
and (later) what-if replay. No provider key, no tool side effects, no sandbox.
16+
17+
(The replay records the same per-call ``Usage`` it read from the trace, so the
18+
replayed ledger reproduces the original cost too; the comparison below checks
19+
tool I/O, final answer, and status - it does not separately diff cost.)
1620
"""
1721
from __future__ import annotations
1822

@@ -145,16 +149,24 @@ def replay_trace(path: str, *, run_id: str, trace_dir: str = "traces") -> Tuple[
145149
res = orch.run(task)
146150

147151
original_results = [e.get("content", "") for e in events if e.get("kind") == "tool_result"]
148-
original_final = next((e.get("final_text", "") for e in events if e.get("kind") == "run_end"), "")
152+
original_run_end = next((e for e in events if e.get("kind") == "run_end"), {})
153+
original_final = original_run_end.get("final_text", "")
154+
original_status = original_run_end.get("status", "")
149155
new_events = load_trace(res.trace_path)
150156
new_results = [e.get("content", "") for e in new_events if e.get("kind") == "tool_result"]
151157

152158
tool_results_match = new_results == original_results
153159
final_match = res.final_text == original_final
160+
# Compare terminal status too: a run that ORIGINALLY ended e.g. "budget_exceeded"
161+
# must not silently replay as "max_steps" and still be reported as a match.
162+
status_match = res.status == original_status
154163
comparison = {
155-
"match": tool_results_match and final_match,
164+
"match": tool_results_match and final_match and status_match,
156165
"tool_results_match": tool_results_match,
157166
"final_match": final_match,
167+
"status_match": status_match,
168+
"original_status": original_status,
169+
"replayed_status": res.status,
158170
"original_final": original_final,
159171
"replayed_final": res.final_text,
160172
"n_tool_results": len(original_results),

examples/README.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Example artifacts
2+
3+
These are **real outputs** committed so you can see what Conductor produces
4+
without cloning and running it. All were generated key-free by the offline demos
5+
(`conductor demo` and `conductor sandbox-demo`).
6+
7+
| File | What it is |
8+
|---|---|
9+
| [`example-trace.jsonl`](example-trace.jsonl) | A run trace: every LLM request/response and tool call/result, one JSON line each (`run_start``llm_request``llm_response``tool_call``tool_result` → … → `run_end`). This is the substrate for observability and deterministic replay. |
10+
| [`example-ledger.jsonl`](example-ledger.jsonl) | The per-provider cost ledger for a two-provider `demo` run — one row per LLM call, keyed by `backend`, so cost splits per provider. |
11+
| [`example-sandbox-trace.jsonl`](example-sandbox-trace.jsonl) | A `sandbox-demo` trace showing the OS-isolation story: `sandbox` setup → a destructive `run_shell` (snapshotted first) → `sandbox_rollback` → teardown, with the host untouched. |
12+
13+
Regenerate them yourself (no API key needed):
14+
15+
```bash
16+
conductor demo --trace-dir examples
17+
conductor sandbox-demo --trace-dir examples
18+
```
19+
20+
Replay a trace deterministically (reproduces the recorded tool I/O + final answer
21+
+ terminal status, with no provider calls and no tool side effects):
22+
23+
```bash
24+
conductor replay --trace examples/example-trace.jsonl
25+
```

examples/example-ledger.jsonl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{"task_id": "1782697895239-anthropic-69bf76", "stage": "llm", "backend": "anthropic", "model": "claude-opus-4-8", "prompt_tokens": 50, "completion_tokens": 0, "total_tokens": 50, "cost_usd": 0.00025, "latency_ms": 0.0, "estimated": true}
2+
{"task_id": "1782697895239-anthropic-69bf76", "stage": "llm", "backend": "anthropic", "model": "claude-opus-4-8", "prompt_tokens": 57, "completion_tokens": 17, "total_tokens": 74, "cost_usd": 0.00071, "latency_ms": 0.0, "estimated": true}
3+
{"task_id": "1782697895239-local-201d09", "stage": "llm", "backend": "local", "model": "qwen2.5:3b-instruct", "prompt_tokens": 50, "completion_tokens": 0, "total_tokens": 50, "cost_usd": 0.0, "latency_ms": 0.0, "estimated": true}
4+
{"task_id": "1782697895239-local-201d09", "stage": "llm", "backend": "local", "model": "qwen2.5:3b-instruct", "prompt_tokens": 57, "completion_tokens": 17, "total_tokens": 74, "cost_usd": 0.0, "latency_ms": 0.0, "estimated": true}

0 commit comments

Comments
 (0)