From d157922fca28d94de78ca9b8d7421a7b7051b703 Mon Sep 17 00:00:00 2001 From: Anish Lakhwara Date: Wed, 6 May 2026 22:04:43 -0700 Subject: [PATCH] docs(audit): reframe scope from dogfooding to Obsidian-compat tightening MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The audit's original framing was that every built-in plugin must be re-implementable through documented Obsidian public API. After 23 issues closed, an adversarial verification pass found that ~38% of recently- RESOLVED issues left tail leakage or perpetuated the patterns the audit was supposed to fix. Root cause: dogfooding is structurally impossible without inventing a parallel Reams API wider than Obsidian's, and implementers were doing exactly that — tightening the obvious sham then bolting on whatever Reams-side mechanism they needed alongside, unprefixed and undocumented. Reframed scope: tightening for Obsidian-compat + code-health discipline. Built-ins are allowed to be privileged (they're not external plugins). Every public-API addition is Obsidian-shape, in the Reams namespace and documented, OR @internal-with-justification. No fourth bucket. Reams namespace itself moves to a separate epic; honest framing in docs that it's a labeling system + CI-enforced gate, not a containment boundary. CJS plugins are silently un-gated; documented. Audit-flow skill changes: - Reframed opening section - New Step 4.5: API-surface enumeration before merge (every addition must be classified Obsidian-shape / Reams-namespace / @internal-with- justification or it doesn't merge) - Strengthened adversary prompt to require API-surface accounting - Mutation gate moved to on-request only (was routine; the 25-40min Stryker runs were slowing well-scoped tightening work without much marginal value) floor-map.json marked DEPRECATED as a gate; preserved as historical record. --- .agents/skills/audit-flow/SKILL.md | 64 +++++++++++++++++++++++++----- .audit-flow/floor-map.json | 1 + AGENTS.md | 8 +++- docs/audits/core-services-audit.md | 30 ++++++++++++++ 4 files changed, 91 insertions(+), 12 deletions(-) diff --git a/.agents/skills/audit-flow/SKILL.md b/.agents/skills/audit-flow/SKILL.md index fdc00974..9b6c1ac9 100644 --- a/.agents/skills/audit-flow/SKILL.md +++ b/.agents/skills/audit-flow/SKILL.md @@ -9,6 +9,13 @@ A focused, single-issue workflow for closing audit-epic-#293 chainlink issues. T This skill replaces the previous heavyweight audit-flow ceremony. There is no plan-edit loop, no characterization-test pre-flight, no dep-map subagent. The plan in the chainlink issue is the plan. +**Scope reframed 2026-05-06.** The audit is **tightening for Obsidian-compat + code-health discipline**, not dogfooding. Every public-API addition must be: +- **Obsidian-shape** — matches a documented Obsidian method, signature, event, or property. +- **Reams-namespace** — lives in the `'reams'` virtual module or under `app.reams*`-prefixed names; documented in `PLUGIN-AUTHOR-GUIDE.md`. +- **`@internal`-with-justification** — the JSDoc/code comment explains why this is host-only and which built-ins consume it. + +There is no fourth bucket. Reams-novel surface that's not in the namespace and not `@internal` is a regression against the audit goal. See `docs/audits/core-services-audit.md` "Scope reframed" header for context. Reams-novel API decisions belong under the namespace epic, not the audit. + ## When to use Use audit-flow when: @@ -22,15 +29,15 @@ Use trycycle (not audit-flow) for new features, bug fixes outside the audit, or These are non-negotiable. They live here, not in AGENTS.md, because they are audit-flow-specific defaults: -1. **Stryker MUST run from the worktree, never from main repo with file copying.** A pre-audit session in May 2026 burned ~2 hours after copying test files between trees and silently measuring stale tests; scores were wrong by 30+ percentage points. The mechanical fix: `cd .worktrees/ && nix develop --command npx stryker run`. Symptoms of regression: scores drop sharply between sessions for files no one touched; `wc -l` differs between worktree and main test files. +1. **Close the chainlink issue AFTER merging to main, NOT in the worktree.** Chainlink uses sqlite3 which does not play nicely with worktrees — close-in-worktree often appears to succeed but the db change is silently lost during merge. The reliable sequence is: merge → switch to main → run `chainlink issue close ` → commit `.chainlink/issues.db`. -2. **Close the chainlink issue AFTER merging to main, NOT in the worktree.** Chainlink uses sqlite3 which does not play nicely with worktrees — close-in-worktree often appears to succeed but the db change is silently lost during merge. The reliable sequence is: merge → switch to main → run `chainlink issue close ` → commit `.chainlink/issues.db`. +2. **/handoff at the very end** with a 2-paragraph summary referencing the closed issue and audit findings flipped to RESOLVED. Without /handoff the next session starts blind. -3. **/handoff at the very end** with a 2-paragraph summary referencing the closed issue and audit findings flipped to RESOLVED. Without /handoff the next session starts blind. +3. **Do not defer scope.** You are Claude. You work faster than any human. Complete the work to a high standard. Surface follow-up issues as new chainlink subissues only if they are genuinely out of scope (different file, different concept), not because the work is large. -4. **Do not defer scope.** You are Claude. You work faster than any human. Complete the work to a high standard. Surface follow-up issues as new chainlink subissues only if they are genuinely out of scope (different file, different concept), not because the work is large. +4. **No `--no-verify` on commits, no force pushes, no `git config` changes, no `--amend` on already-pushed commits.** -5. **No `--no-verify` on commits, no force pushes, no `git config` changes, no `--amend` on already-pushed commits.** +5. **Stryker mutation gate is on-request only.** It is no longer routine. Run only when (a) the issue's acceptance criteria explicitly call for it, (b) the orchestrator requests it, or (c) you are touching code with an existing baseline in `.audit-flow/floor-map.json` AND the orchestrator/user has approved the run for this issue. When you do run Stryker, it MUST run from the worktree (never the main repo with file copying — see the failure mode in step 5 below). Default verification is vitest green plus the API-surface enumeration in step 4.5. ## The cycle @@ -91,6 +98,17 @@ The loop, in the worktree: 1. Dispatch the `@adversary` agent (use `task` tool with `subagent_type: adversary`) and ask it to review the implementation. Specific prompts to include: - Does the implementation match the chainlink approach? - Does it satisfy every acceptance criterion? + - **What new public API surface did this introduce?** List every method, property, event, type, class, or constant added to: + - `src/plugin/obsidian-compat/index.ts` (ESM plugin barrel) + - `src/plugin/loader/plugin-api-global.ts` (CJS shim, `window.__REAMS_PLUGIN_API__`) + - The public type surface of `App`, `Workspace`, `Vault`, `MetadataCache`, `Plugin`, `Editor`, `EditorTransaction`, `CachedMetadata`, etc. + - The Reams namespace (`'reams'` virtual module or `app.reams*` prefixed properties). + - **For each addition, classify it:** + - **Obsidian-shape** (matches documented Obsidian) — fine. + - **Reams-namespace** (under `'reams'` module or `app.reams*` prefix, with documentation in `PLUGIN-AUTHOR-GUIDE.md`) — fine. + - **`@internal`-with-justification** (host-only, JSDoc explains why and which built-ins consume it) — fine. + - **Unprefixed Reams-novel** — REGRESSION. Stops the merge until reclassified or removed. + - Are wiring methods or host-internal mechanisms exposed without `@internal` markers? - Does it introduce hidden API or behavior drift? - Are there missing test cases for edge conditions, error paths, and concurrency? - Are any sham facades introduced or preserved? @@ -106,17 +124,43 @@ The loop, in the worktree: 6. If the adversary surfaces a finding that is genuinely a new chainlink-worthy issue (different scope), file it as a new chainlink subissue under #293 and continue rather than expanding scope here. -Only after the adversary is happy do you proceed to step 5 (mutation gate) and step 7 (merge). The loop happens entirely in the worktree, before any merge to main. +Only after the adversary is happy do you proceed to step 4.5 (API-surface enumeration) and step 7 (merge). The loop happens entirely in the worktree, before any merge to main. + +### 4.5. API-surface enumeration before merge + +Before merging, write down — in the chainlink issue comment or a commit message — every public API addition this issue introduced, classified as: + +- **Obsidian-shape** — symbol name, where it was added (file:line), and the matching Obsidian API method/property it mirrors. +- **Reams-namespace** — symbol name, where it was added, and how it's reachable from the namespace (`'reams'` module, `app.reams.X`, or `app.reamsX` prefix). +- **`@internal`-with-justification** — symbol name, where it was added, and a 1-2 sentence justification for being host-only. + +If any addition does not fit one of those three buckets, **do not merge**. Either: +- Refactor the addition to fit (typically: rename to Reams namespace, or add `@internal` marker). +- Remove the addition (the work might be re-doable through existing public API). +- Surface to the orchestrator/user as a Reams-novel API decision that needs explicit approval — in which case file a separate decision issue under the namespace epic, do not merge this one until the decision lands. + +Empty enumeration ("no public API surface added") is a valid result and common for behavior-only fixes. + +This step is not a substitute for the adversary review; it is a structured artifact that confirms the adversary's classifications and creates a paper trail for future audits. + +### 5. Mutation gate (on-request only) + +The mutation gate is no longer routine. Skip this step unless: +- The issue's acceptance criteria explicitly call for a Stryker run, OR +- The orchestrator/user requested it for this issue, OR +- You are touching code with an existing baseline in `.audit-flow/floor-map.json` AND the orchestrator/user has approved the run. -### 5. Mutation gate (conditional) +When skipped, default verification is vitest green plus the step 4.5 enumeration. -If the touched source file is in `.audit-flow/floor-map.json`, run the strict mutation gate. Otherwise skip. +When run: ```bash # Check whether the touched file has a baseline grep -F '"src/path/to/file.ts"' .audit-flow/floor-map.json -# If present: +# Run from the worktree, NEVER from main repo with file copying. +# A pre-audit session in May 2026 burned ~2 hours after copying test files +# between trees and silently measuring stale tests; scores were wrong by 30+ pp. nix develop --command npx stryker run --mutate 'src/path/to/file.ts' nix develop --command node ../../scripts/check-mutation-baseline.mjs \ --baseline ../../.audit-flow/floor-map.json \ @@ -126,7 +170,7 @@ nix develop --command node ../../scripts/check-mutation-baseline.mjs \ A full Stryker run on a typical file takes ~25-40 minutes. The script output tells you whether `survived` count regressed. If it did, add mutation-killing tests targeting the surviving mutants and re-run. -If the file is NOT in floor-map.json, vitest-only verification is sufficient. Do NOT establish a new baseline as part of this issue (that's separate work). +The floor-map.json baselines are kept as historical record. They are no longer a gate. Symptoms of regression (if you do run): scores drop sharply between sessions for files no one touched; `wc -l` differs between worktree and main test files. Both indicate Stryker-from-main-with-file-copying — fix by running from the worktree. ### 6. Update audit doc diff --git a/.audit-flow/floor-map.json b/.audit-flow/floor-map.json index fab740ec..6e7ca7be 100644 --- a/.audit-flow/floor-map.json +++ b/.audit-flow/floor-map.json @@ -3,6 +3,7 @@ "updated_at": "2026-05-06T15:13:01Z", "floor": 60, "schema_note": "v2 adds {killed, timed_out, survived, total_mutants} per file for absolute regression detection. Per adversary BLOCKER #6: percentage alone allows regressions to hide behind code shrinkage. CI / future audit work should fail on `survived` increasing, not just on percentage dropping.", + "gate_status": "DEPRECATED 2026-05-06 — this file is a historical record, not a gate. Per the audit reframe, mutation gate is on-request only. See `.agents/skills/audit-flow/SKILL.md` step 5. Existing baselines preserved; new audit work does not re-run Stryker per-issue. Refresh on request when files have shifted significantly (e.g. vault.ts post-#317 went 74.85 -> 100; app.ts post-#315 shifted; plugin.ts post-#318 shifted).", "verification_note": "All scores in this file were re-verified by running Stryker FROM THE WORKTREE (not the main repo) on 2026-05-04 03:38-04:10 UTC. Mutation reports are committed at reports/mutation/by-file/.json. Earlier runs done from the main repo with file-copying produced inconsistent results because some test files diverged between worktree and main; that workflow is deprecated.", "files": { "src/plugin/app.ts": { diff --git a/AGENTS.md b/AGENTS.md index f1b43bd1..a9a957fb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -79,11 +79,13 @@ Trycycle is an OpenCode skill that automates planning, building, and reviewing c Audit-flow is a repo-local skill at `.agents/skills/audit-flow/` for executing chainlink issues from the **core services audit** (parent epic #293, source of truth `docs/audits/core-services-audit.md`). +**Scope reframed 2026-05-06.** The audit is no longer about dogfooding (built-ins re-implementable as external plugins). It is about **tightening for Obsidian-compat + code-health discipline**. Every public-API addition must be Obsidian-shape, in the Reams namespace and documented, OR `@internal`-with-justification. Reams-novel surface lives under a separate namespace epic; see the audit doc's "Scope reframed" header for context. Built-ins are allowed to be privileged. + **Use audit-flow instead of trycycle when:** - Working on a chainlink issue under epic #293 or labeled `audit`. - The chainlink issue body already has a concrete approach + acceptance criteria (typical for audit issues — they were filed with implementation guidance baked in). -**Use trycycle for everything else** (new features, bug fixes outside the audit, anything where the plan needs adversarial design review). +**Use trycycle for everything else** (new features, bug fixes outside the audit, anything where the plan needs adversarial design review). Reams-novel API decisions belong under the namespace epic, not the audit. **To use audit-flow:** @@ -94,12 +96,14 @@ Audit-flow is a repo-local skill at `.agents/skills/audit-flow/` for executing c - Create a worktree. - Implement TDD via the `audit-flow-executing` subagent. - Loop with `@adversary` until clean. - - Run the mutation gate (only where a baseline exists in `.audit-flow/floor-map.json`). + - **Enumerate API-surface additions before merge** — every symbol added to plugin-facing barrels and class surfaces, classified as Obsidian-shape / Reams-namespace / `@internal`-with-justification. - Update the audit doc with RESOLVED markers. - Merge to main. - Close the chainlink issue **on main, after merge** (sqlite3-vs-worktree gotcha — closing in the worktree often fails silently). - Run /handoff with a 2-paragraph summary. +**Mutation gate is no longer routine.** Run on request only — when the issue's acceptance criteria explicitly call for it, when the orchestrator requests it, or when touching code with an existing baseline in `.audit-flow/floor-map.json`. Default verification is vitest green plus the API-surface enumeration. The floor-map baselines are kept as historical record. + **Key files:** - `.agents/skills/audit-flow/SKILL.md` — the cycle orchestration - `.agents/skills/audit-flow/subskills/audit-flow-executing/SKILL.md` — TDD implementation subagent diff --git a/docs/audits/core-services-audit.md b/docs/audits/core-services-audit.md index 44ae3448..e6083285 100644 --- a/docs/audits/core-services-audit.md +++ b/docs/audits/core-services-audit.md @@ -1,5 +1,35 @@ # Reams Core Services Audit +--- + +## Scope reframed 2026-05-06 + +The audit's original framing was **dogfooding**: every built-in plugin must be re-implementable through documented Obsidian public API. After 23 issues closed, an adversarial verification pass found that ~38% (8 of 21) of recently-RESOLVED issues left tail leakage or perpetuated the patterns the audit was supposed to fix. Root cause: the dogfooding goal is structurally impossible without inventing a parallel Reams API wider than Obsidian's. Implementers tightened the obvious sham then bolted on whatever Reams-side mechanism they needed alongside, unprefixed and undocumented. + +**The audit's scope is now: tightening for Obsidian-compat + code-health discipline.** + +- Where Obsidian has a documented API for X, Reams matches its shape exactly. Built-ins reaching behind X get refactored. +- Where Obsidian has no API for X but Reams needs one, Reams ships a documented Reams-namespaced API. No more sneaking unprefixed novel surface in. +- Where Reams has genuinely host-internal mechanism, it is `@internal` and documented as such. +- **Built-ins are allowed to be privileged.** They are not external plugins. The "make built-in re-implementable as external plugin" framing is dropped. + +**Decision principle for every public-API addition:** Obsidian-shape, OR in the Reams namespace and documented, OR `@internal`-with-justification. No fourth bucket. + +The Reams namespace itself lives under a separate epic (filed 2026-05-06; see chainlink for the active ID). Honest framing of what the namespace does: it is a **labeling system with CI-enforced gating**, not a containment boundary. Runtime methods exist regardless of imports; the namespace is for code review, type-checking discipline, and CI diff enforcement against accidental accretion. CJS plugins are silently un-gated; this is documented. + +**What changed in the open-issue backlog:** + +- The dogfooding-driven C-series work (move panels into plugin folders so external plugins could replicate) is dropped. +- E.1 / E.2 (#354, #355) shipped; the "real-plugin" framing is accepted but not extended further. +- Findings tagged `REACH-BEHIND` survive only when there's a clean Obsidian-shape or Reams-namespace fix. +- Findings tagged `SHAM-FACADE`, `API-GAP-OBSIDIAN-HAS-IT`, and signature-mismatch findings continue as before. +- The mutation gate is no longer per-issue; floor-map.json baselines remain as historical record only. + +**Worktrees dropped under the reframe:** +- `.worktrees/audit-313-harden-followup` — at adversary review round 14, the diff had widened to include partial reverts of merged audit work (#343, #344, #354, #355) plus +691 lines on `workspace.ts` for `openLinkText` hardening + a user-notice event lifecycle. Not safe to land as one merge. Real bugs round 14 caught get re-filed as standalone issues if they matter. + +--- + **Date:** 2026-05-03 **Method:** Six parallel adversarial subagents, each scoped to one subsystem, with the same prompt template: -- 2.51.2