principles

# Principles Audit Skill Systematic audit of codebase adherence to established software engineering principles. **References**: See `CLAUDE.md` for codebase patterns. See `/audit` for dead code and dependency health. **Context mode**: `context: fork` means this skill runs in an isolated subagent context. The agent gets a read-only snapshot of the codebase — it should never edit files during an audit. If findings require fixes, report them in the output and let the user decide when to act. --- ## Scope Lock (ENFORCED) **Audits are strictly read-only.** This is non-negotiable: - Do NOT use Edit, Write, or any file-modifying tool during an audit - Do NOT let sub-agents edit files — all agents must be read-only - If a finding requires a fix, describe it in the report with file:line references - Implementation is **gated behind explicit user approval** — the report must be delivered and reviewed before any fixes begin --- ## Activation ### Audit (read-only) | Trigger | Action | |---------|--------| | `/principles` | Full codebase principles audit | | `/principles [package]` | Targeted package audit | | `/principles --team` | Parallel team audit (worktree-isolated agents) | | `/principles solid` | SOLID-only audit | | `/principles data` | ACID + BASE + CAP audit | | `/principles arch` | EDA + ADR + C4 audit | ### Execute (after audit) | Trigger | Action | |---------|--------| | `fix latest` | Load the most recent report, implement all findings by priority | | `fix latest critical` | Load the most recent report, implement CRITICAL findings only | | `fix latest S1, EDA1` | Load the most recent report, implement specific findings | | `fix [date]` | Load report from a specific date (e.g., `fix 2026-04-02`) | | `fix [date] critical` | Specific report, CRITICAL findings only | | `fix [date] S1, EDA1` | Specific report, specific findings | | `fix critical` | Alias for `fix latest critical` | | `fix all` | Alias for `fix latest` | | `fix S1, EDA1` | Alias for `fix latest S1, EDA1` | --- ## Scope Confirmation (REQUIRED) Before starting analysis, echo back the scope to the user: ``` Principles audit scope: [package or "full codebase"] Mode: [single-agent | team] Principle groups: [all | subset if specified] Previous audit: [date or "none found"] Proceed? [y/n] ``` --- ## Audit Packages (Priority Order) 1. `packages/shared/src/` — domain modules (highest surface area) 2. `packages/extension/src/` — MV3 browser extension 3. `packages/app/src/` — PWA shell 4. `packages/api/src/` — Hono API server --- ## Part 0: Previous Findings Verification Check `.plans/audits/*-principles.md` for prior reports. Re-verify all Critical and High findings. Apply the same escalation rules as `/audit` (3+ cycles = escalate one level, 5+ cycles = flag as chronic in Executive Summary). --- ## Part 1: SOLID Principles ### S — Single Responsibility Principle For each module in `packages/shared/src/modules/`: - Does each module/class/function have exactly one reason to change? - Identify god-modules that mix domain logic with I/O, UI concerns, or transport — focus on *mixed concerns*, not raw line count (LOC thresholds are covered by `/audit` Part 4) - Check if React components in extension/app mix data fetching, business logic, and presentation - A 200-line file mixing I/O + domain logic is a worse SRP violation than a 400-line file with one clear responsibility ### O — Open/Closed Principle - Are modules extensible without modification? - Look for switch/if-else chains on type discriminators that would need editing to add a new variant - Check if the agent skill pipeline, flow board, and sync layer use plugin/strategy patterns or hardcoded branches - Flag enum-driven dispatch that requires source modification to add new cases ### L — Liskov Substitution Principle - Where interfaces or abstract types are used, can every implementation be substituted without breaking callers? - Check mock vs. live mode implementations (onchain, archive, session) for behavioral parity - Verify that `VITE_COOP_ONCHAIN_MODE=mock` and `VITE_COOP_ONCHAIN_MODE=live` are truly interchangeable at runtime ### I — Interface Segregation Principle - Are consumers forced to depend on interfaces larger than what they use? - Check `@coop/shared` barrel exports — do downstream packages import large surfaces when they only need a few symbols? - Look for "fat" type definitions that force consumers to implement or handle fields they never use ### D — Dependency Inversion Principle - Do high-level modules depend on abstractions or on concrete implementations? - Check if extension/app directly instantiate shared internals or go through factory/provider patterns - Verify that onchain, archive, and sync layers depend on abstractions, not concrete providers --- ## Part 2: Code Quality Principles ### DRY — Don't Repeat Yourself - Search for duplicated logic across packages (especially shared <-> extension, shared <-> app) - Check for repeated patterns in: error handling, state machine transitions, Yjs document access, Safe/onchain call construction, Dexie queries - Flag copy-pasted utility functions that should be consolidated - Note: some duplication across package boundaries is acceptable if it avoids tight coupling — call these out but classify as INFO, not violation ### KISS — Keep It Simple, Stupid - Identify over-engineered abstractions: wrapper classes that add no value, deeply nested generics, unnecessary indirection layers - Flag premature optimization (memoization without profiling evidence, complex caching where simple fetch-on-demand works) - Check for overly complex type gymnastics in TypeScript that could be simplified - Look for functions with cyclomatic complexity > 10 ### YAGNI — You Ain't Gonna Need It > **Dead code detection** (unused exports, files, deps) is handled by `/audit` Part 3 via `knip`. Do not duplicate that work here. Focus on *design-level* speculation. - Find speculative abstractions: interfaces with exactly one implementation that don't serve a mock/test boundary - Check for config options or parameters that are never varied in practice - Flag any "future-proofing" scaffolding that has no current consumer - Identify over-parameterized functions where most callers pass the same values ### SOC — Separation of Concerns > **Layer violations** (business logic in app/extension instead of shared, package .env files) are detected by `/audit` Part 4. Do not re-run those greps here. Focus on *concern leakage patterns* and cross-cutting concern design. - **Vertical**: Does each shared module own its domain without leaking into adjacent modules? Check for circular dependencies between modules - **Horizontal**: Are concerns cleanly separated within components? Look for hooks that mix data fetching, transformation, and side effects in a single function - **Cross-cutting**: How are logging, error handling, and auth threaded through? Are they scattered or channeled through a consistent pattern? --- ## Part 3: Architectural Principles ### EDA — Event-Driven Architecture - Map the current event/message flow: background worker <-> popup <-> sidepanel <-> content scripts - Check for tight coupling via direct function calls where events would be more appropriate - Evaluate Yjs awareness and CRDT update propagation — are observers properly cleaned up? Are there race conditions in event ordering? - Review the WebSocket/WebRTC signaling layer: is the event contract well-defined or ad-hoc? - Check for event handler leaks (addEventListener without removeEventListener, Yjs observers without cleanup) ### ADR — Architecture Decision Records - Check if significant architectural decisions are documented (in `docs/`, `.plans/`, or inline) - Identify undocumented decisions that should have ADRs: - Choice of Yjs over other CRDTs - Safe multisig as coop primitive - Passkey-first auth over wallet-extension-first - The agent skill pipeline design - Mock/live mode split - Browser extension as primary surface (vs. standalone app) - Local-first with explicit publish (vs. auto-sync) - For each gap, note what the decision was (as inferred from code) and why an ADR would help ### C4 Model — Architectural Clarity - **Context (L1)**: Is the system boundary clear? Can a new developer understand what Coop interacts with externally (blockchain, Filecoin, signaling server, TURN, browser APIs)? - **Containers (L2)**: Are the four packages (shared, extension, app, api) well-defined containers with clear responsibilities, or do boundaries blur? - **Components (L3)**: Within each container, are the major components identifiable and their interactions documented? - **Code (L4)**: At the code level, are module boundaries enforced (barrel exports, no deep imports)? --- ## Part 4: Data Principles ### ACID (where applicable) - **Atomicity**: Are multi-step Dexie/IndexedDB writes wrapped in transactions? Can a crash mid-write leave the local DB in an inconsistent state? - **Consistency**: Are Yjs compound CRDT operations atomic (transact blocks)? Are there partial updates that could violate invariants? - **Isolation**: Can concurrent tab/worker operations interfere with each other's data? - **Durability**: Is persisted data resilient to crash recovery? Check Dexie transaction boundaries and Yjs persistence provider configuration. ### BASE — Basically Available, Soft state, Eventually consistent - Review the Yjs sync model: how does the system handle conflicting edits across peers? - Check if the UI correctly reflects soft state (optimistic updates with rollback on conflict) - Identify any places where the code assumes strong consistency but the underlying system provides eventual consistency - Verify that the offline-first model gracefully handles stale reads and delayed writes ### CAP Theorem Positioning Document where the system sits on the CAP spectrum for each data path: | Data Path | Expected Position | Verify | |-----------|------------------|--------| | Local Dexie store | CP (consistent + partition-tolerant, single node) | No distributed conflict possible | | Yjs peer sync | AP (available + partition-tolerant, eventually consistent) | Check merge semantics | | Onchain state (Safe) | CP (consistent on-chain, unavailable during partitions) | Check how UI handles chain unavailability | | API/signaling layer | ? | Determine actual tradeoff | Flag any mismatch where code assumes a CAP property that the underlying system doesn't guarantee. --- ## Part 5: Self-Validation (REQUIRED before report) Before generating the final report, re-verify EVERY finding: 1. **Re-read** the flagged file at the cited line number 2. **Confirm** the code matches what you described in the finding 3. **Check context** — read 10 lines above/below for guards, comments, or patterns that invalidate the finding 4. **Assign confidence**: `HIGH` (verified in code) / `MEDIUM` (likely but context unclear) 5. **Drop findings below HIGH confidence** — only include verified findings --- ## Part 6: Report Generation Create at `.plans/audits/[date]-principles.md`: ```markdown # Principles Audit Report - [Date] ## Executive Summary - **Packages analyzed**: [list] - **Mode**: Single-agent | Team - **Principle groups audited**: [list] ### Scorecard | Principle | Score | Top Issue | Effort | |-----------|-------|-----------|--------| | S (SRP) | GREEN/YELLOW/RED | ... | S/M/L | | O (OCP) | ... | ... | ... | | L (LSP) | ... | ... | ... | | I (ISP) | ... | ... | ... | | D (DIP) | ... | ... | ... | | DRY | ... | ... | ... | | KISS | ... | ... | ... | | YAGNI | ... | ... | ... | | SOC | ... | ... | ... | | EDA | ... | ... | ... | | ADR | ... | ... | ... | | C4 | ... | ... | ... | | ACID | ... | ... | ... | | BASE | ... | ... | ... | | CAP | ... | ... | ... | --- ## Previous Findings Status _Tracked from: [previous audit date or "first audit"]_ | ID | Finding | Status | Notes | |----|---------|--------|-------| --- ## Findings by Principle ### SOLID #### S1. [Title] — [severity: CRITICAL | HIGH | MEDIUM | LOW] - **Principle**: SRP - **File**: `package/path/to/file.ts:line` - **Issue**: [Description] - **Evidence**: [Code snippet or reference] - **Recommendation**: [Actionable fix] ... ### Code Quality (DRY / KISS / YAGNI / SOC) ... ### Architecture (EDA / ADR / C4) ... ### Data (ACID / BASE / CAP) ... --- ## Priority Queue Top 10 highest-impact fixes across all principles, ordered by severity and effort: 1. **[Fix title]** — [Principle] — `file:line` — [Effort: S/M/L] 2. ... --- ## Trend (last N audits) | Principle | [date1] | [current] | |-----------|---------|-----------| | S (SRP) | ... | ... | | ... | ... | ... | --- ## Next Steps > **This audit is read-only.** To apply fixes, reply with: > - `fix critical` — address Critical findings only > - `fix all` — address all findings by priority > - `fix S1, D3` — address specific findings by ID ``` --- ## Part 6b: Chat Output (REQUIRED) After writing the report file, output a detailed summary to the user in chat. This is what they see — make it count. ### Format ````markdown ## Principles Audit — [Date] ### Scorecard | Principle | Score | Top Issue | |-----------|-------|-----------| | S (SRP) | GREEN/YELLOW/RED | One-line summary or "—" if green | | O (OCP) | ... | ... | | ... (all 15 principles) | ... | ... | ### Findings For EACH non-INFO finding, output a block like: #### [ID]. [Title] — [SEVERITY] **[Principle]** · `file:line` · Effort: [S/M/L] [2-3 sentence description of the issue and its impact]