diff --git a/README.md b/README.md index 2fb635c..53d67dd 100644 --- a/README.md +++ b/README.md @@ -51,10 +51,11 @@ uv sync --extra dev The stock protocol-v3 contract spans three owned projects. It requires `team-harness>=0.5.4` for caller-owned run records, nested assignment context, capability-roster propagation, and canonical agent streams. When a workflow -uses evaluation, `eval-banana>=0.3.5` supplies hermetic +uses evaluation, `eval-banana>=0.5.0` supplies hermetic `--no-project-config` runs, explicit harness selection, canonical check -digests, and retained judge evidence. Install compatible releases of all three -projects together. +digests, retained judge evidence, and the `--result-out` provenance-stamped +result the stock `inner_outer_eval` eval runner writes to `eval_results.md` +(D14). Install compatible releases of all three projects together. For coordinated development across the repositories, install the corresponding team-harness and eval-banana checkouts as editable dependencies: diff --git a/design/analysis/inner-outer-eval-nonconvergence-postmortem.md b/design/analysis/inner-outer-eval-nonconvergence-postmortem.md new file mode 100644 index 0000000..5fe0bab --- /dev/null +++ b/design/analysis/inner-outer-eval-nonconvergence-postmortem.md @@ -0,0 +1,140 @@ +# Analysis — why an `inner_outer_eval` layer stalled after the work was done + +*Working notes, 2026-07-22. Non-binding (`design/analysis/`). The conclusions that +survive review are recorded as **D13–D15** in `design/decisions.md` and specified in +`design/designs/deterministic-handoff-and-eval-provenance.md`.* + +## The incident (worked example) + +A real Phase-0 build loop (`inner_outer_eval`, session +`001_goal-implement-programming-phase-0`, delivery child +`01_m1-integrated-auth-member-invitations`) delivered milestone **M1**: `inner` +implemented it and merged **PR #38**. A follow-up corrective, **PR #39**, was then opened, +went **green and mergeable**, and the loop's own pre-merge review wrote an explicit +**"APPROVE FOR MERGE — merge PR #39 at head `4cc2221`; residual medium items are not a +reopen of the original blockers"** verdict. PR #39 was never merged. The outer coordinator +instead spent hours re-polishing non-blocking items and never converged. `inner` behaved +correctly throughout; the failure was entirely in `outer`'s decision loop. + +Three independent substrate weaknesses combined to produce it. None is a bug in a single +function — each is a *missing* determinism guarantee that let a smart agent be misled by +its own durable state. + +## Root cause 1 — a schema-valid handoff can be silently stale + +`project_state/handoff.json` (owner: `outer`) is the rolling semantic summary a later +attempt trusts. At the stall it was **schema-valid but frozen** — `updated_at` hours old, +`accepted_outcomes: []`, `open_work` still naming "merge M1" — while the repository had +moved on (PR #38 merged; #39 open and approved). + +The engine already observes the handoff on every v3 finish +(`_observe_layer_handoff`, `coordinator_app.py`) and even checks producer identity — but +the check is too weak. `_handoff_producer_is_known` (5764–5783) accepts the handoff when its +producer is the current attempt **or any matching entry in the active session's history** +(the entry need not even have *succeeded*): + +```python +if (active.session_id == state.active_session_id + and producer.workflow_id == active.workflow_id + and producer.attempt_id == active.attempt_id): + return True +return any( + entry.session_id == state.active_session_id + and entry.workflow_id == producer.workflow_id + and entry.attempt_id == producer.attempt_id + for entry in state.history +) +``` + +A prior owner-role check (5715–5726) already limits this to the declared handoff owner, so the +remaining hole is purely *currency*: a handoff last stamped by a *prior* owner attempt passes as +`valid`. The revision-monotonicity check doesn't catch it either — an untouched file has the same +digest and revision, so "revision moved backward" never triggers. And terminal-control validation +(`_validate_control_handoff_reference`, 5559–5577) only checks the cited handoff matches the +*accepted snapshot* — but a stale handoff **is** the accepted snapshot, so completion citing it +passes. The engine had no way to know the current handoff-owner attempt had **not re-stamped** the +file — because nothing required it to. Staleness was undetectable by construction, at both the +next-attempt-reads-it and the declare-completion boundaries. + +## Root cause 2 — the eval verdict was written off-channel, and freshness was invisible + +The `eval_runner` prompt tells the agent to run `eval-banana`, read the resulting +`report.md`, and then **hand-transcribe** the verdict into `project_state/eval_results.md`. +Two failure modes follow from "the agent hand-copies a verdict into a path it types by +hand": + +- **Wrong destination.** In the incident the decisive approval landed in + `project_state/evidence/m1-corrective-review.md`, not the canonical + `project_state/eval_results.md` the outer prompt tells `outer` to read. `outer` read the + canonical (empty/stale) file, never saw the approval, and had no signal that a verdict + existed elsewhere. +- **No freshness signal.** `eval_results.md` carries no machine-readable provenance, so even + when it *is* present, a reader cannot tell whether a "PASS" reflects the current commit or + a verdict from three commits ago. The engine's expected-outputs advertisement compounds + this: the check-runner role advertises only `project_state/eval_state.md`, **never + `eval_results.md`** (see the comment at `_build_workflow_roster`), so the file the whole + role exists to produce is not a declared, enforceable output at all. + +The engine's decision here was deliberate and correct at the policy level — evaluation is +optional advisory evidence (**D11**), and the engine must not make a passing eval a gate. +But "advisory" was conflated with "un-located and un-versioned." Advisory evidence can still +be required to *land in a known place with known provenance* when the role runs; that is a +structural-integrity property (D8), not a semantic gate. + +## Root cause 3 — `outer` was never told where the ground truth is + +The outer prompt says "review inner's attempt on its merits — the diff, the repository +state, checks, and git/CI facts" and "read their verdict in `project_state/eval_results.md`." +It never tells `outer`: + +- that the raw per-iteration harness traces exist and where (`/raw//…`), or +- what to do when the durable summary is missing, ambiguous, or **contradicts the live + repository** — namely reconcile against live git/CI state rather than trust the summary. + +`paths.json` hands `outer` its own attempt's `trace_root`, but exposes **no** path to the +sibling/prior iterations' raw traces, so even a diligent `outer` had nothing to open. When +root cause 1 and 2 fed it a stale handoff and an empty results file, `outer` had no +instructed fallback, so it re-derived a to-do list from stale state and looped. + +This must be reconciled with **D12**: continuity should *not* routinely depend on re-reading +raw traces; the coordinator promotes conclusions into compact artifacts. The fix is not +"read traces every time" — it is "when the compact artifacts are missing or contradict the +repo, reconcile against live state and, as a bounded fallback, the traces." That is a safety +net for exactly the case where the compact-artifact contract was violated. + +## What is *not* the problem (rejected fixes) + +- **A "merge when approved + green" engine rule.** Rejected. That is a semantic completion + gate — precisely what D8/D11 forbid the engine from owning. The anti-over-polish guidance + belongs in the (user-owned, replaceable) workflow-set prompt, not the engine. +- **Making evals a mandatory gate.** Rejected — this is the exact D11 overcorrection already + reverted once. Evals stay optional and advisory. +- **A convergence/loop-budget heuristic in the engine.** Rejected. The bet is: fix the + deterministic substrate (provenance, currency, discoverability), keep the engine minimal, + and trust capable agents plus user-owned prompts. + +## The three determinism gaps, stated as requirements + +1. **Handoff currency is deterministic.** A successful handoff-owner attempt must re-stamp the + handoff with its own producer identity. A terminal completion citing an un-re-stamped handoff + is hard-rejected via the existing control-repair path; a non-completion owner finish that + leaves it stale is a soft, lenient, non-fatal nudge (diagnostic + repair note) — never a + death-spiral. Frozen-gated on a contract discriminator so live sessions are untouched. Eval + outputs carry producer identity so a past-attempt verdict is visibly stale. → **D13**. +2. **Advisory eval output is located by construction and freshness-visible — diagnostic, not + enforced.** eval-banana writes the machine-readable result to a caller-named destination with + *observed* provenance (commit + dirty digest, per-check `model`); the workflow-set contract + declares that destination; the engine emits at most a **presence diagnostic** (`eval_missing`) + and **never** reads the file, fails, respawns, or caps — the reading agent judges freshness + (D11 — evals stay optional/advisory). → **D14**, split loopy-loop ⇄ eval-banana. +3. **The orchestrator reconciles against ground truth.** The engine exposes the session raw root + to the orchestration role; the stock prompt gives `outer` the path map and the instruction to + reconcile the handoff against live repo/CI state (and, as a fallback, specific selected prior + traces) when the durable summary is missing or contradicts the repo. → **D15**. + +All three are structural-integrity / discoverability changes (D8-compatible), plus user-owned +prompt guidance; none adds a semantic gate to the engine. **Honest causality:** #1 and #2 stop the +orchestrator from being *misled* by stale durable state, but the behavior that actually converges +the M1 case — accept delivered/green/approved work, reconcile against the live repo — is the **#3 +prompt**. The substrate fixes are necessary and make the prompt's job reliable; they are not, by +themselves, a stop condition. diff --git a/design/analysis/review-codex-deterministic-provenance-r1.md b/design/analysis/review-codex-deterministic-provenance-r1.md new file mode 100644 index 0000000..bce9c2a --- /dev/null +++ b/design/analysis/review-codex-deterministic-provenance-r1.md @@ -0,0 +1,45 @@ +## Verdict + +Do not implement as written. Change A’s core check is legitimate structural integrity, and Change C belongs in the prompt, but the finish transaction, eval enforcement, and migration model are not implementable safely as specified. + +### Top 3 blockers + +1. **A stale handoff can still terminate successfully.** `_record_finished_task` observes the handoff, then independently applies control ([coordinator_app.py:1490–1547](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:1490)). `_apply_session_control` can set `goal_met` ([coordinator_app.py:5401–5407](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:5401)), and stop precedence gives that priority ([coordinator_app.py:5170–5176](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:5170)). Merely flipping local `success=False`, as proposed ([design:86–91](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/designs/deterministic-handoff-and-eval-provenance.md:86)), does not prevent accepted terminal control. The design must specify one atomic disposition: structural-output validation precedes control; on failure, current-attempt terminal control is not consumed and is neutralized or archived for repair. + +2. **The eval enforcement violates both D11 and the user-owned-contract invariant.** D11 says malformed advisory eval output must not turn an invocation into failure or starve orchestration ([decisions.md:441–451](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/decisions.md:441)). The proposal does exactly that through `success=False` plus repair/caps ([design:140–151](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/designs/deterministic-handoff-and-eval-provenance.md:140)). It also hard-codes `project_state/eval_results.md` in `_build_workflow_roster`, directly contradicting its own rule that this path lives only in the workflow-set ([design:20–25](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/designs/deterministic-handoff-and-eval-provenance.md:20)). Reject/quarantine stale eval evidence and emit a diagnostic; do not fail the iteration or add a terminal eval-output cap. + +3. **Frozen-v3 compatibility is false without a new feature discriminator.** `_validate_finished_binding` only returns the frozen contract ([coordinator_app.py:1615–1691](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:1615)). Old and new contracts are both schema 1/protocol 3 and already declare a handoff owner ([models.py:246–260](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/models.py:246), [contract.yaml:41–49](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/contract.yaml:41)). Therefore freezing does not distinguish old advisory observation from new mandatory currency enforcement. Add an explicit frozen capability/schema field, defaulting off for existing sessions. + +## Claim verification + +- **Historical-producer hole: confirmed, with a qualifier.** `_handoff_producer_is_known` accepts the current attempt or any matching history entry in the active session ([coordinator_app.py:5769–5783](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:5769)). It does not require that history entry to have succeeded. The prior owner-role check limits this to the declared handoff owner ([coordinator_app.py:5715–5726](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:5715)). + +- **`expected_outputs` is advisory-only: confirmed.** It is constructed for the roster ([coordinator_app.py:342–400](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:342)); `_record_finished_task` has no output linter. Handoff is diagnostic unless cited by control, and missing eval receipts remain diagnostic ([coordinator_app.py:5587–5594](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:5587)). Among ordinary role outputs, `control.json` is the only archive/placeholder/repair implementation ([coordinator_app.py:6243–6317](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:6243)). + +- **Runner/owner mismatch: confirmed.** `eval_results.md` is owned by `eval_runner`, while `check_runner_roles` is `[outer]` ([contract.yaml:38–49](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/contract.yaml:38)). Changing it is sensible for the stock template, but not behavior-neutral: `check_runner_roles` means “allowed to produce evaluation receipts” ([models.py:315–321](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/models.py:315)), gates receipt acceptance ([coordinator_app.py:5593–5616](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:5593)), and validates later citations ([coordinator_app.py:5541–5550](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:5541)). It revokes outer-produced receipt authority. Also, `eval_state.md` is not solely “the check author’s inventory”; the contract owns it with `outer` and permits both eval roles to contribute ([contract.yaml:29–31](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/contract.yaml:29)). + +- **Same-revision re-stamp diagnosis: confirmed; proposed safety is under-specified.** Current code flags every same-revision digest change ([coordinator_app.py:5727–5736](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:5727)), so changing producer/time is falsely non-monotonic. Preserve tamper detection by comparing parsed current and accepted snapshots after excluding exactly `producer` and `updated_at`, requiring the exact current owner attempt, and requiring non-decreasing time. A blanket “current producer permits same revision” exception would admit semantic tampering. + +## Additional gaps + +- The eval repair loop cannot respawn as claimed. The runner removes `eval_request.md` ([eval_runner prompt:22–26](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/workflows/eval_runner/prompt.txt:22)); failed-workflow retry still applies `run_when_requested` ([scheduler.py:107–147](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/scheduler.py:107)), whose predicate requires that marker ([scheduler.py:76–81](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/scheduler.py:76)). The trigger is neither contract-declared nor frozen into `CurrentTask`. + +- The stock outer prompt still makes a terminal eval mandatory ([outer prompt:40–46](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/workflows/outer/prompt.txt:40)), contradicting D11’s explicit permission to decide evaluation is unnecessary ([decisions.md:431–439](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/decisions.md:431)). The design never says to remove that sentence. + +- Currency checks must apply only after a mechanically successful attempt. Current observations run even when `request.success` is false. Otherwise a crashed owner attempt archives the prior valid handoff and masks the real failure. + +- `success=False` also increments the general workflow failure cap ([coordinator_app.py:1574–1576](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:1574), [coordinator_app.py:2598–2618](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:2598)). That competes with the proposed dedicated counter, `max_turns`, and normal scheduling. “Same role is re-dispatched” is not guaranteed: failed-role retry is only used when no normally eligible role exists ([scheduler.py:46–55](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/scheduler.py:46)). + +- Eval freshness is not adequately bound to repository state. The proposal tests only `commit != HEAD`, but dirty trees can differ at the same HEAD. Current `EvalReport` contains no git identity ([models.py:144–164](/Users/jpuc/code/moje/eval_banana/eval_banana_1/src/eval_banana/models.py:144)). Capture before/after commit plus dirty-tree digest; caller-echoed `--provenance-commit` is not an observation. + +- Judge provenance cannot be one global `{family, model, effort}`: model overrides are per check ([eval-banana models.py:80–84](/Users/jpuc/code/moje/eval_banana/eval_banana_1/src/eval_banana/models.py:80)), and actual settings are recorded per result ([harness_judge.py:250–298](/Users/jpuc/code/moje/eval_banana/eval_banana_1/src/eval_banana/runners/harness_judge.py:250)). + +- “Write a result on run error/no checks” lacks an implementation boundary. No-check and harness/config failures occur before report/output creation ([runner.py:338–373](/Users/jpuc/code/moje/eval_banana/eval_banana_1/src/eval_banana/runner.py:338)). Define a formal result status/error schema and CLI-level failure envelope. + +- The two design documents disagree: loopy-loop invokes nonexistent `--result-name` ([loopy design:116–122](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/designs/deterministic-handoff-and-eval-provenance.md:116)); the eval-banana interface does not define it ([eval design:33–43](/Users/jpuc/code/moje/eval_banana/eval_banana_1/design/output-destination-capability.md:33)). + +- Raw-trace fallback is discoverable but not bounded. The current assignment exposes only the current attempt trace ([assignments.py:492–495](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/assignments.py:492)); adding the session raw root is reasonable, but the prompt should select specific relevant attempts from history rather than scan the tree. The proposed path map also names undeclared `ledger.md` ([design:181–186](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/designs/deterministic-handoff-and-eval-provenance.md:181)). + +## Required redesign + +Implement a handoff-specific exact-attempt validator first, with explicit finish ordering and a frozen opt-in capability. Keep eval-banana’s atomic caller-named result, but make loopy-loop validation diagnostic/quarantine-only. Derive paths and owners from a typed workflow-set output declaration; do not hard-code `eval_results.md` or `eval_request.md`. Remove the mandatory-final-eval prompt sentence, formalize dirty-state and heterogeneous-judge provenance, and delete the hand-transcription version-skew fallback. diff --git a/design/analysis/review-codex-deterministic-provenance-r2.md b/design/analysis/review-codex-deterministic-provenance-r2.md new file mode 100644 index 0000000..5f44fc8 --- /dev/null +++ b/design/analysis/review-codex-deterministic-provenance-r2.md @@ -0,0 +1,45 @@ +## Blocker status + +1. **PARTIALLY RESOLVED** — `schema_version` is the appropriate contract-shape discriminator, and `_validate_finished_binding` returns the attempt-frozen contract ([coordinator_app.py:1615](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:1615)). But the raw-root addition is only role-gated, not `schema_version >= 2` gated ([design:207](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/designs/deterministic-handoff-and-eval-provenance.md:207)); moreover, `WorkflowSetContract.schema_version` currently accepts arbitrary integers and ignores unknown fields ([models.py:246](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/models.py:246)), so “every old contract is 1” is not a safe user-owned-contract assumption. + +2. **PARTIALLY RESOLVED** — The tiering and A2 exemption are specified ([design:97](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/designs/deterministic-handoff-and-eval-provenance.md:97)), and retaining `success=True` avoids the general cap at [coordinator_app.py:1574](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:1574). But `handoff_ref` remains optional ([models.py:695](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/models.py:695)) and its validator is called only when present ([coordinator_app.py:5472](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:5472)); completion can therefore omit it and bypass A1. Also, after a successful outer finish the stock scheduler unlocks `inner`, not an immediate outer repair ([inner config:1](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/workflows/inner/config.yaml:1), [scheduler.py:87](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/scheduler.py:87)). + +3. **RESOLVED** — Both the operative rule and compatibility section gate currency on the original `request.success` ([design:89](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/designs/deterministic-handoff-and-eval-provenance.md:89), [design:253](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/designs/deterministic-handoff-and-eval-provenance.md:253)). This addresses the current ordering where handoff observation precedes control processing ([coordinator_app.py:1483](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:1483)). + +4. **RESOLVED, with wording nit** — The binding algorithm now requires the exact finishing producer, non-decreasing time, and equality of all remaining parsed fields ([design:123](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/designs/deterministic-handoff-and-eval-provenance.md:123)). It is implementable because the accepted snapshot retains both the parsed `LayerHandoff` and raw JSON ([models.py:495](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/models.py:495)). D13 says “byte-equal,” while the binding design says parsed-model equality; those should use one definition. + +5. **RESOLVED** — Missing, malformed, or stale eval output is diagnostic/quarantine-only, with no success flip, respawn, or cap ([design:172](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/designs/deterministic-handoff-and-eval-provenance.md:172)). Leaving `check_runner_roles` unchanged is correct because it separately governs receipt acceptance and citation authority ([models.py:315](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/models.py:315), [coordinator_app.py:5549](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:5549)). Prompt removal is explicitly in the adoption plan ([design:295](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/designs/deterministic-handoff-and-eval-provenance.md:295)); the current pre-implementation sentence remains at [outer prompt:44](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/workflows/outer/prompt.txt:44). + +6. **PARTIALLY RESOLVED** — `--result-name` is removed, the new flags agree between documents, and observed git plus a status envelope are specified ([eval design:36](/Users/jpuc/code/moje/eval_banana/eval_banana_1/design/output-destination-capability.md:36), [eval design:78](/Users/jpuc/code/moje/eval_banana/eval_banana_1/design/output-destination-capability.md:78)). But only `model` is configurable per check ([models.py:80](/Users/jpuc/code/moje/eval_banana/eval_banana_1/src/eval_banana/models.py:80)); family and reasoning effort remain run-level settings ([harness_judge.py:244](/Users/jpuc/code/moje/eval_banana/eval_banana_1/src/eval_banana/runners/harness_judge.py:244)). The claim that all three can differ per check is false. The dirty-digest comparison contract is also undefined. + +7. **RESOLVED** — D15 is explicitly identified as load-bearing, while D13/D14 only prevent misleading substrate state ([design:17](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/designs/deterministic-handoff-and-eval-provenance.md:17), [decisions.md:635](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/decisions.md:635)). + +## New or remaining inconsistencies + +- D13 claims a handoff “can never be older than the owner’s last successful finish” ([decisions.md:570](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/decisions.md:570)), while A2 explicitly records a successful owner finish and leaves stale bytes intact ([decisions.md:539](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/decisions.md:539)). These cannot both be true. + +- D13 says terminal control “must cite” the handoff ([decisions.md:533](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/decisions.md:533)); the binding design only checks completion “citing the handoff” ([design:97](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/designs/deterministic-handoff-and-eval-provenance.md:97)), preserving the omission bypass. + +- A1 is not solely a control-cap disposition. `_reject_v2_control` increments the control counter ([coordinator_app.py:6270](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:6270)), then invalid control flips iteration success false and also reaches the general workflow cap ([coordinator_app.py:1544](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:1544), [coordinator_app.py:1574](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/coordinator_app.py:1574)). That contradicts the claimed “one disposition” ([design:255](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/designs/deterministic-handoff-and-eval-provenance.md:255)). + +- Eval-banana says git commit and dirty digest are observed before and after ([eval design:24](/Users/jpuc/code/moje/eval_banana/eval_banana_1/design/output-destination-capability.md:24)), but its envelope contains two commits and only one ambiguous digest ([eval design:63](/Users/jpuc/code/moje/eval_banana/eval_banana_1/design/output-destination-capability.md:63)). It also omits an algorithm identifier; loopy-loop’s live digest uses a specific algorithm ([git_evidence.py:19](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/src/loopy_loop/git_evidence.py:19)). Cross-tool equality is therefore not defined. + +- `kind: advisory` does not declare `md` versus `json` or an envelope schema, yet the engine is expected to parse provenance. Nor does the design say what quarantine does to the canonical file or accepted state. This leaves the core coupled to an undeclared eval-banana format. + +- The first-round hand-transcription fallback remains: both documents still tell an old runner to hand-write from `report.md` ([design:258](/Users/jpuc/code/moje/loopy_loop_4/loopy-loop/design/designs/deterministic-handoff-and-eval-provenance.md:258), [eval design:112](/Users/jpuc/code/moje/eval_banana/eval_banana_1/design/output-destination-capability.md:112)). + +## Remaining MUST-FIX, ranked + +1. Make `handoff_ref` mandatory for schema-2 contracts declaring `kind: handoff`, and reject omission plus stale producer through `_reject_v2_control`. + +2. Resolve A2’s semantic contradiction: either add a specific non-failure repair scheduling mechanism that runs the owner before successors, or retract the guarantee that every successful owner finish leaves current handoff bytes. + +3. Define the cross-repo eval envelope completely: declared format/schema, digest algorithm, before/after dirty digests, exact status classification, and concrete quarantine storage/acceptance semantics. + +4. Specify cap accounting. Decide whether A1 intentionally increments both control and workflow caps; remove the false “one disposition” claim otherwise. + +5. Gate raw-root exposure on contract schema 2, validate supported contract schema versions, and validate currency paths, uniqueness, confinement, declared owners, and consistency with `orchestration.handoff_owner`. + +6. Remove the hand-transcription fallback and correct the per-check judge/provenance wording. + +**Final verdict: No.** The revision fixes the advisory-eval and causality blockers, but completion still has an omission bypass and the soft handoff path contradicts its claimed currency guarantee. diff --git a/design/analysis/review-grok-deterministic-provenance-r1.md b/design/analysis/review-grok-deterministic-provenance-r1.md new file mode 100644 index 0000000..a1401ee --- /dev/null +++ b/design/analysis/review-grok-deterministic-provenance-r1.md @@ -0,0 +1,133 @@ +I'll critically review the design against the source, starting with the design corpus and the coordinator/model code the claims rest on.Reading the coordinator functions and contract details that the design claims rest on.Checking how assignment surfaces attempt identity (for stamp fallback) and whether `LoopState` schema changes would break frozen sessions:# Design review: deterministic handoff + eval provenance + +## Code-grounded claims: confirm / refute + +| Claim | Verdict | Evidence | +|---|---|---| +| `_handoff_producer_is_known` accepts any past attempt | **Confirmed** (quote slightly incomplete) | Current attempt **or** any `state.history` entry with same session/workflow/attempt (`coordinator_app.py:5764-5783`). Design omits `session_id` match. | +| Untouched stale handoff is `valid` | **Confirmed** | Same digest + same revision never trips `non_monotonic` (`:5727-5736`). Observation is non-gating (`:5672` docstring; success only flipped by control/goal_check). | +| `expected_outputs` advisory-only; only control repaired | **Mostly confirmed; imprecise** | No finish-time linter over `expected_outputs`. **v3 control** is reject/repair (`:6243-6317`). **v1 goal_check** also flips success/caps (`:1501-1526`). Handoff observation feeds **terminal** `handoff_ref` validation only (`:5559-5577`), not mid-loop repair. | +| Stock `check_runner_roles: [outer]`; `eval_results.md` owned by `eval_runner` | **Confirmed** | `contract.yaml:38-49`. Roster advertises `eval_state.md` for check-runner, **not** `eval_results.md` (`coordinator_app.py:367-375`) — despite comment claiming check-runner writes `eval_results.md`. | +| Move check_runner → `[eval_runner]` is safe | **Mostly safe for stock path; not free** | `_accept_current_eval_receipts` only seals when `active.workflow_id ∈ check_runner_roles` and never gates (`:5580-5594`). Stock prompts write `eval_results.md`, not `EvalReceipt` JSON. Caveats below. | +| Same-rev provenance re-stamp is misflagged `non_monotonic` today | **Confirmed** | Same revision + any digest change → `non_monotonic` (`:5727-5736`). Producer/`updated_at` are in `LayerHandoff` bytes (`models.py:420-438`), so a pure re-stamp **must** change digest. | +| Relaxation “won’t break” tamper detection | **Not proven; under-specified** | Design says “provenance-only” but never defines the structural compare. Naïve “same rev + current producer OK” **does** destroy non_monotonic. | + +--- + +## 1. Correctness (wrong / imprecise) + +1. **“Only control.json is enforced”** — wrong for the engine as a whole (goal_check path). Right for v3 mid-loop role outputs of interest. Cite `coordinator_app.py:1501-1547`. + +2. **Postmortem’s `_handoff_producer_is_known` snippet** drops `entry.session_id == state.active_session_id` and the current-task branch (`:5772-5782`). Conclusion still holds. + +3. **Comment vs code at roster** (`:369-375`): comment says check-runner produces `eval_results.md`; code still appends `eval_state.md`. Design inherits that mess when it says “first enforced expected_output.” + +4. **`models.py:588-589`** still says handoff observation “never gates scheduling/control.” Design **does** gate finish success / re-dispatch. That’s a deliberate model change; docs that still say “diagnostic only” are wrong the moment D13 ships. Terminal control already soft-gates via `handoff_ref` (`:5559-5577`). + +5. **D14 / design: “path not in engine core”** while Change B §2 and phase 3 hardcode `project_state/eval_results.md` in `_build_workflow_roster`. That is the same hardcoding pattern as `handoff.json` / `plan.md` today (`:355-375`) — so either the invariant is already violated, or the design’s wording is false. + +6. **“Triggered attempt = eval_request stood at dispatch”** is underspecified relative to real lifecycle: scheduler uses presence of `eval_request.md` (`scheduler.py:76-81`, `sessions.py:904-910`); **eval_runner prompt deletes it during the run**. Finish-time existence is a race against a successful run. For stock `run_when_requested: true` on `eval_runner` only, **“role finished” ≈ “was triggered”** — the dual “request-gated enforcement” path is mostly noise if check_runner is only `eval_runner`. + +--- + +## 2. Invariant violations + +**Inv 1 (D11 optional/advisory eval)** — **OK if implemented carefully.** Presence/stamp of a file when the check-runner **runs** is structural, not “must pass.” Risk: treating `run_passed: false` or commit mismatch as engine failure would violate D11; design keeps that prompt-only (good). Respawning for **missing** stamp is D8-shaped, not D11, as long as sessions with empty `check_runner_roles` are untouched (`:5593` pattern). + +**Inv 2 (user-owned contracts; no hard-coded paths)** — **Violated by design text as written.** Wiring `eval_results.md` into engine roster/enforcement embeds a stock path. Fix: derive currency-tracked outputs from **declared contract** (e.g. `state[]` entries with a `currency: true` / `kind: role_output` flag, or contract-level `currency_outputs`), not string literals in `coordinator_app.py`. + +**Inv 3 (D8 structural vs semantic)** — **Handoff current-attempt ownership is a legitimate D8 carve-out** (matches “current-attempt ownership… stale input,” `decisions.md:288-291`). **Eval stamp presence** is also structural. **Not structural:** engine interpreting “commit ≠ HEAD ⇒ must re-run” as a gate (design leaves to prompts — keep it there). **Danger zone:** same-rev relaxation that accepts arbitrary content rewrite = smuggled hole in hash/monotonic integrity, not a semantic gate, but it **weakens** the structural story. + +**Inv 4 (D12 compact continuity)** — **OK.** Raw root is discoverability only; routine path still handoff/plan. Risk is prompt wording that makes raw-trace reading the default — keep “fallback when summary contradicts live state.” + +**Inv 5 (minimal engine)** — **Mostly OK.** No merge-when-approved rule. Adding a generalized “role-output currency linter” + per-kind counters + markdown provenance parser is still real engine surface. If the parser embeds eval-banana’s fenced schema, that’s cross-tool coupling not declared as an engine protocol. + +--- + +## 3. Risks / gaps / edge cases + +**A. Same-revision re-stamp (highest implement risk)** +Must specify: load `accepted_handoff_snapshot`; accept same `revision` iff +`producer` is the finishing attempt **and** all fields except `producer`/`updated_at` equal the prior accepted payload (or raw canonicalized equal after stripping those keys). +Otherwise either (i) re-stamp stays `non_monotonic` and D13 is self-blocking, or (ii) full content rewrite at fixed revision becomes legal. + +**B. Conditional eval enforcement** +- Finish-time `eval_request.md` check is wrong (file consumed). +- Prefer: enforce currency on **every finish of a declared check_runner role** (stock set already only schedules that role when requested). +- “Skipped/no-request stamped file” is optional polish, not a second enforcement mode. + +**C. Stamp verification for `eval_results.md` under-specified** +eval-banana design gives a parseable fence (`output-destination-capability.md:54-64`). loopy design says “presence + stamp” without: required parse rules, failure if fence missing but file exists, coupling to eval-banana schema version, or whether engine only checks attempt id and ignores `run_passed`. + +**D. Re-stamp every owner iteration** +Outer must rewrite handoff even on no-op iterations or burn `handoff_protocol_consecutive_failures` toward shared cap → `handoff_protocol_broken`. That can terminate a healthy session for forgetfulness, not corruption. No backoff, no “warn N times,” no distinction between missing file and stale producer. + +**E. Dual failure on one finish** +Order today: observe handoff → apply control (`:1491-1547`). If currency fails and control also invalid, two counters / two rejects / which error wins is unspecified. + +**F. Archive semantics** +Control `rename`s the file and writes a placeholder (`:6269-6310`). Design says **don’t** overwrite `handoff.json` for stale case (leave bytes, repair by rewrite). Good, but invalid-schema / missing cases need the same clarity (rename vs leave vs placeholder). + +**G. D13 does not fix the incident alone** +Stall was outer **not merging** while re-polishing with a schema-valid frozen handoff. Forcing re-stamp only makes `open_work: ["merge M1"]` **freshly stamped**, not true. Convergence still depends on **D15 prompt** (anti-over-polish + reconcile). Substrate fix removes one failure mode; it does not create a stop condition. + +**H. `check_runner_roles: [eval_runner]` side effects** +- Roster: outer loses `eval_check_runner` authority; gains nothing for `eval_results.md` unless you also change the advertised path. +- Receipt sealing only on eval_runner finish — fine if receipts are dead for stock v3 (they are; roster retired `eval_receipts/`, tests assert that). +- BYO sets that still put `outer` as runner and write receipts break silently until updated. Frozen contracts keep old behavior — OK. + +**I. Version skew fallback** +Prompt hand-writes stamped markdown if no `--result-out` reintroduces partial hand-copy failure (wrong path, bad fence). Enforcement then thrash-respawns. Document minimum version is not a runtime guard. + +**J. Frozen-contract sessions** +Already-running M1-class sessions **never** get D13/D14. Design admits this; operators must restart/amend. Call that out as operational, not just technical. + +**K. Raw root only for orchestration role** +Need explicit gate in `_write_iteration_paths` (`worker.py:956-1021` has no `raw` key today). Confirm layout: folded `raw/_/…` (`sessions.py:1311-1328`) matches prompt guidance (`raw//…` in design is slightly vague). + +**L. LoopState field** +`handoff_protocol_consecutive_failures` needs default `0` on old state loads (pydantic default OK) and projection/docs; stop_reason string must be allowed wherever stop reasons are enumerated. + +--- + +## 4. Over- / under-engineered + +**Over:** +- Generalized “role-output currency linter” for two outputs before a second kind exists — start with handoff-specific reject path parallel to control; add generic helper when a second kind ships. +- Dual “triggered vs untriggered” eval enforcement modes. +- Shared naming `role_output_rejected/` while control keeps `control_rejected/` mid-migration — fine, but don’t block on unified taxonomy. + +**Under:** +- Exact non_monotonic relaxation algorithm (must be field-level). +- How engine reads stamp from markdown (schema, fail modes). +- Interaction order with control rejection + counters. +- Contract-driven declaration of currency outputs (to satisfy inv 2). +- Whether `success=False` on handoff failure should skip control application or still run it. +- Tests for: semantic same-rev rewrite still `non_monotonic`; non-owner past producer still `valid`; eval_runner without file respawns; no-eval contract unchanged. + +**Simpler sufficient core:** +1) Owner finish requires `producer == finishing attempt` or archive+respawn. +2) Provenance-only same-rev advance via structural compare. +3) Contract/path for eval result declared in workflow-set; enforce presence+attempt only when that role finishes. +4) eval-banana `--result-out`. +5) `paths.json` raw root + outer prompt reconcile/anti-polish. + +Drop: multi-mode trigger logic, deep “linter framework,” engine interpretation of stale commits. + +--- + +## 5. Verdict + +**Not sound to implement as written.** Direction is right; three items must be fixed first. + +### Top 3 blockers + +1. **Specify same-revision “provenance-only” as a structural field/digest compare** so non_monotonic still catches content rewrites. Without this, D13 either self-deadlocks or opens a tamper hole (`coordinator_app.py:5727-5736`). + +2. **Remove engine hardcoding of `eval_results.md` / finish-time `eval_request` fiction.** Enforce currency from **declared contract** on **check_runner role finish**; define stamp parse (fence schema + attempt match) or enforce a tiny sidecar the engine already understands. Align D14 text with inv 2. + +3. **Be honest about causal fix for the stall:** D13/D14 are necessary substrate; **D15 prompt is load-bearing** and untestable by the engine. Ship D13 without turning forgetful re-stamp into an easy `handoff_protocol_broken` death spiral (reset rules, surface repair assignment clearly, consider not sharing the tight control cap until measured). + +Secondary before merge: dual-failure ordering; archive-vs-leave for handoff; LoopState/stop_reason; raw-path prompt accuracy; BYO migration notes for `check_runner_roles`. + +**Implement after those three are written into the binding design with algorithms and tests.** Until then, treat the corpus as directionally correct analysis, not an implementable contract. diff --git a/design/analysis/review-grok-deterministic-provenance-r2.md b/design/analysis/review-grok-deterministic-provenance-r2.md new file mode 100644 index 0000000..7ad4735 --- /dev/null +++ b/design/analysis/review-grok-deterministic-provenance-r2.md @@ -0,0 +1,60 @@ +I'll re-review the revised design against the original blockers and the real source, reading the design docs and first-round reviews first.Next I'll read D13–D15 and the postmortem, then ground-check the design against coordinator/scheduler/model code.Checking one more critical interaction: whether `goal_met` can omit `handoff_ref`, and how config caps are declared.# Re-review: deterministic handoff + eval provenance (r2) + +## Per-blocker (claims 1–7) + +| # | Status | Evidence | +|---|--------|----------| +| **1** Frozen discriminator | **RESOLVED** | Design §0 + D13: `WorkflowSetContract.schema_version` 1→2 + typed `currency_outputs`; new behavior only when frozen `schema_version >= 2` (`deterministic-handoff…md:49–69`, `decisions.md:526–527`). `_validate_finished_binding` returns the attempt-frozen contract (`coordinator_app.py:1615–1691`); finish passes it into `_record_finished_task` (`:1365–1383`). Live stock is `schema_version: 1` (`contract.yaml:1`) — cannot trip new path. `session_protocol_version` is already 3 on both old/new → wrong discriminator; schema bump is right. | +| **2** Tiered handoff disposition | **PARTIALLY** | A1 location is correct: extend `_validate_control_handoff_reference` (`:5559–5578`) called from `_validate_v3_control` (`:5472–5476`) → `_reject_v2_control` (`:6243–6317`). A2 correctly avoids `success=False` / general cap (`:1574–1576`, `:2598–2618`) and does not need `_failed_workflow_retry` — outer is always eligible (`run_every: 1`). **Hole:** `handoff_ref` is optional today (`models.py:695`, `http-contract.md:485–490`); A1 text is only “when … citing” (`deterministic-handoff…md:97–99`) while D13 says goal_met **must cite** (`decisions.md:533–535`). Omitting `handoff_ref` still accepts `goal_met` with a stale accepted snapshot. | +| **3** Currency only if `request.success` | **RESOLVED** | Explicit rule + crash rationale (`deterministic-handoff…md:89–91`, `:145–149`, `:253–254`; D13 `:532–534`). Matches Codex crash-archive concern; ordering vs `_apply_session_control` stated. | +| **4** Same-rev re-stamp algorithm | **RESOLVED** | Exact IFF: producer==finishing owner ∧ `updated_at` non-decreasing ∧ strip({producer,updated_at}) equal else `non_monotonic` (`deterministic-handoff…md:123–141`). Implementable on parsed `LayerHandoff` / `AcceptedHandoffSnapshot` (`models.py:420–438`, `:496–505`); preserves `:5727–5736` tamper case for any other field rewrite. | +| **5** Eval quarantine-only (D11) | **RESOLVED** | No fail/respawn/cap path remains (`deterministic-handoff…md:153–192`, D14 `:597–602`, tests 7–8). `check_runner_roles` unchanged is consistent — it is receipt authority (`models.py:316–320`, `:5593`, `:5549`), independent of `currency_outputs` advisory location. Mandatory-final-eval sentence removal stated (still present in stock outer `prompt.txt:44–46` as work to do). | +| **6** eval-banana observe-git / per-check / envelope | **PARTIALLY** | Docs aligned: `--result-out` only, no `--result-name`, observe commit+dirty, status envelope (`output-destination…md:38–87`; loopy `:163–170`). Per-check **model** matches (`models.py:80–84`, `harness_judge.py:250–298`). **Overclaim:** “family/model/effort can differ by check via per-check model overrides” — only `model` is per-check; effort is global harness config. | +| **7** Honest causality (D15 load-bearing) | **RESOLVED** | Design lead + D15 + postmortem (`deterministic-handoff…md:17–20`, D15 `:635–637`, postmortem `:136–139`). | + +--- + +## NEW inconsistencies / source mismatches introduced by the revision + +1. **D13 vs binding design vs code on `handoff_ref` (critical).** + D13: “Terminal `goal_met` control **must cite** a handoff…” (`decisions.md:533–535`). + Binding A1: only extends validation **when** control cites handoff (`deterministic-handoff…md:97–99`). + Code/docs: `handoff_ref` optional (`models.py:695`, `http-contract.md:485`). + Net: A1 does not close “declare done on stale continuity” without either requiring citation under `schema_version>=2`+`kind:handoff`, or checking producer currency on every `goal_met` independent of citation. + +2. **A1 still double-counts the general workflow failure cap.** + `_reject_v2_control` → `invalid_v3` → `success=False` (`:1544–1547`) → `_track_workflow_failure_cap` (`:1574–1576`). Soft path is correctly exempted; hard path is not. Design only discusses the control cap (`:250–252`). Not a death spiral for rare completion attempts, but unstated dual-counter behavior. + +3. **INV2 residual for handoff path.** + Currency targets come from `currency_outputs` (good for eval). A1 still rides the hard-coded canonical check `reference != "session:/project_state/handoff.json"` (`:5564–5565`) and `handoff_path()`. Design never requires `currency_outputs[kind=handoff].path` ≡ protocol handoff identity. Stock matches; BYO divergence would split observation vs currency. + +4. **“Quarantine” has no existing engine object for `eval_results.md`.** + Receipts have seals (`accepted_eval_receipt_seals`); markdown results do not. Quarantine = emit `eval_stale` only is fine for INV1, but “not accepted as current evidence” implies an acceptance registry that does not exist. Implement as pure diagnostic unless a structure is specified. + +5. **eval-banana “per-check effort differs”** overstates current models (see #6). + +6. **Config surface for soft cap underspecified.** + `handoff_protocol_consecutive_failures_cap` named; no placement on `RootConfig` / `RootConfigSnapshot` (where `goal_check_consecutive_failures_cap` lives, `models.py:106–115`). LoopState field + stop_reason enum allow-list also still needed (r1 secondary). + +7. **Observation docstring will be wrong after A1.** + `models.py:588–589` “never gates scheduling/control” — observation stays non-gating, but control path will gate on handoff currency. Update when implementing. + +--- + +## Remaining MUST-FIX (ranked) + +1. **Close the `goal_met` / `handoff_ref` hole.** Under frozen `schema_version>=2` with a `kind:handoff` currency entry: require `handoff_ref` on `goal_met` **or** reject `goal_met` when `accepted_handoff_snapshot.handoff.producer != completing attempt` even if uncited. Align binding A1 wording with D13. Without this, Codex r1 blocker #1 is only half-fixed. + +2. **State A1 cap interaction:** control reject may also tick `workflow_consecutive_failures` via `success=False`; either accept and document, or exempt control-protocol failures from the general workflow cap (symmetric to A2). + +3. **Contract validation:** if `currency_outputs` declares `kind:handoff`, require path/owner match orchestration handoff identity (or document that A1 always means the canonical protocol handoff, and `currency_outputs` path is advisory for prompts only). + +4. **Define eval quarantine operationally:** diagnostic event only; no iteration effect; optional note that there is no engine “accepted eval_results” seal. + +Nits (non-blocking): fix effort wording; add RootConfig/LoopState/stop_reason fields; update “never gates” comment; keep version-skew hand-write as degrade-to-unverified only (already stated). + +--- + +## Final verdict + +**Yes-with-noted-nits** — direction and r1 structural blockers are largely fixed (discriminator, D11 eval demotion, same-rev algorithm, success gating, soft vs hard tiering, D15 honesty). **Do not implement A1 until MUST-FIX #1 is written into the binding design** (one paragraph + test: `goal_met` without re-stamp, with and without `handoff_ref`, both rejected under schema 2). After that, sound to implement. diff --git a/design/decisions.md b/design/decisions.md index 5e96782..13f5169 100644 --- a/design/decisions.md +++ b/design/decisions.md @@ -17,6 +17,9 @@ Companion docs: accepted v3 contract for orchestration ownership, semantic state/handoff, optional evaluation, schedule/capability context, phase-sized PM dispatch, and cross-harness review. It is implemented in loopy-loop 0.8.0 and team-harness 0.5.4. +- `design/designs/deterministic-handoff-and-eval-provenance.md` — the binding design for + handoff currency, located+fresh advisory eval output, and orchestrator reconciliation + (**D13–D15**), with its eval-banana counterpart spec. - `design/proposals/` — forward-looking changes we are *considering* (not decided; do not treat them as binding or as descriptions of current behavior). - `design/analysis/` — the July 2026 review that produced several of these decisions @@ -514,3 +517,155 @@ provider-internal bytes remain unavailable, and unavailable channels are stated honestly. Raw traces are sensitive local data; correctness-critical facts do not rely on their retention. See the [binding design](designs/recursive-loop-layer-contract.md) and [session layout](../docs/session-layout.md). + +## D13. Layer-handoff currency is a structural-integrity check, hard-gated at completion + +**Decision.** The layer handoff (`project_state/handoff.json`) carries the provenance of the +attempt that authored it (`HandoffProducer{workflow_id, attempt_id}`), and the engine treats its +**currency** as a structural-integrity property, not a semantic one. The behavior activates for any +contract that declares the handoff in a typed, validated `currency_outputs` entry (`kind: handoff`, +which must name the canonical handoff identity: `project_state/handoff.json` owned by +`orchestration.handoff_owner`); a contract without the declaration is unaffected. (Backward +compatibility is **waived** — there is no `schema_version` gate and no retrofit story.) When the +finishing role owns that entry **and the attempt was mechanically successful**, the engine requires +the handoff to carry `producer == {workflow_id, attempt_id}` of that exact attempt. Disposition is +**tiered by whether the finish declares completion**: + +- **Completion (hard, proven path).** In `_validate_v3_control`, a terminal `goal_met` is rejected + when the `accepted_handoff_snapshot` producer ≠ the completing attempt — **independent of whether + `handoff_ref` is cited** (it is optional, so a citation-only check is bypassable; the currency + check runs regardless, and a `kind: handoff` contract additionally requires the citation). + Rejection reuses the existing control-repair machinery (`_reject_v2_control`), re-dispatching the + owner. This closes "declare done on stale continuity," evaluated **before** `goal_met` is + consumed. It rides the existing control-rejection counters (control + general workflow cap) — a + documented, pre-existing property, not a new single-counter disposition. +- **Non-completion owner finish (pure diagnostic).** Leaving the handoff un-re-stamped raises a + `handoff_stale` diagnostic + a repair note the standing owner (`run_every: 1`) reads on its next + turn. It has **no failure semantics**: no `success` flip, no cap, no overwrite of the handoff + bytes. Making it a hard failure would need a repair-scheduling primitive to force the owner ahead + of successors (the scheduler unlocks `inner` next, not an owner repair) — engine surface the + completion gate already makes unnecessary. + +A same-revision re-stamp is legitimate **only** by a field-level compare of the parsed handoff +models (producer is the current owner attempt; `updated_at` non-decreasing; every other field equal +to the accepted snapshot); any other same-revision change stays `non_monotonic`, preserving tamper +detection. A non-owner finish requires no re-stamp and is unchanged. Eval outputs carry the +producing attempt's identity for the same reason (D14), so a verdict from a past attempt is visibly +stale to its reader. + +**Context.** An `inner_outer_eval` layer stalled after its work was objectively done: the +handoff was schema-valid but frozen hours behind the repository, and the engine could not tell. +The provenance check (`_handoff_producer_is_known`) accepted a handoff stamped by *any* past +attempt, and an untouched file passed the revision-monotonicity check, so staleness was +undetectable by construction — and the handoff observation was not wired to any repair path. +The root cause was a missing determinism guarantee, not a semantic misjudgment: the orchestrator +was misled by its own stale durable state. See +`design/analysis/inner-outer-eval-nonconvergence-postmortem.md`. + +**Context — why this is not banned by D8.** D8 forbids *semantic* prevention/vetoes but +explicitly preserves structural protocol integrity: "the engine still validates schemas, +identity, **current-attempt ownership**, hashes, path confinement… Those checks protect the +durable machine from corrupt or **stale** input; they do not decide whether the work product is +semantically good enough." Handoff currency is exactly that — stale-input protection with an +accountable, visible repair route — so it is a D8-permitted structural check, not a new gate. + +**Consequences.** The **hard** guarantee is at the completion boundary: a terminal completion can +never be accepted unless the completing attempt brought the handoff current. Per-iteration currency +is a **visible nudge, not a guarantee** — a forgetful (but successful) owner finish can leave a +briefly stale handoff that a successor might read before the owner's next turn repairs it; the +honest bound is "stale-but-flagged, and never able to leak into a completion." Protecting successors +from acting on a stale summary is the **D15 prompt**'s job (reconcile against live state), which is +the load-bearing convergence fix; D13 keeps the substrate honest so that prompt can trust what it +reads. Backward compatibility is waived: the behavior applies wherever `currency_outputs` is +declared and is otherwise inert. See the +[binding design](designs/deterministic-handoff-and-eval-provenance.md). + +**Refined by / refines.** Extends **D8** (structural integrity carve-out) and the **D11** handoff +ownership model; paired with **D14**/**D15**. + +## D14. Advisory evaluation is located by construction and freshness-visible — engine-diagnostic-only, never enforced + +**Decision.** Evaluation stays optional and advisory (**D11**). The reliability win is that the +verdict lands in the right place **by construction** and its freshness is **visible** — not that +the engine enforces it. Two mechanisms, split by ownership: + +- **eval-banana** gains an output-destination capability (`--result-out `): it writes a + run's self-describing result atomically to a caller-named path, with a provenance block it + **observes** — `commit_before`/`commit_after` plus the working-tree dirty digest recorded with + its **algorithm name**, **per-check `model`** (only `model` is a per-check override; `family` and + `effort` are run-level harness config), tool version, timestamp, verdict, and a formal + status/error envelope for no-checks / pre-report failures. Only the opaque `--provenance-attempt` + is caller-echoed. The tool knows nothing of loopy-loop. +- **loopy-loop** declares the eval output in the typed `currency_outputs` as `kind: advisory` (path + + owner role; no literal in engine core). There is **no engine acceptance store** for markdown + results, and the engine **never reads or parses** the document — its whole involvement is a single + **presence diagnostic** (`eval_missing`) when the declared eval owner finishes and the path is + absent. It **never** flips success, respawns, or caps (that would violate D11), and does **no + staleness check** (that would require reading the file). `check_runner_roles` is left unchanged + (it governs receipt authority + citation validation; repurposing it would silently revoke + `outer`'s receipt authority). The freshness *judgment* — observed commit vs live `HEAD`, dirty + tree ⇒ re-run — is entirely the **reading agent's**, per the prompt; the orchestrator's + acceptance/merge is driven by the task and **live** repo/CI facts, **never gated by a + previously-established eval check.** + +**Context.** In the M1 stall the decisive approval was hand-transcribed by the eval agent into +`project_state/evidence/…` instead of the canonical `eval_results.md`, and even the canonical file +carried no freshness marker. The first draft of this decision over-reached — making a +missing/stale eval output fail+respawn+cap — which adversarial review (Codex, Grok) correctly +flagged as a D11 violation and as a broken respawn (the runner deletes its `eval_request.md` +trigger mid-run, so the scheduler cannot re-elect it). The corrected split fixes the actual M1 +failure (off-channel, unversioned verdict) via eval-banana writing the right file with observed +provenance + an optional engine diagnostic, **without** making evaluation mandatory. It does not +revive the reverted v2 overcorrection; completion authority stays with the orchestrator. + +**Consequences.** eval-banana ships `--result-out` + observed-git/per-check-`model`/status-envelope +provenance and is the **required** version (backward compatibility waived — no hand-write +fallback); if the file is missing/unstamped the engine emits a diagnostic only, never a failure. The +stock `inner_outer_eval` contract declares `eval_results.md` as a `kind: advisory` currency output; +the **mandatory-final-eval sentence is removed** from the outer prompt (it contradicted D11). +Prompts wire the write path (`eval_runner --result-out`) and the read+provenance-check path (outer, +inner), which own the freshness judgment. Sessions without an advisory currency declaration are +unaffected; BYO sets opt in by declaring their own. See the +[binding design](designs/deterministic-handoff-and-eval-provenance.md) and eval-banana +`design/output-destination-capability.md`. + +**Refined by / refines.** Refines **D11** (optional advisory evaluation); paired with **D13** +(shared provenance stamping) and **D15**. + +## D15. The orchestrator reconciles the handoff against live state; raw traces are its bounded fallback + +**Decision.** This is the **load-bearing fix** for the M1-class stall: D13/D14 stop the +orchestrator being *misled*, but the actual convergence behavior lives here, in the (replaceable) +prompt. The orchestration role treats the handoff as a *summary*, not authority. Before selecting +the next task or deciding completion it reconciles the handoff against **live repository and CI +state**; when the handoff is missing, ambiguous, or **contradicts** the repository, it trusts live +state and, as a bounded fallback, reads **specific relevant prior iterations selected from +history** (not a tree scan) rather than re-deriving work from a stale summary. To make that +possible the engine exposes one read-only discoverability path to the orchestration role — the +session **raw root** (`/raw/`, folded layout, each iteration under +`raw/_/…`). The anti-over-polish behavior this enables (accept delivered, green, +approved work; treat residual non-blocking items as new or deferred tasks, not a reopen) is +**prompt guidance in the replaceable workflow-set**, never an engine rule. The path map names only +**declared** artifacts (drop undeclared files rather than cite them). + +**Context.** The stalled orchestrator was never told the raw inner traces existed or where, and +was not instructed to reconcile a suspect summary against the repo — so when D13/D14's gaps fed +it a stale handoff and an empty results file, it looped on stale work. `paths.json` handed it its +own attempt's `trace_root` but no path to sibling/prior iterations' traces. + +**Context — relationship to D12.** D12 says continuity should not *routinely* depend on +re-reading raw traces; the coordinator promotes conclusions into compact artifacts. This decision +is consistent: routine continuity still rides the compact artifacts, and trace-reading is the +safety net for exactly the case where that compact-artifact contract was violated (a stale or +off-channel summary). Discoverability is best-effort; unreadable traces leave the orchestrator +reconciling against live git/CI alone, which stays non-gating (D12). + +**Consequences.** One new best-effort `paths.json` key for the orchestration role; no new gate. +The behavioral guidance (path map, reconcile-before-deciding, anti-over-polish) lives in the +stock `outer` prompt and can be replaced wholesale by a user's own set. This keeps the engine +minimal and the policy user-owned — the standing bet: fix the substrate, trust capable agents +and their prompts. See the +[binding design](designs/deterministic-handoff-and-eval-provenance.md). + +**Refined by / refines.** Refines **D12** (trace-retention/continuity) and **D11** +(orchestrator-owned completion); paired with **D13**/**D14**. diff --git a/design/designs/deterministic-handoff-and-eval-provenance.md b/design/designs/deterministic-handoff-and-eval-provenance.md new file mode 100644 index 0000000..032fe86 --- /dev/null +++ b/design/designs/deterministic-handoff-and-eval-provenance.md @@ -0,0 +1,335 @@ +# Deterministic handoff currency, located+fresh eval output, and orchestrator reconciliation + +*Binding design (`design/designs/`). Records the accepted form of **D13–D15**. Motivated by +`design/analysis/inner-outer-eval-nonconvergence-postmortem.md`. Revised 2026-07-22 after +adversarial review by Codex (`gpt-5.6-sol`) and Grok (`grok-4.5`) — +`design/analysis/review-{codex,grok}-deterministic-provenance-r1.md`; this revision is what +their blockers require. The eval-banana half is specified in that repo's +`design/output-destination-capability.md`; this document is the loopy-loop side and the +contract between the two.* + +## Problem, in one paragraph + +A capable orchestrator can be defeated by its own durable state. When the rolling handoff can be +schema-valid yet stale, when the advisory eval verdict is hand-copied to an unpredictable path +with no freshness marker, and when the orchestrator is never told where ground truth lives, the +loop can spin after the work is objectively done (the M1 incident). The fix is three +**substrate-determinism** changes — not a new engine behavioral rule. **Be honest about causality: +the load-bearing fix for the M1 stall is the D15 *prompt* (reconcile-against-live-state + +anti-over-polish); D13/D14 are the necessary substrate that stops the orchestrator being *misled*, +but neither creates a stop condition.** Each change is a structural-integrity or discoverability +property (D8-compatible) or prompt guidance; none makes the engine decide whether work is *good*. + +## Design invariants (any violation is a defect) + +- **INV1 — Evaluation is optional and advisory (D11).** No change may make a passing eval a gate, + make an eval role mandatory, or turn absent/malformed/stale advisory eval output into iteration + failure, a respawn, or a cap. A session with no eval role behaves exactly as today. +- **INV2 — Workflow-sets are user-owned; the engine enforces *declared* contracts, never + hard-coded paths.** No new path literal (`project_state/eval_results.md`, `eval_request.md`) in + engine core; currency targets are read from a typed contract declaration. +- **INV3 — Detection with accountable repair, never prevention/semantic gates (D8),** using only + D8's explicit structural carve-out (schema, identity, current-attempt ownership, hashes, + stale-input protection). +- **INV4 — Compact artifacts carry continuity (D12);** raw-trace reading is a bounded fallback. +- **INV5 — Minimal engine:** no convergence heuristic, loop budget, structured-verdict, or + merge-when-approved rule in the engine; behavioral guidance lives in the replaceable prompt. + +*(An earlier draft carried an INV6 "frozen contracts stay on old behavior" and a `schema_version` +1→2 discriminator. **The operator has waived backward compatibility** (2026-07-22): already-live +sessions need not keep the old behavior. The discriminator is dropped — new behavior simply applies +to any v3 contract that **declares** the feature via `currency_outputs`. Old contracts that predate +the field lack it and get nothing; that is a natural consequence of the declaration, not a +compat guarantee we maintain.)* + +--- + +## 0. The typed currency declaration (foundational, no version gate) + +Add a **typed, validated `currency_outputs`** list to `WorkflowSetContract` (rather than reading +the untyped `state: list[dict]` or hard-coding paths — INV2). It is the sole trigger for the new +behavior; there is no `schema_version` gate. + +```yaml +currency_outputs: + - path: project_state/handoff.json + owner_role: outer + kind: handoff # structural continuity: completion-gated + repairable (Change A) + - path: project_state/eval_results.md + owner_role: eval_runner + kind: advisory # optional evidence: engine emits a diagnostic only, never fails (Change B) +``` + +The engine reads `path`/`owner_role`/`kind` from here; **no path literal lives in engine core** +(INV2). `kind` selects the disposition: `handoff` → structural enforcement (Change A); +`advisory` → observability diagnostic only (Change B). A contract with no `currency_outputs` +enforces nothing new. + +**Strict contract validation (added, since compat is waived — we can be strict):** on contract +load the engine validates `currency_outputs` — each `path` is repo-relative and confined (no +absolute/`..`), paths are unique, `owner_role` names a declared role, and — critically — a +`kind: handoff` entry **must** name the canonical protocol handoff identity: `path == +project_state/handoff.json` and `owner_role == orchestration.handoff_owner`. This closes the +"BYO contract splits observation vs currency" gap (Grok r2 #3 / Codex r2 #5): `kind: handoff` is an +opt-in *marker on the engine's canonical handoff*, whose identity is fixed by protocol — only the +`advisory` path is genuinely user-chosen. `orchestration` is required for a `kind: handoff` entry. + +--- + +## Change A — Deterministic handoff currency (engine) → D13 + +### Current behavior (verified) + +`_record_finished_task` runs, for v3, `_accept_current_eval_receipts` then `_observe_layer_handoff` +(coordinator_app.py ~1490–1500), **then** `_apply_session_control` (~1541), which can set +`goal_met` (~5405). `_observe_layer_handoff` is non-gating, and `_handoff_producer_is_known` +(5764–5783) accepts the handoff when its producer is the current attempt **or any past attempt in +the active session's history** — so an un-re-stamped handoff reads `valid`, and an untouched file's +unchanged digest+revision never trips `non_monotonic`. Terminal control validation +(`_validate_control_handoff_reference`, 5559–5577) checks the cited handoff matches the *accepted +snapshot* — but a stale handoff **is** the accepted snapshot, so it passes. Net: **completion can be +declared on a stale handoff, and a successor can read one.** + +### The rule + +For a `kind: handoff` currency output whose `owner_role` is the finishing attempt's role, **and +only when `request.success` is true** (a crashed attempt must not archive or fail against the prior +valid handoff — INV3/robustness), the handoff at `path` must exist, be schema-valid, name this +session/goal, and carry `producer == {workflow_id, attempt_id}` of the finishing attempt. A +non-owner finish (inner, eval roles) requires no re-stamp and keeps today's observation semantics. + +**Two dispositions, by whether this finish declares completion:** + +**(A1) Completion path — hard, via the proven repair machinery.** This is the correctness +guarantee. In `_validate_v3_control`, when a contract declares a `kind: handoff` currency output +and the signal is terminal `goal_met`, add a currency reason **independent of whether `handoff_ref` +is cited**: the `accepted_handoff_snapshot` must exist and its `handoff.producer` must equal the +**completing attempt**. Because `handoff_ref` is optional today (`models.py:695`) and +`_validate_control_handoff_reference` runs only when it is present (`coordinator_app.py:5472`), a +citation-only check would be bypassed by an uncited `goal_met` (Grok/Codex r2 blocker). So the +completion currency check does **not** depend on citation; additionally, a `kind: handoff` contract +**requires** `goal_met` to cite the handoff (a missing `handoff_ref` is itself a reason), so the +two checks compose. Any reason → the existing `_reject_v2_control` flow (archive `control.json`, +`protocol_failures` record, `engine_repair` placeholder, control counter, re-dispatch the owner). +Structural (INV3): a terminal transition must be backed by a handoff the same attempt brought +current — not a judgment of work quality. + +*Cap accounting (honest).* A1 rides the **existing** control-rejection path, which flips +`success=False` and therefore ticks both `control_protocol_consecutive_failures` **and** the general +workflow-failure cap (`_track_workflow_failure_cap`, coordinator_app.py ~1574). This is pre-existing +control-reject behavior, not introduced here; A1 is **not** a single-counter disposition. We accept +and document it (a completion repeatedly citing stale continuity is a real repeated failure worth +counting); a future refinement could exempt control-protocol failures from the general cap +symmetrically to A2, but that is out of scope. + +**(A2) Non-completion owner finish — soft continuity hygiene, purely observability.** A `success` +owner finish that leaves the handoff un-re-stamped emits a `handoff_stale` diagnostic and writes a +`protocol_failures` repair note surfaced in the owner's next assignment. It does **nothing else**: +it does **not** flip `success`, touch any cap, or archive/overwrite `handoff.json` (the continuity +artifact is left intact). The standing owner (`run_every: 1`) re-runs on its own cadence and reads +the repair note; the correctness gate is A1, so an un-repaired stale handoff can never leak into a +completion regardless. A2 is deliberately *not* a hard failure: making it one would require a repair +scheduling primitive that forces the owner to run before successors (the stock scheduler unlocks +`inner`, not an immediate owner repair — Codex r2), adding engine surface against INV5 for a case A1 +already protects. (An optional lenient `handoff_protocol_consecutive_failures` counter/cap could +catch a *permanently* broken owner, but is not required; if added it is exempt from the general cap.) + +This tiering is deliberate: correctness (don't finish on stale state) is hard-enforced through the +proven control path; continuity hygiene is a visible nudge with no failure semantics, and the +successor-protection it aims at is really delivered by the **D15 reconcile-against-live-state +prompt** — the honest load-bearing fix. + +### Same-revision re-stamp — exact algorithm (both reviewers: top blocker) + +A pure re-stamp changes `producer`/`updated_at`, hence the digest, so today it is misflagged +`non_monotonic`. Relax **precisely**, preserving tamper detection. On observing a handoff whose +`revision == state.handoff_revision`: + +``` +accept as a legitimate provenance-only re-stamp IFF + handoff.producer == the finishing owner attempt {workflow_id, attempt_id} AND + handoff.updated_at >= accepted_snapshot.updated_at AND + strip(handoff, {producer, updated_at}) == strip(accepted_snapshot.handoff, {producer, updated_at}) + (field-by-field equality of the parsed models with those two keys removed) +otherwise -> non_monotonic (unchanged tamper detection) +``` + +A revision **increment** with `producer == current owner attempt` is always legitimate (material +change). A same-revision change to any field other than `producer`/`updated_at` stays +`non_monotonic`. This makes "re-stamp" expressible without opening a fixed-revision content-rewrite +hole. + +### Finish ordering (Codex blocker) + +In `_record_finished_task`, currency validation for the finishing role's `kind: handoff` output +runs **before** `_apply_session_control` consumes terminal control. A1 is enforced inside control +validation (so a stale-cited completion is rejected, not consumed); A2 (no terminal control this +finish) takes the soft path. Under no ordering can a stale handoff both fail currency and let +`goal_met` through. + +--- + +## Change B — Located, fresh advisory eval output — **engine-diagnostic-only** → D14 + +Revised from the first draft, which made a missing/stale eval output fail+respawn+cap — that +violates INV1/D11 ("absent or malformed advisory eval is a visible diagnostic; it does not turn a +mechanically completed harness invocation into failure or starve orchestration"). The reliability +win comes from eval-banana writing the result correctly **by construction**, plus engine +an optional engine **diagnostic** on stale/missing evidence — never a gate. + +### eval-banana side (see eval-banana `design/output-destination-capability.md`) + +`eval-banana run` gains `--result-out ` (there is no `--result-name`) writing a single +self-describing result document (a fenced, parseable `eval-banana-result v1` block + human body) +atomically to a caller-named path, with a provenance block eval-banana **observes** rather than +echoes: `commit_before`/`commit_after` (bracketing the run) and the **working-tree dirty digest** +observed at run start, recorded with its **algorithm name** (`loopy-git-status-diff-v1-sha256` is +loopy's algorithm; eval-banana names whatever it uses so a reader knows — cross-tool digest equality +is *not* assumed, see below); the run verdict; **per-check `model`** (only `model` is a per-check +override in eval-banana; `family` and `effort` are run-level harness config — Codex r2); tool +version; timestamp; and a **formal `status` envelope** (`completed | no_checks | config_error | +harness_error`) so a run that fails before a report exists still writes a stamped document. +`--provenance-attempt ` is the one caller-echoed field (opaque to the tool). This removes the +hand-transcription failure mode that put the M1 verdict off-channel. Full spec + schema: +`eval-banana/design/output-destination-capability.md`. + +### loopy-loop side — declared, diagnostic-only, never fatal, no format coupling + +1. **Declared, not hardcoded (INV2).** The `kind: advisory` `currency_outputs` entry names the eval + result path + `owner_role`. The engine reads the path; no literal in core. +2. **A pure presence diagnostic, not a gate, and never opens the file (INV1 + minimalism).** There + is **no** engine "accepted eval_results" seal — markdown results are agent-consumed, unlike + sealed receipts. The engine **never reads or parses the result document at all**: it does not + interpret the verdict, the git observations, or the provenance block, and does not compare git + digests across tools. Its entire involvement is a single **presence diagnostic**: when the + declared eval owner finishes `success` and the declared path is absent, it emits `eval_missing`. + That is all — no `success` flip, no respawn, no cap, and deliberately **no engine staleness + check** (that would require reading the file). The freshness *judgment* — observed commit vs live + HEAD, dirty tree ⇒ re-run — is entirely the **reading agent's**, per the prompt (D14), from the + provenance eval-banana wrote. This keeps the engine's eval involvement to a one-line + `path.exists()`. +3. **`check_runner_roles` is left unchanged.** It governs receipt acceptance + citation authority + (models.py 315; coordinator_app.py 5549, 5593); repurposing it would silently revoke outer's + receipt authority. The advisory currency declaration is independent of it — no regression. +4. **Prompts wire paths and own the freshness decision.** `eval_runner` passes `--result-out + --provenance-attempt `; `outer`/`inner` read that path and **check + the observed `commit_before/after` against live `HEAD` (and treat a dirty tree as needs-fresh-run) + before relying on the verdict** (D14 independence — a verdict whose subject ≠ current HEAD is + stale and is re-run or set aside, never obeyed). The engine does not adjudicate this. +5. **Remove the mandatory-final-eval sentence** from the stock `outer` prompt ("A terminal goal + check is required before you declare the goal met") — it contradicts D11's explicit permission to + decide evaluation is unnecessary (Codex). Replace with: a terminal eval is available and + recommended near completion, but the orchestrator may decide it is unnecessary. + +### D14 decision content + +`eval_results.md` is advisory. The orchestrator's acceptance/merge is driven by the task and **live** +repo/CI facts, **never gated by a previously-established eval check.** eval-banana's observed +provenance makes "previously-established" deterministically visible to the *reading agent*; the +engine's role is location-by-construction (eval-banana writes the right file) plus an optional +diagnostic. **All judgment stays with the orchestrator.** + +--- + +## Change C — Orchestrator reconciles against ground truth (prompt + minimal engine) → D15 + +### Engine (minimal) + +Add one read-only key to the orchestration role's `paths.json`: the session **raw root** +(`/raw/`, folded layout; each iteration under `raw/_/…`, per +`raw_dir_path`/`iteration_dir_name` in `sessions.py`). Add it in `assignments.py`'s attempt-paths +builder (the `is_v3` branch that populates `paths.json`), **gated to the orchestration completion +role only**. Non-orchestration roles are unaffected. The prompt names the real +`raw/_/…` shape (not `raw//…`). + +### Prompt (stock, user-owned — the load-bearing convergence fix) + +Extend the stock `outer` prompt with: + +- **A path map of *declared* artifacts + the raw fallback.** Live repo + git/CI; the compact + durable artifacts (`handoff.json`, `project_state/tasks/`, `finished.md`, `eval_results.md`, and + `ledger.md` **only if it is a declared state path** — otherwise drop it, do not name undeclared + files); and, as a bounded fallback, **specific relevant prior iterations** selected from history + under the raw root — not a tree scan (Codex). +- **Reconcile-before-deciding.** The handoff is a summary, not authority. Before selecting the next + task or declaring completion, reconcile it against live repo/CI. When it is missing, ambiguous, or + **contradicts** the repo (says "merge M1" but the PR is merged; an approval exists the summary + omits), trust live state and, if needed, read the selected raw traces — do not re-derive work from + a stale summary. (D12-consistent: routine continuity still rides compact artifacts; trace-reading + is the violated-contract safety net.) +- **Anti-over-polish (prompt-owned, INV5/D8/D11).** When the selected work is delivered, checks are + green, and an advisory review approves, accept and advance; residual non-blocking items are new or + deferred tasks, not a reopen. Explicitly **not** an engine rule. + +The `inner` prompt gets the "check the eval result's provenance against the observed commit before +relying on it" note (reinforces D14). + +--- + +## What deliberately stays out of the engine + +No convergence heuristic, loop budget, structured-verdict, merge-when-approved rule, mandatory eval, +or engine interpretation of "commit ≠ HEAD ⇒ must re-run." The engine gains only: a contract-declared +handoff-currency check (hard at completion, diagnostic per-iteration), an optional eval-provenance +diagnostic (no parse, no gate, no store), and one read-only discoverability path. Everything +behavioral is prompt-level and user-replaceable. + +## Failure, recovery, adoption (backward compatibility waived) + +The operator has waived backward compatibility, so there is **no frozen discriminator and no +retrofit story**: the new behavior applies to any contract that declares `currency_outputs`, and a +contract without it is simply unaffected. Notes that remain: + +- **Caps.** Completion staleness (A1) rides the existing control-rejection path and ticks its + existing counters (control + general), documented above — not a new death-spiral, and A2 has **no** + failure semantics at all (pure diagnostic), so no cap interaction. +- **`request.success` gating.** Currency runs only after a mechanically successful attempt; a crashed + owner attempt leaves the prior handoff and records the real failure (Codex). +- **Completion ordering.** A1 is evaluated inside `_validate_v3_control` (before `goal_met` is + consumed) and does **not** depend on `handoff_ref` being cited, so an uncited `goal_met` cannot + bypass it. +- **eval-banana version.** The new eval-banana (`--result-out`) is **required** — no hand-write + fallback (compat waived). loopy-loop documents the minimum eval-banana version; the prompts assume + it. If the file is absent/unstamped the engine emits a diagnostic only (never a failure), so even a + misconfiguration degrades to "unverified," not a thrash loop. +- **Trace I/O failure** stays non-gating (D12). + +## Test plan (contract/behavior) + +1. Contract **without** `currency_outputs` → zero new behavior (handoff + eval identical to today). +2. Owner completion (`goal_met`) whose handoff was **not** re-stamped this attempt → control rejected + via `_reject_v2_control`, owner re-dispatched; re-stamped completion accepted. **Covers both a + cited `handoff_ref` and an omitted one** — both rejected. +3. Non-completion owner finish leaving handoff stale → `handoff_stale` diagnostic + repair note, + `success` unchanged, **no cap touched**, `handoff.json` bytes intact. +4. Same-revision re-stamp changing only `producer`/`updated_at` by the current owner → **accepted**; + same-revision change to any other field → **`non_monotonic`**. +5. Non-owner finish with an owner-stamped (past-attempt) handoff → unchanged `valid`. +6. Crashed owner attempt (`success=False`) → no currency failure, real error recorded. +7. Eval owner finishes with the declared result path absent → `eval_missing` **presence diagnostic + only, iteration still succeeds**, no respawn, no cap (INV1); a present-but-stale result triggers + **no engine event** (the engine never reads it — freshness is the agent's job). +8. Contract validation: a `kind: handoff` entry whose path ≠ `project_state/handoff.json` or whose + `owner_role` ≠ `orchestration.handoff_owner` → contract load rejected; duplicate/absolute/`..` + paths rejected. +9. Orchestration role's `paths.json` includes the raw root at `raw/_/…`; other roles + unchanged. +10. eval-banana: `--result-out` written atomically with observed commit(s)+dirty digest(+algorithm) + and per-check `model`; each `status` value still writes a stamped document; symlink refusal; pure + back-compat when `--result-out` absent. + +## Phased adoption + +1. eval-banana `--result-out` capability (observed git, per-check `model`, status envelope, fenced + result schema) + design doc + released min version. +2. loopy-loop schema: typed, **validated** `currency_outputs` on `WorkflowSetContract` (path + confinement/uniqueness; `kind: handoff` ≡ canonical handoff identity). No `schema_version` gate. +3. loopy-loop engine: A1 (`_validate_v3_control` completion currency, citation-independent) + A2 + (per-iteration `handoff_stale` diagnostic), the exact same-revision relaxation, `request.success` + gating, tests. Update the `models.py` "handoff observation never gates" docstring. +4. loopy-loop engine: optional eval-provenance diagnostic driven by the `advisory` entry; raw-root + `paths.json` key gated to the orchestration role. +5. Stock `inner_outer_eval`: add `currency_outputs`; outer prompt path-map + reconcile + + anti-over-polish + **remove mandatory-final-eval**; eval_runner `--result-out`; inner/outer + provenance-check note. +6. D13–D15 recorded; companion-doc links updated. diff --git a/pyproject.toml b/pyproject.toml index dc8dc56..9f977e4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,7 +37,7 @@ classifiers = [ ] dependencies = [ "click>=8.1", - "eval-banana>=0.3.5", + "eval-banana>=0.5.0", "fastapi>=0.115.0", "filelock>=3.16", "httpx>=0.28", diff --git a/src/loopy_loop/assignments.py b/src/loopy_loop/assignments.py index ab661c7..3e66e12 100644 --- a/src/loopy_loop/assignments.py +++ b/src/loopy_loop/assignments.py @@ -47,6 +47,7 @@ from loopy_loop.sessions import parent_acceptance_dir_path from loopy_loop.sessions import project_state_dir_path from loopy_loop.sessions import protocol_failures_dir_path +from loopy_loop.sessions import raw_dir_path from loopy_loop.sessions import scheduler_view_path from loopy_loop.sessions import session_dir_path from loopy_loop.sessions import session_goal_path @@ -519,6 +520,14 @@ def build_attempt_assignment( "accepted_child_request": None, } ) + # D15: expose the session raw-trace root to the orchestration role only, + # as a read-only reconciliation fallback (each iteration lives under + # raw/_/…). Non-orchestration roles do not receive it. + if ( + contract.orchestration is not None + and task.workflow_id == contract.orchestration.completion_role + ): + paths["raw_root"] = raw_dir_path(repo_root=root, session_id=task.session_id) accepted_request_ref = goal_contract.accepted_request_ref if accepted_request_ref is not None: try: diff --git a/src/loopy_loop/coordinator_app.py b/src/loopy_loop/coordinator_app.py index 736aa17..6c60570 100644 --- a/src/loopy_loop/coordinator_app.py +++ b/src/loopy_loop/coordinator_app.py @@ -64,6 +64,7 @@ from loopy_loop.models import SchedulerView from loopy_loop.models import SessionOutcome from loopy_loop.models import SessionUsageTotals +from loopy_loop.models import SignalProducer from loopy_loop.models import STOP_ACTION from loopy_loop.models import TaskResponse from loopy_loop.models import utc_now @@ -1498,6 +1499,16 @@ def _record_finished_task( active=active, workflow_contract=effective_workflow_contract, ) + # D13 (A2) / D14: currency diagnostics run only after a mechanically + # successful attempt so a crashed attempt never fails against, or + # archives, the prior valid handoff. These are pure diagnostics — no + # success flip, no cap, no counter. + if request.success: + self._diagnose_currency_outputs( + state=state, + active=active, + workflow_contract=effective_workflow_contract, + ) elif self._workflow_expects_goal_check_signal(current_task=active): goal_signal_errors: list[str] = [] goal_signal = self._read_goal_check_signal( @@ -5475,6 +5486,18 @@ def _validate_v3_control( state=state, reference=signal.handoff_ref ) ) + # D13 (A1): when the contract makes the handoff a currency output, a + # terminal completion must be backed by a handoff the *completing* + # attempt brought current. This runs independent of whether + # handoff_ref is cited (handoff_ref is optional, so a citation-only + # check is bypassable); a currency contract additionally requires the + # citation so completion always names the handoff it rests on. + if workflow_contract.currency_handoff_owner is not None: + reasons.extend( + self._validate_completion_handoff_currency( + state=state, producer=producer, cited=signal.handoff_ref + ) + ) elif signal.stop_reason == "unresolvable_error": if producer.workflow_id not in ( workflow_contract.terminal_blocker_reporting_roles @@ -5577,6 +5600,41 @@ def _validate_control_handoff_reference( return ["cited handoff lacks a matching engine acceptance snapshot"] return [] + def _validate_completion_handoff_currency( + self, *, state: LoopState, producer: SignalProducer, cited: str | None + ) -> list[str]: + """Require completion to rest on a handoff the completing attempt made current. + + D13 (A1): the accepted handoff snapshot must have been (re)stamped by the + exact attempt that is declaring ``goal_met``. Evaluated independent of any + ``handoff_ref`` citation (the citation is optional and therefore bypassable + on its own); a currency contract additionally requires the citation so a + completion always names the handoff it rests on. A stale snapshot routes to + the existing control-repair path via a non-empty reason. + """ + + reasons: list[str] = [] + if cited is None: + reasons.append( + "goal_met must cite the current handoff when it is a currency output" + ) + snapshot = state.accepted_handoff_snapshot + if snapshot is None or snapshot.handoff.producer is None: + reasons.append( + "no accepted handoff snapshot was (re)stamped by the completing attempt" + ) + return reasons + stamped = snapshot.handoff.producer + if ( + stamped.workflow_id != producer.workflow_id + or stamped.attempt_id != producer.attempt_id + ): + reasons.append( + "the accepted handoff was not re-stamped by the completing attempt " + "(stale continuity); re-stamp the handoff before declaring completion" + ) + return reasons + def _accept_current_eval_receipts( self, *, @@ -5724,16 +5782,25 @@ def _observe_layer_handoff( ): status = "invalid" reason = "producer attempt is not current or present in session history" - if status == "valid" and ( - handoff.revision < state.handoff_revision - or ( - handoff.revision == state.handoff_revision - and state.handoff_sha256 is not None - and digest != state.handoff_sha256 + if status == "valid" and handoff.revision < state.handoff_revision: + status = "non_monotonic" + reason = "revision moved backward" + elif ( + status == "valid" + and handoff.revision == state.handoff_revision + and state.handoff_sha256 is not None + and digest != state.handoff_sha256 + and not self._is_provenance_only_restamp( + state=state, active=active, handoff=handoff ) ): + # D13: a same-revision content change is tampering UNLESS it is a + # provenance-only re-stamp by the current owner attempt (producer and + # timestamp advance, every other field byte-equal to the accepted + # snapshot). That exception keeps "re-stamp without a material change" + # expressible without opening a fixed-revision rewrite hole. status = "non_monotonic" - reason = "revision moved backward or changed without advancing" + reason = "revision unchanged but content changed without a valid re-stamp" if status == "valid" and handoff.revision >= state.handoff_revision: state.handoff_revision = handoff.revision state.handoff_sha256 = digest @@ -5760,6 +5827,114 @@ def _observe_layer_handoff( }, ) + def _diagnose_currency_outputs( + self, + *, + state: LoopState, + active: CurrentTask, + workflow_contract: WorkflowSetContract, + ) -> None: + """Emit non-gating currency diagnostics for the finishing role (D13/D14). + + These never change harness success, respawn, or touch a cap. For the + handoff (`kind: handoff`) a successful owner finish that did not re-stamp + the handoff raises ``handoff_stale`` and leaves a repair note the standing + owner reads on its next turn (correctness is enforced separately at the + completion boundary, A1). For an advisory output (`kind: advisory`, e.g. + the eval result) an absent declared path raises ``eval_missing``; the + engine never reads the file — freshness is the reading agent's judgment. + """ + + session_dir = session_dir_path( + repo_root=self.repo_root, session_id=state.active_session_id + ) + if active.workflow_id == workflow_contract.currency_handoff_owner: + snapshot = state.accepted_handoff_snapshot + stamped = snapshot.handoff.producer if snapshot is not None else None + if ( + stamped is None + or stamped.workflow_id != active.workflow_id + or stamped.attempt_id != active.attempt_id + ): + failure_id = f"handoff-currency-{uuid.uuid4().hex[:12]}" + write_json_atomic( + path=protocol_failures_dir_path( + repo_root=self.repo_root, session_id=state.active_session_id + ) + / f"{failure_id}.json", + payload={ + "schema_version": 1, + "failure_id": failure_id, + "kind": "stale_handoff", + "reasons": [ + "the handoff owner finished without re-stamping " + "project_state/handoff.json with this attempt's " + "producer identity; re-stamp it on your next turn so a " + "successor never reads stale continuity" + ], + "finishing_attempt": { + "workflow_id": active.workflow_id, + "attempt_id": active.attempt_id, + }, + "created_at": utc_now().isoformat().replace("+00:00", "Z"), + }, + ) + self._emit( + session_id=state.active_session_id, + event_type="handoff_stale", + payload={ + "workflow_id": active.workflow_id, + "attempt_id": active.attempt_id, + "protocol_failure_ref": ( + f"session:/protocol_failures/{failure_id}.json" + ), + }, + ) + for entry in workflow_contract.advisory_currency_outputs(): + if active.workflow_id != entry.owner_role: + continue + declared_path = session_dir / entry.path + if not declared_path.is_file(): + self._emit( + session_id=state.active_session_id, + event_type="eval_missing", + payload={ + "workflow_id": active.workflow_id, + "attempt_id": active.attempt_id, + "path": entry.path, + }, + ) + + @staticmethod + def _is_provenance_only_restamp( + *, state: LoopState, active: CurrentTask, handoff: LayerHandoff + ) -> bool: + """Return whether a same-revision handoff is a legitimate re-stamp (D13). + + A re-stamp is legitimate only when the current owner attempt advances the + provenance and timestamp of an otherwise-identical handoff: the producer + is exactly this finishing attempt, ``updated_at`` does not move backward, + and every field other than ``producer``/``updated_at`` equals the accepted + snapshot. Anything else at the same revision is tampering. + """ + + snapshot = state.accepted_handoff_snapshot + producer = handoff.producer + if snapshot is None or producer is None: + return False + if ( + active.session_id != state.active_session_id + or producer.workflow_id != active.workflow_id + or producer.attempt_id != active.attempt_id + ): + return False + if handoff.updated_at < snapshot.handoff.updated_at: + return False + ignore = {"producer", "updated_at"} + return handoff.model_dump(exclude=ignore) == snapshot.handoff.model_dump( + exclude=ignore + ) + @staticmethod def _handoff_producer_is_known( *, state: LoopState, active: CurrentTask, handoff: LayerHandoff diff --git a/src/loopy_loop/models.py b/src/loopy_loop/models.py index 03e7522..a75c649 100644 --- a/src/loopy_loop/models.py +++ b/src/loopy_loop/models.py @@ -243,6 +243,26 @@ class WorkflowEvaluationContract(BaseModel): check_runner_roles: list[str] = Field(default_factory=list) +CANONICAL_HANDOFF_CURRENCY_PATH = "project_state/handoff.json" + + +class CurrencyOutput(BaseModel): + """A role output whose currency the engine tracks per the D13/D14 contract. + + ``kind: handoff`` marks the canonical layer handoff as completion-gated and + per-iteration diagnosed (structural continuity — D13). ``kind: advisory`` + marks an optional evidence file (e.g. the eval result) whose *presence* the + engine may diagnose but never enforces (D14/D11). The path is repo-relative + and confined; the engine reads it from here rather than hard-coding literals. + """ + + model_config = ConfigDict(extra="forbid") + + path: str + owner_role: str + kind: Literal["handoff", "advisory"] + + class WorkflowSetContract(BaseModel): """Frozen role and protocol contract for one durable session layer.""" @@ -256,6 +276,7 @@ class WorkflowSetContract(BaseModel): evaluation: WorkflowEvaluationContract = Field( default_factory=WorkflowEvaluationContract ) + currency_outputs: list[CurrencyOutput] = Field(default_factory=list) task_acceptance_role: str | None = None terminal_blocker_reporting_roles: list[str] = Field(default_factory=list) child_interface: Literal["none", "recursive"] = "recursive" @@ -304,6 +325,77 @@ def validate_role_references(self) -> Self: raise ValueError("protocol v3 must use evaluation, not legacy eval owners") return self + @model_validator(mode="after") + def validate_currency_outputs(self) -> Self: + """Validate declared currency outputs (D13/D14). + + Paths are repo-relative, confined, and unique; every ``owner_role`` names + a declared role. A ``kind: handoff`` entry must name the canonical + protocol handoff identity (its path is fixed by the engine, and its owner + must be the declared ``handoff_owner``) — so ``kind: handoff`` is an + opt-in marker on the engine's own handoff, never a user-chosen path; at + most one may be declared. + """ + + if not self.currency_outputs: + return self + seen_paths: set[str] = set() + handoff_entries = 0 + for entry in self.currency_outputs: + path = entry.path + if ( + not path + or path.startswith("/") + or path.strip() != path + or ".." in path.split("/") + or "\\" in path + ): + raise ValueError( + f"currency_outputs path must be repo-relative and confined: {path!r}" + ) + if path in seen_paths: + raise ValueError(f"duplicate currency_outputs path: {path!r}") + seen_paths.add(path) + if entry.owner_role not in self.roles: + raise ValueError( + f"currency_outputs owner_role names an unknown role: " + f"{entry.owner_role!r}" + ) + if entry.kind == "handoff": + handoff_entries += 1 + if self.orchestration is None: + raise ValueError( + "a kind: handoff currency output requires an orchestration " + "contract" + ) + if path != CANONICAL_HANDOFF_CURRENCY_PATH: + raise ValueError( + "a kind: handoff currency output must name the canonical " + f"handoff path {CANONICAL_HANDOFF_CURRENCY_PATH!r}, got {path!r}" + ) + if entry.owner_role != self.orchestration.handoff_owner: + raise ValueError( + "a kind: handoff currency output must be owned by the " + "declared handoff_owner" + ) + if handoff_entries > 1: + raise ValueError("at most one kind: handoff currency output is allowed") + return self + + @property + def currency_handoff_owner(self) -> str | None: + """Return the role that owns a declared ``kind: handoff`` currency output.""" + + for entry in self.currency_outputs: + if entry.kind == "handoff": + return entry.owner_role + return None + + def advisory_currency_outputs(self) -> list[CurrencyOutput]: + """Return declared ``kind: advisory`` currency outputs (may be empty).""" + + return [entry for entry in self.currency_outputs if entry.kind == "advisory"] + @property def completion_role(self) -> str | None: """Return the version-appropriate successful-control owner.""" @@ -586,7 +678,10 @@ class LoopState(BaseModel): # descendants inherit the same exact payload rather than re-reading YAML. harness_capability_roster: HarnessCapabilityRoster | None = Field(default=None) # Last structurally valid handoff observed after a completed v3 attempt. - # This is diagnostic continuity only; it never gates scheduling/control. + # The observation never gates scheduling. It gates terminal control only when + # the frozen contract declares the handoff a `kind: handoff` currency output + # (D13/A1): a `goal_met` must rest on a snapshot the completing attempt + # re-stamped. Per-iteration staleness is a pure diagnostic (D13/A2). handoff_revision: int = Field(default=0, ge=0) handoff_sha256: str | None = Field(default=None) accepted_eval_receipt_seals: dict[str, AcceptedEvalReceiptSeal] = Field( diff --git a/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/contract.yaml b/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/contract.yaml index 36e10f1..eebc564 100644 --- a/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/contract.yaml +++ b/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/contract.yaml @@ -47,5 +47,16 @@ evaluation: advisory: true check_author_roles: [eval_reviewer, outer] check_runner_roles: [outer] +# D13/D14 currency outputs. handoff: the canonical layer handoff is completion-gated +# and per-iteration diagnosed (outer must re-stamp it before declaring the goal met). +# advisory: the eval runner's result file — the engine only diagnoses its absence; it +# never gates, and freshness is judged by the reading roles (evals stay optional). +currency_outputs: + - path: project_state/handoff.json + owner_role: outer + kind: handoff + - path: project_state/eval_results.md + owner_role: eval_runner + kind: advisory terminal_blocker_reporting_roles: [outer, inner, eval_reviewer, eval_runner] child_interface: none diff --git a/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/workflows/eval_runner/prompt.txt b/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/workflows/eval_runner/prompt.txt index b20210b..2bd6e6f 100644 --- a/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/workflows/eval_runner/prompt.txt +++ b/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/workflows/eval_runner/prompt.txt @@ -10,22 +10,32 @@ and the eval reviewer. Choose the judge family and tier deliberately from the capability roster; a family different from the implementer and check author is useful independence, not a requirement. -Run from the repository root, substituting concrete roster values: +Run from the repository root, substituting concrete roster values. Pass +--result-out so eval-banana writes the canonical, provenance-stamped result +directly to project_state/eval_results.md — do NOT hand-copy a verdict into that +file yourself (that is how a verdict ends up off-channel or unversioned). Use +--provenance-attempt with THIS attempt's id (from your assignment header): eval-banana validate --no-project-config --cwd --check-dir --harness-agent - eval-banana run --no-project-config --flat-output --cwd --check-dir --output-dir --pass-threshold 1.0 --harness-agent --harness-model --harness-reasoning-effort + eval-banana run --no-project-config --flat-output --cwd --check-dir --output-dir --pass-threshold 1.0 --harness-agent --harness-model --harness-reasoning-effort --result-out /eval_results.md --provenance-attempt +eval-banana writes project_state/eval_results.md itself — a human-readable report +with a machine-readable provenance header (the observed commit and dirty state, +per-check model, verdict). Read report.md for the verdicts and reasons — not report.json, which is large and machine-oriented. Open an individual per-check artifact only to investigate a -specific failure. +failure. -Record the outcome in project_state/eval_results.md: the headline verdict, the -effective judge family, model, and effort you actually used, each check's -pass/fail with a one-line reason, and the commit the run observed. Then consume -the request — fold project_state/eval_request.md into that record with its -outcome and remove the pending request so it does not fire again. When outer -asked for this as the final check before declaring the goal met, state the honest -goal-met verdict here; outer weighs it and owns the control decision. +eval_results.md belongs to eval-banana now — do NOT write, append to, or edit it +yourself; hand-editing would corrupt the provenance header the readers rely on. +Then consume the request: remove project_state/eval_request.md so it does not fire +again, and record any commentary of your own (judgment on weak or missing check +definitions, the family/tier you chose and why) in project_state/eval_state.md for +outer and the eval reviewer — not in eval_results.md. When outer asked for this as +the final check before declaring the goal met, the honest goal-met verdict is +already in the result file eval-banana wrote; outer weighs it and owns the control +decision. If eval-banana is too old to support --result-out, upgrade it; do not +fall back to hand-transcription. Keep the full report and verbose logs in your scratch dir. Stopping short is only for a genuinely terminal blocker, after autonomous routes are exhausted. diff --git a/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/workflows/inner/prompt.txt b/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/workflows/inner/prompt.txt index 2f5611f..6cfcd64 100644 --- a/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/workflows/inner/prompt.txt +++ b/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/workflows/inner/prompt.txt @@ -18,6 +18,11 @@ check facts. Composing a team for this — whether to delegate at all, how many agents, and in what roles — is your judgment; it is equally valid to do the work directly. +Base your work on live repository and CI facts, not on stale summaries. If you +consult project_state/eval_results.md, check the commit its provenance header +records: a verdict observed against a commit that is not current HEAD (or a dirty +tree) is stale — treat it as out of date, not as a fact about your change. + Record what changed, what you verified, and what remains in the task's record under project_state/tasks/. Do not touch outer-owned selection, status, or acceptance fields. Return a short upward result naming the task, the changes, the diff --git a/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/workflows/outer/prompt.txt b/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/workflows/outer/prompt.txt index f9f6f0f..51be502 100644 --- a/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/workflows/outer/prompt.txt +++ b/src/loopy_loop/templates/inner_outer_eval/.loopy_loop/workflow_sets/inner_outer_eval/workflows/outer/prompt.txt @@ -20,8 +20,12 @@ What you own - Start from: the tile model in the board module and the drop table in the design notes. - project_state/handoff.json is the rolling layer handoff. Its authoritative - schema is in paths.json → contracts; conform exactly and rewrite it atomically - after any material change. + schema is in paths.json → contracts; conform exactly and rewrite it atomically. + It carries a producer identity — re-stamp it with THIS attempt's identity every + iteration, and always before declaring completion. A handoff still naming a past + attempt is stale continuity: the engine refuses a completion resting on it, and + drops a per-iteration staleness note under paths.json → protocol_failures for you + to repair next turn. A successor must never inherit a stale summary. Sizing the work Each iteration costs a full coordinator run before any work happens. Prefer work @@ -30,28 +34,46 @@ iteration; slice only when a package genuinely exceeds one iteration's reach. Verification and reconciliation ride along with the work they verify; they are rarely their own iteration. +Reconcile against ground truth — do not be misled by a summary +The handoff and plan are summaries, not authority. Before selecting the next task +or deciding completion, reconcile them against LIVE state — the working tree, the +diff, tests, PR and CI facts. When a durable artifact is missing, ambiguous, or +contradicts the repository (it says "merge X" but the PR is merged; an approval it +omits), trust live state, not the stale note. Durable summaries live in +project_state/ (plan.md, tasks/, finished.md, handoff.json, eval_results.md); as a +fallback the inner attempts' raw traces are at paths.json → raw_root, each +iteration under raw/_/… — read the SPECIFIC relevant prior +iteration when a summary is missing or contradicts the repo, don't scan the tree. + Accepting work Review inner's attempt on its merits — the diff, the repository state, checks, and git/CI facts. A clean harness return or an open PR is not acceptance. Accept it, or return it with specific reasons; then select the next task and make its scope and observable result unambiguous. Append accepted work to -project_state/finished.md. +project_state/finished.md. When the selected work is delivered, its checks are +green, and an advisory review approves it, accept and advance — record residual +non-blocking items as new or deferred tasks, not as a reason to reopen a completed +unit. Do not spin re-polishing done work. Evaluation Eval is advisory and runs on request. When you reach a milestone or intend to declare completion, write project_state/eval_request.md — one line naming the scope and why now — and let the eval roles run; read their verdict in -project_state/eval_results.md. A terminal goal check is required before you -declare the goal met, but a failed or missing check is input to your judgment, -not a veto. +project_state/eval_results.md. A terminal eval is available and recommended near +completion, but you may decide it is unnecessary; a failed or missing check is +input to your judgment, never a veto. The result file records the commit it +observed — if that commit is not current HEAD (or the tree was dirty), the verdict +is stale: re-request it or set it aside rather than relying on a past check. Completion -When this layer's goal is met, bring the plan, tasks, ledger, and handoff current, -then atomically publish successful control to control.json. The authoritative -active control fields and logical-reference grammar are in -paths.json → contracts; conform exactly. In human terms: use this attempt's -identity, explain why the goal is met, and cite only durable evidence that -contract permits. +When this layer's goal is met, bring the plan, tasks, ledger, and handoff current +— re-stamp handoff.json with THIS attempt's identity — then atomically publish +successful control to control.json, citing the handoff. The authoritative active +control fields and logical-reference grammar are in paths.json → contracts; +conform exactly. In human terms: use this attempt's identity, explain why the goal +is met, and cite only durable evidence that contract permits. A completion that +rests on a handoff you did not re-stamp this attempt is refused as stale — bring it +current first. Stopping short of the goal is only for a genuinely terminal blocker, after autonomous routes are exhausted. diff --git a/src/tests/test_recursive_loop_contract_v3.py b/src/tests/test_recursive_loop_contract_v3.py index fae48be..8678cf2 100644 --- a/src/tests/test_recursive_loop_contract_v3.py +++ b/src/tests/test_recursive_loop_contract_v3.py @@ -11,6 +11,8 @@ from loopy_loop.assignments import repository_id from loopy_loop.coordinator_app import create_coordinator_app +from loopy_loop.events import events_path +from loopy_loop.events import read_events from loopy_loop.models import LoopState from loopy_loop.models import REQUIRED_V3_WORKER_CAPABILITIES from loopy_loop.models import utc_now @@ -56,11 +58,12 @@ def _v3_contract( completion_role: str, check_runner_roles: list[str] | None = None, child_interface: str = "none", + currency_outputs: list[dict[str, object]] | None = None, ) -> dict[str, object]: """Return a minimal v3 contract with one durable orchestrator.""" roles = dict.fromkeys([*workflow_ids, completion_role]) - return { + contract: dict[str, object] = { "schema_version": 1, "session_protocol_version": 3, "layer_kind": "delivery", @@ -87,6 +90,9 @@ def _v3_contract( "terminal_blocker_reporting_roles": list(roles), "child_interface": child_interface, } + if currency_outputs is not None: + contract["currency_outputs"] = currency_outputs + return contract def _write_contract( @@ -133,6 +139,7 @@ def _build_v3_repo( check_runner_roles: list[str] | None = None, child_interface: str = "none", root_config: dict[str, object] | None = None, + currency_outputs: list[dict[str, object]] | None = None, ) -> Path: """Create a root repository whose selected workflow set uses protocol v3.""" @@ -145,6 +152,7 @@ def _build_v3_repo( completion_role=completion_role, check_runner_roles=check_runner_roles, child_interface=child_interface, + currency_outputs=currency_outputs, ), ) return repo_root @@ -237,6 +245,9 @@ def _write_handoff( task: dict[str, Any], state: LoopState, producer_attempt_id: str | None = None, + revision: int = 1, + summary: str = "The assigned outcome is ready.", + updated_at: str | None = None, ) -> Path: """Write one structurally valid handoff with selectable attempt provenance.""" @@ -247,12 +258,12 @@ def _write_handoff( "schema_version": 1, "session_id": task["session_id"], "goal_sha256": state.goal_hash, - "revision": 1, + "revision": revision, "producer": { "workflow_id": task["workflow_id"], "attempt_id": producer_attempt_id or task["attempt_id"], }, - "summary": "The assigned outcome is ready.", + "summary": summary, "accepted_outcomes": ["assigned-outcome"], "open_work": [], "risks": [], @@ -260,7 +271,11 @@ def _write_handoff( "evidence_refs": [], "delivery_refs": [], "eval_refs": [], - "updated_at": utc_now().isoformat().replace("+00:00", "Z"), + "updated_at": ( + updated_at + if updated_at is not None + else utc_now().isoformat().replace("+00:00", "Z") + ), }, indent=2, ), @@ -980,3 +995,480 @@ def test_v3_non_control_child_stop_writes_outcome_and_resumes_parent( ) assert parent_state.active_child_session_id is None assert parent_state.stop_reason == "no_eligible_workflow" + + +# --------------------------------------------------------------------------- +# D13/D14/D15: handoff currency, advisory eval provenance, raw-trace exposure. +# --------------------------------------------------------------------------- + +_HANDOFF_CURRENCY: list[dict[str, object]] = [ + {"path": "project_state/handoff.json", "owner_role": "outer", "kind": "handoff"} +] + + +def _outer_inner_workflows() -> dict[str, dict[str, object]]: + """Return the alternating outer/inner roles used by the currency tests.""" + + return {"outer": _workflow(priority=10), "inner": _workflow(priority=0)} + + +def _advance_to_outer(*, client: TestClient, task: dict[str, Any]) -> dict[str, Any]: + """Finish `task` and return the next 'outer' assignment (running interleaved roles). + + The stock cadence alternates outer/inner, so the standing orchestrator's next + turn is reached by finishing any interleaved inner attempt. + """ + + response = client.post("/finished", json=_finish_body(task=task)).json() + while response.get("action") == "run" and response["workflow_id"] != "outer": + response = client.post("/finished", json=_finish_body(task=response)).json() + assert response.get("action") == "run" + assert response["workflow_id"] == "outer" + return response + + +def _events_of(*, repo_root: Path, session_id: str, event_type: str) -> list[Any]: + """Return every emitted event of one type for a session.""" + + events = read_events(path=events_path(repo_root=repo_root, session_id=session_id)) + return [event for event in events if event["type"] == event_type] + + +@pytest.mark.parametrize("cite", [True, False], ids=["cited", "uncited"]) +def test_currency_completion_rejected_when_handoff_not_restamped( + repo_builder: Any, monkeypatch: pytest.MonkeyPatch, cite: bool +) -> None: + """A1: goal_met resting on a stale (past-attempt) handoff is rejected. + + The rejection holds whether or not the completion cites handoff_ref, closing + the uncited-goal_met bypass (handoff_ref is optional on control). + """ + + monkeypatch.setenv("OPENROUTER_API_KEY", "secret") + repo_root = _build_v3_repo( + repo_builder=repo_builder, + workflows=_outer_inner_workflows(), + completion_role="outer", + currency_outputs=_HANDOFF_CURRENCY, + ) + client = TestClient(create_coordinator_app(repo_root=repo_root, resume=False)) + task0 = client.post("/register", json=_register_body(repo_root=repo_root)).json() + assert task0["workflow_id"] == "outer" + state = _read_state(repo_root=repo_root, session_id=task0["session_id"]) + _write_handoff(repo_root=repo_root, task=task0, state=state) + + outer2 = _advance_to_outer(client=client, task=task0) + assert outer2["attempt_id"] != task0["attempt_id"] + # outer2 declares completion WITHOUT re-stamping the handoff (still task0's). + _write_goal_met_control( + repo_root=repo_root, + task=outer2, + handoff_ref="session:/project_state/handoff.json" if cite else None, + ) + response = client.post("/finished", json=_finish_body(task=outer2)) + + assert response.status_code == 200 + final = _read_state(repo_root=repo_root, session_id=task0["session_id"]) + assert final.goal_met is False + assert final.history[-1].success is False + assert final.history[-1].error == "invalid_control_output" + assert final.control_protocol_consecutive_failures == 1 + # A1 rides the existing control-reject path, which also ticks the general + # per-workflow failure cap (documented dual-counter, pre-existing behavior). + assert final.workflow_consecutive_failures.get("outer", 0) == 1 + + +def test_currency_completion_accepted_when_handoff_restamped( + repo_builder: Any, monkeypatch: pytest.MonkeyPatch +) -> None: + """A1: goal_met resting on a handoff the completing attempt re-stamped closes.""" + + monkeypatch.setenv("OPENROUTER_API_KEY", "secret") + repo_root = _build_v3_repo( + repo_builder=repo_builder, + workflows=_outer_inner_workflows(), + completion_role="outer", + currency_outputs=_HANDOFF_CURRENCY, + ) + client = TestClient(create_coordinator_app(repo_root=repo_root, resume=False)) + task0 = client.post("/register", json=_register_body(repo_root=repo_root)).json() + state = _read_state(repo_root=repo_root, session_id=task0["session_id"]) + _write_handoff(repo_root=repo_root, task=task0, state=state) + + outer2 = _advance_to_outer(client=client, task=task0) + # outer2 re-stamps the handoff with its own identity (provenance-only re-stamp + # at the same revision) and cites it. + _write_handoff(repo_root=repo_root, task=outer2, state=state) + _write_goal_met_control( + repo_root=repo_root, + task=outer2, + handoff_ref="session:/project_state/handoff.json", + ) + response = client.post("/finished", json=_finish_body(task=outer2)) + + assert response.status_code == 200 + assert response.json()["stop_reason"] == "goal_met" + final = _read_state(repo_root=repo_root, session_id=task0["session_id"]) + assert final.goal_met is True + assert final.history[-1].success is True + + +def test_currency_completion_unaffected_without_declaration( + repo_builder: Any, monkeypatch: pytest.MonkeyPatch +) -> None: + """Opt-in: a contract with no currency_outputs keeps prior completion behavior.""" + + monkeypatch.setenv("OPENROUTER_API_KEY", "secret") + repo_root = _build_v3_repo( + repo_builder=repo_builder, + workflows=_outer_inner_workflows(), + completion_role="outer", + ) + client = TestClient(create_coordinator_app(repo_root=repo_root, resume=False)) + task0 = client.post("/register", json=_register_body(repo_root=repo_root)).json() + state = _read_state(repo_root=repo_root, session_id=task0["session_id"]) + _write_handoff(repo_root=repo_root, task=task0, state=state) + + outer2 = _advance_to_outer(client=client, task=task0) + # Stale handoff (task0's) + goal_met, but no currency declaration → accepted. + _write_goal_met_control(repo_root=repo_root, task=outer2) + response = client.post("/finished", json=_finish_body(task=outer2)) + + assert response.status_code == 200 + assert response.json()["stop_reason"] == "goal_met" + final = _read_state(repo_root=repo_root, session_id=task0["session_id"]) + assert final.goal_met is True + + +def test_currency_handoff_stale_diagnostic_is_pure( + repo_builder: Any, monkeypatch: pytest.MonkeyPatch +) -> None: + """A2: an un-re-stamped owner finish emits handoff_stale with no failure.""" + + monkeypatch.setenv("OPENROUTER_API_KEY", "secret") + repo_root = _build_v3_repo( + repo_builder=repo_builder, + workflows=_outer_inner_workflows(), + completion_role="outer", + currency_outputs=_HANDOFF_CURRENCY, + ) + client = TestClient(create_coordinator_app(repo_root=repo_root, resume=False)) + task0 = client.post("/register", json=_register_body(repo_root=repo_root)).json() + state = _read_state(repo_root=repo_root, session_id=task0["session_id"]) + _write_handoff(repo_root=repo_root, task=task0, state=state) + handoff_file = handoff_path(repo_root=repo_root, session_id=task0["session_id"]) + + outer2 = _advance_to_outer(client=client, task=task0) + bytes_before = handoff_file.read_bytes() + # outer2 finishes successfully but never re-stamps the handoff, no control. + client.post("/finished", json=_finish_body(task=outer2)) + + stale_events = _events_of( + repo_root=repo_root, session_id=task0["session_id"], event_type="handoff_stale" + ) + assert len(stale_events) == 1 + final = _read_state(repo_root=repo_root, session_id=task0["session_id"]) + stale_finish = next( + entry for entry in final.history if entry.attempt_id == outer2["attempt_id"] + ) + assert stale_finish.success is True + assert final.control_protocol_consecutive_failures == 0 + # Pure diagnostic: it touches neither the control cap nor the general cap. + assert final.workflow_consecutive_failures.get("outer", 0) == 0 + assert handoff_file.read_bytes() == bytes_before + + +def test_currency_no_stale_diagnostic_on_crashed_attempt( + repo_builder: Any, monkeypatch: pytest.MonkeyPatch +) -> None: + """Currency diagnostics run only after a mechanically successful attempt.""" + + monkeypatch.setenv("OPENROUTER_API_KEY", "secret") + repo_root = _build_v3_repo( + repo_builder=repo_builder, + workflows=_outer_inner_workflows(), + completion_role="outer", + currency_outputs=_HANDOFF_CURRENCY, + ) + client = TestClient(create_coordinator_app(repo_root=repo_root, resume=False)) + task0 = client.post("/register", json=_register_body(repo_root=repo_root)).json() + state = _read_state(repo_root=repo_root, session_id=task0["session_id"]) + _write_handoff(repo_root=repo_root, task=task0, state=state) + outer2 = _advance_to_outer(client=client, task=task0) + + # A crashed attempt (harness failure) must not raise a currency diagnostic. + client.post("/finished", json=_finish_body(task=outer2, success=False)) + + assert not _events_of( + repo_root=repo_root, session_id=task0["session_id"], event_type="handoff_stale" + ) + + +def test_provenance_only_restamp_vs_tamper_same_revision( + repo_builder: Any, monkeypatch: pytest.MonkeyPatch +) -> None: + """D13: same-revision re-stamp is valid; a same-revision content change is not.""" + + monkeypatch.setenv("OPENROUTER_API_KEY", "secret") + repo_root = _build_v3_repo( + repo_builder=repo_builder, + workflows=_outer_inner_workflows(), + completion_role="outer", + currency_outputs=_HANDOFF_CURRENCY, + ) + client = TestClient(create_coordinator_app(repo_root=repo_root, resume=False)) + task0 = client.post("/register", json=_register_body(repo_root=repo_root)).json() + state = _read_state(repo_root=repo_root, session_id=task0["session_id"]) + _write_handoff(repo_root=repo_root, task=task0, state=state, revision=1) + + # (a) same revision, provenance-only change by the new owner attempt → valid. + outer2 = _advance_to_outer(client=client, task=task0) + _write_handoff(repo_root=repo_root, task=outer2, state=state, revision=1) + outer4 = _advance_to_outer(client=client, task=outer2) + accepted = _read_state(repo_root=repo_root, session_id=task0["session_id"]) + assert accepted.latest_handoff_observation is not None + assert accepted.latest_handoff_observation.status == "valid" + assert accepted.accepted_handoff_snapshot is not None + assert accepted.accepted_handoff_snapshot.handoff.producer is not None + assert ( + accepted.accepted_handoff_snapshot.handoff.producer.attempt_id + == outer2["attempt_id"] + ) + + # (b) same revision, a real content change → non_monotonic (tamper detection). + _write_handoff( + repo_root=repo_root, + task=outer4, + state=state, + revision=1, + summary="A materially different summary at the same revision.", + ) + client.post("/finished", json=_finish_body(task=outer4)) + tampered = _read_state(repo_root=repo_root, session_id=task0["session_id"]) + assert tampered.latest_handoff_observation is not None + assert tampered.latest_handoff_observation.status == "non_monotonic" + + +@pytest.mark.parametrize("write_result", [False, True], ids=["absent", "present"]) +def test_advisory_currency_missing_emits_diagnostic_only( + repo_builder: Any, monkeypatch: pytest.MonkeyPatch, write_result: bool +) -> None: + """D14: an absent advisory output emits eval_missing; never fails the run.""" + + monkeypatch.setenv("OPENROUTER_API_KEY", "secret") + repo_root = _build_v3_repo( + repo_builder=repo_builder, + workflows={ + "outer": _workflow(priority=0), + "eval_runner": _workflow(priority=100), + }, + completion_role="outer", + currency_outputs=[ + *_HANDOFF_CURRENCY, + { + "path": "project_state/eval_results.md", + "owner_role": "eval_runner", + "kind": "advisory", + }, + ], + ) + client = TestClient(create_coordinator_app(repo_root=repo_root, resume=False)) + task = client.post("/register", json=_register_body(repo_root=repo_root)).json() + assert task["workflow_id"] == "eval_runner" + if write_result: + result_path = ( + handoff_path(repo_root=repo_root, session_id=task["session_id"]).parent + / "eval_results.md" + ) + result_path.write_text("# eval result\n", encoding="utf-8") + + client.post("/finished", json=_finish_body(task=task)) + + missing = _events_of( + repo_root=repo_root, session_id=task["session_id"], event_type="eval_missing" + ) + state = _read_state(repo_root=repo_root, session_id=task["session_id"]) + assert state.history[-1].success is True + if write_result: + assert not missing + else: + assert len(missing) == 1 + assert missing[0]["payload"]["path"] == "project_state/eval_results.md" + + +def test_orchestration_role_assignment_exposes_raw_root( + repo_builder: Any, monkeypatch: pytest.MonkeyPatch +) -> None: + """D15: only the orchestration role receives the read-only raw-trace root.""" + + monkeypatch.setenv("OPENROUTER_API_KEY", "secret") + repo_root = _build_v3_repo( + repo_builder=repo_builder, + workflows={ + "outer": _workflow(priority=0), + "eval_runner": _workflow(priority=100), + }, + completion_role="outer", + ) + client = TestClient(create_coordinator_app(repo_root=repo_root, resume=False)) + eval_task = client.post( + "/register", json=_register_body(repo_root=repo_root) + ).json() + assert eval_task["workflow_id"] == "eval_runner" + eval_assignment = json.loads( + Path(eval_task["assignment_path"]).read_text(encoding="utf-8") + ) + assert "raw_root" not in eval_assignment["absolute_paths"] + + outer_task = client.post("/finished", json=_finish_body(task=eval_task)).json() + assert outer_task["workflow_id"] == "outer" + outer_assignment = json.loads( + Path(outer_task["assignment_path"]).read_text(encoding="utf-8") + ) + assert "raw_root" in outer_assignment["absolute_paths"] + assert outer_assignment["absolute_paths"]["raw_root"].endswith("/raw") + + +@pytest.mark.parametrize( + "entry", + [ + {"path": "project_state/other.json", "owner_role": "outer", "kind": "handoff"}, + { + "path": "project_state/handoff.json", + "owner_role": "inner", + "kind": "handoff", + }, + {"path": "/etc/passwd", "owner_role": "outer", "kind": "advisory"}, + {"path": "../escape.md", "owner_role": "outer", "kind": "advisory"}, + {"path": "x.md", "owner_role": "ghost", "kind": "advisory"}, + ], + ids=["handoff-path", "handoff-owner", "absolute", "traversal", "unknown-owner"], +) +def test_currency_outputs_contract_validation_rejects_bad_entries( + entry: dict[str, object], +) -> None: + """Contract load validates currency_outputs (path confinement + identity).""" + + contract = _v3_contract( + workflow_ids=["outer", "inner"], + completion_role="outer", + currency_outputs=[entry], + ) + with pytest.raises(ValueError): + WorkflowSetContract.model_validate(contract) + + +def test_currency_outputs_contract_validation_rejects_duplicate_paths() -> None: + """Duplicate currency_outputs paths are rejected at contract load.""" + + contract = _v3_contract( + workflow_ids=["outer", "eval_runner"], + completion_role="outer", + currency_outputs=[ + {"path": "project_state/x.md", "owner_role": "outer", "kind": "advisory"}, + { + "path": "project_state/x.md", + "owner_role": "eval_runner", + "kind": "advisory", + }, + ], + ) + with pytest.raises(ValueError): + WorkflowSetContract.model_validate(contract) + + +def test_currency_completion_rejected_when_no_handoff_snapshot( + repo_builder: Any, monkeypatch: pytest.MonkeyPatch +) -> None: + """A1: goal_met with no accepted handoff snapshot at all is rejected.""" + + monkeypatch.setenv("OPENROUTER_API_KEY", "secret") + repo_root = _build_v3_repo( + repo_builder=repo_builder, + workflows=_outer_inner_workflows(), + completion_role="outer", + currency_outputs=_HANDOFF_CURRENCY, + ) + client = TestClient(create_coordinator_app(repo_root=repo_root, resume=False)) + task = client.post("/register", json=_register_body(repo_root=repo_root)).json() + # No handoff is ever written, so there is no accepted snapshot to rest on. + _write_goal_met_control( + repo_root=repo_root, + task=task, + handoff_ref="session:/project_state/handoff.json", + ) + response = client.post("/finished", json=_finish_body(task=task)) + + assert response.status_code == 200 + final = _read_state(repo_root=repo_root, session_id=task["session_id"]) + assert final.goal_met is False + assert final.history[-1].success is False + assert final.history[-1].error == "invalid_control_output" + + +def test_currency_non_owner_finish_emits_no_stale_diagnostic( + repo_builder: Any, monkeypatch: pytest.MonkeyPatch +) -> None: + """A2: only the handoff owner is diagnosed; a non-owner finish is never stale.""" + + monkeypatch.setenv("OPENROUTER_API_KEY", "secret") + repo_root = _build_v3_repo( + repo_builder=repo_builder, + workflows=_outer_inner_workflows(), + completion_role="outer", + currency_outputs=_HANDOFF_CURRENCY, + ) + client = TestClient(create_coordinator_app(repo_root=repo_root, resume=False)) + task0 = client.post("/register", json=_register_body(repo_root=repo_root)).json() + state = _read_state(repo_root=repo_root, session_id=task0["session_id"]) + _write_handoff(repo_root=repo_root, task=task0, state=state) + + # outer0 re-stamped and finishes; the interleaved inner attempt (a non-owner) + # then finishes without touching the handoff. + inner1 = client.post("/finished", json=_finish_body(task=task0)).json() + assert inner1["workflow_id"] == "inner" + client.post("/finished", json=_finish_body(task=inner1)) + + assert not _events_of( + repo_root=repo_root, session_id=task0["session_id"], event_type="handoff_stale" + ) + + +def test_provenance_restamp_rejected_when_timestamp_moves_backward( + repo_builder: Any, monkeypatch: pytest.MonkeyPatch +) -> None: + """D13: a same-revision re-stamp whose updated_at regresses is non_monotonic.""" + + monkeypatch.setenv("OPENROUTER_API_KEY", "secret") + repo_root = _build_v3_repo( + repo_builder=repo_builder, + workflows=_outer_inner_workflows(), + completion_role="outer", + currency_outputs=_HANDOFF_CURRENCY, + ) + client = TestClient(create_coordinator_app(repo_root=repo_root, resume=False)) + task0 = client.post("/register", json=_register_body(repo_root=repo_root)).json() + state = _read_state(repo_root=repo_root, session_id=task0["session_id"]) + _write_handoff( + repo_root=repo_root, + task=task0, + state=state, + revision=1, + updated_at="2026-07-22T12:00:00Z", + ) + + outer2 = _advance_to_outer(client=client, task=task0) + # Same revision, current owner, but the timestamp regresses → not a valid + # provenance-only re-stamp. + _write_handoff( + repo_root=repo_root, + task=outer2, + state=state, + revision=1, + updated_at="2026-07-22T11:00:00Z", + ) + client.post("/finished", json=_finish_body(task=outer2)) + + final = _read_state(repo_root=repo_root, session_id=task0["session_id"]) + assert final.latest_handoff_observation is not None + assert final.latest_handoff_observation.status == "non_monotonic" diff --git a/uv.lock b/uv.lock index b6e25d1..5217c53 100644 --- a/uv.lock +++ b/uv.lock @@ -73,16 +73,16 @@ wheels = [ [[package]] name = "eval-banana" -version = "0.3.5" +version = "0.5.0" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "click" }, { name = "pydantic" }, { name = "pyyaml" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/35/7a/37b7290947c2c8431ec4654f10d676d347414264c09826785a90c694a8c0/eval_banana-0.3.5.tar.gz", hash = "sha256:84a86d5a8ecc802200631cb48e9bc228131a91c70b3d42c407bafec246d1c44d", size = 2771312 } +sdist = { url = "https://files.pythonhosted.org/packages/10/d8/df7d0cb41e263354a7d07c4a4a5a54d8bf745b6b5cb043d9e8dff80710db/eval_banana-0.5.0.tar.gz", hash = "sha256:f3a58b5f0b1e613804f6be96b2e76f1a0eafe148977eb4478f4d73685dbdf9c2", size = 2792706 } wheels = [ - { url = "https://files.pythonhosted.org/packages/b2/26/1345603725ad4368903f6cf683452e17cadc7adc73b5f0b767980452d0f1/eval_banana-0.3.5-py3-none-any.whl", hash = "sha256:7ea8041e93175c588a782dd43e7d6c667be12d7f21c6028810861f6bc82a2210", size = 38861 }, + { url = "https://files.pythonhosted.org/packages/5d/aa/005a74440eacc94320fd59cecf2e7f4fa0bb7491e6ff262cfddb5fad584c/eval_banana-0.5.0-py3-none-any.whl", hash = "sha256:860b213e0a7b7f6d0e25e2de621051b8ef702f3adb4c3618d925ad94f09f023f", size = 46081 }, ] [[package]] @@ -272,7 +272,7 @@ wheels = [ [[package]] name = "loopy-loop" -version = "0.10.1" +version = "0.11.0" source = { editable = "." } dependencies = [ { name = "click" }, @@ -298,7 +298,7 @@ dev = [ [package.metadata] requires-dist = [ { name = "click", specifier = ">=8.1" }, - { name = "eval-banana", specifier = ">=0.3.5" }, + { name = "eval-banana", specifier = ">=0.5.0" }, { name = "fastapi", specifier = ">=0.115.0" }, { name = "filelock", specifier = ">=3.16" }, { name = "httpx", specifier = ">=0.28" },