feat(supervise): accept checked direct results - #661
Conversation
|
Real-artifact integration proof against the exact assignment-MGF validator:
|
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — a4d0a535
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
tangletools · auto-approval · reason: drewstone_author · 2026-07-30T06:47:58Z
tangletools
left a comment
There was a problem hiding this comment.
🟢 Value Audit — sound
| Verdict | sound |
| Concerns | 0 (none) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 144.9s (2 bridge agents) |
| Total | 144.9s |
💰 Value — sound
Lets the supervisor return its OWN directly-produced work as the run output, but only after the same injected completion check that gates workers passes — coherent, in-grain, no existing equivalent does this.
- What it does: Adds a
submit_resultcoordination verb, exposed ONLY when aDeliverableSpecis configured. The driver (both the in-process router arm and the sandboxed-harness arm) can submit a structured result; the samedeliverable.checkthat workers face viagateOnDeliverableruns against it, fail-closed. The first passing submission is frozen and retained, setsstopped=true(so the tool-loop's `stop - Goals it achieves: Unblocks the case where the driver does the work itself (via its own reasoning/work-tools) and that work independently passes the requested check, yet previously returned no winner because only DELIVERED children could enter completion. Extends the 'Foreman 0/18 — done means a check passed' invariant to direct driver work, WITHOUT letting the driver self-judge its raw prose (still ineligible).
- Assessment: Sound and well-aligned with the codebase. It reuses the existing
DeliverableSpectype and its exact fail-closed semantics (throwing check => not accepted, mirroring completion-gate.ts:53-57), threads the option through all four layers with the identical...(x ? {x} : {})spread pattern every other option uses, and checkssubmittedResult()before the finalizer in both arms so the children-onl - Better / existing approach: none — this is the right approach. I considered three alternatives and ruled each out: (1) Wrap the driver's executor with
gateOnDeliverable(src/runtime/supervise/completion-gate.ts:45) — rejected because it gates settlement validity from a PARENT's view, does not stop the driver's own loop early, and would gate unstructured chat prose (exactly what the Foreman invariant forbids; the root drive - Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content
🎯 Usefulness — sound
Lets a supervisor leader submit its own work through the same injected completion check workers already face, plumbed via the established DeliverableSpec seam into both supervisor arms — no new watcher/scanner/result-format.
- Integration: Fully wired and reachable. The deliverable flows supervise() (supervise.ts:237) → supervisorAgent (supervisor-agent.ts:167,200) → BOTH arms: the router/driverAgent (coordination-driver.ts:314 mounts it, :337-348 exposes submit_result as a real brain-visible tool) and the sandbox/serveCoordinationMcp arm (coordination-mcp.ts:97). Acceptance stops the loop the same way
stopdoes (coord.isStopped() - Fit with existing patterns: Fits the codebase grain precisely. It reuses the existing DeliverableSpec oracle (completion-gate.ts:32) — the SAME check workers gate on via gateOnDeliverable (completion-gate.ts:45). It extends the completion path to driver-authored work WITHOUT contradicting the documented 'driver cannot self-declare done' invariant: raw leader prose stays ineligible; only a check-passing submission counts
- Real-world viability: Handles the non-happy paths. Failing check → {accepted:false, stop:false}, driver continues (coordination.ts:1040); throwing check → fail-closed (coordination.ts:1037-1039); backward compatible (no deliverable ⇒ no submit_result tool ⇒ finalizer fallback preserved). First-passing-wins under concurrent remote submissions is correct for single-threaded async — the double
submittedguard straddles - Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
No concerns — sound change, no better or existing approach found. ✅
What this audit checks
It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.
| Pass | What it asks |
|---|---|
| Heuristic | Vague title? Whitespace-only or cruft-bearing diff? (content signals only) |
| Duplication | Do added function/class names already exist elsewhere in the repo? |
| Value Audit | What does it do? What goal does it achieve? Is it good? Better architecture or already-exists? |
| Usefulness Audit | Does it integrate and fit? Will it hold up in real use and actually get used? |
Findings are concerns, not blocks — the human reviewer decides what to do with them.
What changed
submit_resultonly when the existingDeliverableSpecis suppliedWhy
A real GLM-5.2 scientific run produced and independently passed the requested artifact, but the supervisor returned no winner because only child outputs could enter completion. This binds the already-existing completion check to direct leader work instead of adding a watcher, scanner, result format, or domain rule.
Proof
GIT_ALLOW_TEST_IDENTITY=1 pnpm test)origin/main(git merge-tree --write-tree origin/main HEAD)