Skip to content

Commit 1b323fc

Browse files
mabry1985claude
andauthored
feat(loop): wire ADR 0064 P2 board seam — execution-grounded coder.solve() (#61)
* feat(loop): wire ADR 0064 P2 board seam — execution-grounded coder.solve() On a fresh build, when the `coder` plugin is importable, the feature has acceptance criteria, and a runnable acceptance-test command is configured, the loop now dispatches through coder.solve()'s execution-grounded ladder (greedy → best-of-k → tree-search) instead of a single delegate_to(acp) shot — gated on the feature's acceptance tests actually passing in a real candidate worktree, never an LLM judge. - coder_seam.py: a thin adapter composing plugins.coder.solve (imported lazily/best-effort — no hard dependency on the coder plugin). generate() creates a fresh throwaway worktree per candidate and dispatches the ACP coder into it; verify() runs the configured acceptance-test command in that worktree and reports real pass/fail. The winner is promoted to the feature's canonical worktree/branch; losers are reaped. A spent budget with no passing candidate raises SolveExhausted (a WorktreeError subclass), which the existing capability-failure handling treats exactly like NoChangesError/CoderTimeout — so the coders-map tier ladder still climbs when search itself stalls. - Honest degrade: should_use_solve requires all three gates (coder importable + acceptance_criteria + a test command); missing any falls back to today's single shot, so no existing deployment can regress. - store.py: record_gens_spent accumulates coder.solve()'s generation cost onto a `gens:<total>` label; `_project` surfaces it as `gens_spent` so portfolio_rollup can read cost without raw reads. - Deferred: compiling EARS acceptance criteria into generated tests. The coder is already prompted to write tests satisfying the acceptance criteria as part of its definition of done; verify() runs whatever tests exist via the configured command rather than fabricating them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(coder_seam): reap candidate worktrees when solve() raises mid-ladder coder.solve() (plugins/coder/solve.py) has no try/except around its injected generate()/verify() callables — it assumes a candidate attempt only ever fails its tests, never errors. But a real ACP dispatch can raise for real (CoderTimeout on one best-of-k candidate, a worktree op erroring), and that propagated straight out of dispatch() uncaught, leaking every worktree already created for that run: untracked in the loop's `_inflight` map until dispatch() returns, and invisible to the health sweep (a `.gN` candidate id isn't a real board feature, so the sweep's own get_feature() lookup raises and it skips reaping). This was a much more common trigger than the "crash mid-ladder" case the PR already called out as a known limitation — a single slow candidate in best-of-k hitting coder_timeout_s would hit it on any normal run. Wrap the solve() call so any exception reaps every candidate seen so far, surfaces the attempted generation count as spent cost (a failed dispatch still spent the gen), and re-raises the original exception unchanged so the loop's existing capability-failure classification (retry/escalate/block) still applies. Also drop the `typing.Optional` usage in this file for the `X | None` style every other module here already uses under `from __future__ import annotations`, and hoist the (always-available, stdlib) `importlib` import to the top of the file with the rest of the module's imports. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(coder_seam,loop): guard record_gens fire-and-forget; fix Max-Mode/coder_solve precedence Two adversarial-review blockers on the ADR 0064 P2 board seam: - coder_seam.dispatch()'s two record_gens(...) calls (mid-ladder-raise and post-solve()) were unguarded, even though store.record_gens_spent's own docstring promises fire-and-forget semantics. A transient BoardError from a `br` hiccup would propagate past _drive's capability-failure handling into the outer `except BoardError`, which never reaps a worktree — leaking an already-verified (or already-reaped) candidate and blocking the feature over a bookkeeping label write. Wrapped both call sites in a shared _record_gens_best_effort() helper that logs and swallows. - _use_coder_solve() let coder.solve() preempt Max-Mode purely because the separate `coder` plugin became importable, even on a board already configured with max_mode_n>1 + local_gate_cmd (the README's own documented execution-grounded Max-Mode recipe). Since coder_solve defaults on and its test-cmd falls back to local_gate_cmd, that board flips from "always ships a best-effort PR" to "blocks outright on an exhausted search" with zero config change of its own. coder.solve now only preempts Max-Mode when max_mode_n<=1; a board wanting the ladder over Max-Mode must opt in explicitly. Added tests pinning both fixes (record_gens raising on the promoted-winner, exhausted, and mid-ladder-raise paths; Max-Mode-wins-precedence). --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 1e40213 commit 1b323fc

9 files changed

Lines changed: 1414 additions & 6 deletions

File tree

README.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,16 @@ board to a PR — or fork it as a starting point.
4646
requires a spec, EARS acceptance criteria, and explicit `files_to_modify`.
4747
- **Escalation (opt-in)** — with a `coders` map of >1 distinct delegate, a capability
4848
failure climbs a model tier (`fast→smart→reasoning`) and blocks at the top.
49+
- **coder.solve() board seam (ADR 0064 P2)** — on a fresh build, when the
50+
[`coder`](https://github.com/protoLabsAI/protoAgent/tree/main/plugins/coder) plugin
51+
is enabled AND the feature has acceptance criteria AND `coder_solve_test_cmd` (or
52+
`local_gate_cmd`) is set, the loop dispatches through `coder.solve()`'s
53+
execution-grounded ladder (greedy → best-of-k → tree-search) instead of a single
54+
`delegate_to(acp)` shot — gated on the feature's acceptance tests actually PASSING
55+
in a real candidate worktree, never an LLM judge. Composes WITH the tier ladder
56+
above (solve() searches *within* a tier; a search that never passes escalates a
57+
tier, or blocks, exactly like a no-diff dispatch). Missing coder/acceptance/test
58+
command ⇒ honest degrade to the single shot — see `coder_seam.py`.
4959
- **Planning layer** — two reasoning subagents (`decompose` + `antagonist`) driven by
5060
the `decompose-project` skill: idea → outline → MADR ADRs → epics › milestones ›
5161
features, hardened by an adversary, with a per-epic human gate.
@@ -99,6 +109,11 @@ project_board:
99109
# With local_gate_cmd set, Max-Mode is EXECUTION-GROUNDED (ADR 0064): the winner is
100110
# picked from candidates whose gate actually PASSES; the LLM judge only breaks ties
101111
# among the passing set (or decides when no gate is set / none pass).
112+
coder_solve: true # OPT-OUT valve for the ADR 0064 P2 seam (default on; the
113+
# real gate below still requires the `coder` plugin +
114+
# acceptance criteria + a test command — see "What it does").
115+
coder_solve_test_cmd: "pytest tests/ -q" # solve()'s verify() oracle; falls back to
116+
# local_gate_cmd if blank, else the seam honest-degrades.
102117
# webhook_secret: "..." # set before exposing /webhook/pr publicly
103118
```
104119

@@ -124,6 +139,7 @@ project_board:
124139
| `store.py` | the `br`/beads wrapper — board projection + the Ready/Done invariants |
125140
| `loop.py` | the puller: `ready → worktree → coder → PR → in_review` (+ opt-in escalation) |
126141
| `worktree.py` | per-feature worktree lifecycle, scoped coder dispatch, `open_pr` |
142+
| `coder_seam.py` | the ADR 0064 P2 seam — dispatches a build through `coder.solve()` when available, else honest-degrades |
127143
| `api.py` | the HTTP API + the `/webhook/pr` Done edge (HMAC-verified) |
128144
| `board_view.py` | the Kanban/list console view |
129145
| `retro.py` | loop-retro mining: bead attempt/outcome history → recurring failure classes (the self-improving flywheel) |

coder_seam.py

Lines changed: 331 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,331 @@
1+
"""The P2 board seam (ADR 0064): dispatch a feature's build through the `coder`
2+
plugin's execution-grounded ``solve()`` ladder instead of a single
3+
``delegate_to(acp)`` shot — greedy → best-of-k → tree-search, gated on the
4+
feature's acceptance tests actually PASSING in a real worktree, never an LLM judge.
5+
6+
**Composes** `plugins.coder.solve` (a separate, git-URL-installed plugin — imported
7+
lazily/best-effort so this repo carries no hard dependency on it and no import-time
8+
coupling) with THIS repo's own worktree primitives. The coder plugin never sees a
9+
board worktree; it only supplies the deterministic ladder (`solve()`, `Budget`,
10+
`Verdict`). Each candidate the ladder tries gets its OWN throwaway worktree — the
11+
"independent-parallel acp attempts" `coder`'s own generator module already flags as
12+
"the P2 path" (`plugins/coder/generate.py`) — and the winning (test-passing)
13+
candidate is PROMOTED to the feature's canonical worktree/branch so the rest of the
14+
drive (fixups, the pre-PR local gate, `open_pr`, the CI bounce, tier escalation) is
15+
UNCHANGED; every other candidate is reaped.
16+
17+
**Honest degrade** (ADR 0064's no-LLM-judge rule, applied at the board layer): the
18+
dispatch decision (``should_use_solve``) requires ALL THREE — the `coder` plugin
19+
importable (the host has it enabled), the feature's acceptance criteria present
20+
(the Ready gate's oracle), and a configured, runnable acceptance-test command (the
21+
actual executable verifier — `solve()` cannot run prose). Missing any of the three
22+
⇒ the caller falls back to today's single ``delegate_to(acp)`` shot; never a silent
23+
best-of-k/judge substitute.
24+
25+
**Deferred** (see the ADR + the PR that lands this): compiling EARS acceptance
26+
criteria into a generated test file. The simplest-correct path used here instead:
27+
the coder is already prompted (``loop._build_prompt``) to write tests satisfying
28+
the acceptance criteria as part of its definition of done; this module's ``verify``
29+
just RUNS whatever tests exist in a candidate's worktree via the configured command
30+
and gates on its exit code — real execution, no fabricated grounding.
31+
"""
32+
33+
from __future__ import annotations
34+
35+
import asyncio
36+
import importlib
37+
import logging
38+
from typing import Callable
39+
40+
from . import worktree
41+
42+
log = logging.getLogger("protoagent.plugins.project_board")
43+
44+
45+
class SolveExhausted(worktree.WorktreeError):
46+
"""``coder.solve()`` spent its whole generation budget against this feature's
47+
acceptance tests and no candidate passed. A CAPABILITY failure for the CURRENT
48+
model tier — real diffs existed, they just failed the tests, so this is NOT
49+
"no diff" — but the loop treats it exactly like ``NoChangesError``/
50+
``CoderTimeout``: escalate a configured tier ladder, or block. Never opens a PR
51+
on an unverified best-partial (ADR 0064's honest-degrade contract)."""
52+
53+
54+
def _import_solve():
55+
"""Best-effort import of the `coder` plugin's solve library. Returns the module,
56+
or ``None`` — `coder` is a separate, git-URL-installed plugin (ADR 0064), not a
57+
dependency of this one, and it ships DISABLED by default, so absent/disabled is
58+
the expected common case (not an error worth logging)."""
59+
try:
60+
return importlib.import_module("plugins.coder.solve")
61+
except Exception: # noqa: BLE001 — coder plugin absent/disabled → honest degrade
62+
return None
63+
64+
65+
def should_use_solve(feature: dict, *, test_cmd: str, _solve_mod=None) -> bool:
66+
"""The P2 dispatch decision (ADR 0064): use `coder.solve()` only when ALL of —
67+
the `coder` plugin is importable, the feature carries acceptance criteria (the
68+
Ready gate's oracle), and a runnable acceptance-test command is configured (the
69+
actual verifier `solve()` gates on — prose acceptance criteria alone isn't
70+
executable). Missing any ⇒ False, the honest degrade to a single delegate_to(acp)
71+
shot. ``_solve_mod`` is a test-injection seam; production callers never pass it —
72+
the real best-effort import happens here."""
73+
mod = _solve_mod if _solve_mod is not None else _import_solve()
74+
if mod is None:
75+
return False
76+
if not str(feature.get("acceptance_criteria") or "").strip():
77+
return False
78+
if not str(test_cmd or "").strip():
79+
return False
80+
return True
81+
82+
83+
def _augment_prompt(task: str, feedback: str | None) -> str:
84+
"""Fold the ladder's failing-test feedback into the next candidate's prompt.
85+
Every candidate gets a FRESH worktree off base (see ``_WorktreeSolveAdapter`` —
86+
the same "fresh-both" discipline ``worktree.dispatch_coder`` already documents
87+
for re-dispatches), so the coder is told explicitly there is no prior diff to
88+
build on in THIS worktree — only the failure to fix."""
89+
if not feedback:
90+
return task
91+
return (
92+
f"{task}\n\n"
93+
"## Your previous attempt FAILED the acceptance tests — fix exactly this\n"
94+
"This is a fresh worktree (no prior diff here); re-implement with the failure "
95+
f"below in mind:\n{feedback.strip()}\n"
96+
)
97+
98+
99+
class _WorktreeSolveAdapter:
100+
"""Adapts `coder.solve()`'s ``generate``/``verify`` contract onto board
101+
worktrees. `solve()` treats a candidate as an opaque string; here that string is
102+
a candidate's WORKTREE PATH, not code text — the coder edits files, it doesn't
103+
return a source string. Each ``generate()`` call creates a fresh throwaway
104+
worktree, dispatches the ACP coder into it, and hands the path back; ``verify()``
105+
then runs the acceptance-test command in that same worktree and reports real
106+
pass/fail. Tracks every candidate worktree it creates so the caller can promote
107+
the winner and reap the losers."""
108+
109+
def __init__(
110+
self,
111+
*,
112+
repo: str,
113+
base: str,
114+
root: str,
115+
fid: str,
116+
coder,
117+
dispatch_timeout: float | None,
118+
test_cmd: str,
119+
test_timeout: float,
120+
verdict_cls,
121+
):
122+
self.repo = repo
123+
self.base = base
124+
self.root = root
125+
self.fid = fid
126+
self.coder = coder
127+
self.dispatch_timeout = dispatch_timeout
128+
self.test_cmd = test_cmd
129+
self.test_timeout = test_timeout
130+
self.verdict_cls = verdict_cls # `plugins.coder.solve.Verdict` — passed in, never imported here
131+
self.candidates: list[tuple[str, str]] = [] # (worktree_path, branch)
132+
# `git worktree add` against the SAME repo must not run concurrently (best-
133+
# of-k dispatches `generate()` via asyncio.gather) — serialize just that
134+
# step; the slow part (the coder dispatch) still runs in parallel.
135+
self._wt_lock = asyncio.Lock()
136+
self._n = 0
137+
138+
async def generate(self, task: str, *, feedback: str | None = None) -> str:
139+
self._n += 1
140+
cid = f"{self.fid}.g{self._n}"
141+
async with self._wt_lock:
142+
wt, branch = await worktree.create_worktree(self.repo, self.base, cid, self.root)
143+
self.candidates.append((wt, branch))
144+
await worktree.dispatch_coder(self.coder, wt, _augment_prompt(task, feedback), timeout=self.dispatch_timeout)
145+
return wt
146+
147+
async def verify(self, candidate_wt: str):
148+
Verdict = self.verdict_cls
149+
try:
150+
proc = await asyncio.create_subprocess_shell(
151+
self.test_cmd,
152+
cwd=candidate_wt,
153+
stdout=asyncio.subprocess.PIPE,
154+
stderr=asyncio.subprocess.STDOUT,
155+
)
156+
except OSError as exc:
157+
return Verdict(passed=False, total=1, failed=1, output=f"could not launch acceptance tests: {exc}")
158+
try:
159+
out, _ = await asyncio.wait_for(proc.communicate(), timeout=self.test_timeout)
160+
except asyncio.TimeoutError:
161+
try:
162+
proc.kill()
163+
except ProcessLookupError:
164+
pass
165+
await proc.wait()
166+
# Unlike the pre-PR local gate (which fails OPEN on a timeout — a broken
167+
# gate must never block otherwise-good work), THIS is the ladder's own
168+
# search oracle: a candidate we couldn't confirm passed must never be
169+
# silently treated as passing, or we'd be faking grounding.
170+
return Verdict(
171+
passed=False, total=1, failed=1, output=f"acceptance tests timed out after {self.test_timeout:.0f}s"
172+
)
173+
text = (out or b"").decode("utf-8", "replace").strip()
174+
ok = proc.returncode == 0
175+
return Verdict(
176+
passed=ok,
177+
total=1,
178+
failed=0 if ok else 1,
179+
failing=[] if ok else [f"{self.test_cmd!r} (exit {proc.returncode})"],
180+
output=text[-4000:],
181+
)
182+
183+
184+
RecordGens = Callable[[int], None]
185+
186+
187+
def _record_gens_best_effort(record_gens: RecordGens, fid: str, n: int) -> None:
188+
"""Call ``record_gens(n)`` and swallow any exception it raises.
189+
190+
``store.record_gens_spent`` documents itself as fire-and-forget ("a br hiccup
191+
here must never fail the build the way a missing PR would") — a transient `br`
192+
failure (lock contention, a flaky CLI invocation, a race with a concurrent
193+
label write) must never propagate out of ``dispatch()``. Left unguarded, it
194+
would surface as an unrelated ``BoardError`` past every capability-failure
195+
handler in the caller's loop, discarding an already-verified (or already-
196+
reaped) candidate purely because of a bookkeeping label write."""
197+
try:
198+
record_gens(n)
199+
except Exception: # noqa: BLE001 — fire-and-forget cost accounting, never fails the build
200+
log.warning("[project_board] %s record_gens(%d) failed (ignored — fire-and-forget)", fid, n, exc_info=True)
201+
202+
203+
async def dispatch(
204+
*,
205+
task: str,
206+
coder,
207+
repo: str,
208+
base: str,
209+
root: str,
210+
fid: str,
211+
dispatch_timeout: float | None,
212+
test_cmd: str,
213+
test_timeout: float,
214+
budget: int,
215+
k: int,
216+
tree_depth: int,
217+
record_gens: RecordGens | None = None,
218+
_solve=None,
219+
_budget_cls=None,
220+
_verdict_cls=None,
221+
) -> tuple[str, str, str]:
222+
"""Run the execution-grounded ladder for one feature build.
223+
224+
Returns ``(worktree, branch, result_text)`` on a passing candidate — the SAME
225+
3-tuple shape ``_dispatch_max_mode`` returns, so the caller's downstream drive
226+
(fixups, local gate, ``open_pr``) is unchanged. Raises :class:`SolveExhausted`
227+
(a capability failure) when the budget is spent with no passing candidate, after
228+
reaping every candidate worktree it created.
229+
230+
``record_gens`` (if given) is called with ``result.gens_spent`` exactly once,
231+
win or lose — the cost accounting (ADR 0064) must survive a failed search too.
232+
``_solve``/``_budget_cls``/``_verdict_cls`` are test-injection seams for
233+
``solve()``/``Budget``/``Verdict``; production callers never pass them (the real
234+
import happens here, deferred so this module carries no hard dependency on the
235+
`coder` plugin).
236+
237+
**``solve()`` itself can raise.** The ladder (`coder`'s own ``solve.py``) has no
238+
try/except around ``generate``/``verify`` — it assumes a candidate attempt never
239+
errors, only that it might fail its tests. A REAL dispatch can still raise
240+
(``CoderTimeout`` on one best-of-k candidate, a worktree op erroring) and that
241+
propagates straight out of ``solve()`` uncaught. Every worktree ``generate()``
242+
already created for THIS run is tracked in ``adapter.candidates`` (appended right
243+
after ``create_worktree`` returns, before the dispatch that might fail) but would
244+
otherwise leak forever: it's untracked in the loop's ``_inflight`` map until this
245+
function returns, and invisible to the health sweep (a `.gN` candidate id isn't a
246+
real board feature, so the sweep's own ``get_feature`` lookup raises and the sweep
247+
skips it rather than reaping). So any exception here reaps every candidate seen so
248+
far, surfaces the attempted cost, and re-raises the ORIGINAL exception unchanged —
249+
the loop's existing capability-failure handling (retry/escalate/block) still
250+
applies to whatever it actually was."""
251+
if _solve is not None:
252+
solve, Budget, Verdict = _solve, _budget_cls, _verdict_cls
253+
else:
254+
from plugins.coder.solve import Budget, Verdict, solve
255+
256+
adapter = _WorktreeSolveAdapter(
257+
repo=repo,
258+
base=base,
259+
root=root,
260+
fid=fid,
261+
coder=coder,
262+
dispatch_timeout=dispatch_timeout,
263+
test_cmd=test_cmd,
264+
test_timeout=test_timeout,
265+
verdict_cls=Verdict,
266+
)
267+
try:
268+
result = await solve(
269+
task,
270+
generate=adapter.generate,
271+
verify=adapter.verify,
272+
budget=Budget(budget),
273+
k=k,
274+
tree_depth=tree_depth,
275+
)
276+
except Exception as exc:
277+
for wt, branch in adapter.candidates:
278+
await worktree.remove_worktree(repo, wt, branch)
279+
if record_gens is not None and adapter._n:
280+
# `solve()` never got to return a `gens_spent` count — the attempted
281+
# generation count is the honest stand-in (a failed dispatch still spent
282+
# the gen; ADR 0064's cost accounting doesn't get to look the other way).
283+
# Best-effort per store.record_gens_spent's own contract ("a br hiccup
284+
# here must never fail the build"): the worktrees above are ALREADY
285+
# reaped and the original exception below is what the loop must see —
286+
# a transient `br` failure recording the spend must never mask it.
287+
_record_gens_best_effort(record_gens, fid, adapter._n)
288+
log.warning(
289+
"[project_board] %s coder.solve raised mid-ladder (%d candidate(s) reaped): %s",
290+
fid,
291+
len(adapter.candidates),
292+
exc,
293+
)
294+
raise
295+
if record_gens is not None:
296+
# Same fire-and-forget contract as above: a bookkeeping failure here must
297+
# never discard a candidate that ALREADY exists on disk (test-verified or
298+
# not) — the promote/reap logic below still has to run either way.
299+
_record_gens_best_effort(record_gens, fid, result.gens_spent)
300+
301+
if not result.passed or not result.solution:
302+
for wt, branch in adapter.candidates:
303+
await worktree.remove_worktree(repo, wt, branch)
304+
detail = result.verdict.feedback() if result.verdict else ""
305+
log.info(
306+
"[project_board] %s coder.solve exhausted (rung=%s, gens=%d/%d) — no candidate passed",
307+
fid,
308+
result.rung,
309+
result.gens_spent,
310+
budget,
311+
)
312+
raise SolveExhausted(
313+
f"coder.solve exhausted after {result.gens_spent} generation(s) (rung={result.rung}): "
314+
f"{detail or result.note}"
315+
)
316+
317+
win_wt = result.solution
318+
win_branch = next(b for wt, b in adapter.candidates if wt == win_wt)
319+
canon_wt, canon_branch = await worktree.promote_worktree(repo, win_wt, win_branch, fid, root)
320+
for wt, branch in adapter.candidates:
321+
if wt != win_wt:
322+
await worktree.remove_worktree(repo, wt, branch)
323+
log.info(
324+
"[project_board] %s coder.solve verified by acceptance tests (rung=%s, gens=%d/%d)",
325+
fid,
326+
result.rung,
327+
result.gens_spent,
328+
budget,
329+
)
330+
result_text = f"[coder.solve rung={result.rung} gens={result.gens_spent}] {result.note}"
331+
return canon_wt, canon_branch, result_text

0 commit comments

Comments
 (0)