From a8f02725a56c02a14212306a08df5ec063f7763e Mon Sep 17 00:00:00 2001 From: rysweet Date: Tue, 28 Jul 2026 05:01:11 +0000 Subject: [PATCH 1/2] fix(ooda): declarative standing seed goals + warm-board self-heal (#4927) The live standing goal `articulate-repo-hygiene-backlog` was re-parked every OODA cycle and fed the UNCLEAR-CRITERIA issue storm (#4927/#4930/ #4934, root-caused in #4935) purely because it was never tagged perpetual: the no-progress breaker's `!is_perpetual()` exemption never fired for it. Close the gap at the seed-declaration + reconcile surface, reusing the single existing `ActiveGoal::is_perpetual()` predicate (no second notion of "perpetual"): - Add declarative `standing: bool` (default false) to `SeedGoal` with a `.standing()` builder, and to `TomlSeedGoal` via `#[serde(default)]` while KEEPING `deny_unknown_fields` (a typo'd flag still fails loud). - Carry `standing` through the identity file_loader conversion. - `seed_board_from_seed_goals` marks a `standing` seed perpetual at cold-start; ordinary seeds are pushed unchanged (no reclassification). - Add idempotent load-time `reconcile_standing_markers` that stamps the standing marker onto already-persisted goals matching a `standing` seed's normalized title slug (EXACT match only, never fuzzy prose), self-healing the live warm-board goal. Wire it into the OODA cold-start path in cycle.rs. - Refactor `mark_standing` onto a shared in-place `mark_standing_in_place`. Additive and non-breaking: ordinary goal convergence is unchanged, proven by tests. Docs: new howto + reference, linked in mkdocs nav. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/howto/declare-a-standing-seed-goal.md | 154 +++++++++ .../standing-seed-goal-declaration-api.md | 306 ++++++++++++++++++ mkdocs.yml | 2 + src/goal_curation/mod.rs | 9 +- src/goal_curation/operations.rs | 51 ++- .../tests_no_progress_breaker.rs | 95 ++++++ src/goal_curation/tests_operations.rs | 189 +++++++++++ src/goal_curation/types.rs | 13 +- src/identity/coverage_tests.rs | 82 +++++ src/identity/file_loader.rs | 5 +- src/identity/manifest.rs | 21 ++ src/identity/toml_types.rs | 6 + src/ooda_loop/cycle.rs | 19 ++ src/ooda_loop/tests_no_progress.rs | 145 +++++++++ 14 files changed, 1089 insertions(+), 8 deletions(-) create mode 100644 docs/howto/declare-a-standing-seed-goal.md create mode 100644 docs/reference/standing-seed-goal-declaration-api.md diff --git a/docs/howto/declare-a-standing-seed-goal.md b/docs/howto/declare-a-standing-seed-goal.md new file mode 100644 index 000000000..73333b252 --- /dev/null +++ b/docs/howto/declare-a-standing-seed-goal.md @@ -0,0 +1,154 @@ +--- +title: How to declare a standing (perpetual) seed goal +description: Mark a seed goal `standing = true` so it is treated as perpetual — exempt from the no-progress breaker's re-parking and issue-storm, never marked complete — and verify the live goal self-heals via reconcile_standing_markers (#4927). +last_updated: 2026-07-28 +review_schedule: as-needed +owner: simard +doc_type: howto +related: + - ../reference/standing-seed-goal-declaration-api.md + - ../concepts/perpetual-goal-no-progress-exemption.md + - ../concepts/identity-scoped-cognition.md + - ../howto/configure-pluggable-identity.md + - ../howto/diagnose-a-no-progress-breaker-issue-storm.md + - ../howto/unblock-stuck-ooda-goals.md + - ../reference/no-progress-breaker-api.md +--- + +# How to declare a standing (perpetual) seed goal + +Some goals never "finish" — they run every OODA cycle by design: repo-hygiene +backlogs, CI stewardship, continuous research. If such a goal is seeded as an +ordinary (convergence-required) goal, the **no-progress breaker** mistakes its +lack of a terminal state for a livelock, re-parks it each cycle, and files a +storm of `goal stuck after guided retry (UNCLEAR-CRITERIA)` issues (the #4927 / +#4930 / #4934 pattern). + +Declaring the goal **standing** with a single field fixes this: a standing goal +reads as `is_perpetual()` and is exempt from the breaker's re-parking and +issue-filing, and is never marked `Completed`. This guide shows how to declare +one, and how to confirm an already-running goal self-heals. + +## When to use `standing = true` + +Use it for a genuinely perpetual, non-terminating goal: + +- a repo-hygiene / stewardship backlog that re-derives work each cycle, +- a continuous CI-health or research goal. + +**Do not** use it for a bounded goal that has a definition of done — those should +keep converging and should trip the breaker if they livelock. `standing` is an +opt-in escape from the safety breaker; apply it only to goals that are perpetual +by design. + +## Option A — declare it in an identity manifest (TOML) + +Add `standing = true` to the `[[identities.seed_goals]]` entry in your +`identity.toml` (see [configure pluggable identities](./configure-pluggable-identity.md) +for the file's location and structure): + +```toml +[[identities.seed_goals]] +priority = 2 +title = "Articulate repo-hygiene backlog" +description = "Turn observations into prioritized, target-scoped repo-hygiene goals on this identity's own board." +repo = "hyenas" +standing = true # ← declares this goal perpetual +``` + +Notes: + +- The field is **optional** and defaults to `false`. Existing manifests that omit + it are unchanged — this is strictly additive. +- The manifest keeps `deny_unknown_fields`, so a typo (e.g. `standng = true`) + fails loudly at load rather than silently leaving the goal non-perpetual. If + Simard refuses to start after your edit, check the flag spelling. + +## Option B — declare it in Rust seed goals + +If you build `SeedGoal` values in code, use the `.standing()` builder: + +```rust +use crate::identity::SeedGoal; + +let goals = vec![ + SeedGoal::new( + 2, + "Articulate repo-hygiene backlog", + "Turn observations into prioritized, target-scoped repo-hygiene goals.", + Some("hyenas".into()), + ) + .standing(), // ← declares this goal perpetual +]; +``` + +The `SeedGoal::new(...)` signature is unchanged (four arguments, `standing` +defaults to `false`); `.standing()` is the opt-in. + +## What happens + +1. **Cold start (empty board).** `seed_board_from_seed_goals` applies the durable + standing marker (`[standing] `) to the goal's description as it creates the + `ActiveGoal`, so `is_perpetual()` returns `true` from the first cycle. +2. **Warm board (goal already persisted).** On every cycle, right after the board + is loaded, `reconcile_standing_markers` stamps the standing marker onto any + already-persisted goal whose **exact id or normalized title-slug** matches a + `standing` seed. This self-heals a goal that a pre-#4927 build persisted + without the marker — no need to reseed or delete the board. +3. **Effect.** From then on the goal is exempt from the no-progress breaker + (no re-parking, no `ooda-stuck` issue) and is never marked `Completed`. + +See the [standing seed-goal declaration API reference](../reference/standing-seed-goal-declaration-api.md) +for the exact types and functions. + +## Verify + +**A running goal self-heals to standing.** After deploying the declaration, watch +one cycle of the OODA daemon (see [run the OODA daemon](./run-ooda-daemon.md)). +`reconcile_standing_markers` emits a bounded structured log line (ids/slugs and a +count only) when it stamps a goal: + +```console +$ simard status --goals +p2 [not-started] [standing] Articulate repo-hygiene backlog … +``` + +The `[standing] ` prefix on the description confirms `is_perpetual()` is now +`true`. The goal stays `not-started`/active across idle cycles instead of +flipping to `blocked: 🔒 [OODA-SAFEGUARD] … needs human review`. + +**No new issue storm.** Confirm the breaker stops filing stuck-goal issues for +this goal: + +```console +$ gh issue list --repo rysweet/Simard --search "articulate-repo-hygiene UNCLEAR-CRITERIA" --state open +``` + +After the fix there should be no *new* entries for this goal. Existing issues +(#4927/#4930/#4934) are historical and are not auto-closed by this change. + +**Ordinary goals still converge.** A goal *without* `standing = true` behaves +exactly as before: it re-parks after `NO_PROGRESS_BREAKER_THRESHOLD` (3) no-action +cycles and files an issue. The declaration changes nothing for ordinary goals. + +## Troubleshooting + +- **Simard won't start after the edit.** A misspelled `standing` field is + rejected by `deny_unknown_fields`. Fix the spelling. +- **The live goal still re-parks.** Confirm the running identity manifest (not + just this repo's copy) declares `standing = true`, and that the goal's id or + title-slug **exactly** matches the seed — reconcile matches exactly, never + fuzzily. As a fallback you can force a re-seed from defaults with the + `.reseed_goals` marker (see + [unblock stuck OODA goals](./unblock-stuck-ooda-goals.md)). +- **An unexpected goal became standing.** Only goals whose exact id/slug matches a + `standing` seed are marked. Check which seed matched; remove `standing = true` + from that seed if it should converge. + +## Related + +- [Standing seed-goal declaration API reference](../reference/standing-seed-goal-declaration-api.md) +- [Standing/perpetual goals are exempt from the no-progress hard-block](../concepts/perpetual-goal-no-progress-exemption.md) +- [Diagnose a no-progress breaker issue storm](./diagnose-a-no-progress-breaker-issue-storm.md) +- [Configure pluggable identities](./configure-pluggable-identity.md) +- [Unblock OODA goals stuck after a safeguard lockout](./unblock-stuck-ooda-goals.md) diff --git a/docs/reference/standing-seed-goal-declaration-api.md b/docs/reference/standing-seed-goal-declaration-api.md new file mode 100644 index 000000000..4e126a2e7 --- /dev/null +++ b/docs/reference/standing-seed-goal-declaration-api.md @@ -0,0 +1,306 @@ +--- +title: Standing seed-goal declaration API reference +description: Reference for declaring a seed goal standing/perpetual declaratively — the `standing: bool` field on `SeedGoal` (src/identity/manifest.rs) and `TomlSeedGoal` (src/identity/toml_types.rs), the seed→ActiveGoal marker application in `seed_board_from_seed_goals`, and the idempotent load-time `reconcile_standing_markers` self-heal that stamps the standing marker onto already-persisted goals (#4927). +last_updated: 2026-07-28 +review_schedule: as-needed +owner: simard +doc_type: reference +status: implemented +related: + - ../concepts/perpetual-goal-no-progress-exemption.md + - ../concepts/identity-scoped-cognition.md + - ../concepts/pluggable-identity.md + - ./no-progress-breaker-api.md + - ./no-progress-breaker-storm-suppression-api.md + - ./standing-research-goal-novelty-directive-api.md + - ./goal-board-api.md + - ../howto/declare-a-standing-seed-goal.md + - ../howto/diagnose-a-no-progress-breaker-issue-storm.md + - ../howto/configure-pluggable-identity.md + - ../../src/identity/manifest.rs + - ../../src/identity/toml_types.rs + - ../../src/identity/file_loader.rs + - ../../src/goal_curation/operations.rs + - ../../src/goal_curation/types.rs + - ../../src/ooda_loop/cycle.rs +--- + +# Standing seed-goal declaration API reference + +> **Status: implemented.** A seed goal can be declared standing/perpetual +> **declaratively** with a single `standing = true` field. The field lives on +> [`SeedGoal`](https://github.com/rysweet/Simard/blob/main/src/identity/manifest.rs) +> and its wire twin +> [`TomlSeedGoal`](https://github.com/rysweet/Simard/blob/main/src/identity/toml_types.rs); +> it is honoured at cold-start seeding by `seed_board_from_seed_goals` and at +> warm-board load by the idempotent `reconcile_standing_markers` self-heal, both +> in [`src/goal_curation/operations.rs`](https://github.com/rysweet/Simard/blob/main/src/goal_curation/operations.rs). +> All three paths converge on the **single** existing standing predicate +> [`ActiveGoal::is_perpetual()`](https://github.com/rysweet/Simard/blob/main/src/goal_curation/types.rs) — +> there is no second notion of "perpetual." + +This reference specifies the declaration surface added in issue #4927. For the +runtime *effect* of being standing (exemption from the no-progress breaker), see +[Standing/perpetual goals are exempt from the no-progress hard-block](../concepts/perpetual-goal-no-progress-exemption.md) +and the [no-progress breaker API reference](./no-progress-breaker-api.md). + +## Why this exists (#4927) + +Before this change, the only way a goal could read as standing was for its +persisted **description** to already carry the `[standing] ` marker +(`STANDING_MARKER_PREFIX`). Seed goals had no way to *declare* that intent — so a +standing hygiene/stewardship seed such as **`Articulate repo-hygiene backlog`** +was seeded as an ordinary, convergence-required goal. Because that goal is +inherently perpetual (it re-runs every OODA cycle and never "completes"), the +no-progress breaker treated its lack of a terminal state as a livelock, re-parked +it each cycle, and filed a storm of `goal stuck after guided retry +(UNCLEAR-CRITERIA)` issues (the #4927 / #4930 / #4934 pattern, root-caused in +#4935). + +The fix closes the gap at the source: let a seed **declare** it is standing, and +make that declaration flow to the same `is_perpetual()` predicate the breaker and +completion gate already honour. Perpetual goals are then exempt from re-parking +and issue-filing; ordinary goals are entirely unchanged. + +## The declaration field + +### `SeedGoal.standing` + +`src/identity/manifest.rs` + +```rust +pub struct SeedGoal { + pub priority: u32, + pub title: String, + pub description: String, + /// Target-repo slug. `None` means the identity's own repo. + pub repo: Option, + /// When `true`, this seed is a standing/perpetual goal: it is exempt from + /// the no-progress breaker and is never marked `Completed`/tombstoned. + /// Defaults to `false` (an ordinary, convergence-required goal). + pub standing: bool, +} +``` + +- **Default:** `false`. Every existing `SeedGoal` and every seed that omits the + field remains an ordinary goal — this change is strictly additive. +- **Constructor compatibility:** `SeedGoal::new(priority, title, description, + repo)` keeps its four-argument signature and sets `standing: false`. Opt in + with the builder below. + +### `SeedGoal::standing()` builder + +```rust +impl SeedGoal { + /// Builder: declare this seed standing/perpetual. Idempotent. + #[must_use] + pub fn standing(mut self) -> Self { + self.standing = true; + self + } +} +``` + +Example: + +```rust +SeedGoal::new(2, "Articulate repo-hygiene backlog", "…", Some("hyenas".into())) + .standing(); +``` + +### `TomlSeedGoal.standing` (identity TOML wire form) + +`src/identity/toml_types.rs` + +```rust +#[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] +pub(crate) struct TomlSeedGoal { + pub priority: u32, + pub title: String, + pub description: String, + #[serde(default)] + pub repo: Option, + /// Declares this `[[identities.seed_goals]]` entry standing/perpetual. + #[serde(default)] // absent ⇒ false; keeps pre-#4927 TOML valid + pub standing: bool, +} +``` + +- `#[serde(default)]` means the field is **optional**: identity manifests written + before #4927 deserialize unchanged (`standing` becomes `false`). +- `#[serde(deny_unknown_fields)]` is **retained**: a misspelled flag + (e.g. `standng = true`) fails loud at load rather than silently leaving a + safety-critical goal non-perpetual. + +### Propagation + +`src/identity/file_loader.rs` copies the field verbatim during +`TomlSeedGoal → SeedGoal` construction — a pure field copy, no interpretation. +`SeedGoal::new` does not set `standing` (it defaults to `false`), so the declared +value is carried through by a conditional `.standing()` builder call: + +```rust +let mut seed = SeedGoal::new( + g.priority, + g.title.clone(), + g.description.clone(), + g.repo.clone(), +); +if g.standing { + seed = seed.standing(); // sets SeedGoal.standing = true +} +``` + +Equivalently, the field may be set directly on the constructed struct +(`seed.standing = g.standing;`). Either way it is a pure copy — no interpretation. + +The declared value then reaches the two honouring paths below. + +## How a declaration becomes `is_perpetual()` + +`standing` is a **declarative front door** — it does not add a parallel notion of +perpetual. It causes the existing durable **description marker** +(`STANDING_MARKER_PREFIX = "[standing] "`) to be applied so that +`ActiveGoal::is_perpetual()` returns `true`. There is exactly one source of +truth, read at runtime from the goal's description. + +### 1. Cold start — `seed_board_from_seed_goals` + +`src/goal_curation/operations.rs` + +When an empty board is seeded from `SeedGoal` values, each seed with +`standing == true` has the standing marker applied to its `ActiveGoal` before it +is inserted: + +```rust +let mut goal = ActiveGoal { + parent_goal_id: None, + priority_explicit: false, + id: crate::goals::goal_slug(&seed.title), + description: seed.description.clone(), + priority: seed.priority, + status: GoalProgress::NotStarted, + assigned_to: None, + repo: seed.repo.clone(), + current_activity: None, + wip_refs: vec![], + last_progress_update_at: None, + labels: vec![crate::goal_curation::labels::SOURCE_SEED.to_string()], +}; +if seed.standing { + goal = goal.mark_standing(); // prepends "[standing] " iff not already present +} +board.active.push(goal); +``` + +`ActiveGoal` is built by the same direct struct literal already used in +`seed_board_from_seed_goals`; the only addition is the `if seed.standing` marker +step. `mark_standing()` is idempotent, so re-seeding is safe. + +### 2. Warm board — `reconcile_standing_markers` + +`src/goal_curation/operations.rs` + +```rust +/// Stamp the standing marker onto already-persisted active goals whose exact id +/// OR normalized title-slug matches a `standing` seed. Pure, total, idempotent. +/// Returns the number of goals newly marked (0 on a second run). +pub fn reconcile_standing_markers(board: &mut GoalBoard, seeds: &[SeedGoal]) -> usize +``` + +Contract: + +| Property | Guarantee | +| --- | --- | +| **Match key** | **Exact** goal id **or** normalized title-slug equality against a `standing == true` seed. No substring / regex / fuzzy matching. | +| **Idempotent** | A goal that already reads `is_perpetual()` is skipped; a second call returns `0`. | +| **Total** | Never panics — safe over an empty board, empty seed list, and unicode/pathological titles. | +| **Additive** | Only ever *adds* the standing marker; never removes a marker, never mutates any other field, never touches non-matching goals. | +| **Observability** | Emits one bounded structured `tracing`/OpenTelemetry event carrying **ids/slugs and a count only** — never full goal descriptions. No `print!`/`println!`. | + +This is what self-heals the **live** `articulate-repo-hygiene-backlog` goal that a +pre-#4927 build already persisted without a marker: it does not require deleting +the board or setting the `.reseed_goals` marker. + +### 3. Per-cycle wiring — `ooda_loop::cycle` + +`src/ooda_loop/cycle.rs` calls `reconcile_standing_markers(&mut board, &seeds)` +immediately after the board is loaded (`load_goal_board`) and **before** the +no-progress breaker is evaluated, on every cycle. Running it per-cycle (not only +at startup) is load-bearing for the same reason as the existing self-heal: the +daemon re-reads the board from disk each cycle, so a one-time stamp would be +overwritten by the next reload. The stamp is in-memory and persisted naturally by +the next `commit_cycle`. + +## Data flow + +``` +identity TOML [[identities.seed_goals]] standing = true + │ (serde, deny_unknown_fields, default=false) + ▼ +TomlSeedGoal.standing ──file_loader──▶ SeedGoal.standing + │ + ├── cold start ─▶ seed_board_from_seed_goals ─▶ ActiveGoal.mark_standing() + │ + └── warm board ─▶ reconcile_standing_markers ─▶ ActiveGoal.mark_standing() + │ + ▼ + ActiveGoal.is_perpetual() == true + │ + ▼ + no-progress breaker EXEMPTS it (no re-park, no issue) +``` + +## Behavioural contract + +| Goal | Re-parked by no-progress breaker? | Files `ooda-stuck` issue? | Marked `Completed`/tombstoned? | +| --- | --- | --- | --- | +| `standing = true` (perpetual) | **No** — exempt | **No** | **No** — rolled to a new cycle | +| omitted / `standing = false` (ordinary) | Yes, after threshold | Yes | Yes, when the done-gate certifies it | + +The exemption is applied by the OODA driver *before* the breaker's +`resolution_for_why()` is consulted (see +[no-progress breaker API](./no-progress-breaker-api.md)); convergence thresholds +for ordinary goals are unchanged. + +## Compatibility & safety + +- **TOML round-trips** with and without `standing` (verified by test); existing + identity manifests remain valid. +- **Fail-loud misconfiguration:** `deny_unknown_fields` is preserved, so a typo'd + flag is a load-time error, never a silently non-perpetual safety goal. +- **No over-broad exemption:** exact id/slug matching only. A genuinely stuck + *ordinary* goal is never accidentally exempted — a regression test asserts an + ordinary stuck goal still re-parks and trips the breaker. +- **No `Bridge` naming**; new code uses `tracing` + OpenTelemetry, no + `print!`/`println!`. + +## Tests + +`src/goal_curation/tests_operations.rs`, +`src/goal_curation/tests_no_progress_breaker.rs`, +`src/identity/coverage_tests.rs`: + +1. A `standing = true` seed → `ActiveGoal` reads `is_perpetual()` after + `seed_board_from_seed_goals`. +2. `reconcile_standing_markers` self-heals an existing unmarked persisted goal by + exact id/slug; a second run returns `0` (idempotent) and is total over + pathological titles. +3. A perpetual/standing goal is **not** re-parked and files **no** `ooda-stuck` + issue. +4. A non-perpetual stuck goal **still** re-parks and trips the breaker (ordinary + behaviour unchanged). +5. `TomlSeedGoal` round-trips with and without `standing` under + `deny_unknown_fields`. + +## Related + +- [Standing/perpetual goals are exempt from the no-progress hard-block](../concepts/perpetual-goal-no-progress-exemption.md) + — the runtime effect this declaration opts into. +- [No-progress breaker API reference](./no-progress-breaker-api.md) and + [issue-storm suppression](./no-progress-breaker-storm-suppression-api.md). +- [Identity-scoped cognition (seed goals, observe-only Act)](../concepts/identity-scoped-cognition.md) + and [Pluggable identity](../concepts/pluggable-identity.md). +- [How-to: declare a standing seed goal](../howto/declare-a-standing-seed-goal.md). +- [How-to: diagnose a no-progress breaker issue storm](../howto/diagnose-a-no-progress-breaker-issue-storm.md). diff --git a/mkdocs.yml b/mkdocs.yml index 3d383b246..984299cdd 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -199,6 +199,7 @@ nav: - 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 - Unblock Stuck OODA Goals: howto/unblock-stuck-ooda-goals.md + - Declare a Standing (Perpetual) Seed Goal: howto/declare-a-standing-seed-goal.md - Diagnose a No-Progress Block and Read Its WHY: howto/diagnose-a-no-progress-block.md - Diagnose a No-Progress Breaker Issue Storm: howto/diagnose-a-no-progress-breaker-issue-storm.md - Re-Investigate Bare-Blocked OODA Goals: howto/reinvestigate-bare-blocked-goals.md @@ -447,6 +448,7 @@ nav: - Completion-Gate Issue-Fallback Merged-PR Recovery API: reference/completion-gate-issue-fallback-api.md - Outcome-Verification API: reference/outcome-verification-api.md - No-Progress Breaker API: reference/no-progress-breaker-api.md + - Standing Seed-Goal Declaration API: reference/standing-seed-goal-declaration-api.md - No-Progress Root-Cause Resolution API: reference/no-progress-root-cause-resolution-api.md - OODA No-Progress WHY Recipe: reference/ooda-no-progress-why-recipe.md - Durable OODA Cycle Counter API: reference/durable-ooda-cycle-counter.md diff --git a/src/goal_curation/mod.rs b/src/goal_curation/mod.rs index db1b3baba..a1329aa5b 100644 --- a/src/goal_curation/mod.rs +++ b/src/goal_curation/mod.rs @@ -29,10 +29,11 @@ pub use operations::{ BoardPlacement, DEFAULT_SEED_GOALS, DEFAULT_STEWARD_SCORE, active_goals_as_records, add_active_goal, add_backlog_item, archive_completed, board_snapshot_hash, clear_goal_assignment, default_seed_goals, load_goal_board, overwrite_memory_cache, - persist_board, promote_to_active, read_latest_carryover, record_as_active_goal, - resolve_seed_goals, rollup_parent_progress, save_goal_board, save_goal_board_with_removals, - seed_board_from_seed_goals, seed_default_board, simard_state_root, update_goal_progress, - update_goal_progress_with_evidence, verify_goal_carryover, write_goal_carryover, + persist_board, promote_to_active, read_latest_carryover, reconcile_standing_markers, + record_as_active_goal, resolve_seed_goals, rollup_parent_progress, save_goal_board, + save_goal_board_with_removals, seed_board_from_seed_goals, seed_default_board, + simard_state_root, update_goal_progress, update_goal_progress_with_evidence, + verify_goal_carryover, write_goal_carryover, }; pub use types::{ ActiveGoal, BacklogItem, CARRYOVER_CONCEPT, GoalBoard, GoalCarryoverRecord, GoalEdge, diff --git a/src/goal_curation/operations.rs b/src/goal_curation/operations.rs index 92457bb5d..aa35c5266 100644 --- a/src/goal_curation/operations.rs +++ b/src/goal_curation/operations.rs @@ -1382,7 +1382,7 @@ pub fn seed_board_from_seed_goals( for goal in goals { let id = crate::goals::goal_slug(&goal.title); - board.active.push(ActiveGoal { + let seeded = ActiveGoal { parent_goal_id: None, priority_explicit: false, id, @@ -1395,12 +1395,61 @@ pub fn seed_board_from_seed_goals( wip_refs: vec![], last_progress_update_at: None, labels: vec![crate::goal_curation::labels::SOURCE_SEED.to_string()], + }; + // A `standing = true` seed produces a perpetual goal so the no-progress + // breaker's `!is_perpetual()` exemption applies (issue #4927). Applied + // via the single standing marker so `is_perpetual()` stays the source + // of truth; an ordinary seed is pushed unchanged (no reclassification). + board.active.push(if goal.standing { + seeded.mark_standing() + } else { + seeded }); } goals.len() } +/// Warm-board self-heal for standing seed declarations (issue #4927). +/// +/// A `standing = true` seed only reaches a *cold* board via +/// [`seed_board_from_seed_goals`] (which no-ops on a non-empty board). The live +/// `articulate-repo-hygiene-backlog` goal, however, already sits on the +/// cognitive-memory board with an UNMARKED description — the exact defect that +/// re-parked it every OODA cycle and fed the `UNCLEAR-CRITERIA` issue storm +/// (#4927/#4930/#4934), because the breaker's `!is_perpetual()` exemption never +/// fired for it. +/// +/// This stamps the standing marker onto each persisted active goal whose id +/// matches a `standing` seed's normalized title slug, turning it perpetual in +/// place. Matching is EXACT (by [`crate::goals::goal_slug`]), never by fuzzy +/// prose — a non-matching or genuinely stuck goal must never be silently +/// exempted from the safety breaker. It is idempotent (an already-perpetual +/// goal is skipped, so a repeat pass heals nothing and never double-stamps) and +/// a no-op when no seed is standing. Returns the number of goals healed. +pub fn reconcile_standing_markers( + board: &mut GoalBoard, + seeds: &[crate::identity::SeedGoal], +) -> usize { + let standing_ids: std::collections::BTreeSet = seeds + .iter() + .filter(|seed| seed.standing) + .map(|seed| crate::goals::goal_slug(&seed.title)) + .collect(); + if standing_ids.is_empty() { + return 0; + } + + let mut healed = 0; + for goal in &mut board.active { + if standing_ids.contains(&goal.id) && !goal.is_perpetual() { + goal.mark_standing_in_place(); + healed += 1; + } + } + healed +} + // --------------------------------------------------------------------------- // GoalBoard -> Vec adapter // --------------------------------------------------------------------------- diff --git a/src/goal_curation/tests_no_progress_breaker.rs b/src/goal_curation/tests_no_progress_breaker.rs index b68d5035f..2d68edbaa 100644 --- a/src/goal_curation/tests_no_progress_breaker.rs +++ b/src/goal_curation/tests_no_progress_breaker.rs @@ -332,3 +332,98 @@ fn four_stuck_supply_chain_goals_all_leave_the_active_loop_via_the_ladder() { } } } + +// =========================================================================== +// Standing/perpetual exemption contract (issue #4927) +// +// The no-progress breaker exempts standing goals via the driver's +// `!is_perpetual()` filter (see `ooda_loop::no_progress`). That exemption never +// fired for the live `articulate-repo-hygiene-backlog` goal because it was +// never tagged perpetual, so it was re-parked every cycle and fed the +// `UNCLEAR-CRITERIA` issue storm. These tests pin the CONTRACT the exemption +// keys on: a goal seeded/self-healed from a `standing` seed reads as +// `is_perpetual()`, while an ordinary seed goal does not and still trips the +// bounded breaker. (`resolution_for_why` itself is deliberately unchanged — the +// exemption is applied by the driver BEFORE this ladder is consulted.) +// =========================================================================== + +#[test] +fn standing_seed_goal_reads_as_perpetual_and_is_breaker_exempt() { + use crate::goal_curation::operations::{ + reconcile_standing_markers, seed_board_from_seed_goals, + }; + use crate::goal_curation::types::GoalBoard; + + let title = "Articulate repo-hygiene backlog"; + let desc = "Turn observations into prioritized repo-hygiene goals."; + let standing = crate::identity::SeedGoal::new(2, title, desc, None).standing(); + + // Cold-start path: the seeded goal is perpetual, so the driver's + // `!is_perpetual()` breaker filter excludes it — no re-park, no issue. + let mut cold = GoalBoard::new(); + assert_eq!( + seed_board_from_seed_goals(&mut cold, std::slice::from_ref(&standing)), + 1 + ); + assert!( + cold.active[0].is_perpetual(), + "a standing seed must produce a breaker-exempt (perpetual) goal (#4927)" + ); + + // Warm-board path: an already-persisted, unmarked live goal is self-healed + // to perpetual so the exemption starts applying to it. + let id = crate::goals::goal_slug(title); + let mut live = ActiveGoal::new(id, desc, 2); + live.status = GoalProgress::NotStarted; + assert!( + !live.is_perpetual(), + "precondition: the live goal is the un-exempt #4927 defect" + ); + let mut warm = GoalBoard::new(); + warm.active.push(live); + assert_eq!( + reconcile_standing_markers(&mut warm, std::slice::from_ref(&standing)), + 1 + ); + assert!( + warm.active[0].is_perpetual(), + "reconcile must self-heal the live goal into the breaker-exempt class (#4927)" + ); +} + +#[test] +fn ordinary_seed_goal_is_not_perpetual_and_still_hits_the_breaker() { + use crate::goal_curation::operations::seed_board_from_seed_goals; + use crate::goal_curation::types::GoalBoard; + + // Regression guard: an ordinary seed goal must stay convergence-required and + // the bounded no-progress breaker must still fire for it unchanged. + let ordinary = + crate::identity::SeedGoal::new(4, "Fix broken features", "audit specs vs impl", None); + let mut board = GoalBoard::new(); + assert_eq!( + seed_board_from_seed_goals(&mut board, std::slice::from_ref(&ordinary)), + 1 + ); + let goal = &board.active[0]; + assert!( + !goal.is_perpetual(), + "an ordinary seed goal must NOT be breaker-exempt" + ); + + let id = goal.id.clone(); + let threshold = NO_PROGRESS_BREAKER_THRESHOLD; + let mut tracker = NoProgressTracker::new(); + let mut last = NoProgressResolution::Continue; + for _ in 0..threshold { + last = tracker.record_and_resolve(&id, threshold, || StuckGoalDisposition::Unresolved); + } + assert!( + last.is_terminal(), + "the breaker must still fire for a non-perpetual goal at the threshold" + ); + assert!( + matches!(last, NoProgressResolution::Escalate { .. }), + "an unresolved ordinary goal must still escalate, got {last:?}" + ); +} diff --git a/src/goal_curation/tests_operations.rs b/src/goal_curation/tests_operations.rs index 647c471d2..e4c1855d8 100644 --- a/src/goal_curation/tests_operations.rs +++ b/src/goal_curation/tests_operations.rs @@ -1685,3 +1685,192 @@ fn board_write_lock_serializes_independent_acquirers() { handle.join().expect("lock thread should join cleanly"); } + +// =========================================================================== +// Standing/perpetual seed declaration + warm-board self-heal (issue #4927) +// +// TEST-FIRST for the un-shipped `standing` seed attribute and the +// `reconcile_standing_markers` warm-board self-heal. The live standing goal +// `articulate-repo-hygiene-backlog` was re-parked every OODA cycle and fed the +// `UNCLEAR-CRITERIA` issue storm (#4927/#4930/#4934) purely because it was +// never tagged perpetual — the no-progress breaker's `!is_perpetual()` +// exemption never fired for it. The fix is entirely in the seed-declaration + +// reconcile surface: a `standing = true` seed must produce a goal that reads as +// `is_perpetual()`, and a persisted (already-live) goal matching a standing +// seed must be self-healed idempotently, by exact id / normalized title-slug +// only — never by fuzzy prose (which would wrongly exempt a genuinely stuck +// goal from the safety breaker). +// =========================================================================== + +const HYGIENE_TITLE: &str = "Articulate repo-hygiene backlog"; +const HYGIENE_DESC: &str = "Turn observations into prioritized, target-scoped repo-hygiene goals on this identity's own board."; + +fn standing_seed(title: &str, desc: &str) -> crate::identity::SeedGoal { + crate::identity::SeedGoal::new(2, title, desc, None).standing() +} + +fn ordinary_seed(title: &str, desc: &str) -> crate::identity::SeedGoal { + crate::identity::SeedGoal::new(2, title, desc, None) +} + +#[test] +fn seed_board_from_seed_goals_marks_a_standing_seed_as_perpetual() { + // Cold start: a `standing = true` seed must produce an ActiveGoal that reads + // as standing/perpetual (the single `is_perpetual()` predicate the breaker + // exemption keys on), so #4927 never recurs on a fresh/re-seeded board. + let mut board = GoalBoard::new(); + let added = + seed_board_from_seed_goals(&mut board, &[standing_seed(HYGIENE_TITLE, HYGIENE_DESC)]); + assert_eq!(added, 1); + assert_eq!(board.active.len(), 1); + assert!( + board.active[0].is_perpetual(), + "a standing=true seed must seed a perpetual goal (issue #4927)" + ); +} + +#[test] +fn seed_board_from_seed_goals_leaves_an_ordinary_seed_non_perpetual() { + // Regression guard: an ordinary (standing omitted) seed must remain a + // convergence-required goal — the fix must NOT broadly reclassify goals. + let mut board = GoalBoard::new(); + let added = seed_board_from_seed_goals( + &mut board, + &[ordinary_seed("Fix broken features", "audit specs")], + ); + assert_eq!(added, 1); + assert!( + !board.active[0].is_perpetual(), + "an ordinary seed must stay non-perpetual (no broad reclassification)" + ); +} + +#[test] +fn reconcile_standing_markers_self_heals_a_persisted_goal_by_id() { + // Warm board: the live `articulate-repo-hygiene-backlog` already sits on the + // cognitive-memory board with an UNMARKED description (the #4927 defect). A + // seed-only tag can't reach it (the empty-board guard no-ops), so a load-time + // reconcile must stamp the standing marker onto the persisted goal whose id + // matches the standing seed's slug — turning it perpetual in place. + let id = crate::goals::goal_slug(HYGIENE_TITLE); + let mut goal = ActiveGoal::new(id.clone(), HYGIENE_DESC, 2); + goal.status = GoalProgress::NotStarted; + assert!( + !goal.is_perpetual(), + "precondition: the live goal starts unmarked — the exact #4927 defect" + ); + + let mut board = GoalBoard::new(); + board.active.push(goal); + + let healed = + reconcile_standing_markers(&mut board, &[standing_seed(HYGIENE_TITLE, HYGIENE_DESC)]); + assert_eq!( + healed, 1, + "the matching persisted goal must be healed exactly once" + ); + assert!( + board.active[0].is_perpetual(), + "reconcile must make the persisted hygiene goal read as perpetual (issue #4927)" + ); +} + +#[test] +fn reconcile_standing_markers_matches_by_normalized_title_slug() { + // Matching is by the NORMALIZED slug, not a byte-exact title, so a persisted + // goal whose id came from a differently-cased/spaced title still self-heals. + let id = "articulate-repo-hygiene-backlog".to_string(); + assert_eq!( + crate::goals::goal_slug("Articulate Repo-Hygiene Backlog"), + id, + "slug normalization must collapse case/whitespace" + ); + let mut goal = ActiveGoal::new(id, HYGIENE_DESC, 2); + goal.status = GoalProgress::NotStarted; + let mut board = GoalBoard::new(); + board.active.push(goal); + + let healed = reconcile_standing_markers( + &mut board, + &[standing_seed( + "Articulate Repo-Hygiene Backlog", + HYGIENE_DESC, + )], + ); + assert_eq!(healed, 1); + assert!(board.active[0].is_perpetual()); +} + +#[test] +fn reconcile_standing_markers_is_idempotent() { + // A second reconcile pass must be a no-op (returns 0) and must not + // double-prepend the marker — `mark_standing` is idempotent by design. + let id = crate::goals::goal_slug(HYGIENE_TITLE); + let mut goal = ActiveGoal::new(id, HYGIENE_DESC, 2); + goal.status = GoalProgress::NotStarted; + let mut board = GoalBoard::new(); + board.active.push(goal); + let seeds = [standing_seed(HYGIENE_TITLE, HYGIENE_DESC)]; + + assert_eq!(reconcile_standing_markers(&mut board, &seeds), 1); + let after_first = board.active[0].description.clone(); + assert_eq!( + reconcile_standing_markers(&mut board, &seeds), + 0, + "a second reconcile must heal nothing" + ); + assert_eq!( + board.active[0].description, after_first, + "reconcile must not double-stamp the standing marker" + ); +} + +#[test] +fn reconcile_standing_markers_ignores_unmatched_goals() { + // Exact-match-only safety property: a goal whose id does NOT match any + // standing seed must never be exempted from the no-progress breaker. + let mut goal = ActiveGoal::new("some-other-goal", "unrelated work", 3); + goal.status = GoalProgress::NotStarted; + let mut board = GoalBoard::new(); + board.active.push(goal); + + let healed = + reconcile_standing_markers(&mut board, &[standing_seed(HYGIENE_TITLE, HYGIENE_DESC)]); + assert_eq!(healed, 0, "no non-matching goal may be stamped standing"); + assert!(!board.active[0].is_perpetual()); +} + +#[test] +fn reconcile_standing_markers_ignores_non_standing_seeds() { + // A seed with standing=false must NEVER stamp a matching persisted goal — + // otherwise every seed would silently become breaker-exempt. + let id = crate::goals::goal_slug(HYGIENE_TITLE); + let mut goal = ActiveGoal::new(id, HYGIENE_DESC, 2); + goal.status = GoalProgress::NotStarted; + let mut board = GoalBoard::new(); + board.active.push(goal); + + let healed = + reconcile_standing_markers(&mut board, &[ordinary_seed(HYGIENE_TITLE, HYGIENE_DESC)]); + assert_eq!( + healed, 0, + "an ordinary (standing=false) seed must heal nothing" + ); + assert!(!board.active[0].is_perpetual()); +} + +#[test] +fn reconcile_standing_markers_skips_already_perpetual_goals() { + // A goal already reading as perpetual must not be re-counted or re-stamped. + let id = crate::goals::goal_slug(HYGIENE_TITLE); + let goal = ActiveGoal::new(id, HYGIENE_DESC, 2).mark_standing(); + assert!(goal.is_perpetual()); + let before = goal.description.clone(); + let mut board = GoalBoard::new(); + board.active.push(goal); + + let healed = + reconcile_standing_markers(&mut board, &[standing_seed(HYGIENE_TITLE, HYGIENE_DESC)]); + assert_eq!(healed, 0, "an already-perpetual goal is not healed again"); + assert_eq!(board.active[0].description, before); +} diff --git a/src/goal_curation/types.rs b/src/goal_curation/types.rs index aa335ab0d..0bd4d3979 100644 --- a/src/goal_curation/types.rs +++ b/src/goal_curation/types.rs @@ -348,10 +348,21 @@ impl ActiveGoal { /// `simard goal add --standing`. #[must_use] pub fn mark_standing(mut self) -> Self { + self.mark_standing_in_place(); + self + } + + /// In-place variant of [`mark_standing`] for reconciling an + /// already-persisted goal on a warm board without moving it out of the + /// board vector (issue #4927). Idempotent — a goal that already reads as + /// standing is left byte-for-byte unchanged, so a repeated reconcile never + /// double-stamps the marker. + /// + /// [`mark_standing`]: ActiveGoal::mark_standing + pub fn mark_standing_in_place(&mut self) { if !self.is_perpetual() { self.description = format!("{STANDING_MARKER_PREFIX}{}", self.description); } - self } /// Roll a standing/perpetual goal into a fresh cycle after its current unit diff --git a/src/identity/coverage_tests.rs b/src/identity/coverage_tests.rs index 8b15922c4..8df2530f5 100644 --- a/src/identity/coverage_tests.rs +++ b/src/identity/coverage_tests.rs @@ -627,3 +627,85 @@ fn identity_load_request_stores_all_fields() { assert_eq!(req.package_version, "2.0.0"); assert_eq!(req.contract, contract); } + +// =========================================================================== +// Standing seed-goal declaration (issue #4927) +// +// TEST-FIRST for the un-shipped declarative `standing` attribute on seed goals. +// `SeedGoal` gains a `standing: bool` (default false) plus a `.standing()` +// builder, and `TomlSeedGoal` gains `#[serde(default)] standing: bool` while +// KEEPING `#[serde(deny_unknown_fields)]` so pre-existing identity TOML stays +// valid and a typo'd flag still fails loud (never silently non-perpetual). +// =========================================================================== + +#[test] +fn seed_goal_standing_defaults_false() { + // The additive field must default false so existing constructors and every + // existing seed goal are unchanged (convergence-required, as today). + let g = SeedGoal::new(1, "ordinary", "do a bounded thing", None); + assert!(!g.standing, "a plain SeedGoal must default to non-standing"); +} + +#[test] +fn seed_goal_standing_builder_marks_standing() { + let g = SeedGoal::new( + 2, + "Articulate repo-hygiene backlog", + "turn observations into goals", + None, + ) + .standing(); + assert!( + g.standing, + "the .standing() builder must set the declarative flag" + ); + // The builder is purely declarative — it does not touch the description. + assert_eq!(g.description, "turn observations into goals"); +} + +#[test] +fn toml_seed_goal_deserializes_standing_true() { + let toml = r#" +priority = 2 +title = "Articulate repo-hygiene backlog" +description = "Turn observations into prioritized repo-hygiene goals." +repo = "hyenas" +standing = true +"#; + let seed: super::toml_types::TomlSeedGoal = + toml::from_str(toml).expect("standing=true seed must deserialize"); + assert!(seed.standing, "standing = true must round-trip from TOML"); +} + +#[test] +fn toml_seed_goal_standing_defaults_false_when_omitted() { + // Back-compat: every existing seed_goals entry omits `standing` and must + // continue to parse, as a non-standing goal. + let toml = r#" +priority = 1 +title = "Observe hyenas repo health" +description = "OBSERVE ONLY" +repo = "hyenas" +"#; + let seed: super::toml_types::TomlSeedGoal = + toml::from_str(toml).expect("seed without standing must still parse"); + assert!(!seed.standing, "omitted standing must default to false"); +} + +#[test] +fn toml_seed_goal_preserves_deny_unknown_fields() { + // The `standing` addition must not weaken the deny_unknown_fields guard: a + // typo'd flag must fail loud rather than silently leave a safety goal + // non-perpetual. + let toml = r#" +priority = 1 +title = "t" +description = "d" +standng = true +"#; + let parsed = toml::from_str::(toml); + assert!( + parsed.is_err(), + "a misspelled flag must be rejected by deny_unknown_fields" + ); +} diff --git a/src/identity/file_loader.rs b/src/identity/file_loader.rs index 630871fb0..b101c814f 100644 --- a/src/identity/file_loader.rs +++ b/src/identity/file_loader.rs @@ -75,12 +75,13 @@ impl FileIdentityLoader { .seed_goals .iter() .map(|g| { - SeedGoal::new( + let seed = SeedGoal::new( g.priority, g.title.clone(), g.description.clone(), g.repo.clone(), - ) + ); + if g.standing { seed.standing() } else { seed } }) .collect(); let target_repos = identity.target_repos.clone(); diff --git a/src/identity/manifest.rs b/src/identity/manifest.rs index 6a79f73f2..d748c7479 100644 --- a/src/identity/manifest.rs +++ b/src/identity/manifest.rs @@ -24,6 +24,14 @@ pub struct SeedGoal { /// Target-repo slug. `None` means the identity's own repo; a slug scopes the /// goal to an ecosystem/target repo, exactly like `ActiveGoal.repo`. pub repo: Option, + /// Declares this a standing/perpetual goal (issue #4927). A standing seed + /// produces a goal that reads as + /// [`crate::goal_curation::ActiveGoal::is_perpetual`], so the no-progress + /// breaker's `!is_perpetual()` exemption applies and the goal is never + /// re-parked or issue-filed for lack of convergence. Additive and + /// defaulting `false`, so every existing seed goal stays + /// convergence-required exactly as before. + pub standing: bool, } impl SeedGoal { @@ -38,8 +46,21 @@ impl SeedGoal { title: title.into(), description: description.into(), repo, + standing: false, } } + + /// Builder: declare this seed a standing/perpetual goal (issue #4927). + /// Purely declarative — it flips the flag and never touches the + /// description; the standing marker is applied later at the seed→ + /// [`crate::goal_curation::ActiveGoal`] conversion so + /// [`crate::goal_curation::ActiveGoal::is_perpetual`] stays the single + /// source of truth. + #[must_use] + pub fn standing(mut self) -> Self { + self.standing = true; + self + } } /// The write-authority posture of an identity — the read-only switch that the diff --git a/src/identity/toml_types.rs b/src/identity/toml_types.rs index 84bfd6ad4..b98dff7a4 100644 --- a/src/identity/toml_types.rs +++ b/src/identity/toml_types.rs @@ -66,6 +66,12 @@ pub(crate) struct TomlSeedGoal { pub description: String, #[serde(default)] pub repo: Option, + /// Declares a standing/perpetual seed goal (issue #4927). `#[serde(default)]` + /// keeps every existing `seed_goals` entry (which omits it) valid as a + /// non-standing goal, while `deny_unknown_fields` still fails loud on a + /// typo'd flag rather than silently leaving a safety goal non-perpetual. + #[serde(default)] + pub standing: bool, } /// An optional `[identities.authority]` table (#3125 / #3067). The read-only diff --git a/src/ooda_loop/cycle.rs b/src/ooda_loop/cycle.rs index 64c896a3f..4d62f6e39 100644 --- a/src/ooda_loop/cycle.rs +++ b/src/ooda_loop/cycle.rs @@ -156,6 +156,25 @@ fn run_ooda_cycle_inner( } } + // #4927: self-heal already-persisted goals that a `standing` seed declares + // perpetual. Cold seeding (above) marks fresh goals, but a warm board loaded + // from cognitive memory carries the live goal with an UNMARKED description, + // so the no-progress breaker's `!is_perpetual()` exemption never fired for + // it — the goal was re-parked and issue-filed every cycle. Reconciling here + // against the SAME resolved seed set stamps the standing marker onto the + // matching persisted goal in place. Idempotent and a no-op when no seed is + // standing, so Simard's default board is unaffected. + { + let resolved = crate::goal_curation::resolve_seed_goals(identity_seed_goals); + let healed = + crate::goal_curation::reconcile_standing_markers(&mut state.active_goals, &resolved); + if healed > 0 { + eprintln!( + "[simard] OODA start: reconciled {healed} persisted goal(s) to standing/perpetual (#4927)" + ); + } + } + // Ingest meeting handoff decisions as new goals. let handoff_dir = crate::meeting_facilitator::default_handoff_dir(); match check_meeting_handoffs( diff --git a/src/ooda_loop/tests_no_progress.rs b/src/ooda_loop/tests_no_progress.rs index b519cd338..4999e0346 100644 --- a/src/ooda_loop/tests_no_progress.rs +++ b/src/ooda_loop/tests_no_progress.rs @@ -1275,3 +1275,148 @@ fn prune_never_touches_non_pr_refs() { "issue/branch refs must pass through the PR-liveness reconcile untouched" ); } + +// =========================================================================== +// #4927 end-to-end: a self-healed standing hygiene goal is breaker-exempt +// +// Reproduction + fix of the recurring-goal-reblock incident. The live +// `articulate-repo-hygiene-backlog` goal sat on the cognitive-memory board with +// an UNMARKED description, so the driver's `!is_perpetual()` exemption never +// applied: it was re-parked and issue-filed every OODA cycle (#4927/#4930/#4934). +// Once the standing seed declares it and `reconcile_standing_markers` self-heals +// the persisted goal to perpetual, driving the breaker N+1 consecutive +// no-action cycles must NEVER block it, escalate it, or file a tracking issue — +// it is a benign perpetual idle. A companion test proves the SAME goal, left +// unmarked (pre-fix), still escalates — so the fix is exactly the standing tag. +// =========================================================================== + +fn hygiene_goal_unmarked() -> ActiveGoal { + let title = "Articulate repo-hygiene backlog"; + let id = crate::goals::goal_slug(title); + let mut g = ActiveGoal::new( + id, + "Turn observations into prioritized repo-hygiene goals.", + 2, + ); + g.status = GoalProgress::NotStarted; + assert!( + !g.is_perpetual(), + "the pre-fix live goal must be unmarked (the #4927 defect)" + ); + g +} + +#[test] +fn reconciled_standing_hygiene_goal_is_exempt_from_the_no_progress_breaker() { + let threshold = NO_PROGRESS_BREAKER_THRESHOLD; + let goal = hygiene_goal_unmarked(); + let id = goal.id.clone(); + + // Self-heal the persisted goal via the standing seed (the #4927 fix). + let mut board = GoalBoard::new(); + board.active.push(goal); + let standing = crate::identity::SeedGoal::new( + 2, + "Articulate repo-hygiene backlog", + "Turn observations into prioritized repo-hygiene goals.", + None, + ) + .standing(); + let healed = crate::goal_curation::reconcile_standing_markers(&mut board, &[standing]); + assert_eq!( + healed, 1, + "reconcile must self-heal the one matching live goal" + ); + assert!( + board.active[0].is_perpetual(), + "post-reconcile the hygiene goal must read as perpetual (#4927)" + ); + + let mut state = OodaState::new(board); + let evidence = FakeEvidence { + pr_merged: false, + issue_closed: false, + deployed: false, + }; + let filer = RecordingFiler::default(); + + // N+1 consecutive no-action cycles — one past where a normal goal is parked. + for cycle in 1..=(threshold + 1) { + let report = apply_no_progress_breaker_with_threshold( + &mut state, + &[no_action_outcome(&id)], + &evidence, + &filer, + threshold, + ); + assert!( + !report.fired(), + "cycle {cycle}: a reconciled standing goal must not fire the breaker (#4927)" + ); + assert!( + report.escalated.is_empty(), + "cycle {cycle}: a reconciled standing goal must never be escalated" + ); + assert_eq!( + report.perpetual_idled, + vec![id.clone()], + "cycle {cycle}: the idle must be recorded as a benign perpetual idle" + ); + assert!( + !matches!( + state.active_goals.active[0].status, + GoalProgress::Blocked(_) + ), + "cycle {cycle}: a reconciled standing goal must never be Blocked" + ); + } + + assert!( + filer.calls.borrow().is_empty(), + "a reconciled standing goal must never file an [OODA-SAFEGUARD] tracking issue (#4927)" + ); +} + +#[test] +fn unmarked_hygiene_goal_still_escalates_proving_the_tag_is_the_fix() { + // Control: the identical hygiene goal, left UNMARKED (no standing seed / + // reconcile), reproduces the pre-fix #4927 behaviour — the breaker fires, + // the goal is escalated with the sentinel, and exactly one issue is filed. + // This proves the exemption keys precisely on the standing tag. + let threshold = NO_PROGRESS_BREAKER_THRESHOLD; + let goal = hygiene_goal_unmarked(); + let id = goal.id.clone(); + let mut state = state_with(goal); + let evidence = FakeEvidence { + pr_merged: false, + issue_closed: false, + deployed: false, + }; + let filer = RecordingFiler::default(); + + let mut fired = false; + for _ in 1..=(threshold + 1) { + let report = apply_no_progress_breaker_with_threshold( + &mut state, + &[no_action_outcome(&id)], + &evidence, + &filer, + threshold, + ); + assert!( + report.perpetual_idled.is_empty(), + "an unmarked goal must never be treated as a perpetual idle" + ); + if report.fired() { + fired = true; + } + } + assert!( + fired, + "an unmarked hygiene goal must still trip the no-progress breaker" + ); + assert!( + !filer.calls.borrow().is_empty(), + "the pre-fix unmarked goal must still file exactly the escalation issue" + ); +} From 43c8554ec80032a9c74998f4f304f12b4d8cfdbc Mon Sep 17 00:00:00 2001 From: Ryan Sweet Date: Wed, 2 Sep 2026 12:45:08 +0000 Subject: [PATCH 2/2] fix(ooda): make standing seed declarations reversible Resolve seed goals once per cycle and remove standing markers only for exact matching source seeds that explicitly declare standing=false. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/howto/declare-a-standing-seed-goal.md | 51 +++- .../standing-seed-goal-declaration-api.md | 182 ++++++++++----- src/goal_curation/mod.rs | 12 +- src/goal_curation/operations.rs | 100 ++++++-- .../tests_no_progress_breaker.rs | 2 +- src/goal_curation/tests_operations.rs | 220 ++++++++++++++++-- src/goal_curation/types.rs | 57 +++++ src/identity/coverage_tests.rs | 74 +++++- src/identity/file_loader.rs | 55 ++++- src/identity/manifest.rs | 50 +++- src/identity/toml_types.rs | 14 +- src/ooda_loop/cycle.rs | 39 ++-- src/ooda_loop/tests_no_progress.rs | 97 +++++++- 13 files changed, 832 insertions(+), 121 deletions(-) diff --git a/docs/howto/declare-a-standing-seed-goal.md b/docs/howto/declare-a-standing-seed-goal.md index 73333b252..6b29fd4cf 100644 --- a/docs/howto/declare-a-standing-seed-goal.md +++ b/docs/howto/declare-a-standing-seed-goal.md @@ -93,11 +93,54 @@ defaults to `false`); `.standing()` is the opt-in. 2. **Warm board (goal already persisted).** On every cycle, right after the board is loaded, `reconcile_standing_markers` stamps the standing marker onto any already-persisted goal whose **exact id or normalized title-slug** matches a - `standing` seed. This self-heals a goal that a pre-#4927 build persisted - without the marker — no need to reseed or delete the board. + `standing = true` seed. This self-heals a goal that a pre-#4927 build persisted + without the marker — no need to reseed or delete the board. The seed set is + resolved **once per cycle** and reused for both cold seeding and this + reconcile, so the two paths always agree. 3. **Effect.** From then on the goal is exempt from the no-progress breaker (no re-parking, no `ooda-stuck` issue) and is never marked `Completed`. +## Reverting a standing declaration + +The declaration is **reversible** without wiping the board, but reversal must be +**explicit**. Set the seed's flag to `standing = false` (in Rust, use the +`.non_standing()` builder) — do **not** just delete the seed or drop the flag: + +```toml +[[identities.seed_goals]] +priority = 2 +title = "Articulate repo-hygiene backlog" +description = "…" +standing = false # ← explicit reversal (must be present) +``` + +```rust +// Rust: the explicit-false builder — NOT merely omitting `.standing()`. +SeedGoal::new(2, "Articulate repo-hygiene backlog", "…", Some("hyenas".into())) + .non_standing(); +``` + +On the next cycle `reconcile_standing_markers` strips the leading `[standing] ` +marker it previously added and the goal converges (and trips the breaker) again. +The reversal is deliberately conservative: + +- It removes **only a leading `[standing] ` marker**, and **only** from a goal + carrying the exact `source:seed` label — i.e. one this seeding path created. A + user-created goal that merely shares the slug is never demoted. +- **Only an *explicit* `standing = false` reverses.** An **omitted** flag is + inert: a seed that simply leaves `standing` out (the default from + `SeedGoal::new`) never strips a marker. This is the three-state distinction — + omitted, explicit true, and explicit false are all preserved distinctly, so an + ordinary non-standing seed can never accidentally demote a perpetual goal. +- **Deleting a seed does not reverse anything.** A removed seed leaves its goal + untouched (so an accidental manifest edit can't silently re-arm a safety + breaker on a goal you meant to keep perpetual). Reversal happens only when the + seed is still present *and* carries an explicit `standing = false`. +- **Standing *phrases* in the prose are never edited.** If a goal's description + independently reads as standing (e.g. it literally contains "standing goal"), + stripping the leading marker leaves it perpetual — only the sentinel prefix is + ever removed. + See the [standing seed-goal declaration API reference](../reference/standing-seed-goal-declaration-api.md) for the exact types and functions. @@ -105,8 +148,8 @@ for the exact types and functions. **A running goal self-heals to standing.** After deploying the declaration, watch one cycle of the OODA daemon (see [run the OODA daemon](./run-ooda-daemon.md)). -`reconcile_standing_markers` emits a bounded structured log line (ids/slugs and a -count only) when it stamps a goal: +The cycle logs a bounded line with the reconcile counts (added/removed only) when +it stamps or reverses a goal: ```console $ simard status --goals diff --git a/docs/reference/standing-seed-goal-declaration-api.md b/docs/reference/standing-seed-goal-declaration-api.md index 4e126a2e7..62b4d9874 100644 --- a/docs/reference/standing-seed-goal-declaration-api.md +++ b/docs/reference/standing-seed-goal-declaration-api.md @@ -33,9 +33,12 @@ related: > and its wire twin > [`TomlSeedGoal`](https://github.com/rysweet/Simard/blob/main/src/identity/toml_types.rs); > it is honoured at cold-start seeding by `seed_board_from_seed_goals` and at -> warm-board load by the idempotent `reconcile_standing_markers` self-heal, both +> warm-board load by the idempotent, **reversible** `reconcile_standing_markers` +> reconcile, both > in [`src/goal_curation/operations.rs`](https://github.com/rysweet/Simard/blob/main/src/goal_curation/operations.rs). -> All three paths converge on the **single** existing standing predicate +> An explicit `standing = false` conservatively reverses a marker the reconcile +> itself added (see the warm-board reconcile section below). +> All paths converge on the **single** existing standing predicate > [`ActiveGoal::is_perpetual()`](https://github.com/rysweet/Simard/blob/main/src/goal_curation/types.rs) — > there is no second notion of "perpetual." @@ -77,35 +80,72 @@ pub struct SeedGoal { pub repo: Option, /// When `true`, this seed is a standing/perpetual goal: it is exempt from /// the no-progress breaker and is never marked `Completed`/tombstoned. - /// Defaults to `false` (an ordinary, convergence-required goal). + /// Defaults to `false` (an ordinary, convergence-required goal). This is the + /// single boolean the cold-seed path keys off — `false` (whether omitted or + /// explicit) cold-seeds an ordinary goal. pub standing: bool, + /// Provenance bit distinguishing an **omitted/default** non-standing seed + /// from an **explicit** `standing = false`. `pub(crate)` — read via the + /// `authorizes_standing_reversal()` accessor, never directly. Only an + /// explicit false authorizes the warm-board reconcile to *reverse* a marker. + pub(crate) standing_explicit: bool, } ``` -- **Default:** `false`. Every existing `SeedGoal` and every seed that omits the - field remains an ordinary goal — this change is strictly additive. +- **Default:** `false`, and *omitted* (`standing_explicit == false`). Every + existing `SeedGoal` and every seed that omits the field remains an ordinary + goal that is also inert with respect to reversal — this change is strictly + additive. +- **Three states, all preserved:** *omitted* (`new`, inert), *explicit true* + (`.standing()`, adds a marker), and *explicit false* (`.non_standing()`, + authorizes reversal). Cold seeding treats omitted and explicit-false + identically (both ordinary); they differ only in the warm-board reconcile. - **Constructor compatibility:** `SeedGoal::new(priority, title, description, - repo)` keeps its four-argument signature and sets `standing: false`. Opt in - with the builder below. + repo)` keeps its four-argument signature and sets `standing: false` with + `standing_explicit: false` (omitted). Opt in with the builders below. -### `SeedGoal::standing()` builder +### `SeedGoal::standing()` / `SeedGoal::non_standing()` builders ```rust impl SeedGoal { - /// Builder: declare this seed standing/perpetual. Idempotent. + /// Builder: declare this seed standing/perpetual. Idempotent. Records the + /// declaration as explicit, but a `true` declaration never reverses. #[must_use] pub fn standing(mut self) -> Self { self.standing = true; + self.standing_explicit = true; self } + + /// Builder: declare this seed *explicitly* non-standing. Stays non-standing + /// (cold-seeds like an omitted seed) but authorizes the reconcile to reverse + /// a marker it previously added to the matching `source:seed` goal. + #[must_use] + pub fn non_standing(mut self) -> Self { + self.standing = false; + self.standing_explicit = true; + self + } + + /// Whether this seed authorizes a *reversal* — true **only** for an explicit + /// `standing = false` (never for an omitted seed, never for `standing = true`). + #[must_use] + pub fn authorizes_standing_reversal(&self) -> bool { + self.standing_explicit && !self.standing + } } ``` Example: ```rust +// perpetual SeedGoal::new(2, "Articulate repo-hygiene backlog", "…", Some("hyenas".into())) .standing(); + +// explicit reversal of a previously-standing seed (NOT merely omitting .standing()) +SeedGoal::new(2, "Articulate repo-hygiene backlog", "…", Some("hyenas".into())) + .non_standing(); ``` ### `TomlSeedGoal.standing` (identity TOML wire form) @@ -122,39 +162,41 @@ pub(crate) struct TomlSeedGoal { #[serde(default)] pub repo: Option, /// Declares this `[[identities.seed_goals]]` entry standing/perpetual. - #[serde(default)] // absent ⇒ false; keeps pre-#4927 TOML valid - pub standing: bool, + /// `Option` so an omitted flag (`None`) is preserved as distinct from + /// an explicit `standing = false` (`Some(false)`) — only the latter reverses. + #[serde(default)] // absent ⇒ None; keeps pre-#4927 TOML valid + pub standing: Option, } ``` - `#[serde(default)]` means the field is **optional**: identity manifests written - before #4927 deserialize unchanged (`standing` becomes `false`). + before #4927 deserialize unchanged (`standing` becomes `None`, i.e. omitted). +- `Option` preserves the three states: `None` (omitted, inert), + `Some(true)` (perpetual), `Some(false)` (explicit reversal). - `#[serde(deny_unknown_fields)]` is **retained**: a misspelled flag (e.g. `standng = true`) fails loud at load rather than silently leaving a safety-critical goal non-perpetual. ### Propagation -`src/identity/file_loader.rs` copies the field verbatim during -`TomlSeedGoal → SeedGoal` construction — a pure field copy, no interpretation. -`SeedGoal::new` does not set `standing` (it defaults to `false`), so the declared -value is carried through by a conditional `.standing()` builder call: +`src/identity/file_loader.rs` maps the wire form during `TomlSeedGoal → SeedGoal` +construction, preserving all three states — no interpretation beyond the direct +mapping: ```rust -let mut seed = SeedGoal::new( +let seed = SeedGoal::new( g.priority, g.title.clone(), g.description.clone(), g.repo.clone(), ); -if g.standing { - seed = seed.standing(); // sets SeedGoal.standing = true +match g.standing { + Some(true) => seed.standing(), // perpetual + Some(false) => seed.non_standing(), // explicit reversal + None => seed, // omitted / inert } ``` -Equivalently, the field may be set directly on the constructed struct -(`seed.standing = g.standing;`). Either way it is a pure copy — no interpretation. - The declared value then reaches the two honouring paths below. ## How a declaration becomes `is_perpetual()` @@ -203,53 +245,74 @@ step. `mark_standing()` is idempotent, so re-seeding is safe. `src/goal_curation/operations.rs` ```rust -/// Stamp the standing marker onto already-persisted active goals whose exact id -/// OR normalized title-slug matches a `standing` seed. Pure, total, idempotent. -/// Returns the number of goals newly marked (0 on a second run). -pub fn reconcile_standing_markers(board: &mut GoalBoard, seeds: &[SeedGoal]) -> usize +/// Reconcile persisted active goals against the resolved seed set. An exact id +/// or normalized title-slug match to a `standing = true` seed stamps the +/// standing marker; a match to an *explicit* `standing = false` seed reverses a +/// leading marker on a `source:seed` goal. Pure, total, idempotent. +pub fn reconcile_standing_markers( + board: &mut GoalBoard, + seeds: &[SeedGoal], +) -> StandingReconciliation + +/// Add/remove tallies for one reconcile pass (both zero on a settled board). +pub struct StandingReconciliation { pub added: usize, pub removed: usize } ``` Contract: | Property | Guarantee | | --- | --- | -| **Match key** | **Exact** goal id **or** normalized title-slug equality against a `standing == true` seed. No substring / regex / fuzzy matching. | -| **Idempotent** | A goal that already reads `is_perpetual()` is skipped; a second call returns `0`. | +| **Match key** | **Exact** goal id **or** normalized title-slug equality against a *present* seed. No substring / regex / fuzzy matching. | +| **Add** | A `standing = true` seed stamps `STANDING_MARKER_PREFIX` onto a matching goal that is not already `is_perpetual()`. A `true` declaration always wins over a `false` one for the same slug. | +| **Reverse (explicit false only)** | A `standing = false` seed strips **only** a leading `STANDING_MARKER_PREFIX`, and **only** from a matching goal carrying the exact `source:seed` label — i.e. one this seeding path created. It never demotes a user-created goal, never edits a standing *phrase* in the prose, and a goal whose prose independently reads perpetual stays perpetual after the leading marker is removed. | +| **No reversal on seed absence** | Deleting a seed entirely (its slug no longer present) leaves its goal untouched; only an *explicit* `standing = false` reverses. This keeps board edits intentional and auditable. | +| **Idempotent** | A goal already reading `is_perpetual()` is skipped by the add path; a stripped goal is skipped by the reverse path; a second call is a no-op (`StandingReconciliation::is_noop()`). | | **Total** | Never panics — safe over an empty board, empty seed list, and unicode/pathological titles. | -| **Additive** | Only ever *adds* the standing marker; never removes a marker, never mutates any other field, never touches non-matching goals. | -| **Observability** | Emits one bounded structured `tracing`/OpenTelemetry event carrying **ids/slugs and a count only** — never full goal descriptions. No `print!`/`println!`. | +| **Observability** | The OODA cycle logs one bounded line carrying the **added/removed counts only** — never full goal descriptions. | This is what self-heals the **live** `articulate-repo-hygiene-backlog` goal that a pre-#4927 build already persisted without a marker: it does not require deleting -the board or setting the `.reseed_goals` marker. +the board or setting the `.reseed_goals` marker. The same surface makes the +declaration **reversible** — flipping the seed back to `standing = false` strips +the marker the reconciler itself added, without a board wipe. ### 3. Per-cycle wiring — `ooda_loop::cycle` -`src/ooda_loop/cycle.rs` calls `reconcile_standing_markers(&mut board, &seeds)` -immediately after the board is loaded (`load_goal_board`) and **before** the -no-progress breaker is evaluated, on every cycle. Running it per-cycle (not only -at startup) is load-bearing for the same reason as the existing self-heal: the -daemon re-reads the board from disk each cycle, so a one-time stamp would be -overwritten by the next reload. The stamp is in-memory and persisted naturally by -the next `commit_cycle`. +`src/ooda_loop/cycle.rs` resolves the seed set **once** per cycle +(`resolve_seed_goals`, identity override or baked-in defaults) and reuses that +single `Vec` for both cold seeding (`seed_board_from_seed_goals`) and this warm +reconcile — there is no second `resolve_seed_goals` call. It calls +`reconcile_standing_markers(&mut board, &resolved)` immediately after the board is +loaded (`load_goal_board`) and **before** the no-progress breaker is evaluated, +on every cycle. Running it per-cycle (not only at startup) is load-bearing for the +same reason as the existing self-heal: the daemon re-reads the board from disk +each cycle, so a one-time stamp would be overwritten by the next reload. The +stamp is in-memory and persisted naturally by the next `commit_cycle`. ## Data flow ``` -identity TOML [[identities.seed_goals]] standing = true - │ (serde, deny_unknown_fields, default=false) +identity TOML [[identities.seed_goals]] standing = true | false | (omitted) + │ (serde Option, deny_unknown_fields, absent ⇒ None) ▼ -TomlSeedGoal.standing ──file_loader──▶ SeedGoal.standing - │ +TomlSeedGoal.standing: Option ──file_loader──▶ SeedGoal (standing + explicit) + │ None⇒new (inert) Some(true)⇒.standing() Some(false)⇒.non_standing() + │ (resolved ONCE per cycle, reused below) ├── cold start ─▶ seed_board_from_seed_goals ─▶ ActiveGoal.mark_standing() + │ (only standing==true; omitted & explicit-false are ordinary) │ - └── warm board ─▶ reconcile_standing_markers ─▶ ActiveGoal.mark_standing() + └── warm board ─▶ reconcile_standing_markers + ├── standing=true ─▶ ActiveGoal.mark_standing_in_place() + ├── explicit false ─▶ ActiveGoal.unmark_standing_in_place() + │ (source:seed goal, leading marker only) + └── omitted / absent ─▶ (inert — never reverses) │ ▼ - ActiveGoal.is_perpetual() == true + ActiveGoal.is_perpetual() == true|false │ ▼ - no-progress breaker EXEMPTS it (no re-park, no issue) + no-progress breaker EXEMPTS perpetual goals + (no re-park, no issue); reversed goals converge again ``` ## Behavioural contract @@ -258,6 +321,7 @@ TomlSeedGoal.standing ──file_loader──▶ SeedGoal.standing | --- | --- | --- | --- | | `standing = true` (perpetual) | **No** — exempt | **No** | **No** — rolled to a new cycle | | omitted / `standing = false` (ordinary) | Yes, after threshold | Yes | Yes, when the done-gate certifies it | +| reverted via *explicit* `standing = false` on a `source:seed` goal | Yes again — exemption dropped | Yes | Yes | The exemption is applied by the OODA driver *before* the breaker's `resolution_for_why()` is consulted (see @@ -273,25 +337,37 @@ for ordinary goals are unchanged. - **No over-broad exemption:** exact id/slug matching only. A genuinely stuck *ordinary* goal is never accidentally exempted — a regression test asserts an ordinary stuck goal still re-parks and trips the breaker. -- **No `Bridge` naming**; new code uses `tracing` + OpenTelemetry, no - `print!`/`println!`. +- **Conservative, reversible reversal:** an explicit `standing = false` only ever + strips a *leading* marker from a `source:seed` goal it previously stamped. + Seed *absence* never reverses; user-created goals and standing *phrases* in + prose are never touched. A goal made perpetual by its own prose stays perpetual. +- **One resolution per cycle:** `ooda_loop::cycle` calls `resolve_seed_goals` + once and reuses the `Vec` for cold seeding and this reconcile — no duplicate + resolution, so cold and warm paths can never disagree about the seed set. ## Tests `src/goal_curation/tests_operations.rs`, `src/goal_curation/tests_no_progress_breaker.rs`, +`src/goal_curation/types.rs`, +`src/ooda_loop/tests_no_progress.rs`, `src/identity/coverage_tests.rs`: 1. A `standing = true` seed → `ActiveGoal` reads `is_perpetual()` after `seed_board_from_seed_goals`. 2. `reconcile_standing_markers` self-heals an existing unmarked persisted goal by - exact id/slug; a second run returns `0` (idempotent) and is total over - pathological titles. -3. A perpetual/standing goal is **not** re-parked and files **no** `ooda-stuck` - issue. -4. A non-perpetual stuck goal **still** re-parks and trips the breaker (ordinary + exact id/slug; a second run is a no-op and is total over pathological titles. +3. An explicit `standing = false` reverses the leading marker on a `source:seed` + exact-slug goal; the same slug **without** `source:seed` is untouched; seed + *absence* reverses nothing; a stripped goal whose prose still reads perpetual + stays perpetual (`unmark_standing_in_place` prefix-only guarantee); reversal is + idempotent. +4. A perpetual/standing goal is **not** re-parked and files **no** `ooda-stuck` + issue; a reverted goal re-enters the breaker and escalates like any ordinary + stuck goal. +5. A non-perpetual stuck goal **still** re-parks and trips the breaker (ordinary behaviour unchanged). -5. `TomlSeedGoal` round-trips with and without `standing` under +6. `TomlSeedGoal` round-trips with and without `standing` under `deny_unknown_fields`. ## Related diff --git a/src/goal_curation/mod.rs b/src/goal_curation/mod.rs index 505d9ca7a..e73066666 100644 --- a/src/goal_curation/mod.rs +++ b/src/goal_curation/mod.rs @@ -26,12 +26,12 @@ mod prioritize; // Re-export all public items so `crate::goal_curation::X` still works. pub use operations::CarryoverVerification; pub use operations::{ - BoardPlacement, DEFAULT_SEED_GOALS, DEFAULT_STEWARD_SCORE, active_goals_as_records, - add_active_goal, add_backlog_item, archive_completed, board_snapshot_hash, - clear_goal_assignment, default_seed_goals, load_goal_board, overwrite_memory_cache, - persist_board, promote_to_active, read_latest_carryover, reconcile_standing_markers, - record_as_active_goal, resolve_seed_goals, rollup_parent_progress, save_goal_board, - save_goal_board_with_removals, seed_board_from_seed_goals, seed_default_board, + BoardPlacement, DEFAULT_SEED_GOALS, DEFAULT_STEWARD_SCORE, StandingReconciliation, + active_goals_as_records, add_active_goal, add_backlog_item, archive_completed, + board_snapshot_hash, clear_goal_assignment, default_seed_goals, load_goal_board, + overwrite_memory_cache, persist_board, promote_to_active, read_latest_carryover, + reconcile_standing_markers, record_as_active_goal, resolve_seed_goals, rollup_parent_progress, + save_goal_board, save_goal_board_with_removals, seed_board_from_seed_goals, seed_default_board, simard_state_root, update_goal_progress, update_goal_progress_with_evidence, verify_goal_carryover, write_goal_carryover, }; diff --git a/src/goal_curation/operations.rs b/src/goal_curation/operations.rs index 94da52e3e..b11b43d5b 100644 --- a/src/goal_curation/operations.rs +++ b/src/goal_curation/operations.rs @@ -1441,7 +1441,29 @@ pub fn seed_board_from_seed_goals( goals.len() } -/// Warm-board self-heal for standing seed declarations (issue #4927). +/// Outcome of a single [`reconcile_standing_markers`] pass — how many persisted +/// goals were newly marked standing (`added`) and how many had a leading +/// standing marker reversed (`removed`). Both are zero on a settled board, so a +/// caller can log an accurate, bounded before/after without re-deriving counts. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct StandingReconciliation { + /// Persisted goals that a `standing = true` seed newly stamped perpetual. + pub added: usize, + /// Persisted `source:seed` goals whose leading standing marker an explicit + /// `standing = false` seed reversed. + pub removed: usize, +} + +impl StandingReconciliation { + /// True when this pass changed nothing — no add and no reversal. + #[must_use] + pub fn is_noop(self) -> bool { + self.added == 0 && self.removed == 0 + } +} + +/// Warm-board reconcile of standing seed declarations against the persisted +/// board (issue #4927). /// /// A `standing = true` seed only reaches a *cold* board via /// [`seed_board_from_seed_goals`] (which no-ops on a non-empty board). The live @@ -1449,36 +1471,78 @@ pub fn seed_board_from_seed_goals( /// cognitive-memory board with an UNMARKED description — the exact defect that /// re-parked it every OODA cycle and fed the `UNCLEAR-CRITERIA` issue storm /// (#4927/#4930/#4934), because the breaker's `!is_perpetual()` exemption never -/// fired for it. +/// fired for it. This heals that in place, and — so the declaration is +/// conservatively reversible — also honours an *explicit* `standing = false`. +/// +/// Matching is EXACT (by [`crate::goals::goal_slug`] against a seed's normalized +/// title), never by fuzzy prose, so a non-matching or genuinely stuck goal is +/// never silently exempted from (nor re-exposed to) the safety breaker. +/// +/// Two directions, both keyed on an exact-slug match to a present seed: +/// +/// - **`standing = true`** stamps [`STANDING_MARKER_PREFIX`] onto a matching +/// persisted goal that is not already perpetual (as the original self-heal +/// did). Idempotent — an already-perpetual goal is skipped, so a repeat pass +/// heals nothing and never double-stamps. +/// - **explicit `standing = false`** reverses a *previously reconciled* +/// declaration by stripping ONLY a leading [`STANDING_MARKER_PREFIX`], and +/// ONLY from a matching goal that carries the exact [`SOURCE_SEED`] label — +/// i.e. a goal this seeding path itself created. "Explicit" is load-bearing: +/// only a seed built via [`crate::identity::SeedGoal::non_standing`] (or TOML +/// `standing = false`) reverses. An **omitted** `standing` — a default seed +/// from [`crate::identity::SeedGoal::new`] — is inert and never reverses, +/// exactly like **seed absence** (deleting a seed entirely leaves its goal +/// untouched). It also never edits a user-created goal (no `source:seed` +/// label) and never rewrites a standing *phrase* in the prose (so a goal +/// whose prose independently reads perpetual stays perpetual). +/// +/// A no-op when no seed matches. Returns the add/remove counts. /// -/// This stamps the standing marker onto each persisted active goal whose id -/// matches a `standing` seed's normalized title slug, turning it perpetual in -/// place. Matching is EXACT (by [`crate::goals::goal_slug`]), never by fuzzy -/// prose — a non-matching or genuinely stuck goal must never be silently -/// exempted from the safety breaker. It is idempotent (an already-perpetual -/// goal is skipped, so a repeat pass heals nothing and never double-stamps) and -/// a no-op when no seed is standing. Returns the number of goals healed. +/// [`SOURCE_SEED`]: crate::goal_curation::labels::SOURCE_SEED pub fn reconcile_standing_markers( board: &mut GoalBoard, seeds: &[crate::identity::SeedGoal], -) -> usize { - let standing_ids: std::collections::BTreeSet = seeds +) -> StandingReconciliation { + use std::collections::BTreeSet; + + let standing_true: BTreeSet = seeds .iter() .filter(|seed| seed.standing) .map(|seed| crate::goals::goal_slug(&seed.title)) .collect(); - if standing_ids.is_empty() { - return 0; + // An explicit `standing = false` reverses only where the same slug is not + // *also* declared standing elsewhere (a `true` declaration always wins). + // Crucially this keys off `authorizes_standing_reversal()` — an EXPLICIT + // false — never merely `!seed.standing`, so an omitted/default seed (which + // is also non-standing) stays inert and never reverses a marker (#4927). + let standing_false: BTreeSet = seeds + .iter() + .filter(|seed| seed.authorizes_standing_reversal()) + .map(|seed| crate::goals::goal_slug(&seed.title)) + .filter(|slug| !standing_true.contains(slug)) + .collect(); + if standing_true.is_empty() && standing_false.is_empty() { + return StandingReconciliation::default(); } - let mut healed = 0; + let mut out = StandingReconciliation::default(); for goal in &mut board.active { - if standing_ids.contains(&goal.id) && !goal.is_perpetual() { - goal.mark_standing_in_place(); - healed += 1; + if standing_true.contains(&goal.id) { + if !goal.is_perpetual() { + goal.mark_standing_in_place(); + out.added += 1; + } + } else if standing_false.contains(&goal.id) + && goal + .labels + .iter() + .any(|l| l == crate::goal_curation::labels::SOURCE_SEED) + && goal.unmark_standing_in_place() + { + out.removed += 1; } } - healed + out } // --------------------------------------------------------------------------- diff --git a/src/goal_curation/tests_no_progress_breaker.rs b/src/goal_curation/tests_no_progress_breaker.rs index 2d68edbaa..93378a977 100644 --- a/src/goal_curation/tests_no_progress_breaker.rs +++ b/src/goal_curation/tests_no_progress_breaker.rs @@ -382,7 +382,7 @@ fn standing_seed_goal_reads_as_perpetual_and_is_breaker_exempt() { let mut warm = GoalBoard::new(); warm.active.push(live); assert_eq!( - reconcile_standing_markers(&mut warm, std::slice::from_ref(&standing)), + reconcile_standing_markers(&mut warm, std::slice::from_ref(&standing)).added, 1 ); assert!( diff --git a/src/goal_curation/tests_operations.rs b/src/goal_curation/tests_operations.rs index 4591e7761..f4aaa372c 100644 --- a/src/goal_curation/tests_operations.rs +++ b/src/goal_curation/tests_operations.rs @@ -1,5 +1,7 @@ use super::operations::*; -use super::types::{ActiveGoal, BacklogItem, GoalBoard, GoalProgress, MAX_ACTIVE_GOALS}; +use super::types::{ + ActiveGoal, BacklogItem, GoalBoard, GoalProgress, MAX_ACTIVE_GOALS, STANDING_MARKER_PREFIX, +}; fn make_goal(id: &str, priority: u32) -> ActiveGoal { ActiveGoal { @@ -1744,6 +1746,13 @@ fn ordinary_seed(title: &str, desc: &str) -> crate::identity::SeedGoal { crate::identity::SeedGoal::new(2, title, desc, None) } +/// An *explicit* `standing = false` seed (issue #4927): non-standing for cold +/// seeding, yet the only non-standing form that authorizes conservative +/// reversal — as opposed to [`ordinary_seed`], whose omitted flag is inert. +fn non_standing_seed(title: &str, desc: &str) -> crate::identity::SeedGoal { + crate::identity::SeedGoal::new(2, title, desc, None).non_standing() +} + #[test] fn seed_board_from_seed_goals_marks_a_standing_seed_as_perpetual() { // Cold start: a `standing = true` seed must produce an ActiveGoal that reads @@ -1797,9 +1806,10 @@ fn reconcile_standing_markers_self_heals_a_persisted_goal_by_id() { let healed = reconcile_standing_markers(&mut board, &[standing_seed(HYGIENE_TITLE, HYGIENE_DESC)]); assert_eq!( - healed, 1, + healed.added, 1, "the matching persisted goal must be healed exactly once" ); + assert_eq!(healed.removed, 0, "an add pass must not remove any marker"); assert!( board.active[0].is_perpetual(), "reconcile must make the persisted hygiene goal read as perpetual (issue #4927)" @@ -1828,7 +1838,7 @@ fn reconcile_standing_markers_matches_by_normalized_title_slug() { HYGIENE_DESC, )], ); - assert_eq!(healed, 1); + assert_eq!(healed.added, 1); assert!(board.active[0].is_perpetual()); } @@ -1843,11 +1853,10 @@ fn reconcile_standing_markers_is_idempotent() { board.active.push(goal); let seeds = [standing_seed(HYGIENE_TITLE, HYGIENE_DESC)]; - assert_eq!(reconcile_standing_markers(&mut board, &seeds), 1); + assert_eq!(reconcile_standing_markers(&mut board, &seeds).added, 1); let after_first = board.active[0].description.clone(); - assert_eq!( - reconcile_standing_markers(&mut board, &seeds), - 0, + assert!( + reconcile_standing_markers(&mut board, &seeds).is_noop(), "a second reconcile must heal nothing" ); assert_eq!( @@ -1867,14 +1876,19 @@ fn reconcile_standing_markers_ignores_unmatched_goals() { let healed = reconcile_standing_markers(&mut board, &[standing_seed(HYGIENE_TITLE, HYGIENE_DESC)]); - assert_eq!(healed, 0, "no non-matching goal may be stamped standing"); + assert!( + healed.is_noop(), + "no non-matching goal may be stamped standing" + ); assert!(!board.active[0].is_perpetual()); } #[test] fn reconcile_standing_markers_ignores_non_standing_seeds() { - // A seed with standing=false must NEVER stamp a matching persisted goal — - // otherwise every seed would silently become breaker-exempt. + // A seed with standing=false must NEVER stamp (add a marker to) a matching + // persisted goal — otherwise every seed would silently become breaker-exempt. + // (An explicit false may *reverse* a leading marker, but only on a + // `source:seed` goal that actually carries one — covered separately below.) let id = crate::goals::goal_slug(HYGIENE_TITLE); let mut goal = ActiveGoal::new(id, HYGIENE_DESC, 2); goal.status = GoalProgress::NotStarted; @@ -1884,8 +1898,8 @@ fn reconcile_standing_markers_ignores_non_standing_seeds() { let healed = reconcile_standing_markers(&mut board, &[ordinary_seed(HYGIENE_TITLE, HYGIENE_DESC)]); assert_eq!( - healed, 0, - "an ordinary (standing=false) seed must heal nothing" + healed.added, 0, + "an ordinary (standing=false) seed must never stamp a goal standing" ); assert!(!board.active[0].is_perpetual()); } @@ -1902,6 +1916,186 @@ fn reconcile_standing_markers_skips_already_perpetual_goals() { let healed = reconcile_standing_markers(&mut board, &[standing_seed(HYGIENE_TITLE, HYGIENE_DESC)]); - assert_eq!(healed, 0, "an already-perpetual goal is not healed again"); + assert!( + healed.is_noop(), + "an already-perpetual goal is not healed again" + ); + assert_eq!(board.active[0].description, before); +} + +// =========================================================================== +// Conservative REVERSAL of a standing declaration (issue #4927 rework) +// +// A standing declaration must be reversible without a board wipe: flipping a +// seed back to `standing = false` should strip the marker the reconciler itself +// added. The reversal is deliberately narrow — leading sentinel only, exact +// slug, `source:seed` label only — so it can never demote a user-created goal, +// never edit standing *phrases* in prose, and never fire for seed *absence*. +// =========================================================================== + +/// A persisted seed goal (carries the `source:seed` label the seeding path +/// stamps) whose id is `HYGIENE_TITLE`'s slug and whose description is unmarked. +fn persisted_seed_goal(desc: &str) -> ActiveGoal { + let id = crate::goals::goal_slug(HYGIENE_TITLE); + let mut g = ActiveGoal::new(id, desc, 2).with_label(crate::goal_curation::labels::SOURCE_SEED); + g.status = GoalProgress::NotStarted; + g +} + +#[test] +fn reconcile_standing_markers_reverses_explicit_false_on_source_seed_goal() { + // add-then-reverse: a standing=true pass marks the source:seed goal, then a + // standing=false pass on the SAME exact slug strips the leading marker. + let mut board = GoalBoard::new(); + board.active.push(persisted_seed_goal(HYGIENE_DESC)); + + let added = + reconcile_standing_markers(&mut board, &[standing_seed(HYGIENE_TITLE, HYGIENE_DESC)]); + assert_eq!(added.added, 1); + assert!(board.active[0].is_perpetual()); + + let reversed = reconcile_standing_markers( + &mut board, + &[non_standing_seed(HYGIENE_TITLE, HYGIENE_DESC)], + ); + assert_eq!( + (reversed.added, reversed.removed), + (0, 1), + "an explicit standing=false must reverse exactly the marker it added" + ); + assert!( + !board.active[0].is_perpetual(), + "after reversal the goal is convergence-required again" + ); + assert_eq!( + board.active[0].description, HYGIENE_DESC, + "reversal must restore the original description byte-for-byte" + ); +} + +#[test] +fn reconcile_standing_markers_does_not_reverse_without_source_seed_label() { + // A user-created goal (no source:seed) that happens to share the slug and + // carry a leading marker must NOT be demoted by an explicit standing=false — + // reversal only touches goals this seeding path created. + let id = crate::goals::goal_slug(HYGIENE_TITLE); + let mut user_goal = ActiveGoal::new(id, HYGIENE_DESC, 2).mark_standing(); + user_goal.status = GoalProgress::NotStarted; + assert!(user_goal.is_perpetual()); + let before = user_goal.description.clone(); + let mut board = GoalBoard::new(); + board.active.push(user_goal); + + let reversed = reconcile_standing_markers( + &mut board, + &[non_standing_seed(HYGIENE_TITLE, HYGIENE_DESC)], + ); + assert!( + reversed.is_noop(), + "a goal without source:seed must never be reversed" + ); + assert_eq!(board.active[0].description, before); + assert!(board.active[0].is_perpetual()); +} + +#[test] +fn reconcile_standing_markers_omitted_seed_does_not_reverse() { + // The three-state guarantee (issue #4927): an OMITTED `standing` — a default + // `SeedGoal::new` seed — must NEVER reverse a marker, even on the exact-slug + // `source:seed` goal that an explicit `standing = false` WOULD reverse. This + // is the case distinct from explicit-false reversal + // (`reconcile_standing_markers_reverses_explicit_false_on_source_seed_goal`). + let mut board = GoalBoard::new(); + let mut marked = persisted_seed_goal(HYGIENE_DESC); + marked.mark_standing_in_place(); + assert!(marked.is_perpetual()); + let before = marked.description.clone(); + board.active.push(marked); + + // Same exact slug + source:seed label, but the seed OMITS `standing`. + let omitted = ordinary_seed(HYGIENE_TITLE, HYGIENE_DESC); + assert!( + !omitted.authorizes_standing_reversal(), + "sanity: an omitted seed must not authorize reversal" + ); + let reversed = reconcile_standing_markers(&mut board, &[omitted]); + assert!( + reversed.is_noop(), + "an omitted (default) seed must never reverse a marker — only explicit false does" + ); assert_eq!(board.active[0].description, before); + assert!( + board.active[0].is_perpetual(), + "the omitted seed leaves the standing goal perpetual" + ); +} + +#[test] +fn reconcile_standing_markers_does_not_reverse_on_seed_absence() { + // Deleting a seed entirely (slug not present in the seed set) must leave its + // marked goal untouched — only an EXPLICIT standing=false reverses. + let mut board = GoalBoard::new(); + let mut marked = persisted_seed_goal(HYGIENE_DESC); + marked.mark_standing_in_place(); + assert!(marked.is_perpetual()); + let before = marked.description.clone(); + board.active.push(marked); + + // An unrelated seed set that does not mention the hygiene slug at all. + let unrelated = crate::identity::SeedGoal::new(1, "Some other seed", "x", None); + let reversed = reconcile_standing_markers(&mut board, &[unrelated]); + assert!( + reversed.is_noop(), + "seed absence must never reverse a marker (only explicit false does)" + ); + assert_eq!(board.active[0].description, before); + assert!(board.active[0].is_perpetual()); +} + +#[test] +fn reconcile_standing_markers_reversal_strips_only_the_leading_sentinel_prose_stays_perpetual() { + // Prefix-only guarantee: a source:seed goal whose PROSE independently reads + // as standing keeps a leading sentinel stripped on reversal, yet stays + // perpetual because the phrase in the prose is never edited. + let mut board = GoalBoard::new(); + // A source:seed goal whose prose already reads perpetual AND that the + // reconciler previously stamped with a leading sentinel. + let mut goal = persisted_seed_goal("standing goal: keep grooming the backlog"); + goal.description = format!("{STANDING_MARKER_PREFIX}standing goal: keep grooming the backlog"); + board.active.push(goal); + assert!(board.active[0].is_perpetual()); + + let reversed = reconcile_standing_markers( + &mut board, + &[non_standing_seed(HYGIENE_TITLE, HYGIENE_DESC)], + ); + assert_eq!(reversed.removed, 1, "the leading sentinel must be stripped"); + assert!( + !board.active[0] + .description + .starts_with(STANDING_MARKER_PREFIX), + "the leading sentinel is gone" + ); + assert!( + board.active[0].is_perpetual(), + "the untouched standing phrase in prose keeps the goal perpetual" + ); +} + +#[test] +fn reconcile_standing_markers_reversal_is_idempotent() { + // A second explicit-false pass after reversal changes nothing. + let mut board = GoalBoard::new(); + let mut marked = persisted_seed_goal(HYGIENE_DESC); + marked.mark_standing_in_place(); + board.active.push(marked); + + let seeds = [non_standing_seed(HYGIENE_TITLE, HYGIENE_DESC)]; + assert_eq!(reconcile_standing_markers(&mut board, &seeds).removed, 1); + let after = board.active[0].description.clone(); + assert!( + reconcile_standing_markers(&mut board, &seeds).is_noop(), + "a second reversal pass must be a no-op" + ); + assert_eq!(board.active[0].description, after); } diff --git a/src/goal_curation/types.rs b/src/goal_curation/types.rs index 0bd4d3979..2e9c544eb 100644 --- a/src/goal_curation/types.rs +++ b/src/goal_curation/types.rs @@ -365,6 +365,29 @@ impl ActiveGoal { } } + /// Strip a single leading [`STANDING_MARKER_PREFIX`] sentinel from this + /// goal's description, in place. Returns `true` iff the description changed. + /// + /// This is the deliberately-narrow inverse of [`mark_standing_in_place`], + /// used to reverse a standing declaration that was applied by the seed + /// reconciler (issue #4927). It removes **only** the leading sentinel — it + /// never rewrites standing *phrases* embedded in the prose (e.g. a + /// description that literally reads "standing goal …"). A goal whose prose + /// independently marks it perpetual therefore stays + /// [`is_perpetual`](ActiveGoal::is_perpetual) after this call, and a goal + /// without the leading sentinel is left byte-for-byte unchanged (returns + /// `false`). + /// + /// [`mark_standing_in_place`]: ActiveGoal::mark_standing_in_place + pub fn unmark_standing_in_place(&mut self) -> bool { + if let Some(rest) = self.description.strip_prefix(STANDING_MARKER_PREFIX) { + self.description = rest.to_string(); + true + } else { + false + } + } + /// Roll a standing/perpetual goal into a fresh cycle after its current unit /// of work finishes, instead of terminating it (issue #2580). Resets the /// goal to an actionable, re-dispatchable state: status back to @@ -860,6 +883,40 @@ mod tests { assert_eq!(again.description, g.description); } + #[test] + fn unmark_standing_in_place_strips_only_the_leading_sentinel() { + // Round-trips a reconciler-added marker and reports the change. + let mut g = ActiveGoal::new("g", "watch CI", 1).mark_standing(); + assert!(g.is_perpetual()); + assert!(g.unmark_standing_in_place(), "must report it changed"); + assert_eq!(g.description, "watch CI"); + assert!(!g.is_perpetual()); + // A second call is a byte-for-byte no-op returning false. + assert!(!g.unmark_standing_in_place()); + assert_eq!(g.description, "watch CI"); + } + + #[test] + fn unmark_standing_in_place_never_edits_a_standing_phrase_in_prose() { + // Prose independently makes it perpetual; stripping the (absent) leading + // sentinel is a no-op and the goal stays perpetual. + let mut g = ActiveGoal::new("g", "Steward CI. Standing goal.", 1); + assert!(g.is_perpetual()); + assert!( + !g.unmark_standing_in_place(), + "no leading sentinel to strip" + ); + assert!(g.is_perpetual(), "the prose phrase is never edited"); + + // With BOTH a leading sentinel and a prose phrase, only the sentinel is + // removed; the prose keeps it perpetual. + let mut both = ActiveGoal::new("g", "Standing goal: steward CI.", 1); + both.description = format!("{STANDING_MARKER_PREFIX}{}", both.description); + assert!(both.unmark_standing_in_place()); + assert!(!both.description.starts_with(STANDING_MARKER_PREFIX)); + assert!(both.is_perpetual(), "prose keeps the goal perpetual"); + } + #[test] fn roll_to_new_cycle_resets_to_actionable_and_stays_perpetual() { let mut g = sample_goal(); diff --git a/src/identity/coverage_tests.rs b/src/identity/coverage_tests.rs index 8df2530f5..9786cb133 100644 --- a/src/identity/coverage_tests.rs +++ b/src/identity/coverage_tests.rs @@ -663,6 +663,49 @@ fn seed_goal_standing_builder_marks_standing() { assert_eq!(g.description, "turn observations into goals"); } +#[test] +fn seed_goal_new_is_omitted_and_never_authorizes_reversal() { + // A plain `new` seed is non-standing AND an omitted declaration: it must not + // authorize the reconciler to reverse a marker (issue #4927). This is the + // three-state distinction — omitted is inert. + let g = SeedGoal::new(1, "ordinary", "do a bounded thing", None); + assert!(!g.standing, "a plain SeedGoal must default to non-standing"); + assert!( + !g.authorizes_standing_reversal(), + "an omitted (default) seed must NEVER authorize reversal" + ); +} + +#[test] +fn seed_goal_non_standing_builder_authorizes_reversal_but_stays_non_standing() { + // An explicit `.non_standing()` seed stays non-standing (cold seeding treats + // it exactly like an omitted seed) yet is the ONLY non-standing form that + // authorizes conservative reversal (issue #4927). + let g = SeedGoal::new(2, "Articulate repo-hygiene backlog", "d", None).non_standing(); + assert!( + !g.standing, + "explicit non_standing must remain non-standing for cold seeding" + ); + assert!( + g.authorizes_standing_reversal(), + "an explicit standing = false must authorize reversal" + ); + // Purely declarative — never touches the description. + assert_eq!(g.description, "d"); +} + +#[test] +fn seed_goal_standing_builder_does_not_authorize_reversal() { + // A `standing = true` declaration is explicit but ADDS a marker; it must + // never be treated as a reversal. + let g = SeedGoal::new(2, "g", "d", None).standing(); + assert!(g.standing); + assert!( + !g.authorizes_standing_reversal(), + "a standing = true seed adds, it does not reverse" + ); +} + #[test] fn toml_seed_goal_deserializes_standing_true() { let toml = r#" @@ -674,7 +717,11 @@ standing = true "#; let seed: super::toml_types::TomlSeedGoal = toml::from_str(toml).expect("standing=true seed must deserialize"); - assert!(seed.standing, "standing = true must round-trip from TOML"); + assert_eq!( + seed.standing, + Some(true), + "standing = true must round-trip from TOML as an explicit Some(true)" + ); } #[test] @@ -689,7 +736,30 @@ repo = "hyenas" "#; let seed: super::toml_types::TomlSeedGoal = toml::from_str(toml).expect("seed without standing must still parse"); - assert!(!seed.standing, "omitted standing must default to false"); + assert_eq!( + seed.standing, None, + "omitted standing must be preserved as None (distinct from Some(false))" + ); +} + +#[test] +fn toml_seed_goal_deserializes_explicit_standing_false() { + // An explicit `standing = false` must be preserved as Some(false), distinct + // from an omitted flag (None) — this is what lets the loader map it to an + // explicit `.non_standing()` seed that authorizes conservative reversal. + let toml = r#" +priority = 1 +title = "Articulate repo-hygiene backlog" +description = "d" +standing = false +"#; + let seed: super::toml_types::TomlSeedGoal = + toml::from_str(toml).expect("standing=false seed must deserialize"); + assert_eq!( + seed.standing, + Some(false), + "explicit standing = false must round-trip as Some(false)" + ); } #[test] diff --git a/src/identity/file_loader.rs b/src/identity/file_loader.rs index b101c814f..082d6e880 100644 --- a/src/identity/file_loader.rs +++ b/src/identity/file_loader.rs @@ -81,7 +81,16 @@ impl FileIdentityLoader { g.description.clone(), g.repo.clone(), ); - if g.standing { seed.standing() } else { seed } + // #4927 three-state mapping: preserve the omitted/explicit + // distinction the `Option` carries. Omitted (`None`) is an + // inert non-standing seed; explicit `false` maps to + // `.non_standing()` (authorizes conservative reversal); `true` + // maps to `.standing()`. + match g.standing { + Some(true) => seed.standing(), + Some(false) => seed.non_standing(), + None => seed, + } }) .collect(); let target_repos = identity.target_repos.clone(); @@ -1169,6 +1178,50 @@ posture = "read-only" assert!(!manifest.authority.permits_spawn()); } + #[test] + fn file_loader_preserves_standing_declaration_three_state() { + let toml = r#" +[package] +name = "crocutus" +version = "0.1.0" + +[[identities]] +name = "crocutus" +default_mode = "engineer" + +[[identities.seed_goals]] +priority = 1 +title = "Explicit standing" +description = "d" +standing = true + +[[identities.seed_goals]] +priority = 2 +title = "Explicit non-standing" +description = "d" +standing = false + +[[identities.seed_goals]] +priority = 3 +title = "Omitted standing" +description = "d" +"#; + let manifest = load_crocutus(toml).expect("three-state identity must load"); + assert_eq!(manifest.seed_goals.len(), 3); + + let explicit_standing = &manifest.seed_goals[0]; + assert!(explicit_standing.standing); + assert!(!explicit_standing.authorizes_standing_reversal()); + + let explicit_non_standing = &manifest.seed_goals[1]; + assert!(!explicit_non_standing.standing); + assert!(explicit_non_standing.authorizes_standing_reversal()); + + let omitted = &manifest.seed_goals[2]; + assert!(!omitted.standing); + assert!(!omitted.authorizes_standing_reversal()); + } + #[test] fn file_loader_target_repos_defaults_to_union_of_seed_goal_repos() { let toml = r#" diff --git a/src/identity/manifest.rs b/src/identity/manifest.rs index d748c7479..2cf0cc71c 100644 --- a/src/identity/manifest.rs +++ b/src/identity/manifest.rs @@ -30,8 +30,22 @@ pub struct SeedGoal { /// breaker's `!is_perpetual()` exemption applies and the goal is never /// re-parked or issue-filed for lack of convergence. Additive and /// defaulting `false`, so every existing seed goal stays - /// convergence-required exactly as before. + /// convergence-required exactly as before. This stays the single boolean + /// the cold-seed path keys off — `true` marks perpetual, `false` (whether + /// omitted or explicit) is an ordinary convergence-required goal. pub standing: bool, + /// Whether the `standing` value was *explicitly declared* in the source + /// (issue #4927 conservative reversal). This is the narrow provenance bit + /// that distinguishes an omitted/default non-standing seed — built via + /// [`SeedGoal::new`], which never authorizes reversal — from one that + /// explicitly declares `standing = false` (via [`SeedGoal::non_standing`], + /// or TOML `standing = false`), which alone authorizes the standing + /// reconciler to REVERSE a marker it previously added. Kept `pub(crate)` + /// because it is a reconciliation-only detail; callers read it through the + /// [`authorizes_standing_reversal`](SeedGoal::authorizes_standing_reversal) + /// accessor rather than the raw field, so cold seeding and the public API + /// keep treating omitted and explicit-false identically. + pub(crate) standing_explicit: bool, } impl SeedGoal { @@ -47,6 +61,7 @@ impl SeedGoal { description: description.into(), repo, standing: false, + standing_explicit: false, } } @@ -55,12 +70,43 @@ impl SeedGoal { /// description; the standing marker is applied later at the seed→ /// [`crate::goal_curation::ActiveGoal`] conversion so /// [`crate::goal_curation::ActiveGoal::is_perpetual`] stays the single - /// source of truth. + /// source of truth. Records the declaration as explicit, but since it is a + /// `true` declaration it never authorizes a reversal. #[must_use] pub fn standing(mut self) -> Self { self.standing = true; + self.standing_explicit = true; self } + + /// Builder: declare this seed *explicitly* non-standing (issue #4927 + /// conservative reversal). Unlike an omitted/default seed from + /// [`SeedGoal::new`] — which is also non-standing but stays inert — this is + /// a deliberate `standing = false` declaration. It leaves `standing` false + /// (so cold seeding still treats it as an ordinary convergence-required + /// goal, exactly like an omitted seed) but records the declaration as + /// explicit, which is the ONLY thing that authorizes the standing + /// reconciler to REVERSE a marker it previously added to the matching + /// `source:seed` goal. Used by the identity file loader when TOML says + /// `standing = false`. + #[must_use] + pub fn non_standing(mut self) -> Self { + self.standing = false; + self.standing_explicit = true; + self + } + + /// Whether this seed authorizes the standing reconciler to REVERSE a prior + /// standing marker (issue #4927). True *only* for an explicit + /// `standing = false` declaration (via [`SeedGoal::non_standing`] or TOML) — + /// never for an omitted/default non-standing seed from [`SeedGoal::new`], + /// and never for a `standing = true` seed (that adds, it does not reverse). + /// This is the single predicate the reconciler uses so that omission stays + /// inert while an explicit false is conservatively reversible. + #[must_use] + pub fn authorizes_standing_reversal(&self) -> bool { + self.standing_explicit && !self.standing + } } /// The write-authority posture of an identity — the read-only switch that the diff --git a/src/identity/toml_types.rs b/src/identity/toml_types.rs index b98dff7a4..7e686d7aa 100644 --- a/src/identity/toml_types.rs +++ b/src/identity/toml_types.rs @@ -66,12 +66,16 @@ pub(crate) struct TomlSeedGoal { pub description: String, #[serde(default)] pub repo: Option, - /// Declares a standing/perpetual seed goal (issue #4927). `#[serde(default)]` - /// keeps every existing `seed_goals` entry (which omits it) valid as a - /// non-standing goal, while `deny_unknown_fields` still fails loud on a - /// typo'd flag rather than silently leaving a safety goal non-perpetual. + /// Declares a standing/perpetual seed goal (issue #4927). Modelled as + /// `Option` so an omitted flag (`None`) is preserved as *distinct* + /// from an explicit `standing = false` (`Some(false)`): only the latter + /// authorizes the standing reconciler to reverse a previously-added marker, + /// while an omitted seed stays inert. `#[serde(default)]` keeps every + /// existing `seed_goals` entry (which omits it) valid as a non-standing + /// goal, while `deny_unknown_fields` still fails loud on a typo'd flag + /// rather than silently leaving a safety goal non-perpetual. #[serde(default)] - pub standing: bool, + pub standing: Option, } /// An optional `[identities.authority]` table (#3125 / #3067). The read-only diff --git a/src/ooda_loop/cycle.rs b/src/ooda_loop/cycle.rs index 4d62f6e39..76153eaa2 100644 --- a/src/ooda_loop/cycle.rs +++ b/src/ooda_loop/cycle.rs @@ -136,9 +136,15 @@ fn run_ooda_cycle_inner( // is unchanged. Goals carry the identity's target-repo slug, so they are // scoped to its targets, never to rysweet/Simard. let identity_seed_goals = &state.identity_cognition.seed_goals; + // Resolve the seed set ONCE per cycle (identity override or baked-in + // defaults) and reuse the same Vec for cold seeding below and the warm-board + // reconcile further down — no second `resolve_seed_goals` call (#4927). + let resolved_seed_goals = crate::goal_curation::resolve_seed_goals(identity_seed_goals); if !identity_seed_goals.is_empty() { - let goals = crate::goal_curation::resolve_seed_goals(identity_seed_goals); - let n = crate::goal_curation::seed_board_from_seed_goals(&mut state.active_goals, &goals); + let n = crate::goal_curation::seed_board_from_seed_goals( + &mut state.active_goals, + &resolved_seed_goals, + ); if n > 0 { let who = state .identity_cognition @@ -156,21 +162,24 @@ fn run_ooda_cycle_inner( } } - // #4927: self-heal already-persisted goals that a `standing` seed declares - // perpetual. Cold seeding (above) marks fresh goals, but a warm board loaded - // from cognitive memory carries the live goal with an UNMARKED description, - // so the no-progress breaker's `!is_perpetual()` exemption never fired for - // it — the goal was re-parked and issue-filed every cycle. Reconciling here - // against the SAME resolved seed set stamps the standing marker onto the - // matching persisted goal in place. Idempotent and a no-op when no seed is - // standing, so Simard's default board is unaffected. + // #4927: reconcile already-persisted goals against the SAME resolved seed + // set. Cold seeding (above) marks fresh goals, but a warm board loaded from + // cognitive memory carries the live goal with an UNMARKED description, so the + // no-progress breaker's `!is_perpetual()` exemption never fired for it — the + // goal was re-parked and issue-filed every cycle. A `standing = true` seed + // stamps the marker onto its matching persisted goal in place; an explicit + // `standing = false` seed conservatively reverses a leading marker it had + // previously added (source:seed goals only). Idempotent and a no-op when no + // seed matches, so Simard's default board is unaffected. { - let resolved = crate::goal_curation::resolve_seed_goals(identity_seed_goals); - let healed = - crate::goal_curation::reconcile_standing_markers(&mut state.active_goals, &resolved); - if healed > 0 { + let recon = crate::goal_curation::reconcile_standing_markers( + &mut state.active_goals, + &resolved_seed_goals, + ); + if !recon.is_noop() { eprintln!( - "[simard] OODA start: reconciled {healed} persisted goal(s) to standing/perpetual (#4927)" + "[simard] OODA start: reconciled standing markers — {} added, {} removed (#4927)", + recon.added, recon.removed ); } } diff --git a/src/ooda_loop/tests_no_progress.rs b/src/ooda_loop/tests_no_progress.rs index 4999e0346..e7c342e73 100644 --- a/src/ooda_loop/tests_no_progress.rs +++ b/src/ooda_loop/tests_no_progress.rs @@ -1324,7 +1324,7 @@ fn reconciled_standing_hygiene_goal_is_exempt_from_the_no_progress_breaker() { .standing(); let healed = crate::goal_curation::reconcile_standing_markers(&mut board, &[standing]); assert_eq!( - healed, 1, + healed.added, 1, "reconcile must self-heal the one matching live goal" ); assert!( @@ -1420,3 +1420,98 @@ fn unmarked_hygiene_goal_still_escalates_proving_the_tag_is_the_fix() { "the pre-fix unmarked goal must still file exactly the escalation issue" ); } + +#[test] +fn reverted_standing_seed_goal_re_enters_the_no_progress_breaker() { + // #4927 rework: a standing declaration is conservatively reversible. A + // source:seed goal marked standing, then reverted by an explicit + // `standing = false` seed, must lose its breaker exemption and escalate + // again exactly like an ordinary stuck goal — proving the reversal actually + // re-arms the safety breaker. + let threshold = NO_PROGRESS_BREAKER_THRESHOLD; + let title = "Articulate repo-hygiene backlog"; + let id = crate::goals::goal_slug(title); + let mut goal = ActiveGoal::new( + id.clone(), + "Turn observations into prioritized repo-hygiene goals.", + 2, + ) + .with_label(crate::goal_curation::labels::SOURCE_SEED); + goal.status = GoalProgress::NotStarted; + + let mut board = GoalBoard::new(); + board.active.push(goal); + + // 1) Declare standing -> exempt. + let standing = crate::identity::SeedGoal::new( + 2, + title, + "Turn observations into prioritized repo-hygiene goals.", + None, + ) + .standing(); + assert_eq!( + crate::goal_curation::reconcile_standing_markers(&mut board, &[standing]).added, + 1 + ); + assert!(board.active[0].is_perpetual()); + + // 2) Revert with an explicit standing=false seed of the SAME slug -> the + // reconciler strips the marker it added; the goal converges again. The + // reversal MUST be explicit (`.non_standing()`), never a merely-omitted + // seed, which stays inert (#4927 three-state semantics). + let reverted = crate::identity::SeedGoal::new( + 2, + title, + "Turn observations into prioritized repo-hygiene goals.", + None, + ) + .non_standing(); + assert!( + reverted.authorizes_standing_reversal(), + "sanity: an explicit standing=false seed authorizes reversal" + ); + assert_eq!( + crate::goal_curation::reconcile_standing_markers(&mut board, &[reverted]).removed, + 1 + ); + assert!( + !board.active[0].is_perpetual(), + "after reversal the goal is no longer breaker-exempt" + ); + + // 3) Drive the breaker: the reverted goal must escalate and file an issue. + let mut state = OodaState::new(board); + let evidence = FakeEvidence { + pr_merged: false, + issue_closed: false, + deployed: false, + }; + let filer = RecordingFiler::default(); + + let mut fired = false; + for _ in 1..=(threshold + 1) { + let report = apply_no_progress_breaker_with_threshold( + &mut state, + &[no_action_outcome(&id)], + &evidence, + &filer, + threshold, + ); + assert!( + report.perpetual_idled.is_empty(), + "a reverted goal must never be treated as a perpetual idle" + ); + if report.fired() { + fired = true; + } + } + assert!( + fired, + "a reverted standing goal must trip the no-progress breaker again" + ); + assert!( + !filer.calls.borrow().is_empty(), + "the reverted goal must file the escalation issue like any ordinary stuck goal" + ); +}