Update documentation with 21 changed files (#4744)

docs/reference/claim-reaper-convergence.md

@@ -0,0 +1,207 @@
+---
+title: "Reference: Claim-Reaper Convergence & Idempotent Archival"
+description: >
+  The terminal Converged verdict and the idempotent per-claim archival guard that
+  stop the stale-engineer investigation from looping on verdict=pending for
+  standing / perpetual research goals. Covers the freshness-window archival dedup
+  (reuse-in-place, minted=false) and the guarantee of one terminal decision +
+  bounded archival (no 59x re-archival every tick) (issue #4755).
+last_updated: 2026-07-26
+review_schedule: as-needed
+owner: simard
+doc_type: reference
+status: implemented
+related:
+  - ./claim-reaper-api.md
+  - ./investigate-stale-engineer-api.md
+  - ./tombstoned-goal-engineer-reaper-api.md
+  - ../howto/investigate-a-stale-engineer-before-reap.md
+  - ../howto/diagnose-perpetual-completion-recuration.md
+  - ../operations/claim-reaper-kill-switch.md
+---
+
+# Reference: Claim-Reaper Convergence & Idempotent Archival
+
+> **Status: implemented.** Present-tense description of shipped behaviour.
+> Primary source:
+> [`src/overseer/claim_reaper.rs`](https://github.com/rysweet/Simard/blob/main/src/overseer/claim_reaper.rs).
+> Tracked by [issue #4755](https://github.com/rysweet/Simard/issues/4755).
+
+## Overview
+
+The stale-engineer investigation seam
+([`StaleEngineerInvestigator`](./investigate-stale-engineer-api.md)) archives an
+engineer's diagnostic evidence and returns an [`InvestigationVerdict`] before the
+reaper decides whether to reclaim the claim. For a **standing / perpetual
+research goal**, the investigation legitimately never reaches `Dead` — but it
+also never reached a *terminal* state. Each Overseer tick re-investigated the
+same still-alive engineer, re-archived byte-identical evidence, and returned
+`verdict=pending`. In production this produced **59× re-archival** of the same
+evidence for stale engineer `70ab8541` and unbounded growth of
+`reaped-engineers/`, with per-cycle band-aid PRs (#4608, #4642) repeatedly
+persisting the same "fail-closed still-alive" verdict.
+
+The two additive mechanisms are **coupled through the `minted` flag**: the first
+tick that mints a fresh evidence epoch (`minted = true`) dispatches the
+investigation and returns `Pending`; every later tick within the freshness window
+reuses that epoch in place (`minted = false`) and returns the terminal
+**`Converged`** — so a standing goal reaches a stable decision instead of spinning:
+
+1. a terminal **`Converged`** verdict — a stable, fail-closed decision that a
+   standing goal's engineer has already been investigated this window and needs no
+   further re-dispatch this run; and
+2. the existing **idempotent per-claim archival guard** — a freshness-window
+   dedup ([`find_recent_archive_epoch`] + [`ARCHIVE_FRESHNESS_WINDOW`]) that
+   REUSES a within-window evidence epoch in place (`minted = false`) instead of
+   minting a new `<key>-<ts>/` directory every tick, and whose `minted = false`
+   result is exactly what the seam maps to `Converged`.
+
+Result: a standing-goal stale engineer reaches **one terminal verdict** and its
+evidence is archived **at most once per freshness window** (not every tick), so
+`reaped-engineers/` stops growing unboundedly.
+
+## The `Converged` verdict
+
+`Converged` is an additive, non-terminal-for-reaping variant of
+[`InvestigationVerdict`]. Like every non-`Dead` verdict it is **fail-closed**:
+it KEEPS the claim (`should_reap()` stays `false`). It differs from `Pending`
+in that it is *stable* — it marks a standing goal as fully investigated for this
+run, so the seam does not treat it as an outstanding, must-resolve investigation.
+
+```rust
+// src/overseer/claim_reaper.rs
+pub enum InvestigationVerdict {
+    /// FALSE POSITIVE — the engineer is actually still working. Never reaped.
+    #[default]
+    StillAlive,
+    /// Stuck on a missing precondition but not dead. Never reaped.
+    Blocked,
+    /// Died from a TRANSIENT condition a relaunch would clear. Not reaped.
+    Recoverable,
+    /// The agentic investigation is still IN FLIGHT. Not reaped; a later sweep
+    /// resolves it.
+    Pending,
+    /// Investigation reached a STABLE terminal decision for a standing /
+    /// perpetual goal: fully investigated, no further re-investigation or
+    /// re-archival this run. Never reaped (fail-closed like every non-Dead
+    /// verdict).
+    Converged,
+    /// Genuinely gone AND unrecoverable. The ONLY verdict that reaps.
+    Dead { cause: InvestigationCause },
+}
+```
+
+`should_reap()` remains `matches!(self, Dead { .. })` — `Converged` never reaps.
+`label()` returns the stable, log-safe token `"converged"`.
+
+> **Label stability, not serialization.** Verdicts are **never serialized or
+> persisted** — the reaper archives an engineer's *evidence* under
+> `reaped-engineers/` (a hand-written `manifest.json` of claim key / goal id /
+> idle age / timestamp / worktree), never the `InvestigationVerdict` itself, and
+> the type derives no serde. `label()` is a stable, name-based token used only in
+> fail-visible log lines. Inserting `Converged` before `Dead` (for readability)
+> therefore shifts no existing token, so existing log tooling / greps for
+> `"pending"`, `"dead"`, etc. keep matching unchanged. No migration is involved.
+
+### Pending vs. Converged
+
+| Verdict | Meaning | Re-investigates next tick? | Reaps? |
+| --- | --- | --- | --- |
+| `Pending` | Investigation launched, not yet resolved | Yes (a later sweep resolves it) | No |
+| `Converged` | Standing goal fully investigated; stable decision | No (terminal for this run) | No |
+
+## Idempotent per-claim archival guard
+
+The re-archival half of the loop is bounded by the reaper's **existing**
+per-claim freshness-window dedup in
+[`archive_stale_engineer_evidence`](https://github.com/rysweet/Simard/blob/main/src/overseer/claim_reaper.rs)
+— no new guard store is introduced.
+
+Before minting a fresh archive directory, the seam calls
+[`find_recent_archive_epoch`]: if a `reaped-engineers/<sanitized_key>-<ts>/`
+evidence epoch for THIS claim already exists within
+[`ARCHIVE_FRESHNESS_WINDOW`] (1 hour), that epoch is **reused in place**
+(`ArchiveOutcome { minted: false, .. }`) rather than creating a sibling
+timestamped directory. Only a freshly-minted epoch (`minted = true`) writes the
+`manifest.json` / `evidence.txt` / `journal.txt` (so the bounded `journalctl`
+capture runs at most once per epoch, not every tick).
+
+```rust
+// src/overseer/claim_reaper.rs — reuse a within-window epoch in place.
+if let Some(existing) = find_recent_archive_epoch(&archive_root, &sanitized, ts) {
+    return Ok(ArchiveOutcome { dir: existing, minted: false });
+}
+```
+
+The `<ts>` is parsed from the directory NAME (not its mtime), so the window is
+derived from the archive epoch itself and is robust to later in-place refreshes.
+
+This is:
+
+- **bounded** — at most one minted archive per claim per window, so a standing
+  goal re-investigated every ~15-minute tick archives once per hour, not 59×;
+- **fail-closed** — an I/O error minting or reading the archive is surfaced as
+  `Err`, and the caller keeps the claim (never reaps without preserved evidence),
+  never fabricating a `Converged`.
+
+## Configuration
+
+Convergence is on by default and requires no configuration to stop the loop. The
+existing reaper kill-switch and interval knobs
+(`SIMARD_CLAIM_REAP_*`, see the
+[claim-reaper kill switch runbook](../operations/claim-reaper-kill-switch.md))
+continue to govern the sweep.
+
+## Examples
+
+### A standing research goal converges once
+
+```text
+tick 1  investigate claim=engineer:70ab8541
+        mint archive epoch 70ab8541-<ts> (first time) → dispatch investigation → verdict=pending
+tick 2  investigate claim=engineer:70ab8541
+        within-window epoch exists → REUSE in place (minted=false) → verdict=converged
+tick N  … same reuse-in-place: one terminal verdict, archival bounded to once per
+        window, no reaped-engineers/ growth every tick
+```
+
+Before this change the same sequence produced:
+
+```text
+tick 1..59  re-investigate → verdict=pending (never terminal)
+            reaped-engineers/ churned every tick; PRs #4608/#4642 re-persist
+```
+
+## Fail-closed guarantees
+
+- `Converged` **never reaps** — it keeps the claim like every non-`Dead` verdict.
+- The archival guard **never fabricates** a `Converged` on failure: an I/O error
+  folds to `StillAlive` (claim kept, no reap). On success the seam derives the
+  verdict from the `minted` flag — a freshly minted epoch is `Pending` (dispatch),
+  a reused-in-place epoch is `Converged` (terminal) — never reaping either way.
+- A new archive epoch is minted once the freshness window elapses, so a genuinely
+  progressing or newly-dead engineer is still re-investigated and can reach
+  `Recoverable` / `Dead`.
+
+## Regression tests
+
+Co-located in
+[`src/overseer/claim_reaper.rs`](https://github.com/rysweet/Simard/blob/main/src/overseer/claim_reaper.rs):
+
+- `standing_goal_converges_to_single_verdict` — a standing-goal stale engineer on
+  a `Converged` outcome is never reaped across 59 sweeps (mirrors the observed
+  59× non-convergence loop); the claim is preserved and its worktree never cleaned.
+- `converged_never_reaps` — `Converged.should_reap()` is `false` and its label is
+  the stable token `"converged"`.
+- `converged_label_does_not_shift_existing_verdict_labels` — adding `Converged`
+  leaves every existing name-tagged label unchanged (no persisted-verdict
+  migration required).
+- `converged_is_kept_like_pending` — both `Pending` and `Converged` keep the
+  claim (fail-closed).
+
+## Related
+
+- [Stale-Engineer-Claim Reaper API](./claim-reaper-api.md)
+- [Investigate-Before-Reap API](./investigate-stale-engineer-api.md)
+- How-to: [Diagnose perpetual completion re-curation](../howto/diagnose-perpetual-completion-recuration.md)
+- Runbook: [Claim-reaper kill switch](../operations/claim-reaper-kill-switch.md)

docs/reference/engineer-inspect-worktree-resolution.md

@@ -0,0 +1,191 @@
+---
+title: "Reference: Engineer-Inspect Worktree Resolution"
+description: >
+  How the engineer-loop inspect phase resolves the engineer's real worktree
+  before probing it, the additive SimardError::MissingWorktree variant that
+  distinguishes an absent worktree from a NotARepo failure, and the fail-closed
+  guarantee that a valid-but-idle engineer is never NOT_A_REPO reaped
+  (issue #4744).
+last_updated: 2026-07-26
+review_schedule: as-needed
+owner: simard
+doc_type: reference
+status: implemented
+related:
+  - ./claim-reaper-api.md
+  - ./investigate-stale-engineer-api.md
+  - ./engineer-worktree-isolation.md
+  - ./engineer-claim-release-api.md
+  - ../howto/inspect-and-clean-engineer-worktrees.md
+  - ../howto/investigate-a-stale-engineer-before-reap.md
+---
+
+# Reference: Engineer-Inspect Worktree Resolution
+
+> **Status: implemented.** Present-tense description of shipped behaviour.
+> Primary sources:
+> [`src/engineer_loop/mod.rs`](https://github.com/rysweet/Simard/blob/main/src/engineer_loop/mod.rs),
+> [`src/engineer_worktree/claim.rs`](https://github.com/rysweet/Simard/blob/main/src/engineer_worktree/claim.rs),
+> [`src/error/mod.rs`](https://github.com/rysweet/Simard/blob/main/src/error/mod.rs).
+> Tracked by [issue #4744](https://github.com/rysweet/Simard/issues/4744).
+
+## Overview
+
+The engineer-loop **inspect phase** examines an engineer's working tree to
+decide whether the engineer is making progress. Previously the phase could probe
+a synthetic, non-repository path (for example a bare `/tmp` directory that is not
+a git worktree). `git` returned exit code 128 (`fatal: not a git repository`),
+the inspection surfaced [`SimardError::NotARepo`], and the engineer was recorded
+as producing nothing. A healthy-but-idle engineer was then **false-stale reaped**,
+discarding whole engineering loops (the goal-board blocker behind `7f5afcca` and
+the repeated no-action/blocked cycles observed in `simard status`).
+
+Inspect now **resolves the engineer's real worktree** at the probe seam before
+running any `git` command, and distinguishes three outcomes:
+
+| Situation | Outcome | Reap? |
+| --- | --- | --- |
+| Valid worktree, engineer idle | Healthy inspection, `worktree_dirty` reflects real state | **No** |
+| Worktree directory genuinely absent | [`SimardError::MissingWorktree`] | Handled distinctly — not a `NotARepo` false positive |
+| A path that exists but is not a git repo | [`SimardError::NotARepo`] | Only genuine non-repos |
+
+The invariant: **a valid engineer worktree never yields `NotARepo`.**
+
+## Worktree resolution seam
+
+The inspect phase no longer accepts an arbitrary caller-supplied path as the repo
+root. It resolves the worktree the engineer loop already tracks through
+`engineer_worktree`:
+
+```rust
+// src/engineer_worktree/claim.rs
+/// Resolve the on-disk worktree path for the engineer holding `claim_key`.
+///
+/// Returns the canonicalized worktree root when the directory exists and lives
+/// under the managed engineer-worktree root. Returns `SimardError::MissingWorktree`
+/// when the claim is known but its worktree directory is absent (reaped, swept,
+/// or never allocated) — a distinct, fail-closed signal, never `NotARepo`.
+pub fn resolve_engineer_worktree(claim_key: &str) -> SimardResult<PathBuf>;
+```
+
+`inspect_workspace` (in `engineer_loop/mod.rs`) is driven from the resolved path:
+
+```rust
+// src/engineer_loop/mod.rs
+pub fn inspect_workspace(workspace_root: &Path, state_root: &Path) -> SimardResult<RepoInspection>;
+```
+
+The caller resolves the worktree first, so the `workspace_root` passed to
+`inspect_workspace` is always the engineer's real, canonicalized tree — never a
+synthetic `/tmp` default.
+
+### Path safety
+
+Resolution is defensive by construction:
+
+- the resolved path is **canonicalized** (`fs::canonicalize`), collapsing `..`
+  and resolving symlinks;
+- the canonical path is confirmed to live **within the managed engineer-worktree
+  root**; a symlink that escapes the root is rejected;
+- no engineer-controlled path fragment is ever interpolated into a shell — all
+  `git` invocations use argv arrays.
+
+## API
+
+### `SimardError::MissingWorktree`
+
+An additive variant of the crate error enum
+([`src/error/mod.rs`](https://github.com/rysweet/Simard/blob/main/src/error/mod.rs)).
+It is **non-breaking**: existing `match` arms that already handle `NotARepo`
+continue to compile because `MissingWorktree` is a new, separate arm.
+
+```rust
+pub enum SimardError {
+    // ...
+    /// The path is a real git repository but could not be inspected.
+    NotARepo { path: PathBuf, reason: String },
+
+    /// A known engineer claim's worktree directory is absent.
+    ///
+    /// Distinct from `NotARepo`: the engineer is not "not a repo", the worktree
+    /// simply does not exist on disk (reaped, swept, or never allocated). The
+    /// reaper treats this as a genuinely-missing worktree, NOT as a healthy
+    /// engineer producing nothing, so it never triggers a false-stale reap of a
+    /// live-but-idle engineer.
+    MissingWorktree { claim_key: String, expected_path: PathBuf },
+    // ...
+}
+```
+
+Its `Display` renders a log-safe, PII-free line (claim key + expected path, no
+secrets, no raw subprocess output).
+
+### Inspection outcomes
+
+| Return | Meaning | Reaper interpretation |
+| --- | --- | --- |
+| `Ok(RepoInspection)` | Worktree resolved and inspected | Idle ≠ dead; `worktree_dirty` reflects real changes |
+| `Err(MissingWorktree { .. })` | Worktree directory genuinely absent | Distinct missing-worktree signal (fail-closed) |
+| `Err(NotARepo { .. })` | Path exists but is not a git repo | Only genuine non-repos |
+
+## Configuration
+
+No new configuration knobs. Behaviour is additive and on by default; the managed
+engineer-worktree root is the existing one used by
+[`engineer_worktree`](./engineer-worktree-isolation.md).
+
+## Examples
+
+### A valid, idle engineer is inspected — not reaped
+
+```text
+inspect: claim=engineer:goal-7f5afcca worktree=/…/worktrees/eng-7f5afcca
+inspect: worktree_dirty=false  (engineer idle, checkpoint resumable)
+reaper : verdict=still-alive  (idle ≠ dead) → claim KEPT
+```
+
+Before this change the same engineer produced:
+
+```text
+inspect: NOT_A_REPO (git exit 128) path=/tmp/…
+reaper : engineer produced nothing → FALSE-STALE REAP
+```
+
+### A genuinely-missing worktree is reported distinctly
+
+```text
+inspect: MissingWorktree claim=engineer:goal-abc expected=/…/worktrees/eng-abc
+```
+
+This is surfaced as its own outcome rather than being conflated with a
+`NotARepo` failure of a healthy engineer.
+
+## Fail-closed guarantees
+
+- A valid engineer worktree **never** yields `NotARepo`.
+- An **idle** engineer (no new files, resumable checkpoint) is distinguished from
+  a **dead** one; idleness alone never reaps.
+- A genuinely absent worktree is a distinct, explicit signal
+  (`MissingWorktree`), keeping the reap decision honest.
+
+## Regression tests
+
+Co-located in
+[`src/engineer_loop/mod.rs`](https://github.com/rysweet/Simard/blob/main/src/engineer_loop/mod.rs)
+and [`src/error`](https://github.com/rysweet/Simard/tree/main/src/error):
+
+- `inspect_on_valid_worktree_is_not_not_a_repo` — inspecting a real engineer
+  worktree returns `Ok(..)`, never `NotARepo`.
+- `inspect_resolves_engineer_worktree_not_synthetic_tmp` — the probe targets the
+  resolved worktree, not a `/tmp` default.
+- `missing_worktree_is_distinct_from_not_a_repo` — an absent worktree yields
+  `MissingWorktree`, and the two variants are not equal.
+- `valid_idle_engineer_is_not_false_stale_reaped` — an idle-but-live engineer is
+  kept, closing the #4744 false-reap chain.
+
+## Related
+
+- [Stale-Engineer-Claim Reaper API](./claim-reaper-api.md)
+- [Investigate-Before-Reap API](./investigate-stale-engineer-api.md)
+- [Engineer-Worktree Isolation](./engineer-worktree-isolation.md)
+- How-to: [Inspect and clean engineer worktrees](../howto/inspect-and-clean-engineer-worktrees.md)