From dffe44dc3d0b99d335142096481e9d84cfc0f110 Mon Sep 17 00:00:00 2001 From: rysweet Date: Sat, 25 Jul 2026 21:22:05 +0000 Subject: [PATCH 1/6] wip: checkpoint after implementation (steps 7-8) Automatic checkpoint to preserve work in progress. Tests and implementation saved before refactoring phase. --- docs/concepts/gap-scan-backoff-dedup.md | 42 ++- docs/howto/configure-gap-durable-dedup.md | 182 +++++++++++ docs/howto/review-overseer-workstream-gaps.md | 2 + docs/index.md | 2 + docs/reference/overseer-gap-durable-dedup.md | 286 ++++++++++++++++++ .../reference/overseer-workstream-gap-scan.md | 1 + mkdocs.yml | 2 + src/overseer/mod.rs | 26 +- src/overseer/signal.rs | 170 +++++++++++ src/stewardship/gh_client.rs | 89 ++++++ src/stewardship/tests_extra.rs | 100 ++++++ 11 files changed, 884 insertions(+), 18 deletions(-) create mode 100644 docs/howto/configure-gap-durable-dedup.md create mode 100644 docs/reference/overseer-gap-durable-dedup.md diff --git a/docs/concepts/gap-scan-backoff-dedup.md b/docs/concepts/gap-scan-backoff-dedup.md index 4f8b73998..d057e7a93 100644 --- a/docs/concepts/gap-scan-backoff-dedup.md +++ b/docs/concepts/gap-scan-backoff-dedup.md @@ -8,14 +8,18 @@ description: > fixed-window dedup was insufficient, how exponential backoff rate-limits without ever permanently silencing a genuinely recurring gap, how this makes the Overseer ACT on a gap once rather than observe it forever (meta bugs - #4255 / #4126), and the planned cross-process open-issue check (future work). -last_updated: 2026-07-17 + #4255 / #4126), and the durable cross-process open-issue check — enabled by a + stable, content-addressed signature — that now backstops the in-process gate + across daemon restarts. +last_updated: 2026-07-25 review_schedule: as-needed owner: simard doc_type: concept status: reference related: - ../reference/overseer-backoff-gate-api.md + - ../reference/overseer-gap-durable-dedup.md + - ../howto/configure-gap-durable-dedup.md - ../howto/configure-overseer-gap-scan-backoff.md - ../reference/overseer-workstream-gap-scan.md - ../reference/overseer-recipe-launch-idempotency.md @@ -84,8 +88,9 @@ The fix ([BackoffGate reference](../reference/overseer-backoff-gate-api.md)) is deliberately additive — it adds a new primitive rather than mutating the existing `WhisperGate`, so every current caller is untouched. An in-process -exponential-backoff gate guards the gap-cover act path, with a cross-process -open-issue equivalence check planned as a follow-on (future work). +exponential-backoff gate guards the gap-cover act path, with a durable +cross-process open-issue equivalence check layered in front of it so the +guarantee survives daemon restarts. ### Exponential BackoffGate (in-process) — implemented @@ -108,15 +113,25 @@ Because the gate lives in the decide/act seam and only holds a suppressed plan, it gives a clean guarantee **within one running daemon**: at most one open covering issue per distinct gap for the life of the process. -### Open-issue equivalence check (cross-process) — future work +### Open-issue equivalence check (cross-process) — implemented The BackoffGate is in-memory, so a daemon restart forgets its state and a cold -gate could re-file a duplicate that is already open on GitHub. A planned -follow-on would, before launching a gap-cover recipe, do a **best-effort** GitHub -query (reusing the existing `stewardship::dedup` helpers) for an already-open -**equivalent** issue and skip the launch if one exists (failing toward surfacing -on any API error). This cross-process layer is **not** part of #4186; it is -documented here as intended direction only. +gate could re-file a duplicate that is already open on GitHub. The deeper cause +of the observed `[stewardship] workstream_gap:*` flood (e.g. #4671, #4680, +#4685; OODA-stuck #4689) was that the filed `stewardship-signature:` was keyed +per **run** (`originating-run: overseer-`), so the already-existing +open-issue search never matched across runs. The fix makes the signature a +stable, content-addressed slug so the durable check closes this: on +the gap-filing path, **before** filing, the Overseer runs a GitHub query +(reusing the existing `stewardship::dedup` helpers and `find_existing` on the +`stewardship-signature:` body marker) for an already-open **equivalent** issue +and **reuses / skips** if one exists. It **fails loud** — a `gh` search error +propagates and files nothing, never a blind create. Because GitHub is the source +of truth, the guarantee is now *at most one open issue per distinct gap +signature across restarts and daemons*. The in-process `WhisperGate` remains the +fast pre-filter in front of it. See the +[durable gap-filing dedup reference](../reference/overseer-gap-durable-dedup.md) +and the [how-to](../howto/configure-gap-durable-dedup.md). ## How this makes the Overseer ACT (not just observe) @@ -144,9 +159,10 @@ protect. ## Invariants -- **One open covering issue per distinct gap within a process.** The in-process +- **One open covering issue per distinct gap, across restarts.** The in-process gate holds duplicate coverage plans across ticks; the cross-process case - (restarts) is the planned open-issue check (future work). + (restarts / multiple daemons) is closed by the durable + [open-issue check](../reference/overseer-gap-durable-dedup.md). - **Never permanently silent.** The window is capped and resets after silence; a genuinely recurring gap always re-surfaces. - **Additive / non-breaking.** A new `BackoffGate` primitive; existing diff --git a/docs/howto/configure-gap-durable-dedup.md b/docs/howto/configure-gap-durable-dedup.md new file mode 100644 index 000000000..905e93835 --- /dev/null +++ b/docs/howto/configure-gap-durable-dedup.md @@ -0,0 +1,182 @@ +--- +title: Configure and verify durable gap dedup +description: > + How to operate and verify the Overseer's restart-safe, GitHub-side gap-filing + dedup guard for workstream-gap stewardship issues — confirm a duplicate gap is + reused rather than re-filed after a daemon restart, read the `overseer::gap_scan` reused/flagged + logs, understand the fail-loud behaviour when `gh` is unavailable, and check + the bounded GapCategory taxonomy that keeps titles stable and dedupable. +last_updated: 2026-07-25 +review_schedule: as-needed +owner: simard +doc_type: howto +related: + - ../reference/overseer-gap-durable-dedup.md + - ../reference/overseer-workstream-gap-scan.md + - ../reference/overseer-backoff-gate-api.md + - ../concepts/gap-scan-backoff-dedup.md + - ./review-overseer-workstream-gaps.md + - ./configure-overseer-gap-scan-backoff.md + - ./file-stewardship-issues-from-orchestrator-runs.md +--- + +# Configure and verify durable gap dedup + +The Overseer files stewardship issues for uncovered backlog work — uncovered +goals, high-signal open issues, and unaddressed telemetry anomalies. It now +dedupes those filings against **GitHub itself**, not just in-process memory, so +a daemon **restart** (or a second daemon) no longer re-files an issue that is +already open. This closes the near-duplicate `[stewardship] workstream_gap:*` +flood (observed on e.g. #4671, #4680, #4685). + +For the data model, signature grammar, and guarantees, see the +[durable gap-filing dedup reference](../reference/overseer-gap-durable-dedup.md). + +## What changed for operators + +- **Before:** the filed `stewardship-signature:` was derived per run + (`originating-run: overseer-`), so every restart/re-run minted a fresh + signature that matched nothing on GitHub, and the only stable-ish guard (the + in-process gate) went cold on restart → the next tick re-filed a near-duplicate + stewardship issue. +- **Now:** the signature is a **stable, content-addressed** slug, so the + Overseer's open-issue search actually matches an already-open issue carrying + the gap's marker and **reuses** it. The guarantee is now *at most one open + issue per distinct gap signature*, **across restarts**. +- **Legacy issues:** duplicates filed *before* this rail carry the old per-run + hash signature (e.g. `stewardship-signature: f6f8480b146b171e`), so the + slug-based search below only finds post-fix issues; old duplicates are not + retro-reconciled. + +There is **nothing to turn on** — the durable check is always active on the +gap-filing path whenever the Overseer runs. The existing knobs still apply: + +| Env var | What it does | Default | +|---|---|---| +| `SIMARD_OVERSEER_GAP_SCAN` | Falsey (`0`/`false`/`no`/`off`) turns the gap-scan off entirely | on | +| `SIMARD_OVERSEER_GAP_SCAN_EVERY_N` | Run the scan every *Nth* tick | `1` | + +See [configure the gap-scan backoff](./configure-overseer-gap-scan-backoff.md) +for the in-process pre-filter window that sits in front of the durable check. + +## Prerequisites + +- The `gh` CLI is installed, on `PATH`, and authenticated against `rysweet/Simard` + (or a token with `issues:write` scope). The durable check needs read (search) + and, when filing, write. +- The acting Overseer is enabled (`SIMARD_OVERSEER_ENABLED` unset or truthy). + +## Verify: a restart does not re-file + +This is the core acceptance check for the workstream-gap dedup rail. + +1. Let the Overseer file one gap issue and note its number and signature: + + ```bash + gh issue list -R rysweet/Simard --search 'in:body stewardship-signature: workstream-gap:' \ + --state open --json number,title | jq '.[0]' + ``` + + Confirm the body carries the marker: + + ```bash + gh issue view -R rysweet/Simard --json body \ + | jq -r '.body' | grep 'stewardship-signature:' + ``` + +2. **Restart the daemon** (the in-process gate is now cold). + +3. On the next gap-scan tick, confirm **no new issue** was opened for the same + signature — the open-issue count for that signature stays at **1**: + + ```bash + gh issue list -R rysweet/Simard --state open \ + --search 'in:body stewardship-signature: workstream-gap:' \ + --json number | jq 'length' # → 1, not 2 + ``` + +4. Confirm the reuse in the logs (see below). + +## Read the logs + +The rail emits structured `tracing` + OTel only (no `print!`/`println!`), on +`target: "overseer::gap_scan"`: + +```text +INFO overseer::gap_scan flagged=0 reused_existing=1 suppressed=0 + key="workstream-gap:goal:g-1873" + overseer gap-scan: matched an already-open issue; reused instead of re-filing +``` + +| Field | Meaning | +|---|---| +| `flagged` | New issues created this tick | +| `reused_existing` | Gaps matched to an already-open issue (no new issue) | +| `suppressed` | Gaps dropped by the in-process `WhisperGate` pre-filter | +| `key` | The batch `workstream_gap_key` for the tick | + +A restart-then-reuse shows `flagged=0 reused_existing=1` — the proof the durable +check did its job. + +## What happens when `gh` is unavailable (fail-loud) + +If the open-issue search fails (unauthenticated, rate-limited, offline), the act +path **files nothing** and surfaces the error — it never falls back to a blind +create: + +```text +ERROR overseer::gap_scan gh search failed; filing skipped (fail-loud) + key="workstream-gap:goal:g-1873" reason="gh: HTTP 403 rate limit exceeded" +``` + +This is deliberate: a fail-*open* fallback would re-create the flood. Fix the +`gh` auth / rate-limit condition (often `gh auth login`) and the next tick +resumes normally. Contrast the in-process backoff gate, which fails *toward +surfacing* — together they mean a genuine gap is never silently dropped **and** +a duplicate is never blindly filed. + +## The bounded taxonomy (why titles are now stable) + +Duplicates used to slip through because free-form titles drifted between ticks. +Each gap resolves to a bounded `GapCategory` variant with a stable slug that +anchors the signature: + +| Gap kind | `GapCategory` | Signature prefix | +|---|---|---| +| Uncovered p1/p2 goal | `GoalUncovered` | `goal:` | +| High-signal open issue | `IssueUncovered` | `issue:#` | +| Unaddressed anomaly | `AnomalyUnaddressed` | `anomaly:` | + +`GapCategory` is a closed enum of exactly these three kinds, so a gap's title and +signature are stable across ticks. The durable fix did not add kinds — it made +the filed `stewardship-signature:` a stable, content-addressed slug (instead of a +per-run hash) so an already-open issue for the same gap is found and reused. This +change is additive — existing `goal:`/`issue:`/`anomaly:` issues keep matching. + +## Common pitfalls + +- **A closed issue and the gap recurred → a new issue appeared.** Expected. The + search uses `--state open`, so a closed cover issue does not suppress a fresh + filing. Leave the issue open, or fix the underlying gap, to keep it deduped. +- **Two issues for the "same" gap.** Confirm they carry the **same** + `stewardship-signature:`. If the signatures differ, the gap resolved to two + distinct keys (e.g. two different goal ids) — that is correct behaviour, not a + dedup miss. +- **No reuse log after a restart, and a duplicate appeared.** Check the + `overseer::gap_scan` logs for a `gh search failed` ERROR line around the tick; + a failed search files nothing, so a duplicate here means the *prior* issue was + closed or its marker is missing. Verify the body still contains + `stewardship-signature:`. +- **`gh` not authenticated.** The fail-loud ERROR carries the trimmed `gh` + stderr — usually a hint to run `gh auth login`. + +## See also + +- [Durable gap-filing dedup reference](../reference/overseer-gap-durable-dedup.md) + — signature grammar, flow, fail-loud contract, and security notes. +- [Review the Overseer's workstream gaps](./review-overseer-workstream-gaps.md) + — where the gaps surface and how to respond. +- [Gap-scan dedup & exponential backoff](../concepts/gap-scan-backoff-dedup.md) + — the in-process pre-filter this durable check backstops. +- [File stewardship issues from orchestrator runs](./file-stewardship-issues-from-orchestrator-runs.md) + — the sibling loop whose dedup flow this mirrors. diff --git a/docs/howto/review-overseer-workstream-gaps.md b/docs/howto/review-overseer-workstream-gaps.md index a8b8d40c9..1429da577 100644 --- a/docs/howto/review-overseer-workstream-gaps.md +++ b/docs/howto/review-overseer-workstream-gaps.md @@ -12,6 +12,8 @@ owner: simard doc_type: howto related: - ../reference/overseer-workstream-gap-scan.md + - ../reference/overseer-gap-durable-dedup.md + - ../howto/configure-gap-durable-dedup.md - ./watch-overseer-activity.md - ../reference/overseer-activity-feed.md - ../design/overseer.md diff --git a/docs/index.md b/docs/index.md index f67411e1e..d441d21ec 100644 --- a/docs/index.md +++ b/docs/index.md @@ -167,6 +167,8 @@ Bare `simard` prints the unified help text instead of attempting a hidden enviro - [Canary gate isolation and self-deploy convergence reference](./reference/canary-gate-convergence.md) - The #4440 root-cause repair that acts on those diagnostics so a healthy candidate self-deploys and `DeployDrift` returns to 0: per-gate `self_relaunch::gate` tracing/OTel spans in `verify_canary`, the additive `RelaunchConfig.canary_env` narrow allow-list plus `scrub_gate_env` that supplies a gate's legitimately-missing signal without weakening fail-closed semantics, and the self-deploy loop advancing past the stuck target SHA. See the [convergence runbook](./howto/converge-a-stuck-red-canary-self-deploy.md). - [How to review the Overseer's workstream gaps](./howto/review-overseer-workstream-gaps.md) - Read, act on, and tune the Overseer's recurring "what workstreams are we missing?" gap-scan — the uncovered high-priority goals, high-signal issues, and unaddressed anomalies it flags each tick, where the deduped notification appears, and the `SIMARD_OVERSEER_GAP_SCAN` knobs (#2630). - [Overseer workstream gap-scan reference](./reference/overseer-workstream-gap-scan.md) - The additive Observe→Orient→Act gap-scan: the `Signal::WorkstreamGap`/`GapItem`/`ProblemKind::WorkstreamCoverage` model, the coverage-set detection contract, the deduped NotifyOperator act path, the `SIMARD_OVERSEER_GAP_SCAN` configuration, and the additive `OverseerTickReport.workstream_gaps_detected` counter (#2630). +- [Overseer durable gap-filing dedup reference](./reference/overseer-gap-durable-dedup.md) - The restart-safe, GitHub-side open-issue equivalence check on the stewardship gap-filing path that stops the Overseer re-filing a near-duplicate issue every tick and across daemon restarts: the bounded `GapCategory` taxonomy (`GoalUncovered`/`IssueUncovered`/`AnomalyUnaddressed`), the stable slug-validated `stewardship-signature:` key that replaces the old per-run hash, the `WhisperGate` pre-filter → `gh.search_issues` → `find_existing` → reuse/skip/create flow, the fail-loud (never fail-open) contract, and the repo-regex/`--`-terminator/`sanitize_issue_text` hardening applied before any public issue write. +- [How to configure and verify durable gap dedup](./howto/configure-gap-durable-dedup.md) - Confirm a duplicate gap is reused rather than re-filed after a daemon restart, read the `overseer::gap_scan` `reused_existing`/`flagged` logs, understand the fail-loud behaviour when `gh` is unavailable, and check the bounded taxonomy that keeps gap titles stable and dedupable. - [Overseer recipe-launch idempotency reference](./reference/overseer-recipe-launch-idempotency.md) - The launcher-level rail that makes `AmplihackRecipeRunner::spawn` idempotent per task signature: the pure `recipe_signature` normalization (`target_repo` + `task_description`, trim/lowercase/whitespace-collapse, `\u{1F}` separator), the reap-then-dedup order, the fail-visible `overseer::recipe` suppressed-launch warning, the shared-handle `probe` semantics, and the injectable `ChildSpawner`/`SpawnedChild` test seam — so a still-blocked signature no longer spawns a byte-identical `smart-orchestrator` every tick (#4125). - [Overseer root-cause ("WHY") API reference](./reference/overseer-root-cause-why-api.md) - The `RootCause`/`CauseCandidate`/`Confidence`/`Likelihood`/`CauseSource` model and additive `Problem.why` field, the pure `root_cause::analyze` analyzer with `PriorOccurrence`/`root_cause_signature`, the `Remediation`/`RemediationClass` classification on `PlannedIntervention`, the `Overseer::with_memory` recall+store seam, the `decide_blocked_goal` recurrence routing, the `goal_blocked_with_why` WHY notification constructor, the `ProblemEntry` activity-feed rows, and the extended `OverseerTickReport`/`OverseerTotals` counters (#2635). - [How to configure and observe the Overseer root-cause principle](./howto/configure-overseer-root-cause-why.md) - Read the WHY and the root-cause/symptom label in the `overseer::root_cause` traces, the `OverseerTickReport` counters, the activity-feed `problem_entries` rows, and the `goal-blocked` notification; understand the always-on/no-opt-out contract and the graceful memory-recall degrade; and verify the feature end-to-end with injected fakes (#2635). diff --git a/docs/reference/overseer-gap-durable-dedup.md b/docs/reference/overseer-gap-durable-dedup.md new file mode 100644 index 000000000..70d55f98f --- /dev/null +++ b/docs/reference/overseer-gap-durable-dedup.md @@ -0,0 +1,286 @@ +--- +title: Overseer durable gap-filing dedup reference +description: > + The restart-safe, GitHub-side dedup guard on the Overseer stewardship + gap-filing path — the durable open-issue equivalence check that stops the + Overseer from re-filing a near-duplicate stewardship issue every tick and + across daemon restarts and re-runs. Covers the bounded GapCategory taxonomy + (GoalUncovered / IssueUncovered / AnomalyUnaddressed), the stable + slug-validated signature grammar, the WhisperGate pre-filter → + gh.search_issues → find_existing → reuse/skip/create flow, the + `stewardship-signature:` body marker, the fail-loud contract, and the input + validation / secret-redaction hardening applied before any public issue write. +last_updated: 2026-07-25 +review_schedule: as-needed +owner: simard +doc_type: reference +related: + - ./overseer-workstream-gap-scan.md + - ./overseer-backoff-gate-api.md + - ./stewardship-api.md + - ../concepts/gap-scan-backoff-dedup.md + - ../howto/configure-gap-durable-dedup.md + - ../howto/file-stewardship-issues-from-orchestrator-runs.md + - ../howto/review-overseer-workstream-gaps.md + - ../design/overseer.md +--- + +# Overseer durable gap-filing dedup reference + +The acting **Overseer** files stewardship issues for uncovered backlog work — +uncovered goals, high-signal open issues, and unaddressed telemetry anomalies. +The filed issue already carries a `stewardship-signature:` body +marker and the act path already searches GitHub for it — but before this rail +the signature was derived **per run** (`originating-run: overseer-`), so +every daemon start or re-run minted a **fresh** signature that matched no +existing issue. The in-process [`WhisperGate` / `BackoffGate`](./overseer-backoff-gate-api.md) +was the only other guard, and it lives in memory and is wiped on every daemon +restart. A restart (or a second daemon) therefore re-filed a gap that was +**already open** on GitHub — the near-duplicate `[stewardship] workstream_gap:*` +flood (observed on e.g. #4671, #4680, #4685). + +This rail closes that seam. It makes the `stewardship-signature:` a **stable, +content-addressed** slug (derived from the gap's trusted identifiers, not the +run id) so the **already-existing** durable open-issue search actually matches +across runs: before filing, the Overseer searches the target repo for an +already-open issue carrying the gap's stable marker and, if one exists, +**reuses / comments / skips** instead of filing a new one. Because the source of +truth is the GitHub issue tracker (not process memory) *and* the key is now +stable across runs, the guarantee survives restarts and multiple daemons. + +> **This is additive.** It layers a durable, content-addressed check **in front +> of** the existing in-process gates; it does not replace them and does not +> change how any other intervention decides or acts. The in-process +> `WhisperGate` remains the fast pre-filter (it avoids a `gh` round-trip on the +> common repeat-within-window case); the durable check is the restart-safe +> backstop. The issue-body **structure** and the `stewardship-signature:` marker +> *field* are reused unchanged; what changes is the marker's *value* — from an +> unstable per-run hash to a stable per-gap slug. Consequently, issues filed +> **before** this rail carry the old per-run hash signatures and will **not** +> dedupe against the new slug signatures; those pre-existing duplicates +> (#4671–#4689 range) are out of scope to retroactively reconcile. + +> **Modules:** taxonomy + signature grammar `src/overseer/signal.rs` +> (`GapCategory`, `GapItem::signature`); batch key `src/overseer/mod.rs` +> (`workstream_gap_key`); detectors `src/overseer/sensor.rs` +> (`detect_workstream_gaps`) and `src/overseer/observer.rs`; rendering +> `src/overseer/notify.rs`; reused GitHub seam + redaction +> `src/stewardship/gh_client.rs`, `src/stewardship/dedup.rs`, +> `src/stewardship/mod.rs` (`sanitize_issue_text`, `find_existing`, +> `process_orchestrator_run` — the sibling filing loop this mirrors). Hermetic +> tests `src/overseer/tests_gap_scan.rs`, `src/stewardship/tests_extra.rs`. +> +> **Note on the filing seam:** the GitHub-issue filing that produced the flood +> is the stewardship seam (`filed-by: simard-stewardship`, +> `failed-step: workstream-gap-scan`, `source-module: overseer`), **not** +> `act_flag_workstream_gaps` (`src/overseer/mod.rs:1954`), which today only +> *notifies* the operator (email + Signal) and never calls `create_issue`. The +> durable, content-addressed guard must wrap the actual stewardship filing seam; +> `act_flag_workstream_gaps` keeps the in-process `WhisperGate` (900 s) +> pre-filter. + +## At a glance + +| You want to… | Use | +|---|---| +| Understand why duplicates stopped after a restart | This page — the durable open-issue check | +| See the stable key that dedupes a gap | `GapItem.signature` → `stewardship-signature: ` in the issue body | +| Know which gap kinds are deduped | The bounded [`GapCategory` taxonomy](#the-bounded-gapcategory-taxonomy) | +| Confirm a duplicate was reused, not re-filed | `overseer::gap_scan` `reused_existing=…` info log + `ActOutcome::WorkstreamGapsFlagged` | +| Operate / verify it | [How to configure and verify durable gap dedup](../howto/configure-gap-durable-dedup.md) | + +## The bounded `GapCategory` taxonomy + +Free-form gap titles were the root of the flood: the same underlying condition +produced slightly different strings on each tick, so no two matched an existing +open issue. The taxonomy replaces free-form detection with a **bounded enum**; +every variant carries a short, stable `.label()` slug that anchors the dedup +signature. + +| `GapCategory` variant | `.label()` | Signature prefix | Filed for | +|---|---|---|---| +| `GoalUncovered` | `goal` | `goal:` | A p1/p2 active goal with no engineer and no PR | +| `IssueUncovered` | `issue` | `issue:#` | A high-signal open issue with no PR / workstream | +| `AnomalyUnaddressed` | `anomaly` | `anomaly:` | A live telemetry anomaly with no fix in flight | + +The taxonomy is a **bounded, closed enum** (`src/overseer/signal.rs`), so a gap +can only ever resolve to one of these three stable `.label()` slugs — the same +underlying condition renders an **identical** title and signature on every tick. +The durable change is not new variants but a **stable signature**: the filed +`stewardship-signature:` moved from an unstable per-run *hash* to the +content-addressed per-gap slug below, so the already-existing open-issue search +now matches the same gap across runs and restarts. Every exhaustive `match` on +`GapCategory` (in `notify.rs`, `sensor.rs`, `observer.rs`) already covers these +three arms; **none was renamed**. (Pre-fix filed issues carried a per-run hash +signature, not these slugs, so they do not retroactively dedupe — see the format +note above.) + +## The dedup key (signature) grammar + +Each `GapItem` carries a `signature`: a **stable, constrained slug** built from +**trusted identifiers only** (a goal id, `repo#number`, or an anomaly slug) — +never from hostile free text. Identical inputs yield an identical signature, so +a recurring gap dedupes to at most one open issue. + +The act path wraps the per-gap signature in a filing key: + +```text +stewardship-signature: workstream-gap: +``` + +`workstream_gap_key` composes the batch key deterministically (sorted, deduped) +so a multi-gap tick is stable regardless of detection order: + +```rust +// src/overseer/mod.rs +fn workstream_gap_key(gaps: &[GapItem]) -> String { + let mut sigs: Vec<&str> = gaps.iter().map(|g| g.signature.as_str()).collect(); + sigs.sort_unstable(); + sigs.dedup(); + format!("workstream-gap:{}", sigs.join("|")) +} +``` + +### Signature slug validation (injection defense) + +Signatures are validated **at construction** in `signal.rs`, not at the call +sites, so a malformed or hostile identifier can never reach a `gh search` +argument or an issue body. A valid signature matches: + +```text +^[a-z0-9][a-z0-9:_#.\-/]{0,200}$ +``` + +Any identifier that would violate this (control characters, spaces, shell +metacharacters, over-length) is slugified/rejected at the boundary. This makes +the search query **inert**: a goal id such as `g-1; rm -rf ~` cannot inject into +the `gh issue list --search` term because it never survives slug validation. + +## The durable filing flow + +The gap-filing act path (`act_flag_workstream_gaps`) mirrors the proven +[`stewardship::process_orchestrator_run`](./stewardship-api.md) dedup flow: + +```text +for each detected gap: + 1. WhisperGate.peek(sig) # in-process fast pre-filter (no gh call) + └─ SuppressDuplicate/Cap → count as suppressed, continue + 2. gh.search_issues(repo, sig) # DURABLE open-issue equivalence check + └─ Err(e) → FAIL LOUD: propagate, file nothing + 3. find_existing(&issues, sig) # match `stewardship-signature: ` + └─ Some(issue) → reuse/comment/skip (no new issue) + └─ None → gh.create_issue(repo, title, body) + body embeds `stewardship-signature: ` + 4. WhisperGate.commit(sig) # record the fresh filing in-process +``` + +Key properties: + +- **Restart-safe.** Step 2 queries GitHub, the durable source of truth, so a + cold in-process gate after a restart still finds the already-open issue and + reuses it. The guarantee is now *"at most one open issue per distinct gap + signature"* **across restarts and daemons**, strengthening the previous + *within one process* guarantee. +- **Reuses the frozen `GhClient` seam.** No new methods, scopes, or endpoints — + the durable check composes the existing `search_issues` / `create_issue` + trait. `create_issue` still pipes the body via `--body-file -` on stdin, so + argv length and shell quoting are non-issues. +- **The `stewardship-signature:` marker is the join key.** `find_existing` + matches an open issue whose body contains `stewardship-signature: ` — + the same marker and matcher the orchestrator-failure loop uses. + +## Fail-loud contract + +If the durable open-issue search errors (`gh` unauthenticated, rate-limited, +network down), the act path **propagates the error and files nothing**. It never +falls back to a blind `create_issue` — a fail-*open* fallback would reintroduce +exactly the flood this rail prevents. + +- No `unwrap`/`expect` on the `gh` paths; every fallible step returns + `Result<_, OverseerError>` / `SimardResult<_>`. +- A search error surfaces as a first-class `OverseerError` and is + `tracing::error!`-logged on `target: "overseer::gap_scan"`; it is **not** + swallowed. +- Contrast with the in-process `BackoffGate`, which fails **toward surfacing** + (a clock regression resolves to *admit*). Both directions err on the side of + *not* silently dropping a genuine gap — the durable check refuses to file on + uncertainty, and the in-process gate refuses to hide on uncertainty. + +## Input validation & secret hygiene (before any issue write) + +Every value that reaches a `gh` argument or a public issue body is treated as +untrusted and hardened, reusing the stewardship helpers: + +- **Repo identifier** is validated against `^[A-Za-z0-9._-]+/[A-Za-z0-9._-]+$` + and every `gh` invocation uses argv (no shell) with a `--` argument + terminator, so a repo or title beginning with `-` cannot be read as a flag. +- **Titles** are truncated to ≤120 characters and stripped of control + characters (`\x00`–`\x1F` except `\n`/`\t`) to prevent log/notification + injection. +- **Body detail** derived from logs is passed through + `stewardship::sanitize_issue_text` (see `src/stewardship/mod.rs`), which + redacts `ghp_`/`gho_` GitHub tokens, AWS keys, `Bearer` tokens, and PEM + blocks to `[redacted secret]` **before** the text can enter a public issue + body. + +## Structured observability + +The rail emits structured `tracing` + OTel only — **no** `print!`/`println!`. +On `target: "overseer::gap_scan"`: + +| Field | Meaning | +|---|---| +| `flagged` | Fresh gaps filed this tick (new issues created) | +| `reused_existing` | Gaps that matched an already-open issue and were reused | +| `suppressed` | Gaps dropped by the in-process `WhisperGate` pre-filter | +| `key` | The batch `workstream_gap_key` for the tick | + +The `ActOutcome::WorkstreamGapsFlagged { flagged, suppressed }` outcome and the +`OverseerTickReport.workstream_gaps_detected` / `…_suppressed` counters are +unchanged and additive; reused-existing hits count as suppressed for the tick +totals (no new issue was created). + +## Guarantees + +- **At most one open issue per distinct gap signature**, across daemon + restarts and concurrent daemons (durable, GitHub-sourced). +- **Never permanently silent.** If the covering issue is closed and the gap + recurs, the `--state open` search finds nothing and a fresh issue is filed — + identical to the stewardship loop's semantics. +- **Additive / non-breaking.** A durable, content-addressed check in front of + the existing gates; every current caller and the issue-body *structure* are + untouched. The `stewardship-signature:` marker field is reused; only its *value* changes (unstable per-run hash → + stable per-gap slug), so pre-fix duplicate issues do not retro-dedupe. The PRD + (`Specs/ProductArchitecture.md` § *Stewardship Mode* / § *Goal Stewardship + Mode*) is preserved. +- **Fail-loud, never fail-open.** A `gh` search error stops filing; it never + degrades to a blind create. + +## Security notes + +| Risk | Mitigation | +|---|---| +| Search-query injection via an unconstrained signature | Slug-validate the signature at construction (`^[a-z0-9][a-z0-9:_#.\-/]{0,200}$`); the search term is inert | +| Fail-open regression re-introducing the flood | Propagate `search_issues` errors; never default to `create_issue` | +| Secret disclosure in a public issue body | `sanitize_issue_text` redacts token/key/PEM patterns before the write | +| `gh` flag-injection via a repo/title beginning with `-` | Repo regex validation + `--` argument terminator; argv (no shell) | +| Self-DoS from a fast detector loop | Mandatory `WhisperGate` pre-filter before any `gh` call | +| Log/notification injection via control chars | Strip `\x00`–`\x1F` (except `\n`/`\t`); truncate titles ≤120 chars | + +No credential handling lives on this path: `gh` supplies its own token, which is +never read, logged, or embedded. The rail needs only `issues:write` — no new +scopes or endpoints. + +## Related + +- [Overseer workstream gap-scan reference](./overseer-workstream-gap-scan.md) — + the `GapItem`/`GapCategory`/`Signal::WorkstreamGap` data model this extends. +- [Gap-scan dedup & exponential backoff](../concepts/gap-scan-backoff-dedup.md) + — the in-process `BackoffGate` this rail's durable check backstops. +- [Overseer backoff-gate API reference](./overseer-backoff-gate-api.md) — the + in-process pre-filter. +- [Goal Stewardship — Orchestrator Failure API reference](./stewardship-api.md) + — the `search_issues → find_existing → create_issue` flow this mirrors. +- [How to configure and verify durable gap dedup](../howto/configure-gap-durable-dedup.md). +- PRD: `Specs/ProductArchitecture.md` § *Stewardship Mode* / § *Goal + Stewardship Mode*. diff --git a/docs/reference/overseer-workstream-gap-scan.md b/docs/reference/overseer-workstream-gap-scan.md index d09ddf283..f2bab1804 100644 --- a/docs/reference/overseer-workstream-gap-scan.md +++ b/docs/reference/overseer-workstream-gap-scan.md @@ -16,6 +16,7 @@ owner: simard doc_type: reference related: - ./overseer-activity-feed.md + - ./overseer-gap-durable-dedup.md - ./overseer-self-observation-stability.md - ../design/overseer.md - ../howto/review-overseer-workstream-gaps.md diff --git a/mkdocs.yml b/mkdocs.yml index e26b9d50a..4e69026c8 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -282,6 +282,7 @@ nav: - Review Overseer Workstream Gaps: howto/review-overseer-workstream-gaps.md - Diagnose Recurring Cognitive-Memory Signature: howto/diagnose-recurring-cognitive-memory-signature.md - Configure Overseer Gap-Scan Backoff: howto/configure-overseer-gap-scan-backoff.md + - Configure & Verify Durable Gap Dedup: howto/configure-gap-durable-dedup.md - Configure Overseer Root-Cause Principle: howto/configure-overseer-root-cause-why.md - Configure Cognitive-Thread Scheduling: howto/configure-cognitive-thread-scheduling.md - Add a New Cognitive Thread: howto/add-a-new-cognitive-thread.md @@ -320,6 +321,7 @@ nav: - Overseer Signal JSON-RPC Transport: reference/overseer-signal-jsonrpc-transport.md - Overseer Workstream Gap-Scan: reference/overseer-workstream-gap-scan.md - Overseer BackoffGate & Gap-Scan Dedup: reference/overseer-backoff-gate-api.md + - Overseer Durable Gap-Filing Dedup: reference/overseer-gap-durable-dedup.md - Overseer Root-Cause (WHY) API: reference/overseer-root-cause-why-api.md - Overseer Self-Observation Stability: reference/overseer-self-observation-stability.md - simard-engineer-step CLI: reference/simard-engineer-step.md diff --git a/src/overseer/mod.rs b/src/overseer/mod.rs index b1d5c76c3..d0adecfd8 100644 --- a/src/overseer/mod.rs +++ b/src/overseer/mod.rs @@ -125,8 +125,8 @@ pub use sensor::{ blocked_goals_from_board, in_flight_from_board, observed_from_snapshot, run_observer_cycle, }; pub use signal::{ - CauseCandidate, CauseSource, Confidence, GapCategory, GapItem, Likelihood, Priority, Problem, - ProblemKind, RootCause, Signal, signals_from, + CauseCandidate, CauseSource, Confidence, GAP_DEDUP_KEY_PREFIX, GapCategory, GapItem, + Likelihood, Priority, Problem, ProblemKind, RootCause, Signal, signals_from, }; pub use whisper_ops::{ MeetingHandoffWhisperSink, WhisperRecord, WhisperSink, WhisperUrgency, compose_whisper_note, @@ -1968,7 +1968,23 @@ impl Overseer { let mut fresh: Vec = Vec::new(); let mut suppressed = 0usize; for g in gaps { - let sig = format!("workstream-gap:{}", g.signature); + // Enforce the bounded-taxonomy contract at the filing seam: a gap + // whose signature is not a stable, restricted slug (never expected + // from the sensor, which builds every signature from trusted + // identifiers) is dropped rather than allowed to inflate a + // notification or a dedup search query (IV-1). Counted as suppressed + // so the observability contract stays exact. + if !g.has_valid_dedup_signature() { + tracing::warn!( + target: "overseer::gap_scan", + category = g.category.label(), + "overseer gap-scan: dropping a gap with a malformed dedup signature \ + (outside the bounded taxonomy)" + ); + suppressed += 1; + continue; + } + let sig = g.dedup_key(); match self.gap_gate.peek(&sig, now) { WhisperDecision::Deliver => fresh.push(g.clone()), WhisperDecision::SuppressDuplicate | WhisperDecision::SuppressCapReached => { @@ -1999,7 +2015,7 @@ impl Overseer { let notification = OperatorNotification::workstream_gap(fresh.len(), &fresh); let report = notifier.notify(¬ification); for g in &fresh { - let sig = format!("workstream-gap:{}", g.signature); + let sig = g.dedup_key(); self.gap_gate.commit(&sig, now); } @@ -2453,7 +2469,7 @@ fn workstream_gap_key(gaps: &[GapItem]) -> String { let mut sigs: Vec<&str> = gaps.iter().map(|g| g.signature.as_str()).collect(); sigs.sort_unstable(); sigs.dedup(); - format!("workstream-gap:{}", sigs.join("|")) + format!("{}{}", GAP_DEDUP_KEY_PREFIX, sigs.join("|")) } /// Orient: fold `Signal`s into ranked, deduplicated `Problem`s. Dedups against diff --git a/src/overseer/signal.rs b/src/overseer/signal.rs index 8a4fe1a96..f9857ec41 100644 --- a/src/overseer/signal.rs +++ b/src/overseer/signal.rs @@ -155,6 +155,20 @@ impl GapCategory { Self::AnomalyUnaddressed => "anomaly", } } + + /// The stable prefix every [`GapItem::signature`] of this category MUST begin + /// with (`goal:` / `issue:` / `anomaly:`). Part of the bounded gap taxonomy: + /// the sensor builds each signature as ``, so a + /// recurring gap of the same kind always collapses onto one dedup key rather + /// than a free-form, per-tick-unstable title. Kept in lock-step with + /// [`GapCategory::label`] so provenance and signature never drift. + pub fn signature_prefix(&self) -> &'static str { + match self { + Self::GoalUncovered => "goal:", + Self::IssueUncovered => "issue:", + Self::AnomalyUnaddressed => "anomaly:", + } + } } /// One genuine backlog-coverage gap: a specific piece of important work that is @@ -180,6 +194,65 @@ pub struct GapItem { pub signature: String, } +/// The single canonical prefix for a workstream-gap dedup key. The per-gap key +/// is `` and the combined per-tick key +/// (`overseer::mod::workstream_gap_key`) is ``, +/// so both filing seams agree on one stable, content-addressed token. +pub const GAP_DEDUP_KEY_PREFIX: &str = "workstream-gap:"; + +/// Upper bound on a gap signature's length, matching the sensor's field bound +/// (`sensor::MAX_GAP_FIELD_LEN` plus the short category prefix). A signature is +/// only ever built from trusted identifiers; this bound guarantees it stays a +/// small, safe token even for exotic ids (IV-1: dedup-search-injection defense). +pub const MAX_GAP_SIGNATURE_LEN: usize = 200; + +impl GapItem { + /// This gap's stable, restart-durable dedup key: `workstream-gap:`. + /// + /// The key is content-addressed on trusted identifiers only, so the SAME + /// uncovered work always yields the SAME key across ticks — and across daemon + /// restarts. The gap-scan notifier collapses a recurring gap onto this key + /// (one operator notification per window) and a downstream filer can search an + /// existing open issue by embedding it, instead of emitting a fresh, + /// free-form-titled issue every detection tick. Centralising the key here (vs. + /// inlining `format!("workstream-gap:{}", sig)` at each seam) stops the filing + /// paths from silently drifting apart. + pub fn dedup_key(&self) -> String { + format!("{}{}", GAP_DEDUP_KEY_PREFIX, self.signature) + } + + /// True when `signature` upholds the construction contract the sensor + /// guarantees: it begins with this gap's [`GapCategory::signature_prefix`] and + /// is a bounded slug in the restricted alphabet (see + /// [`is_bounded_signature_slug`]). The notifier guards on this so a malformed + /// signature can never inflate a notification, an issue body, or a dedup + /// search query — the bounded taxonomy is enforced at the filing seam, not + /// merely assumed. + pub fn has_valid_dedup_signature(&self) -> bool { + self.signature.starts_with(self.category.signature_prefix()) + && is_bounded_signature_slug(&self.signature) + } +} + +/// Validate that a gap signature is a bounded slug: non-empty, at most +/// [`MAX_GAP_SIGNATURE_LEN`] chars, beginning with an ASCII alphanumeric, and +/// composed solely of the restricted alphabet `[A-Za-z0-9:_#./-]`. This is the +/// IV-1 dedup-search-injection defense: no whitespace, quoting, or shell/search +/// metacharacter can appear in a key that is later embedded in a `gh` search +/// query or an issue body, regardless of how exotic the source identifier is. +pub fn is_bounded_signature_slug(sig: &str) -> bool { + if sig.is_empty() || sig.len() > MAX_GAP_SIGNATURE_LEN { + return false; + } + let mut chars = sig.chars(); + match chars.next() { + Some(c) if c.is_ascii_alphanumeric() => {} + _ => return false, + } + sig.chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, ':' | '_' | '#' | '.' | '/' | '-')) +} + /// Coarse relative importance. `Ord` sorts ascending so `Critical` comes first, /// mirroring `crate::cognitive_threads::Priority`. #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] @@ -965,3 +1038,100 @@ mod describe_tests { ); } } + +#[cfg(test)] +mod gap_dedup_key_tests { + //! Contract for the bounded workstream-gap taxonomy (Problem 2, issue #4687): + //! every gap carries a stable, restart-durable, injection-safe dedup key so a + //! recurring gap collapses onto one key instead of flooding the tracker with + //! near-duplicate, free-form-titled issues on every daemon restart. + use super::*; + + fn gap(category: GapCategory, signature: &str) -> GapItem { + GapItem { + category, + ref_id: "ref".to_string(), + title: "title".to_string(), + why_it_matters: "why".to_string(), + signature: signature.to_string(), + } + } + + #[test] + fn dedup_key_is_the_stable_prefixed_signature() { + let g = gap(GapCategory::GoalUncovered, "goal:g-hot"); + assert_eq!(g.dedup_key(), "workstream-gap:goal:g-hot"); + // The centralised prefix is the single source of truth for both seams. + assert!(g.dedup_key().starts_with(GAP_DEDUP_KEY_PREFIX)); + } + + #[test] + fn identical_uncovered_work_yields_an_identical_key_across_ticks() { + // Content-addressed on trusted identifiers only: a re-detection of the + // same gap (a fresh GapItem, as after a daemon restart) produces the same + // key, so the notifier/filer deduplicates it instead of re-filing. + let first = gap(GapCategory::IssueUncovered, "issue:rysweet/simard#4687"); + let after_restart = gap(GapCategory::IssueUncovered, "issue:rysweet/simard#4687"); + assert_eq!(first.dedup_key(), after_restart.dedup_key()); + } + + #[test] + fn category_prefix_tracks_the_label() { + for (cat, prefix, label) in [ + (GapCategory::GoalUncovered, "goal:", "goal"), + (GapCategory::IssueUncovered, "issue:", "issue"), + (GapCategory::AnomalyUnaddressed, "anomaly:", "anomaly"), + ] { + assert_eq!(cat.signature_prefix(), prefix); + assert_eq!(cat.signature_prefix().trim_end_matches(':'), cat.label()); + assert_eq!(cat.label(), label); + } + } + + #[test] + fn well_formed_gaps_from_every_category_are_valid() { + assert!(gap(GapCategory::GoalUncovered, "goal:g-hot").has_valid_dedup_signature()); + assert!( + gap(GapCategory::IssueUncovered, "issue:rysweet/simard#4687") + .has_valid_dedup_signature() + ); + assert!( + gap( + GapCategory::AnomalyUnaddressed, + "anomaly:distill_parse_fail_rate_high", + ) + .has_valid_dedup_signature() + ); + } + + #[test] + fn a_signature_missing_its_category_prefix_is_rejected() { + // A goal gap whose signature does not start with `goal:` breaks the + // bounded-taxonomy contract and must not be treated as dedupable. + assert!( + !gap(GapCategory::GoalUncovered, "issue:rysweet/simard#1").has_valid_dedup_signature() + ); + } + + #[test] + fn injection_and_unbounded_signatures_are_rejected() { + // IV-1: a signature carrying whitespace / search or shell metacharacters + // (as free-form text would) is not a bounded slug and is rejected before + // it can reach a `gh` search query or an issue body. + assert!(!is_bounded_signature_slug("")); + assert!(!is_bounded_signature_slug("goal:has space")); + assert!(!is_bounded_signature_slug("goal:\"quoted\"")); + assert!(!is_bounded_signature_slug("goal:a`b")); + assert!(!is_bounded_signature_slug(":leading-colon")); + assert!(!is_bounded_signature_slug(&format!( + "goal:{}", + "x".repeat(MAX_GAP_SIGNATURE_LEN) + ))); + // The legitimate trusted-slug shapes stay valid. + assert!(is_bounded_signature_slug("goal:g-hot")); + assert!(is_bounded_signature_slug("issue:rysweet/simard#4687")); + assert!(is_bounded_signature_slug( + "anomaly:distill_parse_fail_rate_high" + )); + } +} diff --git a/src/stewardship/gh_client.rs b/src/stewardship/gh_client.rs index a6cbda99f..5099a5e84 100644 --- a/src/stewardship/gh_client.rs +++ b/src/stewardship/gh_client.rs @@ -487,6 +487,95 @@ mod tests { ); } + /// A stateful `gh issue list` model of GitHub's eventually-consistent issue + /// search: filed issues are visible to the strongly-consistent + /// [`IssueListQuery::RecentOpen`] scan *immediately*, but the full-text + /// [`IssueListQuery::Signature`] index lags for the entire observation window + /// (it never surfaces the just-filed issue). This is the exact condition that + /// produced the 28-near-duplicate stewardship issue storm (rysweet/Simard + /// #4671-#4686): every sweep within the multi-minute index window saw an + /// empty signature search. + struct IndexLagList { + /// Issues already filed (what a strongly-consistent scan would return). + recent: RefCell>, + signature_queries: Cell, + recent_queries: Cell, + } + + impl IndexLagList { + fn new() -> Self { + Self { + recent: RefCell::new(Vec::new()), + signature_queries: Cell::new(0), + recent_queries: Cell::new(0), + } + } + fn run(&self, query: &IssueListQuery) -> Result, SimardError> { + match query { + // The search index lags for the whole window: always empty. + IssueListQuery::Signature(_) => { + self.signature_queries.set(self.signature_queries.get() + 1); + Ok(Vec::new()) + } + // Strongly consistent: returns everything filed so far. + IssueListQuery::RecentOpen(_) => { + self.recent_queries.set(self.recent_queries.get() + 1); + Ok(self.recent.borrow().clone()) + } + } + } + fn file(&self, issue: GhIssue) { + self.recent.borrow_mut().push(issue); + } + } + + /// Regression for the stewardship duplicate-issue *storm* (rysweet/Simard + /// #4671-#4686): many detection sweeps of the SAME failure within GitHub's + /// search-index window must collapse to exactly ONE filed issue, not one per + /// sweep. This drives the real [`resolve_dedup_candidates`] dedup — mirroring + /// the file-if-no-match decision `process_orchestrator_run` makes — across a + /// window in which the full-text index never catches up. The + /// strongly-consistent recent-open scan is what keeps the count at one; if + /// that fallback were removed each sweep would see an empty search and file + /// afresh, reproducing the flood. + #[test] + fn resolve_collapses_a_full_window_of_sweeps_to_a_single_filed_issue() { + let sig = "cafef00dcafef00d"; + let gh = IndexLagList::new(); + let sweeps = 12; + let mut filed = 0u64; + + for _ in 0..sweeps { + let candidates = resolve_dedup_candidates(|q| gh.run(q), sig).unwrap(); + if super::find_existing(&candidates, sig).is_none() { + // No open issue carries this signature yet — file exactly one. + filed += 1; + gh.file(issue(4671, sig)); + } + } + + assert_eq!( + filed, 1, + "a full index-lag window of {sweeps} identical detections must file ONE issue, \ + not one per sweep (the #4671-#4686 storm)" + ); + assert_eq!( + gh.recent.borrow().len(), + 1, + "exactly one tracking issue exists after the whole window" + ); + assert_eq!( + gh.signature_queries.get(), + sweeps, + "every sweep still runs the fast signature search first" + ); + assert_eq!( + gh.recent_queries.get(), + sweeps, + "every empty search must fall back to the strongly-consistent recent scan" + ); + } + #[test] fn resolve_returns_no_match_when_neither_query_has_signature() { let sig = "cafef00dcafef00d"; diff --git a/src/stewardship/tests_extra.rs b/src/stewardship/tests_extra.rs index ab09c2411..1930292b2 100644 --- a/src/stewardship/tests_extra.rs +++ b/src/stewardship/tests_extra.rs @@ -362,6 +362,106 @@ fn process_run_routes_overseer_to_default_and_dedups() { ); } +// ─────────────────────────── Storm regression (multi-sweep dedup) ─────────── + +/// A stateful `GhClient` that models the daemon's real dedup surface once the +/// search-index-lag fallback is in play: a filed tracking issue is visible to a +/// subsequent `search_issues` call (as the strongly-consistent recent-open scan +/// makes it), so re-observing the same failure matches instead of re-filing. +/// Unlike the seeded [`FakeGhClient`], this one accumulates created issues and +/// answers searches from that live store — the shape a full observation window +/// actually sees. +#[derive(Default)] +struct StatefulGhClient { + issues: Mutex>, + create_calls: Mutex, + next_number: Mutex, +} + +impl StatefulGhClient { + fn new() -> Self { + Self { + issues: Mutex::new(Vec::new()), + create_calls: Mutex::new(0), + next_number: Mutex::new(4671), + } + } + fn create_call_count(&self) -> usize { + *self.create_calls.lock().unwrap() + } + fn open_issue_count(&self) -> usize { + self.issues.lock().unwrap().len() + } +} + +impl GhClient for StatefulGhClient { + fn search_issues(&self, _repo: &str, signature: &str) -> Result, SimardError> { + let needle = format!("stewardship-signature: {signature}"); + Ok(self + .issues + .lock() + .unwrap() + .iter() + .filter(|i| i.body.contains(&needle)) + .cloned() + .collect()) + } + fn create_issue(&self, repo: &str, title: &str, body: &str) -> Result { + *self.create_calls.lock().unwrap() += 1; + let mut num = self.next_number.lock().unwrap(); + let issue = GhIssue { + number: *num, + url: format!("https://github.com/{repo}/issues/{num}"), + title: title.to_string(), + body: body.to_string(), + }; + *num += 1; + self.issues.lock().unwrap().push(issue.clone()); + Ok(issue) + } +} + +/// End-to-end regression for the stewardship issue *storm* (rysweet/Simard +/// #4671-#4686): a whole observation window of identical detections routed +/// through [`process_orchestrator_run`] must produce exactly ONE open issue — +/// the first sweep files, every later sweep dedups to `MatchedExisting`. This is +/// the public-API expression of the "duplicate detections within a window +/// produce at most one open issue" contract; it would fail the moment the +/// file-if-no-match dedup guard regressed and the loop filed per-tick. +#[test] +fn process_run_files_one_issue_across_a_full_window_of_identical_detections() { + let gh = StatefulGhClient::new(); + let run = sample_run(); + + let mut filed_new = 0usize; + let mut matched_existing = 0usize; + for _ in 0..15 { + match process_orchestrator_run(&run, &gh).unwrap() { + StewardshipOutcome::FiledNew { .. } => filed_new += 1, + StewardshipOutcome::MatchedExisting { .. } => matched_existing += 1, + } + } + + assert_eq!( + filed_new, 1, + "only the first sweep files a new tracking issue" + ); + assert_eq!( + matched_existing, 14, + "every subsequent sweep dedups to the existing issue" + ); + assert_eq!( + gh.create_call_count(), + 1, + "exactly one `create_issue` across the whole window — no duplicate flood" + ); + assert_eq!( + gh.open_issue_count(), + 1, + "exactly one open tracking issue remains after the window" + ); +} + // ─────────────────────────── Input validation ─────────────────────────── #[test] From 3b0394e7ac67d9fb33a05d25f1607fe9c0880aed Mon Sep 17 00:00:00 2001 From: rysweet Date: Sat, 25 Jul 2026 21:30:30 +0000 Subject: [PATCH 2/6] refactor(overseer): single-pass bounded-slug signature check Collapse is_bounded_signature_slug's two-pass scan (a separate first-char match plus a full chars().all() pass) into one char_indices().all() pass. Behavior is identical: an alphanumeric is allowed anywhere, the restricted separators only at index > 0, so the slug must still open with an alphanumeric. Simpler and clearer with no change to the IV-1 injection-safety contract or its tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/overseer/signal.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/overseer/signal.rs b/src/overseer/signal.rs index f9857ec41..dd1eea51f 100644 --- a/src/overseer/signal.rs +++ b/src/overseer/signal.rs @@ -244,13 +244,12 @@ pub fn is_bounded_signature_slug(sig: &str) -> bool { if sig.is_empty() || sig.len() > MAX_GAP_SIGNATURE_LEN { return false; } - let mut chars = sig.chars(); - match chars.next() { - Some(c) if c.is_ascii_alphanumeric() => {} - _ => return false, - } - sig.chars() - .all(|c| c.is_ascii_alphanumeric() || matches!(c, ':' | '_' | '#' | '.' | '/' | '-')) + // Single pass: an ASCII alphanumeric is allowed anywhere; the restricted + // separators are allowed only after the first char (so the slug must open + // with an alphanumeric, never a separator). + sig.char_indices().all(|(i, c)| { + c.is_ascii_alphanumeric() || (i > 0 && matches!(c, ':' | '_' | '#' | '.' | '/' | '-')) + }) } /// Coarse relative importance. `Ord` sorts ascending so `Critical` comes first, From d298a859c7c52e9d06f10f34b2665b40c877dc75 Mon Sep 17 00:00:00 2001 From: rysweet Date: Sat, 25 Jul 2026 21:36:12 +0000 Subject: [PATCH 3/6] perf(overseer): O(1) coverage lookup and bounded field truncation detect_workstream_gaps probed the covered-signature set with a per-candidate linear scan (O(candidates x coverage) every observer cycle); build a HashSet once for O(1) membership. truncate_field counted every char of arbitrarily long board fields before deciding to truncate; short-circuit at the bound via char_indices().nth. Both are behavior-preserving. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/overseer/sensor.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/overseer/sensor.rs b/src/overseer/sensor.rs index e9abf08e5..b564ade2b 100644 --- a/src/overseer/sensor.rs +++ b/src/overseer/sensor.rs @@ -18,6 +18,7 @@ //! - File deduped issues: `stewardship::process_orchestrator_run` (via //! [`StewardshipIssueFiler`](crate::overseer::observer::StewardshipIssueFiler)). +use std::collections::HashSet; use std::sync::Arc; use std::time::{Duration, Instant}; @@ -319,7 +320,11 @@ pub fn detect_workstream_gaps( anomalies: &[String], coverage: &[String], ) -> Vec { - let is_covered = |sig: &str| coverage.iter().any(|c| c == sig); + // Build the coverage set once for O(1) membership tests. Each of the three + // candidate loops below probes it per item; a linear `coverage.iter().any` + // would be O(candidates x coverage) every observer cycle. + let covered: HashSet<&str> = coverage.iter().map(String::as_str).collect(); + let is_covered = |sig: &str| covered.contains(sig); let mut gaps: Vec = Vec::new(); // 1. Goal board — uncovered high-priority goals. @@ -424,10 +429,11 @@ fn goal_has_active_workstream(goal: &ActiveGoal) -> bool { /// Bound a rendered gap field to [`MAX_GAP_FIELD_LEN`] `char`s (trimmed). fn truncate_field(s: &str) -> String { let s = s.trim(); - if s.chars().count() <= MAX_GAP_FIELD_LEN { - s.to_string() - } else { - s.chars().take(MAX_GAP_FIELD_LEN).collect() + // Short-circuit at the bound: stop scanning at the (MAX_GAP_FIELD_LEN)th + // char instead of counting every char of an arbitrarily long board field. + match s.char_indices().nth(MAX_GAP_FIELD_LEN) { + None => s.to_string(), + Some((byte_idx, _)) => s[..byte_idx].to_string(), } } From 678f5851c4dd273268ccb6fa032f9c747714d794 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Sat, 25 Jul 2026 23:12:47 +0000 Subject: [PATCH 4/6] docs(overseer): correct gap-dedup docs to match notify-only code (#4687) The gap-filing dedup docs claimed act_flag_workstream_gaps runs a durable, restart-safe gh.search_issues -> find_existing -> create_issue flow. The code path is notification-only: stable-signature slug validation (IV-1) + in-process WhisperGate dedup + operator notification. It never queries or files GitHub issues and is not restart-safe on its own. This PR had flipped honest "future work / planned" labels to false "implemented" claims (Zero-BS violation flagged by code, security, and philosophy reviews). Restore truthful framing: - Gap path documented as notify-only, stable-signature, in-process dedup. - Durable cross-process GitHub-side check scoped as follow-on the signature enables (not wired on the gap path today). - Removed fabricated restart-verification steps and reused_existing/gh-search logs from the how-to. - Retitled the two new docs and their mkdocs nav entries (drop "Durable"). Docs-only; mkdocs build introduces no new warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/concepts/gap-scan-backoff-dedup.md | 53 +-- docs/howto/configure-gap-durable-dedup.md | 181 +++++------ docs/index.md | 4 +- docs/reference/overseer-gap-durable-dedup.md | 320 ++++++++----------- mkdocs.yml | 4 +- 5 files changed, 254 insertions(+), 308 deletions(-) diff --git a/docs/concepts/gap-scan-backoff-dedup.md b/docs/concepts/gap-scan-backoff-dedup.md index d057e7a93..a526a9007 100644 --- a/docs/concepts/gap-scan-backoff-dedup.md +++ b/docs/concepts/gap-scan-backoff-dedup.md @@ -8,9 +8,9 @@ description: > fixed-window dedup was insufficient, how exponential backoff rate-limits without ever permanently silencing a genuinely recurring gap, how this makes the Overseer ACT on a gap once rather than observe it forever (meta bugs - #4255 / #4126), and the durable cross-process open-issue check — enabled by a - stable, content-addressed signature — that now backstops the in-process gate - across daemon restarts. + #4255 / #4126), and the stable, content-addressed gap signature — the + foundation that a future durable cross-process open-issue check will build on + to survive daemon restarts. last_updated: 2026-07-25 review_schedule: as-needed owner: simard @@ -88,9 +88,10 @@ The fix ([BackoffGate reference](../reference/overseer-backoff-gate-api.md)) is deliberately additive — it adds a new primitive rather than mutating the existing `WhisperGate`, so every current caller is untouched. An in-process -exponential-backoff gate guards the gap-cover act path, with a durable -cross-process open-issue equivalence check layered in front of it so the -guarantee survives daemon restarts. +exponential-backoff gate guards the gap-cover act path. A stable, +content-addressed gap signature (shipped here) lays the foundation for a +durable cross-process open-issue equivalence check — the follow-on that will let +the guarantee survive daemon restarts. ### Exponential BackoffGate (in-process) — implemented @@ -113,24 +114,31 @@ Because the gate lives in the decide/act seam and only holds a suppressed plan, it gives a clean guarantee **within one running daemon**: at most one open covering issue per distinct gap for the life of the process. -### Open-issue equivalence check (cross-process) — implemented +### Open-issue equivalence check (cross-process) — future work The BackoffGate is in-memory, so a daemon restart forgets its state and a cold gate could re-file a duplicate that is already open on GitHub. The deeper cause of the observed `[stewardship] workstream_gap:*` flood (e.g. #4671, #4680, -#4685; OODA-stuck #4689) was that the filed `stewardship-signature:` was keyed -per **run** (`originating-run: overseer-`), so the already-existing -open-issue search never matched across runs. The fix makes the signature a -stable, content-addressed slug so the durable check closes this: on -the gap-filing path, **before** filing, the Overseer runs a GitHub query -(reusing the existing `stewardship::dedup` helpers and `find_existing` on the -`stewardship-signature:` body marker) for an already-open **equivalent** issue -and **reuses / skips** if one exists. It **fails loud** — a `gh` search error -propagates and files nothing, never a blind create. Because GitHub is the source -of truth, the guarantee is now *at most one open issue per distinct gap -signature across restarts and daemons*. The in-process `WhisperGate` remains the -fast pre-filter in front of it. See the -[durable gap-filing dedup reference](../reference/overseer-gap-durable-dedup.md) +#4685; OODA-stuck #4689) was that the gap signature was keyed +per **run** (`originating-run: overseer-`), so the in-process gate key +churned every run and any GitHub-side search would never match across runs. + +This change fixes the **root cause** by making the signature a **stable, +content-addressed slug** (derived from trusted identifiers, not the run id), so +the in-process gate now collapses a recurring gap to a single notification +**within a running daemon**, and the slug is a valid, restart-safe join key. +That stable signature is the **prerequisite** for a durable cross-process check. + +The durable check itself is **future work**: before notifying/filing, the +Overseer would run a GitHub query (reusing the existing `stewardship::dedup` +helpers and `find_existing` on the `stewardship-signature:` body marker) for an +already-open **equivalent** issue and reuse/skip if one exists, failing loud on +any `gh` error. The gap-notification path (`act_flag_workstream_gaps`) does +**not** perform this query today — it applies the in-process `WhisperGate` and +notifies the operator. The proven durable pattern lives on the sibling +stewardship filing seam (`stewardship::process_orchestrator_run`); wiring it +onto the gap path is the follow-on this stable signature enables. See the +[gap-filing dedup reference](../reference/overseer-gap-durable-dedup.md) and the [how-to](../howto/configure-gap-durable-dedup.md). ## How this makes the Overseer ACT (not just observe) @@ -159,9 +167,10 @@ protect. ## Invariants -- **One open covering issue per distinct gap, across restarts.** The in-process +- **One open covering issue per distinct gap within a process.** The in-process gate holds duplicate coverage plans across ticks; the cross-process case - (restarts / multiple daemons) is closed by the durable + (restarts / multiple daemons) is not yet closed — the stable signature shipped + here is the foundation for the future durable [open-issue check](../reference/overseer-gap-durable-dedup.md). - **Never permanently silent.** The window is capped and resets after silence; a genuinely recurring gap always re-surfaces. diff --git a/docs/howto/configure-gap-durable-dedup.md b/docs/howto/configure-gap-durable-dedup.md index 905e93835..6ea7137c7 100644 --- a/docs/howto/configure-gap-durable-dedup.md +++ b/docs/howto/configure-gap-durable-dedup.md @@ -1,11 +1,12 @@ --- -title: Configure and verify durable gap dedup +title: Configure and verify gap dedup description: > - How to operate and verify the Overseer's restart-safe, GitHub-side gap-filing - dedup guard for workstream-gap stewardship issues — confirm a duplicate gap is - reused rather than re-filed after a daemon restart, read the `overseer::gap_scan` reused/flagged - logs, understand the fail-loud behaviour when `gh` is unavailable, and check - the bounded GapCategory taxonomy that keeps titles stable and dedupable. + How to operate and verify the Overseer's stable-signature gap dedup for + workstream-gap notifications — confirm a recurring gap is deduped to one + operator notification within a running daemon, read the + overseer::gap_scan flagged/suppressed logs, understand that a daemon restart + resets the in-process gate, and check the bounded GapCategory taxonomy that + keeps gap signatures stable and dedupable. last_updated: 2026-07-25 review_schedule: as-needed owner: simard @@ -20,36 +21,41 @@ related: - ./file-stewardship-issues-from-orchestrator-runs.md --- -# Configure and verify durable gap dedup +# Configure and verify gap dedup -The Overseer files stewardship issues for uncovered backlog work — uncovered -goals, high-signal open issues, and unaddressed telemetry anomalies. It now -dedupes those filings against **GitHub itself**, not just in-process memory, so -a daemon **restart** (or a second daemon) no longer re-files an issue that is -already open. This closes the near-duplicate `[stewardship] workstream_gap:*` -flood (observed on e.g. #4671, #4680, #4685). +The Overseer flags uncovered backlog work — uncovered goals, high-signal open +issues, and unaddressed telemetry anomalies — on its recurring gap-scan and +**notifies the operator** (email + Signal) about it. This rail makes the gap's +dedup signature a **stable, content-addressed** slug (instead of a per-run hash), +so a recurring gap is deduped to **one notification within a running daemon** +rather than re-notified every tick. This is the root-cause fix behind the +near-duplicate `[stewardship] workstream_gap:*` noise (observed on e.g. #4671, +#4680, #4685). For the data model, signature grammar, and guarantees, see the -[durable gap-filing dedup reference](../reference/overseer-gap-durable-dedup.md). +[gap-filing dedup reference](../reference/overseer-gap-durable-dedup.md). + +> **Scope.** The gap-notification path dedupes via the **in-process** +> `WhisperGate` and notifies the operator; it does **not** create GitHub issues +> and is **not restart-safe on its own** — a daemon restart resets the gate. A +> durable, GitHub-sourced cross-process check is scoped as follow-on work and is +> **not** wired on this path yet; see the reference doc's *Future work* section. ## What changed for operators -- **Before:** the filed `stewardship-signature:` was derived per run +- **Before:** the gap signature was derived per run (`originating-run: overseer-`), so every restart/re-run minted a fresh - signature that matched nothing on GitHub, and the only stable-ish guard (the - in-process gate) went cold on restart → the next tick re-filed a near-duplicate - stewardship issue. + key and the in-process gate could not collapse a recurring gap → the scan + re-notified a near-duplicate gap every tick. - **Now:** the signature is a **stable, content-addressed** slug, so the - Overseer's open-issue search actually matches an already-open issue carrying - the gap's marker and **reuses** it. The guarantee is now *at most one open - issue per distinct gap signature*, **across restarts**. -- **Legacy issues:** duplicates filed *before* this rail carry the old per-run - hash signature (e.g. `stewardship-signature: f6f8480b146b171e`), so the - slug-based search below only finds post-fix issues; old duplicates are not - retro-reconciled. + in-process gate collapses a recurring gap to **one notification per dedup + window** for the life of the daemon. +- **Restart behaviour:** the in-process gate is memory-resident, so a restart + resets it and the first post-restart tick may re-notify. Cross-restart dedup + awaits the durable check (future work). -There is **nothing to turn on** — the durable check is always active on the -gap-filing path whenever the Overseer runs. The existing knobs still apply: +There is **nothing to turn on** — stable-signature dedup is always active on the +gap path whenever the Overseer runs. The existing knobs still apply: | Env var | What it does | Default | |---|---|---| @@ -57,85 +63,65 @@ gap-filing path whenever the Overseer runs. The existing knobs still apply: | `SIMARD_OVERSEER_GAP_SCAN_EVERY_N` | Run the scan every *Nth* tick | `1` | See [configure the gap-scan backoff](./configure-overseer-gap-scan-backoff.md) -for the in-process pre-filter window that sits in front of the durable check. +for the in-process dedup window. ## Prerequisites -- The `gh` CLI is installed, on `PATH`, and authenticated against `rysweet/Simard` - (or a token with `issues:write` scope). The durable check needs read (search) - and, when filing, write. - The acting Overseer is enabled (`SIMARD_OVERSEER_ENABLED` unset or truthy). +- An operator notifier is configured (email and/or Signal) so gap notifications + have somewhere to go. -## Verify: a restart does not re-file +## Verify: a recurring gap is deduped within a run This is the core acceptance check for the workstream-gap dedup rail. -1. Let the Overseer file one gap issue and note its number and signature: +1. With a standing, uncovered gap present, let the Overseer run one gap-scan tick + and confirm it notifies **once** (a `flagged>=1` info line): - ```bash - gh issue list -R rysweet/Simard --search 'in:body stewardship-signature: workstream-gap:' \ - --state open --json number,title | jq '.[0]' + ```text + INFO overseer::gap_scan flagged=1 suppressed=0 + overseer recorded uncovered backlog work and notified the operator ``` - Confirm the body carries the marker: - - ```bash - gh issue view -R rysweet/Simard --json body \ - | jq -r '.body' | grep 'stewardship-signature:' - ``` - -2. **Restart the daemon** (the in-process gate is now cold). - -3. On the next gap-scan tick, confirm **no new issue** was opened for the same - signature — the open-issue count for that signature stays at **1**: +2. On the **next** tick within the dedup window, confirm the **same** gap is + suppressed rather than re-notified — the count moves to `suppressed`: - ```bash - gh issue list -R rysweet/Simard --state open \ - --search 'in:body stewardship-signature: workstream-gap:' \ - --json number | jq 'length' # → 1, not 2 + ```text + DEBUG overseer::gap_scan flagged=0 suppressed=1 + overseer gap-scan: every observed gap is within the dedup window (suppressed) ``` -4. Confirm the reuse in the logs (see below). +3. `flagged=0 suppressed=1` for the recurring gap is the proof the stable + signature deduped it. (A daemon restart between steps 1 and 2 resets the gate + and may re-notify — that is expected until the durable check lands.) ## Read the logs -The rail emits structured `tracing` + OTel only (no `print!`/`println!`), on +The gap path emits structured `tracing` + OTel only (no `print!`/`println!`), on `target: "overseer::gap_scan"`: -```text -INFO overseer::gap_scan flagged=0 reused_existing=1 suppressed=0 - key="workstream-gap:goal:g-1873" - overseer gap-scan: matched an already-open issue; reused instead of re-filing -``` - | Field | Meaning | |---|---| -| `flagged` | New issues created this tick | -| `reused_existing` | Gaps matched to an already-open issue (no new issue) | -| `suppressed` | Gaps dropped by the in-process `WhisperGate` pre-filter | -| `key` | The batch `workstream_gap_key` for the tick | - -A restart-then-reuse shows `flagged=0 reused_existing=1` — the proof the durable -check did its job. +| `flagged` | Fresh gaps notified this tick | +| `suppressed` | Gaps dropped by the in-process gate or by a malformed signature | +| `dispatched` / `all_sent` | Operator-notification delivery status | -## What happens when `gh` is unavailable (fail-loud) +## Malformed signatures are dropped (injection defense) -If the open-issue search fails (unauthenticated, rate-limited, offline), the act -path **files nothing** and surfaces the error — it never falls back to a blind -create: +A gap whose signature is not a valid restricted slug +(`^[a-z0-9][a-z0-9:_#.\-/]{0,200}$`) is **dropped at the filing seam** and counted +as suppressed — it never reaches an operator notification: ```text -ERROR overseer::gap_scan gh search failed; filing skipped (fail-loud) - key="workstream-gap:goal:g-1873" reason="gh: HTTP 403 rate limit exceeded" +WARN overseer::gap_scan category="goal" + overseer gap-scan: dropping a gap with a malformed dedup signature + (outside the bounded taxonomy) ``` -This is deliberate: a fail-*open* fallback would re-create the flood. Fix the -`gh` auth / rate-limit condition (often `gh auth login`) and the next tick -resumes normally. Contrast the in-process backoff gate, which fails *toward -surfacing* — together they mean a genuine gap is never silently dropped **and** -a duplicate is never blindly filed. +This is deliberate: signatures come from trusted identifiers only, so a malformed +one signals a bug or an injection attempt, not a real gap. -## The bounded taxonomy (why titles are now stable) +## The bounded taxonomy (why signatures are now stable) Duplicates used to slip through because free-form titles drifted between ticks. Each gap resolves to a bounded `GapCategory` variant with a stable slug that @@ -147,36 +133,29 @@ anchors the signature: | High-signal open issue | `IssueUncovered` | `issue:#` | | Unaddressed anomaly | `AnomalyUnaddressed` | `anomaly:` | -`GapCategory` is a closed enum of exactly these three kinds, so a gap's title and -signature are stable across ticks. The durable fix did not add kinds — it made -the filed `stewardship-signature:` a stable, content-addressed slug (instead of a -per-run hash) so an already-open issue for the same gap is found and reused. This -change is additive — existing `goal:`/`issue:`/`anomaly:` issues keep matching. +`GapCategory` is a closed enum of exactly these three kinds, so a gap's signature +is stable across ticks. The fix did not add kinds — it made the signature a +stable, content-addressed slug (instead of a per-run hash) so the in-process gate +recognises the same gap across ticks. This change is additive. ## Common pitfalls -- **A closed issue and the gap recurred → a new issue appeared.** Expected. The - search uses `--state open`, so a closed cover issue does not suppress a fresh - filing. Leave the issue open, or fix the underlying gap, to keep it deduped. -- **Two issues for the "same" gap.** Confirm they carry the **same** - `stewardship-signature:`. If the signatures differ, the gap resolved to two - distinct keys (e.g. two different goal ids) — that is correct behaviour, not a - dedup miss. -- **No reuse log after a restart, and a duplicate appeared.** Check the - `overseer::gap_scan` logs for a `gh search failed` ERROR line around the tick; - a failed search files nothing, so a duplicate here means the *prior* issue was - closed or its marker is missing. Verify the body still contains - `stewardship-signature:`. -- **`gh` not authenticated.** The fail-loud ERROR carries the trimmed `gh` - stderr — usually a hint to run `gh auth login`. +- **A duplicate notification appeared right after a restart.** Expected — the + in-process gate is reset on restart. Cross-restart dedup is future work. +- **Two notifications for the "same" gap.** Confirm the gaps carry the **same** + signature. If the signatures differ, the gap resolved to two distinct keys + (e.g. two different goal ids) — correct behaviour, not a dedup miss. +- **A gap I expected was never notified.** Check for a `dropping a gap with a + malformed dedup signature` WARN — a signature outside the bounded taxonomy is + dropped by design. ## See also -- [Durable gap-filing dedup reference](../reference/overseer-gap-durable-dedup.md) - — signature grammar, flow, fail-loud contract, and security notes. +- [Gap-filing dedup reference](../reference/overseer-gap-durable-dedup.md) + — signature grammar, the in-process flow, and the scoped durable follow-on. - [Review the Overseer's workstream gaps](./review-overseer-workstream-gaps.md) — where the gaps surface and how to respond. - [Gap-scan dedup & exponential backoff](../concepts/gap-scan-backoff-dedup.md) - — the in-process pre-filter this durable check backstops. + — the in-process gate this stable signature feeds. - [File stewardship issues from orchestrator runs](./file-stewardship-issues-from-orchestrator-runs.md) - — the sibling loop whose dedup flow this mirrors. + — the sibling loop whose durable dedup flow the future gap check would mirror. diff --git a/docs/index.md b/docs/index.md index d441d21ec..ad8dae1b9 100644 --- a/docs/index.md +++ b/docs/index.md @@ -167,8 +167,8 @@ Bare `simard` prints the unified help text instead of attempting a hidden enviro - [Canary gate isolation and self-deploy convergence reference](./reference/canary-gate-convergence.md) - The #4440 root-cause repair that acts on those diagnostics so a healthy candidate self-deploys and `DeployDrift` returns to 0: per-gate `self_relaunch::gate` tracing/OTel spans in `verify_canary`, the additive `RelaunchConfig.canary_env` narrow allow-list plus `scrub_gate_env` that supplies a gate's legitimately-missing signal without weakening fail-closed semantics, and the self-deploy loop advancing past the stuck target SHA. See the [convergence runbook](./howto/converge-a-stuck-red-canary-self-deploy.md). - [How to review the Overseer's workstream gaps](./howto/review-overseer-workstream-gaps.md) - Read, act on, and tune the Overseer's recurring "what workstreams are we missing?" gap-scan — the uncovered high-priority goals, high-signal issues, and unaddressed anomalies it flags each tick, where the deduped notification appears, and the `SIMARD_OVERSEER_GAP_SCAN` knobs (#2630). - [Overseer workstream gap-scan reference](./reference/overseer-workstream-gap-scan.md) - The additive Observe→Orient→Act gap-scan: the `Signal::WorkstreamGap`/`GapItem`/`ProblemKind::WorkstreamCoverage` model, the coverage-set detection contract, the deduped NotifyOperator act path, the `SIMARD_OVERSEER_GAP_SCAN` configuration, and the additive `OverseerTickReport.workstream_gaps_detected` counter (#2630). -- [Overseer durable gap-filing dedup reference](./reference/overseer-gap-durable-dedup.md) - The restart-safe, GitHub-side open-issue equivalence check on the stewardship gap-filing path that stops the Overseer re-filing a near-duplicate issue every tick and across daemon restarts: the bounded `GapCategory` taxonomy (`GoalUncovered`/`IssueUncovered`/`AnomalyUnaddressed`), the stable slug-validated `stewardship-signature:` key that replaces the old per-run hash, the `WhisperGate` pre-filter → `gh.search_issues` → `find_existing` → reuse/skip/create flow, the fail-loud (never fail-open) contract, and the repo-regex/`--`-terminator/`sanitize_issue_text` hardening applied before any public issue write. -- [How to configure and verify durable gap dedup](./howto/configure-gap-durable-dedup.md) - Confirm a duplicate gap is reused rather than re-filed after a daemon restart, read the `overseer::gap_scan` `reused_existing`/`flagged` logs, understand the fail-loud behaviour when `gh` is unavailable, and check the bounded taxonomy that keeps gap titles stable and dedupable. +- [Overseer gap-filing dedup reference](./reference/overseer-gap-durable-dedup.md) - The stable, content-addressed gap signature that stops the Overseer re-notifying a near-duplicate gap every tick within a running daemon: the bounded `GapCategory` taxonomy (`GoalUncovered`/`IssueUncovered`/`AnomalyUnaddressed`), the slug-validated `stewardship-signature:` key that replaces the old per-run hash, the in-process `WhisperGate` dedup + operator notification on the gap path, and the injection-defense slug validation enforced at the filing seam. The durable, restart-safe GitHub-side open-issue check is scoped as the follow-on this signature enables. +- [How to configure and verify gap dedup](./howto/configure-gap-durable-dedup.md) - Confirm a recurring gap is deduped to one operator notification within a running daemon, read the `overseer::gap_scan` `flagged`/`suppressed` logs, understand that a daemon restart resets the in-process gate, and check the bounded taxonomy that keeps gap signatures stable and dedupable. - [Overseer recipe-launch idempotency reference](./reference/overseer-recipe-launch-idempotency.md) - The launcher-level rail that makes `AmplihackRecipeRunner::spawn` idempotent per task signature: the pure `recipe_signature` normalization (`target_repo` + `task_description`, trim/lowercase/whitespace-collapse, `\u{1F}` separator), the reap-then-dedup order, the fail-visible `overseer::recipe` suppressed-launch warning, the shared-handle `probe` semantics, and the injectable `ChildSpawner`/`SpawnedChild` test seam — so a still-blocked signature no longer spawns a byte-identical `smart-orchestrator` every tick (#4125). - [Overseer root-cause ("WHY") API reference](./reference/overseer-root-cause-why-api.md) - The `RootCause`/`CauseCandidate`/`Confidence`/`Likelihood`/`CauseSource` model and additive `Problem.why` field, the pure `root_cause::analyze` analyzer with `PriorOccurrence`/`root_cause_signature`, the `Remediation`/`RemediationClass` classification on `PlannedIntervention`, the `Overseer::with_memory` recall+store seam, the `decide_blocked_goal` recurrence routing, the `goal_blocked_with_why` WHY notification constructor, the `ProblemEntry` activity-feed rows, and the extended `OverseerTickReport`/`OverseerTotals` counters (#2635). - [How to configure and observe the Overseer root-cause principle](./howto/configure-overseer-root-cause-why.md) - Read the WHY and the root-cause/symptom label in the `overseer::root_cause` traces, the `OverseerTickReport` counters, the activity-feed `problem_entries` rows, and the `goal-blocked` notification; understand the always-on/no-opt-out contract and the graceful memory-recall degrade; and verify the feature end-to-end with injected fakes (#2635). diff --git a/docs/reference/overseer-gap-durable-dedup.md b/docs/reference/overseer-gap-durable-dedup.md index 70d55f98f..bcee47809 100644 --- a/docs/reference/overseer-gap-durable-dedup.md +++ b/docs/reference/overseer-gap-durable-dedup.md @@ -1,15 +1,14 @@ --- -title: Overseer durable gap-filing dedup reference +title: Overseer gap-filing dedup reference description: > - The restart-safe, GitHub-side dedup guard on the Overseer stewardship - gap-filing path — the durable open-issue equivalence check that stops the - Overseer from re-filing a near-duplicate stewardship issue every tick and - across daemon restarts and re-runs. Covers the bounded GapCategory taxonomy - (GoalUncovered / IssueUncovered / AnomalyUnaddressed), the stable - slug-validated signature grammar, the WhisperGate pre-filter → - gh.search_issues → find_existing → reuse/skip/create flow, the - `stewardship-signature:` body marker, the fail-loud contract, and the input - validation / secret-redaction hardening applied before any public issue write. + The stable, content-addressed gap signature on the Overseer stewardship + gap-notification path — the bounded GapCategory taxonomy + (GoalUncovered / IssueUncovered / AnomalyUnaddressed), the slug-validated + signature grammar that replaces the old per-run hash, the in-process + WhisperGate dedup + operator notification the gap path actually performs, the + injection-defense slug validation enforced at the filing seam, and the durable + cross-process open-issue check this signature is the foundation for (scoped as + follow-on work, not yet wired on the gap path). last_updated: 2026-07-25 review_schedule: as-needed owner: simard @@ -25,77 +24,62 @@ related: - ../design/overseer.md --- -# Overseer durable gap-filing dedup reference - -The acting **Overseer** files stewardship issues for uncovered backlog work — -uncovered goals, high-signal open issues, and unaddressed telemetry anomalies. -The filed issue already carries a `stewardship-signature:` body -marker and the act path already searches GitHub for it — but before this rail -the signature was derived **per run** (`originating-run: overseer-`), so -every daemon start or re-run minted a **fresh** signature that matched no -existing issue. The in-process [`WhisperGate` / `BackoffGate`](./overseer-backoff-gate-api.md) -was the only other guard, and it lives in memory and is wiped on every daemon -restart. A restart (or a second daemon) therefore re-filed a gap that was -**already open** on GitHub — the near-duplicate `[stewardship] workstream_gap:*` -flood (observed on e.g. #4671, #4680, #4685). - -This rail closes that seam. It makes the `stewardship-signature:` a **stable, -content-addressed** slug (derived from the gap's trusted identifiers, not the -run id) so the **already-existing** durable open-issue search actually matches -across runs: before filing, the Overseer searches the target repo for an -already-open issue carrying the gap's stable marker and, if one exists, -**reuses / comments / skips** instead of filing a new one. Because the source of -truth is the GitHub issue tracker (not process memory) *and* the key is now -stable across runs, the guarantee survives restarts and multiple daemons. - -> **This is additive.** It layers a durable, content-addressed check **in front -> of** the existing in-process gates; it does not replace them and does not -> change how any other intervention decides or acts. The in-process -> `WhisperGate` remains the fast pre-filter (it avoids a `gh` round-trip on the -> common repeat-within-window case); the durable check is the restart-safe -> backstop. The issue-body **structure** and the `stewardship-signature:` marker -> *field* are reused unchanged; what changes is the marker's *value* — from an -> unstable per-run hash to a stable per-gap slug. Consequently, issues filed -> **before** this rail carry the old per-run hash signatures and will **not** -> dedupe against the new slug signatures; those pre-existing duplicates -> (#4671–#4689 range) are out of scope to retroactively reconcile. +# Overseer gap-filing dedup reference + +The acting **Overseer** watches for uncovered backlog work — uncovered goals, +high-signal open issues, and unaddressed telemetry anomalies — and flags it on +the recurring gap-scan. Before this rail the gap signature was derived **per +run** (`originating-run: overseer-`), so every daemon start or re-run +minted a **fresh** signature. The only dedup guard on the gap path is the +in-process [`WhisperGate` / `BackoffGate`](./overseer-backoff-gate-api.md), which +keys on that signature; a churning per-run key meant the same gap re-surfaced as +noise, and the near-duplicate `[stewardship] workstream_gap:*` flood was observed +downstream (e.g. #4671, #4680, #4685). + +This rail fixes the **root cause**: it makes the signature a **stable, +content-addressed** slug (derived from the gap's trusted identifiers, not the run +id). With a stable key, the in-process gate now collapses a recurring gap to a +**single operator notification within a running daemon**, and the slug is a +valid, restart-safe join key that a future durable check can search on. + +> **Scope — read this first.** What ships here is the **stable signature**, the +> **bounded `GapCategory` taxonomy**, and **injection-defense slug validation**. +> The gap-notification path (`act_flag_workstream_gaps`, +> `src/overseer/mod.rs`) applies the in-process `WhisperGate` and **notifies the +> operator** (email + Signal); it does **not** call `gh.search_issues`, +> `find_existing`, or `create_issue`, and it is **not restart-safe on its own** +> (the gate is in memory and is wiped on restart). The durable, GitHub-sourced +> open-issue check described under [Future work](#future-work-the-durable-cross-process-check) +> is **not yet wired** on the gap path; the stable signature is the prerequisite +> that makes it viable. The proven durable pattern currently lives on the sibling +> stewardship filing seam (`stewardship::process_orchestrator_run`). > **Modules:** taxonomy + signature grammar `src/overseer/signal.rs` -> (`GapCategory`, `GapItem::signature`); batch key `src/overseer/mod.rs` -> (`workstream_gap_key`); detectors `src/overseer/sensor.rs` -> (`detect_workstream_gaps`) and `src/overseer/observer.rs`; rendering -> `src/overseer/notify.rs`; reused GitHub seam + redaction -> `src/stewardship/gh_client.rs`, `src/stewardship/dedup.rs`, -> `src/stewardship/mod.rs` (`sanitize_issue_text`, `find_existing`, -> `process_orchestrator_run` — the sibling filing loop this mirrors). Hermetic -> tests `src/overseer/tests_gap_scan.rs`, `src/stewardship/tests_extra.rs`. -> -> **Note on the filing seam:** the GitHub-issue filing that produced the flood -> is the stewardship seam (`filed-by: simard-stewardship`, -> `failed-step: workstream-gap-scan`, `source-module: overseer`), **not** -> `act_flag_workstream_gaps` (`src/overseer/mod.rs:1954`), which today only -> *notifies* the operator (email + Signal) and never calls `create_issue`. The -> durable, content-addressed guard must wrap the actual stewardship filing seam; -> `act_flag_workstream_gaps` keeps the in-process `WhisperGate` (900 s) -> pre-filter. +> (`GapCategory`, `GapItem::signature`, `has_valid_dedup_signature`); batch key +> and filing seam `src/overseer/mod.rs` (`workstream_gap_key`, +> `act_flag_workstream_gaps`); detectors `src/overseer/sensor.rs` +> (`detect_workstream_gaps`). The durable pattern this signature is designed to +> feed lives in `src/stewardship/gh_client.rs` and `src/stewardship/mod.rs` +> (`find_existing`, `process_orchestrator_run`). Hermetic tests +> `src/overseer/signal.rs` (`gap_dedup_key_tests`), +> `src/stewardship/tests_extra.rs`. ## At a glance | You want to… | Use | |---|---| -| Understand why duplicates stopped after a restart | This page — the durable open-issue check | -| See the stable key that dedupes a gap | `GapItem.signature` → `stewardship-signature: ` in the issue body | +| Understand why a recurring gap stopped re-notifying every tick | This page — the stable signature + in-process gate | +| See the stable key that dedupes a gap | `GapItem.signature` → `stewardship-signature: workstream-gap:` | | Know which gap kinds are deduped | The bounded [`GapCategory` taxonomy](#the-bounded-gapcategory-taxonomy) | -| Confirm a duplicate was reused, not re-filed | `overseer::gap_scan` `reused_existing=…` info log + `ActOutcome::WorkstreamGapsFlagged` | -| Operate / verify it | [How to configure and verify durable gap dedup](../howto/configure-gap-durable-dedup.md) | +| Confirm a recurring gap was deduped, not re-notified | `overseer::gap_scan` `flagged=…`/`suppressed=…` info log + `ActOutcome::WorkstreamGapsFlagged` | +| Operate / verify it | [How to configure and verify gap dedup](../howto/configure-gap-durable-dedup.md) | ## The bounded `GapCategory` taxonomy -Free-form gap titles were the root of the flood: the same underlying condition -produced slightly different strings on each tick, so no two matched an existing -open issue. The taxonomy replaces free-form detection with a **bounded enum**; -every variant carries a short, stable `.label()` slug that anchors the dedup -signature. +Free-form gap titles let the same underlying condition render slightly different +strings on each tick, so no two dedup keys matched. The taxonomy replaces +free-form detection with a **bounded enum**; every variant carries a short, +stable `.label()` slug that anchors the signature. | `GapCategory` variant | `.label()` | Signature prefix | Filed for | |---|---|---|---| @@ -105,22 +89,16 @@ signature. The taxonomy is a **bounded, closed enum** (`src/overseer/signal.rs`), so a gap can only ever resolve to one of these three stable `.label()` slugs — the same -underlying condition renders an **identical** title and signature on every tick. -The durable change is not new variants but a **stable signature**: the filed -`stewardship-signature:` moved from an unstable per-run *hash* to the -content-addressed per-gap slug below, so the already-existing open-issue search -now matches the same gap across runs and restarts. Every exhaustive `match` on -`GapCategory` (in `notify.rs`, `sensor.rs`, `observer.rs`) already covers these -three arms; **none was renamed**. (Pre-fix filed issues carried a per-run hash -signature, not these slugs, so they do not retroactively dedupe — see the format -note above.) +underlying condition renders an **identical** signature on every tick. Every +exhaustive `match` on `GapCategory` already covers these three arms; **none was +renamed**. ## The dedup key (signature) grammar Each `GapItem` carries a `signature`: a **stable, constrained slug** built from **trusted identifiers only** (a goal id, `repo#number`, or an anomaly slug) — -never from hostile free text. Identical inputs yield an identical signature, so -a recurring gap dedupes to at most one open issue. +never from hostile free text. Identical inputs yield an identical signature, so a +recurring gap dedupes to at most one live notification per daemon. The act path wraps the per-gap signature in a filing key: @@ -143,144 +121,124 @@ fn workstream_gap_key(gaps: &[GapItem]) -> String { ### Signature slug validation (injection defense) -Signatures are validated **at construction** in `signal.rs`, not at the call -sites, so a malformed or hostile identifier can never reach a `gh search` -argument or an issue body. A valid signature matches: +Signatures are validated **at construction** in `signal.rs` and re-checked at the +filing seam (`has_valid_dedup_signature` in `act_flag_workstream_gaps`), so a +malformed or hostile identifier can never reach a notification body — or a +future `gh search` argument. A valid signature matches: ```text ^[a-z0-9][a-z0-9:_#.\-/]{0,200}$ ``` Any identifier that would violate this (control characters, spaces, shell -metacharacters, over-length) is slugified/rejected at the boundary. This makes -the search query **inert**: a goal id such as `g-1; rm -rf ~` cannot inject into -the `gh issue list --search` term because it never survives slug validation. +metacharacters, over-length) is rejected at the boundary and the gap is dropped +(counted as suppressed so the observability contract stays exact). This makes any +downstream search query **inert**: a goal id such as `g-1; rm -rf ~` cannot +survive slug validation, so it can never inject into a `gh issue list --search` +term when the durable check is wired. -## The durable filing flow +## What the gap path does today -The gap-filing act path (`act_flag_workstream_gaps`) mirrors the proven -[`stewardship::process_orchestrator_run`](./stewardship-api.md) dedup flow: +The gap-notification act path (`act_flag_workstream_gaps`) runs, per tick: ```text for each detected gap: - 1. WhisperGate.peek(sig) # in-process fast pre-filter (no gh call) - └─ SuppressDuplicate/Cap → count as suppressed, continue - 2. gh.search_issues(repo, sig) # DURABLE open-issue equivalence check - └─ Err(e) → FAIL LOUD: propagate, file nothing - 3. find_existing(&issues, sig) # match `stewardship-signature: ` - └─ Some(issue) → reuse/comment/skip (no new issue) - └─ None → gh.create_issue(repo, title, body) - body embeds `stewardship-signature: ` - 4. WhisperGate.commit(sig) # record the fresh filing in-process + 1. has_valid_dedup_signature(gap) # IV-1: drop a malformed/hostile signature + └─ invalid → count suppressed, continue (never notified) + 2. WhisperGate.peek(sig) # in-process dedup (no gh call) + └─ SuppressDuplicate/Cap → count suppressed, continue + └─ Deliver → collect as a FRESH gap +if any fresh gaps: + 3. notifier.notify(workstream_gap) # ONE consolidated operator notification + # (email + Signal), never create_issue + 4. WhisperGate.commit(sig) # record the fresh notification in-process ``` -Key properties: - -- **Restart-safe.** Step 2 queries GitHub, the durable source of truth, so a - cold in-process gate after a restart still finds the already-open issue and - reuses it. The guarantee is now *"at most one open issue per distinct gap - signature"* **across restarts and daemons**, strengthening the previous - *within one process* guarantee. -- **Reuses the frozen `GhClient` seam.** No new methods, scopes, or endpoints — - the durable check composes the existing `search_issues` / `create_issue` - trait. `create_issue` still pipes the body via `--body-file -` on stdin, so - argv length and shell quoting are non-issues. -- **The `stewardship-signature:` marker is the join key.** `find_existing` - matches an open issue whose body contains `stewardship-signature: ` — - the same marker and matcher the orchestrator-failure loop uses. - -## Fail-loud contract - -If the durable open-issue search errors (`gh` unauthenticated, rate-limited, -network down), the act path **propagates the error and files nothing**. It never -falls back to a blind `create_issue` — a fail-*open* fallback would reintroduce -exactly the flood this rail prevents. - -- No `unwrap`/`expect` on the `gh` paths; every fallible step returns - `Result<_, OverseerError>` / `SimardResult<_>`. -- A search error surfaces as a first-class `OverseerError` and is - `tracing::error!`-logged on `target: "overseer::gap_scan"`; it is **not** - swallowed. -- Contrast with the in-process `BackoffGate`, which fails **toward surfacing** - (a clock regression resolves to *admit*). Both directions err on the side of - *not* silently dropping a genuine gap — the durable check refuses to file on - uncertainty, and the in-process gate refuses to hide on uncertainty. - -## Input validation & secret hygiene (before any issue write) - -Every value that reaches a `gh` argument or a public issue body is treated as -untrusted and hardened, reusing the stewardship helpers: - -- **Repo identifier** is validated against `^[A-Za-z0-9._-]+/[A-Za-z0-9._-]+$` - and every `gh` invocation uses argv (no shell) with a `--` argument - terminator, so a repo or title beginning with `-` cannot be read as a flag. -- **Titles** are truncated to ≤120 characters and stripped of control - characters (`\x00`–`\x1F` except `\n`/`\t`) to prevent log/notification - injection. -- **Body detail** derived from logs is passed through - `stewardship::sanitize_issue_text` (see `src/stewardship/mod.rs`), which - redacts `ghp_`/`gho_` GitHub tokens, AWS keys, `Bearer` tokens, and PEM - blocks to `[redacted secret]` **before** the text can enter a public issue - body. +Key properties **as implemented**: + +- **Notification-only.** Routine gap observations notify the operator; they do + **not** create GitHub issues or stewardship backlog items on this path. +- **Deduped within a process.** The stable signature makes the in-process gate + collapse a recurring gap to one notification per dedup window (default 900 s). +- **Not restart-safe on its own.** The `WhisperGate` lives in memory, so a daemon + restart resets it. Cross-restart dedup depends on the future durable check + below. +- **Fail-closed at the seam.** A malformed signature is dropped **before** it can + reach a notification body — never rendered into operator-facing text. + +## Future work: the durable cross-process check + +The stable signature exists so a durable, restart-safe check can be layered in +**without** re-deriving a key. The intended follow-on mirrors the proven +[`stewardship::process_orchestrator_run`](./stewardship-api.md) flow: + +```text +for each detected gap (after the in-process gate): + gh.search_issues(repo, sig) # DURABLE open-issue equivalence check + └─ Err(e) → FAIL LOUD: propagate, file nothing + find_existing(&issues, sig) # match `stewardship-signature: ` + └─ Some(issue) → reuse/comment/skip (no new issue) + └─ None → gh.create_issue(repo, title, body) +``` + +When wired, this would upgrade the guarantee to *"at most one open issue per +distinct gap signature across restarts and daemons"* and must **fail loud** — a +`gh` search error stops filing rather than falling back to a blind +`create_issue`, which would reintroduce exactly the flood this work prevents. +**This flow is not implemented on the gap-notification path today.** ## Structured observability -The rail emits structured `tracing` + OTel only — **no** `print!`/`println!`. -On `target: "overseer::gap_scan"`: +The gap path emits structured `tracing` + OTel only — **no** +`print!`/`println!`. On `target: "overseer::gap_scan"`: | Field | Meaning | |---|---| -| `flagged` | Fresh gaps filed this tick (new issues created) | -| `reused_existing` | Gaps that matched an already-open issue and were reused | -| `suppressed` | Gaps dropped by the in-process `WhisperGate` pre-filter | -| `key` | The batch `workstream_gap_key` for the tick | +| `flagged` | Fresh gaps notified this tick | +| `suppressed` | Gaps dropped by the in-process `WhisperGate` or by a malformed signature | +| `dispatched` / `all_sent` | Operator-notification delivery status | The `ActOutcome::WorkstreamGapsFlagged { flagged, suppressed }` outcome and the `OverseerTickReport.workstream_gaps_detected` / `…_suppressed` counters are -unchanged and additive; reused-existing hits count as suppressed for the tick -totals (no new issue was created). +unchanged and additive. ## Guarantees -- **At most one open issue per distinct gap signature**, across daemon - restarts and concurrent daemons (durable, GitHub-sourced). -- **Never permanently silent.** If the covering issue is closed and the gap - recurs, the `--state open` search finds nothing and a fresh issue is filed — - identical to the stewardship loop's semantics. -- **Additive / non-breaking.** A durable, content-addressed check in front of - the existing gates; every current caller and the issue-body *structure* are - untouched. The `stewardship-signature:` marker field is reused; only its *value* changes (unstable per-run hash → - stable per-gap slug), so pre-fix duplicate issues do not retro-dedupe. The PRD - (`Specs/ProductArchitecture.md` § *Stewardship Mode* / § *Goal Stewardship - Mode*) is preserved. -- **Fail-loud, never fail-open.** A `gh` search error stops filing; it never - degrades to a blind create. +- **At most one live notification per distinct gap signature within a running + daemon** (in-process, `WhisperGate`-sourced). Cross-restart / cross-daemon + dedup is **not** yet guaranteed — see [Future work](#future-work-the-durable-cross-process-check). +- **Never permanently silent.** The in-process gate window is capped and resets + after silence; a genuinely recurring gap always re-surfaces. +- **Injection-safe.** A gap whose signature is not a valid restricted slug is + dropped at the seam, so no hostile identifier reaches a notification body or a + future search term. +- **Additive / non-breaking.** Only the signature's *value* changes (unstable + per-run hash → stable per-gap slug); the `stewardship-signature:` marker field, + the notification structure, and every existing caller are untouched. Pre-fix + issues carry the old per-run hash and do not retro-dedupe. ## Security notes | Risk | Mitigation | |---|---| -| Search-query injection via an unconstrained signature | Slug-validate the signature at construction (`^[a-z0-9][a-z0-9:_#.\-/]{0,200}$`); the search term is inert | -| Fail-open regression re-introducing the flood | Propagate `search_issues` errors; never default to `create_issue` | -| Secret disclosure in a public issue body | `sanitize_issue_text` redacts token/key/PEM patterns before the write | -| `gh` flag-injection via a repo/title beginning with `-` | Repo regex validation + `--` argument terminator; argv (no shell) | -| Self-DoS from a fast detector loop | Mandatory `WhisperGate` pre-filter before any `gh` call | -| Log/notification injection via control chars | Strip `\x00`–`\x1F` (except `\n`/`\t`); truncate titles ≤120 chars | +| Search-query / notification injection via an unconstrained signature | Slug-validate the signature at construction **and** at the seam (`^[a-z0-9][a-z0-9:_#.\-/]{0,200}$`); drop on failure | +| Fail-open regression re-introducing the flood (when the durable check is wired) | Propagate `search_issues` errors; never default to `create_issue` | +| Self-DoS from a fast detector loop | Mandatory `WhisperGate` pre-filter before any external call | -No credential handling lives on this path: `gh` supplies its own token, which is -never read, logged, or embedded. The rail needs only `issues:write` — no new -scopes or endpoints. +No credential handling lives on this path: the future durable check would let +`gh` supply its own token, which is never read, logged, or embedded. ## Related - [Overseer workstream gap-scan reference](./overseer-workstream-gap-scan.md) — the `GapItem`/`GapCategory`/`Signal::WorkstreamGap` data model this extends. - [Gap-scan dedup & exponential backoff](../concepts/gap-scan-backoff-dedup.md) - — the in-process `BackoffGate` this rail's durable check backstops. + — the in-process `BackoffGate` this signature stabilizes. - [Overseer backoff-gate API reference](./overseer-backoff-gate-api.md) — the - in-process pre-filter. + in-process gate the gap path uses. - [Goal Stewardship — Orchestrator Failure API reference](./stewardship-api.md) - — the `search_issues → find_existing → create_issue` flow this mirrors. -- [How to configure and verify durable gap dedup](../howto/configure-gap-durable-dedup.md). + — the `search_issues → find_existing → create_issue` flow the future durable + check would mirror. +- [How to configure and verify gap dedup](../howto/configure-gap-durable-dedup.md). - PRD: `Specs/ProductArchitecture.md` § *Stewardship Mode* / § *Goal Stewardship Mode*. diff --git a/mkdocs.yml b/mkdocs.yml index 4e69026c8..2bc3d53c3 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -282,7 +282,7 @@ nav: - Review Overseer Workstream Gaps: howto/review-overseer-workstream-gaps.md - Diagnose Recurring Cognitive-Memory Signature: howto/diagnose-recurring-cognitive-memory-signature.md - Configure Overseer Gap-Scan Backoff: howto/configure-overseer-gap-scan-backoff.md - - Configure & Verify Durable Gap Dedup: howto/configure-gap-durable-dedup.md + - Configure & Verify Gap Dedup: howto/configure-gap-durable-dedup.md - Configure Overseer Root-Cause Principle: howto/configure-overseer-root-cause-why.md - Configure Cognitive-Thread Scheduling: howto/configure-cognitive-thread-scheduling.md - Add a New Cognitive Thread: howto/add-a-new-cognitive-thread.md @@ -321,7 +321,7 @@ nav: - Overseer Signal JSON-RPC Transport: reference/overseer-signal-jsonrpc-transport.md - Overseer Workstream Gap-Scan: reference/overseer-workstream-gap-scan.md - Overseer BackoffGate & Gap-Scan Dedup: reference/overseer-backoff-gate-api.md - - Overseer Durable Gap-Filing Dedup: reference/overseer-gap-durable-dedup.md + - Overseer Gap-Filing Dedup: reference/overseer-gap-durable-dedup.md - Overseer Root-Cause (WHY) API: reference/overseer-root-cause-why-api.md - Overseer Self-Observation Stability: reference/overseer-self-observation-stability.md - simard-engineer-step CLI: reference/simard-engineer-step.md From 8ee6640041b54e48833bd7253dd4245c7ff2238b Mon Sep 17 00:00:00 2001 From: rysweet Date: Sat, 25 Jul 2026 23:53:37 +0000 Subject: [PATCH 5/6] docs+comment: track durable-dedup follow-up as #4717; clarify byte/char bound (#4687) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builds on 678f5851c (the parallel B1 doc fix that corrected the gap-dedup docs to the shipped notify-only behavior). Adds the two remaining Step 18b items: - Tracking issue: reference #4717 (filed to track the P2 restart-safe, GitHub-side durable dedup gap) from the "Future work" sections of the reference, concept, and how-to docs, so the deferred work is discoverable. - S2: clarify the byte-vs-char length bound in `is_bounded_signature_slug` (comment/doc-comment only) — the ASCII-only alphabet makes `sig.len()` bytes equal the `char` count for every valid slug, so MAX_GAP_SIGNATURE_LEN is an exact character bound. Validation: fmt clean, release clippy (-D warnings) clean, overseer::signal 21/21 tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/concepts/gap-scan-backoff-dedup.md | 3 ++- docs/howto/configure-gap-durable-dedup.md | 3 ++- docs/reference/overseer-gap-durable-dedup.md | 3 ++- src/overseer/signal.rs | 9 ++++++++- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/docs/concepts/gap-scan-backoff-dedup.md b/docs/concepts/gap-scan-backoff-dedup.md index a526a9007..e883d3b52 100644 --- a/docs/concepts/gap-scan-backoff-dedup.md +++ b/docs/concepts/gap-scan-backoff-dedup.md @@ -137,7 +137,8 @@ any `gh` error. The gap-notification path (`act_flag_workstream_gaps`) does **not** perform this query today — it applies the in-process `WhisperGate` and notifies the operator. The proven durable pattern lives on the sibling stewardship filing seam (`stewardship::process_orchestrator_run`); wiring it -onto the gap path is the follow-on this stable signature enables. See the +onto the gap path is the follow-on this stable signature enables, tracked in +[#4717](https://github.com/rysweet/Simard/issues/4717). See the [gap-filing dedup reference](../reference/overseer-gap-durable-dedup.md) and the [how-to](../howto/configure-gap-durable-dedup.md). diff --git a/docs/howto/configure-gap-durable-dedup.md b/docs/howto/configure-gap-durable-dedup.md index 6ea7137c7..86114e8b8 100644 --- a/docs/howto/configure-gap-durable-dedup.md +++ b/docs/howto/configure-gap-durable-dedup.md @@ -38,7 +38,8 @@ For the data model, signature grammar, and guarantees, see the > **Scope.** The gap-notification path dedupes via the **in-process** > `WhisperGate` and notifies the operator; it does **not** create GitHub issues > and is **not restart-safe on its own** — a daemon restart resets the gate. A -> durable, GitHub-sourced cross-process check is scoped as follow-on work and is +> durable, GitHub-sourced cross-process check is scoped as follow-on work +> ([#4717](https://github.com/rysweet/Simard/issues/4717)) and is > **not** wired on this path yet; see the reference doc's *Future work* section. ## What changed for operators diff --git a/docs/reference/overseer-gap-durable-dedup.md b/docs/reference/overseer-gap-durable-dedup.md index bcee47809..c2a1ef735 100644 --- a/docs/reference/overseer-gap-durable-dedup.md +++ b/docs/reference/overseer-gap-durable-dedup.md @@ -185,7 +185,8 @@ When wired, this would upgrade the guarantee to *"at most one open issue per distinct gap signature across restarts and daemons"* and must **fail loud** — a `gh` search error stops filing rather than falling back to a blind `create_issue`, which would reintroduce exactly the flood this work prevents. -**This flow is not implemented on the gap-notification path today.** +**This flow is not implemented on the gap-notification path today; it is +tracked in [#4717](https://github.com/rysweet/Simard/issues/4717).** ## Structured observability diff --git a/src/overseer/signal.rs b/src/overseer/signal.rs index dd1eea51f..c17574304 100644 --- a/src/overseer/signal.rs +++ b/src/overseer/signal.rs @@ -235,12 +235,19 @@ impl GapItem { } /// Validate that a gap signature is a bounded slug: non-empty, at most -/// [`MAX_GAP_SIGNATURE_LEN`] chars, beginning with an ASCII alphanumeric, and +/// [`MAX_GAP_SIGNATURE_LEN`] bytes (which equals characters for the ASCII-only +/// alphabet enforced here), beginning with an ASCII alphanumeric, and /// composed solely of the restricted alphabet `[A-Za-z0-9:_#./-]`. This is the /// IV-1 dedup-search-injection defense: no whitespace, quoting, or shell/search /// metacharacter can appear in a key that is later embedded in a `gh` search /// query or an issue body, regardless of how exotic the source identifier is. pub fn is_bounded_signature_slug(sig: &str) -> bool { + // `sig.len()` is the UTF-8 byte length. Because the character check below + // admits only ASCII bytes (alphanumerics + the restricted separators), a + // slug that passes this function is pure ASCII, so its byte length equals + // its `char` count — the `MAX_GAP_SIGNATURE_LEN` bound is therefore an exact + // character bound for every *valid* slug. Checking bytes up front also + // cheaply rejects an over-long multi-byte input before the per-char scan. if sig.is_empty() || sig.len() > MAX_GAP_SIGNATURE_LEN { return false; } From 9a5288a1c339bc2e1ed394f3f3b50bb225385413 Mon Sep 17 00:00:00 2001 From: rysweet Date: Sun, 26 Jul 2026 00:23:13 +0000 Subject: [PATCH 6/6] docs(overseer): correct gap-scan reference to notify-only reality (#4687) The gap-scan reference still described act_flag_workstream_gaps as filing a deduped stewardship GitHub issue (a 'FileIssue' act step, a 'stewardship' dedup layer, and 'reused' find_existing plumbing). The shipped code only peeks/commits the in-process WhisperGate and emits one operator notification; it files no issue. The real IssueFiler/FileIssue path is reached by the QualityRegression CI-cluster problem, not the gap scan. Corrected the Zero-BS violations to match code: - dedup is in-process on both paths (notify WhisperGate; coverage in-flight + exponential backoff), not a gate+stewardship two-layer with issue upsert - removed the false 'FileIssue' act step; documented notify-only side effect - durable cross-process GitHub open-issue check is future work (#4717) - fixed at-a-glance/invariants/security bullets that implied issue filing Validated: cargo test --test docs_integrity (4 passed; all intra-repo links resolve). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../reference/overseer-workstream-gap-scan.md | 106 ++++++++++-------- 1 file changed, 60 insertions(+), 46 deletions(-) diff --git a/docs/reference/overseer-workstream-gap-scan.md b/docs/reference/overseer-workstream-gap-scan.md index f2bab1804..9b05b4788 100644 --- a/docs/reference/overseer-workstream-gap-scan.md +++ b/docs/reference/overseer-workstream-gap-scan.md @@ -84,7 +84,7 @@ gap reaches a person exactly once without creating recursive tracking work. | See the gaps the Overseer flagged | Dashboard **Overseer** tab / TUI **Overseer** pane / `simard status` → **OVERSEER** — each tick's line reads e.g. `flagged 2 workstream gaps` | | Read the gaps as JSON | `GET /api/overseer` → `data.recent[].report.workstream_gaps_detected` / `…_suppressed` | | Get told when a genuine gap appears | The deduped operator notification (email + Signal), kind `workstream-gap` | -| Find the filed gap issues | GitHub issues in `rysweet/Simard` opened by the Overseer's identity (`simard-overseer[bot]`) with the `workstream-gap` signature | +| See what covers a gap | The gated coverage `LaunchRecipe` (`WORKSTREAM_COVERAGE_GROUP`) the gap-scan decides to since #4128 — the gap scan itself files **no** GitHub issue | | Turn the scan up, down, or off | `SIMARD_OVERSEER_GAP_SCAN` + `SIMARD_OVERSEER_GAP_SCAN_EVERY_N` (see [Configuration](#configuration)) | ## What counts as a gap @@ -171,7 +171,8 @@ pub struct GapItem { /// is uncovered (e.g. "p1 goal with no engineer and no PR"). pub why_it_matters: String, /// Stable dedup signature for this gap (see the signature grammar below). - /// Used to de-duplicate notifications and filed issues across ticks. + /// Used to de-duplicate operator notifications (and coverage launches) across + /// ticks. pub signature: String, } @@ -264,8 +265,9 @@ contract: ## Signature grammar and de-duplication -Every gap carries a **stable signature** so the same recurring gap is notified -and filed **at most once**, matching the existing M1 dedup behaviour. The +Every gap carries a **stable signature** so the same recurring gap is acted on +(notified, or covered by a launch) **at most once** per window within a running +daemon, matching the existing M1 dedup behaviour. The signature is a restricted slug — only `[A-Za-z0-9_\-#/:]`, never raw titles, quotes, or newlines (input-validation guideline **V3**): @@ -275,20 +277,29 @@ quotes, or newlines (input-validation guideline **V3**): | `IssueUncovered` | `issue:/#` | `issue:rysweet/Simard#2630` | | `AnomalyUnaddressed` | `anomaly:` | `anomaly:distill_parse_fail` | -The dedup key committed to the gate is `workstream-gap:`. De-dup runs -in **two layers**, so neither a fast tick cadence nor a restart floods the -operator: - -1. **Gate layer.** Before acting on a gap, `act_flag_workstream_gaps` **peeks** the - `WhisperGate` for `workstream-gap:`; if already committed within the - window it records a *suppressed* outcome and skips. On success it **commits** - the key. The gate **fails closed** on an identity/read error (guardrail - **A2**) — an indeterminate gate suppresses rather than risks a duplicate. -2. **Stewardship layer.** The filed issue still goes through - `stewardship::{failure_signature, find_existing}`, so even across gate resets a - matching open issue is updated, not duplicated. - -The result: **one deduped item per recurring gap signature**, not one per tick. +The dedup key committed to the gate is `workstream-gap:`. De-duplication +is **in-process** on both act paths, so a fast tick cadence does not flood the +operator or the backlog: + +1. **Notify path (`act_flag_workstream_gaps`).** Before notifying, it **peeks** the + in-process `WhisperGate` (`gap_gate`) for `workstream-gap:`; if + already committed within the window it records a *suppressed* outcome and + skips. On success it **commits** the key. The gate **fails closed** on an + identity/read error (guardrail **A2**) — an indeterminate gate suppresses + rather than risks a duplicate. This path **only notifies the operator**; it + does **not** file or upsert a GitHub issue. +2. **Coverage path (`LaunchRecipe`, the #4128 default).** An equivalent covering + launch is **held** while one is already in flight (`inflight_investigations`) + and, after it completes, suppressed within a growing bounded window by the + in-process exponential `coverage_backoff` gate (keyed by `recipe_dedup_key`). + +Both gates are **in-memory**, so their state is per-process. A durable, +cross-process GitHub open-issue equivalence check that would survive a daemon +restart is **future work**, tracked in +[#4717](https://github.com/rysweet/Simard/issues/4717); see the +[gap-filing dedup reference](./overseer-gap-durable-dedup.md). The result today is +**one deduped item per recurring gap signature within a running daemon**, not one +per tick. ## Act path — the coverage closing edge (issue #4128, D3b) @@ -318,28 +329,23 @@ The result: **one deduped item per recurring gap signature**, not one per tick. When it is invoked directly, `act_flag_workstream_gaps` (`src/overseer/mod.rs`) acts through the Overseer's **existing** escalation -machinery — the same paths `goal_health` and M1 use — with no new bypass: +machinery — the same notifier `goal_health` and M1 use — with no new bypass: -1. **De-dupe** each gap via the gate (above); suppressed gaps are counted, not - acted on. +1. **De-dupe** each gap via the in-process `WhisperGate` (above); suppressed gaps + are counted, not acted on. A gap whose signature is not a valid bounded slug is + dropped and counted as suppressed. 2. **NotifyOperator.** Emit **one consolidated** `OperatorNotification::workstream_gap(count, top_gaps)` on **both** channels (email + Signal) through the `DualChannelNotifier` — a notification is never silently dropped; an unconfigured channel is `Queued` and logged. -3. **FileIssue.** File a **deduped** stewardship issue per gap signature through - the same `IssueFiler` path M1 uses - (`stewardship::process_orchestrator_run` + `find_existing`). The gap briefs - carry `source_module = "overseer"`, which matches no routing keyword; the - stewardship router's **default-repo fallback** therefore routes them to - `rysweet/Simard` (the `DEFAULT_TARGET_REPO` constant) and logs the fallback - with `tracing::warn!`. This is what lets the gap-scan actually file/upsert one - rolling tracking issue per gap signature every tick, rather than failing with - `overseer intervention failed … flag_workstream_gaps … cannot route - source-module 'overseer'`. - -Both the notify and the file happen as **side effects of this one act** — exactly -as `goal_health`'s escalate notifies both channels from a single intervention. -The act then returns **one** summarising `ActOutcome` (see + +That is the **entire** side effect of this act: it **notifies the operator only**. +It does **not** file or upsert a GitHub issue, and it does **not** call +`stewardship::process_orchestrator_run` / `find_existing`. (An `IssueFiler` / +`FileIssue` path does exist on the Overseer, but it is reached by the +`QualityRegression` CI-failure-cluster problem, **not** by the gap scan.) The act +then returns **one** summarising +`ActOutcome::WorkstreamGapsFlagged { flagged, suppressed }` (see [Tick counters and totals](#tick-counters-and-totals)); it does **not** return a separate `Escalated` / `IssueFiled` outcome, so gap activity is counted only on the gap-scan's own dedicated counters, never on the generic ones. @@ -537,9 +543,11 @@ pub fn gap_scan_every_n() -> u64; The scan honours the Overseer's shared cadence (`SIMARD_OVERSEER_INTERVAL_SECS`, 15-minute default, clamped to a 60 s floor) — `EVERY_N` multiplies that interval -rather than introducing a second clock. There is no per-scan state file: dedup -lives in the shared `WhisperGate` and in GitHub issues (durable findings are -issues or code, never committed snapshot docs). +rather than introducing a second clock. There is no per-scan state file: notify +-path dedup lives in the in-process `WhisperGate` and coverage-path dedup in the +in-process in-flight / exponential-backoff guards; durable findings are issues or +code, never committed snapshot docs. A durable GitHub-side open-issue check is +future work ([#4717](https://github.com/rysweet/Simard/issues/4717)). ## Using cognitive memory (best-effort) @@ -558,11 +566,15 @@ It never blocks a tick and never writes. - **Genuine gaps only.** Every candidate is deduped against the coverage set (in-flight refs ∪ open PRs); blocked goals are delegated to `goal_health`, so the scan never re-flags work already in motion. -- **Deduped delivery.** Two-layer dedup (gate + stewardship) yields **one** - notification and **one** issue per recurring gap signature — never a flood. -- **Reuses existing plumbing.** Notify and file go through the same - `DualChannelNotifier` / `IssueFiler` paths `goal_health` / M1 use — same - escalation, same gates, no `--admin`, no `--no-verify`, no new bypass. Since +- **Deduped delivery.** In-process dedup on the notify path (the `WhisperGate` + `gap_gate`) yields **one** operator notification per recurring gap signature, + and the coverage path's in-flight + exponential-backoff guards yield **one** + covering launch per signature — never a flood. Neither path files a GitHub + issue today; a durable cross-process check is future work + ([#4717](https://github.com/rysweet/Simard/issues/4717)). +- **Reuses existing plumbing.** The notify path goes through the same + `DualChannelNotifier` `goal_health` / M1 use — same escalation, same gates, no + `--admin`, no `--no-verify`, no new bypass. Since issue #4128 a coverage gap decides to a **gated closing-edge** `LaunchRecipe` (tagged `WORKSTREAM_COVERAGE_GROUP`): it is admitted only through the existing launch gate, **fails closed** without a distinct steward identity, and is @@ -582,7 +594,7 @@ It never blocks a tick and never writes. ## Security notes External GitHub issue and PR text is **untrusted input** that flows into -notifications, filed-issue bodies, and `gh` reads. The gap-scan therefore: +notifications, launched coverage `task_description`s, and `gh` reads. The gap-scan therefore: - **A1/A2:** exposes the new counter only on the already `require_auth`-gated `/api/overseer` (no new/unauthenticated route); `act_flag_workstream_gaps` **fails @@ -602,7 +614,7 @@ notifications, filed-issue bodies, and `gh` reads. The gap-scan therefore: - **S1/S2/S3:** the only launch the gap-scan adds — the issue #4128 coverage closing edge — stays **behind the existing launch gate** plus a fail-closed steward-identity guard and in-flight dedup (no unguarded auto-launch path); - two-layer dedup prevents notify/issue floods; the `SIMARD_OVERSEER_GAP_SCAN` + in-process dedup prevents notification and duplicate-launch floods; the `SIMARD_OVERSEER_GAP_SCAN` kill-switch is honoured (its opt-out holds the coverage launch too). ## Testing @@ -637,6 +649,8 @@ built on synthetic pictures — no network, no real `gh`, no clock dependence: - [Overseer design](../design/overseer.md) — the meta-OODA loop, the capability/guardrail model, and the `goal_health` pattern this scan mirrors. - [Stewardship API](./stewardship-api.md) — the deduped issue-filing path - (`failure_signature` / `find_existing`) reused here. + (`failure_signature` / `find_existing`) used by the Overseer's + `QualityRegression` CI-cluster path; the gap scan does **not** file issues but + shares the same signature-stability philosophy. - [No-progress breaker API](./no-progress-breaker-api.md) — the "needs human review" marker delegated to `goal_health` rather than re-flagged as a gap.