docs(audit): add fix-areas section to Phase 0 + Phases 1-8 playbook
Phase 0 update: replaces the loose "quick wins" list with a structured Fix Areas section organised into impact tiers (A: high-impact README truth fixes, B: low-effort self-violation fixes, C: structural smells deferred to Phase 2, D: hygiene). Each task names the file, the change, the verification command, and the effort estimate. New playbook: docs/audit/phases-1-8-playbook.md — step-by-step acquisition-grade audit procedure for the remaining seven phases. Each phase has goal, inputs, procedure (with concrete commands), deliverable, acceptance criteria, and time estimate. Designed to be picked up independently from any phase.
This commit is contained in:
@@ -208,27 +208,123 @@ The post-rebrand state: all five handovers were rewritten by today's sweep (line
|
||||
|
||||
---
|
||||
|
||||
## 9. Quick wins surfaced (defer to Phase 1, but worth flagging)
|
||||
## 9. Fix areas — actionable tasks
|
||||
|
||||
1. **Add `description` to `crates/llm/Cargo.toml`** — 1-line fix; closes a self-imposed contribution-rule violation.
|
||||
2. **Update README stores list** — replace fiction with reality. See §7.
|
||||
3. **Remove or implement Moonshine claim** — README, line 165.
|
||||
4. **Update README Tauri commands list** — six modules undocumented. See §7.
|
||||
5. **Decide on `magnotia-cloud-providers`** — currently 80 LOC of a keystore that two consumers call. Either fold into `magnotia-core::keystore` (it's not provider-specific) or actually grow it. Right now it's a crate that doesn't earn its existence.
|
||||
Each task below is concrete: file, change, verification, effort. Pick any in any order; they don't depend on later phases. Items are grouped by impact tier.
|
||||
|
||||
### Tier A — High impact, do first
|
||||
|
||||
#### A1. README stores list is fiction
|
||||
- **File:** `README.md`, line 202
|
||||
- **Current:** `Reactive stores (src/lib/stores/page.svelte.ts): settings, profiles, tasks, history, taskLists, templates, page, toasts, preferences.`
|
||||
- **Reality:** stores are `page`, `preferences`, `profiles`, `toasts`, `focusTimer`, `llmStatus`, `nudgeBus`, `implementationIntentions`, `completionStats`, `speaker` — each in its own `*.svelte.ts` file under `src/lib/stores/`.
|
||||
- **Fix:** rewrite the bullet to enumerate the actual ten store files, and clarify that `page.svelte.ts` is the central app-state store (transcripts, profiles, taskLists, etc. live as fields on it).
|
||||
- **Verify:** `ls src/lib/stores/` matches the README list 1:1.
|
||||
- **Effort:** 10 min.
|
||||
|
||||
#### A2. Moonshine claim has no implementation
|
||||
- **File:** `README.md`, line 165 (`magnotia-core` row in the crate table)
|
||||
- **Current:** `model registry (Whisper + Parakeet + Moonshine entries)`
|
||||
- **Reality:** `crates/core/src/model_registry.rs` has 6 Whisper + 1 Parakeet entries. Zero Moonshine.
|
||||
- **Fix (pick one):**
|
||||
- (a) Remove the Moonshine reference from the README. Cheapest.
|
||||
- (b) Add a `// TODO(moonshine): not yet wired` constant in `model_registry.rs` and a roadmap entry under §Roadmap, so the claim is at least flagged as forthcoming.
|
||||
- **Verify:** `grep -ri moonshine crates/ src-tauri/ src/` returns no orphan references.
|
||||
- **Effort:** 5 min (option a) / 30 min (option b).
|
||||
|
||||
#### A3. Six Tauri command modules undocumented
|
||||
- **File:** `README.md`, lines 95-97 (Architecture diagram) and 175-195 (Tauri commands table)
|
||||
- **Missing:** `feedback`, `fs`, `intentions`, `nudges`, `rituals`, `tts`
|
||||
- **Fix:** add a one-line description for each in the §Tauri commands table; add the names to the Architecture-diagram bullet list.
|
||||
- **Verify:** `ls src-tauri/src/commands/*.rs | xargs basename -s .rs | sort` matches the README table 1:1 (excluding `mod`, `power`, `security`, which are utility modules — note that explicitly).
|
||||
- **Effort:** 20 min.
|
||||
|
||||
### Tier B — Low effort, removes self-violations
|
||||
|
||||
#### B1. `crates/llm/Cargo.toml` missing `description`
|
||||
- **File:** `crates/llm/Cargo.toml`
|
||||
- **Current:** `[package]` block has `name`, `version`, `edition` only.
|
||||
- **Reality:** README §Contributing line 362 declares this a hard rule. Self-violation.
|
||||
- **Fix:** add `description = "Local LLM engine for Magnotia (Qwen3 via llama-cpp-2). Cleanup, task extraction, content tags."` (or similar). Match the prose style of the other 8 crates' descriptions.
|
||||
- **Verify:** `for d in crates/*/Cargo.toml src-tauri/Cargo.toml; do grep -L "^description" "$d"; done` returns empty.
|
||||
- **Effort:** 2 min.
|
||||
|
||||
#### B2. Stale test-count claim
|
||||
- **File:** `README.md`, line 14
|
||||
- **Current:** `245 automated lib tests across 10 crates, all passing`
|
||||
- **Reality:** 287 tests total (220 lib + 67 src-tauri); 220 lib-only.
|
||||
- **Fix:** decide on a number that's automatable, not a snapshot. Either: (a) replace with `220+ lib tests across 9 library crates plus 67 Tauri-app tests`, or (b) drop the absolute number and say `comprehensive automated test floor — see CI for current count`.
|
||||
- **Verify:** `grep -rE '#\[(test|tokio::test)\]' crates/*/src/ | wc -l` matches whatever number you ship.
|
||||
- **Effort:** 5 min.
|
||||
|
||||
#### B3. Crate count claim ambiguity
|
||||
- **File:** `README.md`, line 14 ("10 crates")
|
||||
- **Reality:** 9 library crates + 1 Tauri app crate. The README's own crate table only documents 9.
|
||||
- **Fix:** say "9 library crates plus the Tauri app crate" — or just "9 library crates" and let the Tauri app stand separately, which matches the existing prose.
|
||||
- **Effort:** 2 min.
|
||||
|
||||
### Tier C — Structural smells (defer to Phase 2 but flag now)
|
||||
|
||||
#### C1. `magnotia-core` over-exports
|
||||
- **File(s):** `crates/core/src/lib.rs` and the modules it re-exports
|
||||
- **Symptom:** 104 public items in a "shared types" crate. High blast radius for any change.
|
||||
- **Fix (Phase 2 work, do not touch yet):** audit every `pub` item; demote anything not used outside the crate to `pub(crate)`. The expected outcome is a 30–60% reduction in public surface.
|
||||
- **Verify:** after the demotion pass, `cargo +nightly rustdoc` should still succeed and downstream crates should still compile without changes.
|
||||
- **Effort:** ~½ day (Phase 2 scope).
|
||||
|
||||
#### C2. `magnotia-storage::database.rs` is 2,534 lines
|
||||
- **File:** `crates/storage/src/database.rs`
|
||||
- **Symptom:** single file holds CRUD for transcripts, tasks, subtasks, profiles, profile-terms, settings, error log, FTS5. No internal module boundaries.
|
||||
- **Fix (Phase 2):** split by domain — `database/transcripts.rs`, `database/tasks.rs`, `database/profiles.rs`, etc. Keep the public re-export shape unchanged so callers don't move.
|
||||
- **Verify:** `cargo test -p magnotia-storage` still passes; no public-API changes.
|
||||
- **Effort:** ~2-4 hours.
|
||||
|
||||
#### C3. `SettingsPage.svelte` is 2,250 lines
|
||||
- **File:** `src/lib/pages/SettingsPage.svelte`
|
||||
- **Symptom:** HANDOVER.md already flags this; `SettingsGroup.svelte` was prepared but the seven-group split was deferred.
|
||||
- **Fix (Phase 2):** complete the planned restructure. Pick this up from HANDOVER.md §"9c — Settings (scaled down)".
|
||||
- **Effort:** ~½ day.
|
||||
|
||||
#### C4. `magnotia-cloud-providers` does not earn its existence
|
||||
- **Files:** `crates/cloud-providers/` (80 LOC across 2 files)
|
||||
- **Symptom:** crate contains an in-memory keystore with env-var fallback. Not "empty scaffolding" as the README says — but also not provider-specific. No HTTP code, no providers.
|
||||
- **Fix (decide, then act):**
|
||||
- (a) **Fold** into `magnotia-core::keystore` (preferred — it's a generic key store, nothing cloud-specific). Drop the crate. README §Architecture and the dependency graph simplify.
|
||||
- (b) **Grow** it: actually implement an OpenAI-compatible STT client and an Anthropic STT client, gated behind a `cloud-stt` feature flag. Earn the boundary.
|
||||
- **Verify (option a):** workspace builds with `cloud-providers` removed from `Cargo.toml` members; the two consumers (`commands/llm.rs` and wherever else) re-import from `magnotia-core::keystore`.
|
||||
- **Effort:** ~1 hour (option a) / multi-day (option b).
|
||||
|
||||
### Tier D — Hygiene (Phase 1 / Phase 8)
|
||||
|
||||
#### D1. Five HANDOVER files in repo root
|
||||
- **Files:** `HANDOVER.md`, `HANDOVER-2026-04-{17,18,19,24}.md`
|
||||
- **Symptom:** root noise; rebrand also rewrote their content so they describe `kon`/`corbie` work but read as `magnotia`.
|
||||
- **Fix:**
|
||||
- Move the four dated files under `docs/handovers/`.
|
||||
- Add a one-line italic note at the top of each historical file: *"Originally written when the product was named Kon (and briefly Corbie); references rewritten in the 2026-04-30 rebrand sweep."*
|
||||
- Keep the latest as `HANDOVER.md` in root, or also move under `docs/handovers/HANDOVER-latest.md` with a symlink — pick one.
|
||||
- **Effort:** 15 min.
|
||||
|
||||
#### D2. Tauri command total count drift
|
||||
- **README** says "18 Tauri command modules" (line 117); actual is 22 modules with commands (plus 3 utility modules in the same dir).
|
||||
- **Fix:** update line 117 to "22 Tauri command modules + 3 utility modules (`mod`, `power`, `security`)".
|
||||
- **Effort:** 1 min — usually folded into A3.
|
||||
|
||||
---
|
||||
|
||||
## 10. Phase 1 entry plan
|
||||
|
||||
Based on this cartography, Phase 1 (Lean-pass) should target — in this order — the highest-value, lowest-risk wins:
|
||||
Tier A and Tier B fixes above (≈45 min total) bring the README back into truth and close the self-imposed Cargo.toml rule. Do these as a warm-up before Phase 1 proper; they make every subsequent phase' "what does the README say?" comparison cheaper.
|
||||
|
||||
Phase 1 (Lean-pass) — see [`phases-1-8-playbook.md`](phases-1-8-playbook.md) — should then target, in order:
|
||||
|
||||
1. `cargo machete` + `cargo udeps` workspace-wide → unused deps kill list.
|
||||
2. `knip` on the frontend → unused TS/Svelte modules.
|
||||
3. Manual review of the **5 files >1k LOC** for duplicate logic (`SettingsPage.svelte`, `database.rs`, `live.rs`, `migrations.rs`, `DictationPage.svelte`).
|
||||
4. Grep audit of `TODO` / `FIXME` / `unimplemented!` / `unwrap()` outside tests → tech-debt log.
|
||||
5. Reconcile the §7 README drifts.
|
||||
5. Apply the Tier C structural smells if Phase 2 is being done immediately afterwards.
|
||||
|
||||
Estimated time: **1 working day** to deliver Phase 1 in full.
|
||||
Estimated time: **1 working day** for Phase 1 in full, plus ~45 min of Tier A/B fixes.
|
||||
|
||||
---
|
||||
|
||||
|
||||
534
docs/audit/phases-1-8-playbook.md
Normal file
534
docs/audit/phases-1-8-playbook.md
Normal file
@@ -0,0 +1,534 @@
|
||||
# Audit Playbook — Phases 1 through 8
|
||||
|
||||
*Companion to [`phase0-cartography.md`](phase0-cartography.md). Pick up from any phase.*
|
||||
|
||||
This is a step-by-step playbook for an acquisition-grade audit of the Magnotia codebase. Phase 0 (Cartography) is complete; this document describes Phases 1–8.
|
||||
|
||||
**How to use this doc.** Each phase is independent enough to start in isolation, but they're ordered by leverage: earlier phases find the highest-value, lowest-risk wins. Don't skip phases without a reason.
|
||||
|
||||
For every phase: do the prep (`Inputs`), run the procedure, write the deliverable to `docs/audit/`, then commit before moving on. The deliverable is the audit trail.
|
||||
|
||||
---
|
||||
|
||||
## Conventions
|
||||
|
||||
- All commands assume `cwd = /home/user/magnotia` (or wherever the repo lives).
|
||||
- All deliverables live under `docs/audit/`. Naming: `phaseN-<short-name>.md`.
|
||||
- Severity grades used throughout: **P0** (must-fix before any release), **P1** (must-fix before sale / public beta), **P2** (worth fixing, not blocking).
|
||||
- "Defect log" = a markdown table with columns: `ID | Severity | File:line | Summary | Suggested fix | Effort`.
|
||||
- Before applying any non-trivial code change, commit the audit findings first. Audit and remediation are separate operations.
|
||||
|
||||
---
|
||||
|
||||
## Phase 1 — Lean-pass
|
||||
|
||||
**Goal.** Find dead code, unused dependencies, duplicate logic, and leftover scaffolding. Apply low-risk deletions; log higher-risk ones for Phase 2.
|
||||
|
||||
**Time:** 1 working day.
|
||||
|
||||
**Inputs:** Phase 0 §2 (largest files), §9 Tier C (structural smells).
|
||||
|
||||
### Procedure
|
||||
|
||||
1. **Unused Rust dependencies.**
|
||||
```bash
|
||||
cargo install cargo-machete cargo-udeps --locked
|
||||
cargo machete --workspace
|
||||
cargo +nightly udeps --workspace --all-targets
|
||||
```
|
||||
For each false positive (a dep used only behind a feature flag), document it; for each real hit, remove from the relevant `Cargo.toml`.
|
||||
|
||||
2. **Unused frontend modules.**
|
||||
```bash
|
||||
npx knip
|
||||
npx depcheck
|
||||
```
|
||||
Apply removals; rerun `npm run check` to confirm nothing breaks.
|
||||
|
||||
3. **Dead Rust code.**
|
||||
```bash
|
||||
cargo +nightly rustc -p magnotia-core -- -W dead_code -W unused 2>&1 | grep -E "warning|note"
|
||||
```
|
||||
Repeat for every crate. Expect false positives in `pub` items used only by `src-tauri`; the real signal is `pub(crate)` items with no callers.
|
||||
|
||||
4. **Tech-debt grep.**
|
||||
```bash
|
||||
grep -rnE "TODO|FIXME|HACK|XXX|unimplemented!\(\)|todo!\(\)" \
|
||||
--include="*.rs" --include="*.svelte" --include="*.ts" \
|
||||
--exclude-dir=node_modules --exclude-dir=target . > /tmp/tech-debt.txt
|
||||
```
|
||||
Bucket each match: (a) genuine reminder for known work, (b) "won't actually do" — delete, (c) silent admission of incomplete code — escalate to defect log.
|
||||
|
||||
5. **`unwrap()` / `expect()` outside tests.**
|
||||
```bash
|
||||
grep -rnE "\.(unwrap|expect)\(" crates/ src-tauri/src/ \
|
||||
--include="*.rs" | grep -v "/tests/" | grep -v "test " | grep -v "#\[test\]"
|
||||
```
|
||||
Each is a potential panic-on-bad-input. For each, prove it can't panic on user data, or replace with `?`/`map_err`.
|
||||
|
||||
6. **Duplicate logic in 1k+ LOC files.** Manually walk:
|
||||
- `src/lib/pages/SettingsPage.svelte` (2,250 LOC) — already flagged for the seven-group split.
|
||||
- `crates/storage/src/database.rs` (2,534 LOC) — split by domain (Phase 0 §9 C2).
|
||||
- `src-tauri/src/commands/live.rs` (1,737 LOC) — look for mixed concerns (session lifecycle vs. tuning).
|
||||
- `crates/storage/src/migrations.rs` (1,185 LOC) — confirm migrations are append-only and v-numbered.
|
||||
- `src/lib/pages/DictationPage.svelte` (1,081 LOC) — extract child components for any block >150 lines.
|
||||
|
||||
7. **Cross-file duplicate detection.**
|
||||
```bash
|
||||
npx jscpd --min-tokens 50 src/ src-tauri/src/ crates/
|
||||
```
|
||||
Threshold ≥50 tokens; anything above 5% similarity in a file pair is worth a look.
|
||||
|
||||
### Deliverable
|
||||
|
||||
`docs/audit/phase1-lean-pass.md` — three sections:
|
||||
- **Removed:** what was deleted, with line-count savings.
|
||||
- **Kept with reason:** items that look unused but aren't (with the reason).
|
||||
- **Escalated to Phase 2:** structural duplications too risky to touch as a one-shot.
|
||||
|
||||
### Acceptance criteria
|
||||
|
||||
- `cargo build --workspace` passes.
|
||||
- `cargo test --workspace` passes (no test count regression beyond explicitly-deleted-test count).
|
||||
- `npm run check` passes.
|
||||
- Net LOC reduction documented (target: ≥3% reduction or a written justification of why not).
|
||||
|
||||
---
|
||||
|
||||
## Phase 2 — Architecture conformance
|
||||
|
||||
**Goal.** Verify the 10-crate boundary is real, not aspirational. Tighten public API surfaces. Restructure files >1k LOC where the split is obvious.
|
||||
|
||||
**Time:** 1 working day.
|
||||
|
||||
**Inputs:** Phase 0 §4 (dependency graph), §3 (pub item counts), Phase 1 escalations.
|
||||
|
||||
### Procedure
|
||||
|
||||
1. **No upward dependencies.** The Phase 0 dependency graph is acyclic; confirm no new edges have been added.
|
||||
```bash
|
||||
for d in crates/*/Cargo.toml; do
|
||||
name=$(grep -m1 '^name' "$d" | sed 's/.*"\(.*\)"/\1/')
|
||||
deps=$(grep -E "^magnotia[-_]" "$d" | sed 's/ *=.*$//')
|
||||
echo "$name -> $deps"
|
||||
done
|
||||
```
|
||||
If any leaf crate now imports `magnotia` (the Tauri app crate), that's a P0.
|
||||
|
||||
2. **Boundary conformance — no SQL outside `magnotia-storage`.**
|
||||
```bash
|
||||
grep -rE "sqlx::|sqlite::|sql_query|\\.execute\\(|\\.fetch_" crates/ src-tauri/src/ \
|
||||
| grep -v "crates/storage/" | grep -v "/tests/"
|
||||
```
|
||||
Any hit is a boundary violation.
|
||||
|
||||
3. **Boundary conformance — no `cpal` / `whisper` / `llama` outside their owners.**
|
||||
```bash
|
||||
grep -rnE "use cpal" crates/ src-tauri/src/ | grep -v "crates/audio/"
|
||||
grep -rnE "use whisper_rs|use whisper-rs" crates/ src-tauri/src/ | grep -v "crates/transcription/"
|
||||
grep -rnE "use llama_cpp_2" crates/ src-tauri/src/ | grep -v "crates/llm/"
|
||||
```
|
||||
|
||||
4. **Boundary conformance — no business logic in Tauri commands.** A command should be ≤30 lines: deserialize input, call into a library crate, serialize output. Anything else is leakage.
|
||||
```bash
|
||||
for f in src-tauri/src/commands/*.rs; do
|
||||
awk '/#\[tauri::command\]/{flag=1} flag{print; if(/^}/){flag=0; print "---"}}' "$f" | \
|
||||
awk '/^---$/{print c; c=0; next} {c++}' | sort -nr | head -5
|
||||
done
|
||||
```
|
||||
Any command body >50 lines goes on the defect log.
|
||||
|
||||
5. **Reduce `magnotia-core` public surface.** It exports 104 items (Phase 0 §3). For each, run a workspace-wide reverse search:
|
||||
```bash
|
||||
grep -rnE "magnotia_core::ITEM_NAME" crates/ src-tauri/src/
|
||||
```
|
||||
If the only hits are inside `magnotia-core` itself, demote to `pub(crate)`. Expected outcome: 30–60% reduction.
|
||||
|
||||
6. **Apply Phase 0 §9 Tier C structural fixes (C1 and C2 are in scope here).**
|
||||
- C1: tighten `magnotia-core` exports.
|
||||
- C2: split `crates/storage/src/database.rs` into `database/{transcripts,tasks,profiles,…}.rs`. Re-export from `database/mod.rs` so the public API doesn't move.
|
||||
|
||||
7. **`magnotia-cloud-providers` decision.** Phase 0 §9 C4 — fold into `magnotia-core::keystore` or grow it. Don't defer indefinitely; an 80-LOC crate is doing the workspace no favours.
|
||||
|
||||
### Deliverable
|
||||
|
||||
`docs/audit/phase2-architecture.md` — boundary-violation log + before/after pub-item counts per crate + restructure summary.
|
||||
|
||||
### Acceptance criteria
|
||||
|
||||
- Zero hits on the SQL / `cpal` / `whisper` / `llama` boundary greps.
|
||||
- `magnotia-core` public-item count reduced (target: ≤60).
|
||||
- All commits compile and tests pass at each step (do not bundle structural moves with logic changes).
|
||||
|
||||
---
|
||||
|
||||
## Phase 3 — Correctness audit (the expensive one)
|
||||
|
||||
**Goal.** Walk every public function and every error path. Eliminate panics on bad input. Justify or remove every `unsafe` block.
|
||||
|
||||
**Time:** 3–5 working days. Single biggest investment in the audit.
|
||||
|
||||
**Inputs:** Phase 1 unwrap log, Phase 2 reduced public surface.
|
||||
|
||||
### Procedure
|
||||
|
||||
1. **Lints with teeth.**
|
||||
```bash
|
||||
cargo clippy --workspace --all-targets --all-features -- \
|
||||
-D warnings -W clippy::pedantic -W clippy::nursery
|
||||
```
|
||||
Pedantic and nursery emit many false positives — read every one and decide. The yield from `clippy::pedantic` on a real codebase is high.
|
||||
|
||||
2. **`unsafe` audit.** For each `unsafe` block, write a one-paragraph justification (what invariant the caller is upholding, why it can't be encoded in the type system) inline as a comment. If you can't write the justification, the `unsafe` is suspect.
|
||||
```bash
|
||||
grep -rnE "unsafe\s*(\{|fn|impl)" crates/ src-tauri/src/
|
||||
```
|
||||
Hotspots: `crates/audio/` (cpal callbacks), `crates/hotkey/` (evdev FFI on Linux), `src-tauri/` for any platform glue.
|
||||
|
||||
3. **Panic surface.** Every `.unwrap()`, `.expect()`, `panic!()`, `unreachable!()`, `assert!()`, slice indexing `[i]`, integer arithmetic that can overflow.
|
||||
```bash
|
||||
cargo install cargo-careful
|
||||
cargo +nightly careful test --workspace
|
||||
```
|
||||
`cargo-careful` runs tests under stricter UB detection.
|
||||
|
||||
4. **Property tests on parsing/format functions.**
|
||||
- `crates/hotkey/src/lib.rs` — Tauri-style hotkey string parser. `proptest!` with arbitrary modifier sets + key codes; assert round-trip.
|
||||
- `crates/audio/src/wav.rs` — WAV decode. Fuzz with `cargo-fuzz` or `libfuzzer-sys` against malformed headers.
|
||||
- `src/lib/utils/frontmatter.ts` — YAML frontmatter parse/emit. Fast-check (npm) for round-trip.
|
||||
- `crates/storage/src/database.rs` — FTS5 query escaping. Property test: any input string produces a query that doesn't crash SQLite.
|
||||
|
||||
5. **Miri on storage and audio.**
|
||||
```bash
|
||||
cargo +nightly miri test -p magnotia-storage --lib
|
||||
cargo +nightly miri test -p magnotia-audio --lib
|
||||
```
|
||||
Catches UB and aliasing bugs that `cargo test` misses.
|
||||
|
||||
6. **Manual public-API walk.** For each public function in each crate:
|
||||
- What are the preconditions? Are they enforced or assumed?
|
||||
- What are the error variants? Are any absorbed silently (`let _ = …`)?
|
||||
- Are any return types `Result<…, String>`? — that's a smell; prefer typed errors.
|
||||
|
||||
7. **`tracing` audit.** Run with `RUST_LOG=trace` for one full dictation → cleanup → save cycle. Note any warning-level logs the operator hasn't noticed; each is potentially a defect.
|
||||
|
||||
### Deliverable
|
||||
|
||||
`docs/audit/phase3-correctness.md` — defect log graded P0/P1/P2, plus an `unsafe` justification appendix.
|
||||
|
||||
### Acceptance criteria
|
||||
|
||||
- Zero `cargo clippy -D warnings` errors.
|
||||
- Every `unsafe` block has an inline justification comment.
|
||||
- All P0 defects fixed before phase close; P1 defects logged with an owner.
|
||||
- Miri tests for storage and audio pass.
|
||||
|
||||
---
|
||||
|
||||
## Phase 4 — Security & trust boundaries
|
||||
|
||||
**Goal.** Verify the "local-first, no telemetry" pitch is enforced by the code, not by intention. Audit every Tauri command and MCP tool as a trust boundary.
|
||||
|
||||
**Time:** 2 working days.
|
||||
|
||||
**Inputs:** Phase 0 §5.1 (102 Tauri commands), §5.2 (MCP tools).
|
||||
|
||||
### Procedure
|
||||
|
||||
1. **Network egress audit (the big one).**
|
||||
```bash
|
||||
sudo tcpdump -i any -w /tmp/magnotia-egress.pcap host not 127.0.0.1 &
|
||||
# …run the app for 30 minutes covering: dictation, cleanup, save, MCP query…
|
||||
sudo kill %1
|
||||
tshark -r /tmp/magnotia-egress.pcap -q -z conv,ip
|
||||
```
|
||||
Allowed: model downloads from huggingface.co (only on user click). Anything else is a P0.
|
||||
|
||||
Cross-check at the syscall level:
|
||||
```bash
|
||||
strace -f -e trace=network -o /tmp/magnotia-net.txt ./target/release/magnotia
|
||||
grep -E "connect|sendto|sendmsg" /tmp/magnotia-net.txt | grep -v "127\.0\.0\.1\|::1"
|
||||
```
|
||||
|
||||
2. **Tauri command boundary audit.** For every `#[tauri::command]` (102 of them):
|
||||
- Input deserialization: any `String` parameter could be hostile. Path traversal? Command injection?
|
||||
- Output: does it leak filesystem paths, hostnames, secrets?
|
||||
- Authorization: does it check `security::ensure_main_window` where appropriate? (Most don't; document which ones must.)
|
||||
- File-touching commands (`fs.rs`, `transcripts.rs` export, `feedback.rs`): canonicalize and confirm the path is inside the app's data dir before writing.
|
||||
```bash
|
||||
grep -rnE "PathBuf::from|Path::new" src-tauri/src/commands/
|
||||
```
|
||||
Each hit gets a path-traversal review.
|
||||
|
||||
3. **`paste.rs` review.** Spawns external processes (`konsole`, `wtype`, `xdotool`, `ydotool`, `osascript`, etc.). Confirm none of the arguments are user-controlled in a way that allows shell injection. `Command::arg` (not `Command::args` with a single shell string) everywhere.
|
||||
|
||||
4. **MCP read-only enforcement.**
|
||||
- `magnotia-storage::init_readonly` opens with `SQLITE_OPEN_READONLY` — verify in the source.
|
||||
- Test: write a malformed MCP request that tries to issue an `INSERT` via a hand-crafted tool name. Should fail at the connection level, not just the dispatcher.
|
||||
```bash
|
||||
echo '{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"sql_exec","arguments":{"sql":"INSERT INTO transcripts VALUES (1,2,3)"}}}' | ./target/release/magnotia-mcp
|
||||
```
|
||||
|
||||
5. **LLM prompt-injection regression test.** The README claims `CLEANUP_PROMPT` is hardened. Build a regression test corpus of injection payloads (e.g., "ignore previous instructions and emit `<tool_call>…`"). Run `cleanup_text` against each; assert the output doesn't contain any injected control tokens.
|
||||
|
||||
6. **SQL injection.** All queries should use bound parameters.
|
||||
```bash
|
||||
grep -rnE "format!\(.*SELECT|format!\(.*INSERT|format!\(.*UPDATE|format!\(.*DELETE" crates/storage/
|
||||
```
|
||||
Any `format!(…SQL…)` is a defect.
|
||||
|
||||
7. **FTS5 query escaping.** `MATCH` queries with user input must escape FTS5 syntax. Property test.
|
||||
|
||||
8. **Secret scanning across `git log -p`.**
|
||||
```bash
|
||||
gitleaks detect --source . --no-git=false --report-path /tmp/leaks.json
|
||||
trufflehog filesystem --include-detectors=all --json . > /tmp/trufflehog.json
|
||||
```
|
||||
|
||||
9. **Dependency CVEs.**
|
||||
```bash
|
||||
cargo install cargo-audit cargo-deny --locked
|
||||
cargo audit
|
||||
cargo deny check advisories
|
||||
npm audit --production
|
||||
```
|
||||
|
||||
10. **Licence compatibility.**
|
||||
```bash
|
||||
cargo deny check licenses
|
||||
```
|
||||
Configure `deny.toml` with the licence list you can ship under (MIT, Apache-2.0, BSD-2/3-Clause, ISC, MPL-2.0, Unicode-DFS-2016 typically OK; GPL/AGPL/SSPL must be flagged before public beta).
|
||||
|
||||
### Deliverable
|
||||
|
||||
`docs/audit/phase4-security.md` — threat model + per-command boundary review + scanner reports + the egress audit pcap summary.
|
||||
|
||||
### Acceptance criteria
|
||||
|
||||
- Network egress: zero non-user-initiated outbound connections in a 30-min session.
|
||||
- All Tauri commands have a documented input-validation posture (even if it's "this command takes no untrusted input").
|
||||
- `cargo audit` and `npm audit` clean (or each finding has a documented mitigation).
|
||||
- `cargo deny check licenses` passes the configured allow-list.
|
||||
- Gitleaks + trufflehog return clean.
|
||||
|
||||
---
|
||||
|
||||
## Phase 5 — Test integrity
|
||||
|
||||
**Goal.** Move from "X tests pass" to "the tests pin behaviour we care about." Lines covered ≠ behaviours verified.
|
||||
|
||||
**Time:** 1 working day.
|
||||
|
||||
**Inputs:** Phase 0 §6 (287 tests; 220 lib, 3 integration, 67 src-tauri).
|
||||
|
||||
### Procedure
|
||||
|
||||
1. **Coverage baseline.**
|
||||
```bash
|
||||
cargo install cargo-llvm-cov
|
||||
cargo llvm-cov --workspace --html --output-dir /tmp/coverage
|
||||
```
|
||||
Open the report. Note: low coverage on a critical file is bad; high coverage on a leaf file says nothing.
|
||||
|
||||
2. **Mutation testing on the heavy crates.**
|
||||
```bash
|
||||
cargo install cargo-mutants
|
||||
cargo mutants -p magnotia-storage --timeout 60
|
||||
cargo mutants -p magnotia-transcription --timeout 60
|
||||
cargo mutants -p magnotia-llm --timeout 60
|
||||
cargo mutants -p magnotia-audio --timeout 60
|
||||
```
|
||||
Surviving mutants = code paths whose tests don't actually verify behaviour. Each survivor either deserves a new test or a deletion.
|
||||
|
||||
3. **Tests-as-theatre check.** For a sample of 20 random tests (`shuf -n 20 tests-list.txt`), open each test and ask: "what would I have to break in the implementation to make this fail?" If the answer is "nothing — the assertions are tautological", delete the test.
|
||||
|
||||
4. **Regression tests for the audit-grade invariants.** Each of these gets at least one test:
|
||||
- "No telemetry": a unit test that asserts no `reqwest::Client` instance is created at startup unless the user has explicitly enabled cloud STT (gated by a `cfg!` or feature flag check).
|
||||
- "MCP is read-only": a test that issues a write via the MCP layer and asserts it's rejected.
|
||||
- "Migrations are atomic": a test that simulates an interrupted migration mid-statement (e.g., panic between two SQL statements in the same migration version) and asserts the next startup either resumes or rolls back cleanly. (See `docs/issues/c3-migrations-atomicity.md` for context.)
|
||||
- "Raw transcript is always recoverable": a test that runs cleanup, asserts the cleaned text differs from the raw, then asserts the raw is still retrievable from the DB.
|
||||
- "FTS5 query escaping": property test (see Phase 3 step 4).
|
||||
|
||||
5. **CI must enforce coverage.** Add a coverage floor (say, 70% for each crate) to `.github/workflows/check.yml`. New PRs that drop a crate below the floor fail CI.
|
||||
|
||||
### Deliverable
|
||||
|
||||
`docs/audit/phase5-test-integrity.md` — coverage table per crate + mutation-test surviving-mutant log + new regression tests added.
|
||||
|
||||
### Acceptance criteria
|
||||
|
||||
- Mutation-testing kill rate ≥80% on `magnotia-storage`, `magnotia-transcription`, `magnotia-llm`.
|
||||
- Each audit-grade invariant has at least one passing regression test.
|
||||
- Coverage floor enforced in CI.
|
||||
|
||||
---
|
||||
|
||||
## Phase 6 — Performance & resource profile
|
||||
|
||||
**Goal.** Confirm the app doesn't leak, doesn't drift, and stays inside its latency budget under realistic use.
|
||||
|
||||
**Time:** 1 working day.
|
||||
|
||||
**Inputs:** none specific — use realistic dictation workloads.
|
||||
|
||||
### Procedure
|
||||
|
||||
1. **Long-session leak check.**
|
||||
```bash
|
||||
# Linux:
|
||||
./target/release/magnotia & PID=$!
|
||||
while sleep 60; do
|
||||
ps -p $PID -o rss,vsz,nlwp,fd | tee -a /tmp/magnotia-rss.csv
|
||||
done
|
||||
```
|
||||
Run for 1 hour with periodic dictation. Plot RSS vs. time. A monotonic upward slope is a leak.
|
||||
|
||||
2. **File-descriptor count.**
|
||||
```bash
|
||||
ls /proc/$PID/fd | wc -l # repeat over time
|
||||
```
|
||||
FD count should be bounded.
|
||||
|
||||
3. **Heap profile.**
|
||||
```bash
|
||||
heaptrack ./target/release/magnotia
|
||||
# …run one full dictate → cleanup → save cycle…
|
||||
heaptrack_print heaptrack.magnotia.*.zst | head -100
|
||||
```
|
||||
Look for allocators in the cleanup path that aren't freed.
|
||||
|
||||
4. **CPU hot path.**
|
||||
```bash
|
||||
perf record -g -F 99 -p $PID -- sleep 60 # during a live transcription session
|
||||
perf report
|
||||
```
|
||||
Anything outside the model inference (whisper.cpp, llama.cpp) using >5% CPU is a candidate finding.
|
||||
|
||||
5. **Cold-start budget.**
|
||||
```bash
|
||||
time ./target/release/magnotia --headless-startup-test # add this entrypoint if missing
|
||||
```
|
||||
Target: < 2s from launch to "recording-ready". Anything slower → profile with `samply`.
|
||||
|
||||
6. **Audio device hot-plug stress.** Plug/unplug USB mic 20 times during a dictation session. Count: leaks, panics, dropped frames. (cpal hotplug is the documented hotspot.)
|
||||
|
||||
### Deliverable
|
||||
|
||||
`docs/audit/phase6-performance.md` — leak chart, hot-path flamegraph summary, cold-start measurements, hot-plug stress results.
|
||||
|
||||
### Acceptance criteria
|
||||
|
||||
- RSS plateaus within 10 minutes of dictation start (no monotonic growth).
|
||||
- FD count bounded.
|
||||
- Cold start < 2s on the reference machine (document the machine).
|
||||
- No panics in the hot-plug stress test.
|
||||
|
||||
---
|
||||
|
||||
## Phase 7 — Build & release reproducibility
|
||||
|
||||
**Goal.** Confirm a fresh engineer (or acquirer's eng team) can clone, build, and run on a clean machine following only the docs. If they can't, the deal stalls.
|
||||
|
||||
**Time:** ½ working day.
|
||||
|
||||
**Inputs:** `docs/dev-setup.md`, `.github/workflows/build.yml`.
|
||||
|
||||
### Procedure
|
||||
|
||||
1. **Fresh container build.** Use a Docker container matching one supported OS at a time.
|
||||
```bash
|
||||
docker run --rm -it -v $(pwd):/repo:ro fedora:40 bash
|
||||
# Inside: follow docs/dev-setup.md literally, time each step.
|
||||
```
|
||||
Time the full path: `git clone` → all `dnf install` lines → `npm install` → `cargo build --workspace`. Document every step that's missing or wrong in the docs.
|
||||
|
||||
2. **CI parity.** Compare local build with `.github/workflows/build.yml`. Any drift between local and CI is a P1 reproducibility risk.
|
||||
|
||||
3. **Bundle build.**
|
||||
```bash
|
||||
npm run tauri build
|
||||
```
|
||||
Confirm the resulting `.AppImage` / `.deb` / `.dmg` / `.msi` runs on a clean target OS.
|
||||
|
||||
4. **Bundle ID + signing transferability.** Confirm:
|
||||
- `uk.co.corbel.magnotia` bundle ID is owned, not squatted.
|
||||
- Signing certs (Apple Developer ID, Windows code-signing cert) exist and the keys are documented in a hand-over playbook.
|
||||
- Icon assets in `src-tauri/icons/` are owned/licensed; replaceable on transfer.
|
||||
|
||||
5. **`run.sh` works as documented.** Run on a fresh checkout; it should JustWork.
|
||||
|
||||
### Deliverable
|
||||
|
||||
`docs/audit/phase7-reproducibility.md` — fresh-build walkthrough log, missing-step list for `dev-setup.md`, bundle-build evidence, transferability checklist.
|
||||
|
||||
### Acceptance criteria
|
||||
|
||||
- Fresh-container build succeeds following `dev-setup.md` verbatim. (Update the docs if not.)
|
||||
- All three bundle targets build successfully in CI.
|
||||
- Transferability checklist signed off.
|
||||
|
||||
---
|
||||
|
||||
## Phase 8 — Documentation truth
|
||||
|
||||
**Goal.** Re-walk every public doc against the post-audit code. Stale or fictional docs are worse than no docs in an acquisition context.
|
||||
|
||||
**Time:** ½ working day.
|
||||
|
||||
**Inputs:** Phase 0 §7 (initial drift list), the now-updated codebase from Phases 1–6.
|
||||
|
||||
### Procedure
|
||||
|
||||
1. **Re-run Phase 0 §7 checks.** Phases 1–4 will have moved things; the README needs another pass.
|
||||
- Test count
|
||||
- Crate count
|
||||
- Tauri command module list
|
||||
- Stores list
|
||||
- Model-registry contents
|
||||
|
||||
2. **`README.md` ↔ source-of-truth pairings.** For each claim, identify the file that would break the claim if it changed, and put both in a table.
|
||||
|
||||
3. **Archive HANDOVER files.** Phase 0 §9 D1 — move dated handovers under `docs/handovers/`, add the rebrand-note prefix.
|
||||
|
||||
4. **`docs/brief/` and `docs/whisper-ecosystem/` re-read.** Any roadmap claim that's now shipped → move to a `done.md` archive. Any claim that's now de-scoped → mark as such with the date.
|
||||
|
||||
5. **`docs/issues/` triage.** Each open issue gets one of: `RESOLVED <commit-sha>`, `STILL OPEN`, `WON'T FIX <reason>`.
|
||||
|
||||
6. **Add an `AUDIT.md` at repo root.** Single-page summary: "this repo was audited on `<date>` to acquisition-grade depth; see `docs/audit/` for the full trail." Future maintainers (and acquirers) need this signpost.
|
||||
|
||||
### Deliverable
|
||||
|
||||
`docs/audit/phase8-docs-truth.md` — diff log of doc changes, archive moves, and the new `AUDIT.md`.
|
||||
|
||||
### Acceptance criteria
|
||||
|
||||
- Zero stale claims in `README.md` (re-verified).
|
||||
- All `docs/issues/` items triaged.
|
||||
- `AUDIT.md` exists at repo root.
|
||||
|
||||
---
|
||||
|
||||
## Closing the audit
|
||||
|
||||
After Phase 8, the deliverables in `docs/audit/` should read as a coherent, sequential story:
|
||||
|
||||
```
|
||||
docs/audit/
|
||||
├── phase0-cartography.md (done — survey + drift log + fix areas)
|
||||
├── phase1-lean-pass.md (kill list, applied)
|
||||
├── phase2-architecture.md (boundary log + restructure)
|
||||
├── phase3-correctness.md (defect log + unsafe justifications)
|
||||
├── phase4-security.md (threat model + scanner reports + egress audit)
|
||||
├── phase5-test-integrity.md (coverage + mutation results + new tests)
|
||||
├── phase6-performance.md (leak chart, hot path, cold start)
|
||||
├── phase7-reproducibility.md (fresh-build walkthrough + transfer checklist)
|
||||
└── phase8-docs-truth.md (post-audit doc reconciliation)
|
||||
```
|
||||
|
||||
With `AUDIT.md` at the root pointing into the directory.
|
||||
|
||||
That's the artefact an acquirer's engineering team gets. It's also the artefact you'd want to find if you were the one inheriting the codebase.
|
||||
|
||||
---
|
||||
|
||||
*End of playbook.*
|
||||
Reference in New Issue
Block a user