From fe456c3622e57d8f0494c81991c3b66f1661c945 Mon Sep 17 00:00:00 2001 From: rysweet Date: Sun, 26 Jul 2026 02:30:27 +0000 Subject: [PATCH 1/4] fix(ooda): graceful ACHIEVED completion + bounded reflection safeguard (#1025) Wire the pure done-gate-driven completion decision layer into the live OODA daemon loop so a goal whose deliverable PR is merged/green with all success criteria met stops re-reflecting, instead of spinning further validation rounds forever. - src/ooda_loop/completion.rs: pure, side-effect-free decision layer (LoopDecision, ReflectionBounds, evaluate/goal_achieved/goals_all_achieved). Introduces no new evidence source; consumes the existing CompletionVerdict. - src/operator_commands_ooda/daemon/mod.rs: read ReflectionBounds::from_env at daemon start; emit one terminal graceful-completion log line per gate-verified goal; opt-in bounded no-progress safeguard (SIMARD_OODA_MAX_REFLECTION_CYCLES) yields stuck non-perpetual goals with a recorded blocker; opt-in graceful idle stop (SIMARD_OODA_STOP_WHEN_ACHIEVED) for bounded batch hosts. - Perpetual-safe defaults: bound disabled (0) and stop-when-achieved off, so standing instances stay perpetual and no goal is spin-capped unless opted in. - Perpetual/standing goals are always exempt from the bound. Tests: 22 completion unit + 5 daemon glue + 7 integration, all green. Docs: concept, howto, reference wired into mkdocs nav. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/concepts/graceful-ooda-completion.md | 150 ++++++ .../configure-graceful-ooda-completion.md | 118 +++++ .../reference/ooda-graceful-completion-api.md | 191 +++++++ mkdocs.yml | 3 + src/ooda_loop/completion.rs | 487 ++++++++++++++++++ src/ooda_loop/mod.rs | 6 + src/operator_commands_ooda/daemon/mod.rs | 212 ++++++++ ...issue_1025_graceful_achieved_completion.rs | 139 +++++ 8 files changed, 1306 insertions(+) create mode 100644 docs/concepts/graceful-ooda-completion.md create mode 100644 docs/howto/configure-graceful-ooda-completion.md create mode 100644 docs/reference/ooda-graceful-completion-api.md create mode 100644 src/ooda_loop/completion.rs create mode 100644 tests/issue_1025_graceful_achieved_completion.rs diff --git a/docs/concepts/graceful-ooda-completion.md b/docs/concepts/graceful-ooda-completion.md new file mode 100644 index 000000000..d728b7832 --- /dev/null +++ b/docs/concepts/graceful-ooda-completion.md @@ -0,0 +1,150 @@ +--- +title: "Concept: graceful OODA completion and the bounded reflection safeguard" +description: Intended behavior for issue #1025 — once a goal's deliverable PR is verifiably green and its success criteria are met, the OODA reflection loop reaches a terminal ACHIEVED state and stops re-reflecting, with a bounded max-reflection safeguard that prevents unbounded LLM/compute spend while preserving Simard's perpetual-by-default posture. +last_updated: 2026-07-26 +review_schedule: as-needed +owner: simard +doc_type: concept +issues: ["#1025"] +related: + - ../reference/ooda-graceful-completion-api.md + - ../howto/configure-graceful-ooda-completion.md + - ./deploy-aware-done-gate.md + - ../reference/completion-evidence-gate-api.md + - ./closed-loop-outcome-verification.md + - ./perpetual-goal-no-progress-exemption.md + - ../howto/run-ooda-daemon.md +--- + +# [PLANNED - Implementation Pending] Concept: graceful OODA completion and the bounded reflection safeguard + +This document describes the intended feature behavior for issue #1025. + +Before this feature, the OODA reflection loop kept invoking the +reflect/verify-goal step even after a goal's deliverable PR was already merged +and green. The completion predicate never treated "deliverable PR green + +success criteria met" as a *terminal* state, so the loop re-reflected +indefinitely — a continuous, avoidable drain on the LLM budget and host +compute. + +Graceful completion closes that gap. When — and only when — the existing +[deploy-aware done-gate](./deploy-aware-done-gate.md) has already returned a +verified `CompletionVerdict::Complete` for a goal, the reflection layer now +recognises that goal as **ACHIEVED** and breaks its loop instead of scheduling +another reflection cycle. A separate, bounded **max-reflection safeguard** +caps how many *no-progress* reflection cycles a goal may burn before the loop +yields, so a goal that can never satisfy its gate still cannot spin forever. + +## What this feature is, and is not + +This feature is a thin **decision layer** on top of evidence that already +exists. It does **not**: + +- introduce a new evidence source or a new "done" signal; +- let a model self-report "I'm done" and have that terminate the loop; +- change the [deploy-aware done-gate](./deploy-aware-done-gate.md) clauses + (merged PR + closed issue + verified deploy for self-affecting change); or +- alter the PRD's perpetual-operation posture by default. + +It **only** consumes the gate's existing verified +[`CompletionVerdict`](../reference/completion-evidence-gate-api.md) and turns a +`Complete` verdict — together with the goal's recorded success criteria — into +a terminal loop decision. + +## The terminal predicate + +The predicate that decides whether a goal is achieved is pure and side-effect +free. It answers a single question: + +> Are **all** of this goal's success criteria satisfied by **gate-verified** +> evidence? + +A goal is `ACHIEVED` when the done-gate's verdict for the goal `is_complete()` +— i.e. its deliverable PR is merged/green, its linked issue is closed, and, for +self-affecting changes, the change is verifiably deployed. + +> **Note on "success criteria".** `ActiveGoal` has no separate +> `success_criteria` collection; a goal's criteria are its `description`, and the +> done-gate is the component that evaluates them into the `CompletionVerdict`. +> The terminal predicate therefore consumes **only** `verdict.is_complete()` and +> must **not** perform a second, independent criteria check — doing so would +> re-derive evidence outside the gate, violating the evidence-only rule below. + +If the verdict is not `Complete`, the goal is **not** achieved and the loop +continues, so the loop can never terminate on optimism — it terminates on +gate-verified evidence. + +## The three loop-control decisions + +Each reflection tick maps to exactly one decision: + +| Decision | When | Effect on the loop | +| --- | --- | --- | +| `Continue` | Goal not yet achieved and reflection budget not exhausted | Run the next reflection cycle normally | +| `GracefulComplete` | Terminal predicate holds (gate-verified achieved) | Mark goal ACHIEVED, break the loop cleanly, emit a terminal `tracing` span | +| `BoundExceeded` | Goal still not achieved after `max_reflection_cycles` **no-progress** cycles | Yield the loop with a recorded blocker; do **not** claim success | + +`GracefulComplete` is a success termination. `BoundExceeded` is a safety +termination that never fabricates completion — it stops the *spin* and surfaces +*why* the goal is still open, exactly as the +[no-progress breaker](./no-progress-root-cause-resolution.md) does. + +## The no-progress streak, and why it resets + +The safeguard counts **consecutive no-progress reflection cycles**, not total +cycles. Any cycle that produces shippable progress (a new commit, a PR state +change, a criterion newly satisfied, a blocker resolved) **resets the streak to +zero**. Only an unbroken run of purely reflective, evidence-unchanged cycles +advances the counter toward `max_reflection_cycles`. + +This distinction matters: a healthy goal that is actively moving toward its +deliverable is never penalised, no matter how many cycles it takes. Only a goal +that is genuinely stuck — reflecting without changing any evidence — trips the +bound. + +## Perpetual-by-default is preserved + +Simard is a perpetual daemon. Graceful *goal* completion must never silently +turn her into a run-once process. Therefore: + +- Graceful completion applies **per goal**, not to the daemon. When a goal is + ACHIEVED the daemon frees that goal and carries on with the rest of the goal + board and its standing research goal. +- Whether reaching all-ACHIEVED lets the *daemon loop itself* idle is gated by + `SIMARD_OODA_STOP_WHEN_ACHIEVED`, which defaults to **off**. With the default, + an all-ACHIEVED board keeps the daemon alive and steerable — consistent with + the [standing research goal never idling](./research-goal-never-idle.md). +- The [perpetual-goal no-progress exemption](./perpetual-goal-no-progress-exemption.md) + still applies: perpetual/standing goals are exempt from the `BoundExceeded` + hard-yield, because "no shippable PR yet" is their normal steady state. + +See the [configuration guide](../howto/configure-graceful-ooda-completion.md) +for the exact defaults and how to opt in to daemon-level idling. + +## Observability + +Every terminal decision is a structured `tracing` event (no `print!`/`println!` +anywhere in the new code path), carrying the goal id, the decision variant, the +no-progress streak, and — for `GracefulComplete` — the verified evidence that +satisfied the gate. Operators reading the OODA daemon log see a single, clear +"goal … ACHIEVED (gate-verified), reflection loop closed" line instead of an +unbounded stream of re-reflection ticks. + +## Why the split (predicate vs. daemon wiring) + +The terminal predicate and the reflection-bounds policy live in a pure module +with no daemon coupling, so they are unit-testable as a truth table and a +decision matrix. The daemon's `run_ooda_daemon` loop merely *consumes* the +decision. This mirrors the +[steerable-daemon rails split](./steerable-ooda-daemon.md): judgment stays in a +small, verifiable core; the daemon owns only orchestration and state mutation. + +## Acceptance behavior + +- A goal whose deliverable PR is merged/green with all success criteria met is + marked ACHIEVED and its reflection loop exits (terminal path). +- A goal whose criteria are not yet met keeps reflecting (running path). +- A stuck, non-perpetual goal yields after the configured no-progress bound with + a recorded blocker — never a false "complete". +- With defaults unchanged, the daemon stays perpetual even when the whole board + is ACHIEVED. diff --git a/docs/howto/configure-graceful-ooda-completion.md b/docs/howto/configure-graceful-ooda-completion.md new file mode 100644 index 000000000..8562edc40 --- /dev/null +++ b/docs/howto/configure-graceful-ooda-completion.md @@ -0,0 +1,118 @@ +--- +title: How to configure graceful OODA completion and the reflection bound +description: Procedure for tuning the issue #1025 graceful-completion layer — keep Simard perpetual by default, optionally let an all-ACHIEVED board idle the daemon, and cap self-inflicted no-progress reflection spin with a bounded safeguard. +last_updated: 2026-07-26 +review_schedule: as-needed +owner: simard +doc_type: howto +issues: ["#1025"] +related: + - ../concepts/graceful-ooda-completion.md + - ../reference/ooda-graceful-completion-api.md + - ./run-ooda-daemon.md + - ./diagnose-perpetual-completion-recuration.md + - ../reference/completion-evidence-gate-api.md +--- + +# How to configure graceful OODA completion and the reflection bound + +The graceful-completion layer (issue #1025) needs no configuration to be safe: +its defaults keep Simard perpetual and only stop the *reflection spin* on goals +that are already gate-verified ACHIEVED. This guide covers the two knobs you can +turn when you want different behavior. + +All settings are environment variables read at daemon start (and re-read on the +daemon's `exec()` self-reload). For a systemd deployment, set them in the +`simard-ooda.service` unit environment; for a manual run, export them before +`simard ooda`. + +## Defaults at a glance + +| Variable | Default | Meaning | +| --- | --- | --- | +| `SIMARD_OODA_STOP_WHEN_ACHIEVED` | `0` (off) | When `1`, an all-ACHIEVED goal board lets the daemon loop idle instead of staying perpetual. | +| `SIMARD_OODA_MAX_REFLECTION_CYCLES` | `0` (disabled) | Consecutive no-progress reflection cycles a **non-perpetual** goal may burn before the loop yields with a recorded blocker. `0` = no cap. | + +With both at their defaults, per-goal graceful completion is **always active** +(a goal whose deliverable PR is green with all criteria met stops re-reflecting), +the daemon **stays perpetual**, and there is **no** no-progress cap beyond the +existing no-progress breaker. + +## 1. Keep the perpetual default (recommended) + +Do nothing. A goal that reaches a gate-verified `Complete` verdict with all +success criteria satisfied is marked ACHIEVED and its reflection loop closes; +the daemon frees the goal and continues with the rest of the board and its +standing research goal. This is the behavior issue #1025 fixes — no more endless +re-reflection over an already-green deliverable — without turning Simard into a +run-once process. + +Verify from the daemon log: + +```bash +journalctl --user -u simard-ooda -f | grep -i "ACHIEVED (gate-verified)" +``` + +You should see one terminal line per completed goal, not a stream of repeated +reflection ticks. + +## 2. Let an idle board pause the daemon loop + +Only if you *want* the daemon to stop cycling once every goal is ACHIEVED (for +example, a bounded batch host rather than a standing autonomous instance): + +```bash +export SIMARD_OODA_STOP_WHEN_ACHIEVED=1 +``` + +With this set, when `goals_all_achieved` holds for the whole board the daemon +loop idles. Leave it **unset/`0`** for any standing instance — a perpetual +Simard should keep her standing research goal moving even when the delivery +board is empty. + +## 3. Cap self-inflicted no-progress spin + +To bound how long a genuinely stuck, **non-perpetual** goal may reflect without +producing shippable progress: + +```bash +export SIMARD_OODA_MAX_REFLECTION_CYCLES=25 +``` + +After 25 *consecutive* no-progress reflection cycles the loop yields that goal +with a recorded blocker (it never fabricates completion). Any cycle that makes +shippable progress resets the counter, so an actively moving goal is never +capped. Perpetual/standing goals are exempt from this bound by design. + +Choose a value comfortably above your normal cycle count for a healthy goal. +`0` disables the cap entirely. + +## 4. Apply the change + +For a systemd deployment, add the variables to the service unit and restart: + +```bash +systemctl --user set-environment SIMARD_OODA_MAX_REFLECTION_CYCLES=25 +systemctl --user restart simard-ooda.service +``` + +Confirm the daemon logged the effective policy at startup: + +```bash +journalctl --user -u simard-ooda | grep -i "reflection bound" +``` + +## Troubleshooting + +- **A green, done goal still re-reflects.** Confirm the done-gate is actually + returning `Complete` for it — graceful completion consumes that verdict and + will not fire until the merged-PR / closed-issue / deployed clauses hold. See + [the completion-evidence gate](../reference/completion-evidence-gate-api.md) + and [diagnose perpetual completion re-curation](./diagnose-perpetual-completion-recuration.md). +- **A goal yielded `BoundExceeded` too early.** Raise + `SIMARD_OODA_MAX_REFLECTION_CYCLES` or set it to `0`; check the recorded + blocker for the WHY. +- **The daemon stopped cycling unexpectedly.** Ensure + `SIMARD_OODA_STOP_WHEN_ACHIEVED` is not set to `1` on a standing instance. +- **A malformed value.** Non-numeric or unparseable settings fall back to the + safe default and log a `tracing::warn!`; the daemon never panics on bad input. diff --git a/docs/reference/ooda-graceful-completion-api.md b/docs/reference/ooda-graceful-completion-api.md new file mode 100644 index 000000000..f277ec98f --- /dev/null +++ b/docs/reference/ooda-graceful-completion-api.md @@ -0,0 +1,191 @@ +--- +title: "OODA graceful-completion API" +description: Reference for the issue #1025 terminal-completion decision layer — the pure ooda_loop::completion module (goals_all_achieved, ReflectionBounds, LoopDecision, evaluate), the no-progress streak plumbed through ooda_loop::cycle, and the run_ooda_daemon wiring that breaks the reflection loop on a gate-verified ACHIEVED goal while preserving perpetual-by-default operation. +last_updated: 2026-07-26 +review_schedule: as-needed +owner: simard +doc_type: reference +issues: ["#1025"] +related: + - ../concepts/graceful-ooda-completion.md + - ../howto/configure-graceful-ooda-completion.md + - ./completion-evidence-gate-api.md + - ./completion-gate-issue-fallback-api.md + - ./ooda-per-goal-cycle-api.md + - ./durable-ooda-cycle-counter.md +--- + +# OODA graceful-completion API + +The graceful-completion layer (issue #1025) lives in +`src/ooda_loop/completion.rs`. It is a **pure** module: every function is +side-effect free and depends only on values the OODA cycle already computed — +principally the [`CompletionVerdict`](./completion-evidence-gate-api.md) +produced by the deploy-aware done-gate. The daemon +(`src/operator_commands_ooda/daemon/mod.rs`) consumes its decision; the module +never touches the network, `gh`, or the goal store. + +> **Naming.** No symbol added by this feature contains `bridge`/`Bridge` +> (enforced by `tests/no_bridge_naming.rs`). All diagnostics use structured +> `tracing`; there is no `print!`/`println!` in this path. + +## `goals_all_achieved` + +```rust +/// Returns `true` only when every goal on the board is complete by +/// gate-verified evidence (`verdict.is_complete()`). +/// +/// Consumes the done-gate verdicts already computed this cycle. It never +/// re-derives evidence and never treats a model-reported "done" as complete. +/// (The goal's success criteria — its `description` — are already evaluated +/// inside the `CompletionVerdict`; there is no separate criteria check here.) +pub fn goals_all_achieved( + board: &GoalBoard, + verdicts: &BTreeMap, +) -> bool; +``` + +A single goal is achieved when its verdict `is_complete()`. That verdict already +encapsulates the goal's success-criteria evaluation, so the predicate adds no +second evidence source. `goals_all_achieved` is the board-level conjunction used +for the optional daemon-idle decision. The per-goal predicate is exposed as +`goal_achieved(verdict) -> bool` for the loop-break path. + +## `ReflectionBounds` + +```rust +/// Policy for the bounded no-progress safeguard. Perpetual by default. +#[derive(Clone, Debug)] +pub struct ReflectionBounds { + /// Consecutive no-progress reflection cycles a non-perpetual goal may burn + /// before `evaluate` yields `BoundExceeded`. `0` disables the bound. + pub max_reflection_cycles: u32, + + /// When `true`, an all-ACHIEVED board lets the daemon loop idle. Sourced + /// from `SIMARD_OODA_STOP_WHEN_ACHIEVED`. Defaults to `false` (perpetual). + pub stop_when_idle: bool, +} + +impl Default for ReflectionBounds { + /// Perpetual-safe defaults: `max_reflection_cycles = 0`-guarded via env + /// (see `from_env`), `stop_when_idle = false`. + fn default() -> Self { /* ... */ } +} + +impl ReflectionBounds { + /// Build from the environment. Malformed values fall back to the safe + /// default and emit a `tracing::warn!` — never a panic. + /// + /// * `SIMARD_OODA_MAX_REFLECTION_CYCLES` -> `max_reflection_cycles` + /// * `SIMARD_OODA_STOP_WHEN_ACHIEVED` -> `stop_when_idle` + pub fn from_env() -> Self; +} +``` + +## `LoopDecision` + +```rust +/// The single decision `evaluate` returns for one reflection tick. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum LoopDecision { + /// Goal not yet achieved and reflection budget not exhausted — reflect again. + Continue, + /// Terminal predicate holds (gate-verified achieved) — break the loop cleanly. + GracefulComplete, + /// Non-perpetual goal still not achieved after `max_reflection_cycles` + /// consecutive no-progress cycles — yield with a recorded blocker. + BoundExceeded, +} +``` + +## `evaluate` + +```rust +/// Map one reflection tick to a `LoopDecision`. +/// +/// Precedence: +/// 1. `GracefulComplete` if `goal_achieved(verdict)` (i.e. `verdict.is_complete()`). +/// 2. `BoundExceeded` if the goal is not perpetual, `bounds.max_reflection_cycles > 0`, +/// and `no_progress_streak >= bounds.max_reflection_cycles`. +/// 3. `Continue` otherwise. +/// +/// `goal` is used only to determine perpetual/standing status (for the +/// exemption); achievement is decided from `verdict` alone. +/// Perpetual/standing goals never receive `BoundExceeded`; they fall through to +/// `Continue`, preserving the perpetual-goal no-progress exemption. +pub fn evaluate( + goal: &ActiveGoal, + verdict: &CompletionVerdict, + no_progress_streak: u32, + bounds: &ReflectionBounds, +) -> LoopDecision; +``` + +`evaluate` checks achievement **before** the bound, so a goal that becomes +achieved on the same cycle it would have tripped the bound completes gracefully +rather than yielding. + +## No-progress streak (in `ooda_loop::cycle`) + +`src/ooda_loop/cycle.rs` threads a `no_progress_streak: u32` through the +per-cycle result: + +- The streak **increments** on a reflection cycle that produced no shippable + progress (no new commit, no PR state change, no criterion newly satisfied, no + blocker resolved). +- The streak **resets to `0`** on any cycle that produced shippable progress. + +The streak is the input to `evaluate`'s `BoundExceeded` clause. It counts +*consecutive* stalled cycles, so an actively progressing goal never approaches +the bound. The streak is surfaced on the cycle result for the daemon log and +the dashboard thinking-cycle history. + +## Daemon wiring (`run_ooda_daemon`) + +Inside the `run_ooda_daemon` loop, after the done-gate verdicts are computed for +the cycle, the daemon calls `completion::evaluate` per active goal: + +```rust +match completion::evaluate(goal, verdict, streak, &bounds) { + LoopDecision::GracefulComplete => { + // Mark ACHIEVED, free the goal, emit a terminal tracing span, continue + // with the rest of the board. NOT a daemon exit. + mark_goal_achieved(goal, verdict); + tracing::info!(goal_id = %goal.id, verdict = ?verdict, + "goal ACHIEVED (gate-verified); reflection loop closed"); + } + LoopDecision::BoundExceeded => { + // Record a blocker with the WHY; never claim completion. + record_reflection_bound_blocker(goal, streak); + } + LoopDecision::Continue => { /* normal reflection */ } +} +``` + +The existing `shutdown` and `max_cycles` break paths are unchanged. Daemon-level +idling on an all-ACHIEVED board only occurs when +`ReflectionBounds::stop_when_idle` is `true` **and** `goals_all_achieved` +returns `true`; with defaults, the daemon stays perpetual. + +## Failure and safety semantics + +- **No panics.** Malformed env values degrade to safe defaults with a + `tracing::warn!`. +- **Evidence-only completion.** `GracefulComplete` is reachable only through a + `CompletionVerdict::Complete`; there is no self-report shortcut. +- **Bounded spin.** `max_reflection_cycles` caps self-inflicted no-progress spin + for non-perpetual goals; `0` (disabled) keeps prior behavior for operators who + want no cap. +- **Perpetual preservation.** Perpetual goals and the default daemon posture are + never terminated by this layer. + +## Tests + +- `src/ooda_loop/completion.rs` inline `#[cfg(test)]`: `goals_all_achieved` + truth table, `evaluate` decision matrix (precedence, perpetual exemption, + bound-disabled), streak-reset behavior. +- `tests/issue_1025_graceful_achieved_completion.rs`: the daemon breaks a goal's + reflection loop on a gate-verified all-ACHIEVED state when + `stop_when_idle` is set, and stays perpetual by default; a criteria-unmet goal + keeps reflecting (running path); a stuck non-perpetual goal yields + `BoundExceeded` with a recorded blocker and no false completion. diff --git a/mkdocs.yml b/mkdocs.yml index e26b9d50a..a8a05e71c 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -138,6 +138,7 @@ nav: - Unified Telemetry and Status: concepts/unified-telemetry-and-status.md - Goal-Board Persistence: concepts/goal-board-persistence.md - Keeping the OODA Daemon Steerable: concepts/steerable-ooda-daemon.md + - Graceful OODA Completion + Bounded Reflection (#1025): concepts/graceful-ooda-completion.md - Perpetual Goals Exempt from No-Progress Hard-Block: concepts/perpetual-goal-no-progress-exemption.md - Novelty-First Steering for Standing Research Goals: concepts/novelty-first-standing-research-steering.md - The Standing Research Goal Never Idles (Idle = Fault): concepts/research-goal-never-idle.md @@ -192,6 +193,7 @@ nav: - Grant Engineer Write Permissions: howto/grant-engineer-write-permissions.md - Edit the Engineer System Prompt: howto/edit-the-engineer-system-prompt.md - Run the OODA Daemon: howto/run-ooda-daemon.md + - Configure Graceful OODA Completion (#1025): howto/configure-graceful-ooda-completion.md - Spawn Engineers from the OODA Daemon: howto/spawn-engineers-from-ooda-daemon.md - Diagnose a Deferred/Serialized Engineer Spawn (overlap): howto/diagnose-a-deferred-engineer-spawn.md - Configure Resource-Aware Engineer Admission: howto/configure-resource-aware-admission.md @@ -292,6 +294,7 @@ nav: - CLI Reference: reference/simard-cli.md - Simard Installer: reference/simard-installer.md - OODA Capability API: reference/ooda-capability-api.md + - OODA Graceful-Completion API (#1025): reference/ooda-graceful-completion-api.md - Engineer-Claim Release & Reclaim API: reference/engineer-claim-release-api.md - Stale-Engineer-Claim Reaper API: reference/claim-reaper-api.md - Investigate-Before-Reap API: reference/investigate-stale-engineer-api.md diff --git a/src/ooda_loop/completion.rs b/src/ooda_loop/completion.rs new file mode 100644 index 000000000..eeb682d47 --- /dev/null +++ b/src/ooda_loop/completion.rs @@ -0,0 +1,487 @@ +//! Graceful OODA completion + bounded reflection safeguard (issue #1025). +//! +//! A **pure**, side-effect-free decision layer that turns the deploy-aware +//! done-gate's already-computed [`CompletionVerdict`] into a terminal loop +//! decision. It introduces no new evidence source, never lets a model self- +//! report "done", and never touches the network, `gh`, or the goal store. +//! +//! Before this layer the reflection loop kept re-invoking reflect/verify even +//! after a goal's deliverable PR was merged and green, because the completion +//! predicate never treated "gate-verified complete" as a *terminal* state. This +//! module supplies that terminal predicate plus a bounded no-progress safeguard, +//! while preserving Simard's perpetual-by-default posture. +//! +//! See `docs/concepts/graceful-ooda-completion.md` and +//! `docs/reference/ooda-graceful-completion-api.md`. + +use std::collections::BTreeMap; + +use crate::goal_curation::{ActiveGoal, CompletionVerdict, GoalBoard}; + +/// Environment variable overriding [`ReflectionBounds::max_reflection_cycles`]. +pub const MAX_REFLECTION_CYCLES_ENV: &str = "SIMARD_OODA_MAX_REFLECTION_CYCLES"; +/// Environment variable overriding [`ReflectionBounds::stop_when_idle`]. +pub const STOP_WHEN_ACHIEVED_ENV: &str = "SIMARD_OODA_STOP_WHEN_ACHIEVED"; + +/// The single decision [`evaluate`] returns for one reflection tick. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum LoopDecision { + /// Goal not yet achieved and reflection budget not exhausted — reflect again. + Continue, + /// Terminal predicate holds (gate-verified achieved) — break the loop cleanly. + GracefulComplete, + /// Non-perpetual goal still not achieved after `max_reflection_cycles` + /// consecutive no-progress cycles — yield with a recorded blocker. + BoundExceeded, +} + +/// Policy for the bounded no-progress safeguard. Perpetual by default. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ReflectionBounds { + /// Consecutive no-progress reflection cycles a non-perpetual goal may burn + /// before [`evaluate`] yields [`LoopDecision::BoundExceeded`]. `0` disables + /// the bound (prior, uncapped behavior). + pub max_reflection_cycles: u32, + + /// When `true`, an all-ACHIEVED board lets the daemon loop idle. Sourced + /// from [`STOP_WHEN_ACHIEVED_ENV`]. Defaults to `false` (perpetual). + pub stop_when_idle: bool, +} + +impl Default for ReflectionBounds { + /// Perpetual-safe defaults: bound disabled (`0`) and `stop_when_idle = false`, + /// so the daemon stays perpetual and no goal is ever spin-capped unless an + /// operator opts in via the environment. + fn default() -> Self { + Self { + max_reflection_cycles: 0, + stop_when_idle: false, + } + } +} + +impl ReflectionBounds { + /// Build from the environment. Malformed values fall back to the safe + /// default and emit a `tracing::warn!` — never a panic. + /// + /// * [`MAX_REFLECTION_CYCLES_ENV`] -> `max_reflection_cycles` + /// * [`STOP_WHEN_ACHIEVED_ENV`] -> `stop_when_idle` + pub fn from_env() -> Self { + Self::from_env_values( + std::env::var(MAX_REFLECTION_CYCLES_ENV).ok().as_deref(), + std::env::var(STOP_WHEN_ACHIEVED_ENV).ok().as_deref(), + ) + } + + /// Pure core of [`from_env`](Self::from_env), split out so it is testable + /// without mutating process-global environment state. + /// + /// `None` (unset) yields the corresponding [`Default`] field. A present but + /// malformed value also degrades to the default and warns. + pub fn from_env_values(max_cycles: Option<&str>, stop_when_idle: Option<&str>) -> Self { + let default = Self::default(); + + let max_reflection_cycles = match max_cycles.map(str::trim) { + None | Some("") => default.max_reflection_cycles, + Some(raw) => match raw.parse::() { + Ok(n) => n, + Err(_) => { + tracing::warn!( + env = MAX_REFLECTION_CYCLES_ENV, + value = raw, + "malformed reflection-cycle bound; using default" + ); + default.max_reflection_cycles + } + }, + }; + + let stop_when_idle = match stop_when_idle.map(str::trim) { + None | Some("") => default.stop_when_idle, + Some(raw) => match parse_bool(raw) { + Some(b) => b, + None => { + tracing::warn!( + env = STOP_WHEN_ACHIEVED_ENV, + value = raw, + "malformed stop-when-achieved flag; using default" + ); + default.stop_when_idle + } + }, + }; + + Self { + max_reflection_cycles, + stop_when_idle, + } + } + + /// True when a **non-perpetual** goal has burned at least + /// `max_reflection_cycles` consecutive no-progress cycles, so the loop + /// should yield it with a recorded blocker rather than reflect forever. + /// + /// Returns `false` when the bound is disabled (`max_reflection_cycles == 0`) + /// or the goal is perpetual/standing. This is the exact predicate + /// [`evaluate`] uses for its [`LoopDecision::BoundExceeded`] arm, exposed so + /// a caller iterating goals that are already known **not** gate-complete + /// (e.g. the daemon's post-cycle board) can consult it without synthesizing + /// a [`CompletionVerdict`](crate::goal_curation::CompletionVerdict). + pub fn bound_exhausted(&self, is_perpetual: bool, no_progress_streak: u32) -> bool { + self.max_reflection_cycles > 0 + && !is_perpetual + && no_progress_streak >= self.max_reflection_cycles + } +} + +/// Lenient truthy/falsey parse for the `stop_when_idle` flag. Returns `None` +/// for anything unrecognised so the caller can warn and fall back. +fn parse_bool(raw: &str) -> Option { + match raw.trim().to_ascii_lowercase().as_str() { + "1" | "true" | "yes" | "on" => Some(true), + "0" | "false" | "no" | "off" => Some(false), + _ => None, + } +} + +/// A single goal is achieved when its done-gate verdict `is_complete()`. +/// +/// The verdict already encapsulates the goal's success-criteria (its +/// `description`) evaluation, so this predicate adds **no** second evidence +/// source and never treats a model-reported "done" as complete. +pub fn goal_achieved(verdict: &CompletionVerdict) -> bool { + verdict.is_complete() +} + +/// Returns `true` only when every active goal on the board is complete by +/// gate-verified evidence (`verdict.is_complete()`). +/// +/// A goal with no verdict this cycle is treated as **not** achieved (conservative +/// — absence of gate-verified evidence is never completion). An empty board is +/// not "all achieved": there is nothing to have achieved, so the perpetual +/// daemon has no reason to idle. +pub fn goals_all_achieved( + board: &GoalBoard, + verdicts: &BTreeMap, +) -> bool { + !board.active.is_empty() + && board + .active + .iter() + .all(|goal| verdicts.get(&goal.id).is_some_and(goal_achieved)) +} + +/// Map one reflection tick to a [`LoopDecision`]. +/// +/// Precedence: +/// 1. [`LoopDecision::GracefulComplete`] if `goal_achieved(verdict)`. +/// 2. [`LoopDecision::BoundExceeded`] if the goal is **not** perpetual, +/// `bounds.max_reflection_cycles > 0`, and +/// `no_progress_streak >= bounds.max_reflection_cycles`. +/// 3. [`LoopDecision::Continue`] otherwise. +/// +/// `goal` is used only to determine perpetual/standing status (for the +/// exemption); achievement is decided from `verdict` alone. Achievement is +/// checked **before** the bound, so a goal that becomes achieved on the same +/// cycle it would have tripped the bound completes gracefully rather than +/// yielding. +pub fn evaluate( + goal: &ActiveGoal, + verdict: &CompletionVerdict, + no_progress_streak: u32, + bounds: &ReflectionBounds, +) -> LoopDecision { + if goal_achieved(verdict) { + return LoopDecision::GracefulComplete; + } + + if bounds.bound_exhausted(goal.is_perpetual(), no_progress_streak) { + return LoopDecision::BoundExceeded; + } + + LoopDecision::Continue +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::goal_curation::CompletionEvidence; + + fn complete_verdict() -> CompletionVerdict { + CompletionVerdict::Complete(CompletionEvidence { + pr_merged: true, + issue_closed: true, + self_affecting: false, + deployed: true, + }) + } + + fn blocked_verdict() -> CompletionVerdict { + CompletionVerdict::Blocked { + evidence: CompletionEvidence { + pr_merged: false, + issue_closed: false, + self_affecting: false, + deployed: true, + }, + missing: vec![crate::goal_curation::MissingEvidence::PrNotMerged], + } + } + + fn normal_goal(id: &str) -> ActiveGoal { + ActiveGoal::new(id, format!("deliver {id}"), 100) + } + + fn perpetual_goal(id: &str) -> ActiveGoal { + ActiveGoal::new(id, format!("standing {id}"), 100).mark_standing() + } + + // --- goal_achieved / goals_all_achieved truth table ------------------- + + #[test] + fn goal_achieved_only_for_complete() { + assert!(goal_achieved(&complete_verdict())); + assert!(!goal_achieved(&blocked_verdict())); + } + + #[test] + fn goals_all_achieved_true_when_every_goal_complete() { + let mut board = GoalBoard::new(); + board.active.push(normal_goal("a")); + board.active.push(normal_goal("b")); + let mut verdicts = BTreeMap::new(); + verdicts.insert("a".to_string(), complete_verdict()); + verdicts.insert("b".to_string(), complete_verdict()); + assert!(goals_all_achieved(&board, &verdicts)); + } + + #[test] + fn goals_all_achieved_false_when_one_blocked() { + let mut board = GoalBoard::new(); + board.active.push(normal_goal("a")); + board.active.push(normal_goal("b")); + let mut verdicts = BTreeMap::new(); + verdicts.insert("a".to_string(), complete_verdict()); + verdicts.insert("b".to_string(), blocked_verdict()); + assert!(!goals_all_achieved(&board, &verdicts)); + } + + #[test] + fn goals_all_achieved_false_when_verdict_missing() { + // A goal with no gate verdict this cycle is never "achieved". + let mut board = GoalBoard::new(); + board.active.push(normal_goal("a")); + board.active.push(normal_goal("b")); + let mut verdicts = BTreeMap::new(); + verdicts.insert("a".to_string(), complete_verdict()); + assert!(!goals_all_achieved(&board, &verdicts)); + } + + #[test] + fn goals_all_achieved_false_for_empty_board() { + // Nothing achieved => no reason for the perpetual daemon to idle. + let board = GoalBoard::new(); + let verdicts = BTreeMap::new(); + assert!(!goals_all_achieved(&board, &verdicts)); + } + + // --- evaluate decision matrix ----------------------------------------- + + #[test] + fn evaluate_graceful_complete_on_verified_completion() { + let goal = normal_goal("g"); + let bounds = ReflectionBounds { + max_reflection_cycles: 3, + stop_when_idle: false, + }; + // Even with a huge streak, a gate-verified completion terminates gracefully. + assert_eq!( + evaluate(&goal, &complete_verdict(), 99, &bounds), + LoopDecision::GracefulComplete + ); + } + + #[test] + fn evaluate_continue_when_not_achieved_and_under_bound() { + let goal = normal_goal("g"); + let bounds = ReflectionBounds { + max_reflection_cycles: 3, + stop_when_idle: false, + }; + assert_eq!( + evaluate(&goal, &blocked_verdict(), 2, &bounds), + LoopDecision::Continue + ); + } + + #[test] + fn evaluate_bound_exceeded_at_threshold() { + let goal = normal_goal("g"); + let bounds = ReflectionBounds { + max_reflection_cycles: 3, + stop_when_idle: false, + }; + // streak == max => bound exceeded (>= semantics). + assert_eq!( + evaluate(&goal, &blocked_verdict(), 3, &bounds), + LoopDecision::BoundExceeded + ); + // and above the threshold too. + assert_eq!( + evaluate(&goal, &blocked_verdict(), 7, &bounds), + LoopDecision::BoundExceeded + ); + } + + #[test] + fn evaluate_achievement_takes_precedence_over_bound() { + // Achieved on the very cycle the bound would trip => GracefulComplete. + let goal = normal_goal("g"); + let bounds = ReflectionBounds { + max_reflection_cycles: 3, + stop_when_idle: false, + }; + assert_eq!( + evaluate(&goal, &complete_verdict(), 3, &bounds), + LoopDecision::GracefulComplete + ); + } + + #[test] + fn evaluate_bound_disabled_when_zero() { + let goal = normal_goal("g"); + let bounds = ReflectionBounds { + max_reflection_cycles: 0, + stop_when_idle: false, + }; + // 0 disables the bound: never yields, always continues while not achieved. + assert_eq!( + evaluate(&goal, &blocked_verdict(), 1_000, &bounds), + LoopDecision::Continue + ); + } + + #[test] + fn evaluate_perpetual_goal_is_exempt_from_bound() { + let goal = perpetual_goal("standing"); + assert!(goal.is_perpetual()); + let bounds = ReflectionBounds { + max_reflection_cycles: 3, + stop_when_idle: false, + }; + // A standing goal that never completes falls through to Continue, + // preserving the perpetual-goal no-progress exemption. + assert_eq!( + evaluate(&goal, &blocked_verdict(), 10_000, &bounds), + LoopDecision::Continue + ); + } + + #[test] + fn evaluate_perpetual_goal_can_still_complete_gracefully() { + // Exemption is only from BoundExceeded; a gate-verified perpetual goal + // still reports GracefulComplete (the daemon rolls it to a fresh cycle). + let goal = perpetual_goal("standing"); + let bounds = ReflectionBounds::default(); + assert_eq!( + evaluate(&goal, &complete_verdict(), 0, &bounds), + LoopDecision::GracefulComplete + ); + } + + // --- ReflectionBounds config ------------------------------------------ + + #[test] + fn reflection_bounds_default_is_perpetual_safe() { + let d = ReflectionBounds::default(); + assert_eq!(d.max_reflection_cycles, 0, "bound disabled by default"); + assert!(!d.stop_when_idle, "daemon perpetual by default"); + } + + #[test] + fn from_env_values_unset_yields_default() { + assert_eq!( + ReflectionBounds::from_env_values(None, None), + ReflectionBounds::default() + ); + } + + #[test] + fn from_env_values_parses_valid() { + let b = ReflectionBounds::from_env_values(Some("5"), Some("true")); + assert_eq!(b.max_reflection_cycles, 5); + assert!(b.stop_when_idle); + } + + #[test] + fn from_env_values_accepts_bool_aliases() { + for truthy in ["1", "true", "YES", "On"] { + assert!( + ReflectionBounds::from_env_values(None, Some(truthy)).stop_when_idle, + "{truthy} should parse truthy" + ); + } + for falsey in ["0", "false", "NO", "Off"] { + assert!( + !ReflectionBounds::from_env_values(None, Some(falsey)).stop_when_idle, + "{falsey} should parse falsey" + ); + } + } + + #[test] + fn from_env_values_malformed_degrades_to_default_without_panic() { + let b = ReflectionBounds::from_env_values(Some("not-a-number"), Some("maybe")); + assert_eq!(b, ReflectionBounds::default()); + } + + #[test] + fn from_env_values_blank_is_treated_as_unset() { + let b = ReflectionBounds::from_env_values(Some(" "), Some(" ")); + assert_eq!(b, ReflectionBounds::default()); + } + + // --- bound_exhausted (shared BoundExceeded predicate) ----------------- + + #[test] + fn bound_exhausted_disabled_when_cap_zero() { + let bounds = ReflectionBounds::default(); // max_reflection_cycles == 0 + assert!(!bounds.bound_exhausted(false, 100_000)); + } + + #[test] + fn bound_exhausted_exempts_perpetual_goals() { + let bounds = ReflectionBounds { + max_reflection_cycles: 3, + stop_when_idle: false, + }; + assert!(!bounds.bound_exhausted(true, 100_000)); + } + + #[test] + fn bound_exhausted_fires_at_or_above_cap_for_non_perpetual() { + let bounds = ReflectionBounds { + max_reflection_cycles: 3, + stop_when_idle: false, + }; + assert!(!bounds.bound_exhausted(false, 2), "under cap keeps going"); + assert!(bounds.bound_exhausted(false, 3), "at cap yields"); + assert!(bounds.bound_exhausted(false, 4), "over cap yields"); + } + + #[test] + fn bound_exhausted_matches_evaluate_bound_arm() { + // Single-source-of-truth: `evaluate`'s BoundExceeded arm must agree with + // the standalone predicate the daemon consults for post-cycle goals. + let bounds = ReflectionBounds { + max_reflection_cycles: 3, + stop_when_idle: false, + }; + let g = normal_goal("stuck"); + assert_eq!( + evaluate(&g, &blocked_verdict(), 3, &bounds), + LoopDecision::BoundExceeded + ); + assert!(bounds.bound_exhausted(g.is_perpetual(), 3)); + } +} diff --git a/src/ooda_loop/mod.rs b/src/ooda_loop/mod.rs index f9cf9e7fe..cde120611 100644 --- a/src/ooda_loop/mod.rs +++ b/src/ooda_loop/mod.rs @@ -8,6 +8,8 @@ pub mod adaptive_scaling; mod client_factory; mod curate; +// Issue #1025: graceful OODA completion + bounded reflection safeguard (pure). +pub mod completion; // Issue #2359 (BUG 2): per-cycle goal coverage allocator. pub mod coverage; pub mod cycle; @@ -117,5 +119,9 @@ pub fn act( crate::ooda_actions::dispatch_actions_bounded(actions, memories, state, max_concurrency) } +pub use completion::{ + LoopDecision, ReflectionBounds, evaluate as evaluate_reflection, goal_achieved, + goals_all_achieved, +}; pub use cycle::run_ooda_cycle; pub use cycle::{compose_procedure_name, derive_triggers_from_objective}; diff --git a/src/operator_commands_ooda/daemon/mod.rs b/src/operator_commands_ooda/daemon/mod.rs index 4c2b795db..348d6a5b6 100644 --- a/src/operator_commands_ooda/daemon/mod.rs +++ b/src/operator_commands_ooda/daemon/mod.rs @@ -197,6 +197,53 @@ fn fail_closed_identity_cognition(identity_name: &str) -> crate::ooda_loop::Iden } } +/// Issue #1025: active goals that have exhausted the reflection bound and must +/// be yielded with a recorded blocker, returned as `(goal_id, streak)` pairs. +/// +/// Pure and side-effect-free so the daemon's opt-in bounded-reflection safeguard +/// is unit-testable without spinning the full loop. Goals already in a terminal +/// state (`Blocked` / `Completed`) are skipped, and perpetual/standing goals are +/// exempt (delegated to [`ReflectionBounds::bound_exhausted`]). Returns an empty +/// vec when the bound is disabled (`max_reflection_cycles == 0`). +fn reflection_bound_yields( + active: &[crate::goal_curation::ActiveGoal], + tracker: &crate::goal_curation::NoProgressTracker, + bounds: &crate::ooda_loop::ReflectionBounds, +) -> Vec<(String, u32)> { + active + .iter() + .filter(|g| { + !matches!( + g.status, + crate::goal_curation::GoalProgress::Blocked(_) + | crate::goal_curation::GoalProgress::Completed + ) + }) + .filter_map(|g| { + let streak = tracker.consecutive(&g.id); + bounds + .bound_exhausted(g.is_perpetual(), streak) + .then(|| (g.id.clone(), streak)) + }) + .collect() +} + +/// Issue #1025: graceful idle-stop predicate for the daemon loop. +/// +/// Returns `true` only when the operator opted in (`stop_when_idle`), the board +/// held delivery work at cycle start (`had_active_at_cycle_start`), and it is now +/// fully drained (`active_now_empty && backlog_now_empty`). A board that started +/// empty never trips this — nothing was achieved — and with the flag unset (the +/// default) it never fires, preserving the perpetual posture. Pure and testable. +fn should_graceful_idle_stop( + stop_when_idle: bool, + had_active_at_cycle_start: bool, + active_now_empty: bool, + backlog_now_empty: bool, +) -> bool { + stop_when_idle && had_active_at_cycle_start && active_now_empty && backlog_now_empty +} + /// Run one or more OODA cycles as a daemon-style loop. /// /// Launches all memories, opens a RustyClawd session via [`SessionBuilder`] @@ -642,6 +689,23 @@ pub fn run_ooda_daemon( &format!("[simard] OODA daemon: cycle interval = {interval_secs}s"), ); + // ── Issue #1025: graceful OODA completion + bounded reflection safeguard ── + // Read the effective policy once at daemon start (re-read on the exec() + // self-reload, which re-enters this function). Perpetual-safe defaults: + // the reflection cap is disabled (0) and the daemon never idles on an + // all-ACHIEVED board. Operators opt in via SIMARD_OODA_MAX_REFLECTION_CYCLES + // (bound self-inflicted no-progress spin on non-perpetual goals) and + // SIMARD_OODA_STOP_WHEN_ACHIEVED (let a drained board pause a batch host). + let reflection_bounds = crate::ooda_loop::ReflectionBounds::from_env(); + daemon_log( + &state_root, + &format!( + "[simard] OODA daemon: reflection bound = {} consecutive no-progress cycle(s) \ + (0 = disabled); stop-when-achieved = {}", + reflection_bounds.max_reflection_cycles, reflection_bounds.stop_when_idle, + ), + ); + // --- embedded dashboard ------------------------------------------------ // Spawn the dashboard as a background tokio task so both OODA loop and // dashboard share a single process. On daemon restart (auto-reload or @@ -1437,6 +1501,19 @@ pub fn run_ooda_daemon( newly_done.join(", "), ), ); + // Issue #1025: emit one terminal graceful-completion + // line per goal. This is the "deliverable achieved" + // short-circuit — a gate-verified goal (its PR merged/ + // green with success criteria met) stops re-reflecting + // instead of spinning further validation rounds. + for gid in &newly_done { + daemon_log( + &state_root, + &format!( + "[simard] OODA graceful completion: goal {gid} ACHIEVED (gate-verified) — closing reflection loop (issue #1025)" + ), + ); + } } } @@ -1950,6 +2027,68 @@ pub fn run_ooda_daemon( } } + // ── Issue #1025: bounded reflection safeguard (opt-in) ──────────── + // With SIMARD_OODA_MAX_REFLECTION_CYCLES > 0, a NON-perpetual active + // goal that has burned that many consecutive no-progress cycles is + // yielded with a recorded blocker instead of reflecting forever. It + // never fabricates completion (a green deliverable is handled by the + // done-gate above, not here). Disabled by default (0), so perpetual + // instances are unaffected; perpetual/standing goals are always exempt. + if reflection_bounds.max_reflection_cycles > 0 { + let yields = reflection_bound_yields( + &state.active_goals.active, + &state.no_progress_tracker, + &reflection_bounds, + ); + if !yields.is_empty() { + let streak_by_id: std::collections::HashMap<&str, u32> = + yields.iter().map(|(id, s)| (id.as_str(), *s)).collect(); + for goal in state.active_goals.active.iter_mut() { + if let Some(&streak) = streak_by_id.get(goal.id.as_str()) { + goal.status = crate::goal_curation::GoalProgress::Blocked(format!( + "reflection bound exhausted: {streak} consecutive no-progress cycle(s) \ + (>= SIMARD_OODA_MAX_REFLECTION_CYCLES={}); needs human review (issue #1025)", + reflection_bounds.max_reflection_cycles, + )); + } + } + for (gid, _streak) in &yields { + daemon_log( + &state_root, + &format!( + "[simard] OODA reflection bound: goal {gid} yielded (BoundExceeded) after \ + reaching SIMARD_OODA_MAX_REFLECTION_CYCLES={} — recorded blocker for \ + human review (issue #1025)", + reflection_bounds.max_reflection_cycles, + ), + ); + } + } + } + + // ── Issue #1025: graceful idle stop (opt-in) ────────────────────── + // With SIMARD_OODA_STOP_WHEN_ACHIEVED=1 (a bounded batch host, NOT a + // standing autonomous instance) break the perpetual loop once the + // board that had delivery work at cycle start has been fully drained — + // every goal resolved (the gate-verified completions are logged above). + // Perpetual-by-default is preserved: with the flag unset (the default), + // or while any goal (including a standing research goal) remains active + // or queued, this never fires. + if should_graceful_idle_stop( + reflection_bounds.stop_when_idle, + !pre_cycle_active_ids.is_empty(), + state.active_goals.active.is_empty(), + state.active_goals.backlog.is_empty(), + ) { + daemon_log( + &state_root, + "[simard] OODA daemon: goal board idle — all delivery goals resolved \ + (gate-verified completions logged above); graceful stop \ + (SIMARD_OODA_STOP_WHEN_ACHIEVED, issue #1025)", + ); + break; + } + // Skip the inter-cycle sleep if this was the last requested cycle. if max_cycles > 0 && cycles_run >= max_cycles { continue; @@ -2381,4 +2520,77 @@ mod tests { assert_eq!(seed_cycle_count(0, tmp.path()), 42); assert_eq!(seed_cycle_count(0, tmp.path()), 42); } + + // --- Issue #1025: daemon graceful-completion glue --------------------- + + use crate::goal_curation::{ActiveGoal, GoalProgress, NoProgressTracker}; + use crate::ooda_loop::ReflectionBounds; + + fn bounds(max: u32) -> ReflectionBounds { + ReflectionBounds { + max_reflection_cycles: max, + stop_when_idle: false, + } + } + + #[test] + fn reflection_bound_yields_empty_when_disabled() { + let goals = vec![ActiveGoal::new("g", "deliver g", 100)]; + let mut tracker = NoProgressTracker::new(); + for _ in 0..50 { + tracker.record_no_action("g"); + } + assert!(reflection_bound_yields(&goals, &tracker, &bounds(0)).is_empty()); + } + + #[test] + fn reflection_bound_yields_stuck_non_perpetual_goal() { + let goals = vec![ActiveGoal::new("stuck", "deliver stuck", 100)]; + let mut tracker = NoProgressTracker::new(); + for _ in 0..3 { + tracker.record_no_action("stuck"); + } + let yields = reflection_bound_yields(&goals, &tracker, &bounds(3)); + assert_eq!(yields, vec![("stuck".to_string(), 3)]); + } + + #[test] + fn reflection_bound_exempts_perpetual_and_terminal_goals() { + let mut standing = ActiveGoal::new("research", "standing", 100).mark_standing(); + assert!(standing.is_perpetual()); + standing.status = GoalProgress::InProgress { percent: 0 }; + let mut already_blocked = ActiveGoal::new("blocked", "deliver", 100); + already_blocked.status = GoalProgress::Blocked("prior reason".to_string()); + + let goals = vec![standing, already_blocked]; + let mut tracker = NoProgressTracker::new(); + for _ in 0..99 { + tracker.record_no_action("research"); + tracker.record_no_action("blocked"); + } + assert!(reflection_bound_yields(&goals, &tracker, &bounds(3)).is_empty()); + } + + #[test] + fn reflection_bound_leaves_moving_goal_alone() { + // Streak under the cap => not yielded (an actively moving goal). + let goals = vec![ActiveGoal::new("moving", "deliver moving", 100)]; + let mut tracker = NoProgressTracker::new(); + tracker.record_no_action("moving"); // streak = 1, cap = 3 + assert!(reflection_bound_yields(&goals, &tracker, &bounds(3)).is_empty()); + } + + #[test] + fn idle_stop_requires_opt_in_and_drained_board() { + // Opted in, board had work at cycle start, now fully drained => stop. + assert!(should_graceful_idle_stop(true, true, true, true)); + // Not opted in => never stop (perpetual default). + assert!(!should_graceful_idle_stop(false, true, true, true)); + // Board started empty => nothing was achieved => never stop. + assert!(!should_graceful_idle_stop(true, false, true, true)); + // Active goal remains => still working => never stop. + assert!(!should_graceful_idle_stop(true, true, false, true)); + // Backlog remains => queued work => never stop. + assert!(!should_graceful_idle_stop(true, true, true, false)); + } } diff --git a/tests/issue_1025_graceful_achieved_completion.rs b/tests/issue_1025_graceful_achieved_completion.rs new file mode 100644 index 000000000..4ac765b60 --- /dev/null +++ b/tests/issue_1025_graceful_achieved_completion.rs @@ -0,0 +1,139 @@ +//! Integration tests for issue #1025 — graceful OODA completion + bounded +//! reflection safeguard, exercised through the crate's **public** pure API +//! (`simard::ooda_loop::completion`). +//! +//! These lock the terminal-completion contract at the public boundary the +//! daemon (`run_ooda_daemon`) consumes: +//! +//! * terminal path — a gate-verified goal yields `GracefulComplete`; +//! * running path — a criteria-unmet goal keeps `Continue`; +//! * bound path — a stuck non-perpetual goal yields `BoundExceeded`; +//! * perpetual — standing goals are exempt from the bound; +//! * board idle — `goals_all_achieved` only when every goal is verified; +//! * perpetual-default — with default bounds nothing is spin-capped. +//! +//! The daemon consumes these decisions in `run_ooda_daemon` +//! (`operator_commands_ooda::daemon`): a gate-verified goal is auto-completed +//! and logged `ACHIEVED (gate-verified)`, the opt-in `SIMARD_OODA_STOP_WHEN_ACHIEVED` +//! idles a drained board, and `SIMARD_OODA_MAX_REFLECTION_CYCLES` yields a stuck +//! non-perpetual goal with a recorded blocker. The daemon-side glue +//! (`reflection_bound_yields`, `should_graceful_idle_stop`) is unit-tested in +//! that module; this suite locks the pure decision contract it builds on. + +use std::collections::BTreeMap; + +use simard::goal_curation::{ + ActiveGoal, CompletionEvidence, CompletionVerdict, GoalBoard, MissingEvidence, +}; +use simard::ooda_loop::completion::{LoopDecision, ReflectionBounds, evaluate, goals_all_achieved}; + +fn complete() -> CompletionVerdict { + CompletionVerdict::Complete(CompletionEvidence { + pr_merged: true, + issue_closed: true, + self_affecting: false, + deployed: true, + }) +} + +fn blocked() -> CompletionVerdict { + CompletionVerdict::Blocked { + evidence: CompletionEvidence { + pr_merged: false, + issue_closed: false, + self_affecting: false, + deployed: true, + }, + missing: vec![MissingEvidence::PrNotMerged], + } +} + +fn goal(id: &str) -> ActiveGoal { + ActiveGoal::new(id, format!("deliver {id}"), 100) +} + +fn standing_goal(id: &str) -> ActiveGoal { + ActiveGoal::new(id, format!("standing {id}"), 100).mark_standing() +} + +fn bounds(max: u32, stop_when_idle: bool) -> ReflectionBounds { + ReflectionBounds { + max_reflection_cycles: max, + stop_when_idle, + } +} + +#[test] +fn terminal_path_green_pr_completes_gracefully() { + // Deliverable PR merged/green + gate verified => loop exits. + let g = goal("terminal"); + assert_eq!( + evaluate(&g, &complete(), 0, &bounds(5, false)), + LoopDecision::GracefulComplete + ); +} + +#[test] +fn running_path_criteria_unmet_keeps_reflecting() { + // Criteria not yet met, under bound => keep reflecting. + let g = goal("running"); + assert_eq!( + evaluate(&g, &blocked(), 1, &bounds(5, false)), + LoopDecision::Continue + ); +} + +#[test] +fn bound_path_stuck_non_perpetual_goal_yields() { + // Stuck non-perpetual goal past the no-progress bound => yield (no false done). + let g = goal("stuck"); + let decision = evaluate(&g, &blocked(), 5, &bounds(5, false)); + assert_eq!(decision, LoopDecision::BoundExceeded); + // Crucially, BoundExceeded is NOT a completion. + assert_ne!(decision, LoopDecision::GracefulComplete); +} + +#[test] +fn perpetual_goal_exempt_from_bound() { + let g = standing_goal("research"); + assert!(g.is_perpetual()); + assert_eq!( + evaluate(&g, &blocked(), 100_000, &bounds(5, false)), + LoopDecision::Continue + ); +} + +#[test] +fn perpetual_default_bounds_never_spin_cap() { + // Default bounds disable the cap => a non-perpetual stuck goal still Continues. + let g = goal("uncapped"); + assert_eq!( + evaluate(&g, &blocked(), 100_000, &ReflectionBounds::default()), + LoopDecision::Continue + ); +} + +#[test] +fn board_all_achieved_only_when_every_goal_verified() { + let mut board = GoalBoard::new(); + board.active.push(goal("a")); + board.active.push(goal("b")); + + let mut verdicts: BTreeMap = BTreeMap::new(); + verdicts.insert("a".to_string(), complete()); + // b not yet complete. + verdicts.insert("b".to_string(), blocked()); + assert!(!goals_all_achieved(&board, &verdicts)); + + // Now b is verified complete too. + verdicts.insert("b".to_string(), complete()); + assert!(goals_all_achieved(&board, &verdicts)); +} + +#[test] +fn from_env_defaults_are_perpetual_safe() { + // Regardless of ambient env in CI, the parsed-from-values default is perpetual. + let d = ReflectionBounds::from_env_values(None, None); + assert_eq!(d.max_reflection_cycles, 0); + assert!(!d.stop_when_idle); +} From c4d81bb75e1a0e0809d347431c5126a698272b42 Mon Sep 17 00:00:00 2001 From: rysweet Date: Sun, 26 Jul 2026 02:32:31 +0000 Subject: [PATCH 2/4] wip: checkpoint after implementation (steps 7-8) Automatic checkpoint to preserve work in progress. Tests and implementation saved before refactoring phase. --- .../manifest-derived-pin-invariants.md | 98 ++++++++++++++++ .../manifest-derived-pin-invariants.md | 107 ++++++++++++++++++ 2 files changed, 205 insertions(+) create mode 100644 docs/concepts/manifest-derived-pin-invariants.md create mode 100644 docs/reference/manifest-derived-pin-invariants.md diff --git a/docs/concepts/manifest-derived-pin-invariants.md b/docs/concepts/manifest-derived-pin-invariants.md new file mode 100644 index 000000000..643dd334e --- /dev/null +++ b/docs/concepts/manifest-derived-pin-invariants.md @@ -0,0 +1,98 @@ +--- +title: "Concept: manifest-derived pin invariants (single source of truth)" +description: Intended behavior for issue #1018 — the amplihack pin-bump acceptance test derives the expected dependency pins from the Cargo.toml / Cargo.lock manifest (the single source of truth) and asserts supply-chain invariants, instead of hardcoding a frozen SHA that the workflow-publish step-14 version bump then breaks. +last_updated: 2026-07-26 +review_schedule: as-needed +owner: simard +doc_type: concept +issues: ["#1018"] +related: + - ../reference/manifest-derived-pin-invariants.md + - ../reference/amplihack-pin-bump-2626.md + - ../howto/self-maintain-dependency-pins.md + - ./amplihack-freshness-gate.md + - ../reference/dependency-trust-policy.md + - ../reference/supply-chain-audit.md +--- + +# [PLANNED - Implementation Pending] Concept: manifest-derived pin invariants (single source of truth) + +This document describes the intended feature behavior for issue #1018. + +The dependency-pin acceptance test +(`tests/issue_2626_amplihack_pin_bump.rs`) hardcodes the **expected pin literals** +as `const … _TARGET_REV`/`_STALE_REV` constants and asserts the manifest's rev +equals them. Every time the `amplihack-*` dependencies are bumped, those frozen +constants must be manually chased in lock-step or the test turns red; and where +the test also freezes Simard's **own version**, the workflow-publish **step-14** +version bump invalidates that literal and blocks automated publish. In both +cases the root cause is the same: a copy of a fact (the pin, the version) is +frozen in the test source alongside the authoritative copy in the manifest, and +the two drift apart. + +The fix removes the conflict at its root: make the test **derive** what it +expects from the manifest — the single source of truth — rather than freezing a +copy of it in the test source. Two things that describe the same fact can no +longer disagree, because there is now only one thing. + +## Single source of truth + +`Cargo.toml` and `Cargo.lock` are the authoritative record of Simard's +dependency pins and her own version. The acceptance test must read those files +and assert **properties** of them, never re-state their contents as literals: + +- the crate's own version comes from the manifest (e.g. `env!("CARGO_PKG_VERSION")` + or a direct `Cargo.toml` read), not a hardcoded string the bump would + invalidate; +- the expected dependency pins are read from `Cargo.toml`/`Cargo.lock` and + checked for the invariants that actually matter (below), not compared against + a frozen SHA constant. + +When step-14 bumps the version, the test re-derives from the same manifest and +stays green — publish is no longer self-blocking. + +## From frozen equality to supply-chain invariants + +The test still exists to protect the same supply-chain guarantees the +[dependency-trust policy](../reference/dependency-trust-policy.md) requires; it +just expresses them as invariants over the manifest instead of equality against +a literal: + +| Invariant | What it protects | +| --- | --- | +| Every pin is a **full 40-char commit SHA** — never a branch or tag | Reproducibility; no moving target (anti-mutable-ref) | +| Each `amplihack-*` crate resolves only from its **allowlisted `rysweet/…` remote** | Anti-typosquat / anti-source-swap | +| `Cargo.toml` and `Cargo.lock` **agree** on each pin | Anti-downgrade / lockfile parity | +| Exactly **one** `lbug` engine line resolves (the lockstep) | One on-disk store format; no dual-engine link | + +None of these invariants reference a specific version string, so none is +disturbed by step-14's bump. They fail loudly only on a real supply-chain +regression (a mutable ref, a foreign remote, a lockfile mismatch, a second +engine), which is exactly when the gate should fire. + +## Deterministic and offline + +The test reads the raw `Cargo.toml` / `Cargo.lock` with `std` only — **no +network, no `git ls-remote`, no toolchain, no crate import**. An operator +running the equivalent `grep` gets the same answer CI does, the check stays +decoupled from the heavy `simard` build, and it can never flake on network +conditions. This preserves the file-shaped property the existing +`issue_2626_amplihack_pin_bump.rs` was written to have. + +## Why not just update the literal on every bump? + +Because that re-creates the bug on the next bump. Hardcoding the expected +version couples an independent, automated action (step-14 version bump) to a +manual edit of an unrelated test. Deriving from the manifest decouples them +permanently: the version bump and the pin test can no longer conflict, so +automated publish proceeds without human reconciliation. + +## Acceptance behavior + +- After a step-14 version bump, `tests/issue_2626_amplihack_pin_bump.rs` passes + without any edit to the test source — publish is unblocked. +- A pin regression (branch/tag instead of SHA, a non-`rysweet` remote, a + `Cargo.toml`↔`Cargo.lock` mismatch, or a second `lbug` line) still fails the + test loudly. +- The test remains offline and file-shaped (std-only, no network), and adds no + `print!`/`println!` or `bridge`/`Bridge` naming. diff --git a/docs/reference/manifest-derived-pin-invariants.md b/docs/reference/manifest-derived-pin-invariants.md new file mode 100644 index 000000000..6064976f5 --- /dev/null +++ b/docs/reference/manifest-derived-pin-invariants.md @@ -0,0 +1,107 @@ +--- +title: "Manifest-derived pin invariants (test API)" +description: Reference for the issue #1018 refactor of tests/issue_2626_amplihack_pin_bump.rs — deriving the expected crate version and dependency pins from Cargo.toml / Cargo.lock (single source of truth) and asserting supply-chain invariants (full-SHA pins, rysweet remote allowlist, lock parity, single lbug engine) instead of frozen-literal equality, so the workflow-publish step-14 version bump no longer blocks publish. +last_updated: 2026-07-26 +review_schedule: as-needed +owner: simard +doc_type: reference +issues: ["#1018"] +related: + - ../concepts/manifest-derived-pin-invariants.md + - ./amplihack-pin-bump-2626.md + - ../howto/self-maintain-dependency-pins.md + - ./dependency-trust-policy.md + - ./supply-chain-audit.md + - ../reference/amplihack-freshness-gate.md +--- + +# Manifest-derived pin invariants (test API) + +Issue #1018 refactors `tests/issue_2626_amplihack_pin_bump.rs` so its +expectations are **derived from the manifest** rather than frozen as literals. +The single source of truth is `Cargo.toml` / `Cargo.lock`. The test asserts +supply-chain **invariants** over that manifest, none of which reference a version +string — so the workflow-publish step-14 version bump can no longer turn the +test red and block publish. + +> **Constraints.** Std-only, offline (no network, no `git ls-remote`), file- +> shaped. No `print!`/`println!`; no `bridge`/`Bridge` naming. + +## What is derived, not hardcoded + +| Fact | Old (frozen) | New (derived from SoT) | +| --- | --- | --- | +| Simard's own version | Hardcoded string constant | `env!("CARGO_PKG_VERSION")` / parsed from `Cargo.toml` | +| Expected dependency pins | Hardcoded 40-char SHA constants | Read from `Cargo.toml`/`Cargo.lock` and checked for invariants | + +The test no longer contains a `const … _TARGET_REV: &str = "…"` that a bump must +chase. + +## Helpers (std-only manifest readers) + +```rust +/// Parse the root `Cargo.toml` into a queryable manifest (std + a toml reader, +/// no network, no cargo invocation). +fn read_cargo_toml(root: &Path) -> Manifest; + +/// Parse `Cargo.lock` into the resolved package set. +fn read_cargo_lock(root: &Path) -> Lockfile; + +/// The crate's own version, from the manifest — the SoT, never a literal. +fn own_version() -> &'static str { env!("CARGO_PKG_VERSION") } + +/// The git-rev pin recorded for a dependency in `Cargo.toml`. +fn manifest_pin(manifest: &Manifest, crate_name: &str) -> GitPin; + +/// The resolved rev for a dependency in `Cargo.lock`. +fn lock_pin(lock: &Lockfile, crate_name: &str) -> GitPin; +``` + +## Invariants asserted + +Each is a property of the manifest, independent of any version string: + +```rust +/// A pin is a full 40-char lowercase hex commit SHA — never a branch or tag. +fn assert_pin_is_full_sha(pin: &GitPin); + +/// A crate resolves only from its allowlisted rysweet/ git remote +/// (anti-typosquat / anti-source-swap). +fn assert_remote_allowlisted(pin: &GitPin, allowed: &[&str]); + +/// `Cargo.toml` and `Cargo.lock` agree on the crate's rev (anti-downgrade / +/// lockfile parity). +fn assert_toml_lock_parity(manifest: &Manifest, lock: &Lockfile, crate_name: &str); + +/// Exactly one `lbug` engine line resolves in the lockfile (the lbug lockstep — +/// one engine, one on-disk store format). +fn assert_single_lbug_engine(lock: &Lockfile); +``` + +The suite runs these for each `amplihack-*` git-pinned crate +(`amplihack-agent-eval` from `rysweet/amplihack-rs`, `amplihack-memory` from +`rysweet/amplihack-memory-lib`) plus the direct `lbug` pin. + +## Behavior across a version bump + +- **Before bump / after bump:** the test derives `own_version()` and the pins + from the current manifest each run, so a step-14 increment changes nothing the + test asserts — it stays **green**. Publish proceeds. +- **On a real regression:** a pin expressed as a branch/tag, a foreign remote, a + `Cargo.toml`↔`Cargo.lock` mismatch, or a second `lbug` engine line fails the + corresponding invariant loudly. + +## Determinism guarantees + +- Reads only local `Cargo.toml` / `Cargo.lock` via `std` — no network, no + `git ls-remote`, no cargo/toolchain invocation. +- An operator running the equivalent `grep`/`rg` over the manifest gets the same + verdict CI does. +- Decoupled from the heavy `simard` (LadybugDB) build. + +## Tests + +`tests/issue_2626_amplihack_pin_bump.rs` (refactored): the pin-invariant +assertions above, all derived from the manifest. The suite passes after a +step-14 version bump with no edit to the test source (issue #1018 acceptance), +and fails on any supply-chain pin regression. From ca89eb639485648efb7208efbdd9e14e68a3d0b0 Mon Sep 17 00:00:00 2001 From: rysweet Date: Sun, 26 Jul 2026 02:49:32 +0000 Subject: [PATCH 3/4] refactor(ooda): trim dead completion re-exports + correct #1025 reference doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 9 (refactor & simplify) over the issue #1025 graceful-completion work. - src/ooda_loop/mod.rs: narrow the completion re-export to ReflectionBounds (the only symbol the daemon consumes via the ooda_loop short path). The rest of the pure decision contract (LoopDecision, evaluate, goal_achieved, goals_all_achieved) stays reachable through ooda_loop::completion::* — how the acceptance tests already import it — shrinking an unused public surface with no behavior change. - docs/reference/ooda-graceful-completion-api.md: rewrite the Daemon wiring section to match the real code. It previously described a completion::evaluate per-goal loop calling mark_goal_achieved / record_reflection_bound_blocker helpers that do not exist; the daemon actually uses the done-gate newly_done graceful-completion log, reflection_bound_yields (delegating to the shared bound_exhausted predicate), and should_graceful_idle_stop (board-drain). Also corrects the goals_all_achieved idle claim and the Tests section. No production behavior change. Completion unit (22), daemon glue, and the #1025 integration acceptance suite (7) all green; cargo clippy --lib clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../reference/ooda-graceful-completion-api.md | 85 ++++++++++++------- src/ooda_loop/mod.rs | 8 +- 2 files changed, 56 insertions(+), 37 deletions(-) diff --git a/docs/reference/ooda-graceful-completion-api.md b/docs/reference/ooda-graceful-completion-api.md index f277ec98f..917a58875 100644 --- a/docs/reference/ooda-graceful-completion-api.md +++ b/docs/reference/ooda-graceful-completion-api.md @@ -47,9 +47,11 @@ pub fn goals_all_achieved( A single goal is achieved when its verdict `is_complete()`. That verdict already encapsulates the goal's success-criteria evaluation, so the predicate adds no -second evidence source. `goals_all_achieved` is the board-level conjunction used -for the optional daemon-idle decision. The per-goal predicate is exposed as -`goal_achieved(verdict) -> bool` for the loop-break path. +second evidence source. `goals_all_achieved` is the board-level conjunction +exposed for an all-verified check (the daemon's own idle stop instead uses a +board-drain predicate; see [Daemon wiring](#daemon-wiring-run_ooda_daemon)). The +per-goal predicate is exposed as `goal_achieved(verdict) -> bool` for the +loop-break path. ## `ReflectionBounds` @@ -142,30 +144,40 @@ the dashboard thinking-cycle history. ## Daemon wiring (`run_ooda_daemon`) -Inside the `run_ooda_daemon` loop, after the done-gate verdicts are computed for -the cycle, the daemon calls `completion::evaluate` per active goal: - -```rust -match completion::evaluate(goal, verdict, streak, &bounds) { - LoopDecision::GracefulComplete => { - // Mark ACHIEVED, free the goal, emit a terminal tracing span, continue - // with the rest of the board. NOT a daemon exit. - mark_goal_achieved(goal, verdict); - tracing::info!(goal_id = %goal.id, verdict = ?verdict, - "goal ACHIEVED (gate-verified); reflection loop closed"); - } - LoopDecision::BoundExceeded => { - // Record a blocker with the WHY; never claim completion. - record_reflection_bound_blocker(goal, streak); - } - LoopDecision::Continue => { /* normal reflection */ } -} -``` - -The existing `shutdown` and `max_cycles` break paths are unchanged. Daemon-level -idling on an all-ACHIEVED board only occurs when -`ReflectionBounds::stop_when_idle` is `true` **and** `goals_all_achieved` -returns `true`; with defaults, the daemon stays perpetual. +`run_ooda_daemon` reads `ReflectionBounds::from_env()` once at daemon start and +consumes the decision layer through three concrete mechanisms — it does **not** +call `completion::evaluate` per goal (that function is the pure, unit-tested +decision spec; the daemon reuses its shared predicate rather than re-running it): + +1. **Graceful completion.** For each goal the done-gate reports newly complete + this cycle (`newly_done`), the daemon emits one terminal line — + `OODA graceful completion: goal {id} ACHIEVED (gate-verified) — closing + reflection loop` — and moves on to the rest of the board. This is the + loop-break for a delivered goal, **not** a daemon exit. + +2. **Bounded no-progress safeguard (opt-in).** When + `max_reflection_cycles > 0`, `reflection_bound_yields(active, tracker, bounds)` + selects the non-perpetual, non-terminal goals whose consecutive no-progress + streak has reached the cap. Each is marked `GoalProgress::Blocked(..)` with the + streak in the WHY and logged for human review. It delegates the per-goal + decision to `ReflectionBounds::bound_exhausted` — the same predicate + `evaluate`'s `BoundExceeded` arm uses (locked by + `bound_exhausted_matches_evaluate_bound_arm`), so there is one source of truth + for the bound. It never fabricates completion. + +3. **Graceful idle stop (opt-in).** When `stop_when_idle` is set, + `should_graceful_idle_stop(stop_when_idle, had_active_at_cycle_start, + active_now_empty, backlog_now_empty)` breaks the perpetual loop once a board + that held delivery work at cycle start has been fully drained. A board that + started empty never trips it, and with the flag unset (the default) it never + fires. + +The existing `shutdown` and `max_cycles` break paths are unchanged. With +defaults (`max_reflection_cycles == 0`, `stop_when_idle == false`) none of the +opt-in paths fire and the daemon stays perpetual. `goals_all_achieved` is the +board-level conjunction exposed for callers (and the acceptance tests) that want +an all-verified check; the daemon's own idle stop uses the board-drain predicate +above. ## Failure and safety semantics @@ -183,9 +195,16 @@ returns `true`; with defaults, the daemon stays perpetual. - `src/ooda_loop/completion.rs` inline `#[cfg(test)]`: `goals_all_achieved` truth table, `evaluate` decision matrix (precedence, perpetual exemption, - bound-disabled), streak-reset behavior. -- `tests/issue_1025_graceful_achieved_completion.rs`: the daemon breaks a goal's - reflection loop on a gate-verified all-ACHIEVED state when - `stop_when_idle` is set, and stays perpetual by default; a criteria-unmet goal - keeps reflecting (running path); a stuck non-perpetual goal yields - `BoundExceeded` with a recorded blocker and no false completion. + bound-disabled), `ReflectionBounds::from_env_values` parsing (aliases, blank, + malformed-degrades-without-panic), and the shared `bound_exhausted` predicate + (including `bound_exhausted_matches_evaluate_bound_arm`). +- `src/operator_commands_ooda/daemon/mod.rs` inline `#[cfg(test)]`: the daemon + glue — `reflection_bound_yields` (disabled cap, stuck non-perpetual yield, + perpetual/terminal exemption, moving-goal left alone) and + `should_graceful_idle_stop` (opt-in + drained-board matrix). +- `tests/issue_1025_graceful_achieved_completion.rs`: locks the pure decision + contract at the crate's public boundary — terminal path (gate-verified goal ⇒ + `GracefulComplete`), running path (criteria unmet ⇒ `Continue`), bound path (a + stuck non-perpetual goal ⇒ `BoundExceeded`, explicitly **not** a completion), + perpetual exemption, `goals_all_achieved` only when every goal is verified, and + perpetual-safe `from_env` defaults. diff --git a/src/ooda_loop/mod.rs b/src/ooda_loop/mod.rs index cde120611..63f785aa2 100644 --- a/src/ooda_loop/mod.rs +++ b/src/ooda_loop/mod.rs @@ -119,9 +119,9 @@ pub fn act( crate::ooda_actions::dispatch_actions_bounded(actions, memories, state, max_concurrency) } -pub use completion::{ - LoopDecision, ReflectionBounds, evaluate as evaluate_reflection, goal_achieved, - goals_all_achieved, -}; +// The daemon consumes `ReflectionBounds` via this short path; the rest of the +// pure decision contract (`LoopDecision`, `evaluate`, `goal_achieved`, +// `goals_all_achieved`) is reached through `crate::ooda_loop::completion::*`. +pub use completion::ReflectionBounds; pub use cycle::run_ooda_cycle; pub use cycle::{compose_procedure_name, derive_triggers_from_objective}; From 74963328a87835d12536c5eb9ab42caa9bcbf2e8 Mon Sep 17 00:00:00 2001 From: rysweet Date: Sun, 26 Jul 2026 03:03:43 +0000 Subject: [PATCH 4/4] chore(docs): drop misplaced P1018 manifest-pin docs from #1025 branch Review-pass (step 10) finding: two `manifest-derived-pin-invariants.md` pages (concepts + reference) landed on the #1025 branch during the step 7-8 WIP checkpoint. They document the P1018 manifest-derived pin feature whose code (`tests/issue_2626_amplihack_pin_bump.rs`) is NOT on this branch, are unregistered in mkdocs nav, and per the design sequence belong to the separate P1018 PR. Shipping them here would document an absent feature. Removed to keep the #1025 PR focused; content preserved for the P1018 work. No code paths reference these files; docs_integrity and the #1025 suites stay green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../manifest-derived-pin-invariants.md | 98 ---------------- .../manifest-derived-pin-invariants.md | 107 ------------------ 2 files changed, 205 deletions(-) delete mode 100644 docs/concepts/manifest-derived-pin-invariants.md delete mode 100644 docs/reference/manifest-derived-pin-invariants.md diff --git a/docs/concepts/manifest-derived-pin-invariants.md b/docs/concepts/manifest-derived-pin-invariants.md deleted file mode 100644 index 643dd334e..000000000 --- a/docs/concepts/manifest-derived-pin-invariants.md +++ /dev/null @@ -1,98 +0,0 @@ ---- -title: "Concept: manifest-derived pin invariants (single source of truth)" -description: Intended behavior for issue #1018 — the amplihack pin-bump acceptance test derives the expected dependency pins from the Cargo.toml / Cargo.lock manifest (the single source of truth) and asserts supply-chain invariants, instead of hardcoding a frozen SHA that the workflow-publish step-14 version bump then breaks. -last_updated: 2026-07-26 -review_schedule: as-needed -owner: simard -doc_type: concept -issues: ["#1018"] -related: - - ../reference/manifest-derived-pin-invariants.md - - ../reference/amplihack-pin-bump-2626.md - - ../howto/self-maintain-dependency-pins.md - - ./amplihack-freshness-gate.md - - ../reference/dependency-trust-policy.md - - ../reference/supply-chain-audit.md ---- - -# [PLANNED - Implementation Pending] Concept: manifest-derived pin invariants (single source of truth) - -This document describes the intended feature behavior for issue #1018. - -The dependency-pin acceptance test -(`tests/issue_2626_amplihack_pin_bump.rs`) hardcodes the **expected pin literals** -as `const … _TARGET_REV`/`_STALE_REV` constants and asserts the manifest's rev -equals them. Every time the `amplihack-*` dependencies are bumped, those frozen -constants must be manually chased in lock-step or the test turns red; and where -the test also freezes Simard's **own version**, the workflow-publish **step-14** -version bump invalidates that literal and blocks automated publish. In both -cases the root cause is the same: a copy of a fact (the pin, the version) is -frozen in the test source alongside the authoritative copy in the manifest, and -the two drift apart. - -The fix removes the conflict at its root: make the test **derive** what it -expects from the manifest — the single source of truth — rather than freezing a -copy of it in the test source. Two things that describe the same fact can no -longer disagree, because there is now only one thing. - -## Single source of truth - -`Cargo.toml` and `Cargo.lock` are the authoritative record of Simard's -dependency pins and her own version. The acceptance test must read those files -and assert **properties** of them, never re-state their contents as literals: - -- the crate's own version comes from the manifest (e.g. `env!("CARGO_PKG_VERSION")` - or a direct `Cargo.toml` read), not a hardcoded string the bump would - invalidate; -- the expected dependency pins are read from `Cargo.toml`/`Cargo.lock` and - checked for the invariants that actually matter (below), not compared against - a frozen SHA constant. - -When step-14 bumps the version, the test re-derives from the same manifest and -stays green — publish is no longer self-blocking. - -## From frozen equality to supply-chain invariants - -The test still exists to protect the same supply-chain guarantees the -[dependency-trust policy](../reference/dependency-trust-policy.md) requires; it -just expresses them as invariants over the manifest instead of equality against -a literal: - -| Invariant | What it protects | -| --- | --- | -| Every pin is a **full 40-char commit SHA** — never a branch or tag | Reproducibility; no moving target (anti-mutable-ref) | -| Each `amplihack-*` crate resolves only from its **allowlisted `rysweet/…` remote** | Anti-typosquat / anti-source-swap | -| `Cargo.toml` and `Cargo.lock` **agree** on each pin | Anti-downgrade / lockfile parity | -| Exactly **one** `lbug` engine line resolves (the lockstep) | One on-disk store format; no dual-engine link | - -None of these invariants reference a specific version string, so none is -disturbed by step-14's bump. They fail loudly only on a real supply-chain -regression (a mutable ref, a foreign remote, a lockfile mismatch, a second -engine), which is exactly when the gate should fire. - -## Deterministic and offline - -The test reads the raw `Cargo.toml` / `Cargo.lock` with `std` only — **no -network, no `git ls-remote`, no toolchain, no crate import**. An operator -running the equivalent `grep` gets the same answer CI does, the check stays -decoupled from the heavy `simard` build, and it can never flake on network -conditions. This preserves the file-shaped property the existing -`issue_2626_amplihack_pin_bump.rs` was written to have. - -## Why not just update the literal on every bump? - -Because that re-creates the bug on the next bump. Hardcoding the expected -version couples an independent, automated action (step-14 version bump) to a -manual edit of an unrelated test. Deriving from the manifest decouples them -permanently: the version bump and the pin test can no longer conflict, so -automated publish proceeds without human reconciliation. - -## Acceptance behavior - -- After a step-14 version bump, `tests/issue_2626_amplihack_pin_bump.rs` passes - without any edit to the test source — publish is unblocked. -- A pin regression (branch/tag instead of SHA, a non-`rysweet` remote, a - `Cargo.toml`↔`Cargo.lock` mismatch, or a second `lbug` line) still fails the - test loudly. -- The test remains offline and file-shaped (std-only, no network), and adds no - `print!`/`println!` or `bridge`/`Bridge` naming. diff --git a/docs/reference/manifest-derived-pin-invariants.md b/docs/reference/manifest-derived-pin-invariants.md deleted file mode 100644 index 6064976f5..000000000 --- a/docs/reference/manifest-derived-pin-invariants.md +++ /dev/null @@ -1,107 +0,0 @@ ---- -title: "Manifest-derived pin invariants (test API)" -description: Reference for the issue #1018 refactor of tests/issue_2626_amplihack_pin_bump.rs — deriving the expected crate version and dependency pins from Cargo.toml / Cargo.lock (single source of truth) and asserting supply-chain invariants (full-SHA pins, rysweet remote allowlist, lock parity, single lbug engine) instead of frozen-literal equality, so the workflow-publish step-14 version bump no longer blocks publish. -last_updated: 2026-07-26 -review_schedule: as-needed -owner: simard -doc_type: reference -issues: ["#1018"] -related: - - ../concepts/manifest-derived-pin-invariants.md - - ./amplihack-pin-bump-2626.md - - ../howto/self-maintain-dependency-pins.md - - ./dependency-trust-policy.md - - ./supply-chain-audit.md - - ../reference/amplihack-freshness-gate.md ---- - -# Manifest-derived pin invariants (test API) - -Issue #1018 refactors `tests/issue_2626_amplihack_pin_bump.rs` so its -expectations are **derived from the manifest** rather than frozen as literals. -The single source of truth is `Cargo.toml` / `Cargo.lock`. The test asserts -supply-chain **invariants** over that manifest, none of which reference a version -string — so the workflow-publish step-14 version bump can no longer turn the -test red and block publish. - -> **Constraints.** Std-only, offline (no network, no `git ls-remote`), file- -> shaped. No `print!`/`println!`; no `bridge`/`Bridge` naming. - -## What is derived, not hardcoded - -| Fact | Old (frozen) | New (derived from SoT) | -| --- | --- | --- | -| Simard's own version | Hardcoded string constant | `env!("CARGO_PKG_VERSION")` / parsed from `Cargo.toml` | -| Expected dependency pins | Hardcoded 40-char SHA constants | Read from `Cargo.toml`/`Cargo.lock` and checked for invariants | - -The test no longer contains a `const … _TARGET_REV: &str = "…"` that a bump must -chase. - -## Helpers (std-only manifest readers) - -```rust -/// Parse the root `Cargo.toml` into a queryable manifest (std + a toml reader, -/// no network, no cargo invocation). -fn read_cargo_toml(root: &Path) -> Manifest; - -/// Parse `Cargo.lock` into the resolved package set. -fn read_cargo_lock(root: &Path) -> Lockfile; - -/// The crate's own version, from the manifest — the SoT, never a literal. -fn own_version() -> &'static str { env!("CARGO_PKG_VERSION") } - -/// The git-rev pin recorded for a dependency in `Cargo.toml`. -fn manifest_pin(manifest: &Manifest, crate_name: &str) -> GitPin; - -/// The resolved rev for a dependency in `Cargo.lock`. -fn lock_pin(lock: &Lockfile, crate_name: &str) -> GitPin; -``` - -## Invariants asserted - -Each is a property of the manifest, independent of any version string: - -```rust -/// A pin is a full 40-char lowercase hex commit SHA — never a branch or tag. -fn assert_pin_is_full_sha(pin: &GitPin); - -/// A crate resolves only from its allowlisted rysweet/ git remote -/// (anti-typosquat / anti-source-swap). -fn assert_remote_allowlisted(pin: &GitPin, allowed: &[&str]); - -/// `Cargo.toml` and `Cargo.lock` agree on the crate's rev (anti-downgrade / -/// lockfile parity). -fn assert_toml_lock_parity(manifest: &Manifest, lock: &Lockfile, crate_name: &str); - -/// Exactly one `lbug` engine line resolves in the lockfile (the lbug lockstep — -/// one engine, one on-disk store format). -fn assert_single_lbug_engine(lock: &Lockfile); -``` - -The suite runs these for each `amplihack-*` git-pinned crate -(`amplihack-agent-eval` from `rysweet/amplihack-rs`, `amplihack-memory` from -`rysweet/amplihack-memory-lib`) plus the direct `lbug` pin. - -## Behavior across a version bump - -- **Before bump / after bump:** the test derives `own_version()` and the pins - from the current manifest each run, so a step-14 increment changes nothing the - test asserts — it stays **green**. Publish proceeds. -- **On a real regression:** a pin expressed as a branch/tag, a foreign remote, a - `Cargo.toml`↔`Cargo.lock` mismatch, or a second `lbug` engine line fails the - corresponding invariant loudly. - -## Determinism guarantees - -- Reads only local `Cargo.toml` / `Cargo.lock` via `std` — no network, no - `git ls-remote`, no cargo/toolchain invocation. -- An operator running the equivalent `grep`/`rg` over the manifest gets the same - verdict CI does. -- Decoupled from the heavy `simard` (LadybugDB) build. - -## Tests - -`tests/issue_2626_amplihack_pin_bump.rs` (refactored): the pin-invariant -assertions above, all derived from the manifest. The suite passes after a -step-14 version bump with no edit to the test source (issue #1018 acceptance), -and fails on any supply-chain pin regression.