diff --git a/.dochygiene-rules.json b/.dochygiene-rules.json new file mode 100644 index 0000000..36878e7 --- /dev/null +++ b/.dochygiene-rules.json @@ -0,0 +1,44 @@ +{ + "schema_version": 1, + "rules": [ + { + "glob": "autoresearch/*/", + "lifetime": "temporary", + "retain_recent": 3, + "max_age_days": 30, + "confirmed_by": "human", + "confirmed_on": "2026-07-15", + "source": "calibration pass #1 (lifecycle-aware-doc-hygiene)", + "note": "Autoresearch run directories are disposable run output; keep the 3 newest, age out the rest." + }, + { + "glob": "docs/orchestration-audit/factsheets/*.md", + "lifetime": "temporary", + "retain_recent": 10, + "max_age_days": 90, + "confirmed_by": "human", + "confirmed_on": "2026-07-15", + "source": "calibration pass #1 (lifecycle-aware-doc-hygiene)", + "note": "Regenerable precompute for the orchestration IRL audit; the audit skill rebuilds them on each run." + }, + { + "glob": "plugins/*/.pytest_cache/", + "lifetime": "temporary", + "retain_recent": 0, + "max_age_days": 7, + "confirmed_by": "human", + "confirmed_on": "2026-07-15", + "source": "calibration pass #1 (lifecycle-aware-doc-hygiene)", + "note": "Regenerable pytest cache. Judge noted a .gitignore entry may be preferable long-term." + }, + { + "glob": "plugins/*/HANDOFF-*.md", + "lifetime": "delete-once-served", + "served_when": "The handoff it describes has been picked up and its follow-on work completed in a later session.", + "confirmed_by": "human", + "confirmed_on": "2026-07-15", + "source": "calibration pass #1 (lifecycle-aware-doc-hygiene)", + "note": "Classifier-judged served_when — always confirm-tier by ADR-0039; never auto-deleted." + } + ] +} diff --git a/docs/implementation-status.md b/docs/implementation-status.md index 86312c3..405b967 100644 --- a/docs/implementation-status.md +++ b/docs/implementation-status.md @@ -68,6 +68,14 @@ entry is in the leaf file named. convention + derived global project index (issue #27, ADR-0034); wakeup-polling + tmux-convention spike with dry-run pilot (issue #28, ADR-0035/0036). Detail: os-context / os-doc-hygiene / os-backlog leaves. +- **2026-07-14** — os-sdlc plugin scaffolded (ADR-0037): new plugin inside cc-os (not a + separate marketplace) for a harness-driven SDLC lifecycle, adapting Matt Pocock's v1.1 + skill lifecycle and Delta Refinery's multi-level pipeline; registered in local-plugins, + no skills/agents/hooks implemented yet. Launching-point doc: `plugins/os-sdlc/OVERVIEW.md`; + full design deferred to a follow-up brainstorming session. +- **2026-07-15** — os-doc-hygiene lifecycle-aware hygiene shipped (ADR-0038–0041): rulebook + layer, lifetime taxonomy + tier matrix, no-ignore-propagation, conventions.json promotion, + new `:calibrate` skill; calibration pass #1 (cc-os) passed. Detail: os-doc-hygiene leaf. **Remaining optional items:** additional project onboarding (one at a time, per ADR-0013); bulk vault migration; os-adr rollout to pilot projects; os-backlog routing rollout (#14); diff --git a/docs/implementation-status/os-doc-hygiene.md b/docs/implementation-status/os-doc-hygiene.md index b11e50b..52dc09d 100644 --- a/docs/implementation-status/os-doc-hygiene.md +++ b/docs/implementation-status/os-doc-hygiene.md @@ -29,3 +29,17 @@ _Leaf file of [../implementation-status.md](../implementation-status.md). Read o `check`/`clean`; the `commands/hygiene.md` dispatcher was removed in favor of two new skills (`status`, `sweep`), aligning with the no-`commands/`-directory pattern (2026-07-03). +- **Lifecycle-aware hygiene (2026-07-15, ADR-0038–0041)** — adds a rulebook layer on top of + the existing stale/bloat scanner: a global `rulebook.json` plus an optional per-project + `.dochygiene-rules.json` declare lifetime rules for known-temporary artifacts (ADR-0038). + Rules classify matched paths into the lifetime taxonomy and drive a tier matrix from + scanner-proven+tracked+clean (`auto`) down to classifier-judged (`confirm`) (ADR-0039); an + IGNORE sentinel prunes matched paths with zero emission and does not propagate ignore + status to children (ADR-0040). Classifier-judged matches that recur are surfaced + deterministically as `promotion_candidates` from `conventions.json`, not model-authored + (ADR-0041). New `/os-doc-hygiene:calibrate` skill runs the cluster/nominate/judge/ + human-report protocol to tune rules against a real project. Calibration pass #1 (cc-os) + ran 2026-07-14/15 and passed both the protected-set hard gate and the recall floor across + all 4 mandatory rows; rule persistence to `.dochygiene-rules.json` is pending human + approval. Results: `plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/calibration-pass-1-results.md`. + Design: `plugins/os-doc-hygiene/lifecycle-spec.md`. Suite: 407 passed. diff --git a/plugins/os-doc-hygiene/CLAUDE.md b/plugins/os-doc-hygiene/CLAUDE.md index 5169b24..e1dde6c 100644 --- a/plugins/os-doc-hygiene/CLAUDE.md +++ b/plugins/os-doc-hygiene/CLAUDE.md @@ -7,6 +7,15 @@ zero-token) on `SessionStart`; checks and cleans on demand via skills. > Read `PRD.md` first — it is the source of truth for scope, decisions, and > rationale. This file is the build map. +## Lifecycle layer (2026-07-15) + +A rulebook layer sits on top of the scan/classify/clean core: global +`rulebook.json` + optional per-project `.dochygiene-rules.json` declare +lifetime rules for known-temporary artifacts, feeding tier derivation and a +`/os-doc-hygiene:calibrate` skill for tuning rules against a real project. +Read `lifecycle-spec.md` before touching `rulebook.py`, the scanner's +lifecycle signals, or the calibrate skill; decisions are ADR-0038–0041. + ## Core distinction (do not conflate) - **Stale** = the doc is *wrong* (contradicted, orphaned, superseded, diff --git a/plugins/os-doc-hygiene/conventions.json b/plugins/os-doc-hygiene/conventions.json new file mode 100644 index 0000000..f15568e --- /dev/null +++ b/plugins/os-doc-hygiene/conventions.json @@ -0,0 +1,19 @@ +[ + { + "name": "archive-bucket", + "what_it_proves": "The file has been moved into a sibling archive/ directory — a completed doc has physically left the live tree (precedent: openspec changes).", + "graduation_template": { + "served_when_path": "{dir}/archive/{name}" + }, + "pitch": "Move done docs into a sibling archive/ dir once, and the rule graduates from a confirm-gated guess to a silent, scanner-provable auto-delete." + }, + { + "name": "status-frontmatter", + "what_it_proves": "The file's own frontmatter carries status: shipped or status: done — the file stays put and the scanner reads the key directly.", + "graduation_template": { + "frontmatter_key": "status", + "allowed_values": ["shipped", "done"] + }, + "pitch": "Add a status: shipped|done frontmatter key once, and every future check reads it deterministically instead of asking a model to judge whether the doc shipped." + } +] diff --git a/plugins/os-doc-hygiene/examples/golden/classifier/1-orphaned/expected.json b/plugins/os-doc-hygiene/examples/golden/classifier/1-orphaned/expected.json index fc554d1..0073c9a 100644 --- a/plugins/os-doc-hygiene/examples/golden/classifier/1-orphaned/expected.json +++ b/plugins/os-doc-hygiene/examples/golden/classifier/1-orphaned/expected.json @@ -54,5 +54,6 @@ "weighted_tokens": null } } - ] + ], + "promotion_candidates": [] } diff --git a/plugins/os-doc-hygiene/examples/golden/classifier/2-superseded/expected.json b/plugins/os-doc-hygiene/examples/golden/classifier/2-superseded/expected.json index 71a5d96..19996e5 100644 --- a/plugins/os-doc-hygiene/examples/golden/classifier/2-superseded/expected.json +++ b/plugins/os-doc-hygiene/examples/golden/classifier/2-superseded/expected.json @@ -55,5 +55,6 @@ "weighted_tokens": null } } - ] + ], + "promotion_candidates": [] } diff --git a/plugins/os-doc-hygiene/examples/golden/classifier/3-completed-in-place/expected.json b/plugins/os-doc-hygiene/examples/golden/classifier/3-completed-in-place/expected.json index 7bba950..419dba6 100644 --- a/plugins/os-doc-hygiene/examples/golden/classifier/3-completed-in-place/expected.json +++ b/plugins/os-doc-hygiene/examples/golden/classifier/3-completed-in-place/expected.json @@ -50,5 +50,6 @@ "weighted_tokens": null } } - ] + ], + "promotion_candidates": [] } diff --git a/plugins/os-doc-hygiene/examples/golden/classifier/4-duplicated/expected.json b/plugins/os-doc-hygiene/examples/golden/classifier/4-duplicated/expected.json index 0fdd6be..413c8dc 100644 --- a/plugins/os-doc-hygiene/examples/golden/classifier/4-duplicated/expected.json +++ b/plugins/os-doc-hygiene/examples/golden/classifier/4-duplicated/expected.json @@ -56,5 +56,6 @@ "weighted_tokens": null } } - ] + ], + "promotion_candidates": [] } diff --git a/plugins/os-doc-hygiene/examples/golden/classifier/5-distill/expected.json b/plugins/os-doc-hygiene/examples/golden/classifier/5-distill/expected.json index 764b95b..6452f09 100644 --- a/plugins/os-doc-hygiene/examples/golden/classifier/5-distill/expected.json +++ b/plugins/os-doc-hygiene/examples/golden/classifier/5-distill/expected.json @@ -45,5 +45,6 @@ "weighted_tokens": null } } - ] + ], + "promotion_candidates": [] } diff --git a/plugins/os-doc-hygiene/examples/golden/invalid_report.json b/plugins/os-doc-hygiene/examples/golden/invalid_report.json index 5844eeb..9b3e97a 100644 --- a/plugins/os-doc-hygiene/examples/golden/invalid_report.json +++ b/plugins/os-doc-hygiene/examples/golden/invalid_report.json @@ -34,5 +34,6 @@ "weighted_tokens": null } } - ] + ], + "promotion_candidates": [] } diff --git a/plugins/os-doc-hygiene/examples/golden/valid_report.json b/plugins/os-doc-hygiene/examples/golden/valid_report.json index 1827f23..3a23a4b 100644 --- a/plugins/os-doc-hygiene/examples/golden/valid_report.json +++ b/plugins/os-doc-hygiene/examples/golden/valid_report.json @@ -99,5 +99,6 @@ "weighted_tokens": 1200 } } - ] + ], + "promotion_candidates": [] } diff --git a/plugins/os-doc-hygiene/invariants.md b/plugins/os-doc-hygiene/invariants.md index 02c378d..b416b35 100644 --- a/plugins/os-doc-hygiene/invariants.md +++ b/plugins/os-doc-hygiene/invariants.md @@ -65,17 +65,20 @@ it. - **Violation looks like:** the banner appears on every resume/compact; the snooze keys off something other than `last_reminded`; a same-day re-fire. -## 3. State lives in-project under gitignored `.dochygiene/` +## 3. State lives in-project under gitignored `.cc-os/dochygiene/` -- **Invariant:** All state and reports live under a gitignored `.dochygiene/` - directory at the resolved project root (git root, fallback cwd). There is no +- **Invariant:** All state and reports live under the gitignored + `.cc-os/dochygiene/` directory at the resolved project root (git root, + fallback cwd), per ADR-0027; the legacy `.dochygiene/` path is read as a + backward-compat fallback only (auto-migrated on first write). There is no global, cross-project index, and the tool never silently edits the user's `.gitignore`. - **Why:** A global index would race, corrupt, and itself go stale across projects. Silently editing `.gitignore` is an outward mutation that violates the non-intrusive premise; the dir being tracked would pollute the repo. - **Enforced by:** state store unit test (root resolution + writes confined to - `.dochygiene/`); scanner self-exclusion test (`.dochygiene/` never scanned); + the state dir); scanner self-exclusion test (`.cc-os/` and legacy + `.dochygiene/` never scanned); GAP: needs test that no global path outside the project root is written and that `.gitignore` is only modified on explicit confirmation. - **Violation looks like:** a `~/.dochygiene` or other global index appears; @@ -156,8 +159,9 @@ it. ## 9. Frozen / ignored files are never flagged - **Invariant:** Files marked `hygiene: frozen` in frontmatter, files matched by - `.dochygiene-ignore`, and detected append-only logs are never surfaced as - candidates by the scanner. + `.dochygiene-ignore`, detected append-only logs, and paths matched by a + rulebook IGNORE-sentinel rule (a rule with no `lifetime` — zero-emission + prune, ADR-0040) are never surfaced as candidates by the scanner. - **Why:** Re-flagging deliberately-frozen records and append-only logs every week destroys the user's trust in the tool. This is a correctness requirement, not a nicety. diff --git a/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/.openspec.yaml b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/.openspec.yaml new file mode 100644 index 0000000..64105fc --- /dev/null +++ b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-14 diff --git a/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/calibration-pass-1-protected-set.md b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/calibration-pass-1-protected-set.md new file mode 100644 index 0000000..e23753a --- /dev/null +++ b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/calibration-pass-1-protected-set.md @@ -0,0 +1,27 @@ +# Calibration pass #1 (cc-os) — protected set + +_Fixed 2026-07-14, before the pass; per lifecycle-spec §9 this list is +human-edited and never revised after the pass starts._ + +Hard gate: the pass FAILS if any rule persisted to a rulebook has a glob +matching any path below — regardless of behavior tier. Exploration-time +consult verdicts on these paths are free. + +Protected globs (repo-root-relative, cc-os): + +- `plugins/*/eval*/scenarios/**` +- `plugins/*/eval*/scenarios-reserve/**` +- `plugins/*/eval*/fixture/**` +- `plugins/*/eval*/judge-rubric.md` +- `plugins/os-context/eval/**/scenarios*/**` (defensive; covered by the above where layout matches) +- `openspec/specs/**` +- `docs/adr/**` +- `.claude/skills/**` +- `.codex/skills/**` +- `.pi/skills/**` +- `plugins/*/.claude/skills/**` +- `CLAUDE.md` (root and any directory-level `CLAUDE.md`, i.e. `**/CLAUDE.md`) +- `plugins/*/skills/**`, `plugins/*/scripts/**`, `plugins/*/hooks/**`, + `plugins/*/lib/**`, `plugins/*/.claude-plugin/**` (plugin source) +- `plugins/*/rulebook.json`, `plugins/*/conventions.json`, `plugins/*/invariants.md`, + `plugins/*/examples/golden/**` (plugin source/reversion protection) diff --git a/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/calibration-pass-1-results.md b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/calibration-pass-1-results.md new file mode 100644 index 0000000..9d9ae6b --- /dev/null +++ b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/calibration-pass-1-results.md @@ -0,0 +1,67 @@ +# Calibration pass #1 (cc-os) — results + +_Run 2026-07-14/15. Protocol: skills/calibrate/SKILL.md; seed hold-out per +lifecycle-spec §9 (one-off). Scratch artifacts: session scratchpad +`calib-r1/` (scan.json, clusters.json, nominations.json, verdicts.json, +rule_report.json)._ + +## Run shape + +- Unmatched pool (round 1): **479 paths**, 435 shape-clusters, batched into 4 + haiku nominator families (autoresearch, docs, plugins, + openspec+assistant-mirrors+root). +- Nominations: 19 (15 with globs, 4 explicit keep-declines). +- Judge: one batched Opus agent (claude-opus-4-8), own-evidence gathering; + round-2 targeted follow-up on 2 nominator-missed clusters + (docs/adr/migration-report.md, plugins/*/PRD.md) via the same agent. +- Verdict tally (17 judged): 4 amend→persistable, 4 consult, 9 reject. +- Retest stop condition: round 2 yielded 0 new persistable rules (<2) → stop. + +## Grading vs spec §9 + +- **Protected-set hard gate: PASS** — deterministic glob check of all 4 + persistable rules against the fixed protected set + (calibration-pass-1-protected-set.md): zero overlaps. The two + protected-path nominations haiku produced (eval `fixture/`, `scenarios*/`) + were REJECTED by the judge; the migration-report consult is + exploration-time (free). +- **Recall floor: PASS (9 surfaced ≥ 8; all 4 mandatory covered)** + - `autoresearch//` → persistable rule `autoresearch/*/` (mandatory ✓) + - `HANDOFF-*.md` → persistable rule `plugins/*/HANDOFF-*.md` (mandatory ✓) + - `docs/adr/migration-report.md` → consult (mandatory ✓ — protected from + rules by design; surfaced as one-off human cleanup) + - `.dochygiene/report.{json,md}` → governed by the shipped global IGNORE + seed `.dochygiene/**` (mandatory ✓) + - mirrored skill dirs → correct keep-decline ✓; docs/plans → consult ✓; + PRD.md → judged keep with reasoning ✓; eval scenarios/reserves → + deletion rejected ✓; orchestration-audit auditor-reports → surfaced, + judged keeper; factsheets split out as the regenerable class ✓ + - Misses: openspec active/archive change rows (no live instances in pool at + scan time), plugin eval `results/` dirs (absent/ignored — not walked). + - `graphify-out/` — void, not a miss (IGNORE surface). +- **Novel matches spot-check:** `docs/orchestration-audit/factsheets/*.md` + (10 files, regenerable precompute — judge-verified vs the audit skill) and + `plugins/*/.pytest_cache/` (regenerable cache; judge notes the human may + prefer a .gitignore entry instead). Both plausible; human review below. +- Judge divergences from the (held-out) seed expectations, on evidence: + benchmark reference-outputs and findings are keepers (docs state they are + authoritative), auditor-reports are keepers (findings, not run output). + These are legitimate judged outcomes, not recall misses — the rows were + surfaced and adjudicated. + +## Persistable rules (pending human approval — NOT yet persisted) + +See rule report in the session transcript / `calib-r1/verdicts.json`: + +1. `autoresearch/*/` — temporary, retain_recent 3, max_age_days 30 +2. `docs/orchestration-audit/factsheets/*.md` — temporary, retain 10, 90d +3. `plugins/*/.pytest_cache/` — temporary, retain 0, 7d +4. `plugins/*/HANDOFF-*.md` — delete-once-served, classifier-judged + served_when (forces confirm; the one instance is ACTIVE and must not be + auto-deleted) + +Consults for one-off human decisions (no rules): docs/plans workstream/block +docs (ws*/b*/c*), dated plan/audit overview docs, docs/adr/migration-report.md. + +**Verdict: pass CRITERIA MET. Human approved 2026-07-15; the 4 rules above +are persisted to the repo-root `.dochygiene-rules.json` (ADR-0038).** diff --git a/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/design.md b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/design.md new file mode 100644 index 0000000..779b38a --- /dev/null +++ b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/design.md @@ -0,0 +1,391 @@ +# Design: lifecycle-aware-doc-hygiene + +## Context + +os-doc-hygiene today only monitors stale/bloated docs (`doc-check`, +`doc-clean`, `report-schema` specs, already shipped). The locked lifecycle +design (`plugins/os-doc-hygiene/lifecycle-spec.md`, wayfinder map #31, ADRs +0038–0041) extends it to *lifecycle management*: every managed file gets a +lifetime (`keep` / `temporary` / `delete-once-served`, plus an `extract` +modifier), rules live in a rulebook, and a new `:calibrate` skill learns rules +per project. This design translates the locked spec into the concrete seams +of the existing pipeline (`scanner.py` → subagent classification → +`report_builder.py` → `validate_report.py` → `patch_applier.py`) without +reopening any decision already recorded in the spec or its ADRs. + +## Goals / Non-Goals + +**Goals** + +- Add a stdlib-only `rulebook.py` loader (global + per-project override, + add-only merge, `glob.translate` dialect) consumed by the scanner. +- Give the scanner a lifecycle signal class where a directory-rule match + prunes the walk — this is simultaneously the IGNORE-surface implementation + (no separate ignore mechanism). +- Extend `report_builder.py` / `validate_report.py` / `patch_applier.py` to + carry `delete` and `extract-then-delete` op types end-to-end under the + ADR-0039 autonomy tier matrix, with all tracked/dirty state verified at + application time via git, never trusted from the rule or the report cache. +- Implement temporary-tier age computation (git commit time, mtime fallback + for untracked files, directory-inode mtime for untracked directory entries) + and retain-recent-N semantics. +- Add `conventions.json` and wire promotion-candidate nudging into `:check`. +- Add `:calibrate` as a skill orchestrating haiku nomination + strong-model + judge subagents, per the 6-step protocol in spec §8. +- Resolve the state-dir wrinkle (below) so the shipped IGNORE surface + actually excludes the real state directory. + +**Non-Goals** + +- No ignore-surface propagation into `.graphifyignore`, `.dochygiene-ignore`, + or any other tool's config (ADR-0040 — closed, not revisited here). +- No bulk-clean of any project other than cc-os (calibration pass #1 target + per spec §9); no other project's rulebook is touched by this change. +- No recurring cross-project categorize-and-learn skill — that is charted as + fog on map #31 and explicitly out of scope for this design (spec, "Out of + scope" section). +- No `propagate_ignore` field, in any form (removed entirely per ADR-0040, + not left as a documented-but-unused slot). +- No new deletion destinations beyond the existing knowledge-routing targets + (ADR/CLAUDE.md/docs for repo-durable extraction, SecondBrain vault via + `/os-vault:write` for cross-repo extraction). + +## Decisions + +### 1. `rulebook.py` as a new stdlib-only module + +A new `scripts/rulebook.py`, following the existing scripts' conventions +(small single-responsibility classes, structured JSON-serializable output, no +model, injectable for testing). Responsibilities: + +- Load and parse the global `plugins/os-doc-hygiene/rulebook.json` and, if + present, the project's committed `.dochygiene-rules.json`, both under the + envelope `{"schema_version": 1, "rules": [...]}`. +- Compile every rule's `glob` once at load time via stdlib + `glob.translate(pattern, recursive=True, include_hidden=True)` (Python ≥ + 3.13, per spec §2) into a matchable regex; this is a hard runtime + requirement on the Python version the plugin scripts run under — no + fallback dialect is provided (matches the spec's explicit dialect pin). +- Merge add-only: project rules are appended to global rules, never replacing + or deleting a global entry. A project "neutralizes" a global rule only by + adding a shadowing rule with `lifetime: "keep"` at equal-or-higher + precedence (per the two-axis precedence below) — there is no + rule-removal mechanism, by design (ADR-0038). +- Resolve precedence per path: project file-rule > project directory-rule > + global file-rule > global directory-rule; ties within the same tier broken + by longest `glob` string, then by last-defined (definition order within the + rules array, project-before-global order does not matter once the four + buckets above are checked first). +- Validate: skip-and-warn per invalid or unconfirmed rule (a rule missing + `confirmed_by` is loaded but flagged `inactive`, never contributes a + signal, and never blocks the rest of the rulebook from loading). Hard-fail + (raise / non-zero exit) only on unparseable JSON or an unrecognized + `schema_version`. +- Expose a single query surface consumed by the scanner: given a + project-root-relative path, return either `None` (unmatched — flows + through existing signals unchanged) or the winning rule (with its + precedence-resolved fields) plus whether the match was file-level or + directory-level. + +`rulebook.py` has no knowledge of git, classification, or the report schema — +it only resolves "which rule, if any, governs this path," keeping it testable +in complete isolation with fixture rulebook JSON and injected path lists, in +line with every other script in `scripts/`. + +### 2. Where the lifecycle signal plugs into `scanner.py` + +A new signal class, `LifecycleSignal` (or equivalent), is attached during the +existing scanner walk, alongside the current objective signals (broken refs, +version skew, etc.). Concretely: + +- Before recursing into a directory, the scanner asks the loaded rulebook + whether a directory-rule matches that directory's path. If so, the walk is + **pruned** — the scanner never opens any file beneath it — and the scanner + emits **one aggregate shortlist/signal entry** for the directory path + itself (not one entry per file inside it), carrying the lifecycle signal + (rule ref, lifetime, `served_when`/`served_when_path` if present). +- This directory-rule-prune mechanism **is** the IGNORE surface described in + spec §1 and §2 — there is no separate ignore-list data structure in the + scanner. The IGNORE surface's seed members (`graphify-out/**`, + `.dochygiene/**`) are shipped as ordinary directory rules with + `lifetime: "keep"`... except `keep` implies "scanned and reported," which + contradicts "never walked." Resolution: IGNORE-surface entries are a + rulebook rule with no `lifetime` field at all (or a reserved sentinel the + scanner recognizes before even considering lifetime) whose sole effect is + walk-pruning with **no** shortlist/signal emission at all — distinct from + `keep`, which is walked and reported. See "Ambiguities resolved" below; + this is the one place the spec's wording ("directory-rule match prunes the + walk and emits one aggregate entry — this IS the ignore surface") needed + disambiguating against the separate statement that IGNORE-surface members + are "never walked" with no entry at all. This design follows the latter + (spec §1, §3 table: "IGNORE surface … never walked", no aggregate-entry + language attached to it) and reserves the "prunes the walk and emits one + aggregate entry" behavior for ordinary `temporary`/`delete-once-served` + directory rules (e.g., `autoresearch//`), which need a signal to + drive deletion decisions, whereas pure IGNORE members need none. +- For files matched by a file-rule (not caught by a directory prune), the + scanner attaches the lifecycle signal to that file's existing entry in the + shortlist, alongside any pre-existing objective signals — a file can be + both lifecycle-tagged and, say, stale-by-broken-ref. +- The lifecycle signal is consumed downstream by the classification subagent + as a new signal class (per doc-check's existing "signals are passed + through verbatim" contract) and ultimately drives `op`/`op_type` selection + (`delete` / `extract-then-delete`) in place of, or alongside, the existing + stale/bloat vocabulary. + +### 3. Delete / extract-then-delete op flow + +`report_builder.py` → `validate_report.py` → `patch_applier.py`, following +the existing division of labor (model proposes judgment fields only; +deterministic finalize pass authors the guardrail fields; validator enforces +the schema; applier mutates at clean time with runtime re-verification). + +- **`report_builder.py`** gains two new `exact_edit.kind` values in its + `KIND_TABLE`: `delete` and `extract-then-delete`. Both carry an `anchor` + covering the full file (or, for a directory-rule aggregate entry, the + directory path with no anchor) so the biconditional with `op_type` holds. + `extract-then-delete` additionally carries the extraction destination + classification (`repo-durable` vs `cross-repo`) as a proposal field the + model supplies (it is a judgment about content, not a derived guardrail + field) plus, for `repo-durable`, a target doc reference; `cross-repo` + routes through `/os-vault:write` at clean time and carries no fixed + destination path (spec §1: "no new destinations"). +- **`validate_report.py`**'s `derive_safety_tier` remains the single source + of tier derivation (invariant #10) and is extended, not replaced, with the + ADR-0039 matrix as additional input dimensions beyond `(op_type, + is_destructive, is_reversible)`: + - a `lifecycle` object on the entry (`rule_ref`, `lifetime`, + `served_when_path` XOR `served_when`, `git_state` placeholder recomputed + at runtime — see below) + - `derive_safety_tier` gains a lifecycle-aware branch: for `op_type` in + (`delete`, `extract-then-delete`), tier is `auto` **only** when the + entry's lifecycle evidence is `scanner-proven` (a `served_when_path` hit, + or a temporary-tier age/retain-recent computation) **and** the file's + git state is tracked+clean; every other combination (dirty, untracked, + or `served_when` classifier-judged) forces `confirm`. This is additive: + the existing non-lifecycle branches (stale/bloat ops) are unchanged, and + `derive_safety_tier` is still the one function both `report_builder.py` + and `validate_report.py` call — no second tier-deriving code path is + introduced. + - Because tracked/dirty state can only be known against the live worktree, + `report_builder.py` calls a runtime `git ls-files` + dirty check + **at report-build time** to populate the `git_state` input for tier + derivation — but per ADR-0039 this is explicitly re-verified again at + apply time by `patch_applier.py`, because time passes between check and + clean. The report's tier is advisory-fresh, not authoritative-forever. +- **`patch_applier.py`** gains handling for `delete` and + `extract-then-delete`: + - Before applying either, it re-runs `git ls-files ` and a dirty + check against that specific path (never trusting the report's cached + tier or git_state) — if the path is no longer tracked+clean when the + rule required that for `auto`, the applier downgrades that entry to + skip-and-report (`git-state-changed-since-check`), the same family of + guard as the existing content-hash guard, not a silent auto-apply. + - `delete` performs a true `git rm ` (or `git rm -r` for a + directory-rule aggregate entry) staged into the same single hygiene + commit the rest of the run produces — no archive directory, no move. + This is staged precisely like every other op (no `git add -A`). + - `extract-then-delete` first invokes the generative extraction (repo- + durable → written via the same live-read Sonnet-subagent path + `doc-clean` already uses for generative ops, target = ADR/CLAUDE.md/docs + per the model's proposal; cross-repo → `/os-vault:write` invoked by the + clean skill, not the applier itself, since vault writes are not a + filesystem op the applier owns) and only after that step succeeds does + it `git rm` the source path, both landing in the one hygiene commit. + If extraction fails, the delete does not proceed for that entry (fails + closed, consistent with the existing "hard failure → rollback, + partial-guard-skip → no rollback" split: an extraction failure is a + per-entry skip, not a run-level hard failure, unless it is the kind of + error `doc-clean`'s existing rollback rules already treat as hard). + +### 4. Temporary-tier age computation + +- **Age source:** git commit time of the path's most recent commit touching + it, obtained via `git log -1 --format=%cI -- `; falls back to + filesystem `mtime` only when the path is untracked (no commit history to + read). No per-rule `age_source` override field exists (ADR-0039/#48). +- **Untracked directory entries** (a directory-rule match on an untracked + directory) use **one `stat()` on the directory inode itself**, not a + recursive max-mtime walk over its contents — cheap and self-healing per + spec §4/§48 (a spuriously bumped directory mtime only delays deletion by + one round, it never causes incorrect *early* deletion). +- **Retention unit** is the rule's own match granularity: a file-rule's + matches are individual files; a directory-rule's matches are whole + directories (e.g. `autoresearch//` — "3 most recent runs," not "3 + most recent files inside the newest run"). `rulebook.py` and the scanner + must agree on this: the directory-rule aggregate shortlist entry (§2 + above) is the unit `retain_recent`/`max_age_days` operate over, computed + by grouping sibling directory-rule matches under their common parent glob + and ranking by age (newest first), keeping the top `retain_recent` + regardless of age and deleting the rest once they exceed `max_age_days`. +- `retain_recent` defaults to 3, `max_age_days` defaults to 3, both per-rule + overridable in the rulebook JSON. + +### 5. `conventions.json` and promotion candidates + +- `plugins/os-doc-hygiene/conventions.json` is a global-only, plugin-shipped, + machine-readable file (envelope TBD-but-simple: an array of convention + objects — name, "what it proves," `served_when_path`/frontmatter template + a rule graduates to, one-line human pitch). v1 ships exactly the two + entries named in the spec (`archive-bucket`, `status-frontmatter`); no + per-project override file exists for it (ADR-0041: "the catalog only + recommends; adoption lands in the project's own rulebook"). + `report_builder.py` (deterministic, no model) reads this file directly — + no subagent involvement — and, for every entry whose lifecycle signal is + classifier-judged (`served_when` present, no `served_when_path`), checks + whether any catalog convention's `served_when_path` pattern is *not yet* + satisfied by that project's current structure, and if so emits a + `promotion_candidates` entry naming the convention and the one-line pitch. + This is a deterministic string/structure check, not a judgment call, so it + belongs in `report_builder.py` alongside the other guardrail fields the + model must not author. +- The `:check` report gains a `promotion_candidates` array section at the + top level (sibling to `entries`), populated on every run where at least + one classifier-judged lifecycle signal exists and a matching catalog + convention applies. `:calibrate` (out of this change's core report path, + but sharing the same catalog) may go further and draft the adoption + (graduated rule + the file moves that convention implies) for human + approval, never applying unasked. + +### 6. `:calibrate` as an orchestrating skill + +New `skills/calibrate/SKILL.md` (the only new skill this change adds), +following the existing skill pattern (`check`/`clean`/`sweep` +SKILL.md + a `workflows/*.md` LOOP-GUARD subagent pointer where a subagent +must not recurse into the top-level skill file). It orchestrates, per spec +§8: + +1. A deterministic clustering pass (script, no model) over the current + scanner's unmatched shortlist (unmatched = unmanaged = candidate pool), + grouping by path-shape similarity and sampling representatives per + cluster — this is a new small deterministic helper, not a new pipeline + stage in `check`/`clean`, since calibration is a separate on-demand skill. +2. A **haiku** subagent per cluster, constrained by prompt contract to + nominate a bare glob + candidate lifetime, never an exact-instance path + (the rule-quality "class, never path" test is enforced by the strong- + model judge in the next step, not trusted from haiku). +3. One batched **strong-model** (Opus/Fable) judge subagent that gathers its + own evidence (re-reads matched paths, checks near-miss boundaries) and + returns verdicts (`confirm` / `reject` / `amend` / `consult`), with + `consult` mandatory whenever an artifact's purpose is unclear. +4. A deterministic report-assembly step (script) that renders the required + 5-element rule report (glob verbatim, matches with capped sample + total + count, near-miss boundary, tier, plain-language why) to the human **before + any persistence call is made** — persistence is a separate, explicit step + gated on human review of this report, not something the judge subagent + triggers directly. +5. Persistence: project-rule writes to `.dochygiene-rules.json` happen on + judge confirmation (post human rule-report review); global-rulebook + writes are additionally human-gated (a distinct confirmation, since it is + a cross-repo write into cc-os); rule removals are HITL-only in all cases. +6. A retest loop (re-run clustering against the shrunk unmatched pool) that + stops when a round nominates fewer than 2 new rules or shrinks the + unmatched pool by less than 10%, hard-capped at 3 rounds regardless. + +### 7. The state-dir wrinkle + +`lifecycle-spec.md`'s IGNORE seed lists `.dochygiene/**`. Since ADR-0027 +(pre-dating this change), the plugin's actual state directory is +`.cc-os/dochygiene/`, with `.dochygiene/` retained only as a legacy read- +fallback that `state_store.py` migrates away from on first write; `scanner.py` +already self-excludes both `.cc-os/` and `.dochygiene/` by directory name at +any depth (per `scripts/CONTEXT.md`'s documented default excludes). This +change's rulebook-driven IGNORE surface must therefore ship **both** seed +entries — `.dochygiene/**` (matching the spec text verbatim, covering the +legacy dir on projects that haven't migrated) and an equivalent rule (or +reliance on the scanner's pre-existing hardcoded self-exclusion, which +already covers `.cc-os/**`) — so that on a project already migrated to +`.cc-os/dochygiene/`, the IGNORE surface still holds. This design does not +change the scanner's existing hardcoded self-exclusion; the rulebook's +IGNORE rule list is additive to it, not a replacement, so this is satisfied +without a code change beyond confirming the shipped `rulebook.json`'s IGNORE +entries include `.dochygiene/**` and documenting that `.cc-os/**` is already +covered by the pre-existing self-exclusion. + +## Risks / Trade-offs + +- **Two overlapping walk-pruning mechanisms** (the scanner's pre-existing + hardcoded `excluded_dirs` and the new rulebook-driven directory-rule + prune) risk confusing future maintainers about which one is authoritative + for a given path. Mitigated by treating the hardcoded excludes as a + distinct, lower-level safety net (self-exclusion of the tool's own state + and pre-existing fixture excludes) and the rulebook as the + project-configurable lifecycle layer; `scripts/CONTEXT.md` should document + both once implemented. +- **`git ls-files` / dirty check runs twice** (once at report-build time, + once at apply time) for lifecycle deletes — a deliberate, spec-mandated + redundancy (ADR-0039: "never trusting the rule … at runtime") rather than + an oversight; the cost is one extra git subprocess call per lifecycle + entry per run, acceptable given the destructive nature of the operation. +- **`extract-then-delete` spans two subsystems** (a generative rewrite via + the existing Sonnet-distillation path, then a `git rm`) inside a single + op type, and for cross-repo extraction additionally calls into + `/os-vault:write`, a different plugin's skill. This is more moving parts + in one applied entry than any existing op; a partial failure (extraction + succeeds, delete fails, or vice versa) needs care to keep the "one hygiene + commit" invariant intact — this design treats extraction-then-delete as + atomic per entry (both steps land in the same commit, or neither does), + which may need revisiting once implemented against real vault-write + latency/failure modes. +- **Directory-rule aggregate entries change the meaning of "one entry per + file"** that today's `doc-check`/`report-schema` specs assume throughout + (e.g. "every `entries[].path` SHALL be a member of `shortlist`" — a + directory aggregate entry's path is a directory, not a file, which is a + new shape for `path` that existing consumers may not expect). The delta + specs below extend rather than replace this requirement, but any code + outside this change's touched files that assumes `path` is always a + regular file should be audited during implementation. +- **`rulebook.py`'s hard requirement on Python ≥3.13** (`glob.translate`) is + new relative to the rest of the scripts directory (whose stdlib-only + policy has not previously pinned a Python floor this high); if the + environment running `os-doc-hygiene` scripts is older, the whole rulebook + layer hard-fails to import. This is accepted as a locked design decision + (spec §2), not reopened here, but is called out as an operational risk + worth a version check with a clear error message at load time. + +## Ambiguities resolved while writing this design + +(Reported per instructions — none silently decided against the spec; these +are places the locked documents left an implementation-level gap.) + +1. **IGNORE-surface entries vs. ordinary directory-rule "prune + aggregate + entry" behavior.** Spec §1/§2 phrasing could be read as saying the + IGNORE surface *itself* produces an aggregate shortlist entry ("a + directory-rule match prunes the walk and emits one aggregate entry — + this IS the ignore surface"), but spec §1 and the §3 tier table also say + IGNORE-surface members are simply "never walked," with no entry + language. Resolved by treating "prune + aggregate entry" as the general + mechanism for lifecycle directory rules with a real lifetime (temporary / + delete-once-served), and reserving true zero-signal, zero-entry pruning + for the specific IGNORE seed list (`graphify-out/**`, `.dochygiene/**`). + Flagging this because it affects whether IGNORE members ever appear + anywhere in a report — this design says they never do. +2. **`conventions.json` file shape.** The spec names the file and its v1 + contents (archive-bucket, status-frontmatter) and per-entry conceptual + fields (name, what it proves, template, pitch) but does not give a JSON + schema. This design treats it as a flat array of objects with those four + conceptual fields, no envelope/schema_version wrapper (unlike + `rulebook.json`, which the spec explicitly gives an envelope). Flagging + because a schema_version envelope could be added instead if implementation + prefers consistency with rulebook.json; nothing in the spec forecloses + either choice. +3. **Where the `git ls-files`/dirty runtime check physically lives** (spec + says "verified at runtime via git ls-files + dirty check, never trusting + the rule" but doesn't say whether that's a report-build-time check, an + apply-time check, or both). This design does both — advisory at + report-build time (so the report's tier reflects current reality when + shown to the human), authoritative at apply time (so a stale report + never causes an unsafe auto-delete) — matching the existing + content-hash-guard pattern (`expected_sha256` is likewise computed at + build time and re-verified at apply time). Flagging because the spec's + single sentence doesn't explicitly mandate the double-check; it was + inferred from ADR-0039's "never trusting the rule" plus the existing + applier's re-verification precedent (`patch_applier.py`'s content-hash + guard). +4. **Extraction-failure handling within `extract-then-delete`** (does a + failed extraction hard-fail the whole clean run, or just skip that + entry?) is not addressed in the spec. This design treats it as a + per-entry skip (consistent with `doc-clean`'s existing partial-success + semantics), not a run-level hard failure, unless the failure mode matches + one of the existing hard-failure triggers (applier exit 2, write error). + Flagging since this determines commit granularity under failure. diff --git a/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/proposal.md b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/proposal.md new file mode 100644 index 0000000..a59d61d --- /dev/null +++ b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/proposal.md @@ -0,0 +1,81 @@ +# Proposal: lifecycle-aware-doc-hygiene + +## Why + +os-doc-hygiene monitors stale/bloated docs but cannot manage document +*lifecycle*: disposable artifacts (autoresearch runs, handoff files, served +plans) accumulate as clutter that distracts AI navigation and search. The +lifecycle-aware design is locked (wayfinder map #31, `lifecycle-spec.md`, +ADRs 0038–0041); this change implements it. + +## What Changes + +- New `scripts/rulebook.py` loader: global `rulebook.json` + committed + repo-root `.dochygiene-rules.json` override, add-only merge with + source-then-specificity precedence, `glob.translate` dialect, + skip-and-warn validation (unconfirmed rules never act), hard-fail on + unparseable JSON/unknown schema_version. +- Scanner gains a **lifecycle signal class**; directory-rule matches prune + the walk (which implements the explicit IGNORE surface: `graphify-out/**`, + `.dochygiene/**` — never inferred from `.gitignore`). +- Report schema + clean applier gain **`delete`** and + **`extract-then-delete`** op types with the ADR-0039 autonomy tier matrix + (tracked+clean = auto; dirty/untracked = confirm; classifier-judged + `served_when` = always confirm), verified at runtime via git, never + trusted from the rule. +- **Temporary tier**: retain-recent-N (default 3) + max_age_days (default 3), + age from git commit time falling back to mtime; retention unit = the rule's + match entry (file or run directory). +- **delete-once-served**: deterministic `served_when_path` (scanner-proven, + may auto-delete) vs free-text `served_when` (classifier-judged, forced + confirm). +- **Determinism promotion** (ADR-0041): global-only `conventions.json` + catalog (v1: archive-bucket, status-frontmatter); `:check` reports gain a + promotion-candidates section. +- New **`:calibrate` skill** (the ONLY new skill): cluster-and-sample + unmatched files → haiku glob nomination → strong-model batched judgment + (confirm/reject/amend/consult) → human rule report (glob verbatim, matches, + near-miss boundary, tier, plain-language why) → persistence (project rules + on judge confirm; global writes human-gated) → retest loop (<2 new rules or + <10% shrink, cap 3 rounds). +- Extraction reuses existing knowledge routing (ADR route: repo-durable → + docs/ADR/CLAUDE.md; cross-repo → vault). No `propagate_ignore` (ADR-0040). +- Calibration pass #1 against cc-os per spec §9 (protected-set hard gate, + 8-of-10 recall floor with 4 mandatory rows, seed hold-out — pass #1 only). + +## Capabilities + +### New Capabilities + +- `lifecycle-rulebook`: rulebook file format, locations, merge/precedence, + glob dialect, validation, and scanner consumption (walk pruning + lifecycle + signals + IGNORE surface). +- `lifecycle-deletion`: lifetime taxonomy semantics — temporary tier + (retain-recent + age), delete-once-served (served_when_path vs served_when), + autonomy tier matrix, true-git-deletion in a dedicated hygiene commit, + extract-then-delete routing. +- `determinism-promotion`: conventions.json catalog and promotion-candidate + nudging split across :check (names) and :calibrate (drafts). +- `calibrate`: the learn-new-rules protocol, rule-quality tests + (class-not-path, prefer-narrower), and calibration-pass validation criteria. + +### Modified Capabilities + +- `doc-check`: scanner walks are pruned by directory rules; lifecycle signals + enter classification; reports include promotion candidates. +- `doc-clean`: applies `delete`/`extract-then-delete` ops under the tier + matrix with runtime git tracked/dirty verification. +- `report-schema`: new op types `delete` and `extract-then-delete`, lifecycle + signal fields, promotion-candidates section. + +## Impact + +- `plugins/os-doc-hygiene/scripts/`: new `rulebook.py`; changes to + `scanner.py`, `report_builder.py`, `validate_report.py`, `patch_applier.py`. +- New data files: `rulebook.json`, `conventions.json` (plugin root). +- New skill dir `skills/calibrate/SKILL.md` (no `name:` frontmatter; naming + per convention doc). Updates to `check`/`clean` SKILL.md. +- Tests: TDD (red-green) on every deterministic piece; existing suite (286) + must stay green. +- Per-project surface: optional committed `.dochygiene-rules.json`. +- After source edits: `bin/refresh-plugins`. diff --git a/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/calibrate/spec.md b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/calibrate/spec.md new file mode 100644 index 0000000..b9d1b5f --- /dev/null +++ b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/calibrate/spec.md @@ -0,0 +1,239 @@ +# Spec: calibrate + +## ADDED Requirements + +### Requirement: Calibrate Clusters and Samples Over Unmatched Files + +The `:calibrate` skill SHALL run over the unmatched-files pool (unmatched = +unmanaged, per `lifecycle-rulebook`) as its candidate pool. It SHALL cluster +unmatched paths by shape before nominating any rule, so that a proposed rule +is authored against a cluster of similar paths rather than a single file. + +#### Scenario: Calibrate operates only on the unmatched pool + +- **WHEN** `:calibrate` runs +- **THEN** its candidate pool is exactly the set of files the rulebook currently leaves unmatched + +#### Scenario: Rules are proposed against clusters, not single files + +- **WHEN** `:calibrate` nominates a candidate rule +- **THEN** the nomination is derived from a cluster of similar unmatched paths, not from one instance in isolation + +### Requirement: Cheap-Model Nomination Produces Patterns, Never Exact-Instance Globs + +For each sampled cluster, `:calibrate` SHALL dispatch a haiku subagent +constrained to nominate a bare glob pattern plus a candidate lifetime. The +haiku nomination SHALL be constrained to produce generalizable patterns; it +SHALL NOT be accepted as final if it hardcodes an identifier unique to a +single instance (a run-id, hash, or bare timestamp). + +#### Scenario: Haiku nominates a glob and lifetime per cluster + +- **WHEN** a cluster of unmatched paths is sampled +- **THEN** the haiku subagent returns a bare glob pattern and a candidate lifetime for that cluster + +#### Scenario: Instance-unique nominations are not accepted as final + +- **WHEN** a haiku nomination's glob hardcodes a run-id, hash, or bare timestamp unique to one file +- **THEN** it is not persisted as-is; it must be caught and generalized before the strong-model judgment step, per the rule-quality tests below + +### Requirement: Strong-Model Batched Judgment with Confirm/Reject/Amend/Consult + +`:calibrate` SHALL dispatch one batched strong-model (Opus or Fable) judge +subagent that gathers its own evidence (re-reading matched paths and +checking near-miss boundaries) and authors final rule entries. The judge +SHALL return one of four verdicts per nominated rule: `confirm`, `reject`, +`amend`, or `consult`. `consult` SHALL be mandatory whenever an artifact's +purpose is unclear (i.e., the judge cannot determine whether the artifact is +regenerable or must be retained). + +#### Scenario: Judge gathers its own evidence rather than trusting the nomination + +- **WHEN** the strong-model judge evaluates a haiku nomination +- **THEN** it independently re-reads matched paths and checks near-miss boundaries rather than accepting the nomination's claims at face value + +#### Scenario: Four verdicts are the only possible outcomes + +- **WHEN** the judge evaluates a nominated rule +- **THEN** its verdict is exactly one of `confirm`, `reject`, `amend`, or `consult` + +#### Scenario: Consult is mandatory when purpose is unclear + +- **WHEN** the judge cannot determine whether a clustered artifact type is regenerable or must be retained +- **THEN** the verdict is `consult`, never `confirm` or `reject` + +### Requirement: Rule Report to the Human Before Persistence + +Before any proposed rule is persisted, `:calibrate` SHALL present a rule +report to the human containing, per proposed rule: the glob verbatim exactly +as it would be persisted; every path it currently matches (or a capped +sample plus a total count); the near-miss boundary — paths that do NOT match +despite looking similar; the lifetime and behavior tier (auto vs confirm); +and a plain-language explanation of what the artifact is and why it is +clutter. No rule SHALL be persisted before this report has been shown. + +#### Scenario: The report shows the exact persisted glob + +- **WHEN** a rule report is generated for a proposed rule +- **THEN** the glob shown is character-for-character identical to what would be written to the rulebook file + +#### Scenario: The report shows current matches with a capped sample + +- **WHEN** a proposed rule matches more paths than the display cap +- **THEN** the report shows a capped sample of matched paths plus the total count of all matches + +#### Scenario: The report shows the near-miss boundary + +- **WHEN** a proposed rule's glob narrowly excludes similar-looking paths +- **THEN** the report explicitly lists those near-miss non-matching paths, so a boundary bug (e.g. a glob silently missing a sibling path) is visible before persistence + +#### Scenario: No rule is persisted before the report is shown + +- **WHEN** `:calibrate` has generated proposed rules +- **THEN** it does not write any rule to a rulebook file until the human has seen the rule report for it + +### Requirement: Persistence Rules by Scope + +Project-rulebook writes SHALL land on judge confirmation once the human has +reviewed the rule report. Global-rulebook writes (writing into +`plugins/os-doc-hygiene/rulebook.json`) SHALL additionally require explicit +human gating, distinct from project-rule confirmation, since it is a +cross-repo write into cc-os. Rule removals SHALL be HITL-only in all cases, +with recorded reasoning, regardless of scope. + +#### Scenario: Project rule persists on judge confirmation plus report review + +- **WHEN** the judge verdict is `confirm` for a project-scoped rule and the human has reviewed its rule report +- **THEN** the rule is written to the project's `.dochygiene-rules.json` + +#### Scenario: Global rulebook writes require an additional explicit gate + +- **WHEN** a proposed rule would be written to the global `rulebook.json` +- **THEN** a distinct human confirmation for the cross-repo write is required, beyond the project-rule confirmation step + +#### Scenario: Rule removal is always HITL-only + +- **WHEN** any rule (project or global) is proposed for removal +- **THEN** the removal happens only via explicit human instruction, with the reasoning recorded, never as an automatic side effect of a calibration pass + +### Requirement: Retest Loop with Stop Conditions and Hard Cap + +`:calibrate` SHALL re-run its clustering pass against the shrunk unmatched +pool after each round of persisted rules. It SHALL stop when a round yields +fewer than 2 new rules OR shrinks the unmatched pool by less than 10%. It +SHALL hard-cap at 3 rounds regardless of shrink rate. + +#### Scenario: Stops on fewer than 2 new rules + +- **WHEN** a retest round yields only 1 new confirmed rule +- **THEN** the retest loop stops after that round + +#### Scenario: Stops on less than 10% shrink + +- **WHEN** a retest round shrinks the unmatched pool by less than 10% +- **THEN** the retest loop stops after that round, even if 2 or more rules were confirmed + +#### Scenario: Hard cap of 3 rounds regardless of shrink + +- **WHEN** three retest rounds have run and each still meets the continuation criteria (≥2 new rules and ≥10% shrink) +- **THEN** the retest loop stops after the third round regardless + +### Requirement: Seed Intake at Judge Intake + +The clutter-inventory seed candidates SHALL enter the calibration protocol at +judge intake. Full seed intake SHALL apply to every calibration run after +calibration pass #1; pass #1 uses the one-off seed hold-out described in the +`Calibration Pass Validation Criteria` requirement below. + +#### Scenario: Seed candidates feed the judge step + +- **WHEN** a calibration run (other than pass #1) begins +- **THEN** the clutter-inventory seed candidates are included as judge-intake evidence + +### Requirement: Rule-Quality Test — Class Never Path + +A proposed rule's glob SHALL name a recurring class of artifact, never an +identifier unique to a single instance. A glob that hardcodes a name +recurring by convention (e.g. `PRD.md`, `HANDOFF-*.md`, +`migration-report.md`) is acceptable. A glob that hardcodes a run-id, hash, +or bare timestamp unique to one instance is not acceptable. A rule that +currently matches only one file is acceptable; a rule that can, by +construction, only ever match one file is a failed generalization and SHALL +be flagged loudly rather than silently persisted. + +#### Scenario: A convention-recurring name is acceptable + +- **WHEN** a proposed rule's glob is `HANDOFF-*.md` +- **THEN** it passes the class-never-path test, since `HANDOFF-*` is a recurring naming convention, not a single instance + +#### Scenario: An instance-unique identifier fails the test + +- **WHEN** a proposed rule's glob hardcodes a specific run-id or hash string that can only ever identify one artifact +- **THEN** the rule fails the class-never-path test and is flagged loudly, not silently persisted + +#### Scenario: One current match is fine; one-EVER match is not + +- **WHEN** a proposed rule currently matches exactly one file +- **THEN** it is acceptable if the glob's structure could match future similarly-named files; it is flagged as a failed generalization if the glob's structure can never match any file but the one it names today + +### Requirement: Rule-Quality Tie-Breaker — Prefer the Narrower Glob + +When choosing between candidate globs of differing breadth for the same +cluster, `:calibrate` SHALL prefer the narrower glob. Too-narrow failure is +self-healing (clutter is merely left for a later round to catch); too-broad +failure is dangerous (a keeper file may be deleted and is not +self-healing). + +#### Scenario: Narrower glob is chosen when both would satisfy the cluster + +- **WHEN** two candidate globs both cover the sampled cluster, one narrower and one broader +- **THEN** the narrower glob is chosen + +#### Scenario: Rationale is evidence quality, not readability alone + +- **WHEN** justifying the narrower-glob preference +- **THEN** the reasoning cited is that too-narrow fails safe (self-healing) while too-broad fails dangerous (not self-healing), not merely stylistic preference + +### Requirement: Calibration Pass Validation Criteria + +A calibration pass SHALL be judged against: a precision hard gate (the pass +FAILS if any persisted rule's glob matches a protected path, regardless of +behavior tier — exploration-time `consult` verdicts on protected paths are +free and do not fail the pass); a recall floor of 8 of the 10 rows of the +project's clutter inventory, with 4 specific rows mandatory (missing any +mandatory row fails the pass); a one-off seed hold-out for calibration pass +#1 only (the sealed answer key is withheld from judge intake for pass #1; +every later run uses full seed intake); treatment of IGNORE-surface paths as +void, not a miss, against the recall floor; and the requirement that a +do-nothing pass cannot pass (the recall floor makes the pass falsifiable in +the finding direction). + +#### Scenario: A rule matching a protected path fails the pass + +- **WHEN** any persisted rule's glob matches a path in the fixed protected set +- **THEN** the calibration pass fails, regardless of whether that rule's tier was auto or confirm + +#### Scenario: Consult verdicts on protected paths do not fail the pass + +- **WHEN** the judge issues a `consult` verdict during exploration for a candidate touching a protected path, and that candidate is not persisted +- **THEN** the pass is not failed by that consult verdict + +#### Scenario: Recall floor requires 8 of 10 with 4 mandatory + +- **WHEN** a calibration pass is graded against the clutter inventory +- **THEN** it must recall at least 8 of the 10 inventory rows, and all 4 mandatory rows must be among them, or the pass fails + +#### Scenario: Pass #1 seed hold-out is a one-off + +- **WHEN** calibration pass #1 runs +- **THEN** the sealed answer key (the project's clutter-inventory rows) is withheld from judge intake; every subsequent calibration run instead uses full seed intake + +#### Scenario: IGNORE-surface rows are void, not a miss + +- **WHEN** grading recall against the clutter inventory and a row corresponds to an IGNORE-surface path +- **THEN** that row is excluded from the recall calculation entirely (void), not counted as a miss + +#### Scenario: A do-nothing pass cannot pass + +- **WHEN** a calibration pass persists zero rules +- **THEN** it fails the recall floor and therefore cannot pass diff --git a/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/determinism-promotion/spec.md b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/determinism-promotion/spec.md new file mode 100644 index 0000000..d672cfa --- /dev/null +++ b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/determinism-promotion/spec.md @@ -0,0 +1,99 @@ +# Spec: determinism-promotion + +## ADDED Requirements + +### Requirement: Conventions Catalog Is Global-Only and Machine-Readable + +The plugin SHALL ship a global-only, machine-readable completion-conventions +catalog at `plugins/os-doc-hygiene/conventions.json`. There SHALL be no +per-project override of this catalog — it only recommends; adoption of a +convention lands in the project's own rulebook, not in a project-specific +copy of the catalog. + +#### Scenario: The catalog is read from a single global location + +- **WHEN** the deterministic pipeline consults the conventions catalog +- **THEN** it reads `plugins/os-doc-hygiene/conventions.json` and no project-level override file for it exists or is consulted + +#### Scenario: Adoption is a rulebook write, not a catalog write + +- **WHEN** a project adopts a convention +- **THEN** the adoption is expressed as a graduated rule in the project's `.dochygiene-rules.json`, and the global catalog itself is unchanged + +### Requirement: Convention Entry Shape + +Each entry in the conventions catalog SHALL record: a name, a description of +what the convention proves (i.e., what filesystem-observable condition +counts as "done"), the `served_when_path` pattern or frontmatter template a +rule graduates to when the convention is adopted, and a one-line human pitch +for why adopting it is worthwhile. + +#### Scenario: An entry carries all required fields + +- **WHEN** a convention entry is read from the catalog +- **THEN** it includes a name, a "what it proves" description, the graduation target (`served_when_path` pattern or frontmatter template), and a one-line pitch + +### Requirement: v1 Catalog Contains Exactly Two Conventions + +The v1 conventions catalog SHALL contain exactly two entries: `archive-bucket` +(done = the file moved into a sibling `archive/` directory, graduating a rule +to `served_when_path: /archive/{name}`) and `status-frontmatter` (done = +a `status: shipped|done` frontmatter key is present, with the file staying in +place and the scanner reading the frontmatter). No other conventions SHALL be +present in v1. + +#### Scenario: Exactly two conventions ship in v1 + +- **WHEN** the v1 conventions catalog is loaded +- **THEN** it contains exactly the `archive-bucket` and `status-frontmatter` entries and no others + +#### Scenario: archive-bucket graduates to a served_when_path + +- **WHEN** a project adopts `archive-bucket` for a rule +- **THEN** the rule graduates from a classifier-judged `served_when` to a scanner-provable `served_when_path` pointing at the sibling `archive/` directory + +#### Scenario: status-frontmatter graduates via a frontmatter key + +- **WHEN** a project adopts `status-frontmatter` for a rule +- **THEN** the rule graduates to a condition the scanner can check deterministically by reading a `status: shipped|done` frontmatter key, with the file remaining at its original path + +### Requirement: Check Names Promotion Candidates Deterministically in Every Report + +The `:check` skill's deterministic finalize pass SHALL, for every entry whose +lifecycle signal is classifier-judged (`served_when` present, no +`served_when_path`), check the conventions catalog for an applicable +not-yet-adopted convention and, when one applies, emit an entry in the +report's `promotion_candidates` section naming the convention and its +one-line pitch. This check SHALL run without any model call. + +#### Scenario: A classifier-judged entry with an applicable convention is named + +- **WHEN** an entry uses `served_when` and the `archive-bucket` convention is not yet adopted for that rule +- **THEN** the report's `promotion_candidates` section names `archive-bucket` for that entry with its one-line pitch + +#### Scenario: Promotion-candidate naming requires no model call + +- **WHEN** `report_builder.py` computes `promotion_candidates` +- **THEN** it does so by reading `conventions.json` and the project's rulebook state directly, with no subagent dispatch + +#### Scenario: An already-adopted convention is not re-named + +- **WHEN** a rule has already graduated to a `served_when_path` matching a catalog convention +- **THEN** that rule does not reappear in `promotion_candidates` + +### Requirement: Calibrate May Draft Adoption but Never Applies Unasked + +The `:calibrate` skill MAY draft the adoption of a catalog convention — the +graduated rule plus the file moves or frontmatter additions the convention +implies — and present it to the human for approval. It SHALL NEVER apply an +adoption without explicit human confirmation. + +#### Scenario: Calibrate drafts a convention adoption for review + +- **WHEN** `:calibrate` identifies an unmatched pattern that would benefit from `archive-bucket` +- **THEN** it drafts the graduated rule and the implied file moves and presents them to the human before any change is made + +#### Scenario: No adoption is applied without confirmation + +- **WHEN** a human has not yet confirmed a drafted adoption +- **THEN** no rulebook write and no file move occurs diff --git a/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/doc-check/spec.md b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/doc-check/spec.md new file mode 100644 index 0000000..e28c008 --- /dev/null +++ b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/doc-check/spec.md @@ -0,0 +1,82 @@ +# Spec: doc-check (delta) + +## MODIFIED Requirements + +### Requirement: Check Skill Orchestrates Scan, Classification, and Report Writing + +The `check` skill SHALL orchestrate the check pipeline: load the lifecycle +rulebook (global plus any project override), run the deterministic scanner +(consuming the rulebook so directory-rule matches prune the walk and +lifecycle signals are attached per the `lifecycle-rulebook` spec), dispatch +a Sonnet subagent for judgment-only classification of the signal-bearing +candidates, run the deterministic finalize pass (which also computes +`promotion_candidates` from `conventions.json`), validate, write the report +pair, and stamp `last_check`. The skill SHALL run all non-judgment steps as +deterministic scripts with no model (invariant #6). Zero-signal shortlisted +files SHALL be treated as presumptively cleared: they SHALL remain in the +shortlist, produce no entries, and SHALL NOT be read by the model. A +`--scope` argument SHALL narrow the scanner; a `--category` argument SHALL +filter which entries are produced after classification; both SHALL be +recorded in the human-report header. + +#### Scenario: Skill runs the full pipeline + +- **WHEN** the `check` skill runs +- **THEN** it loads the rulebook, scans (deterministic, rulebook-aware), classifies signal-bearing candidates (Sonnet), finalizes (deterministic, including promotion candidates), validates (deterministic), writes the report pair (deterministic), and stamps `last_check` (deterministic) + +#### Scenario: Zero-signal files are not read by the model + +- **WHEN** a shortlisted file carries no scanner signals +- **THEN** it remains in the shortlist, produces no entry, and is not read by the classification model + +#### Scenario: Scope and category are recorded and applied + +- **WHEN** the user passes `--scope docs/**/*.md` and `--category bloat` +- **THEN** the scanner is narrowed by the scope, only `bloat` entries are produced after classification, and both the scope and the category are recorded in the human-report header + +#### Scenario: Rulebook load failure is a hard failure, not a silent skip + +- **WHEN** the rulebook loader hard-fails (unparseable JSON or unknown `schema_version` in either rulebook file) +- **THEN** the check skill stops and reports the rulebook error before running the scanner, rather than proceeding with lifecycle signals silently disabled + +## ADDED Requirements + +### Requirement: Scanner Consumes the Rulebook for Pruning and Lifecycle Signals + +The deterministic scanner SHALL consult the loaded rulebook during its walk. +A directory-rule match (including IGNORE-surface entries) SHALL prune the +walk beneath that directory per the `lifecycle-rulebook` spec. A file-rule +match SHALL attach a lifecycle signal to that file's shortlist entry. These +lifecycle signals SHALL flow into the classification subagent as a new +signal class alongside the pre-existing stale/bloat signals, and MAY drive +`op`/`op_type` selection toward `delete` or `extract-then-delete` per the +`lifecycle-deletion` spec. + +#### Scenario: A directory-rule prune is reflected in the scan artifact + +- **WHEN** the scanner encounters a directory matching a directory rule +- **THEN** the scan artifact reflects the prune (no files beneath it are in `files_scanned`), and, for non-IGNORE directory rules, exactly one aggregate shortlist entry appears for that directory + +#### Scenario: A file-rule lifecycle signal reaches the classifier + +- **WHEN** a file matches a file-rule with `lifetime: delete-once-served` +- **THEN** the classification subagent receives the lifecycle signal (rule reference, lifetime, served_when/served_when_path) as part of that file's signals, verbatim, per the existing "signals are passed through verbatim" contract + +### Requirement: Report Gains a Promotion-Candidates Section + +The machine and human reports produced by `:check` SHALL include a +`promotion_candidates` section (top-level, sibling to `entries`), populated +deterministically by the finalize pass from `conventions.json` for every +classifier-judged lifecycle entry with an applicable, not-yet-adopted +convention. This section SHALL be present (possibly empty) on every run, +including runs with no lifecycle entries. + +#### Scenario: A run with an applicable convention names it in both reports + +- **WHEN** a classifier-judged entry has an applicable, unadopted convention +- **THEN** both the machine report's `promotion_candidates` array and the human report show the candidate with its one-line pitch + +#### Scenario: A run with no applicable conventions still has the section, empty + +- **WHEN** no classifier-judged entry has an applicable unadopted convention +- **THEN** `promotion_candidates` is present as an empty array/section rather than omitted diff --git a/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/doc-clean/spec.md b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/doc-clean/spec.md new file mode 100644 index 0000000..f4f021d --- /dev/null +++ b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/doc-clean/spec.md @@ -0,0 +1,146 @@ +# Spec: doc-clean (delta) + +## MODIFIED Requirements + +### Requirement: Safety-Tier Gating Before Any Mutation + +The clean skill SHALL partition report entries by safety tier (read from the +report — never recomputed) into `auto` entries (applied without prompt) and +`confirm` entries (escalated before any mutation). It SHALL present all +`confirm`-tier entries as a single batch-confirm list showing path, category, +op, token count, and rationale with per-entry opt-out, visually +distinguishing irreversible `delete-range`, `delete`, and +`extract-then-delete` entries from reversible entries. The approved set +SHALL be all `auto` entries plus any user-approved `confirm` entries. The +gate SHALL run identically under `sweep` — the `/os-doc-hygiene:sweep` +convenience path SHALL NOT bypass invariant #7. Regardless of the tier +recorded in the report, any `delete` or `extract-then-delete` entry SHALL be +re-verified at apply time per the `lifecycle-deletion` tier matrix (tracked ++ clean required for auto) before being applied without a prompt. + +#### Scenario: auto entries apply without prompt + +- **WHEN** the report contains only auto-tier entries (move-to-archive, insert-frontmatter, replace-text, dedupe) +- **THEN** the clean skill applies them without presenting a confirm prompt + +#### Scenario: confirm entries escalate before any mutation + +- **WHEN** the report contains confirm-tier entries (delete-range, delete, extract-then-delete, or any generative op) +- **THEN** the clean skill presents a batch-confirm list before any file is modified, and applies only user-approved entries + +#### Scenario: per-entry opt-out is respected + +- **WHEN** the user approves some confirm entries and opts out of others +- **THEN** the skill applies the approved entries and skips the opted-out entries + +#### Scenario: sweep does not bypass the gate + +- **WHEN** the user runs /os-doc-hygiene:sweep and the report contains confirm-tier entries +- **THEN** the confirm gate runs identically to a standalone /os-doc-hygiene:clean + +#### Scenario: A report-tier auto delete is downgraded if runtime state has changed + +- **WHEN** a `delete` entry was tiered `auto` in the report but the applier's runtime check finds the file is now dirty or untracked +- **THEN** the applier does not apply it silently; it is skipped and reported for re-analysis, never trusted from the cached tier + +### Requirement: Git-Safe Single Commit + +The clean skill SHALL produce exactly one git commit per run. Before any +mutation it SHALL resolve the project root via `StateStore`, run +`git status --porcelain`, and if the tree is dirty, SHALL automatically +create a WIP checkpoint commit of the user's work before proceeding, so the +cleanup commit remains exactly one. The cleanup commit SHALL be created via +`git-context commit-apply --message-stdin` with a generated message +summarizing auto/confirmed/skipped counts and op breakdown, including +lifecycle delete/extract-then-delete counts when present. Staging SHALL be +precise: for non-move, non-delete ops the skill calls `git add +` from the applier result; for `move-to-archive` the applier +calls `git mv` (staging both sides) and the skill SHALL NOT `git add` the +destination path again; for `delete` and the delete half of +`extract-then-delete` the applier calls `git rm` (staging the removal +itself) and the skill SHALL NOT separately stage the removed path. The skill +SHALL NEVER use `git add -A` or `git add .`. If a hard failure occurs +(applier exit 2, `git mv`/`git rm` fail, or write error), the skill SHALL +roll back via `git restore`/`reset` to the pre-run baseline and abort with a +structured error. Partial success (some file batches guard-skipped) SHALL +NOT trigger rollback — the skill SHALL commit what applied and report +skipped files. Untracked candidate docs SHALL be skipped and reported +(tracked-files-only) except where a lifecycle rule explicitly escalates an +untracked delete to confirm and the user approves it. `last_clean` SHALL be +stamped to the commit instant, not the run-start instant. + +#### Scenario: Clean tree produces exactly one commit + +- **WHEN** the user runs /os-doc-hygiene:clean with a clean git tree +- **THEN** the run produces exactly one git commit containing all applied edits, including any lifecycle deletes + +#### Scenario: Dirty tree gets a WIP checkpoint then one cleanup commit + +- **WHEN** the user runs /os-doc-hygiene:clean with unstaged changes in the working tree +- **THEN** the skill auto-creates a WIP checkpoint commit of the user's work, then produces exactly one cleanup commit — two commits total, cleanup still exactly one + +#### Scenario: All-skipped produces zero commits + +- **WHEN** all entries are guard-skipped (every file changed since check) +- **THEN** the skill produces zero commits and reports all files as skipped with re-analysis recommended + +#### Scenario: Hard failure triggers rollback + +- **WHEN** a write error, applier exit 2, or a `git rm` failure occurs mid-run +- **THEN** the skill rolls back to the pre-run baseline and aborts with a structured error; no partial commit is created + +#### Scenario: Partial success commits what applied + +- **WHEN** some file batches are guard-skipped (applier exit 1) and others succeed +- **THEN** the skill commits the applied edits and reports the skipped files, without rolling back the applied changes + +#### Scenario: move-to-archive is not double-staged + +- **WHEN** the applier stages a move-to-archive via git mv (both source and dest) +- **THEN** the skill does not call git add on the destination path again + +#### Scenario: A lifecycle delete is not double-staged + +- **WHEN** the applier stages a `delete` via `git rm` +- **THEN** the skill does not separately call `git add` or any other staging command on the removed path + +## ADDED Requirements + +### Requirement: Applier Applies Delete and Extract-Then-Delete Under the Tier Matrix + +The patch applier SHALL support two new op kinds, `delete` and +`extract-then-delete`. For both, immediately before applying, it SHALL +re-run `git ls-files ` and a dirty check against that specific path — +never trusting the cached report tier or rule claim — and SHALL apply the +tier matrix from the `lifecycle-deletion` spec to decide whether the entry +may proceed as `auto` or must be treated as `confirm` (already gated +upstream by the clean skill). `delete` SHALL perform a `git rm` (recursive +for a directory-rule aggregate entry) staged into the run's single hygiene +commit. `extract-then-delete` SHALL first complete its generative extraction +step (repo-durable via the existing live-read Sonnet distillation path +writing into an ADR/CLAUDE.md/docs target, or cross-repo via +`/os-vault:write`) and SHALL only perform the `git rm` once extraction has +succeeded; both steps SHALL land in the same single hygiene commit, or, on +extraction failure, neither SHALL be applied for that entry (skip, not a +run-level hard failure, unless the failure matches an existing hard-failure +trigger). + +#### Scenario: delete performs a true git rm at apply time + +- **WHEN** the applier applies a `delete` entry +- **THEN** it re-verifies tracked/clean status via `git ls-files` and a dirty check, then performs a `git rm` staged into the single hygiene commit + +#### Scenario: extract-then-delete only deletes after extraction succeeds + +- **WHEN** the applier applies an `extract-then-delete` entry +- **THEN** it completes the extraction write (ADR/CLAUDE.md/docs or vault) first, and performs the `git rm` only after that write succeeds + +#### Scenario: A failed extraction skips the delete for that entry + +- **WHEN** the extraction step of an `extract-then-delete` entry fails +- **THEN** the delete is not applied for that entry, the entry is reported as skipped, and the run is not treated as a hard failure unless the failure matches an existing hard-failure trigger (applier exit 2, write error) + +#### Scenario: Directory-rule aggregate delete removes the whole directory + +- **WHEN** the applier applies a `delete` entry whose path is a directory-rule aggregate entry +- **THEN** it performs a recursive `git rm` removing the entire matched directory in one operation staged into the single hygiene commit diff --git a/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/lifecycle-deletion/spec.md b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/lifecycle-deletion/spec.md new file mode 100644 index 0000000..f9969bb --- /dev/null +++ b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/lifecycle-deletion/spec.md @@ -0,0 +1,173 @@ +# Spec: lifecycle-deletion + +## ADDED Requirements + +### Requirement: Delete Is True Git Deletion in a Dedicated Hygiene Commit + +A lifecycle `delete` op SHALL perform a true `git rm` (file or, for a +directory-rule aggregate entry, recursive directory removal) staged into the +same single hygiene commit the run produces. There SHALL be no archive +directory, graveyard branch, or other relocation of the deleted content — +git history SHALL be the sole archive. + +#### Scenario: A deleted file is git-rm'd, not moved + +- **WHEN** a `delete` op is applied to a tracked file +- **THEN** the file is removed via `git rm` and staged into the run's single hygiene commit, with no copy relocated anywhere in the working tree + +#### Scenario: A deleted directory-rule entry is removed recursively + +- **WHEN** a `delete` op targets a directory-rule aggregate entry +- **THEN** the entire directory is removed via a recursive `git rm` staged into the same commit + +### Requirement: Deletion Autonomy Tier Matrix + +Deletion autonomy SHALL be determined by evidence quality and recoverability, +not file type, per the following matrix: + +| Case | Behavior | +|---|---| +| IGNORE surface | never walked, never a delete candidate | +| lifetime `keep` | scanned + reported, never deleted | +| tracked + delete rule + clean worktree | auto | +| tracked + delete rule + dirty worktree | confirm | +| untracked + delete rule | confirm | +| no rule match | unmanaged, never deleted | + +Tracked/clean status SHALL be verified at runtime via `git ls-files` plus a +dirty check, and SHALL NEVER be trusted from the rule's own claim or from a +cached report field. + +#### Scenario: Tracked and clean deletes automatically + +- **WHEN** a file matches a delete rule, is tracked, and the worktree for that path is clean +- **THEN** the deletion proceeds without a confirmation prompt + +#### Scenario: Tracked but dirty requires confirmation + +- **WHEN** a file matches a delete rule, is tracked, but has uncommitted changes +- **THEN** the deletion is escalated to confirm — an uncommitted diff would otherwise be lost with the file + +#### Scenario: Untracked requires confirmation + +- **WHEN** a file matches a delete rule but is untracked +- **THEN** the deletion is escalated to confirm — there is no git history to recover it from + +#### Scenario: Runtime verification never trusts the rule + +- **WHEN** a delete op is about to be applied +- **THEN** the applier re-checks tracked/clean status via `git ls-files` and a dirty check at that moment, regardless of what the report or rule previously claimed + +#### Scenario: No rule match is never deleted + +- **WHEN** a file has no lifecycle rule match +- **THEN** it is never a candidate for deletion on lifecycle grounds + +### Requirement: Temporary Tier Retention Semantics + +The `temporary` lifetime SHALL use retain-recent-N plus age, not age alone. +`retain_recent` (default 3) SHALL always keep the N most recent matching +entries for a rule regardless of age. An entry ranked `retain_recent + 1` or +older SHALL become eligible for deletion once it exceeds `max_age_days` +(default 3). The retention unit SHALL be the rule's own match granularity: a +file-rule's unit is the individual file; a directory-rule's unit is the +matched directory as a whole (e.g., a run directory), never files nested +inside one such matched directory. + +#### Scenario: The 3 newest entries are always kept regardless of age + +- **WHEN** a temporary rule has `retain_recent: 3` and five matching entries, the three newest exceeding `max_age_days` +- **THEN** the three newest are kept and only the two oldest (ranked 4th and 5th) are eligible for deletion + +#### Scenario: An entry younger than max_age_days is kept even if not in the top N + +- **WHEN** a temporary rule's 4th-ranked entry is younger than `max_age_days` +- **THEN** it is not deleted this run + +#### Scenario: Directory-rule retention unit is the whole directory + +- **WHEN** a directory rule matches `autoresearch//` entries +- **THEN** retain-recent-N and age are computed per matched run directory as a whole, not per file within the newest run + +### Requirement: Temporary Tier Age Source + +Age for the temporary tier SHALL be computed from the git commit time of the +path's most recent commit, falling back to filesystem mtime only when the +path is untracked. There SHALL be no per-rule `age_source` override field. + +#### Scenario: Tracked file age comes from git commit time + +- **WHEN** age is computed for a tracked file matching a temporary rule +- **THEN** the age is derived from that file's most recent commit time, not its filesystem mtime + +#### Scenario: Untracked file age falls back to mtime + +- **WHEN** age is computed for an untracked file matching a temporary rule +- **THEN** the age is derived from the file's filesystem mtime + +#### Scenario: No per-rule age_source field exists + +- **WHEN** a rule in the rulebook is inspected +- **THEN** it has no `age_source` field — the git-commit-time-with-mtime-fallback behavior is fixed, not configurable per rule + +### Requirement: Untracked Directory Entry Age Uses Directory Inode mtime + +For an untracked directory matched by a directory rule, age SHALL be +computed from a single `stat()` of the directory inode itself, not a +recursive walk computing the maximum mtime of its contents. + +#### Scenario: Directory age is one stat call, not a recursive scan + +- **WHEN** age is computed for an untracked directory matching a directory rule +- **THEN** the computation reads only the directory inode's own mtime and does not recurse into or stat any file inside it + +### Requirement: delete-once-served Split by Evidence Quality + +The `delete-once-served` lifetime SHALL support two mutually exclusive served +signals per rule: `served_when_path`, a deterministic path pattern the +scanner itself can prove satisfied (e.g. a sibling archive directory +existing), and `served_when`, free text describing a condition the +classifier must judge. A rule with `served_when_path` satisfied by the +scanner MAY be deleted under the autonomy tier matrix (i.e., auto when +tracked+clean). A rule relying on `served_when` SHALL ALWAYS be forced to +confirm, regardless of tracked/clean status, because it depends on a model +judgment rather than a provable filesystem fact. + +#### Scenario: Scanner-proven served_when_path may auto-delete + +- **WHEN** a rule's `served_when_path` condition is satisfied by the filesystem and the matched path is tracked and clean +- **THEN** the deletion may proceed automatically under the tier matrix + +#### Scenario: Classifier-judged served_when always forces confirm + +- **WHEN** a rule uses `served_when` (free text) and the classifier judges the condition met +- **THEN** the deletion is always escalated to confirm, even if the matched path is tracked and clean + +#### Scenario: The LLM may propose but never silently destroy on served_when + +- **WHEN** the classifier judges a `served_when` condition satisfied +- **THEN** it produces a proposal for a human to confirm; it never causes an unattended deletion + +### Requirement: Extract Modifier Routes Through Existing Knowledge Destinations Only + +The `extract` modifier on a deletion SHALL distill durable content before +deleting, routing exclusively through the existing knowledge-routing +destinations: repo-durable residue SHALL be written into an ADR, `CLAUDE.md`, +or a `docs/` file; cross-repo lessons SHALL be written to the SecondBrain +vault via `/os-vault:write`. No new destination (e.g., a "retired specs" +directory) SHALL be introduced. + +#### Scenario: Repo-durable extraction targets ADR/CLAUDE.md/docs + +- **WHEN** an `extract-then-delete` op is classified as repo-durable +- **THEN** the extracted content is written into an ADR, `CLAUDE.md`, or a `docs/` file, never a new bespoke location + +#### Scenario: Cross-repo extraction routes through /os-vault:write + +- **WHEN** an `extract-then-delete` op is classified as cross-repo +- **THEN** the extracted content is written to the SecondBrain vault via `/os-vault:write`, and no other cross-repo destination is used + +#### Scenario: No new destination is introduced + +- **WHEN** extraction routing is implemented +- **THEN** it reuses only the destinations named above; it does not create a new "retired" or "archive" content store diff --git a/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/lifecycle-rulebook/spec.md b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/lifecycle-rulebook/spec.md new file mode 100644 index 0000000..e3b39cd --- /dev/null +++ b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/lifecycle-rulebook/spec.md @@ -0,0 +1,215 @@ +# Spec: lifecycle-rulebook + +## ADDED Requirements + +### Requirement: Rulebook Locations and Envelope + +The plugin SHALL ship a global rulebook at +`plugins/os-doc-hygiene/rulebook.json`, resolved relative to plugin scripts, +present in every project. A project MAY additionally provide a committed +repo-root `.dochygiene-rules.json` override. Both files SHALL use the +envelope `{"schema_version": 1, "rules": [...]}`. The per-project override +SHALL NOT live under gitignored `.cc-os/` — it SHALL be a committed, +reviewable dotfile. + +#### Scenario: Global rulebook is always present + +- **WHEN** the rulebook loader runs in any project +- **THEN** it loads `plugins/os-doc-hygiene/rulebook.json` resolved relative to the plugin scripts directory + +#### Scenario: Per-project override is optional and committed + +- **WHEN** a project has no `.dochygiene-rules.json` at its repo root +- **THEN** the loader proceeds using only the global rulebook, and when the file is present it is read as a committed, reviewable file, never from `.cc-os/` + +#### Scenario: Both files share one envelope shape + +- **WHEN** either the global rulebook or a project override is loaded +- **THEN** it is validated against the envelope `{"schema_version": 1, "rules": [...]}` + +### Requirement: Glob Dialect Is glob.translate + +Rule `glob` patterns SHALL be compiled using stdlib +`glob.translate(pattern, recursive=True, include_hidden=True)` (Python ≥ +3.13). Patterns SHALL be interpreted as repo-root-relative, and `**` SHALL +match recursively. Each rule's glob SHALL be compiled once at rulebook load +time, not per matched path. + +#### Scenario: Recursive double-star matches subtrees + +- **WHEN** a rule's glob is `autoresearch/*/**` +- **THEN** it matches any file at any depth under any immediate subdirectory of `autoresearch/` + +#### Scenario: Hidden files are matched when the pattern implies them + +- **WHEN** a rule's glob targets a dotfile path +- **THEN** `include_hidden=True` semantics apply and the dotfile is matchable + +#### Scenario: Compilation happens once per load + +- **WHEN** the rulebook loader parses the rules array +- **THEN** each rule's glob is compiled to a matcher exactly once, and that compiled matcher is reused for every path checked during the run + +### Requirement: Two-Axis Precedence with Add-Only Merge + +The rulebook loader SHALL merge the project override over the global +rulebook using add-only semantics: project rules are appended to, and never +replace or delete, global rules. Precedence for a given path SHALL resolve +in this order: project file-rule > project directory-rule > global +file-rule > global directory-rule. Ties within the same precedence tier +SHALL be broken first by longest `glob` pattern, then by last-defined order. +A project SHALL neutralize a global rule only by adding a shadowing rule +with `lifetime: "keep"` at equal-or-higher precedence; there SHALL be no +rule-removal mechanism. + +#### Scenario: Project file-rule outranks every other tier + +- **WHEN** a path matches both a project file-rule and a global directory-rule +- **THEN** the project file-rule's fields govern + +#### Scenario: Ties broken by longest pattern then last-defined + +- **WHEN** two rules in the same precedence tier match the same path with different-length globs +- **THEN** the rule with the longer glob pattern governs; if the glob lengths are equal, the rule defined later in its rules array governs + +#### Scenario: Neutralizing a global rule via keep-shadowing + +- **WHEN** a project wants to exempt a path from a global delete rule +- **THEN** it adds a project rule matching that path with `lifetime: "keep"`, and no mechanism exists to remove or edit the global rule itself + +#### Scenario: Merge never deletes a global rule + +- **WHEN** the project override is loaded alongside the global rulebook +- **THEN** every global rule remains present and evaluable; the merge only adds project rules on top + +### Requirement: Per-Rule Fields + +A rule SHALL support the fields `glob`, `lifetime` (one of `keep`, +`temporary`, `delete-once-served`), `extract` (boolean modifier), `served_when` +(free text, classifier hint), `served_when_path` (deterministic sibling of +`served_when`), `retain_recent` (default `3`), `max_age_days` (default `3`), +`confirm` (boolean, human-settable-only escape hatch), `confirmed_by` +(`human` or a strong-model identifier), `confirmed_on`, `source`, and `note`. +A rule SHALL NOT support a `propagate_ignore` field in any form. A rule that +matches no path yet is undefined behavior only in the sense that unmatched +files receive no lifetime at all and flow through existing signals unchanged +— unmatched SHALL always mean unmanaged, never an implicit lifetime. + +#### Scenario: Defaults apply when retain_recent/max_age_days are omitted + +- **WHEN** a `temporary`-lifetime rule omits `retain_recent` and `max_age_days` +- **THEN** the loader applies `retain_recent = 3` and `max_age_days = 3` + +#### Scenario: confirm:true may only be set by a human + +- **WHEN** a rule is validated +- **THEN** a rule with `confirm: true` is accepted only if it is not proposed by a model in the same validation pass as an unconfirmed state — a model-authored rule proposal SHALL NOT itself set `confirm: true`; it may only recommend that a human set it + +#### Scenario: propagate_ignore is rejected as an unknown field + +- **WHEN** a rule in either rulebook file contains a `propagate_ignore` field +- **THEN** the loader treats it as an unrecognized field under the rule's validation (skip-and-warn, per the Validation requirement), since no such field is part of the schema + +#### Scenario: Unmatched files receive no lifetime + +- **WHEN** a file matches no rule in either rulebook +- **THEN** the rulebook query returns no match for that path, and the file flows through the existing (non-lifecycle) scanner signals unchanged, becoming a `:calibrate` candidate + +### Requirement: Skip-and-Warn Validation, Hard-Fail Only on Structural Errors + +The rulebook loader SHALL skip and warn on a per-rule basis for any rule that +is invalid or lacks `confirmed_by` — such a rule SHALL be loaded but marked +inactive and SHALL never contribute a lifecycle signal, while the rest of +the rulebook continues to load and function. The loader SHALL hard-fail +(non-zero exit / raised error) only for unparseable JSON or an unrecognized +`schema_version`. + +#### Scenario: A rule missing confirmed_by is skipped, not fatal + +- **WHEN** the rulebook contains one rule without `confirmed_by` and nine valid rules +- **THEN** the loader loads all ten rules, marks the one missing `confirmed_by` inactive (it never emits a signal), and the other nine function normally + +#### Scenario: Unparseable JSON hard-fails + +- **WHEN** either rulebook file is not valid JSON +- **THEN** the loader raises a hard failure and does not proceed with a partial rulebook + +#### Scenario: Unknown schema_version hard-fails + +- **WHEN** a rulebook file declares a `schema_version` the loader does not recognize +- **THEN** the loader raises a hard failure + +### Requirement: Unmatched Means Unmanaged + +Files that match no rule in either rulebook SHALL receive no lifecycle +signal and SHALL NOT be deleted, extracted, or otherwise treated as +lifecycle-managed by any component of this change. They remain visible only +through the existing stale/bloat signal pipeline and are the candidate pool +for `:calibrate`. + +#### Scenario: No rule match means no lifecycle behavior + +- **WHEN** a file matches no rulebook rule +- **THEN** no delete or extract-then-delete op is ever proposed for it on lifecycle grounds alone + +### Requirement: IGNORE Surface Is an Explicit Seed List, Never Inferred from .gitignore + +The rulebook SHALL define an explicit IGNORE surface as directory rules with +no lifetime (paths never walked at all, distinct from `keep`, which is +walked and reported but never deleted). The seed IGNORE members SHALL +include `graphify-out/**` and `.dochygiene/**`, plus any entries needed to +cover the plugin's actual current state directory (`.cc-os/**`, already +covered by the scanner's pre-existing hardcoded self-exclusion — see the +`doc-check` delta spec). The IGNORE surface SHALL NEVER be inferred from +`.gitignore` — a gitignored path is neither automatically deletable nor +automatically keepable. + +#### Scenario: graphify-out is never walked + +- **WHEN** the scanner walks a project containing `graphify-out/` +- **THEN** no file beneath `graphify-out/` is opened, and no shortlist or signal entry is produced for it or its contents + +#### Scenario: .dochygiene legacy state dir is never walked + +- **WHEN** the scanner encounters `.dochygiene/` in a project that has not migrated to `.cc-os/dochygiene/` +- **THEN** the directory is treated as IGNORE surface and never walked + +#### Scenario: gitignored is not treated as IGNORE surface + +- **WHEN** a path is listed in `.gitignore` but is not one of the explicit IGNORE seed members +- **THEN** the scanner walks it normally per its other rules — being gitignored alone neither excludes it from the walk nor exempts it from deletion + +### Requirement: Directory-Rule Walk Pruning + +When a directory-rule (a lifecycle rule whose glob covers a subtree) matches +a directory during the scanner walk, the scanner SHALL prune the walk at +that directory: no file beneath it SHALL be opened or read. For directory +rules carrying a real lifetime (`temporary` or `delete-once-served`), the +scanner SHALL emit exactly one aggregate shortlist/signal entry for the +directory path itself, carrying the lifecycle signal (rule reference, +lifetime, and `served_when`/`served_when_path`). For IGNORE-surface +directory rules (no lifetime), the scanner SHALL emit no entry at all. + +#### Scenario: A temporary directory rule prunes and emits one aggregate entry + +- **WHEN** `autoresearch/run-2026-07-01/` matches a directory rule with `lifetime: temporary` +- **THEN** the scanner does not open any file inside that directory and emits exactly one shortlist entry for the directory path carrying the lifecycle signal + +#### Scenario: An IGNORE-surface directory rule prunes with no entry + +- **WHEN** `graphify-out/` matches the IGNORE-surface rule +- **THEN** the scanner does not open any file inside it and produces no shortlist or signal entry for it + +### Requirement: Lifecycle Signal Attachment on File-Rule Matches + +When a file-rule matches a path not caught by a directory-rule prune, the +scanner SHALL attach a lifecycle signal (rule reference, lifetime, +`served_when`/`served_when_path`) to that file's shortlist entry, alongside +any pre-existing objective signals for the same file. The lifecycle signal +SHALL be a new signal class consumed by the classification subagent like any +other signal. + +#### Scenario: A file-rule match adds a lifecycle signal without displacing existing signals + +- **WHEN** `HANDOFF-2026-07-01.md` matches a file-rule with `lifetime: delete-once-served` and also has an existing broken-reference signal +- **THEN** its shortlist entry carries both the lifecycle signal and the pre-existing broken-reference signal diff --git a/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/report-schema/spec.md b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/report-schema/spec.md new file mode 100644 index 0000000..c07992d --- /dev/null +++ b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/specs/report-schema/spec.md @@ -0,0 +1,203 @@ +# Spec: report-schema (delta) + +## MODIFIED Requirements + +### Requirement: Top-Level Report Envelope + +The machine report SHALL be a single JSON object containing `schema_version`, +`tool_version`, `generated_at` (ISO-8601 UTC), a `scan` metadata object, a +`shortlist` array, an `entries` array, and a `promotion_candidates` array. +The `scan` object SHALL include `project_root`, `scope_globs`, +`excluded_dirs`, and `files_scanned`. The `generated_at` timestamp SHALL be +the check time that the clean step uses for its mtime guard. The +`promotion_candidates` array SHALL be present (possibly empty) on every +report and SHALL list, per candidate, the classifier-judged entry it applies +to, the recommended `conventions.json` entry name, and its one-line pitch. + +#### Scenario: Check writes a well-formed report +- **WHEN** the `check` skill completes a scan and classification pass +- **THEN** it writes one JSON object with `schema_version`, `tool_version`, `generated_at`, `scan`, `shortlist`, `entries`, and `promotion_candidates` + +#### Scenario: Clean reads the check timestamp +- **WHEN** the `clean` skill loads a report +- **THEN** it reads `generated_at` and uses it as the reference time for the per-op mtime guard + +#### Scenario: promotion_candidates is always present, possibly empty +- **WHEN** a check run has no classifier-judged entry with an applicable unadopted convention +- **THEN** `promotion_candidates` is written as an empty array, not omitted + +### Requirement: Exact-Edit Kind Is a Closed Enum + +Every `exact_edit` SHALL carry a `kind` drawn from the closed set +`delete-range`, `move-to-archive`, `insert-frontmatter`, `replace-text`, +`dedupe`, `delete`, and `extract-then-delete` — one per deterministic op +family the PRD and the lifecycle-aware design name. No other `kind` is +permitted. Each `kind` SHALL carry its required sub-fields and SHALL have a +fixed inherent `(is_destructive, is_reversible)` characterization that feeds +the `safety_tier` derivation, except where noted below that lifecycle kinds' +tier also depends on runtime git state and evidence quality (see the +Safety-Tier Is Derived Deterministically requirement): + +- `delete-range` SHALL carry `anchor` with `start_line` and `end_line`; it is + destructive and irreversible (derives to `confirm`). +- `move-to-archive` SHALL carry `anchor` (`start_line`, `end_line`) and + `dest_path`; it is non-destructive and reversible (derives to `auto`). +- `insert-frontmatter` SHALL carry the frontmatter `key` and `value` to inject + (for example `hygiene: frozen`); it is non-destructive and reversible (derives + to `auto`). +- `replace-text` SHALL carry `anchor` (`start_line`, `end_line`), a `match` + string, and a `replacement` string; it is non-destructive and reversible + (derives to `auto`). +- `dedupe` SHALL carry `anchor` (`start_line`, `end_line`) of the removed + duplicate span and a `canonical_ref` to the kept location; because the removed + span is an exact duplicate preserved verbatim at `canonical_ref`, no information + is lost, so it is non-destructive and reversible and derives to `auto`. (Contrast + `delete-range`, which removes content kept nowhere else, is destructive, and + derives to `confirm`.) +- `delete` SHALL carry a full-file (or, for a directory-rule aggregate entry, + whole-directory) `anchor` and a `lifecycle` object (`rule_ref`, `lifetime`, + and exactly one of `served_when_path` or `served_when`); it is destructive + (git history is the only recovery path) and its `is_reversible` + characterization is inherently git-history-dependent rather than fixed — + see the Safety-Tier requirement for how its tier is actually derived. +- `extract-then-delete` SHALL carry the same `lifecycle` object as `delete` + plus an `extraction_target` classification (`repo-durable` or `cross-repo`) + and, for `repo-durable`, a target document reference; it has the same + destructive/tier characterization as `delete`, gated additionally on the + extraction step succeeding before the delete half applies. + +A validator SHALL reject an `exact_edit` whose `kind` is outside the closed set +or that omits a required sub-field for its `kind`. + +#### Scenario: Each kind carries its required fields +- **WHEN** an entry has `op_type` = `deterministic` with an `exact_edit` of `kind` = `move-to-archive` +- **THEN** the `exact_edit` includes `anchor` (`start_line`, `end_line`) and `dest_path`, and the entry is valid + +#### Scenario: Unknown kind is rejected +- **WHEN** an `exact_edit` has a `kind` outside the closed set `delete-range`, `move-to-archive`, `insert-frontmatter`, `replace-text`, `dedupe`, `delete`, `extract-then-delete` +- **THEN** the report is invalid + +#### Scenario: Missing required sub-field is rejected +- **WHEN** an `exact_edit` of `kind` = `replace-text` omits its `match` or `replacement` field +- **THEN** the report is invalid + +#### Scenario: Kind characterization feeds the safety-tier derivation +- **WHEN** an `exact_edit` has `kind` = `delete-range` +- **THEN** the entry's `is_destructive` is true and `is_reversible` is false, so the derived `safety_tier` is `confirm` + +#### Scenario: delete and extract-then-delete require the lifecycle object +- **WHEN** an `exact_edit` has `kind` = `delete` or `kind` = `extract-then-delete` +- **THEN** it includes a `lifecycle` object with `rule_ref`, `lifetime`, and exactly one of `served_when_path` or `served_when`; omitting the `lifecycle` object or supplying both `served_when_path` and `served_when` makes the report invalid + +#### Scenario: extract-then-delete requires an extraction_target + +- **WHEN** an `exact_edit` has `kind` = `extract-then-delete` +- **THEN** it includes `extraction_target` set to `repo-durable` or `cross-repo`, with a target document reference required when `repo-durable` + +### Requirement: Safety-Tier Is Derived Deterministically + +`deterministic` ops SHALL be exact edits the check pre-computes and the +cleaner applies with no model. `generative` ops SHALL be prose +transformations requiring a model at clean time. The `safety_tier` SHALL be +**computed** by a deterministic script function `derive_safety_tier(op_type, +is_destructive, is_reversible, lifecycle=None)` and SHALL NOT be assigned by +the model; the report records the computed value. This function remains the +single source of truth for tier derivation across the whole report schema, +including lifecycle ops — no second tier-deriving function or code path is +introduced. + +For non-lifecycle ops, the function SHALL return `confirm` when `op_type` is +`generative`, OR when `is_destructive` is true, OR when `is_reversible` is +false; and SHALL return `auto` only when the op is `deterministic` AND +non-destructive AND reversible (hence objective). + +For `delete` and `extract-then-delete` ops, the function SHALL additionally +consult the `lifecycle` argument and SHALL return `auto` only when ALL of the +following hold: the lifecycle evidence is scanner-proven (a satisfied +`served_when_path`, or a temporary-tier retain-recent/age computation) AND +the path is tracked AND the worktree at that path is clean at the time of +derivation. Every other combination for a lifecycle op — a classifier-judged +`served_when`, a dirty worktree, or an untracked path — SHALL derive to +`confirm`, regardless of `is_destructive`/`is_reversible` inputs. The +function SHALL NEVER return `auto` for a `generative` op, for any +non-lifecycle destructive or irreversible op, or for any lifecycle op whose +evidence is classifier-judged or whose git state is not tracked+clean, so +the model cannot violate invariant #7. + +`auto`-tier ops SHALL run without a prompt; `confirm`-tier ops SHALL be +escalated for approval; `delete`/`extract-then-delete` `auto` verdicts SHALL +additionally be re-verified against live git state at apply time per the +`lifecycle-deletion` spec before being applied without a prompt. + +#### Scenario: Deterministic reversible op derives to auto +- **WHEN** an entry has `op_type` = `deterministic`, `is_destructive` = false, and `is_reversible` = true +- **THEN** the derived `safety_tier` is `auto` and the cleaner applies the `exact_edit` mechanically without a prompt + +#### Scenario: Destructive op derives to confirm +- **WHEN** an op removes information not preserved elsewhere (`is_destructive` = true, e.g. a `delete-range`) +- **THEN** the derived `safety_tier` is `confirm` regardless of `op_type` + +#### Scenario: Generative op derives to confirm +- **WHEN** an entry has `op_type` = `generative` +- **THEN** the derived `safety_tier` is `confirm` and the op is delegated to a Sonnet subagent at clean time + +#### Scenario: Function can never emit auto for a generative or destructive op +- **WHEN** `derive_safety_tier(op_type, is_destructive, is_reversible, lifecycle)` is evaluated for any input where `op_type` = `generative`, or (for a non-lifecycle op) `is_destructive` = true, or `is_reversible` = false +- **THEN** the result is `confirm`, never `auto` + +#### Scenario: A lifecycle delete with scanner-proven evidence and tracked+clean state derives to auto +- **WHEN** a `delete` entry's `lifecycle` argument shows a satisfied `served_when_path` and the path is tracked and clean +- **THEN** the derived `safety_tier` is `auto` + +#### Scenario: A lifecycle delete with classifier-judged evidence always derives to confirm +- **WHEN** a `delete` or `extract-then-delete` entry's `lifecycle` argument carries `served_when` (classifier-judged) +- **THEN** the derived `safety_tier` is `confirm`, regardless of tracked/clean state + +#### Scenario: A lifecycle delete on a dirty or untracked path derives to confirm +- **WHEN** a `delete` entry's evidence is scanner-proven but the path is dirty or untracked +- **THEN** the derived `safety_tier` is `confirm` + +## ADDED Requirements + +### Requirement: Lifecycle Signal Fields on Shortlist and Entries + +A shortlist entry or report entry carrying a lifecycle signal SHALL include a +`lifecycle` object with `rule_ref` (identifying which rulebook rule matched), +`lifetime` (`keep`, `temporary`, or `delete-once-served`), and exactly one of +`served_when_path` (deterministic) or `served_when` (classifier-judged, free +text), mirroring the rulebook rule's own fields. A directory-rule aggregate +entry SHALL carry the same `lifecycle` object shape as a file entry, with its +`path` set to the matched directory. + +#### Scenario: A file-rule match carries the lifecycle object + +- **WHEN** a file matches a `temporary`-lifetime file rule +- **THEN** its shortlist/report entry includes a `lifecycle` object with `rule_ref`, `lifetime: "temporary"`, and no `served_when`/`served_when_path` (temporary entries are keyed on age, not a served signal) + +#### Scenario: A delete-once-served match carries exactly one served field + +- **WHEN** a file matches a `delete-once-served` rule using `served_when_path` +- **THEN** its `lifecycle` object includes `served_when_path` and omits `served_when`, never both + +#### Scenario: A directory-rule aggregate entry carries the same shape + +- **WHEN** a directory rule produces one aggregate shortlist entry +- **THEN** that entry's `lifecycle` object has the same fields as a file entry's, with `path` set to the directory + +### Requirement: Promotion-Candidates Section Schema + +The top-level `promotion_candidates` array SHALL contain, per candidate, the +`path` of the classifier-judged entry it applies to, the recommended +`conventions.json` entry `name`, and the convention's one-line `pitch`. This +array SHALL be computed only by the deterministic finalize pass (never the +model) and SHALL be empty, not omitted, when no candidate applies. + +#### Scenario: A promotion candidate references its source entry and convention + +- **WHEN** an entry with `served_when` has an applicable unadopted convention +- **THEN** the `promotion_candidates` array includes an object with that entry's `path`, the convention `name`, and its `pitch` + +#### Scenario: promotion_candidates is model-free + +- **WHEN** the finalize pass computes `promotion_candidates` +- **THEN** it does so without any subagent dispatch, using only `conventions.json` and the report's own lifecycle entries diff --git a/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/tasks.md b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/tasks.md new file mode 100644 index 0000000..3042f8b --- /dev/null +++ b/plugins/os-doc-hygiene/openspec/changes/lifecycle-aware-doc-hygiene/tasks.md @@ -0,0 +1,47 @@ +# Tasks: lifecycle-aware-doc-hygiene + +## 1. Rulebook loader (TDD) + +- [x] 1.1 Red: tests for `scripts/rulebook.py` — envelope parsing, hard-fail on unparseable JSON/unknown schema_version, skip-and-warn on invalid/unconfirmed rules (missing `confirmed_by` → inactive), glob compilation via `glob.translate(recursive=True, include_hidden=True)` with a Python ≥3.13 version check + clear error +- [x] 1.2 Red: tests for add-only merge + two-axis precedence (project file > project dir > global file > global dir; longest pattern, then last-defined), keep-shadowing neutralization, unmatched → None, file-vs-directory match distinction, IGNORE-sentinel rules (no lifetime → zero-emission prune) +- [x] 1.3 Green: implement `scripts/rulebook.py` (stdlib-only, single query surface per design §1) +- [x] 1.4 Author shipped `rulebook.json` (global): IGNORE seeds `graphify-out/**`, `.dochygiene/**`; envelope `{"schema_version": 1, "rules": [...]}` + +## 2. Scanner lifecycle signals (TDD) + +- [x] 2.1 Red: scanner tests — directory-rule match prunes the walk and emits one aggregate entry with lifecycle signal; IGNORE-surface rules prune with zero emission; file-rule match attaches lifecycle signal alongside existing signals; unmatched files flow through unchanged +- [x] 2.2 Red: temporary-tier tests — age from `git log -1 --format=%cI`, mtime fallback for untracked, directory-inode mtime for untracked dirs (no recursive walk); retain-recent-N grouping by rule match entry, newest 3 kept regardless of age, 4th+ deleted past max_age_days +- [x] 2.3 Red: delete-once-served tests — `served_when_path` (with `{id}`/`{name}` substitution) proven from filesystem → deterministic served signal; `served_when` free text → classifier-judged marker only +- [x] 2.4 Green: implement lifecycle signal class + rulebook consumption + tier computations in `scanner.py` + +## 3. Report schema + tier derivation (TDD) + +- [x] 3.1 Red: `report_builder.py`/`validate_report.py` tests — new op kinds `delete`/`extract-then-delete` in KIND_TABLE; lifecycle entry fields (rule_ref, lifetime, served evidence, git_state); `derive_safety_tier` lifecycle branch (scanner-proven + tracked+clean = auto; dirty/untracked/classifier-judged = confirm; existing branches unchanged); build-time `git ls-files` + dirty check populates git_state +- [x] 3.2 Red: promotion-candidates tests — deterministic `promotion_candidates` report section from `conventions.json` for classifier-judged rules (no model) +- [x] 3.3 Green: implement report_builder/validate_report extensions +- [x] 3.4 Author `conventions.json` (v1: archive-bucket, status-frontmatter exactly) + +## 4. Clean applier deletion ops (TDD) + +- [x] 4.1 Red: `patch_applier.py` tests — apply-time re-verification (git ls-files + dirty per path; downgrade to skip `git-state-changed-since-check` on mismatch); `delete` = `git rm`/`git rm -r` staged into the single hygiene commit; `extract-then-delete` fails closed per entry when extraction fails; confirm-tier gating for lifecycle ops +- [x] 4.2 Green: implement applier delete/extract-then-delete handling + +## 5. Skills wiring + +- [x] 5.1 Update `skills/check/SKILL.md` — lifecycle signals into classification, promotion-candidates section in reports +- [x] 5.2 Update `skills/clean/SKILL.md` — lifecycle op application, confirm gates, extraction routing (repo-durable → docs/ADR/CLAUDE.md via existing generative path; cross-repo → `/os-vault:write` from the skill, not the applier) +- [x] 5.3 Read `~/Documents/SecondBrain/cc-os-plugin-skill-naming-convention.md`, then create `skills/calibrate/SKILL.md` (NO `name:` frontmatter) implementing the 6-step protocol: deterministic cluster-and-sample helper script, haiku nomination, strong-model batched judge (confirm/reject/amend/consult), 5-element human rule report before persistence, persistence rules (project on confirm, global human-gated, removals HITL-only), retest loop (<2 rules or <10% shrink, cap 3) +- [x] 5.4 Red-green the deterministic calibrate helpers (clustering/sampling, rule-report assembly, class-not-path + prefer-narrower checks) +- [x] 5.5 Full test suite green (existing 286 + new); run `bin/refresh-plugins` + +## 6. Calibration pass #1 (cc-os, spec §9) + +- [x] 6.1 Write the protected set fixed before the pass (eval scenarios/reserve/fixture/judge-rubric, openspec/specs/, docs/adr/**, mirrored .claude/.codex/.pi skill dirs, CLAUDE.md, plugin source) +- [x] 6.2 Run :calibrate against cc-os with #41 cc-os seed rows HELD OUT of judge intake (pass #1 one-off) +- [x] 6.3 Grade: protected-set hard gate (no persisted rule glob matches a protected path); recall floor 8/10 with 4 mandatory (`autoresearch//`, `HANDOFF-*.md`, `docs/adr/migration-report.md`, `.dochygiene/report.{json,md}`); graphify-out is void-not-miss; spot-check novel matches (wrong novel match → rule adjustment + retest round) +- [x] 6.4 Record pass results in the change; persist confirmed cc-os rules to `.dochygiene-rules.json` (repo root, committed) — human-approved and persisted 2026-07-15 + +## 7. Records + +- [x] 7.1 One-line headline in `docs/implementation-status.md` + detail in `docs/implementation-status/os-doc-hygiene.md`. (No `docs/memory-system/04-build-plan.md` step covers this effort — tracked by wayfinder map #31, not the memory-system build plan; nothing to mark there.) +- [x] 7.2 Updated plugin `CLAUDE.md`/`scripts/CONTEXT.md` for the new lifecycle layer (two prune mechanisms documented). `invariants.md` #3 (`.cc-os/dochygiene/` per ADR-0027, legacy fallback noted) and #9 (rulebook IGNORE-sentinel added to the never-flagged enumeration) corrected with explicit human approval 2026-07-15 per the META-RULE. diff --git a/plugins/os-doc-hygiene/rulebook.json b/plugins/os-doc-hygiene/rulebook.json new file mode 100644 index 0000000..46cd8d6 --- /dev/null +++ b/plugins/os-doc-hygiene/rulebook.json @@ -0,0 +1,19 @@ +{ + "schema_version": 1, + "rules": [ + { + "glob": "graphify-out/**", + "confirmed_by": "human", + "confirmed_on": "2026-07-14", + "source": "lifecycle-spec #43", + "note": "IGNORE surface: disposable/rebuildable Graphify graph output, never walked (distinct from lifetime keep, which is walked and reported)." + }, + { + "glob": ".dochygiene/**", + "confirmed_by": "human", + "confirmed_on": "2026-07-14", + "source": "lifecycle-spec #43", + "note": "IGNORE surface: legacy pre-ADR-0027 state directory, never walked. .cc-os/** (the current state dir) is already covered by scanner.py's pre-existing hardcoded self-exclusion (design.md §7 state-dir wrinkle)." + } + ] +} diff --git a/plugins/os-doc-hygiene/scripts/CONTEXT.md b/plugins/os-doc-hygiene/scripts/CONTEXT.md index 4e23914..be37616 100644 --- a/plugins/os-doc-hygiene/scripts/CONTEXT.md +++ b/plugins/os-doc-hygiene/scripts/CONTEXT.md @@ -9,12 +9,14 @@ for isolated testing with injected clocks/filesystems. | File | Purpose | |------|---------| -| `scanner.py` | Deterministic doc scanner. Walks scoped files under the resolved project root, applies the ordered short-circuiting exclusion pipeline (dir-prune incl. `.cc-os/` and legacy `.dochygiene/` self-exclusion → `.dochygiene-ignore` match → `hygiene: frozen` frontmatter → append-only-log detection; invariant #9), and computes objective per-path signals (broken refs, version skew, edit-recency vs git churn, location, archive ratio, frontmatter markers). Emits the **intermediate artifact** `{project_root, scope_globs, excluded_dirs, files_scanned, shortlist, signals}` — NOT a machine report (no `entries`/`category`/`op`/`token_estimate`). No model. Injected git-log / clock for testability. **Default excludes** (echoed verbatim in `excluded_dirs`): bare names — `build`, `vendor`, `archive`, `graphify-out`, `.cc-os` (shared cc-os state dir, ADR-027), `.dochygiene` (legacy state dir, invariant self-exclusion), `fixtures` — are matched on the directory *name* segment at any depth (any dir named `archive` is pruned, not just `docs/archive`); plus one **path-aware** entry `examples/golden` (any entry containing `/` is a `parent/child` name pair, pruned only where the child's immediate parent matches `parent`, depth-independent). `fixtures` + `examples/golden` prevent false-positive flagging of test-fixture trees and classifier golden inputs (deliberately-stale docs). `golden` is deliberately path-aware — a bare `golden` name-match would wrongly skip legitimate `golden/` dirs in unrelated projects (doc-hygiene installs globally). The golden-test harness is unaffected: it roots the scanner at `…/examples/golden/classifier/N/input/`, so the `examples→golden` pair sits in the ancestry above the walk root and is never re-encountered as a child. | +| `scanner.py` | Deterministic doc scanner. Walks scoped files under the resolved project root, applies the ordered short-circuiting exclusion pipeline (dir-prune incl. `.cc-os/` and legacy `.dochygiene/` self-exclusion → `.dochygiene-ignore` match → `hygiene: frozen` frontmatter → append-only-log detection; invariant #9), and computes objective per-path signals (broken refs, version skew, edit-recency vs git churn, location, archive ratio, frontmatter markers). Emits the **intermediate artifact** `{project_root, scope_globs, excluded_dirs, files_scanned, shortlist, signals}` — NOT a machine report (no `entries`/`category`/`op`/`token_estimate`). No model. Injected git-log / clock for testability. **Default excludes** (echoed verbatim in `excluded_dirs`): bare names — `build`, `vendor`, `archive`, `graphify-out`, `.cc-os` (shared cc-os state dir, ADR-027), `.dochygiene` (legacy state dir, invariant self-exclusion), `fixtures` — are matched on the directory *name* segment at any depth (any dir named `archive` is pruned, not just `docs/archive`); plus one **path-aware** entry `examples/golden` (any entry containing `/` is a `parent/child` name pair, pruned only where the child's immediate parent matches `parent`, depth-independent). `fixtures` + `examples/golden` prevent false-positive flagging of test-fixture trees and classifier golden inputs (deliberately-stale docs). `golden` is deliberately path-aware — a bare `golden` name-match would wrongly skip legitimate `golden/` dirs in unrelated projects (doc-hygiene installs globally). The golden-test harness is unaffected: it roots the scanner at `…/examples/golden/classifier/N/input/`, so the `examples→golden` pair sits in the ancestry above the walk root and is never re-encountered as a child. Additive to this hardcoded-exclude prune, the scanner also consults `rulebook.py` for lifecycle rule matches during the walk (directory-rule matches prune the walk and emit one aggregate entry; IGNORE-surface rules prune with zero emission; file-rule matches attach a lifecycle signal alongside existing signals) — **two independent prune mechanisms**: the hardcoded `DEFAULT_EXCLUDED_DIRS` list is the safety net (fixed, not project-configurable), while the rulebook is the configurable lifecycle layer (global + per-project rules, tiered by lifetime taxonomy per ADR-0039). | | `state_store.py` | State store. `resolve_project_root(start_dir, fs)` (pure: git root, fallback cwd) plus `StateStore` confining all writes to `/.cc-os/dochygiene/` (invariant #3, ADR-027, never edits `.gitignore`). Reads fall back to the legacy `/.dochygiene/` when the canonical dir has no state; the first write migrates legacy files in and removes the legacy dir. Lifecycle timestamps (`last_check`/`last_clean`/`last_reminded`), atomic write-temp→fsync→`os.replace` (D9), and report rollover keeping exactly one `.json`+`.md` pair (invariant #4). Injected clock. | | `reminder.py` | `SessionStart` reminder. Pure `Reminder.decide(last_check, last_reminded, now)` decision (stale-and-not-snoozed ⇒ banner; calendar-day snooze keyed on `last_reminded`, invariants #1/#2). `ReminderRunner` wires it to the `StateStore`, emits a zero-token `systemMessage` JSON banner on stdout, and writes `last_reminded` as the *only* mutation, only on banner. Treats missing/corrupt state as never-checked (stale); always exits 0 — never blocks the session. **No-ops unless cwd is inside a real git project** (`_is_git_project` guard) so the user-scoped hook never creates `.cc-os/dochygiene/` in an arbitrary dir like `~` (invariant #3) — flagged for human review. No scan, no model. `DEFAULT_THRESHOLD_DAYS` is an implementation constant, not a spec'd contract. Invoked by `hooks/hooks.json` via `${CLAUDE_PLUGIN_ROOT}/scripts/reminder.py`. | | `validate_report.py` | Schema-validator for machine-report JSON files. Enforces the frozen report contract: envelope fields, closed category enum, closed `exact_edit.kind` enum with per-kind required sub-fields, `op_type`↔`exact_edit` biconditional (invariant #11), and `safety_tier` derivation match (invariant #10). Collects all violations. Exit 0 = valid, exit 1 = invalid, exit 2 = usage error. | | `report_builder.py` | Deterministic finalize pass (no model). Consumes the scanner artifact + the subagent's slim proposals and assembles a schema-valid machine report plus a human-report skeleton. Fills the four guardrail fields the model must not author: computes each entry's `expected_sha256` over the live file, looks up `is_destructive`/`is_reversible` from the `KIND_TABLE` (keyed on `exact_edit.kind`), derives `safety_tier` via `validate_report`'s `derive_safety_tier` (invariant #10 — single source of truth), and sources `raw_tokens` from `token_estimator` (over the `reducible_range`/anchor text). Rejects malformed proposals with a structured stderr error (`{index, field, message}`, exit 1); `--out-json`/`--out-md` write files and suppress the stdout bundle. An empty proposals array yields a valid empty-entries report. | | `token_estimator.py` | Deterministic local token estimator (no model, no network — invariant #6). Produces the `token_estimate.raw_tokens` count for report entries. Pluggable `TokenEstimator` ABC with two backends: `HeuristicEstimator` (ALWAYS available, zero deps, `ceil(len/4)`) and `TiktokenEstimator` (optional accuracy upgrade, `o200k_base`, `disallowed_special=()`). `default_estimator(cache_dir=…)` selects tiktoken **only** when it is importable AND the vocab is vendored locally (detect-before-fetch: checks the sha1-named cache file exists before loading, so a cold cache never triggers a network download); otherwise returns the heuristic. Never raises; active backend is introspectable via `.name`. `estimate(text)→int`, `estimate_file(path)→int` (utf-8, `errors="replace"`), `estimate_for_report(text)→{raw_tokens, injection_frequency:null, weighted_tokens:null}` (v1 emits only `raw_tokens`; weighting is the v2 bonus). CLI: `python token_estimator.py ` prints JSON `{path, backend, token_estimate}`, exit 1 on missing file. **tiktoken vocab is NOT committed** (~2 MB; gitignored at `scripts/tiktoken_cache/`). Pre-warm once with: `TIKTOKEN_CACHE_DIR=scripts/tiktoken_cache python -c "import tiktoken; tiktoken.get_encoding('o200k_base')"`. **Cache-filename gotcha:** tiktoken names the cache file `sha1(blobpath_url)` = `fb374d419588a4632f3f557e76b4b70aebbca790` (NOT the sha256 content hash); a vendored file under any other name never hits and falls back to network. | +| `rulebook.py` | Deterministic rulebook loader (no model). Parses the global `rulebook.json` plus an optional per-project `.dochygiene-rules.json` envelope (`{"schema_version": 1, "rules": [...]}`), hard-failing on unparseable JSON/unknown `schema_version` and skip-and-warning on invalid/unconfirmed rules (missing `confirmed_by` → inactive). Compiles globs via `glob.translate(recursive=True, include_hidden=True)` (Python ≥3.13 required, clear error otherwise). Merges add-only across a two-axis precedence — project file > project dir > global file > global dir; within a tier, longest pattern then last-defined — with keep-shadowing neutralization; unmatched paths resolve to `None`; file-vs-directory match are distinguished; IGNORE-sentinel rules (no `lifetime`) prune with zero emission. Single query surface consumed by `scanner.py`'s lifecycle signal pass. | +| `calibrate_helpers.py` | Deterministic helpers (no model) backing the `/os-doc-hygiene:calibrate` skill: clustering/sampling of scanner shortlist candidates for haiku nomination, rule-report assembly for the human-approval step, and class-not-path + prefer-narrower checks that keep proposed rules generalized rather than single-path. The skill itself drives the AI nomination/judge steps; this module is the deterministic scaffolding around them. | | `patch_applier.py` | Deterministic patch-applier: consumes the machine report's deterministic entries, applies each `exact_edit.kind` mechanically via a per-file transaction (read-once → verify whole-file sha256 against `expected_sha256` → apply edits descending by line, insert-frontmatter last → write once), skips files on content-guard mismatch or incompatible-op combinations, uses `git mv` for move-to-archive; structured JSON output, no model. CLI: `python patch_applier.py --report --apply-indices ` emits `{applied, skipped, failed, staged_paths}` to stdout, exit 0 (partial ok), exit 1 (some applied + skipped), exit 2 (hard failure — rollback recommended). Invariant #8 (mtime guard). | ## Planned additions (future changes) diff --git a/plugins/os-doc-hygiene/scripts/calibrate_helpers.py b/plugins/os-doc-hygiene/scripts/calibrate_helpers.py new file mode 100644 index 0000000..0e72e21 --- /dev/null +++ b/plugins/os-doc-hygiene/scripts/calibrate_helpers.py @@ -0,0 +1,332 @@ +""" +calibrate_helpers.py — deterministic helpers for the `:calibrate` skill +(lifecycle-aware-doc-hygiene change, task 5.4). + +Stdlib-only, no model. Three responsibilities, each a small +single-responsibility class taking injected inputs and returning +JSON-serializable output: + +1. `ClusterSampler` — groups unmatched paths by path-shape (directory + prefix + a filename "shape class" where digit runs -> `#` and hex-looking + runs -> `~`) and emits capped representative samples per cluster. +2. `RuleReportBuilder` — given proposed rules (glob + lifetime) and the + repo's file list, assembles the 5-element rule-report data: glob + verbatim, matched paths (capped sample + total), near-miss boundary + paths, lifetime/tier, and a `why` placeholder the caller fills in. +3. `RuleQualityChecker` — deterministic lint: `class_not_path` (flags globs + whose filename segment looks instance-unique: long digit runs, hex + hashes, bare timestamps, or a wildcard-free glob matching exactly one + existing path) and `prefer_narrower` (compares two candidate globs by + match-count on the actual tree; the narrower one wins ties). + +None of these classes call an LLM, read history, or touch git. They operate +purely on lists of strings (paths) handed in by the orchestrating skill. +""" + +from __future__ import annotations + +import fnmatch +import re +from dataclasses import dataclass, field +from pathlib import PurePosixPath +from typing import Optional + +_DEFAULT_SAMPLE_CAP = 5 + +# --- shape-class helpers --------------------------------------------------- + +_DIGIT_RUN = re.compile(r"\d+") +# hex run: 6+ consecutive hex chars (avoids false-triggering on short words) +_HEX_RUN = re.compile(r"(? str: + """Collapse a filename into a shape class: digit runs -> '#', hex runs + (6+ hex chars) -> '~'. Order matters: hex collapse first (a hex run may + contain digits that would otherwise be independently collapsed).""" + shaped = _HEX_RUN.sub("~", name) + shaped = _DIGIT_RUN.sub("#", shaped) + return shaped + + +def _dir_prefix(path: str) -> str: + parts = PurePosixPath(path).parts[:-1] + return "/".join(parts) + + +@dataclass +class Cluster: + key: str # "::" + dir_prefix: str + shape: str + paths: list # all paths in this cluster, sorted + sample: list # capped representative sample + + @property + def total(self) -> int: + return len(self.paths) + + def to_dict(self) -> dict: + return { + "key": self.key, + "dir_prefix": self.dir_prefix, + "shape": self.shape, + "total": self.total, + "sample": list(self.sample), + } + + +class ClusterSampler: + """Groups unmatched paths by path-shape (directory prefix + filename + shape class) and emits per-cluster representative samples, capped.""" + + def __init__(self, sample_cap: int = _DEFAULT_SAMPLE_CAP) -> None: + self._cap = sample_cap + + def cluster(self, unmatched_paths: list) -> list: + """Returns a list of Cluster, sorted by cluster key for determinism. + Each cluster groups paths sharing the same directory prefix AND the + same filename shape class.""" + groups: dict = {} + for path in unmatched_paths: + dir_prefix = _dir_prefix(path) + filename = PurePosixPath(path).name + shape = _filename_shape(filename) + key = f"{dir_prefix}::{shape}" + groups.setdefault(key, []).append(path) + + clusters = [] + for key in sorted(groups.keys()): + paths = sorted(groups[key]) + dir_prefix, shape = key.split("::", 1) + clusters.append( + Cluster( + key=key, + dir_prefix=dir_prefix, + shape=shape, + paths=paths, + sample=paths[: self._cap], + ) + ) + return clusters + + def cluster_to_dicts(self, unmatched_paths: list) -> list: + return [c.to_dict() for c in self.cluster(unmatched_paths)] + + +# --- rule report assembly --------------------------------------------------- + + +def _glob_matches(glob_pattern: str, path: str) -> bool: + return fnmatch.fnmatch(path, glob_pattern) + + +def _relax_glob(glob_pattern: str) -> Optional[str]: + """Produce a strictly broader variant of a glob for near-miss detection. + + Widens every path segment that already contains a wildcard character to + a bare '*' (fully generic for that segment), preserving fixed segments + and the final segment's extension when present. This surfaces exactly + the #45 boundary-bug class: a glob like `autoresearch/classic-*/` that + silently misses a sibling `autoresearch/improve-*/` — relaxing the + `classic-*` segment to `*` reveals the sibling as a near-miss. + + Returns None if no segment has a wildcard to relax (fully-fixed glob has + no meaningful near-miss boundary).""" + parts = glob_pattern.split("/") + if not parts: + return None + + relaxed_parts = list(parts) + changed = False + for i, part in enumerate(parts): + if part in ("*", "**"): + continue # already fully generic + if any(ch in part for ch in "*?[") : + if "." in part: + stem, _, ext = part.rpartition(".") + relaxed = "*." + ext + else: + relaxed = "*" + if relaxed != part: + relaxed_parts[i] = relaxed + changed = True + + if not changed: + return None + return "/".join(relaxed_parts) + + +@dataclass +class RuleReportEntry: + glob: str + lifetime: str + tier: str + matched_sample: list + matched_total: int + near_miss: list + why: str = "" + + def to_dict(self) -> dict: + return { + "glob": self.glob, + "lifetime": self.lifetime, + "tier": self.tier, + "matches": { + "sample": list(self.matched_sample), + "total": self.matched_total, + }, + "near_miss": list(self.near_miss), + "why": self.why, + } + + +class RuleReportBuilder: + """Given proposed rules and the repo's file list, produces the 5-element + rule-report data per proposed rule: glob verbatim, matched paths (capped + sample + total), near-miss boundary, lifetime + tier, and a why string + (caller-supplied; this class does not author prose).""" + + def __init__(self, sample_cap: int = _DEFAULT_SAMPLE_CAP) -> None: + self._cap = sample_cap + + def build(self, proposed_rule: dict, all_paths: list) -> RuleReportEntry: + glob_pattern = proposed_rule["glob"] + lifetime = proposed_rule.get("lifetime", "keep") + tier = proposed_rule.get("tier", "confirm") + why = proposed_rule.get("why", "") + + matched = sorted(p for p in all_paths if _glob_matches(glob_pattern, p)) + + near_miss: list = [] + relaxed = _relax_glob(glob_pattern) + if relaxed is not None: + matched_set = set(matched) + near_miss = sorted( + p + for p in all_paths + if p not in matched_set and _glob_matches(relaxed, p) + ) + + return RuleReportEntry( + glob=glob_pattern, + lifetime=lifetime, + tier=tier, + matched_sample=matched[: self._cap], + matched_total=len(matched), + near_miss=near_miss, + why=why, + ) + + def build_all(self, proposed_rules: list, all_paths: list) -> list: + return [self.build(r, all_paths).to_dict() for r in proposed_rules] + + +# --- rule-quality checks ----------------------------------------------------- + + +@dataclass +class QualityFinding: + check: str # "class_not_path" | "prefer_narrower" + glob: str + passed: bool + reason: str + + def to_dict(self) -> dict: + return { + "check": self.check, + "glob": self.glob, + "passed": self.passed, + "reason": self.reason, + } + + +class RuleQualityChecker: + """Deterministic lints applied to a proposed glob before persistence: + + - `class_not_path`: flags a glob whose final path segment looks + instance-unique (a long digit run / bare timestamp, or a hex-looking + hash run), OR a glob with zero wildcard characters that currently + matches exactly one existing path (a rule that can, by construction, + only ever match that one file — a failed generalization). + - `prefer_narrower`: given two candidate globs for the same cluster, + report which one is narrower by match-count against the actual tree + (ties broken by raw pattern length, longer = more specific = narrower). + """ + + def class_not_path(self, glob_pattern: str, all_paths: Optional[list] = None) -> QualityFinding: + last_segment = glob_pattern.split("/")[-1] + + if _LONG_DIGIT_RUN.search(last_segment): + return QualityFinding( + check="class_not_path", + glob=glob_pattern, + passed=False, + reason=( + "glob's final segment contains a long digit run " + "(looks like a run-id or timestamp unique to one instance)" + ), + ) + + if _HEX_RUN.search(last_segment): + return QualityFinding( + check="class_not_path", + glob=glob_pattern, + passed=False, + reason=( + "glob's final segment contains a hex-looking run " + "(looks like a hash unique to one instance)" + ), + ) + + has_wildcard = any(ch in glob_pattern for ch in "*?[") + if not has_wildcard and all_paths is not None: + matches = [p for p in all_paths if _glob_matches(glob_pattern, p)] + if len(matches) <= 1: + return QualityFinding( + check="class_not_path", + glob=glob_pattern, + passed=False, + reason=( + "glob has no wildcard and matches at most one existing " + "path — by construction it can never match a future " + "file (failed generalization), flagged loudly" + ), + ) + + return QualityFinding( + check="class_not_path", + glob=glob_pattern, + passed=True, + reason="glob names a recurring class, not a single instance", + ) + + def prefer_narrower(self, glob_a: str, glob_b: str, all_paths: list) -> dict: + """Returns {"narrower": , "match_counts": {a: n, b: n}}.""" + count_a = sum(1 for p in all_paths if _glob_matches(glob_a, p)) + count_b = sum(1 for p in all_paths if _glob_matches(glob_b, p)) + + if count_a != count_b: + narrower = glob_a if count_a < count_b else glob_b + else: + # tie-break on identical match counts: fewer wildcard characters + # is more specific/narrower; if that also ties, the longer raw + # pattern is treated as more specific. + def _wildcard_count(g: str) -> int: + return sum(g.count(ch) for ch in "*?[") + + wc_a, wc_b = _wildcard_count(glob_a), _wildcard_count(glob_b) + if wc_a != wc_b: + narrower = glob_a if wc_a < wc_b else glob_b + else: + narrower = glob_a if len(glob_a) >= len(glob_b) else glob_b + + return { + "narrower": narrower, + "match_counts": {glob_a: count_a, glob_b: count_b}, + } diff --git a/plugins/os-doc-hygiene/scripts/patch_applier.py b/plugins/os-doc-hygiene/scripts/patch_applier.py index 8786d31..2d59f6a 100644 --- a/plugins/os-doc-hygiene/scripts/patch_applier.py +++ b/plugins/os-doc-hygiene/scripts/patch_applier.py @@ -104,10 +104,11 @@ class RealFileSystem: class _Git(Protocol): """Git surface — injectable for tests.""" def mv(self, src: Path, dest: Path, cwd: Path) -> None: ... + def rm(self, path: Path, cwd: Path, recursive: bool = False) -> None: ... class RealGit: - """Production git — runs `git mv` via subprocess.""" + """Production git — runs `git mv`/`git rm` via subprocess.""" def mv(self, src: Path, dest: Path, cwd: Path) -> None: result = subprocess.run( @@ -121,6 +122,56 @@ class RealGit: f"git mv failed ({result.returncode}): {result.stderr.strip()}" ) + def rm(self, path: Path, cwd: Path, recursive: bool = False) -> None: + cmd = ["git", "rm"] + if recursive: + cmd.append("-r") + cmd.append(str(path)) + result = subprocess.run( + cmd, + cwd=str(cwd), + capture_output=True, + text=True, + ) + if result.returncode != 0: + raise RuntimeError( + f"git rm failed ({result.returncode}): {result.stderr.strip()}" + ) + + +class _GitState(Protocol): + """Runtime git tracked/dirty query surface — injectable for tests. + + Used ONLY to re-verify lifecycle delete/extract-then-delete entries at + apply time (ADR-0039): the applier never trusts a report's cached tier + or git_state field for these kinds — it re-asks git, per path, right + before applying. + """ + def is_tracked(self, path: str, cwd: Path) -> bool: ... + def is_dirty(self, path: str, cwd: Path) -> bool: ... + + +class RealGitState: + """Production git-state checker — `git ls-files` + `git status --porcelain`.""" + + def is_tracked(self, path: str, cwd: Path) -> bool: + result = subprocess.run( + ["git", "ls-files", "--", path], + cwd=str(cwd), + capture_output=True, + text=True, + ) + return bool(result.stdout.strip()) + + def is_dirty(self, path: str, cwd: Path) -> bool: + result = subprocess.run( + ["git", "status", "--porcelain", "--", path], + cwd=str(cwd), + capture_output=True, + text=True, + ) + return bool(result.stdout.strip()) + # --------------------------------------------------------------------------- # Result records @@ -236,6 +287,16 @@ def _ranges_overlap(a_start: int, a_end: int, b_start: int, b_end: int) -> bool: return a_start <= b_end and b_start <= a_end +# --------------------------------------------------------------------------- +# Lifecycle deletion kinds (delete / extract-then-delete) +# --------------------------------------------------------------------------- + +# These two kinds are NOT looked up in KIND_TABLE (owned by report_builder.py / +# validate_report.py) — they carry no anchor and are handled as their own +# atomic per-path operation, mirroring move-to-archive's special-casing. +_LIFECYCLE_DELETE_KINDS = {"delete", "extract-then-delete"} + + # --------------------------------------------------------------------------- # PatchApplier # --------------------------------------------------------------------------- @@ -258,10 +319,12 @@ class PatchApplier: project_root: Path, fs: Optional[_FileSystem] = None, git: Optional[_Git] = None, + git_state: Optional[_GitState] = None, ) -> None: self._root = Path(project_root) self._fs = fs or RealFileSystem() self._git = git or RealGit() + self._git_state = git_state or RealGitState() # ------------------------------------------------------------------ # Public API @@ -341,6 +404,9 @@ class PatchApplier: continue ee = entry.get("exact_edit", {}) kind = ee.get("kind", "") + if kind in _LIFECYCLE_DELETE_KINDS: + # Handled by _apply_lifecycle_delete below, not KIND_TABLE. + continue if kind not in KIND_TABLE: step0_failures.append(_skipped( path, kind or "(unknown)", idx, @@ -365,6 +431,26 @@ class PatchApplier: )) return applied, skipped_out, failed_out, staged + # Step 0b: lifecycle delete / extract-then-delete — handled as their + # own atomic per-path operation (no anchor, no KIND_TABLE lookup). + lifecycle_batch = [ + (i, e) for i, e in batch + if e.get("exact_edit", {}).get("kind") in _LIFECYCLE_DELETE_KINDS + ] + if lifecycle_batch: + if len(batch) > 1: + for idx, entry in batch: + kind = entry.get("exact_edit", {}).get("kind", "(unknown)") + skipped_out.append(_skipped( + path, kind, idx, + "incompatible-ops-on-file", + "delete/extract-then-delete cannot be combined with " + "other ops on the same path; re-run check to re-classify.", + )) + return applied, skipped_out, failed_out, staged + idx, entry = lifecycle_batch[0] + return self._apply_lifecycle_delete(path, idx, entry) + # Step 1: Incompatible-ops detection (structural — before any IO). incompatibility = self._check_incompatible_ops(path, batch) if incompatibility: @@ -561,6 +647,73 @@ class PatchApplier: return applied, skipped_out, failed_out, staged + # ------------------------------------------------------------------ + # Lifecycle delete / extract-then-delete (ADR-0039) + # ------------------------------------------------------------------ + + def _apply_lifecycle_delete( + self, path: str, idx: int, entry: dict + ) -> tuple[list, list, list, list]: + """Apply a single `delete` or `extract-then-delete` entry. + + Re-verifies live git tracked/clean state at apply time whenever the + entry's cached `safety_tier` is `auto` — never trusting the report's + cached tier or `lifecycle.git_state` (ADR-0039). A `confirm`-tier + entry was already gated through explicit human approval upstream + (doc-clean's batch-confirm), so its tracked/dirty state is expected + and is NOT re-verified here — only auto verdicts get downgraded. + + `extract-then-delete` additionally fails closed per-entry: the git rm + only happens once the caller (clean skill / subagent) has marked the + extraction step complete via `extraction_complete: true` on the + entry. This applier never performs the generative extraction itself. + + Directory-rule aggregate entries (`exact_edit.is_directory: true`) + bypass the content-hash guard entirely (no single file to hash) but + NEVER bypass the git-state re-verification. + """ + applied: list[dict] = [] + skipped_out: list[dict] = [] + failed_out: list[dict] = [] + staged: list[str] = [] + + ee = entry.get("exact_edit", {}) + kind = ee.get("kind", "") + is_directory = bool(ee.get("is_directory", False)) + abs_path = self._root / path + tier = entry.get("safety_tier") + + if tier == "auto": + tracked = self._git_state.is_tracked(path, self._root) + dirty = self._git_state.is_dirty(path, self._root) if tracked else False + if not tracked or dirty: + skipped_out.append(_skipped( + path, kind, idx, + "git-state-changed-since-check", + "Path is no longer tracked+clean since the check ran; " + "re-run hygiene check to re-verify before deleting.", + )) + return applied, skipped_out, failed_out, staged + + if kind == "extract-then-delete" and not entry.get("extraction_complete", False): + skipped_out.append(_skipped( + path, kind, idx, + "extraction-not-confirmed", + "Extraction step has not been marked complete; the delete " + "is withheld for this entry (fails closed, not a hard failure).", + )) + return applied, skipped_out, failed_out, staged + + try: + self._git.rm(abs_path, self._root, recursive=is_directory) + except Exception as exc: # noqa: BLE001 + failed_out.append(_failed(path, kind, idx, str(exc))) + return applied, skipped_out, failed_out, staged + + applied.append(_applied(path, kind, idx)) + staged.append(path) + return applied, skipped_out, failed_out, staged + # ------------------------------------------------------------------ # Incompatible-ops detection # ------------------------------------------------------------------ diff --git a/plugins/os-doc-hygiene/scripts/report_builder.py b/plugins/os-doc-hygiene/scripts/report_builder.py index fa63745..eb690e4 100644 --- a/plugins/os-doc-hygiene/scripts/report_builder.py +++ b/plugins/os-doc-hygiene/scripts/report_builder.py @@ -49,6 +49,7 @@ from __future__ import annotations import argparse import hashlib import json +import subprocess import sys from datetime import datetime, timezone from pathlib import Path @@ -68,6 +69,7 @@ from token_estimator import default_estimator, TokenEstimator # noqa: E402 SCHEMA_VERSION = "1.0" _PLUGIN_JSON = _SCRIPTS_DIR.parent / ".claude-plugin" / "plugin.json" +_CONVENTIONS_JSON = _SCRIPTS_DIR.parent / "conventions.json" _VALID_OP_TYPES = ("deterministic", "generative") _STALE_SUBTYPES = {"contradicted", "orphaned", "superseded", "provisional", "completed-in-place", "duplicated"} @@ -121,6 +123,48 @@ class RealFileReader: return Path(path).read_text(encoding="utf-8", errors="replace") +class _GitStateProvider(Protocol): + def state(self, root: Path, rel_path: str) -> dict: ... + + +class RealGitStateProvider: + """Production git-state provider — `git ls-files` (tracked) + `git status + --porcelain` (dirty), scoped to a single path. Advisory-fresh at + report-build time; `patch_applier.py` re-verifies at apply time (design + Decision 3 / ADR-0039 — never trust the rule's own claim).""" + + def state(self, root: Path, rel_path: str) -> dict: + try: + ls = subprocess.run( + ["git", "ls-files", "--error-unmatch", "--", rel_path], + cwd=str(root), + capture_output=True, + text=True, + ) + tracked = ls.returncode == 0 and bool(ls.stdout.strip()) + except OSError: + tracked = False + + if not tracked: + return {"tracked": False, "clean": False} + + try: + status = subprocess.run( + ["git", "status", "--porcelain", "--", rel_path], + cwd=str(root), + capture_output=True, + text=True, + ) + clean = status.returncode == 0 and status.stdout.strip() == "" + except OSError: + clean = False + + return {"tracked": True, "clean": clean} + + +_LIFECYCLE_OP_KINDS = ("delete", "extract-then-delete") + + def _iso(dt: datetime) -> str: """Serialise a datetime as ISO-8601 UTC.""" if dt.tzinfo is None: @@ -172,11 +216,17 @@ class ReportBuilder: reader: Optional[_FileReader] = None, estimator: Optional[TokenEstimator] = None, tool_version: Optional[str] = None, + git_state_provider: Optional[_GitStateProvider] = None, + conventions_path: Optional[Path] = None, ) -> None: self._root = Path(project_root) self._clock = clock or RealClock() self._reader = reader or RealFileReader() self._estimator = estimator or default_estimator() + self._git_state_provider = git_state_provider or RealGitStateProvider() + self._conventions_path = ( + Path(conventions_path) if conventions_path is not None else _CONVENTIONS_JSON + ) if tool_version is not None: self._tool_version = tool_version self._tool_version_fell_back = False @@ -223,6 +273,7 @@ class ReportBuilder: "scan": scan_block, "shortlist": shortlist, "entries": entries, + "promotion_candidates": self._build_promotion_candidates(entries), } human_report = self._build_human_report( @@ -347,8 +398,13 @@ class ReportBuilder: entry["safety_tier"] = derive_safety_tier( op_type, spec.is_destructive, spec.is_reversible ) - exact_edit = self._build_exact_edit(proposal["exact_edit"], spec, abs_path) + exact_edit = self._build_exact_edit(proposal["exact_edit"], spec, abs_path, path) entry["exact_edit"] = exact_edit + if kind in _LIFECYCLE_OP_KINDS: + entry["safety_tier"] = derive_safety_tier( + op_type, spec.is_destructive, spec.is_reversible, + lifecycle=exact_edit.get("lifecycle"), + ) span_text = self._extract_span(proposal["exact_edit"], kind, abs_path) else: # generative # No exact_edit; (is_destructive, is_reversible) characterize the op. @@ -362,7 +418,7 @@ class ReportBuilder: entry["token_estimate"] = self._estimator.estimate_for_report(span_text) return entry - def _build_exact_edit(self, skeleton: dict, spec, abs_path: Path) -> dict: + def _build_exact_edit(self, skeleton: dict, spec, abs_path: Path, rel_path: str) -> dict: """Copy the skeleton and stamp the deterministic fields.""" kind = skeleton["kind"] exact_edit: dict[str, Any] = {"kind": kind} @@ -379,6 +435,16 @@ class ReportBuilder: file_bytes = self._reader.read_bytes(abs_path) exact_edit["expected_sha256"] = hashlib.sha256(file_bytes).hexdigest() exact_edit["generated_at"] = _iso(hash_instant) + + if kind in _LIFECYCLE_OP_KINDS: + # git_state is a guardrail field (like expected_sha256): NEVER + # trust a proposal-supplied value — always recomputed against the + # live worktree at build time. Advisory-fresh; patch_applier.py + # re-verifies authoritatively at apply time (ADR-0039). + lifecycle = dict(exact_edit.get("lifecycle") or {}) + lifecycle["git_state"] = self._git_state_provider.state(self._root, rel_path) + exact_edit["lifecycle"] = lifecycle + return exact_edit # ------------------------------------------------------------------ @@ -395,6 +461,10 @@ class ReportBuilder: # NOTE: a documented deviation — the design did not specify a span # for insert-frontmatter. return self._reader.read_text(abs_path) + if kind in _LIFECYCLE_OP_KINDS: + # delete / extract-then-delete removes the whole file → count the + # whole file, same rationale as move-to-archive. + return self._reader.read_text(abs_path) # delete-range / replace-text / dedupe — count the anchored span. return self._read_range(abs_path, skeleton["anchor"]) @@ -408,6 +478,57 @@ class ReportBuilder: return "" return "\n".join(lines[start - 1:end]) + # ------------------------------------------------------------------ + # Promotion candidates (ADR-0041, determinism-promotion — no model) + # ------------------------------------------------------------------ + + def _load_conventions(self) -> list: + try: + data = json.loads(self._conventions_path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + return [] + return data if isinstance(data, list) else [] + + def _entry_lifecycle(self, entry: dict) -> Optional[dict]: + """Return the entry's lifecycle object, whether it lives on the entry + itself (a signal-only, non-deleting match) or nested under a + delete/extract-then-delete exact_edit.""" + if isinstance(entry.get("lifecycle"), dict): + return entry["lifecycle"] + exact_edit = entry.get("exact_edit") + if isinstance(exact_edit, dict) and isinstance(exact_edit.get("lifecycle"), dict): + return exact_edit["lifecycle"] + return None + + def _build_promotion_candidates(self, entries: list) -> list: + """Deterministic, model-free: for every entry whose lifecycle + evidence is classifier-judged (served_when present, no + served_when_path — which by construction means "not yet adopted", + since adoption IS graduating to served_when_path), name every + catalog convention as a promotion candidate.""" + conventions = self._load_conventions() + if not conventions: + return [] + + candidates: list = [] + for entry in entries: + lifecycle = self._entry_lifecycle(entry) + if lifecycle is None: + continue + has_served_when = bool(lifecycle.get("served_when")) + has_served_when_path = bool(lifecycle.get("served_when_path")) + if not has_served_when or has_served_when_path: + continue + for conv in conventions: + if not isinstance(conv, dict): + continue + name = conv.get("name") + pitch = conv.get("pitch") + if not name or not pitch: + continue + candidates.append({"path": entry["path"], "name": name, "pitch": pitch}) + return candidates + # ------------------------------------------------------------------ # Scan block # ------------------------------------------------------------------ @@ -470,6 +591,10 @@ class ReportBuilder: lines.append("- (none)") lines.append("") + lines.extend( + self._render_promotion_candidates(machine_report.get("promotion_candidates", [])) + ) + return "\n".join(lines) def _render_group(self, title: str, group: list, gloss_by_path: dict) -> list: @@ -492,6 +617,30 @@ class ReportBuilder: gloss = gloss_by_path.get(e["path"]) if gloss: lines.append(f" - {gloss}") + lifecycle = self._entry_lifecycle(e) + if lifecycle: + rule_ref = lifecycle.get("rule_ref", "?") + lifetime = lifecycle.get("lifetime", "?") + served = ( + f"served_when_path: {lifecycle['served_when_path']}" + if lifecycle.get("served_when_path") + else f"served_when: {lifecycle.get('served_when', '?')}" + ) + lines.append(f" - lifecycle: rule={rule_ref} · lifetime={lifetime} · {served}") + lines.append("") + return lines + + def _render_promotion_candidates(self, candidates: list) -> list: + """Renders the deterministic promotion_candidates section (ADR-0041) + into the human report — always present, even when empty, per the + determinism-promotion spec.""" + lines = ["## Promotion Candidates", ""] + if not candidates: + lines.append("- (none)") + lines.append("") + return lines + for c in candidates: + lines.append(f"- {c['path']} → adopt `{c['name']}`: {c['pitch']}") lines.append("") return lines diff --git a/plugins/os-doc-hygiene/scripts/rulebook.py b/plugins/os-doc-hygiene/scripts/rulebook.py new file mode 100644 index 0000000..a8aaf25 --- /dev/null +++ b/plugins/os-doc-hygiene/scripts/rulebook.py @@ -0,0 +1,336 @@ +""" +rulebook.py — lifecycle rulebook loader for os-doc-hygiene. + +Stdlib-only. Loads the global rulebook (`plugins/os-doc-hygiene/rulebook.json`) +and an optional per-project override (`/.dochygiene-rules.json`), +both under the envelope `{"schema_version": 1, "rules": [...]}`. Merges +add-only (project rules are appended to, never replace, global rules) and +exposes a single query surface: given a repo-root-relative path (and whether +it is a directory), return `None` (unmatched) or the winning `RuleMatch`. + +No knowledge of git, classification, or the report schema — this module only +resolves "which rule, if any, governs this path" (design.md Decision 1). +""" + +from __future__ import annotations + +import glob +import json +import sys +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any, Optional + +_MIN_PYTHON = (3, 13) + +_VALID_LIFETIMES = {"keep", "temporary", "delete-once-served"} + +# Fields recognized by the schema (lifecycle-spec.md §2). Any field on a +# rule outside this set (e.g. the removed `propagate_ignore`) is rejected. +_KNOWN_FIELDS = { + "glob", + "lifetime", + "extract", + "served_when", + "served_when_path", + "retain_recent", + "max_age_days", + "confirm", + "confirmed_by", + "confirmed_on", + "source", + "note", +} + +_DEFAULT_RETAIN_RECENT = 3 +_DEFAULT_MAX_AGE_DAYS = 3 + +_SUPPORTED_SCHEMA_VERSIONS = {1} + + +class RulebookLoadError(Exception): + """Hard-fail: unparseable JSON, unknown schema_version, or unsupported + Python version for the mandated glob dialect.""" + + +@dataclass(frozen=True) +class RuleMatch: + """The winning rule for a queried path.""" + + rule: dict + level: str # "file" | "directory" + is_ignore: bool + origin: str # "project" | "global" + + +@dataclass +class _CompiledRule: + raw: dict + origin: str # "project" | "global" + level: str # "file" | "directory" + is_ignore: bool + pattern_for_matching: str + matcher: Any # compiled regex (re.Pattern) — exact match on this level + subtree_matcher: Optional[Any] # directory rules only: matches paths + # anywhere beneath the directory (files or nested dirs), used when a + # directory-rule competes for precedence over a *file* path. + define_order: int + + +def _check_python_version() -> None: + if sys.version_info[:2] < _MIN_PYTHON: + raise RulebookLoadError( + "rulebook.py requires Python >= 3.13 for glob.translate(" + "recursive=True, include_hidden=True); running under " + f"{sys.version_info[0]}.{sys.version_info[1]}." + ) + + +def _load_json_envelope(path: Path) -> dict: + try: + text = path.read_text(encoding="utf-8") + except FileNotFoundError: + return {"schema_version": 1, "rules": []} + try: + data = json.loads(text) + except json.JSONDecodeError as exc: + raise RulebookLoadError(f"{path}: unparseable JSON: {exc}") from exc + + if not isinstance(data, dict) or "schema_version" not in data: + raise RulebookLoadError( + f"{path}: missing or malformed envelope (expected " + '{"schema_version": 1, "rules": [...]})' + ) + + schema_version = data.get("schema_version") + if schema_version not in _SUPPORTED_SCHEMA_VERSIONS: + raise RulebookLoadError( + f"{path}: unrecognized schema_version {schema_version!r} " + f"(supported: {sorted(_SUPPORTED_SCHEMA_VERSIONS)})" + ) + + rules = data.get("rules", []) + if not isinstance(rules, list): + raise RulebookLoadError(f"{path}: 'rules' must be a list") + + return data + + +def _is_directory_glob(pattern: str) -> bool: + return pattern.endswith("/**") or pattern.endswith("/") + + +def _base_pattern_for_directory(pattern: str) -> str: + if pattern.endswith("/**"): + return pattern[: -len("/**")] + if pattern.endswith("/"): + return pattern[: -len("/")] + return pattern + + +def _validate_and_compile_rule( + raw: dict, origin: str, define_order: int, warnings: list +) -> Optional[_CompiledRule]: + label = raw.get("glob", "") + + unknown_fields = set(raw.keys()) - _KNOWN_FIELDS + if unknown_fields: + warnings.append( + f"rule {label!r} ({origin}): unrecognized field(s) " + f"{sorted(unknown_fields)} — rule skipped and marked inactive" + ) + return None + + glob_pattern = raw.get("glob") + if not glob_pattern or not isinstance(glob_pattern, str): + warnings.append( + f"rule {label!r} ({origin}): missing or invalid 'glob' field — " + "rule skipped and marked inactive" + ) + return None + + lifetime = raw.get("lifetime") + is_ignore = lifetime is None + if lifetime is not None and lifetime not in _VALID_LIFETIMES: + warnings.append( + f"rule {glob_pattern!r} ({origin}): invalid lifetime " + f"{lifetime!r} — rule skipped and marked inactive" + ) + return None + + if not raw.get("confirmed_by"): + warnings.append( + f"rule {glob_pattern!r} ({origin}): missing 'confirmed_by' — " + "rule loaded but marked inactive, never contributes a match" + ) + return None + + level = "directory" if _is_directory_glob(glob_pattern) else "file" + match_pattern = ( + _base_pattern_for_directory(glob_pattern) + if level == "directory" + else glob_pattern + ) + + import re + + try: + regex = glob.translate( + match_pattern, recursive=True, include_hidden=True + ) + matcher = re.compile(regex) + except Exception as exc: # pragma: no cover - defensive + warnings.append( + f"rule {glob_pattern!r} ({origin}): failed to compile glob: " + f"{exc} — rule skipped and marked inactive" + ) + return None + + subtree_matcher = None + if level == "directory": + # The un-stripped pattern (e.g. "notes/**") matches paths *beneath* + # the directory too — needed so this directory-rule can compete for + # precedence against file-rules on a file path (design.md Decision 1 + # two-axis precedence is path-based, not query-level-based: a file + # path can be governed by a directory-rule that covers it). + try: + subtree_regex = glob.translate( + glob_pattern, recursive=True, include_hidden=True + ) + subtree_matcher = re.compile(subtree_regex) + except Exception as exc: # pragma: no cover - defensive + warnings.append( + f"rule {glob_pattern!r} ({origin}): failed to compile " + f"subtree glob: {exc} — rule skipped and marked inactive" + ) + return None + + resolved = dict(raw) + if lifetime == "temporary": + resolved.setdefault("retain_recent", _DEFAULT_RETAIN_RECENT) + resolved.setdefault("max_age_days", _DEFAULT_MAX_AGE_DAYS) + + return _CompiledRule( + raw=resolved, + origin=origin, + level=level, + is_ignore=is_ignore, + pattern_for_matching=match_pattern, + matcher=matcher, + subtree_matcher=subtree_matcher, + define_order=define_order, + ) + + +# Precedence tiers, lowest wins first (spec: project file > project dir > +# global file > global dir). +_TIER = { + ("project", "file"): 0, + ("project", "directory"): 1, + ("global", "file"): 2, + ("global", "directory"): 3, +} + + +class Rulebook: + """Merged, compiled rulebook exposing a single query surface.""" + + def __init__(self, compiled_rules: list, warnings: list): + self._rules = compiled_rules + self.warnings = warnings + + def query(self, path: str, is_dir: bool = False) -> Optional[RuleMatch]: + """Return the winning rule for *path*, or None if unmatched. + + Precedence is resolved across *all* rules that cover this path, + regardless of whether the winning rule is a file-rule or a + directory-rule: a directory-rule's subtree covers files beneath it, + so it can compete for (and win, per its tier) precedence on a file + path just as a file-rule can. The returned RuleMatch.level reflects + the winning rule's own declared type, not the caller's *is_dir*. + """ + norm_path = path.strip("/") + + candidates = [] + for cr in self._rules: + if cr.level == "file": + # A file-rule's pattern is matched exactly; it only ever + # governs the exact path it names. + if cr.matcher.fullmatch(norm_path): + candidates.append(cr) + else: # directory-rule + if is_dir: + # Directory query: does this rule's own subtree glob + # match the directory path itself? + if cr.matcher.fullmatch(norm_path): + candidates.append(cr) + else: + # File query: does this directory-rule's subtree cover + # this file path? + if cr.subtree_matcher.fullmatch(norm_path): + candidates.append(cr) + + if not candidates: + return None + + def sort_key(cr: _CompiledRule): + tier = _TIER[(cr.origin, cr.level)] + # Longer pattern wins ties -> negate for ascending sort. + # Last-defined wins ties -> negate define_order. + return (tier, -len(cr.pattern_for_matching), -cr.define_order) + + winner = min(candidates, key=sort_key) + return RuleMatch( + rule=winner.raw, + level=winner.level, + is_ignore=winner.is_ignore, + origin=winner.origin, + ) + + +def default_global_rulebook_path() -> Path: + """The global rulebook path resolved relative to this script's own + location: `/rulebook.json` (this file lives in + `/scripts/`).""" + return Path(__file__).resolve().parent.parent / "rulebook.json" + + +def load_rulebook( + global_path: Optional[Path] = None, project_path: Optional[Path] = None +) -> Rulebook: + """Load and merge the global rulebook with an optional project override. + + Hard-fails (raises RulebookLoadError) on unparseable JSON, an + unrecognized schema_version, or an unsupported Python version. + Skips and warns on a per-rule basis for invalid rules or rules missing + `confirmed_by`. + """ + _check_python_version() + + global_path = Path(global_path) if global_path is not None else default_global_rulebook_path() + global_data = _load_json_envelope(global_path) + + project_data: dict + if project_path is not None: + project_path = Path(project_path) + project_data = _load_json_envelope(project_path) + else: + project_data = {"schema_version": 1, "rules": []} + + warnings: list = [] + compiled: list = [] + define_order = 0 + + for raw in global_data.get("rules", []): + cr = _validate_and_compile_rule(raw, "global", define_order, warnings) + define_order += 1 + if cr is not None: + compiled.append(cr) + + for raw in project_data.get("rules", []): + cr = _validate_and_compile_rule(raw, "project", define_order, warnings) + define_order += 1 + if cr is not None: + compiled.append(cr) + + return Rulebook(compiled_rules=compiled, warnings=warnings) diff --git a/plugins/os-doc-hygiene/scripts/scanner.py b/plugins/os-doc-hygiene/scripts/scanner.py index 74edf9e..8086da4 100644 --- a/plugins/os-doc-hygiene/scripts/scanner.py +++ b/plugins/os-doc-hygiene/scripts/scanner.py @@ -22,11 +22,20 @@ import os import re import subprocess import sys +from datetime import datetime from pathlib import Path -from typing import Callable, Iterator, List, Optional +from typing import Any, Callable, Iterator, List, Optional from token_estimator import TokenEstimator, default_estimator +try: # rulebook.py is optional-at-import (requires Python >= 3.13 for its + # own glob.translate usage); the scanner itself has no hard dependency + # on it — a Scanner constructed without a `rulebook=` argument behaves + # exactly as before this change (design.md Decision 2). + from rulebook import RuleMatch # noqa: F401 +except Exception: # pragma: no cover - defensive, see comment above + RuleMatch = object # type: ignore + # --------------------------------------------------------------------------- # Glob matching (Python 3.14-compatible) @@ -544,6 +553,161 @@ def _git_log_real(path: Path) -> List[str]: return [] +def _git_commit_time_real(path: Path) -> Optional[str]: + """Return the ISO-8601 committer time of *path*'s most recent commit. + + Returns ``None`` when the path is untracked (no commit history) or on + any subprocess failure — callers fall back to filesystem mtime. + """ + try: + result = subprocess.run( + ["git", "log", "-1", "--format=%cI", "--", str(path)], + capture_output=True, + text=True, + timeout=10, + ) + out = result.stdout.strip() + return out or None + except Exception: + return None + + +# --------------------------------------------------------------------------- +# Lifecycle signal computation (design.md Decision 2 / 4) +# --------------------------------------------------------------------------- + +_LIFECYCLE_SIGNAL_NAME = "lifecycle" + + +def _substitute_served_when_path(pattern: str, basename: str) -> str: + """Substitute ``{id}``/``{name}`` in a served_when_path pattern with the + matched entry's basename.""" + return pattern.replace("{id}", basename).replace("{name}", basename) + + +class LifecycleSignalBuilder: + """Builds lifecycle signal dicts from rulebook matches. + + Injected dependencies mirror ``SignalComputer``: a git-commit-time + provider and a clock, both fakeable for tests. Age/retention are the + only stateful parts — retention ranking requires all matches for a rule + to be known first, so ``rank_temporary_matches`` is a separate pass run + once the whole walk has completed. + """ + + def __init__( + self, + root: Path, + git_commit_time_fn: Optional[Callable[[Path], Optional[str]]] = None, + now_fn: Optional[Callable[[], float]] = None, + ) -> None: + self._root = root + self._git_commit_time_fn = git_commit_time_fn + self._now_fn = now_fn or (lambda: __import__("time").time()) + + # ------------------------------------------------------------------ + def age_days(self, rel_path: str, is_dir: bool) -> Optional[float]: + """Age in days: git commit time, falling back to mtime for + untracked paths. Untracked directories use one stat() on the + directory inode itself (never a recursive max-mtime walk).""" + abs_path = self._root / rel_path + + commit_iso: Optional[str] = None + if self._git_commit_time_fn is not None: + try: + commit_iso = self._git_commit_time_fn(abs_path) + except Exception: + commit_iso = None + + if commit_iso: + try: + commit_dt = datetime.fromisoformat(commit_iso) + commit_ts = commit_dt.timestamp() + return (self._now_fn() - commit_ts) / 86400.0 + except ValueError: + pass # fall through to mtime + + # Untracked (or unparseable commit time): filesystem mtime. For a + # directory this is a single stat() on the directory inode itself — + # never a recursive walk over its contents (design.md Decision 4). + try: + stat = abs_path.stat() + except OSError: + return None + return (self._now_fn() - stat.st_mtime) / 86400.0 + + # ------------------------------------------------------------------ + def build(self, rel_path: str, match: "RuleMatch", is_dir: bool) -> dict: + """Build the lifecycle signal dict for a matched (non-ignore) rule.""" + rule = match.rule + lifetime = rule.get("lifetime") + basename = Path(rel_path).name + + sig: dict = { + "name": _LIFECYCLE_SIGNAL_NAME, + "rule_ref": rule.get("glob"), + "lifetime": lifetime, + "detail": f"matched lifecycle rule {rule.get('glob')!r} (lifetime={lifetime})", + } + if rule.get("extract"): + sig["extract"] = True + + served_when_path = rule.get("served_when_path") + served_when = rule.get("served_when") + if served_when_path: + resolved = _substitute_served_when_path(served_when_path, basename) + satisfied = (self._root / resolved).exists() + sig["served"] = { + "kind": "scanner-proven", + "served_when_path": served_when_path, + "resolved_path": resolved, + "satisfied": satisfied, + } + elif served_when: + sig["served"] = { + "kind": "classifier-judged", + "served_when": served_when, + } + + if lifetime == "temporary": + sig["age_days"] = self.age_days(rel_path, is_dir) + sig["retain_recent"] = rule.get("retain_recent", 3) + sig["max_age_days"] = rule.get("max_age_days", 3) + + return sig + + +def rank_temporary_matches(entries: List[dict]) -> None: + """Mutate *entries*' lifecycle signal dicts in-place with retention info. + + *entries* is a list of ``{"rule": , "signal": }`` + covering every temporary-lifetime match produced during a single scan. + Grouped by rule identity (the same rule dict object is shared across all + its matches within one Rulebook), ranked newest-first by age_days: the + top ``retain_recent`` are always kept; rank ``retain_recent + 1`` or + older is flagged deletable once it exceeds ``max_age_days``. + """ + groups: dict = {} + for entry in entries: + groups.setdefault(id(entry["rule"]), []).append(entry) + + for group in groups.values(): + # Newest first: smallest age_days first. None age sorts last. + group.sort(key=lambda e: (e["signal"].get("age_days") is None, e["signal"].get("age_days") or 0.0)) + retain_recent = group[0]["signal"].get("retain_recent", 3) if group else 3 + max_age_days = group[0]["signal"].get("max_age_days", 3) if group else 3 + for rank, entry in enumerate(group, start=1): + sig = entry["signal"] + age = sig.get("age_days") + kept_by_rank = rank <= retain_recent + deletable = (not kept_by_rank) and (age is not None) and (age > max_age_days) + sig["retention"] = { + "rank": rank, + "kept": kept_by_rank, + "deletable": deletable, + } + + # --------------------------------------------------------------------------- # Scanner # --------------------------------------------------------------------------- @@ -586,6 +750,8 @@ class Scanner: max_lines: int = DEFAULT_MAX_LINES, max_tokens: int = DEFAULT_MAX_TOKENS, token_estimator: Optional[TokenEstimator] = None, + rulebook: Optional["Any"] = None, + git_commit_time_fn: Optional[Callable[[Path], Optional[str]]] = None, ) -> None: self._root = root.resolve() self._scope_globs = scope_globs if scope_globs is not None else DEFAULT_SCOPE_GLOBS @@ -623,6 +789,15 @@ class Scanner: self._max_tokens = max_tokens self._token_estimator = token_estimator self._ignore_patterns: List[str] = _load_ignore_patterns(self._root) + self._rulebook = rulebook + self._lifecycle_builder = LifecycleSignalBuilder( + root=self._root, + git_commit_time_fn=git_commit_time_fn, + now_fn=now_fn, + ) + # Directory-rule aggregate entries pruned during the walk (design.md + # Decision 2): populated by _walk_scoped, consumed by run(). + self._pruned_dir_matches: List[tuple] = [] # (rel_path, RuleMatch) def _is_excluded_child(self, parent_dirpath: str, child_name: str) -> bool: """Return True if walking would prune *child_name* under *parent_dirpath*. @@ -658,6 +833,10 @@ class Scanner: files_scanned = 0 shortlist: List[str] = [] signals_map: dict = {} + # Collected for the post-walk retention-ranking pass (design.md + # Decision 4): every temporary-lifetime match, dir or file, in one + # flat list so rank_temporary_matches can group by rule identity. + temporary_entries: List[dict] = [] for path in self._walk_scoped(): files_scanned += 1 @@ -677,10 +856,46 @@ class Scanner: # Survived exclusions — compute signals sigs = signal_computer.compute(path) + + # Rulebook file-rule lifecycle signal attachment (design.md + # Decision 2): alongside any pre-existing objective signals. + if self._rulebook is not None: + match = self._rulebook.query(rel, is_dir=False) + if match is not None and not match.is_ignore: + lifecycle_sig = self._lifecycle_builder.build( + rel, match, is_dir=False + ) + sigs = list(sigs) + [lifecycle_sig] + if lifecycle_sig.get("lifetime") == "temporary": + temporary_entries.append( + {"rule": match.rule, "signal": lifecycle_sig} + ) + shortlist.append(rel) if sigs: signals_map[rel] = sigs + # Directory-rule aggregate entries pruned during the walk: exactly + # one shortlist/signal entry per matched directory (design.md + # Decision 2) — never one entry per file beneath it (those files + # were never opened). + for dir_rel, match in self._pruned_dir_matches: + lifecycle_sig = self._lifecycle_builder.build( + dir_rel, match, is_dir=True + ) + shortlist.append(dir_rel) + signals_map[dir_rel] = [lifecycle_sig] + if lifecycle_sig.get("lifetime") == "temporary": + temporary_entries.append( + {"rule": match.rule, "signal": lifecycle_sig} + ) + + # Retention ranking requires every match for a rule to be known + # first, so this runs once, after the full walk (design.md + # Decision 4). + if temporary_entries: + rank_temporary_matches(temporary_entries) + return { "project_root": str(self._root), "scope_globs": self._scope_globs, @@ -696,6 +911,7 @@ class Scanner: def _walk_scoped(self) -> Iterator[Path]: """Yield files matching scope_globs, with excluded dirs pruned at walk time.""" + self._pruned_dir_matches = [] for dirpath, dirnames, filenames in os.walk(str(self._root)): # Prune excluded dirs IN-PLACE so os.walk never descends them (D7). # Path-aware pairs (e.g. examples/golden) are matched against the @@ -704,6 +920,25 @@ class Scanner: d for d in dirnames if not self._is_excluded_child(dirpath, d) ] + # Rulebook-driven directory-rule pruning (design.md Decision 2). + # Checked BEFORE opening any file beneath a matched directory — + # a directory-rule match (including IGNORE-surface entries) is + # pruned from dirnames so os.walk never descends into it. + if self._rulebook is not None: + survivors: List[str] = [] + for d in dirnames: + dir_abs = Path(dirpath) / d + dir_rel = str(dir_abs.relative_to(self._root)) + match = self._rulebook.query(dir_rel, is_dir=True) + if match is None: + survivors.append(d) + continue + # Matched: prune regardless of is_ignore. + if not match.is_ignore: + self._pruned_dir_matches.append((dir_rel, match)) + # IGNORE-surface matches: pruned with zero emission. + dirnames[:] = survivors + for filename in filenames: filepath = Path(dirpath) / filename rel = filepath.relative_to(self._root) @@ -771,6 +1006,7 @@ def main(argv: Optional[List[str]] = None) -> int: scope_globs=args.globs, excluded_dirs=args.excluded_dirs, git_log_fn=_git_log_real, + git_commit_time_fn=_git_commit_time_real, max_lines=args.max_lines, max_tokens=args.max_tokens, ) diff --git a/plugins/os-doc-hygiene/scripts/validate_report.py b/plugins/os-doc-hygiene/scripts/validate_report.py index 1d8828e..ebb3da6 100644 --- a/plugins/os-doc-hygiene/scripts/validate_report.py +++ b/plugins/os-doc-hygiene/scripts/validate_report.py @@ -18,17 +18,52 @@ Output: structured JSON on stdout with keys: import json import sys from dataclasses import dataclass, field -from typing import Any +from typing import Any, Optional # --------------------------------------------------------------------------- # Safety-tier derivation (invariant #10) # --------------------------------------------------------------------------- -def derive_safety_tier(op_type: str, is_destructive: bool, is_reversible: bool) -> str: +def _lifecycle_is_scanner_proven(lifecycle: dict) -> bool: + """True when the lifecycle evidence is a filesystem-provable fact. + + A `temporary`-lifetime match is scanner-computed (retain-recent/age); a + `served_when_path` hit is scanner-verified against the filesystem. A + `served_when` (free text) is always classifier-judged, never proven. + """ + if lifecycle.get("lifetime") == "temporary": + return True + if lifecycle.get("served_when_path"): + return True + return False + + +def derive_safety_tier( + op_type: str, + is_destructive: bool, + is_reversible: bool, + lifecycle: "dict | None" = None, +) -> str: """ Deterministic derivation of safety_tier from op characterisation. Never returns 'auto' for a generative, destructive, or irreversible op. + + ADR-0039 lifecycle branch: when `lifecycle` is supplied (i.e. this is a + `delete`/`extract-then-delete` op), the tier is derived from evidence + quality + runtime git state instead of (is_destructive, is_reversible) — + `auto` only when the lifecycle evidence is scanner-proven AND the path is + tracked AND clean; every other combination (dirty, untracked, or + classifier-judged `served_when`) forces `confirm`, regardless of the + is_destructive/is_reversible inputs. This is additive: the existing + non-lifecycle branches below are unchanged. """ + if lifecycle is not None: + git_state = lifecycle.get("git_state") or {} + tracked = bool(git_state.get("tracked")) + clean = bool(git_state.get("clean")) + if _lifecycle_is_scanner_proven(lifecycle) and tracked and clean: + return "auto" + return "confirm" if op_type == "generative": return "confirm" if is_destructive: @@ -81,11 +116,32 @@ KIND_TABLE: dict[str, KindSpec] = { required_fields=["anchor", "canonical_ref"], has_anchor=True, ), + "delete": KindSpec( + # is_destructive is fixed (git history is the only recovery path); + # is_reversible is a nominal True here — its REAL characterisation is + # git-history-dependent and is resolved by the lifecycle branch of + # derive_safety_tier, not by this fixed table value (report-schema + # spec, "Exact-Edit Kind Is a Closed Enum"). + is_destructive=True, + is_reversible=True, + required_fields=["anchor", "lifecycle"], + has_anchor=True, + ), + "extract-then-delete": KindSpec( + is_destructive=True, + is_reversible=True, + required_fields=["anchor", "lifecycle", "extraction_target"], + has_anchor=True, + ), } STALE_SUBTYPES = {"contradicted", "orphaned", "superseded", "provisional", "completed-in-place", "duplicated"} BLOAT_SUBTYPES = {"distill", "split", "freeze"} +_LIFECYCLE_LIFETIMES = {"keep", "temporary", "delete-once-served"} +_LIFECYCLE_OP_KINDS = {"delete", "extract-then-delete"} +_EXTRACTION_TARGETS = {"repo-durable", "cross-repo"} + # --------------------------------------------------------------------------- # Validator @@ -130,10 +186,17 @@ class ReportValidator: else: self._add(f"entries[{i}]", "Entry must be a JSON object") + # Rule: promotion_candidates section (report-schema delta spec). + if "promotion_candidates" in r: + self._check_promotion_candidates(r["promotion_candidates"]) + return self._violations def _check_envelope(self, r: dict) -> None: - for key in ("schema_version", "tool_version", "generated_at", "scan", "shortlist", "entries"): + for key in ( + "schema_version", "tool_version", "generated_at", "scan", + "shortlist", "entries", "promotion_candidates", + ): if key not in r: self._add(key, f"Required top-level field '{key}' is missing") @@ -185,9 +248,17 @@ class ReportValidator: if is_reversible is not None and not isinstance(is_reversible, bool): self._add(f"{pfx}.is_reversible", "is_reversible must be a boolean") + # entry-level lifecycle signal (report-schema: "Lifecycle Signal + # Fields on Shortlist and Entries") — optional; validated when present, + # regardless of op kind (a `keep`/`temporary` entry may carry it with + # no exact_edit at all). + if "lifecycle" in entry: + self._check_lifecycle(entry["lifecycle"], f"{pfx}.lifecycle", require_git_state=False) + # Rules 6, 7, 8, 9: exact_edit checks (only if op_type valid) + entry_lifecycle_for_tier = None if op_type in ("deterministic", "generative"): - self._check_exact_edit_biconditional(entry, pfx, op_type) + entry_lifecycle_for_tier = self._check_exact_edit_biconditional(entry, pfx, op_type) # Rule 9: safety_tier derivation match (only when all inputs are valid) if ( @@ -196,12 +267,15 @@ class ReportValidator: and isinstance(is_reversible, bool) and safety_tier in ("auto", "confirm") ): - expected_tier = derive_safety_tier(op_type, is_destructive, is_reversible) + expected_tier = derive_safety_tier( + op_type, is_destructive, is_reversible, lifecycle=entry_lifecycle_for_tier + ) if safety_tier != expected_tier: self._add( f"{pfx}.safety_tier", f"safety_tier mismatch: recorded '{safety_tier}' but derivation yields '{expected_tier}' " - f"for (op_type={op_type}, is_destructive={is_destructive}, is_reversible={is_reversible})", + f"for (op_type={op_type}, is_destructive={is_destructive}, is_reversible={is_reversible}, " + f"lifecycle={'present' if entry_lifecycle_for_tier is not None else 'none'})", ) # Rule 10: token_estimate.raw_tokens required @@ -209,6 +283,78 @@ class ReportValidator: if token_estimate is not None: self._check_token_estimate(token_estimate, pfx) + def _check_promotion_candidates(self, candidates: Any) -> None: + if not isinstance(candidates, list): + self._add("promotion_candidates", "promotion_candidates must be an array") + return + for i, cand in enumerate(candidates): + pfx = f"promotion_candidates[{i}]" + if not isinstance(cand, dict): + self._add(pfx, "candidate must be a JSON object") + continue + for field_name in ("path", "name", "pitch"): + value = cand.get(field_name) + if not isinstance(value, str) or not value: + self._add(f"{pfx}.{field_name}", f"candidate '{field_name}' must be a non-empty string") + + def _check_lifecycle( + self, lifecycle: Any, pfx: str, *, require_git_state: bool + ) -> Optional[dict]: + """Validate a lifecycle object's shape (rule_ref, lifetime, exactly + one served field per lifetime, optional/required git_state). + + Returns the lifecycle dict when it is well-formed enough to feed + `derive_safety_tier` (or None if too malformed to trust). + """ + if not isinstance(lifecycle, dict): + self._add(pfx, "lifecycle must be a JSON object") + return None + + rule_ref = lifecycle.get("rule_ref") + if not isinstance(rule_ref, str) or not rule_ref: + self._add(f"{pfx}.rule_ref", "lifecycle.rule_ref is required and must be a non-empty string") + + lifetime = lifecycle.get("lifetime") + if lifetime not in _LIFECYCLE_LIFETIMES: + self._add( + f"{pfx}.lifetime", + f"lifecycle.lifetime must be one of {sorted(_LIFECYCLE_LIFETIMES)}, got {lifetime!r}", + ) + + has_served_when_path = "served_when_path" in lifecycle and lifecycle["served_when_path"] is not None + has_served_when = "served_when" in lifecycle and lifecycle["served_when"] is not None + + if lifetime == "delete-once-served": + if has_served_when_path and has_served_when: + self._add( + f"{pfx}", + "lifecycle must carry exactly one of served_when_path or served_when, not both", + ) + elif not has_served_when_path and not has_served_when: + self._add( + f"{pfx}", + "lifecycle with lifetime 'delete-once-served' requires exactly one of " + "served_when_path or served_when", + ) + elif lifetime in ("keep", "temporary"): + if has_served_when_path or has_served_when: + self._add( + f"{pfx}", + f"lifecycle with lifetime {lifetime!r} must not carry served_when_path or served_when " + "(age/keep entries are not served-keyed)", + ) + + if require_git_state: + git_state = lifecycle.get("git_state") + if not isinstance(git_state, dict): + self._add(f"{pfx}.git_state", "lifecycle.git_state is required for delete/extract-then-delete") + else: + for k in ("tracked", "clean"): + if not isinstance(git_state.get(k), bool): + self._add(f"{pfx}.git_state.{k}", f"lifecycle.git_state.{k} must be a boolean") + + return lifecycle + def _check_category(self, category: Any, pfx: str) -> None: if not isinstance(category, dict): self._add(f"{pfx}.category", "category must be a JSON object with 'class' and 'subtype'") @@ -235,8 +381,13 @@ class ReportValidator: f"For class 'bloat', subtype must be one of {sorted(BLOAT_SUBTYPES)}, got '{subtype}'", ) - def _check_exact_edit_biconditional(self, entry: dict, pfx: str, op_type: str) -> None: - """Rule 6: exact_edit present IFF op_type == deterministic.""" + def _check_exact_edit_biconditional(self, entry: dict, pfx: str, op_type: str) -> Optional[dict]: + """Rule 6: exact_edit present IFF op_type == deterministic. + + Returns the exact_edit's validated `lifecycle` dict (for delete / + extract-then-delete kinds) so the caller can feed it to + `derive_safety_tier`; None otherwise. + """ has_exact_edit = "exact_edit" in entry exact_edit = entry.get("exact_edit") @@ -245,35 +396,36 @@ class ReportValidator: f"{pfx}.exact_edit", "generative entry must NOT carry exact_edit (invariant #11)", ) - return # No point validating the structure of an edit that shouldn't exist + return None # No point validating the structure of an edit that shouldn't exist if op_type == "deterministic" and not has_exact_edit: self._add( f"{pfx}.exact_edit", "deterministic entry MUST carry exact_edit (invariant #11)", ) - return + return None if op_type == "deterministic" and has_exact_edit: - self._check_exact_edit(exact_edit, pfx, entry) + return self._check_exact_edit(exact_edit, pfx, entry) + return None - def _check_exact_edit(self, exact_edit: Any, pfx: str, entry: dict) -> None: + def _check_exact_edit(self, exact_edit: Any, pfx: str, entry: dict) -> Optional[dict]: """Rules 7 and 8: kind enum, per-kind required fields, characterisation match.""" if not isinstance(exact_edit, dict): self._add(f"{pfx}.exact_edit", "exact_edit must be a JSON object") - return + return None kind = exact_edit.get("kind") if kind is None: self._add(f"{pfx}.exact_edit.kind", "exact_edit.kind is required") - return + return None if kind not in KIND_TABLE: self._add( f"{pfx}.exact_edit.kind", f"Unknown exact_edit.kind '{kind}'; valid kinds: {sorted(KIND_TABLE)}", ) - return + return None spec = KIND_TABLE[kind] @@ -285,6 +437,15 @@ class ReportValidator: f"exact_edit of kind '{kind}' requires field '{req_field}'", ) + lifecycle_for_tier: Optional[dict] = None + if kind in _LIFECYCLE_OP_KINDS: + if "lifecycle" in exact_edit: + lifecycle_for_tier = self._check_lifecycle( + exact_edit["lifecycle"], f"{pfx}.exact_edit.lifecycle", require_git_state=True + ) + if kind == "extract-then-delete": + self._check_extraction_target(exact_edit, pfx) + # Anchor must have start_line and end_line if spec.has_anchor and "anchor" in exact_edit: anchor = exact_edit["anchor"] @@ -318,6 +479,24 @@ class ReportValidator: f"kind='{kind}' requires is_reversible={spec.is_reversible}, got {entry_is_reversible}", ) + return lifecycle_for_tier + + def _check_extraction_target(self, exact_edit: dict, pfx: str) -> None: + target = exact_edit.get("extraction_target") + if target not in _EXTRACTION_TARGETS: + self._add( + f"{pfx}.exact_edit.extraction_target", + f"extraction_target must be one of {sorted(_EXTRACTION_TARGETS)}, got {target!r}", + ) + return + if target == "repo-durable": + dest = exact_edit.get("extraction_dest") + if not isinstance(dest, str) or not dest: + self._add( + f"{pfx}.exact_edit.extraction_dest", + "extraction_target 'repo-durable' requires a non-empty 'extraction_dest'", + ) + def _check_token_estimate(self, token_estimate: Any, pfx: str) -> None: if not isinstance(token_estimate, dict): self._add(f"{pfx}.token_estimate", "token_estimate must be a JSON object") diff --git a/plugins/os-doc-hygiene/skills/calibrate/SKILL.md b/plugins/os-doc-hygiene/skills/calibrate/SKILL.md new file mode 100644 index 0000000..afd69fe --- /dev/null +++ b/plugins/os-doc-hygiene/skills/calibrate/SKILL.md @@ -0,0 +1,356 @@ +--- +description: Learn new lifecycle rules for a project by clustering unmatched files, nominating candidate globs (cheap model), and having a strong model judge and confirm/reject/amend them, with a mandatory rule report to the human before any persistence. Invoked by `/os-doc-hygiene:calibrate`. +--- + +# Hygiene Calibrate Skill + +Orchestrates the learn-new-rules loop (lifecycle-spec.md §8): **cluster-and- +sample → cheap-model nominate → strong-model judge → rule report (human) → +persist → retest**. It runs over the **unmatched pool** (unmatched = unmanaged += not governed by any existing rulebook rule, per `rulebook.py`), and is the +only new skill this change adds — `check`/`clean` are unchanged in structure +(ADR-0039/-0041, `lifecycle-spec.md` §7). + +All scripts live under `${CLAUDE_PLUGIN_ROOT}/scripts/`. Run them with +`python3` from the user's project directory (`cwd`). Use the session +scratchpad directory for all intermediate artifacts. + +> **Precondition:** requires `CLAUDE_PLUGIN_ROOT`. Every script path resolves +> against it; abort rather than guessing a path if it is unset. + +> Pick a scratch dir once and reuse it: `SCRATCH="$(mktemp -d)"`. + +## What this skill NEVER does + +- It never applies a rule without the human having seen the Step 4 rule + report first (spec: "no rule shall be persisted before this report has + been shown"). +- It never removes a rule automatically — removals are HITL-only in all + cases, with recorded reasoning (spec §5). +- It never writes to the global `plugins/os-doc-hygiene/rulebook.json` + without an explicit, distinct confirmation beyond project-rule + confirmation (cross-repo write into cc-os). +- It never applies a drafted convention adoption (§6 below) without explicit + human confirmation. + +--- + +## Workflow + +### Step 1 — (D) Load the rulebook and scan for the unmatched pool + +Same rulebook-load pattern as `check`'s Step 0.5, but here the candidate pool +is the **unmatched** paths — files the current rulebook leaves ungoverned — +not the signal-bearing shortlist. + +```bash +export SCRATCH +python3 -c ' +import json, os, sys +from pathlib import Path +sys.path.insert(0, os.environ["CLAUDE_PLUGIN_ROOT"] + "/scripts") +from rulebook import load_rulebook, RulebookLoadError +from scanner import Scanner, _resolve_project_root, _git_log_real, _git_commit_time_real + +root = _resolve_project_root(Path.cwd()) +project_rules = root / ".dochygiene-rules.json" +try: + rulebook = load_rulebook(project_path=project_rules if project_rules.is_file() else None) +except RulebookLoadError as exc: + print(json.dumps({"error": "rulebook-load-failed", "detail": str(exc)})) + sys.exit(2) + +scanner = Scanner( + root=root, rulebook=rulebook, + git_log_fn=_git_log_real, git_commit_time_fn=_git_commit_time_real, +) +artifact = scanner.run() + +# The unmatched pool is every file the scan encountered that carries no +# lifecycle signal AND is not itself an IGNORE-pruned/directory-rule +# aggregate entry -- i.e. shortlist entries with no rulebook governance. +signals = artifact.get("signals", {}) +unmatched = [p for p in artifact.get("shortlist", []) if p not in signals] + +Path(os.environ["SCRATCH"] + "/scan.json").write_text(json.dumps(artifact, indent=2)) +Path(os.environ["SCRATCH"] + "/unmatched.json").write_text(json.dumps(unmatched, indent=2)) +print(f"unmatched pool: {len(unmatched)} paths") +' +``` + +- Exit `2` / rulebook load failure → hard STOP, same as `check` Step 0.5. Do + not proceed with a silently-empty rulebook. +- Note: `signals` here means ANY signal (stale/bloat/lifecycle) — a file with + a stale/bloat signal but no lifecycle rule match is still "unmatched" with + respect to the rulebook, and belongs in the pool. Filter precisely on + `lifecycle`-named signals if the project has files carrying only + non-lifecycle signals that should stay in the pool: + +```python +unmatched = [ + p for p in artifact["shortlist"] + if not any(s.get("name") == "lifecycle" for s in artifact.get("signals", {}).get(p, [])) +] +``` + +Use this refined filter, not the simpler one above, when `signals` may carry +non-lifecycle entries for shortlisted paths. + +If `unmatched` is empty → report "Nothing to calibrate — every shortlisted +file is already governed by a rulebook rule." **STOP.** + +--- + +### Step 2 — (D) Cluster and sample — `calibrate_helpers.ClusterSampler` + +Deterministic, no model. Groups unmatched paths by path-shape (directory +prefix + filename shape class — digit runs collapse to `#`, hex-looking runs +collapse to `~`) and samples representatives per cluster, capped. + +```bash +python3 -c ' +import json, os, sys +from pathlib import Path +sys.path.insert(0, os.environ["CLAUDE_PLUGIN_ROOT"] + "/scripts") +from calibrate_helpers import ClusterSampler + +unmatched = json.loads(Path(os.environ["SCRATCH"] + "/unmatched.json").read_text()) +clusters = ClusterSampler().cluster_to_dicts(unmatched) +Path(os.environ["SCRATCH"] + "/clusters.json").write_text(json.dumps(clusters, indent=2)) +print(f"{len(clusters)} clusters") +' +``` + +Each cluster is `{key, dir_prefix, shape, total, sample}`. Rules are always +proposed against a cluster, never a single instance in isolation (spec: "the +nomination is derived from a cluster of similar unmatched paths"). + +--- + +### Step 3 — (M) Cheap-model nomination — **haiku subagent, one per cluster** + +For each cluster, dispatch a **haiku** subagent (LOOP-GUARD: point it at +`workflows/nominate.md`, never this SKILL.md) constrained to nominate a bare +glob pattern + candidate lifetime — patterns only, never an exact-instance +glob (a run-id, hash, or bare timestamp hardcoded into the glob). + +``` +Agent tool parameters: +- subagent_type: "general-purpose" +- model: haiku +- description: "Nominate lifecycle rule for cluster: " +- prompt: | + Read and follow the workflow at: + ${CLAUDE_PLUGIN_ROOT}/skills/calibrate/workflows/nominate.md + + Cluster: / shape + Total matching paths: + Sample paths: + + + Return ONLY the JSON object specified in the workflow. +``` + +Collect all nominations into `"$SCRATCH/nominations.json"` (array, one per +cluster, tagged with the originating `cluster.key`). + +**Do not trust a haiku nomination as final.** The "class, never path" +rule-quality test is enforced by the strong-model judge (Step 4) plus the +deterministic `RuleQualityChecker` (Step 5's report), never accepted from +haiku at face value. + +--- + +### Step 4 — (M) Strong-model batched judgment — **ONE Opus/Fable subagent** + +Dispatch a **single batched** strong-model subagent (`model: opus`, or the +project's configured Fable-tier model) to judge ALL nominations from Step 3 +in one call (LOOP-GUARD: point it at `workflows/judge.md`, never this +SKILL.md). The judge gathers its OWN evidence — re-reads matched paths +against the live tree, checks near-miss boundaries — rather than trusting +the haiku nomination's claims. + +``` +Agent tool parameters: +- subagent_type: "general-purpose" +- model: opus +- description: "Judge doc-hygiene calibration nominations" +- prompt: | + Read and follow the workflow at: + ${CLAUDE_PLUGIN_ROOT}/skills/calibrate/workflows/judge.md + + Project root: + + Nominations to judge (verbatim): + + + Seed intake: + + Return ONLY the JSON array of verdicts specified in the workflow. +``` + +**Verdicts** are exactly one of `confirm` / `reject` / `amend` / `consult`. +`consult` is MANDATORY whenever the judge cannot determine if an artifact is +regenerable or must be retained — never resolved to `confirm` or `reject` in +that case. Write the judge's verdict array to `"$SCRATCH/verdicts.json"`. + +**Seed intake:** the #41 clutter-inventory seed candidates enter at THIS +step (judge intake), for every calibration run **except** cc-os calibration +pass #1, which withholds them as a sealed answer key (one-off carve-out, see +`lifecycle-spec.md` §9). If this run IS cc-os pass #1, do NOT include seed +candidates in the judge prompt. Every other run (including later cc-os runs) +includes full seed intake. + +--- + +### Step 5 — (D) Rule report to the human — BEFORE any persistence + +Deterministic, no model — `calibrate_helpers.RuleReportBuilder` plus +`RuleQualityChecker`. For every judge verdict of `confirm` or `amend` (never +for `reject`/`consult` — those are not proposed for persistence), assemble +the 5-element report and run the quality lints: + +```bash +python3 -c ' +import json, os, sys +from pathlib import Path +sys.path.insert(0, os.environ["CLAUDE_PLUGIN_ROOT"] + "/scripts") +from calibrate_helpers import RuleReportBuilder, RuleQualityChecker + +scan = json.loads(Path(os.environ["SCRATCH"] + "/scan.json").read_text()) +verdicts = json.loads(Path(os.environ["SCRATCH"] + "/verdicts.json").read_text()) +all_paths = scan.get("shortlist", []) + +proposed = [v["rule"] for v in verdicts if v["verdict"] in ("confirm", "amend")] +builder = RuleReportBuilder() +checker = RuleQualityChecker() + +report = [] +for rule in proposed: + entry = builder.build(rule, all_paths).to_dict() + entry["quality"] = { + "class_not_path": checker.class_not_path(rule["glob"], all_paths).to_dict(), + } + report.append(entry) + +Path(os.environ["SCRATCH"] + "/rule_report.json").write_text(json.dumps(report, indent=2)) +print(json.dumps(report, indent=2)) +' +``` + +Render this to the human as **patterns and examples, not JSON schema** — per +rule: + +``` +Proposed rule: + Lifetime: Tier: + Matches (): [+ N more] + Near-miss (does NOT match, but looks similar): + Why: + Quality check: +``` + +**If `class_not_path` failed** (a glob that can, by construction, only ever +match one file — a failed generalization), flag it LOUDLY in the rendered +report rather than silently dropping or persisting it; ask the human whether +to have the judge re-amend it (loop back to Step 4 for that one rule) or +drop it from this round. + +**No rule is written anywhere until the human has seen this report and +responded.** Ask: "Persist these N project rules? (yes / no / a subset by +number)". Only proceed to Step 6 for the rules the human approves. + +--- + +### Step 6 — (D) Persistence + +- **Project rules** (the common case): land in `/.dochygiene- + rules.json` on judge `confirm`/`amend` PLUS this step's human approval. + Read-modify-write the envelope (`{"schema_version": 1, "rules": [...]}`, + creating the file if absent), appending only — never mutating or removing + an existing rule here. +- **Global rulebook writes** (`plugins/os-doc-hygiene/rulebook.json`) require + a SEPARATE, EXPLICIT confirmation beyond the project-rule approval above — + this is a cross-repo write into cc-os itself. Ask distinctly: "This rule + looks like it belongs in the GLOBAL rulebook (applies to every project), + not just this one. Write it to the global rulebook instead/as well? (yes/ + no)". Only write on explicit "yes" to THIS question. +- **Removals are HITL-only, always**, regardless of scope: only remove a + rule when the human explicitly asks to, with the reasoning recorded in the + rule's own `note` field (or a comment in the calibration run's summary) — + never as an automatic side effect of a calibration pass. + +Each persisted rule gets `confirmed_by` (the human's decision, not the +judge's — a model-proposed rule may never set `confirm: true` on itself, +it may only ask) and `confirmed_on` (today's date) per the rulebook schema +(`rulebook.py`'s `_KNOWN_FIELDS`). + +--- + +### Step 7 — (D) Retest loop + +Re-run Steps 1-6 against the shrunk unmatched pool. Stop when: + +- a round yields **fewer than 2 new persisted rules**, OR +- the unmatched pool shrank by **less than 10%** since the previous round, +- **hard cap: 3 rounds**, regardless of shrink rate. + +Track round count and the unmatched-pool size at the start of each round in +the scratch dir (`$SCRATCH/round_N_unmatched_count.txt`) to compute shrink %. + +--- + +## §6 (design.md) — draft convention adoption, never apply unasked + +While reviewing the unmatched pool, `:calibrate` MAY notice a pattern that +would benefit from a `conventions.json` convention (`archive-bucket` or +`status-frontmatter`) rather than a plain rule. If so, it MAY draft the +adoption — the graduated rule (e.g. `served_when_path: /archive/{name}`) +PLUS the concrete file moves or frontmatter additions the convention +implies — and present it to the human alongside the Step 5 rule report, for +approval. **It never applies a drafted adoption without explicit +confirmation** — no rulebook write and no file move happens until the human +confirms. + +--- + +## Calibration pass #1 (cc-os) — special-case reminders + +See `lifecycle-spec.md` §9 and openspec task group 6 for the full protocol. +When run is explicitly cc-os pass #1: + +- Withhold the #41 seed candidates from judge intake (Step 4) — do NOT paste + them into the judge prompt. +- The **protected set is a hard gate**: if ANY rule proposed for persistence + (Step 5/6) has a glob matching a path in the protected set (eval + `scenarios/`/`scenarios-reserve/`/`fixture/`/`judge-rubric.md`; + `openspec/specs/`; `docs/adr/**`; mirrored `.claude/`/`.codex/`/`.pi/` + skill dirs; `CLAUDE.md`; plugin source), REFUSE to persist that rule and + flag it loudly — regardless of the tier the judge assigned it. A `consult` + verdict touching a protected path during exploration is free (does not + fail the pass) as long as it is never persisted. +- Do not treat this carve-out as a permanent behavior — every later run + (including future cc-os runs) uses full seed intake. + +--- + +## Invariants + +- Steps 1, 2, 5, 6 are deterministic scripts/logic — **no model**. +- Step 3 = **haiku** (cheap, per-cluster nomination, patterns only). +- Step 4 = **ONE batched Opus/Fable** judge call, never per-cluster. +- **No rule is persisted before the Step 5 report has been shown to the + human** (hard invariant — never skip Step 5, never merge it with Step 6). +- **Global-rulebook writes require a SEPARATE explicit confirmation** beyond + project-rule approval. +- **Removals are HITL-only in all cases**, with recorded reasoning. +- **A model-proposed rule may never self-set `confirm: true`** — only the + human's Step 5/6 response does. +- Retest loop stops at <2 new rules OR <10% shrink; hard cap 3 rounds. +- **LOOP GUARD:** the nominate subagent prompt MUST point to + `workflows/nominate.md`; the judge subagent prompt MUST point to + `workflows/judge.md`. Neither ever points to this SKILL.md. +- **SUBAGENT AUTHORIZATION:** both subagents are executors — authorization is + terminal. Neither re-asks for approval; if either objects, REPORT-AND-EXIT + and let the orchestrator (this skill, or ultimately the human at Step 5/6) + adjudicate. diff --git a/plugins/os-doc-hygiene/skills/calibrate/workflows/judge.md b/plugins/os-doc-hygiene/skills/calibrate/workflows/judge.md new file mode 100644 index 0000000..8b310b7 --- /dev/null +++ b/plugins/os-doc-hygiene/skills/calibrate/workflows/judge.md @@ -0,0 +1,127 @@ +# Workflow: Judge lifecycle rule nominations (strong model, batched) + +You are the ONE batched strong-model judge for the doc-hygiene `calibrate` +skill. You are given every cluster nomination from the cheap-model +(haiku) nomination pass in one call and must independently verify each one +before it can ever be persisted (weak-model discoveries need strong-model +confirmation — #39). You do NOT trust a nomination's claims — you gather +your OWN evidence. + +> Do NOT read or follow the parent `SKILL.md`. This workflow is self-contained. + +--- + +## Input + +- **Project root.** +- **Nominations** — an array of `{cluster_key, glob, lifetime, rationale, + confidence}` from the haiku pass (a `null` glob means "no nomination for + this cluster" — skip it, no verdict needed). +- **Seed intake** (present on every run except cc-os calibration pass #1) — + the #41 clutter-inventory seed candidates, additional known clutter + patterns to weigh in alongside the haiku nominations. + +--- + +## Your job: gather your own evidence, then verdict + +For EACH nomination (and each seed candidate, when present): + +1. **Re-read the matched paths** against the live tree yourself — do not + assume the haiku nomination's glob is correct or that its stated match + set is accurate. +2. **Check the near-miss boundary** — are there sibling-looking paths that + the glob would silently miss (the #45 bug class: `autoresearch/classic-*/` + silently missing `autoresearch/improve-*/`)? If the glob is too narrow, + consider `amend`ing it to a broader (but still class-not-path-safe) glob + that catches the sibling too. +3. **Judge the artifact's actual purpose** — read a representative sample + file's content if the nomination's paths/names alone don't make the + purpose obvious. Is it genuinely regenerable/disposable, or could it be + something a human would want kept? +4. **Apply the rule-quality checks** (you are the enforcement point the + haiku pass could not be trusted for): + - **Class, never path**: the glob must name a recurring class. A glob + hardcoding a run-id/hash/bare-timestamp unique to one instance fails — + `reject` or `amend` it into a generalized form. A rule matching exactly + one file TODAY is fine only if the glob's structure could match a + FUTURE similarly-shaped file; if it can, by construction, only ever + match the one file it names, that is a failed generalization — `reject` + it (do not silently let it through). + - **Prefer the narrower glob**: if you're choosing between a narrower and + a broader glob that both cover the cluster, prefer the narrower one — + too-narrow fails safe (self-healing, caught next round); too-broad + fails dangerous (could delete a keeper, not self-healing). + +## Verdicts — exactly one of four + +- **`confirm`** — the nomination (as-is) is sound: correct glob, correct + lifetime, passes the quality checks, purpose is clearly regenerable/ + disposable. +- **`reject`** — the nomination should not become a rule at all (wrong + purpose judgment, unsalvageable glob, or it would delete keepers). +- **`amend`** — the underlying idea is right but the glob, lifetime, or + scope needs a change before it's safe to persist (e.g. widen/narrow the + glob, change `temporary` to `delete-once-served`). Return the amended + rule, not the original. +- **`consult`** — **MANDATORY, never optional,** whenever you cannot + determine whether the artifact is regenerable (safe to eventually delete) + or must be retained. Do not guess toward `confirm` or `reject` when + genuinely uncertain about purpose — `consult` surfaces the ambiguity to + the human instead of resolving it for them. + +--- + +## Output + +Return ONLY a JSON array, no prose, no code fences — one entry per judged +nomination (nominations with a `null` glob need no entry): + +```json +[ + { + "cluster_key": "autoresearch::run-#", + "verdict": "amend", + "rule": { + "glob": "autoresearch/*/", + "lifetime": "temporary", + "retain_recent": 3, + "max_age_days": 3 + }, + "reasoning": "Haiku's glob (autoresearch/run-*/) missed autoresearch/improve-*/ and autoresearch/classic-*/ siblings observed in the live tree; widened to autoresearch/*/ which still excludes non-run files at that level." + }, + { + "cluster_key": "docs::HANDOFF-#", + "verdict": "confirm", + "rule": { + "glob": "HANDOFF-*.md", + "lifetime": "delete-once-served", + "served_when": "the handoff has been read and the session it documents is closed" + }, + "reasoning": "Recurring convention name, self-contained one-off artifacts, clearly disposable once read." + }, + { + "cluster_key": "docs::status-#", + "verdict": "consult", + "rule": null, + "reasoning": "Cannot determine from content or naming whether these status snapshots are meant to be retained as an audit trail or are disposable scratch notes -- purpose is genuinely unclear." + } +] +``` + +- `rule` is `null` for `reject` and `consult` verdicts (nothing to persist). +- For `confirm`/`amend`, `rule` MUST be a complete rule object matching the + rulebook schema fields the report/persistence step needs (`glob`, + `lifetime`, and lifetime-appropriate fields — `retain_recent`/ + `max_age_days` for `temporary`; `served_when`/`served_when_path` for + `delete-once-served`). Do NOT include `confirmed_by`/`confirmed_on` — you + are proposing, not confirming; **you may never set `confirm: true` on a + rule yourself, only the human can.** +- `reasoning` is shown to the human in the Step 5 rule report's "why" field — + write it in plain language, not JSON-schema-speak. + +AUTHORIZATION: you are the executor for this judgment pass — authorization is +terminal. Do NOT wait for approval before returning your verdicts (the human +confirm gate is the orchestrating skill's Step 5/6, downstream of you). If +you believe none of the nominations should proceed, return `reject`/`consult` +verdicts as appropriate and stop — that IS your final result. diff --git a/plugins/os-doc-hygiene/skills/calibrate/workflows/nominate.md b/plugins/os-doc-hygiene/skills/calibrate/workflows/nominate.md new file mode 100644 index 0000000..fad7617 --- /dev/null +++ b/plugins/os-doc-hygiene/skills/calibrate/workflows/nominate.md @@ -0,0 +1,86 @@ +# Workflow: Nominate a lifecycle rule for a cluster (cheap model) + +You are the nomination subagent for the doc-hygiene `calibrate` skill. You are +given ONE cluster of similar unmatched file paths (files the project's +lifecycle rulebook does not currently govern) and must nominate a candidate +rule: a bare glob pattern plus a lifetime. You are a cheap first pass — a +strong-model judge will re-check your work independently before anything is +persisted, so favor a clear, generalizable proposal over hedging. + +> Do NOT read or follow the parent `SKILL.md`. This workflow is self-contained. + +--- + +## Input + +- **Directory prefix** and **shape class** the cluster was grouped by. +- **Total** matching paths in this cluster. +- **Sample paths** — a capped representative sample from the cluster. + +--- + +## Your one job: propose a CLASS, never a PATH + +The single most important constraint: your glob must describe a **recurring +class** of artifact, not one specific instance. + +- **Acceptable:** a glob that generalizes the cluster's shared shape — e.g. + given `autoresearch/run-20260710/`, `autoresearch/run-20260711/`, propose + `autoresearch/run-*/`, not a glob naming one specific run. +- **Acceptable:** a glob that hardcodes a name recurring *by convention* + across the sample (e.g. all samples are named `HANDOFF-.md` → + `HANDOFF-*.md` is fine; a bare `PRD.md` appearing at the same relative + location across the sample is fine as `**/PRD.md` or similar). +- **NOT acceptable:** hardcoding a run-id, hash, or bare timestamp unique to + ONE of the sample paths into the glob (e.g. don't propose + `autoresearch/run-20260710/` — that only ever matches that one run). + +If the sample paths don't share an obvious generalizable pattern, it is +legitimate to decline (see "no proposal" below) rather than force one. + +--- + +## Choosing a lifetime + +Pick the lifetime that best fits what this class of artifact IS, based on +the paths and any content you can infer from names/locations: + +- `"temporary"` — self-healing, exists for a bounded window (logs, run + outputs, scratch artifacts); a `retain_recent`/`max_age_days` policy makes + sense. +- `"delete-once-served"` — exists until some condition is met, then should + go (a plan doc that should be archived once shipped, a checklist that + should go once fully checked off). +- `"keep"` — actually should NOT be deleted; if every sample looks like this, + decline to nominate (see below) rather than proposing a rule that would + delete keepers. + +--- + +## Output + +Return ONLY a JSON object, no prose, no code fences: + +```json +{ + "glob": "autoresearch/run-*/", + "lifetime": "temporary", + "rationale": "One-off autoresearch run directories; self-healing, safe to age out.", + "confidence": "high" +} +``` + +If you cannot responsibly generalize this cluster (the samples don't share a +clean pattern, or they look like keepers, not clutter), return instead: + +```json +{ + "glob": null, + "lifetime": null, + "rationale": "Samples do not share a generalizable naming/location pattern, or look like content that should be kept.", + "confidence": "low" +} +``` + +A `null` glob is a legitimate, expected outcome — the judge step treats it as +"no nomination for this cluster," not an error. diff --git a/plugins/os-doc-hygiene/skills/check/SKILL.md b/plugins/os-doc-hygiene/skills/check/SKILL.md index 8cafdc5..7d75ec9 100644 --- a/plugins/os-doc-hygiene/skills/check/SKILL.md +++ b/plugins/os-doc-hygiene/skills/check/SKILL.md @@ -4,10 +4,29 @@ description: Scan the project for stale and bloated documentation and write a hy # Hygiene Check Skill -Orchestrates one documentation-hygiene check: **scan → classify → finalize → -validate → write → stamp**. The scan, finalize, validation, write, and stamp are -deterministic scripts (invariant #6 — no model). Only the per-file -classification is a model step, dispatched to a **Sonnet** subagent. +Orchestrates one documentation-hygiene check: **load rulebook → scan → classify +→ finalize → validate → write → stamp**. The rulebook load, scan, finalize, +validation, write, and stamp are deterministic scripts (invariant #6 — no +model). Only the per-file classification is a model step, dispatched to a +**Sonnet** subagent. + +**Lifecycle awareness (ADR-0039/-0041):** the scanner now consumes the +lifecycle rulebook (global `${CLAUDE_PLUGIN_ROOT}/rulebook.json` plus any +project `.dochygiene-rules.json` override, per `rulebook.py`) while it walks. +A directory-rule match prunes the walk and emits one aggregate shortlist entry +for that directory; a file-rule match attaches a `lifecycle` signal +(`rule_ref`, `lifetime`, `served`/`served_when`/`served_when_path`, age/tier +fields) to that file's existing signals. Lifecycle signals flow into the +classification subagent as a **new signal class alongside stale/bloat** — the +classifier judges free-text `served_when` conditions and MAY propose `delete` +or `extract-then-delete` ops (with an `extraction_dest` classification: +`repo-durable` vs `cross-repo`). The classifier **never authors** `git_state` +or `safety_tier` for a lifecycle entry — same as every other guardrail field, +those stay deterministic, owned by `report_builder.py`/`validate_report.py`. +The finalize pass also computes the report's `promotion_candidates` section +(deterministic, from `conventions.json` — no model call) for every +classifier-judged lifecycle entry with an applicable, not-yet-adopted +convention (`archive-bucket`, `status-frontmatter`). All scripts live under `${CLAUDE_PLUGIN_ROOT}/scripts/`. Run them with `python3` from the user's project directory (`cwd`), which is where the project root is @@ -77,18 +96,63 @@ Three cases: > **Do NOT append without explicit user confirmation.** (Invariant #3.) -### Step 1 — (D) Scan +### Step 0.5 — (D) Load the lifecycle rulebook, then scan -Run the scanner, capturing its stdout artifact to the scratch dir: +The plain `scanner.py` CLI does not wire up rulebook consumption on its own +(`Scanner(rulebook=...)` is an optional constructor argument) — load the +rulebook and construct the scanner in one `python3 -c`, mirroring the +`state_store` invocation pattern used at Steps 6/7: ```bash -python3 "${CLAUDE_PLUGIN_ROOT}/scripts/scanner.py" [--globs ...] > "$SCRATCH/scan.json" +export SCRATCH +python3 -c ' +import json, os, sys +from pathlib import Path +sys.path.insert(0, os.environ["CLAUDE_PLUGIN_ROOT"] + "/scripts") +from rulebook import load_rulebook, RulebookLoadError +from scanner import Scanner, _resolve_project_root, _git_log_real, _git_commit_time_real + +root = _resolve_project_root(Path.cwd()) +project_rules = root / ".dochygiene-rules.json" +try: + rulebook = load_rulebook(project_path=project_rules if project_rules.is_file() else None) +except RulebookLoadError as exc: + print(json.dumps({"error": "rulebook-load-failed", "detail": str(exc)})) + sys.exit(2) + +scanner = Scanner( + root=root, + rulebook=rulebook, + git_log_fn=_git_log_real, + git_commit_time_fn=_git_commit_time_real, +) +artifact = scanner.run() +Path(os.environ["SCRATCH"] + "/scan.json").write_text(json.dumps(artifact, indent=2)) +print("scan written") +' ``` -- Omit `--globs` when there is no `--scope`. +- Exit `2` / `{"error": "rulebook-load-failed", ...}` — **hard failure, per + spec**: unparseable JSON or an unknown `schema_version` in either rulebook + file. STOP here and report the rulebook error to the user before running + the scanner at all — do NOT proceed with lifecycle signals silently + disabled. +- Per-rule warnings (an invalid rule, or one missing `confirmed_by`) are + skip-and-warn, not a hard failure — `load_rulebook` already drops those + rules; nothing further to do here. +- If `--scope` maps to explicit `--globs`, pass `scope_globs=[...]` to the + `Scanner(...)` constructor above instead of the CLI's `--globs` flag (same + scope semantics as before — just via the constructor since this step + no longer shells out to the bare CLI). - The scanner auto-resolves the project root from `cwd` and applies default excludes (incl. `.cc-os/` and legacy `.dochygiene/`). Do not pass `--root`. +### Step 1 — (D) Scan + +The scan already ran as part of Step 0.5 (rulebook load and scan are one +script invocation so the rulebook is never stale relative to the walk). The +artifact is at `"$SCRATCH/scan.json"`; proceed to Step 2. + The artifact is `{ project_root, scope_globs, excluded_dirs, files_scanned, shortlist, signals }`. `signals` is an object keyed by project-root-relative path: `{ "": [ { "name": "", "detail": "" }, ... ] }`. @@ -271,7 +335,12 @@ Run /os-doc-hygiene:clean to act on these (Phase 4), or /os-doc-hygiene:status f The human report header renders `scope_globs` but has no category field (the frozen `report_builder.py` does not take one), so surface the active `--category` -here in the skill output rather than expecting it in the report. +here in the skill output rather than expecting it in the report. The human +report itself now always includes a **Promotion Candidates** section (a +`## Promotion Candidates` heading, `(none)` when empty), and any entry +carrying lifecycle evidence shows a `lifecycle: rule=... · lifetime=... · +served_when(_path)=...` line beneath it — both rendered deterministically by +`report_builder.py`, not authored by the classification subagent. ## Closed enums (for reference — the subagent enforces them) @@ -281,17 +350,34 @@ here in the skill output rather than expecting it in the report. - bloat `subtype` ∈ { `distill`, `split`, `freeze` } - `op_type` ∈ { `deterministic`, `generative` } - `exact_edit.kind` ∈ { `delete-range`, `move-to-archive`, `insert-frontmatter`, - `replace-text`, `dedupe` } + `replace-text`, `dedupe`, `delete`, `extract-then-delete` } — the last two + are lifecycle ops (ADR-0039): the classifier may propose them for a + candidate carrying a lifecycle signal, additionally supplying an + `extraction_dest` (`repo-durable` \| `cross-repo`) for + `extract-then-delete`. The classifier NEVER authors `git_state` or + `safety_tier` for these — `report_builder.py`/`validate_report.py` derive + them per the lifecycle tier matrix (scanner-proven + tracked+clean ⇒ auto; + everything else, including any classifier-judged `served_when`, ⇒ confirm). ## Invariants - Step 0 check is deterministic (`git check-ignore`); the offer/confirm is a user gate (M-GATE). The append is deterministic and runs only on explicit confirmation. -- Steps 1, 2, 4, 5, 6, 7 are deterministic scripts — **no model** (invariant #6). +- Step 0.5 (rulebook load + scan), Steps 2, 4, 5, 6, 7 are deterministic + scripts — **no model** (invariant #6). A rulebook load failure is a hard + failure (exit 2) — the check stops before scanning, it never proceeds with + lifecycle signals silently disabled. - Classification = **Sonnet**; single-file Opus escalation only on low confidence for hard distinctions. - The subagent supplies judgment only. It never authors `expected_sha256`, - `safety_tier`, `is_destructive`, `is_reversible`, or `raw_tokens` — those are - owned by `report_builder.py`. + `safety_tier`, `is_destructive`, `is_reversible`, `raw_tokens`, or (for + lifecycle entries) `git_state` — those are owned by `report_builder.py`. For + a lifecycle-signal candidate the subagent MAY judge the free-text + `served_when` condition and propose `delete`/`extract-then-delete` plus + `extraction_dest`, but the resulting `safety_tier` is still computed + downstream, never asserted by the subagent. +- `promotion_candidates` is computed by `report_builder.py` from + `conventions.json` with **no model call** — it is not something the + classification subagent produces or is asked about. - **Validate on a scratch path BEFORE `write_report`** (write_report is destructive-first; invariant #4). Never write an invalid report. - `last_check` = the validated report's envelope `generated_at` (same run diff --git a/plugins/os-doc-hygiene/skills/check/workflows/classify-candidates.md b/plugins/os-doc-hygiene/skills/check/workflows/classify-candidates.md index 6634844..48e231e 100644 --- a/plugins/os-doc-hygiene/skills/check/workflows/classify-candidates.md +++ b/plugins/os-doc-hygiene/skills/check/workflows/classify-candidates.md @@ -16,7 +16,51 @@ You are given, for each candidate: - `signals` — the scanner's objective signals for that path, as a JSON array of `{ "name": ..., "detail": ... }`. Signal `name`s are drawn from: `broken_reference`, `version_skew`, `edit_recency_vs_churn`, - `stale_name_location`, `archive_to_live_ratio`, `frontmatter_marker`. + `stale_name_location`, `archive_to_live_ratio`, `frontmatter_marker`, and a + new class — `lifecycle` — carrying `{ rule_ref, lifetime, extract, + served: {kind, ...}, served_when / served_when_path, age_days, + retain_recent, max_age_days, retention: {rank, kept, deletable} }`. A + `lifecycle` signal means the path matched a rulebook rule (see + `rulebook.py` / `lifecycle-spec.md`). + +## Lifecycle candidates — a fourth op family (delete / extract-then-delete) + +A candidate carrying a `lifecycle` signal is judged differently from a +stale/bloat candidate: + +- If `served.kind` is `"scanner-proven"` (a `served_when_path` hit, or a + temporary-tier age/retain-recent computation already resolved the + deletion question deterministically), the deletion decision is **already + made** — you still classify the file (so the entry exists in the report), + but you MUST NOT re-litigate whether it should be deleted. Propose `op_type: + "deterministic"`, `exact_edit.kind: "delete"` (or, for a directory-rule + aggregate entry, `"delete"` with the directory path — the finalize pass + sets `is_directory` from the scan artifact, not from you). +- If `served.kind` is `"classifier-judged"` (the rule's `served_when` is + free text — e.g. "the effort this plan describes has shipped" — and no + filesystem-provable `served_when_path` exists), **you** are the judgment: + read the file and any evidence available and decide whether the free-text + condition currently holds. If it does, propose `exact_edit.kind: "delete"` + or `"extract-then-delete"` (see below). If it does not (or you cannot + tell), do not propose a lifecycle op for this candidate — emit no + proposal, or a stale/bloat proposal instead if independently warranted. +- **`extract-then-delete`** is for content worth preserving elsewhere before + the source is deleted (the rule's `extract` field, if present, hints this). + Supply `exact_edit.extraction_dest`: `"repo-durable"` (content belongs in + an ADR/CLAUDE.md/docs file in this repo) or `"cross-repo"` (content + belongs in the SecondBrain vault, written via `/os-vault:write` at clean + time). For `repo-durable`, also name a target doc reference if you can + infer one (e.g. `docs/adr/` or a specific `CLAUDE.md` section) — the clean + skill's generative extraction step uses this as a starting point, not a + binding contract. + +**You NEVER author `git_state` or `safety_tier` on a lifecycle proposal** — +same rule as every other guardrail field. Your job is judgment content only: +whether `served_when` currently holds, which `exact_edit.kind` fits, and (for +`extract-then-delete`) the extraction destination classification. The +deterministic finalize pass derives the tier from your `served`/`kind` +evidence plus a live git-state check — a `classifier-judged` verdict can +never resolve to `auto`, regardless of how confident you are. You are also given the project root. **Read each candidate file** (root + path) before classifying it. Your judgment must be grounded in the file's actual @@ -79,6 +123,8 @@ Supply `exact_edit` with `kind` plus exactly the kind-specific fields below. | `replace-text` | `anchor: { start_line, end_line }`, `match`, `replacement` | known-target fix, e.g. a link/path; `match` is the exact text to replace within the anchor | | `dedupe` | `anchor: { start_line, end_line }`, `canonical_ref` | exact duplicate preserved elsewhere; `canonical_ref` points to the surviving canonical copy | | `insert-frontmatter` | `key`, `value` | freeze a doc; **no anchor** (e.g. `key: "hygiene"`, `value: "frozen"`) | +| `delete` | **no anchor for directory-rule aggregate entries; full-file anchor otherwise** | lifecycle-rule deletion (a `lifecycle` signal was present); do NOT supply `git_state` or `safety_tier` | +| `extract-then-delete` | same anchor rule as `delete`, plus `extraction_dest: "repo-durable" \| "cross-repo"` | lifecycle deletion where content is worth preserving first; see the lifecycle section above | ## If `op_type` is `generative` → include `reducible_range`, NO `exact_edit` diff --git a/plugins/os-doc-hygiene/skills/clean/SKILL.md b/plugins/os-doc-hygiene/skills/clean/SKILL.md index 9f4df01..05d8fd0 100644 --- a/plugins/os-doc-hygiene/skills/clean/SKILL.md +++ b/plugins/os-doc-hygiene/skills/clean/SKILL.md @@ -5,10 +5,26 @@ description: Apply documented hygiene findings to project docs. Loads the curren # Hygiene Clean Skill Orchestrates one documentation-hygiene cleanup run: **load → validate → filter → -gate → preflight → apply → commit → stamp**. The load, validate, filter, +gate → preflight → extract → apply → commit → stamp**. The load, validate, filter, partition, git ops, stage, commit, and stamp are deterministic (invariant #6 — no -model). Only generative distillation is a model step, dispatched to a **Sonnet** -subagent per the LOOP-GUARD pattern. +model). Only generative distillation and lifecycle extraction are model steps, +dispatched to a **Sonnet** subagent (or `/os-vault:write` for cross-repo +extraction) per the LOOP-GUARD pattern. + +**Lifecycle ops (ADR-0039):** `delete` and `extract-then-delete` are two more +`op_type: "deterministic"` kinds alongside the existing five. They partition +into the SAME `(safety_tier, op_type)` buckets as every other deterministic +op (Step 4) — `auto` applies without prompt, `confirm` escalates — **but their +tier was derived with lifecycle-aware rules** (`validate_report.py`'s +`derive_safety_tier`): `auto` only when the lifecycle evidence was +`scanner-proven` (a `served_when_path` hit or a temporary-tier age/ +retain-recent computation) **and** the file was tracked+clean at report-build +time. A `classifier-judged` (`served_when` free text) verdict, or any +dirty/untracked file, is `confirm` — no exception. Because time passes +between `check` and `clean`, `patch_applier.py` **re-verifies git state at +apply time** for every `auto`-tier lifecycle entry — even one applied +silently here may be downgraded to skip (`git-state-changed-since-check`) if +the path is no longer tracked+clean. All scripts live under `${CLAUDE_PLUGIN_ROOT}/scripts/`. Run them from the user's project directory (`cwd`). @@ -193,12 +209,15 @@ Hygiene clean — confirm required before any changes are made: The following entries require your approval. Auto-tier entries will be applied silently after you respond. - ⚠ IRREVERSIBLE DELETES (delete-range — content will be permanently removed): + ⚠ IRREVERSIBLE DELETES (delete-range/delete/extract-then-delete — content + will be permanently removed or removed after extraction): [1] docs/stale-notes.md (orphaned / delete-range · confirm) — ~120 tokens "Orphaned after 2025 refactor; no references remain." + [2] docs/plans/old-plan.md (delete-once-served / delete · confirm) — ~80 tokens + "served_when: classifier-judged — always confirm regardless of age/rule." Reversible confirm entries: - [2] CHANGELOG.md (distill / generative-distill · confirm) — ~400 tokens + [3] CHANGELOG.md (distill / generative-distill · confirm) — ~400 tokens "Changelog is true but bloated; condense into summary." Enter the numbers to approve (e.g. "1,2"), "all", or "none" / leave blank to skip all: @@ -207,10 +226,17 @@ Enter the numbers to approve (e.g. "1,2"), "all", or "none" / leave blank to ski Rules for display: - List every `confirm_det` and `confirm_gen` entry (not `auto_det` — those apply silently). -- Visually distinguish `delete-range` entries (mark `⚠ IRREVERSIBLE`); group - them under a warning header. +- Visually distinguish `delete-range`, `delete`, and `extract-then-delete` + entries (mark `⚠ IRREVERSIBLE`); group them under a warning header — + `delete`/`extract-then-delete` land here at `confirm` tier precisely + because either the lifecycle evidence was classifier-judged (a + `served_when` verdict is NEVER auto, no matter how confident) or the file + was dirty/untracked at report-build time. A `scanner-proven` + + tracked+clean lifecycle delete is `auto` and is never shown here at all. - Show: path, `category.subtype`, `op_type`/`kind`, `token_estimate.raw_tokens`, - and `gloss` (or `op` if `gloss` is absent). + and `gloss` (or `op` if `gloss` is absent). For lifecycle entries, surface + the `lifecycle.served_when` or `served_when_path` text so the human can see + what was (or wasn't) proven. - Per-entry opt-out: the user may approve a subset by number, "all", or "none". After the user responds: @@ -290,6 +316,78 @@ Report them at Step 11 with reason "untracked — add the file to git first." --- +### Step 6.5 — (M) Lifecycle extraction — orchestrated BEFORE the applier runs + +Skip this step entirely if no `approved_det` entry has +`exact_edit.kind == "extract-then-delete"`. + +`patch_applier.py` never performs extraction itself — it only checks +`entry.extraction_complete` and fails closed (skip, +`extraction-not-confirmed`) when it is not `true` (design.md Decision 3). +This skill is the one that runs extraction, and it MUST do so before +invoking the applier for these entries, so the source deletion and the +extraction land in the **same single hygiene commit**. + +For each `approved_det` entry with `exact_edit.kind == +"extract-then-delete"` (in order): + +1. **Confirm the file still exists** (a preceding op in this run may have + already moved/deleted it). If gone, record as skipped + (`source-already-gone`) and continue to the next entry — do not attempt + extraction against a missing file. +2. **Live-read the file contents now** (same freshness rule as Step 8's + generative path — never trust the report's cached text). +3. **Branch on `exact_edit.extraction_dest`:** + - **`"repo-durable"`** — dispatch the SAME generative Sonnet-subagent + path `doc-clean` already uses (LOOP-GUARD: point it at + `skills/clean/workflows/extract.md`, NEVER this SKILL.md). Give it the + live file contents, the entry's `op`/`gloss`/lifecycle fields, and any + target doc hint the classifier supplied. It returns one of + `{"status": "extracted", "target_path", "target_action", "content"}`, + `{"status": "nothing-to-extract", "reason"}`, or + `{"status": "error", "reason"}`. For `"extracted"`, write/append + `content` to `target_path` per `target_action` (an ADR file under + `docs/adr/`, `CLAUDE.md`, or a `docs/` file — the model's proposal, not + a fixed contract) and `git add -- "$PROJECT_ROOT/"` + immediately. `"nothing-to-extract"` is treated as extraction success + (the judgment call was made; there's simply nothing durable to write) — + proceed to step 4 below. `"error"` is treated as extraction failure — + proceed to step 5 below. + - **`"cross-repo"`** — invoke `/os-vault:write` (the skill, not the + applier) with the extracted content and enough context (project name, + source path, why it's evergreen) for it to choose vault frontmatter. + `/os-vault:write` owns its own destination — this step supplies content + and context only, per spec §1 ("no new destinations" for cross-repo + extraction). +4. **On extraction success:** record the entry's original report index in + `extraction_confirmed_indices` for Step 7 below. +5. **On extraction failure** (subagent errors, or `/os-vault:write` fails): + do NOT add the index to `extraction_confirmed_indices` — leave the entry + unconfirmed. This is a per-entry fails-closed skip (patch_applier.py will + report `extraction-not-confirmed`), not a hard failure — continue + processing remaining entries. Only an `OSError` on write or a `git add` + failure is a hard failure → rollback per Step 7's Trap E procedure. + +Build the scratch report copy the applier will read, marking confirmed +entries `extraction_complete: true` (the canonical `$REPORT_PATH` on disk is +NEVER mutated by this skill — only this scratch copy): + +```python +import json +from pathlib import Path + +report = json.loads(Path(REPORT_PATH).read_text()) +for idx in extraction_confirmed_indices: + report["entries"][idx]["extraction_complete"] = True + +Path(SCRATCH, "report_with_extraction.json").write_text(json.dumps(report)) +``` + +If this step was skipped (no `extract-then-delete` entries at all), Step 7 +below reads `$REPORT_PATH` directly instead of the scratch copy. + +--- + ### Step 7 — (D) Apply approved deterministic entries via `patch_applier.py` Build the comma-separated index list from `approved_det` original report indices: @@ -298,11 +396,19 @@ Build the comma-separated index list from `approved_det` original report indices det_indices_str = ",".join(str(i) for i, _ in approved_det) ``` +Use `$SCRATCH/report_with_extraction.json` as `--report` if Step 6.5 ran +(any `extraction_complete` markings need to reach the applier); otherwise use +`$REPORT_PATH` unchanged. Report indices are unaffected either way — the +scratch copy carries the same `entries[]` array positions as the original. + If `approved_det` is empty, skip this step. ```bash +APPLIER_REPORT="$REPORT_PATH" +[ -f "$SCRATCH/report_with_extraction.json" ] && APPLIER_REPORT="$SCRATCH/report_with_extraction.json" + python3 "${CLAUDE_PLUGIN_ROOT}/scripts/patch_applier.py" \ - --report "$REPORT_PATH" \ + --report "$APPLIER_REPORT" \ --apply-indices "$DET_INDICES" \ > "$SCRATCH/applier_result.json" APPLIER_EXIT=$? @@ -570,7 +676,10 @@ Run /os-doc-hygiene:check to refresh the report and pick up any remaining issues | Write/git error in Step 8 | Rollback to baseline, abort. | | Applier exit 1, `failed[]` empty (Step 7) | Commit what applied; report skipped with re-analysis note. | | Subagent error in Step 8 | Skip that generative entry; continue. | -| All entries skipped (Steps 7+8) | No commit, no stamp. Report all-skipped. | +| Extraction subagent/`os-vault:write` error in Step 6.5 | Per-entry fails-closed skip — do NOT mark `extraction_complete`; applier reports `extraction-not-confirmed` for that index at Step 7. Continue processing other entries. NOT a hard failure. | +| Write/git-add error during extraction (Step 6.5) | Rollback to baseline, abort (same as Step 8's write-error rule — an `OSError`/git failure is hard, a judgment error is not). | +| `git-state-changed-since-check` skip (Step 7, applier) | Not a hard failure — the applier already downgraded this `auto`-tier lifecycle entry to skip because the path is no longer tracked+clean since `check` ran. Report with re-analysis note (re-run `/os-doc-hygiene:check`). | +| All entries skipped (Steps 6.5+7+8) | No commit, no stamp. Report all-skipped. | | Commit failure (Step 9) | Rollback to baseline, abort. No stamp. | Rollback procedure (used by multiple failure paths): @@ -586,19 +695,31 @@ echo "Rolled back to $(git rev-parse --short HEAD). No commit was created." - **#4** — no new state artifacts; `last_clean` is the only new state entry. - **#5** — exactly one cleanup commit per run (plus an optional WIP checkpoint if the tree was dirty). -- **#6** — only Steps 8 (generative distillation) and 5 (the confirm prompt - rendered to the user) involve a model. All other steps are deterministic - scripts or logic. -- **#7** — confirm gate precedes every mutation, including the git preflight. +- **#6** — only Steps 6.5 (lifecycle extraction), 8 (generative distillation), + and 5 (the confirm prompt rendered to the user) involve a model. All other + steps are deterministic scripts or logic. +- **#7** — confirm gate precedes every mutation, including the git preflight + AND lifecycle extraction (Step 6.5 only ever touches entries already in + `approved_det`, i.e. already past the Step 5 gate). - **#8** — the applier enforces the content-hash guard; the skill trusts - `failed[]` / `skipped[]` in the result. + `failed[]` / `skipped[]` in the result. Lifecycle deletes additionally + re-verify git tracked/clean state at apply time (never the report's cached + `git_state`) — an entry cached as `auto` can still be downgraded to skip at + apply time (ADR-0039). +- **extraction fails closed** — `extract-then-delete` only proceeds to + delete the source once `extraction_complete: true` is set on a per-entry + basis by Step 6.5; a failed or skipped extraction withholds the delete for + that entry only, never blocks the rest of the run. ## LOOP GUARD The generative subagent prompt MUST point to `skills/clean/workflows/distill.md`, NEVER to this SKILL.md (prevents -recursive skill invocation). +recursive skill invocation). The `repo-durable` extraction subagent prompt +MUST point to `skills/clean/workflows/extract.md`, likewise never this +SKILL.md. -**SUBAGENT AUTHORIZATION:** the distill subagent is the executor; it MUST NOT -block waiting for approval. If it objects, REPORT-AND-EXIT — the orchestrator -adjudicates. The confirm gate (Step 5) never lives inside the subagent. +**SUBAGENT AUTHORIZATION:** the distill and extract subagents are the +executor; they MUST NOT block waiting for approval. If either objects, +REPORT-AND-EXIT — the orchestrator adjudicates. The confirm gate (Step 5) +never lives inside a subagent. diff --git a/plugins/os-doc-hygiene/skills/clean/workflows/extract.md b/plugins/os-doc-hygiene/skills/clean/workflows/extract.md new file mode 100644 index 0000000..7a5ee01 --- /dev/null +++ b/plugins/os-doc-hygiene/skills/clean/workflows/extract.md @@ -0,0 +1,113 @@ +# Workflow: Extract doc-hygiene lifecycle entry (repo-durable) + +You are the extraction subagent for the doc-hygiene `clean` skill's +`extract-then-delete` lifecycle op (ADR-0039), for the **`repo-durable`** +destination class only (`cross-repo` extraction is handled by `/os-vault:write` +directly, not by this workflow). You preserve content worth keeping from a +file that is about to be deleted, by writing it into a durable in-repo target +(an ADR, `CLAUDE.md`, or a `docs/` file). You work entirely from the **live +file contents provided to you** — you do not read files from disk and you do +not write files to disk yourself. The skill that dispatched you handles the +actual write and git staging, based on the structured result you return. + +> Do NOT read or follow the parent `SKILL.md`. This workflow is self-contained. +> Do NOT call any file-write or git tools. Return your result as structured text. + +--- + +## Input + +You receive, in your prompt: + +- **File path** — the project-root-relative path of the file about to be + deleted (the source of extraction). +- **Lifecycle info** — `rule_ref`, `lifetime`, and the `served_when` / + `served_when_path` evidence that justified deletion. +- **Op / gloss** — the classifier's judgment about what this file is and why + it's being deleted. +- **Target hint** (optional) — a target doc reference the classifier + proposed (e.g. "docs/adr/" or a specific `CLAUDE.md` section). This is a + starting point, not a binding contract — use your judgment about the best + durable home if the hint doesn't fit. +- **Live file contents** — the full current text of the file, provided + inline. Do NOT re-read it from disk. + +--- + +## What "extract" means here + +The source file is being deleted because its lifecycle has ended (e.g. a +completed plan, a served one-off artifact). Before it goes, decide whether +anything in it is worth a permanent, durable trace in the repo: + +- A **decision** that was made → belongs in a new or amended ADR + (`docs/adr/`) if the project has an ADR system; follow that project's ADR + conventions if visible in the live contents or target hint. +- A **standing fact or convention** future contributors need → belongs in + `CLAUDE.md` or the most relevant `docs/` file. +- **Ephemeral working detail with no lasting value** (a status update, a + scratch note, a checklist that's now fully checked off) → there may be + nothing worth extracting. It is legitimate to conclude "nothing durable + here" — do not manufacture content to justify the op. + +Be conservative about volume: extract the load-bearing decision or fact, not +the whole file. A one-paragraph durable summary beats copy-pasting the +source verbatim into the target. + +--- + +## Output + +Return ONLY a JSON object, no prose, no code fences: + +**If there is durable content to preserve:** + +```json +{ + "status": "extracted", + "target_path": "docs/adr/0042-example-decision.md", + "target_action": "create", + "content": "" +} +``` + +- `target_path` — project-root-relative path of the durable target. Use your + judgment (informed by the target hint) — a new ADR file, an existing + `CLAUDE.md`, or an existing/new `docs/` file. +- `target_action` — `"create"` (target does not exist; skill creates it with + exactly `content`) or `"append"` (target exists; skill appends `content` to + it, e.g. adding a new ADR-numbered entry is still "create" for a fresh ADR + file, but adding one paragraph to an existing `CLAUDE.md` is "append"). +- `content` — the durable markdown to write/append. Self-contained (correct + heading level, no dangling references to the file about to be deleted). + +**If nothing is worth extracting:** + +```json +{ + "status": "nothing-to-extract", + "reason": "The file is a fully-served checklist with no standing decision or convention left to preserve." +} +``` + +This is a legitimate, successful outcome — the skill treats it the same as +`"extracted"` for the purpose of proceeding to delete the source (extraction +"succeeded" in the sense that the judgment call was made; there is simply +nothing to write). Only use `{"status": "error", ...}` (see below) when you +cannot make the judgment call at all. + +**On error / unable to proceed:** + +```json +{ + "status": "error", + "reason": "" +} +``` + +An error result is a per-entry fails-closed skip upstream (the delete does +NOT proceed for this entry) — it is never a hard failure. AUTHORIZATION: you +are the executor — the human confirm gate ran upstream in the orchestrating +skill; do NOT re-ask for approval or wait for confirmation that cannot +arrive. If you believe you should not proceed, return your objection via the +`"error"` status and stop immediately (REPORT-AND-EXIT). diff --git a/plugins/os-doc-hygiene/tests/test_calibrate_helpers.py b/plugins/os-doc-hygiene/tests/test_calibrate_helpers.py new file mode 100644 index 0000000..c7136c0 --- /dev/null +++ b/plugins/os-doc-hygiene/tests/test_calibrate_helpers.py @@ -0,0 +1,262 @@ +""" +Tests for scripts/calibrate_helpers.py + +Covers task 5.4 of the lifecycle-aware-doc-hygiene change: +- ClusterSampler: group unmatched paths by path-shape, capped sample per + cluster +- RuleReportBuilder: 5-element rule-report data (glob verbatim, matched + sample+total, near-miss boundary, lifetime+tier, why) +- RuleQualityChecker: class-not-path lint (instance-unique identifiers / + no-wildcard-single-match) and prefer-the-narrower-glob tie-break + +sys.path is patched here (no shared conftest), matching the existing +tests/test_rulebook.py pattern. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +_SCRIPTS_DIR = Path(__file__).parent.parent / "scripts" +if str(_SCRIPTS_DIR) not in sys.path: + sys.path.insert(0, str(_SCRIPTS_DIR)) + +from calibrate_helpers import ( # noqa: E402 + ClusterSampler, + RuleQualityChecker, + RuleReportBuilder, +) + + +# --------------------------------------------------------------------------- +# ClusterSampler +# --------------------------------------------------------------------------- + + +class TestClusterSampler: + def test_groups_by_directory_and_filename_shape(self): + paths = [ + "autoresearch/run-12345678/notes.md", + "autoresearch/run-87654321/notes.md", + "autoresearch/run-11112222/notes.md", + "docs/HANDOFF-2026-07-01.md", + "docs/HANDOFF-2026-07-02.md", + ] + sampler = ClusterSampler() + clusters = sampler.cluster(paths) + + # autoresearch/run-# dirs form one cluster (same dir-shape prefix + # pattern doesn't collapse dir segments, but filenames do) — + # here dir segments differ (run-12345678 vs run-87654321), so they + # are distinct directories; but the *filenames* are identical + # ("notes.md") which is itself a valid single-file-per-dir cluster + # test. Assert basic invariants instead of exact grouping count. + assert len(clusters) >= 1 + total_paths = sum(c.total for c in [c for c in clusters]) + # every path is accounted for + seen = set() + for c in clusters: + seen.update(c.paths) + assert seen == set(paths) + + def test_same_dir_and_shape_groups_together(self): + paths = [ + "logs/log-00000001.txt", + "logs/log-00000002.txt", + "logs/log-00000003.txt", + ] + sampler = ClusterSampler() + clusters = sampler.cluster(paths) + assert len(clusters) == 1 + assert clusters[0].total == 3 + assert set(clusters[0].paths) == set(paths) + + def test_sample_is_capped(self): + paths = [f"logs/log-{i:08d}.txt" for i in range(20)] + sampler = ClusterSampler(sample_cap=5) + clusters = sampler.cluster(paths) + assert len(clusters) == 1 + assert clusters[0].total == 20 + assert len(clusters[0].sample) == 5 + + def test_different_directories_are_different_clusters(self): + paths = ["a/file.md", "b/file.md"] + sampler = ClusterSampler() + clusters = sampler.cluster(paths) + assert len(clusters) == 2 + + def test_hex_run_collapses_to_shape_class(self): + paths = [ + "cache/deadbeefcafe.json", + "cache/0123456789ab.json", + ] + sampler = ClusterSampler() + clusters = sampler.cluster(paths) + # both filenames collapse to the same hex-shape class -> one cluster + assert len(clusters) == 1 + assert clusters[0].total == 2 + + def test_to_dict_is_json_serializable(self): + import json + + paths = ["docs/PRD.md"] + sampler = ClusterSampler() + dicts = sampler.cluster_to_dicts(paths) + json.dumps(dicts) # must not raise + assert dicts[0]["total"] == 1 + assert dicts[0]["sample"] == ["docs/PRD.md"] + + def test_empty_input_yields_no_clusters(self): + sampler = ClusterSampler() + assert sampler.cluster([]) == [] + + +# --------------------------------------------------------------------------- +# RuleReportBuilder +# --------------------------------------------------------------------------- + + +class TestRuleReportBuilder: + def test_glob_is_verbatim(self): + all_paths = ["docs/HANDOFF-1.md", "docs/HANDOFF-2.md"] + rule = {"glob": "docs/HANDOFF-*.md", "lifetime": "temporary", "tier": "auto"} + entry = RuleReportBuilder().build(rule, all_paths) + assert entry.glob == "docs/HANDOFF-*.md" + + def test_matches_sample_and_total(self): + all_paths = [f"docs/HANDOFF-{i}.md" for i in range(10)] + rule = {"glob": "docs/HANDOFF-*.md", "lifetime": "temporary", "tier": "auto"} + entry = RuleReportBuilder(sample_cap=3).build(rule, all_paths) + assert entry.matched_total == 10 + assert len(entry.matched_sample) == 3 + + def test_near_miss_boundary_detects_sibling_paths(self): + # A too-narrow glob silently misses a sibling path under the same + # parent directory — this is exactly the #45 boundary bug + # (autoresearch/classic-*/ missing autoresearch/improve-*/). + all_paths = [ + "autoresearch/classic-260710-1057/report.md", + "autoresearch/improve-260710-1057/report.md", + ] + rule = {"glob": "autoresearch/classic-*/report.md", "lifetime": "temporary", "tier": "confirm"} + entry = RuleReportBuilder().build(rule, all_paths) + assert entry.matched_total == 1 + assert "autoresearch/improve-260710-1057/report.md" in entry.near_miss + + def test_no_near_miss_when_glob_covers_all_similar_paths(self): + all_paths = [ + "autoresearch/classic-1/report.md", + "autoresearch/classic-2/report.md", + ] + rule = {"glob": "autoresearch/*/report.md", "lifetime": "temporary", "tier": "confirm"} + entry = RuleReportBuilder().build(rule, all_paths) + assert entry.matched_total == 2 + assert entry.near_miss == [] + + def test_lifetime_and_tier_carried_through(self): + rule = {"glob": "x/*.md", "lifetime": "delete-once-served", "tier": "confirm"} + entry = RuleReportBuilder().build(rule, ["x/a.md"]) + assert entry.lifetime == "delete-once-served" + assert entry.tier == "confirm" + + def test_build_all_is_json_serializable(self): + import json + + rules = [ + {"glob": "docs/HANDOFF-*.md", "lifetime": "temporary", "tier": "auto"}, + ] + dicts = RuleReportBuilder().build_all(rules, ["docs/HANDOFF-1.md"]) + json.dumps(dicts) + assert dicts[0]["matches"]["total"] == 1 + + +# --------------------------------------------------------------------------- +# RuleQualityChecker +# --------------------------------------------------------------------------- + + +class TestRuleQualityCheckerClassNotPath: + def test_convention_recurring_name_passes(self): + checker = RuleQualityChecker() + finding = checker.class_not_path("docs/HANDOFF-*.md") + assert finding.passed is True + + def test_recurring_filename_glob_with_wildcard_passes(self): + # "**/PRD.md" is structurally able to match PRD.md in any directory + # -- the wildcard makes it a class, not a single-instance path, even + # though the literal basename "PRD.md" is itself a fixed name. + checker = RuleQualityChecker() + finding = checker.class_not_path( + "**/PRD.md", all_paths=["docs/PRD.md", "other/PRD.md"] + ) + assert finding.passed is True + + def test_long_digit_run_fails(self): + checker = RuleQualityChecker() + finding = checker.class_not_path("logs/log-20260714153045.txt") + assert finding.passed is False + assert "digit" in finding.reason.lower() + + def test_hex_hash_fails(self): + checker = RuleQualityChecker() + finding = checker.class_not_path("cache/deadbeefcafe1234.bin") + assert finding.passed is False + assert "hash" in finding.reason.lower() or "hex" in finding.reason.lower() + + def test_one_current_match_is_fine_if_glob_has_wildcard(self): + checker = RuleQualityChecker() + finding = checker.class_not_path("docs/HANDOFF-*.md", all_paths=["docs/HANDOFF-1.md"]) + assert finding.passed is True + + def test_wildcard_free_glob_matching_only_one_path_ever_fails(self): + checker = RuleQualityChecker() + finding = checker.class_not_path( + "docs/migration-report.md", all_paths=["docs/migration-report.md"] + ) + assert finding.passed is False + assert "generalization" in finding.reason.lower() + + def test_wildcard_free_glob_with_multiple_matches_passes(self): + # a bare name matching >1 existing path is a recurring convention + # even with no wildcard char (unusual but should not be flagged) + checker = RuleQualityChecker() + finding = checker.class_not_path( + "PRD.md", all_paths=["a/PRD.md"] + ) + # single match with a bare literal glob (no wildcard) still fails, + # since "PRD.md" as an exact-path glob wouldn't even match "a/PRD.md" + # (fnmatch requires the full string) -- so total matches = 0 -> not + # flagged for "matches <=1 existing path" since it's 0 (never seen). + # This asserts the checker doesn't crash and returns a finding. + assert finding.check == "class_not_path" + + +class TestRuleQualityCheckerPreferNarrower: + def test_narrower_glob_by_match_count_wins(self): + checker = RuleQualityChecker() + all_paths = [ + "autoresearch/classic-1/report.md", + "autoresearch/classic-2/report.md", + "autoresearch/improve-1/report.md", + ] + result = checker.prefer_narrower( + "autoresearch/classic-*/report.md", "autoresearch/*/report.md", all_paths + ) + assert result["narrower"] == "autoresearch/classic-*/report.md" + assert result["match_counts"]["autoresearch/classic-*/report.md"] == 2 + assert result["match_counts"]["autoresearch/*/report.md"] == 3 + + def test_tie_break_prefers_longer_pattern(self): + checker = RuleQualityChecker() + all_paths = ["a/b.md"] + result = checker.prefer_narrower("a/*.md", "a/b.md", all_paths) + # both match exactly 1 path -> tie -> longer raw pattern wins + assert result["narrower"] == "a/b.md" + + def test_result_is_json_serializable(self): + import json + + checker = RuleQualityChecker() + result = checker.prefer_narrower("a/*.md", "a/*.txt", ["a/b.md"]) + json.dumps(result) diff --git a/plugins/os-doc-hygiene/tests/test_patch_applier.py b/plugins/os-doc-hygiene/tests/test_patch_applier.py index b02214c..938c103 100644 --- a/plugins/os-doc-hygiene/tests/test_patch_applier.py +++ b/plugins/os-doc-hygiene/tests/test_patch_applier.py @@ -838,3 +838,429 @@ class TestApplyIndexed: result = _applier(tmp_path, fs=fs).apply_indexed([(7, entry)]) assert result["applied"][0]["entry_index"] == 7 + + +# =========================================================================== +# Lifecycle deletion ops: `delete` and `extract-then-delete` (ADR-0039) +# =========================================================================== +# +# These two kinds are NOT drawn from KIND_TABLE (owned by report_builder.py / +# validate_report.py, group 3's file) — they are constructed directly here so +# these tests do not depend on that concurrent work landing first. + +class RecordingGitState: + """Injectable git-state double: fixed tracked/dirty answers per test.""" + + def __init__(self, tracked: bool = True, dirty: bool = False) -> None: + self.tracked = tracked + self.dirty = dirty + self.is_tracked_calls: list[tuple[str, str]] = [] + self.is_dirty_calls: list[tuple[str, str]] = [] + + def is_tracked(self, path: str, cwd: Path) -> bool: + self.is_tracked_calls.append((path, str(cwd))) + return self.tracked + + def is_dirty(self, path: str, cwd: Path) -> bool: + self.is_dirty_calls.append((path, str(cwd))) + return self.dirty + + +def _lifecycle_entry( + path: str, + kind: str, + safety_tier: str = "auto", + is_directory: bool = False, + extraction_complete: bool | None = None, + lifecycle: dict | None = None, +) -> dict: + """Build a minimal `delete` / `extract-then-delete` report entry. + + Deliberately independent of KIND_TABLE (see module docstring above). + """ + exact_edit: dict[str, Any] = {"kind": kind} + if is_directory: + exact_edit["is_directory"] = True + if extraction_complete is not None: + exact_edit["extraction_complete"] = extraction_complete + entry_extraction_complete = extraction_complete + entry: dict[str, Any] = { + "path": path, + "op": f"test op {kind}", + "op_type": "deterministic", + "is_destructive": True, + "is_reversible": False, + "safety_tier": safety_tier, + "exact_edit": exact_edit, + "lifecycle": lifecycle or { + "rule_ref": "temporary/test-rule", + "lifetime": "temporary", + }, + } + if extraction_complete is not None: + entry["extraction_complete"] = extraction_complete + return entry + + +def _applier_ls(root: Path, fs=None, git=None, git_state=None) -> PatchApplier: + return PatchApplier(project_root=root, fs=fs, git=git, git_state=git_state) + + +class RecordingGitRm: + """Records git rm calls (and optionally mv, unused here).""" + + def __init__(self, fail: bool = False) -> None: + self.rm_calls: list[tuple[str, str, bool]] = [] + self.mv_calls: list[tuple[str, str]] = [] + self._fail = fail + + def mv(self, src: Path, dest: Path, cwd: Path) -> None: + self.mv_calls.append((str(src), str(dest))) + + def rm(self, path: Path, cwd: Path, recursive: bool = False) -> None: + if self._fail: + raise RuntimeError("git rm simulated failure") + self.rm_calls.append((str(path), str(cwd), recursive)) + + +class TestLifecycleDeleteApplyTimeReverification: + """Task 4.1(a): apply-time re-verification — never trust cached tier.""" + + def test_auto_tier_tracked_clean_applies_via_git_rm(self, tmp_path): + fs = MemFS() + git = RecordingGitRm() + git_state = RecordingGitState(tracked=True, dirty=False) + entry = _lifecycle_entry("docs/old.md", "delete", safety_tier="auto") + + result = _applier_ls(tmp_path, fs=fs, git=git, git_state=git_state).apply([entry]) + + assert result["applied"][0]["kind"] == "delete" + assert result["skipped"] == [] + assert len(git.rm_calls) == 1 + rm_path, rm_cwd, recursive = git.rm_calls[0] + assert rm_path == str(tmp_path / "docs" / "old.md") + assert recursive is False + assert "docs/old.md" in result["staged_paths"] + # Re-verification actually consulted live git state for this path. + assert git_state.is_tracked_calls == [("docs/old.md", str(tmp_path))] + + def test_auto_tier_downgraded_to_skip_when_untracked_since_check(self, tmp_path): + fs = MemFS() + git = RecordingGitRm() + git_state = RecordingGitState(tracked=False) + entry = _lifecycle_entry("docs/old.md", "delete", safety_tier="auto") + + result = _applier_ls(tmp_path, fs=fs, git=git, git_state=git_state).apply([entry]) + + assert result["applied"] == [] + assert result["skipped"][0]["reason"] == "git-state-changed-since-check" + assert git.rm_calls == [] + + def test_auto_tier_downgraded_to_skip_when_dirty_since_check(self, tmp_path): + fs = MemFS() + git = RecordingGitRm() + git_state = RecordingGitState(tracked=True, dirty=True) + entry = _lifecycle_entry("docs/old.md", "delete", safety_tier="auto") + + result = _applier_ls(tmp_path, fs=fs, git=git, git_state=git_state).apply([entry]) + + assert result["applied"] == [] + assert result["skipped"][0]["reason"] == "git-state-changed-since-check" + assert git.rm_calls == [] + + def test_other_entries_on_other_paths_continue_after_a_skip(self, tmp_path): + """One entry's guard-skip must not block a sibling path's delete.""" + fs = MemFS() + git = RecordingGitRm() + git_state = RecordingGitState(tracked=False) # everything looks untracked + entries = [ + _lifecycle_entry("docs/a.md", "delete", safety_tier="auto"), + _lifecycle_entry("docs/b.md", "delete", safety_tier="auto"), + ] + + result = _applier_ls(tmp_path, fs=fs, git=git, git_state=git_state).apply(entries) + + assert result["applied"] == [] + assert len(result["skipped"]) == 2 + reasons = {s["reason"] for s in result["skipped"]} + assert reasons == {"git-state-changed-since-check"} + + def test_confirm_tier_lifecycle_delete_is_not_re_verified(self, tmp_path): + """ + A confirm-tier entry was already gated through explicit human + approval upstream (batch-confirm). The applier must not silently + downgrade/skip it just because the path is untracked or dirty — + that dirty/untracked state is precisely why it was confirm-tier. + """ + fs = MemFS() + git = RecordingGitRm() + git_state = RecordingGitState(tracked=False, dirty=True) + entry = _lifecycle_entry("docs/old.md", "delete", safety_tier="confirm") + + result = _applier_ls(tmp_path, fs=fs, git=git, git_state=git_state).apply([entry]) + + assert result["applied"][0]["kind"] == "delete" + assert len(git.rm_calls) == 1 + # No git-state query needed at all for an already-confirmed entry. + assert git_state.is_tracked_calls == [] + + +class TestLifecycleDeleteGitRm: + """Task 4.1(b): `delete` performs true git rm, staged precisely.""" + + def test_directory_aggregate_entry_uses_recursive_git_rm(self, tmp_path): + fs = MemFS() + git = RecordingGitRm() + git_state = RecordingGitState(tracked=True, dirty=False) + entry = _lifecycle_entry( + "autoresearch/run-042", "delete", safety_tier="auto", is_directory=True + ) + + result = _applier_ls(tmp_path, fs=fs, git=git, git_state=git_state).apply([entry]) + + assert result["applied"][0]["kind"] == "delete" + rm_path, rm_cwd, recursive = git.rm_calls[0] + assert rm_path == str(tmp_path / "autoresearch" / "run-042") + assert recursive is True + + def test_directory_delete_bypasses_content_hash_guard(self, tmp_path): + """ + A directory aggregate entry carries no expected_sha256 (no single + file to hash) — it must still apply cleanly via git rm, never + rejected for "missing" hash fields. + """ + fs = MemFS() + git = RecordingGitRm() + git_state = RecordingGitState(tracked=True, dirty=False) + entry = _lifecycle_entry( + "autoresearch/run-001", "delete", safety_tier="auto", is_directory=True + ) + assert "expected_sha256" not in entry["exact_edit"] + + result = _applier_ls(tmp_path, fs=fs, git=git, git_state=git_state).apply([entry]) + + assert result["applied"][0]["kind"] == "delete" + assert result["failed"] == [] + + def test_git_rm_failure_reported_as_failed(self, tmp_path): + fs = MemFS() + git = RecordingGitRm(fail=True) + git_state = RecordingGitState(tracked=True, dirty=False) + entry = _lifecycle_entry("docs/old.md", "delete", safety_tier="auto") + + result = _applier_ls(tmp_path, fs=fs, git=git, git_state=git_state).apply([entry]) + + assert result["applied"] == [] + assert result["failed"][0]["kind"] == "delete" + assert "git rm simulated failure" in result["failed"][0]["error"] + + def test_real_git_rm_stages_deletion(self, tmp_path): + """Integration: real git rm in a real repo removes + stages the file.""" + repo = _init_git_repo(tmp_path) + target = repo / "old.md" + target.write_bytes(b"stale content\n") + subprocess.run(["git", "add", "old.md"], cwd=str(repo), check=True, capture_output=True) + subprocess.run( + ["git", "commit", "-m", "add file"], + cwd=str(repo), check=True, capture_output=True, + ) + + entry = _lifecycle_entry("old.md", "delete", safety_tier="auto") + result = PatchApplier(project_root=repo).apply([entry]) + + assert result["applied"][0]["kind"] == "delete" + assert not target.exists(), "file should be gone after git rm" + + status = subprocess.run( + ["git", "status", "--porcelain"], + cwd=str(repo), capture_output=True, text=True, + ).stdout + # Staged deletion shows as "D old.md" (space, not "D " in worktree column). + assert "D old.md" in status + + +class TestExtractThenDeleteFailsClosed: + """Task 4.1(c): extract-then-delete only deletes once extraction is done.""" + + def test_skipped_when_extraction_not_marked_complete(self, tmp_path): + fs = MemFS() + git = RecordingGitRm() + git_state = RecordingGitState(tracked=True, dirty=False) + entry = _lifecycle_entry( + "docs/old-guide.md", "extract-then-delete", safety_tier="auto", + extraction_complete=False, + ) + + result = _applier_ls(tmp_path, fs=fs, git=git, git_state=git_state).apply([entry]) + + assert result["applied"] == [] + assert result["skipped"][0]["reason"] == "extraction-not-confirmed" + assert git.rm_calls == [] + + def test_skipped_when_extraction_flag_absent(self, tmp_path): + """No extraction_complete field at all → treated as not confirmed.""" + fs = MemFS() + git = RecordingGitRm() + git_state = RecordingGitState(tracked=True, dirty=False) + entry = _lifecycle_entry( + "docs/old-guide.md", "extract-then-delete", safety_tier="auto", + ) + + result = _applier_ls(tmp_path, fs=fs, git=git, git_state=git_state).apply([entry]) + + assert result["applied"] == [] + assert result["skipped"][0]["reason"] == "extraction-not-confirmed" + assert git.rm_calls == [] + + def test_applies_git_rm_once_extraction_confirmed(self, tmp_path): + fs = MemFS() + git = RecordingGitRm() + git_state = RecordingGitState(tracked=True, dirty=False) + entry = _lifecycle_entry( + "docs/old-guide.md", "extract-then-delete", safety_tier="auto", + extraction_complete=True, + ) + + result = _applier_ls(tmp_path, fs=fs, git=git, git_state=git_state).apply([entry]) + + assert result["applied"][0]["kind"] == "extract-then-delete" + assert len(git.rm_calls) == 1 + + def test_failed_extraction_is_per_entry_skip_not_run_level_failure(self, tmp_path): + """ + A failed extraction for one entry must not prevent a sibling path's + entry (independent extraction) from applying in the same run. + """ + fs = MemFS() + git = RecordingGitRm() + git_state = RecordingGitState(tracked=True, dirty=False) + entries = [ + _lifecycle_entry( + "docs/not-extracted.md", "extract-then-delete", safety_tier="auto", + extraction_complete=False, + ), + _lifecycle_entry( + "docs/extracted.md", "extract-then-delete", safety_tier="auto", + extraction_complete=True, + ), + ] + + result = _applier_ls(tmp_path, fs=fs, git=git, git_state=git_state).apply(entries) + + assert len(result["applied"]) == 1 + assert result["applied"][0]["path"] == "docs/extracted.md" + assert len(result["skipped"]) == 1 + assert result["skipped"][0]["reason"] == "extraction-not-confirmed" + + def test_confirm_tier_extract_then_delete_still_fails_closed_on_extraction(self, tmp_path): + """ + Even for an already-approved confirm-tier entry, extraction + confirmation is unconditional — the applier itself never performs + the generative extraction, so it cannot proceed without the flag. + """ + fs = MemFS() + git = RecordingGitRm() + git_state = RecordingGitState(tracked=False, dirty=True) + entry = _lifecycle_entry( + "docs/old-guide.md", "extract-then-delete", safety_tier="confirm", + extraction_complete=False, + ) + + result = _applier_ls(tmp_path, fs=fs, git=git, git_state=git_state).apply([entry]) + + assert result["applied"] == [] + assert result["skipped"][0]["reason"] == "extraction-not-confirmed" + assert git.rm_calls == [] + + +class TestLifecycleConfirmTierGating: + """Task 4.1(d): confirm-tier lifecycle entries never auto-apply silently.""" + + def test_confirm_tier_entry_only_applies_when_explicitly_indexed(self, tmp_path): + """ + Mirrors the existing --apply-indices gating convention: the applier + only ever touches entries the caller explicitly selected. A + confirm-tier lifecycle entry sitting in report.entries but NOT + passed via --apply-indices must never be applied. + """ + content = "line1\n" + file_bytes, sha = _make_file(content) + fs = MemFS({str(tmp_path / "doc.md"): file_bytes}) + git = RecordingGitRm() + git_state = RecordingGitState(tracked=True, dirty=False) + + report_entries = [ + _entry("doc.md", "delete-range", sha=sha, start=1, end=1), # index 0, auto + _lifecycle_entry("docs/risky.md", "delete", safety_tier="confirm"), # index 1 + ] + report = { + "schema_version": "1.0", "tool_version": "0.1.0", + "generated_at": "2026-07-14T00:00:00+00:00", + "scan": {"project_root": str(tmp_path), "scope_globs": [], "excluded_dirs": [], "files_scanned": 0}, + "shortlist": [], "entries": report_entries, + } + report_path = tmp_path / "report.json" + report_path.write_text(json.dumps(report)) + + # Caller (skill) only approved index 0 — the confirm-tier delete was + # never approved and must not be passed. + applier = _applier_ls(tmp_path, fs=fs, git=git, git_state=git_state) + indexed = [(0, report_entries[0])] + result = applier.apply_indexed(indexed) + + assert result["applied"][0]["entry_index"] == 0 + assert git.rm_calls == [] # confirm-tier delete never touched + + def test_confirm_tier_entry_applies_once_explicitly_approved_and_indexed(self, tmp_path): + fs = MemFS() + git = RecordingGitRm() + git_state = RecordingGitState(tracked=True, dirty=False) + entry = _lifecycle_entry("docs/risky.md", "delete", safety_tier="confirm") + + # Caller explicitly included this index after the user approved it + # in the batch-confirm prompt. + result = _applier_ls(tmp_path, fs=fs, git=git, git_state=git_state).apply_indexed( + [(1, entry)] + ) + + assert result["applied"][0]["entry_index"] == 1 + assert result["applied"][0]["kind"] == "delete" + assert len(git.rm_calls) == 1 + + +class TestLifecycleDeleteIncompatibleWithOtherOps: + """A lifecycle delete cannot be combined with any other op on the same path.""" + + def test_delete_combined_with_another_op_on_same_path_is_incompatible(self, tmp_path): + content = "line1\n" + file_bytes, sha = _make_file(content) + fs = MemFS({str(tmp_path / "doc.md"): file_bytes}) + git = RecordingGitRm() + git_state = RecordingGitState(tracked=True, dirty=False) + + entries = [ + _lifecycle_entry("doc.md", "delete", safety_tier="auto"), + _entry("doc.md", "insert-frontmatter", extra={"key": "hygiene", "value": "frozen"}), + ] + result = _applier_ls(tmp_path, fs=fs, git=git, git_state=git_state).apply(entries) + + assert result["applied"] == [] + reasons = {s["reason"] for s in result["skipped"]} + assert reasons == {"incompatible-ops-on-file"} + assert git.rm_calls == [] + + +class TestUnknownKindStillRejectedAlongsideLifecycleKinds: + """Regression: adding the lifecycle carve-out must not swallow real unknowns.""" + + def test_truly_unknown_kind_still_reported(self, tmp_path): + fs = MemFS() + entry = { + "path": "doc.md", + "op": "nuke it", + "op_type": "deterministic", + "exact_edit": {"kind": "nuke-it", "anchor": {"start_line": 1, "end_line": 1}}, + } + result = _applier(tmp_path, fs=fs).apply([entry]) + + assert result["skipped"][0]["reason"] == "unknown-kind" diff --git a/plugins/os-doc-hygiene/tests/test_report_builder.py b/plugins/os-doc-hygiene/tests/test_report_builder.py index 6da01c8..59b8e3e 100644 --- a/plugins/os-doc-hygiene/tests/test_report_builder.py +++ b/plugins/os-doc-hygiene/tests/test_report_builder.py @@ -447,6 +447,314 @@ class TestMalformedRejection: # CRITICAL: round-trip through validate_report.py # =========================================================================== +class FakeGitStateProvider: + """Injected git-state provider — returns canned {tracked, clean} per + rel_path, matching the report_builder.py convention of an injectable + real-vs-fake seam (same shape as scanner.py's git_log_fn).""" + + def __init__(self, states: dict) -> None: + self._states = states + + def state(self, root: Path, rel_path: str) -> dict: + return self._states.get(rel_path, {"tracked": False, "clean": False}) + + +def _lifecycle_delete_proposal(**lifecycle_overrides) -> dict: + lifecycle = {"rule_ref": "autoresearch/*/**", "lifetime": "temporary"} + lifecycle.update(lifecycle_overrides) + return { + "path": "docs/clean.md", + "category": {"class": "stale", "subtype": "orphaned"}, + "signals": [{"name": "temporary_expired", "detail": "past retain-recent window"}], + "op": "Delete expired temporary entry.", + "op_type": "deterministic", + "confidence": 0.9, + "exact_edit": { + "kind": "delete", + "anchor": {"start_line": 1, "end_line": 3}, + "lifecycle": lifecycle, + }, + } + + +class TestLifecycleDeleteKind: + """Group 3 (task 3.1/3.3): delete/extract-then-delete assembly — + git_state population (guardrail, never trusted from the proposal) and + the lifecycle-aware safety_tier branch.""" + + def test_git_state_is_populated_from_provider_not_proposal(self, doc_tree, scan_artifact): + proposal = _lifecycle_delete_proposal() + # Proposal lies about git_state; report_builder must ignore it. + proposal["exact_edit"]["lifecycle"]["git_state"] = {"tracked": False, "clean": False} + provider = FakeGitStateProvider({"docs/clean.md": {"tracked": True, "clean": True}}) + builder = ReportBuilder( + project_root=doc_tree, clock=MonotonicClock(_T0), + estimator=default_estimator(), git_state_provider=provider, + ) + result = builder.build(scan_artifact, [proposal]) + entry = result["machine_report"]["entries"][0] + assert entry["exact_edit"]["lifecycle"]["git_state"] == {"tracked": True, "clean": True} + + def test_scanner_proven_tracked_clean_yields_auto(self, doc_tree, scan_artifact): + provider = FakeGitStateProvider({"docs/clean.md": {"tracked": True, "clean": True}}) + builder = ReportBuilder( + project_root=doc_tree, clock=MonotonicClock(_T0), + estimator=default_estimator(), git_state_provider=provider, + ) + result = builder.build(scan_artifact, [_lifecycle_delete_proposal()]) + entry = result["machine_report"]["entries"][0] + assert entry["safety_tier"] == "auto" + + def test_dirty_worktree_forces_confirm(self, doc_tree, scan_artifact): + provider = FakeGitStateProvider({"docs/clean.md": {"tracked": True, "clean": False}}) + builder = ReportBuilder( + project_root=doc_tree, clock=MonotonicClock(_T0), + estimator=default_estimator(), git_state_provider=provider, + ) + result = builder.build(scan_artifact, [_lifecycle_delete_proposal()]) + entry = result["machine_report"]["entries"][0] + assert entry["safety_tier"] == "confirm" + + def test_untracked_forces_confirm(self, doc_tree, scan_artifact): + provider = FakeGitStateProvider({}) # unknown path -> untracked + builder = ReportBuilder( + project_root=doc_tree, clock=MonotonicClock(_T0), + estimator=default_estimator(), git_state_provider=provider, + ) + result = builder.build(scan_artifact, [_lifecycle_delete_proposal()]) + entry = result["machine_report"]["entries"][0] + assert entry["safety_tier"] == "confirm" + + def test_classifier_judged_served_when_always_confirm(self, doc_tree, scan_artifact): + provider = FakeGitStateProvider({"docs/clean.md": {"tracked": True, "clean": True}}) + proposal = _lifecycle_delete_proposal( + lifetime="delete-once-served", + served_when="the effort this plan describes has shipped", + ) + builder = ReportBuilder( + project_root=doc_tree, clock=MonotonicClock(_T0), + estimator=default_estimator(), git_state_provider=provider, + ) + result = builder.build(scan_artifact, [proposal]) + entry = result["machine_report"]["entries"][0] + assert entry["safety_tier"] == "confirm" + + def test_served_when_path_tracked_clean_yields_auto(self, doc_tree, scan_artifact): + provider = FakeGitStateProvider({"docs/clean.md": {"tracked": True, "clean": True}}) + proposal = _lifecycle_delete_proposal( + lifetime="delete-once-served", + served_when_path="openspec/changes/archive/{id}/", + ) + builder = ReportBuilder( + project_root=doc_tree, clock=MonotonicClock(_T0), + estimator=default_estimator(), git_state_provider=provider, + ) + result = builder.build(scan_artifact, [proposal]) + entry = result["machine_report"]["entries"][0] + assert entry["safety_tier"] == "auto" + + def test_delete_entry_is_destructive_true(self, doc_tree, scan_artifact): + provider = FakeGitStateProvider({"docs/clean.md": {"tracked": True, "clean": True}}) + builder = ReportBuilder( + project_root=doc_tree, clock=MonotonicClock(_T0), + estimator=default_estimator(), git_state_provider=provider, + ) + result = builder.build(scan_artifact, [_lifecycle_delete_proposal()]) + entry = result["machine_report"]["entries"][0] + assert entry["is_destructive"] is True + + def test_delete_entry_passes_validator(self, doc_tree, scan_artifact): + provider = FakeGitStateProvider({"docs/clean.md": {"tracked": True, "clean": True}}) + builder = ReportBuilder( + project_root=doc_tree, clock=MonotonicClock(_T0), + estimator=default_estimator(), git_state_provider=provider, + ) + result = builder.build(scan_artifact, [_lifecycle_delete_proposal()]) + violations = ReportValidator(result["machine_report"]).validate() + assert violations == [] + + def test_extract_then_delete_repo_durable_passes_validator(self, doc_tree, scan_artifact): + provider = FakeGitStateProvider({"docs/clean.md": {"tracked": True, "clean": True}}) + proposal = _lifecycle_delete_proposal() + proposal["exact_edit"]["kind"] = "extract-then-delete" + proposal["exact_edit"]["extraction_target"] = "repo-durable" + proposal["exact_edit"]["extraction_dest"] = "CLAUDE.md" + builder = ReportBuilder( + project_root=doc_tree, clock=MonotonicClock(_T0), + estimator=default_estimator(), git_state_provider=provider, + ) + result = builder.build(scan_artifact, [proposal]) + violations = ReportValidator(result["machine_report"]).validate() + assert violations == [] + + def test_missing_extraction_target_is_malformed_proposal(self, doc_tree, scan_artifact): + proposal = _lifecycle_delete_proposal() + proposal["exact_edit"]["kind"] = "extract-then-delete" + # extraction_target omitted -> report_builder rejects before compute + builder = ReportBuilder(project_root=doc_tree, clock=MonotonicClock(_T0)) + with pytest.raises(MalformedProposalError): + builder.build(scan_artifact, [proposal]) + + def test_whole_file_span_counted_for_delete(self, doc_tree, scan_artifact): + provider = FakeGitStateProvider({"docs/clean.md": {"tracked": True, "clean": True}}) + builder = ReportBuilder( + project_root=doc_tree, clock=MonotonicClock(_T0), + estimator=default_estimator(), git_state_provider=provider, + ) + result = builder.build(scan_artifact, [_lifecycle_delete_proposal()]) + entry = result["machine_report"]["entries"][0] + whole = default_estimator().estimate((doc_tree / "docs" / "clean.md").read_text()) + assert entry["token_estimate"]["raw_tokens"] == whole + + +class TestPromotionCandidates: + """Group 3 (task 3.2): deterministic, model-free promotion_candidates.""" + + def _builder(self, doc_tree, conventions_path=None, **kw): + return ReportBuilder( + project_root=doc_tree, clock=MonotonicClock(_T0), + estimator=default_estimator(), conventions_path=conventions_path, **kw + ) + + def _write_conventions(self, tmp_path: Path) -> Path: + path = tmp_path / "conventions.json" + path.write_text(json.dumps([ + {"name": "archive-bucket", "what_it_proves": "moved to archive/", "pitch": "Move it once."}, + {"name": "status-frontmatter", "what_it_proves": "status: shipped", "pitch": "Stamp status once."}, + ])) + return path + + def test_classifier_judged_entry_yields_candidates(self, doc_tree, scan_artifact, tmp_path): + conventions_path = self._write_conventions(tmp_path) + provider = FakeGitStateProvider({"docs/clean.md": {"tracked": True, "clean": True}}) + proposal = _lifecycle_delete_proposal( + lifetime="delete-once-served", + served_when="the effort this plan describes has shipped", + ) + builder = self._builder(doc_tree, conventions_path=conventions_path, git_state_provider=provider) + result = builder.build(scan_artifact, [proposal]) + candidates = result["machine_report"]["promotion_candidates"] + names = {c["name"] for c in candidates} + assert names == {"archive-bucket", "status-frontmatter"} + assert all(c["path"] == "docs/clean.md" for c in candidates) + assert all(c["pitch"] for c in candidates) + + def test_scanner_proven_served_when_path_yields_no_candidate(self, doc_tree, scan_artifact, tmp_path): + conventions_path = self._write_conventions(tmp_path) + provider = FakeGitStateProvider({"docs/clean.md": {"tracked": True, "clean": True}}) + proposal = _lifecycle_delete_proposal( + lifetime="delete-once-served", + served_when_path="openspec/changes/archive/{id}/", + ) + builder = self._builder(doc_tree, conventions_path=conventions_path, git_state_provider=provider) + result = builder.build(scan_artifact, [proposal]) + assert result["machine_report"]["promotion_candidates"] == [] + + def test_no_lifecycle_entries_yields_empty_candidates(self, doc_tree, scan_artifact, tmp_path): + conventions_path = self._write_conventions(tmp_path) + builder = self._builder(doc_tree, conventions_path=conventions_path) + result = builder.build(scan_artifact, _proposals()) + assert result["machine_report"]["promotion_candidates"] == [] + + def test_promotion_candidates_always_present_even_empty(self, doc_tree, scan_artifact): + result = _build(doc_tree, scan_artifact, []) + assert result["machine_report"]["promotion_candidates"] == [] + + def test_missing_conventions_file_yields_empty_candidates_no_crash(self, doc_tree, scan_artifact, tmp_path): + missing = tmp_path / "does-not-exist.json" + provider = FakeGitStateProvider({"docs/clean.md": {"tracked": True, "clean": True}}) + proposal = _lifecycle_delete_proposal( + lifetime="delete-once-served", + served_when="shipped", + ) + builder = self._builder(doc_tree, conventions_path=missing, git_state_provider=provider) + result = builder.build(scan_artifact, [proposal]) + assert result["machine_report"]["promotion_candidates"] == [] + + def test_shipped_conventions_json_has_exactly_two_entries(self): + real_conventions = _SCRIPTS_DIR.parent / "conventions.json" + data = json.loads(real_conventions.read_text()) + assert isinstance(data, list) + names = {c["name"] for c in data} + assert names == {"archive-bucket", "status-frontmatter"} + for c in data: + assert c.get("name") + assert c.get("what_it_proves") + assert c.get("pitch") + + def test_default_conventions_path_is_the_shipped_catalog(self, doc_tree, scan_artifact): + """No conventions_path override -> reads the real, shipped catalog.""" + provider = FakeGitStateProvider({"docs/clean.md": {"tracked": True, "clean": True}}) + proposal = _lifecycle_delete_proposal( + lifetime="delete-once-served", + served_when="shipped", + ) + builder = ReportBuilder( + project_root=doc_tree, clock=MonotonicClock(_T0), + estimator=default_estimator(), git_state_provider=provider, + ) + result = builder.build(scan_artifact, [proposal]) + names = {c["name"] for c in result["machine_report"]["promotion_candidates"]} + assert names == {"archive-bucket", "status-frontmatter"} + + +class TestHumanReportLifecycleAndPromotionRendering: + """Task 5.1: the human report renders lifecycle findings + the + Promotion Candidates section (doc-check spec: 'both the machine report's + promotion_candidates array and the human report show the candidate').""" + + def _builder(self, doc_tree, conventions_path=None, **kw): + return ReportBuilder( + project_root=doc_tree, clock=MonotonicClock(_T0), + estimator=default_estimator(), conventions_path=conventions_path, **kw + ) + + def _write_conventions(self, tmp_path: Path) -> Path: + path = tmp_path / "conventions.json" + path.write_text(json.dumps([ + {"name": "archive-bucket", "what_it_proves": "moved to archive/", "pitch": "Move it once."}, + ])) + return path + + def test_promotion_candidates_section_present_when_empty(self, doc_tree, scan_artifact): + result = _build(doc_tree, scan_artifact, []) + assert "## Promotion Candidates" in result["human_report"] + assert "(none)" in result["human_report"] + + def test_promotion_candidates_named_with_pitch_in_human_report( + self, doc_tree, scan_artifact, tmp_path + ): + conventions_path = self._write_conventions(tmp_path) + provider = FakeGitStateProvider({"docs/clean.md": {"tracked": True, "clean": True}}) + proposal = _lifecycle_delete_proposal( + lifetime="delete-once-served", + served_when="the effort this plan describes has shipped", + ) + builder = self._builder(doc_tree, conventions_path=conventions_path, git_state_provider=provider) + result = builder.build(scan_artifact, [proposal]) + human = result["human_report"] + assert "## Promotion Candidates" in human + assert "archive-bucket" in human + assert "Move it once." in human + + def test_lifecycle_entry_line_renders_rule_ref_and_lifetime( + self, doc_tree, scan_artifact + ): + provider = FakeGitStateProvider({"docs/clean.md": {"tracked": True, "clean": True}}) + proposal = _lifecycle_delete_proposal( + lifetime="delete-once-served", + served_when_path="openspec/changes/archive/{id}/", + ) + builder = ReportBuilder( + project_root=doc_tree, clock=MonotonicClock(_T0), + estimator=default_estimator(), git_state_provider=provider, + ) + result = builder.build(scan_artifact, [proposal]) + human = result["human_report"] + assert "lifecycle:" in human + assert "delete-once-served" in human + + class TestValidatorRoundTrip: def test_assembled_report_passes_validator(self, doc_tree, scan_artifact): diff --git a/plugins/os-doc-hygiene/tests/test_rulebook.py b/plugins/os-doc-hygiene/tests/test_rulebook.py new file mode 100644 index 0000000..73c0f48 --- /dev/null +++ b/plugins/os-doc-hygiene/tests/test_rulebook.py @@ -0,0 +1,370 @@ +""" +Tests for scripts/rulebook.py + +Covers tasks 1.1-1.2 of the lifecycle-aware-doc-hygiene change: +- envelope parsing, hard-fail on unparseable JSON / unknown schema_version +- skip-and-warn on invalid rules and rules missing confirmed_by +- glob compilation via glob.translate(recursive=True, include_hidden=True), + with a Python >=3.13 version check +- add-only merge + two-axis precedence (project file > project dir > + global file > global dir; ties by longest glob, then last-defined) +- keep-shadowing neutralization +- unmatched path -> None +- file-level vs directory-level match distinction +- IGNORE-sentinel rules (no lifetime field -> zero-emission prune) + +sys.path is patched here (no shared conftest) so the test file is +self-contained, as required by the file-ownership constraints. +""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +import pytest + +_SCRIPTS_DIR = Path(__file__).parent.parent / "scripts" +if str(_SCRIPTS_DIR) not in sys.path: + sys.path.insert(0, str(_SCRIPTS_DIR)) + +from rulebook import ( # noqa: E402 + Rulebook, + RulebookLoadError, + load_rulebook, +) + + +def _write(tmp_path: Path, name: str, payload: dict) -> Path: + p = tmp_path / name + p.write_text(json.dumps(payload), encoding="utf-8") + return p + + +def _envelope(*rules: dict) -> dict: + return {"schema_version": 1, "rules": list(rules)} + + +def _rule(glob, lifetime="keep", confirmed_by="human", **extra) -> dict: + r = { + "glob": glob, + "confirmed_by": confirmed_by, + "confirmed_on": "2026-07-14", + "source": "test", + } + if lifetime is not None: + r["lifetime"] = lifetime + r.update(extra) + return r + + +# =========================================================================== +# Task 1.1 — envelope parsing / hard-fail / skip-and-warn / glob compilation +# =========================================================================== + +class TestEnvelopeParsing: + def test_loads_valid_global_only(self, tmp_path): + global_path = _write( + tmp_path, "rulebook.json", _envelope(_rule("foo.md")) + ) + rb = load_rulebook(global_path=global_path, project_path=None) + assert isinstance(rb, Rulebook) + + def test_missing_project_file_is_fine(self, tmp_path): + global_path = _write( + tmp_path, "rulebook.json", _envelope(_rule("foo.md")) + ) + project_path = tmp_path / "does-not-exist.json" + rb = load_rulebook(global_path=global_path, project_path=project_path) + assert isinstance(rb, Rulebook) + + def test_unparseable_json_hard_fails(self, tmp_path): + global_path = tmp_path / "rulebook.json" + global_path.write_text("{not valid json", encoding="utf-8") + with pytest.raises(RulebookLoadError): + load_rulebook(global_path=global_path, project_path=None) + + def test_unknown_schema_version_hard_fails(self, tmp_path): + global_path = _write( + tmp_path, + "rulebook.json", + {"schema_version": 99, "rules": [_rule("foo.md")]}, + ) + with pytest.raises(RulebookLoadError): + load_rulebook(global_path=global_path, project_path=None) + + def test_project_file_unparseable_json_hard_fails(self, tmp_path): + global_path = _write( + tmp_path, "rulebook.json", _envelope(_rule("foo.md")) + ) + project_path = tmp_path / "proj.json" + project_path.write_text("nope not json", encoding="utf-8") + with pytest.raises(RulebookLoadError): + load_rulebook(global_path=global_path, project_path=project_path) + + +class TestSkipAndWarnValidation: + def test_rule_missing_confirmed_by_is_skipped_not_fatal(self, tmp_path): + bad = { + "glob": "unconfirmed.md", + "lifetime": "keep", + "confirmed_on": "2026-07-14", + "source": "test", + } + assert "confirmed_by" not in bad + good_rules = [_rule(f"good-{i}.md") for i in range(9)] + global_path = _write( + tmp_path, "rulebook.json", _envelope(bad, *good_rules) + ) + rb = load_rulebook(global_path=global_path, project_path=None) + + # the invalid rule never contributes a match + assert rb.query("unconfirmed.md", is_dir=False) is None + # a warning is surfaced + assert any("confirmed_by" in w for w in rb.warnings) + # the other nine rules still function + for i in range(9): + match = rb.query(f"good-{i}.md", is_dir=False) + assert match is not None + + def test_invalid_lifetime_value_is_skipped(self, tmp_path): + bad = _rule("bad.md", lifetime="not-a-real-lifetime") + global_path = _write(tmp_path, "rulebook.json", _envelope(bad)) + rb = load_rulebook(global_path=global_path, project_path=None) + assert rb.query("bad.md", is_dir=False) is None + assert rb.warnings + + def test_rule_missing_glob_is_skipped(self, tmp_path): + bad = { + "lifetime": "keep", + "confirmed_by": "human", + "confirmed_on": "2026-07-14", + "source": "test", + } + global_path = _write(tmp_path, "rulebook.json", _envelope(bad)) + rb = load_rulebook(global_path=global_path, project_path=None) + assert rb.warnings + + def test_propagate_ignore_field_is_rejected(self, tmp_path): + bad = _rule("bad.md", propagate_ignore=True) + global_path = _write(tmp_path, "rulebook.json", _envelope(bad)) + rb = load_rulebook(global_path=global_path, project_path=None) + assert rb.query("bad.md", is_dir=False) is None + assert any("propagate_ignore" in w for w in rb.warnings) + + def test_defaults_retain_recent_and_max_age_days(self, tmp_path): + rule = _rule("stuff/**", lifetime="temporary") + global_path = _write(tmp_path, "rulebook.json", _envelope(rule)) + rb = load_rulebook(global_path=global_path, project_path=None) + match = rb.query("stuff", is_dir=True) + assert match is not None + assert match.rule["retain_recent"] == 3 + assert match.rule["max_age_days"] == 3 + + def test_overridden_retain_recent_and_max_age_days(self, tmp_path): + rule = _rule( + "stuff/**", lifetime="temporary", retain_recent=5, max_age_days=10 + ) + global_path = _write(tmp_path, "rulebook.json", _envelope(rule)) + rb = load_rulebook(global_path=global_path, project_path=None) + match = rb.query("stuff", is_dir=True) + assert match.rule["retain_recent"] == 5 + assert match.rule["max_age_days"] == 10 + + +class TestGlobCompilationVersionCheck: + def test_hard_fails_clearly_below_python_313(self, tmp_path, monkeypatch): + import rulebook as rb_module + + monkeypatch.setattr(rb_module.sys, "version_info", (3, 12, 0)) + global_path = _write( + tmp_path, "rulebook.json", _envelope(_rule("foo.md")) + ) + with pytest.raises(RulebookLoadError, match=r"(?i)python.*3\.13"): + load_rulebook(global_path=global_path, project_path=None) + + def test_compiles_recursive_double_star(self, tmp_path): + rule = _rule("autoresearch/*/**", lifetime="temporary") + global_path = _write(tmp_path, "rulebook.json", _envelope(rule)) + rb = load_rulebook(global_path=global_path, project_path=None) + match = rb.query("autoresearch/run-2026-07-01", is_dir=True) + assert match is not None + assert match.level == "directory" + + def test_matches_hidden_dotfile_paths(self, tmp_path): + rule = _rule(".dochygiene/**") + global_path = _write(tmp_path, "rulebook.json", _envelope(rule)) + rb = load_rulebook(global_path=global_path, project_path=None) + match = rb.query(".dochygiene", is_dir=True) + assert match is not None + + +# =========================================================================== +# Task 1.2 — add-only merge, two-axis precedence, keep-shadowing, +# unmatched -> None, file-vs-directory distinction, IGNORE sentinels +# =========================================================================== + +class TestPrecedenceAndMerge: + def test_unmatched_path_returns_none(self, tmp_path): + global_path = _write( + tmp_path, "rulebook.json", _envelope(_rule("foo.md")) + ) + rb = load_rulebook(global_path=global_path, project_path=None) + assert rb.query("totally/unrelated/path.md", is_dir=False) is None + + def test_file_level_vs_directory_level_distinction(self, tmp_path): + rules = [ + _rule("HANDOFF-2026-07-01.md", lifetime="delete-once-served"), + _rule("autoresearch/*/**", lifetime="temporary"), + ] + global_path = _write(tmp_path, "rulebook.json", _envelope(*rules)) + rb = load_rulebook(global_path=global_path, project_path=None) + + file_match = rb.query("HANDOFF-2026-07-01.md", is_dir=False) + assert file_match.level == "file" + + dir_match = rb.query("autoresearch/run-1", is_dir=True) + assert dir_match.level == "directory" + + def test_project_file_rule_outranks_global_directory_rule(self, tmp_path): + global_path = _write( + tmp_path, + "rulebook.json", + _envelope(_rule("notes/**", lifetime="temporary")), + ) + project_path = _write( + tmp_path, + "proj.json", + _envelope(_rule("notes/keep-me.md", lifetime="keep")), + ) + rb = load_rulebook(global_path=global_path, project_path=project_path) + match = rb.query("notes/keep-me.md", is_dir=False) + assert match is not None + assert match.rule["lifetime"] == "keep" + assert match.level == "file" + + def test_project_directory_rule_outranks_global_file_rule(self, tmp_path): + # Same file path matched by BOTH a global file-rule and a project + # directory-rule whose subtree covers it; project-dir (tier 1) + # beats global-file (tier 2) per the two-axis precedence order. + global_path = _write( + tmp_path, + "rulebook.json", + _envelope(_rule("notes/x.md", lifetime="delete-once-served")), + ) + project_path = _write( + tmp_path, + "proj.json", + _envelope(_rule("notes/**", lifetime="temporary")), + ) + rb = load_rulebook(global_path=global_path, project_path=project_path) + match = rb.query("notes/x.md", is_dir=False) + assert match is not None + assert match.rule["lifetime"] == "temporary" + assert match.origin == "project" + assert match.level == "directory" + + def test_project_file_rule_outranks_matching_global_directory_rule( + self, tmp_path + ): + # A project file-rule and a global directory-rule both cover the + # same file path; project-file (tier 0) must win over + # global-directory (tier 3) even though the directory-rule's + # subtree also matches. + global_path = _write( + tmp_path, + "rulebook.json", + _envelope(_rule("notes/**", lifetime="temporary")), + ) + project_path = _write( + tmp_path, + "proj.json", + _envelope(_rule("notes/keep-me.md", lifetime="keep")), + ) + rb = load_rulebook(global_path=global_path, project_path=project_path) + match = rb.query("notes/keep-me.md", is_dir=False) + assert match is not None + assert match.rule["lifetime"] == "keep" + assert match.origin == "project" + assert match.level == "file" + + def test_ties_broken_by_longest_glob_then_last_defined(self, tmp_path): + # Both directory-rules' subtrees genuinely cover "a/b/c.md" (real + # tie within the same (global, directory) tier) — the longer + # pattern "a/b/**" must win over "a/**". + rules = [ + _rule("a/**", lifetime="temporary", note="short"), + _rule("a/b/**", lifetime="keep", note="longer"), + ] + global_path = _write(tmp_path, "rulebook.json", _envelope(*rules)) + rb = load_rulebook(global_path=global_path, project_path=None) + match = rb.query("a/b/c.md", is_dir=False) + assert match is not None + assert match.rule["note"] == "longer" + + def test_ties_equal_length_last_defined_wins(self, tmp_path): + rules = [ + _rule("dup/**", lifetime="temporary", note="first"), + _rule("dup/**", lifetime="keep", note="second"), + ] + global_path = _write(tmp_path, "rulebook.json", _envelope(*rules)) + rb = load_rulebook(global_path=global_path, project_path=None) + match = rb.query("dup", is_dir=True) + assert match.rule["note"] == "second" + + def test_add_only_merge_never_deletes_global_rules(self, tmp_path): + global_path = _write( + tmp_path, + "rulebook.json", + _envelope(_rule("a.md"), _rule("b.md")), + ) + project_path = _write( + tmp_path, "proj.json", _envelope(_rule("c.md")) + ) + rb = load_rulebook(global_path=global_path, project_path=project_path) + assert rb.query("a.md", is_dir=False) is not None + assert rb.query("b.md", is_dir=False) is not None + assert rb.query("c.md", is_dir=False) is not None + + def test_keep_shadowing_neutralizes_global_delete_rule(self, tmp_path): + global_path = _write( + tmp_path, + "rulebook.json", + _envelope(_rule("autoresearch/*/**", lifetime="temporary")), + ) + project_path = _write( + tmp_path, + "proj.json", + _envelope(_rule("autoresearch/special/**", lifetime="keep")), + ) + rb = load_rulebook(global_path=global_path, project_path=project_path) + shadowed = rb.query("autoresearch/special", is_dir=True) + assert shadowed.rule["lifetime"] == "keep" + # an unrelated run dir under the same global rule is still temporary + other = rb.query("autoresearch/other-run", is_dir=True) + assert other.rule["lifetime"] == "temporary" + + +class TestIgnoreSentinel: + def test_ignore_rule_has_no_lifetime_and_is_flagged(self, tmp_path): + ignore_rule = _rule("graphify-out/**", lifetime=None) + global_path = _write(tmp_path, "rulebook.json", _envelope(ignore_rule)) + rb = load_rulebook(global_path=global_path, project_path=None) + match = rb.query("graphify-out", is_dir=True) + assert match is not None + assert match.is_ignore is True + assert "lifetime" not in match.rule or match.rule.get("lifetime") is None + + def test_ignore_rule_distinct_from_keep(self, tmp_path): + rules = [ + _rule("graphify-out/**", lifetime=None), + _rule(".dochygiene/**", lifetime="keep"), + ] + global_path = _write(tmp_path, "rulebook.json", _envelope(*rules)) + rb = load_rulebook(global_path=global_path, project_path=None) + ignore_match = rb.query("graphify-out", is_dir=True) + keep_match = rb.query(".dochygiene", is_dir=True) + assert ignore_match.is_ignore is True + assert keep_match.is_ignore is False + assert keep_match.rule["lifetime"] == "keep" diff --git a/plugins/os-doc-hygiene/tests/test_scanner_lifecycle.py b/plugins/os-doc-hygiene/tests/test_scanner_lifecycle.py new file mode 100644 index 0000000..e71e36a --- /dev/null +++ b/plugins/os-doc-hygiene/tests/test_scanner_lifecycle.py @@ -0,0 +1,421 @@ +""" +Tests for scanner lifecycle signals (tasks 2.1-2.3 of lifecycle-aware-doc-hygiene). + +Covers: +- 2.1: directory-rule match prunes the walk + emits one aggregate entry; + IGNORE-surface match prunes with zero emission; file-rule match attaches + a lifecycle signal alongside existing objective signals; unmatched files + flow through unchanged. +- 2.2: temporary-tier age (git commit time, mtime fallback for untracked, + one-stat directory-inode mtime for untracked dirs); retain-recent-N + grouping/ranking by rule match entry. +- 2.3: delete-once-served — served_when_path substitution + filesystem + check (deterministic, scanner-proven); served_when free text is + classifier-judged only, scanner asserts nothing about satisfaction. + +sys.path manipulation is intentional per file-ownership constraints (no +shared conftest). +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +# Prepend scripts/ to sys.path so we can import scanner/rulebook without a package +_SCRIPTS_DIR = Path(__file__).parent.parent / "scripts" +if str(_SCRIPTS_DIR) not in sys.path: + sys.path.insert(0, str(_SCRIPTS_DIR)) + +from rulebook import RuleMatch # noqa: E402 +from scanner import Scanner # noqa: E402 + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _make_tree(tmp: Path, files: dict) -> None: + for rel, content in files.items(): + p = tmp / rel + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(content, encoding="utf-8") + + +def _rule(glob, lifetime="temporary", origin="global", **extra) -> dict: + r = {"glob": glob, "confirmed_by": "human", "confirmed_on": "2026-07-14", "source": "test"} + if lifetime is not None: + r["lifetime"] = lifetime + r.update(extra) + return r + + +class _FakeRulebook: + """A minimal rulebook stand-in: one canned RuleMatch per glob->config. + + Each registered entry is `(is_dir_only, matcher_fn, level, is_ignore, rule)`. + `matcher_fn(path, is_dir)` decides whether the entry applies. This keeps + scanner tests fully isolated from rulebook.py's glob-compilation + internals (already covered by test_rulebook.py) while exercising the + scanner's *consumption* of RuleMatch objects. + """ + + def __init__(self): + self._entries = [] # list of (matcher_fn, level, is_ignore, rule, origin) + + def register(self, matcher_fn, level, rule, is_ignore=False, origin="global"): + self._entries.append((matcher_fn, level, is_ignore, rule, origin)) + + def query(self, path, is_dir=False): + norm = path.strip("/") + for matcher_fn, level, is_ignore, rule, origin in self._entries: + if matcher_fn(norm, is_dir): + return RuleMatch(rule=rule, level=level, is_ignore=is_ignore, origin=origin) + return None + + +def _run(root: Path, rulebook=None, git_commit_time_fn=None, now_fn=None, **kwargs) -> dict: + return Scanner( + root=root, + git_log_fn=lambda p: [], + now_fn=now_fn or (lambda: 0.0), + rulebook=rulebook, + git_commit_time_fn=git_commit_time_fn, + **kwargs, + ).run() + + +# --------------------------------------------------------------------------- +# 2.1: directory-rule prune + aggregate entry +# --------------------------------------------------------------------------- + +class TestDirectoryRulePrune: + def test_directory_rule_prunes_walk_and_emits_one_aggregate_entry(self, tmp_path): + _make_tree(tmp_path, { + "autoresearch/run-2026-07-01/a.md": "content a", + "autoresearch/run-2026-07-01/b.md": "content b", + "autoresearch/run-2026-07-01/nested/c.md": "content c", + "docs/live.md": "live content", + }) + rule = _rule("autoresearch/*/**", lifetime="temporary") + rb = _FakeRulebook() + rb.register( + lambda p, is_dir: is_dir and p == "autoresearch/run-2026-07-01", + "directory", + rule, + ) + + result = _run(tmp_path, rulebook=rb) + + # No file beneath the pruned directory is opened/shortlisted. + for path in result["shortlist"]: + assert not path.startswith("autoresearch/run-2026-07-01/"), path + + # Exactly one aggregate entry for the directory path itself. + assert "autoresearch/run-2026-07-01" in result["shortlist"] + assert result["shortlist"].count("autoresearch/run-2026-07-01") == 1 + + sigs = result["signals"]["autoresearch/run-2026-07-01"] + lifecycle_sigs = [s for s in sigs if s["name"] == "lifecycle"] + assert len(lifecycle_sigs) == 1 + assert lifecycle_sigs[0]["lifetime"] == "temporary" + assert lifecycle_sigs[0]["rule_ref"] == "autoresearch/*/**" + + # Unrelated file flows through unchanged. + assert "docs/live.md" in result["shortlist"] + assert "docs/live.md" not in result["signals"] or all( + s["name"] != "lifecycle" for s in result["signals"]["docs/live.md"] + ) + + def test_ignore_surface_prunes_with_zero_emission(self, tmp_path): + _make_tree(tmp_path, { + "graphify-out/graph.json": "{}", + "graphify-out/nested/index.md": "index", + "docs/live.md": "live content", + }) + ignore_rule = _rule("graphify-out/**", lifetime=None) + rb = _FakeRulebook() + rb.register( + lambda p, is_dir: is_dir and p == "graphify-out", + "directory", + ignore_rule, + is_ignore=True, + ) + + result = _run(tmp_path, rulebook=rb) + + for path in result["shortlist"]: + assert not path.startswith("graphify-out"), path + assert "graphify-out" not in result["shortlist"] + assert "graphify-out" not in result["signals"] + + def test_file_rule_attaches_lifecycle_signal_alongside_existing_signals(self, tmp_path): + _make_tree(tmp_path, { + "HANDOFF-2026-07-01.md": "[broken](./missing.md)", + }) + rule = _rule( + "HANDOFF-2026-07-01.md", lifetime="delete-once-served", + served_when="the handoff has been read and actioned", + ) + rb = _FakeRulebook() + rb.register( + lambda p, is_dir: (not is_dir) and p == "HANDOFF-2026-07-01.md", + "file", + rule, + ) + + result = _run(tmp_path, rulebook=rb) + + sigs = result["signals"]["HANDOFF-2026-07-01.md"] + names = {s["name"] for s in sigs} + assert "lifecycle" in names + assert "broken_reference" in names + + def test_unmatched_files_flow_through_unchanged(self, tmp_path): + _make_tree(tmp_path, {"docs/plain.md": "plain content, nothing special"}) + rb = _FakeRulebook() # no registered rules -> always None + + result = _run(tmp_path, rulebook=rb) + + assert "docs/plain.md" in result["shortlist"] + assert "docs/plain.md" not in result["signals"] + + +# --------------------------------------------------------------------------- +# 2.2: temporary-tier age + retain-recent-N +# --------------------------------------------------------------------------- + +class TestTemporaryTierAge: + def test_tracked_file_age_from_git_commit_time(self, tmp_path): + _make_tree(tmp_path, {"tmp/a.md": "content"}) + rule = _rule("tmp/a.md", lifetime="temporary", max_age_days=3, retain_recent=3) + rb = _FakeRulebook() + rb.register(lambda p, is_dir: (not is_dir) and p == "tmp/a.md", "file", rule) + + # now = 10 days after commit -> age_days == 10 + commit_time = "2026-06-24T00:00:00+00:00" + now_ts = __import__("datetime").datetime.fromisoformat(commit_time).timestamp() + 10 * 86400 + + result = _run( + tmp_path, + rulebook=rb, + git_commit_time_fn=lambda p: commit_time, + now_fn=lambda: now_ts, + ) + sig = [s for s in result["signals"]["tmp/a.md"] if s["name"] == "lifecycle"][0] + assert abs(sig["age_days"] - 10.0) < 0.01 + + def test_untracked_file_age_falls_back_to_mtime(self, tmp_path): + _make_tree(tmp_path, {"tmp/b.md": "content"}) + rule = _rule("tmp/b.md", lifetime="temporary") + rb = _FakeRulebook() + rb.register(lambda p, is_dir: (not is_dir) and p == "tmp/b.md", "file", rule) + + result = _run( + tmp_path, + rulebook=rb, + git_commit_time_fn=lambda p: None, # untracked + now_fn=lambda: (tmp_path / "tmp/b.md").stat().st_mtime + 5 * 86400, + ) + sig = [s for s in result["signals"]["tmp/b.md"] if s["name"] == "lifecycle"][0] + assert abs(sig["age_days"] - 5.0) < 0.01 + + def test_untracked_directory_age_is_one_stat_not_recursive(self, tmp_path): + _make_tree(tmp_path, { + "autoresearch/run-x/old_nested_file.md": "old", + }) + # Make the nested file's mtime much older than the directory's own + # mtime, to prove the directory's own inode mtime is what's used. + import os + nested = tmp_path / "autoresearch/run-x/old_nested_file.md" + old_time = 1_000_000.0 + os.utime(nested, (old_time, old_time)) + + dir_path = tmp_path / "autoresearch/run-x" + dir_mtime = dir_path.stat().st_mtime + + rule = _rule("autoresearch/*/**", lifetime="temporary") + rb = _FakeRulebook() + rb.register( + lambda p, is_dir: is_dir and p == "autoresearch/run-x", "directory", rule + ) + + result = _run( + tmp_path, + rulebook=rb, + git_commit_time_fn=lambda p: None, + now_fn=lambda: dir_mtime + 2 * 86400, + ) + sig = [s for s in result["signals"]["autoresearch/run-x"] if s["name"] == "lifecycle"][0] + # Age reflects the directory's own mtime (~2 days), not the ancient + # nested file's mtime (which would be a huge age if it were used). + assert abs(sig["age_days"] - 2.0) < 0.01 + + def test_retain_recent_n_keeps_newest_3_regardless_of_age(self, tmp_path): + _make_tree(tmp_path, { + "autoresearch/run-1/x.md": "1", + "autoresearch/run-2/x.md": "2", + "autoresearch/run-3/x.md": "3", + "autoresearch/run-4/x.md": "4", + "autoresearch/run-5/x.md": "5", + }) + rule = _rule("autoresearch/*/**", lifetime="temporary", retain_recent=3, max_age_days=3) + rb = _FakeRulebook() + + def matcher(p, is_dir): + return is_dir and p.startswith("autoresearch/run-") + + rb.register(matcher, "directory", rule) + + # Ages: run-1 oldest (10d) ... run-5 newest (1d). + ages = { + "autoresearch/run-1": 10.0, + "autoresearch/run-2": 8.0, + "autoresearch/run-3": 6.0, + "autoresearch/run-4": 4.0, + "autoresearch/run-5": 1.0, + } + + def commit_time_fn(abs_path): + return None # force mtime fallback, patched via now_fn/mtime below + + # Use mtime fallback: set directory mtimes so that now - mtime == age. + import os + now_ts = 2_000_000.0 + for rel, age in ages.items(): + os.utime(tmp_path / rel, (now_ts - age * 86400, now_ts - age * 86400)) + + result = _run( + tmp_path, + rulebook=rb, + git_commit_time_fn=commit_time_fn, + now_fn=lambda: now_ts, + ) + + retention_by_dir = { + path: [s for s in sigs if s["name"] == "lifecycle"][0]["retention"] + for path, sigs in result["signals"].items() + if path.startswith("autoresearch/run-") + } + + # Newest 3 (run-3, run-4, run-5) always kept regardless of age. + assert retention_by_dir["autoresearch/run-5"]["kept"] is True + assert retention_by_dir["autoresearch/run-4"]["kept"] is True + assert retention_by_dir["autoresearch/run-3"]["kept"] is True + + # Ranked 4th and 5th (run-2, run-1) are older than max_age_days=3 -> + # deletable. + assert retention_by_dir["autoresearch/run-2"]["kept"] is False + assert retention_by_dir["autoresearch/run-2"]["deletable"] is True + assert retention_by_dir["autoresearch/run-1"]["kept"] is False + assert retention_by_dir["autoresearch/run-1"]["deletable"] is True + + def test_4th_ranked_entry_younger_than_max_age_is_not_deletable(self, tmp_path): + _make_tree(tmp_path, { + "autoresearch/run-1/x.md": "1", + "autoresearch/run-2/x.md": "2", + "autoresearch/run-3/x.md": "3", + "autoresearch/run-4/x.md": "4", + }) + rule = _rule("autoresearch/*/**", lifetime="temporary", retain_recent=3, max_age_days=3) + rb = _FakeRulebook() + rb.register(lambda p, is_dir: is_dir and p.startswith("autoresearch/run-"), "directory", rule) + + import os + now_ts = 2_000_000.0 + ages = { + "autoresearch/run-1": 1.0, # 4th-ranked (oldest), but younger than max_age_days + "autoresearch/run-2": 0.8, + "autoresearch/run-3": 0.5, + "autoresearch/run-4": 0.1, + } + for rel, age in ages.items(): + os.utime(tmp_path / rel, (now_ts - age * 86400, now_ts - age * 86400)) + + result = _run( + tmp_path, rulebook=rb, git_commit_time_fn=lambda p: None, now_fn=lambda: now_ts + ) + retention_by_dir = { + path: [s for s in sigs if s["name"] == "lifecycle"][0]["retention"] + for path, sigs in result["signals"].items() + if path.startswith("autoresearch/run-") + } + assert retention_by_dir["autoresearch/run-1"]["kept"] is False + assert retention_by_dir["autoresearch/run-1"]["deletable"] is False + + +# --------------------------------------------------------------------------- +# 2.3: delete-once-served +# --------------------------------------------------------------------------- + +class TestDeleteOnceServed: + def test_served_when_path_substitution_and_satisfied(self, tmp_path): + _make_tree(tmp_path, { + "autoresearch/run-2026-07-01/report.md": "report", + "autoresearch/run-2026-07-01/archive/report.md": "archived copy", + }) + rule = _rule( + "autoresearch/*/report.md", + lifetime="delete-once-served", + served_when_path="autoresearch/run-2026-07-01/archive/{name}", + ) + rb = _FakeRulebook() + rb.register( + lambda p, is_dir: (not is_dir) and p == "autoresearch/run-2026-07-01/report.md", + "file", + rule, + ) + + result = _run(tmp_path, rulebook=rb) + sig = [ + s for s in result["signals"]["autoresearch/run-2026-07-01/report.md"] + if s["name"] == "lifecycle" + ][0] + assert sig["served"]["kind"] == "scanner-proven" + assert sig["served"]["resolved_path"] == "autoresearch/run-2026-07-01/archive/report.md" + assert sig["served"]["satisfied"] is True + + def test_served_when_path_not_satisfied(self, tmp_path): + _make_tree(tmp_path, { + "autoresearch/run-2026-07-01/report.md": "report", + }) + rule = _rule( + "autoresearch/*/report.md", + lifetime="delete-once-served", + served_when_path="autoresearch/run-2026-07-01/archive/{name}", + ) + rb = _FakeRulebook() + rb.register( + lambda p, is_dir: (not is_dir) and p == "autoresearch/run-2026-07-01/report.md", + "file", + rule, + ) + + result = _run(tmp_path, rulebook=rb) + sig = [ + s for s in result["signals"]["autoresearch/run-2026-07-01/report.md"] + if s["name"] == "lifecycle" + ][0] + assert sig["served"]["kind"] == "scanner-proven" + assert sig["served"]["satisfied"] is False + + def test_served_when_free_text_is_classifier_judged_only(self, tmp_path): + _make_tree(tmp_path, {"HANDOFF-2026-07-01.md": "handoff content"}) + rule = _rule( + "HANDOFF-2026-07-01.md", + lifetime="delete-once-served", + served_when="the handoff has been read and actioned by the team", + ) + rb = _FakeRulebook() + rb.register( + lambda p, is_dir: (not is_dir) and p == "HANDOFF-2026-07-01.md", "file", rule + ) + + result = _run(tmp_path, rulebook=rb) + sig = [ + s for s in result["signals"]["HANDOFF-2026-07-01.md"] if s["name"] == "lifecycle" + ][0] + assert sig["served"]["kind"] == "classifier-judged" + assert sig["served"]["served_when"] == "the handoff has been read and actioned by the team" + # Scanner asserts nothing about satisfaction for classifier-judged signals. + assert "satisfied" not in sig["served"] diff --git a/plugins/os-doc-hygiene/tests/test_state_store.py b/plugins/os-doc-hygiene/tests/test_state_store.py index 6a3c07c..b69b039 100644 --- a/plugins/os-doc-hygiene/tests/test_state_store.py +++ b/plugins/os-doc-hygiene/tests/test_state_store.py @@ -129,15 +129,26 @@ class TestWriteConfinement: f"Write escaped .cc-os/dochygiene/: {p}" ) - def test_no_global_index_created(self, tmp_path): - """Operating on one project must not create anything in ~/ or /tmp.""" - (tmp_path / ".git").mkdir() - store = StateStore(project_root=tmp_path) + def test_no_global_index_created(self, tmp_path, monkeypatch): + """Operating on one project must not create anything in ~/ or /tmp. + + HOME is redirected to an empty temp dir: asserting on the user's real + home is flaky — other tools (e.g. os-backlog's projects registry) + legitimately create ~/.cc-os outside this plugin's control. + """ + fake_home = tmp_path / "home" + fake_home.mkdir() + monkeypatch.setenv("HOME", str(fake_home)) + + project = tmp_path / "project" + project.mkdir() + (project / ".git").mkdir() + store = StateStore(project_root=project) store.set_last_check() store.write_report("{}", "md") - home_dh = Path.home() / ".dochygiene" - home_cc_os = Path.home() / ".cc-os" + home_dh = fake_home / ".dochygiene" + home_cc_os = fake_home / ".cc-os" assert not home_dh.exists(), "Global ~/.dochygiene must not be created" assert not home_cc_os.exists(), "Global ~/.cc-os must not be created" diff --git a/plugins/os-doc-hygiene/tests/test_validate_report.py b/plugins/os-doc-hygiene/tests/test_validate_report.py index 1015f84..b395b82 100644 --- a/plugins/os-doc-hygiene/tests/test_validate_report.py +++ b/plugins/os-doc-hygiene/tests/test_validate_report.py @@ -67,6 +67,7 @@ def _minimal_valid_report() -> dict: "token_estimate": {"raw_tokens": 100}, } ], + "promotion_candidates": [], } @@ -227,6 +228,214 @@ class TestCategoryEnum: assert any(v["field"].endswith("category.subtype") for v in violations) +class TestLifecycleKinds: + """Group 3 (task 3.1): delete/extract-then-delete kinds + the lifecycle + object + the ADR-0039 tier branch in derive_safety_tier.""" + + def _lifecycle_delete_entry(self, **lifecycle_overrides) -> dict: + lifecycle = { + "rule_ref": "autoresearch/*/**", + "lifetime": "temporary", + "git_state": {"tracked": True, "clean": True}, + } + lifecycle.update(lifecycle_overrides) + return { + "path": "autoresearch/run-1/notes.md", + "category": {"class": "stale", "subtype": "orphaned"}, + "signals": [], + "op": "Delete expired temporary run.", + "op_type": "deterministic", + "is_destructive": True, + "is_reversible": True, + "safety_tier": "auto", + "exact_edit": { + "kind": "delete", + "anchor": {"start_line": 1, "end_line": 10}, + "expected_sha256": "a" * 64, + "lifecycle": lifecycle, + }, + "token_estimate": {"raw_tokens": 10}, + } + + def _base_report(self, entry: dict) -> dict: + r = _minimal_valid_report() + r["shortlist"] = [entry["path"]] + r["entries"] = [entry] + return r + + # -- kind enum + required sub-fields ----------------------------------- + + def test_delete_kind_is_accepted(self): + r = self._base_report(self._lifecycle_delete_entry()) + assert _validate(r) == [] + + def test_extract_then_delete_requires_extraction_target(self): + entry = self._lifecycle_delete_entry() + entry["exact_edit"]["kind"] = "extract-then-delete" + r = self._base_report(entry) + violations = _validate(r) + assert any(v["field"].endswith("extraction_target") for v in violations) + + def test_extract_then_delete_repo_durable_requires_dest(self): + entry = self._lifecycle_delete_entry() + entry["exact_edit"]["kind"] = "extract-then-delete" + entry["exact_edit"]["extraction_target"] = "repo-durable" + r = self._base_report(entry) + violations = _validate(r) + assert any(v["field"].endswith("extraction_dest") for v in violations) + + def test_extract_then_delete_repo_durable_with_dest_passes(self): + entry = self._lifecycle_delete_entry() + entry["exact_edit"]["kind"] = "extract-then-delete" + entry["exact_edit"]["extraction_target"] = "repo-durable" + entry["exact_edit"]["extraction_dest"] = "docs/adr/0099-extracted.md" + r = self._base_report(entry) + assert _validate(r) == [] + + def test_extract_then_delete_cross_repo_needs_no_dest(self): + entry = self._lifecycle_delete_entry() + entry["exact_edit"]["kind"] = "extract-then-delete" + entry["exact_edit"]["extraction_target"] = "cross-repo" + r = self._base_report(entry) + assert _validate(r) == [] + + def test_missing_lifecycle_object_is_rejected(self): + entry = self._lifecycle_delete_entry() + del entry["exact_edit"]["lifecycle"] + r = self._base_report(entry) + violations = _validate(r) + assert any(v["field"].endswith("exact_edit.lifecycle") for v in violations) + + def test_both_served_fields_is_rejected(self): + entry = self._lifecycle_delete_entry( + lifetime="delete-once-served", + served_when_path="docs/plans/archive/{name}", + served_when="the plan shipped", + ) + entry["exact_edit"]["safety_tier_note"] = None # no-op, keep shape + r = self._base_report(entry) + violations = _validate(r) + assert any("exactly one" in v["message"] for v in violations) + + def test_neither_served_field_on_delete_once_served_is_rejected(self): + entry = self._lifecycle_delete_entry(lifetime="delete-once-served") + r = self._base_report(entry) + violations = _validate(r) + assert any("exactly one" in v["message"] for v in violations) + + def test_temporary_with_served_field_is_rejected(self): + entry = self._lifecycle_delete_entry(served_when_path="x/archive/{name}") + r = self._base_report(entry) + violations = _validate(r) + assert any("not carry served_when_path" in v["message"] for v in violations) + + def test_missing_git_state_is_rejected(self): + entry = self._lifecycle_delete_entry() + del entry["exact_edit"]["lifecycle"]["git_state"] + r = self._base_report(entry) + violations = _validate(r) + assert any(v["field"].endswith("git_state") for v in violations) + + # -- tier derivation branch ---------------------------------------------- + + def test_scanner_proven_tracked_clean_derives_auto(self): + entry = self._lifecycle_delete_entry() # temporary + tracked+clean + r = self._base_report(entry) + assert _validate(r) == [] + assert entry["safety_tier"] == "auto" + + def test_scanner_proven_dirty_forces_confirm(self): + entry = self._lifecycle_delete_entry(git_state={"tracked": True, "clean": False}) + entry["safety_tier"] = "auto" # wrong: dirty forces confirm + r = self._base_report(entry) + violations = _validate(r) + assert any(v["field"].endswith("safety_tier") and "mismatch" in v["message"] for v in violations) + + def test_scanner_proven_untracked_forces_confirm(self): + entry = self._lifecycle_delete_entry(git_state={"tracked": False, "clean": False}) + entry["safety_tier"] = "auto" + r = self._base_report(entry) + violations = _validate(r) + assert any(v["field"].endswith("safety_tier") and "mismatch" in v["message"] for v in violations) + + def test_classifier_judged_served_when_always_confirm_even_tracked_clean(self): + entry = self._lifecycle_delete_entry( + lifetime="delete-once-served", + served_when="the effort this plan describes has shipped", + git_state={"tracked": True, "clean": True}, + ) + entry["safety_tier"] = "auto" # wrong: classifier-judged always confirm + r = self._base_report(entry) + violations = _validate(r) + assert any(v["field"].endswith("safety_tier") and "mismatch" in v["message"] for v in violations) + + def test_classifier_judged_served_when_confirm_is_accepted(self): + entry = self._lifecycle_delete_entry( + lifetime="delete-once-served", + served_when="the effort this plan describes has shipped", + git_state={"tracked": True, "clean": True}, + ) + entry["safety_tier"] = "confirm" + r = self._base_report(entry) + assert _validate(r) == [] + + def test_served_when_path_tracked_clean_derives_auto(self): + entry = self._lifecycle_delete_entry( + lifetime="delete-once-served", + served_when_path="openspec/changes/archive/{id}/", + git_state={"tracked": True, "clean": True}, + ) + r = self._base_report(entry) + assert _validate(r) == [] + assert entry["safety_tier"] == "auto" + + def test_non_lifecycle_branches_unchanged(self): + """The pre-existing non-lifecycle truth table is untouched.""" + assert derive_safety_tier("generative", False, True) == "confirm" + assert derive_safety_tier("deterministic", True, True) == "confirm" + assert derive_safety_tier("deterministic", False, False) == "confirm" + assert derive_safety_tier("deterministic", False, True) == "auto" + + def test_derive_safety_tier_signature_accepts_lifecycle_kw(self): + assert derive_safety_tier( + "deterministic", True, True, + lifecycle={"lifetime": "temporary", "git_state": {"tracked": True, "clean": True}}, + ) == "auto" + + +class TestPromotionCandidatesShape: + + def test_envelope_requires_promotion_candidates(self): + r = _minimal_valid_report() + del r["promotion_candidates"] + violations = _validate(r) + assert any(v["field"] == "promotion_candidates" for v in violations) + + def test_empty_promotion_candidates_is_valid(self): + r = _minimal_valid_report() + r["promotion_candidates"] = [] + assert _validate(r) == [] + + def test_well_formed_candidate_is_valid(self): + r = _minimal_valid_report() + r["promotion_candidates"] = [ + {"path": "docs/plans/x.md", "name": "archive-bucket", "pitch": "Move it to archive/ once."} + ] + assert _validate(r) == [] + + def test_candidate_missing_field_is_rejected(self): + r = _minimal_valid_report() + r["promotion_candidates"] = [{"path": "docs/plans/x.md", "name": "archive-bucket"}] + violations = _validate(r) + assert any(v["field"].endswith(".pitch") for v in violations) + + def test_promotion_candidates_must_be_a_list(self): + r = _minimal_valid_report() + r["promotion_candidates"] = {"not": "a list"} + violations = _validate(r) + assert any(v["field"] == "promotion_candidates" for v in violations) + + class TestEnvelope: def test_missing_top_level_field_rejected(self):