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..6b29fd4cf --- /dev/null +++ b/docs/howto/declare-a-standing-seed-goal.md @@ -0,0 +1,197 @@ +--- +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 = 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. + +## 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)). +The cycle logs a bounded line with the reconcile counts (added/removed only) when +it stamps or reverses 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..62b4d9874 --- /dev/null +++ b/docs/reference/standing-seed-goal-declaration-api.md @@ -0,0 +1,382 @@ +--- +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, **reversible** `reconcile_standing_markers` +> reconcile, both +> in [`src/goal_curation/operations.rs`](https://github.com/rysweet/Simard/blob/main/src/goal_curation/operations.rs). +> 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." + +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). 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`, 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` with + `standing_explicit: false` (omitted). Opt in with the builders below. + +### `SeedGoal::standing()` / `SeedGoal::non_standing()` builders + +```rust +impl SeedGoal { + /// 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) + +`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. + /// `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 `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` maps the wire form during `TomlSeedGoal → SeedGoal` +construction, preserving all three states — no interpretation beyond the direct +mapping: + +```rust +let seed = SeedGoal::new( + g.priority, + g.title.clone(), + g.description.clone(), + g.repo.clone(), +); +match g.standing { + Some(true) => seed.standing(), // perpetual + Some(false) => seed.non_standing(), // explicit reversal + None => seed, // omitted / inert +} +``` + +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 +/// 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 *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. | +| **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 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` 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 | false | (omitted) + │ (serde Option, deny_unknown_fields, absent ⇒ None) + ▼ +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 + ├── 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|false + │ + ▼ + no-progress breaker EXEMPTS perpetual goals + (no re-park, no issue); reversed goals converge again +``` + +## 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 | +| 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 +[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. +- **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 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). +6. `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 401368d99..e7aef29e8 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -204,6 +204,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 - Quarantine and Recover an Unclear OODA Goal: howto/quarantine-and-recover-an-unclear-ooda-goal.md @@ -463,6 +464,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 e2fe3e449..e73066666 100644 --- a/src/goal_curation/mod.rs +++ b/src/goal_curation/mod.rs @@ -26,13 +26,14 @@ 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, 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, + 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, }; 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 a56dbf634..b11b43d5b 100644 --- a/src/goal_curation/operations.rs +++ b/src/goal_curation/operations.rs @@ -1413,7 +1413,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, @@ -1426,12 +1426,125 @@ 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() } +/// 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 +/// `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 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. +/// +/// [`SOURCE_SEED`]: crate::goal_curation::labels::SOURCE_SEED +pub fn reconcile_standing_markers( + board: &mut GoalBoard, + seeds: &[crate::identity::SeedGoal], +) -> StandingReconciliation { + use std::collections::BTreeSet; + + let standing_true: BTreeSet = seeds + .iter() + .filter(|seed| seed.standing) + .map(|seed| crate::goals::goal_slug(&seed.title)) + .collect(); + // 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 out = StandingReconciliation::default(); + for goal in &mut board.active { + 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; + } + } + out +} + // --------------------------------------------------------------------------- // 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..93378a977 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)).added, + 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 194a0405d..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 { @@ -1716,3 +1718,384 @@ 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) +} + +/// 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 + // 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.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)" + ); +} + +#[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.added, 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).added, 1); + let after_first = board.active[0].description.clone(); + assert!( + reconcile_standing_markers(&mut board, &seeds).is_noop(), + "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!( + 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 (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; + 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.added, 0, + "an ordinary (standing=false) seed must never stamp a goal standing" + ); + 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!( + 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 aa335ab0d..2e9c544eb 100644 --- a/src/goal_curation/types.rs +++ b/src/goal_curation/types.rs @@ -348,10 +348,44 @@ 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 + } + + /// 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 @@ -849,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 8b15922c4..9786cb133 100644 --- a/src/identity/coverage_tests.rs +++ b/src/identity/coverage_tests.rs @@ -627,3 +627,155 @@ 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 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#" +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_eq!( + seed.standing, + Some(true), + "standing = true must round-trip from TOML as an explicit Some(true)" + ); +} + +#[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_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] +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..082d6e880 100644 --- a/src/identity/file_loader.rs +++ b/src/identity/file_loader.rs @@ -75,12 +75,22 @@ impl FileIdentityLoader { .seed_goals .iter() .map(|g| { - SeedGoal::new( + let seed = SeedGoal::new( g.priority, g.title.clone(), g.description.clone(), g.repo.clone(), - ) + ); + // #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(); @@ -1168,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 6a79f73f2..2cf0cc71c 100644 --- a/src/identity/manifest.rs +++ b/src/identity/manifest.rs @@ -24,6 +24,28 @@ 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. 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 { @@ -38,8 +60,53 @@ impl SeedGoal { title: title.into(), description: description.into(), repo, + standing: false, + standing_explicit: 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. 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 84bfd6ad4..7e686d7aa 100644 --- a/src/identity/toml_types.rs +++ b/src/identity/toml_types.rs @@ -66,6 +66,16 @@ pub(crate) struct TomlSeedGoal { pub description: String, #[serde(default)] pub repo: Option, + /// 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: 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 64c896a3f..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,6 +162,28 @@ fn run_ooda_cycle_inner( } } + // #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 recon = crate::goal_curation::reconcile_standing_markers( + &mut state.active_goals, + &resolved_seed_goals, + ); + if !recon.is_noop() { + eprintln!( + "[simard] OODA start: reconciled standing markers — {} added, {} removed (#4927)", + recon.added, recon.removed + ); + } + } + // 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..e7c342e73 100644 --- a/src/ooda_loop/tests_no_progress.rs +++ b/src/ooda_loop/tests_no_progress.rs @@ -1275,3 +1275,243 @@ 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.added, 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" + ); +} + +#[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" + ); +}