general-audit lessons: 2026-04-28

[intendednull]

Lessons from /general-audit run @ 958e1ec (#474). Built on lessons #426 + #438.

What worked (from #438 suggestions, now validated again)

• Lesson Async client + UI refactor: eliminate polling, context-based state #2 applied: orchestrator-only sweep at the threshold. 52-file diff = right at general-audit lessons: 2026-04-27 (run 2) #438's "<50 file" suggested cutoff. Direct sweep still scaled — found 2 real findings (F1 Docker trunk pin, F2 component handler swallow) without agent context blow-up. Shape held.
• Lesson UX navigation overhaul: tabbed settings, confirm dialogs, context menu, command palette #4 applied: pre-fetched existing-issue lists. 69 unique titles (audit + tech-debt + security labels combined). Inline dedup caught 9 dup candidates immediately (AUD-1/AUD-2 already fixed via docs(web): SAFETY comment for DerivedStateActor #445/fix(agent): readonly scope must deny join_links resource #450, WS-1 [WS-1] Web: js_sys::eval(format!()) for pinned-message scroll uses band-aid sanitization #425, TD-14 [TD-14] anyhow used in 8 library crates contradicts CLAUDE.md convention #332, SEC-V-06 [SEC-V-06] TopicAnnounce.topics: Vec<String> has no element-count cap; enables relay CPU amplification + topic-slot exhaustion #235, SEC-V-07 [SEC-V-07] RotateChannelKey.encrypted_keys + Event.deps vectors have no element caps #236, DEP-02 [DEP-02] Docker images use unpinned rust:latest / rust:slim / nginx:alpine tags #313, all cargo-audit warnings).
• Lesson Add deep-review skill for iterative code review #5 applied: orchestrator ran cargo-audit directly. Clean vs CI ignore list. Zero agent needed. (Crates.io 403 on yank-check was harmless — vuln scan completed exit 0.)
• F2 root-cause matched [GEN-07] All UI action handlers swallow errors with no toast / log #350's class. Sibling issues across closed-fix-scope mismatches keep surfacing — last run's AUD-2 was that pattern (sec-authperm scope-gate forgotten in read_resource). This run: handlers.rs got warn_and_toast but components/ was missed. Pattern: closing an issue against one file leaves siblings in adjacent files.

What didn't

• F2 only surfaced because I ran a final "swallowed errors in new code" sweep. The standard sweep set (unsafe/dbg/eval/anyhow/lock-ok/Vec caps) wouldn't have caught it; let _ = h.<method>().await is a UX/observability pattern, not a security one. Add to standard sweep grep set.
• No sub-issue linking step in the skill — I had to call mcp__github__sub_issue_write manually after creating issues. Worth codifying.
• No verification of "did the fix in PR X actually resolve the open issue" step. Audit skill currently treats closed issues as fixed; but PRs sometimes close issues with partial fixes (e.g. [DEP-12] Deploy step does not pin trunk version (cargo install trunk --locked) #319 closed with deploy.yml-only fix, leaving Docker pipeline scope unaddressed → F1 today). Sibling-of-closed pattern needs explicit hunting.
• Cargo-audit registry 403 noise. Crates.io yank-check fails offline / behind proxy; main scan still works. Already using -n (--no-fetch) helps but doesn't suppress yank-check 403s. Consider documenting "ignore yank-check noise" or adding flag.
• Test-coverage concern was sampled, not swept. New code (browser.rs +457, static_assets.rs +164, tests.rs +346, search/tests.rs +99, bootstrap_endpoint.rs) shipped with tests, so I called this strong without exhaustive verification. Existing trackers ([TC-4] Large untested client modules: listeners/joining/connect/persistence_actor/voice #418 [TEST-02] network crate critically under-tested for transport-layer code #340 [TEST-03] relay crate thinly tested for a network-exposed component #341) cover prior gaps. Felt right; not rigorous.

Suggested edits to .claude/skills/general-audit/SKILL.md

1. Add let _ = .<method>().await to the standard sweep grep set. Specifically: rg "let _ = [a-z_]+\.[a-z_]+\(.*\)\.await" crates/web/src/components/. F2 wasn't in the prior sweep list. .ok(); ([TD-04] .ok() silently swallows ~16 errors in listeners.rs event-pump hot loop #253-class) is already there; let _ = await is a sibling and should be too.

2. Add an explicit "sibling-of-closed" pass. Before declaring synthesis complete, look at issues closed since the last audit master and check whether the fix scope was narrower than the bug class. Pattern: gh pr list --state merged --base main --json number,title,closingIssuesReferences, then for each closed issue, scan the codebase for siblings of the original symptom outside the fix scope. F1 (deploy.yml fix didn't cover Docker) and F2 (handlers.rs fix didn't cover components/) both fit.

3. Codify mcp__github__sub_issue_write linking as a numbered step. Currently the skill says "master issue + child issue per finding" but doesn't tell the orchestrator to wire them as GitHub sub-issues. Sub-issue linking surfaces children in master's UI panel.

4. Drop or fold the "8-agent fan-out" example block from SKILL.md. Three runs in a row (general-audit: main @ 401e2fc (2026-04-27) #413 8-agent failure, general-audit: main @ 00aa515 (2026-04-27) #437 0-agent success, general-audit: main @ 958e1ec (2026-04-28) #474 0-agent success) confirm orchestrator-direct is the new default. Lessons general-audit lessons: 2026-04-27 #426/general-audit lessons: 2026-04-27 (run 2) #438/general-audit: main @ 958e1ec (2026-04-28) #474 all suggest this. Time to stop coaching the failed pattern.

5. Update threshold from general-audit lessons: 2026-04-27 (run 2) #438's "<50 files" suggestion. Today's run was 52 files (just over) and orchestrator-direct still worked. Bump to "<100 files" or "<2000 LOC diff" — the real bottleneck is finding density per file, not file count.

6. Sweep templates in skill body. Lessons accumulate but the actual sweep commands live in the orchestrator's head. Move the standard rg/grep sweep set into SKILL.md as a concrete checklist:

# Security sweep
rg -n "(^|\s)unsafe\s+(impl|fn|\{)" crates --glob '!**/tests*'
rg -n "\b(dbg!|todo!\(|unimplemented!\(|FIXME|HACK)" crates --glob '!**/tests*' --glob '!**/main.rs'
rg -n "Arc<\s*(parking_lot::)?(Mutex|RwLock)<" crates --glob '!**/tests*' | grep -v "lock-ok"
rg -n "(js_sys::eval|innerHTML|set_inner_html)" crates --glob '!**/tests*'

# Observability / UX
rg -n "let _ = [a-z_]+\.[a-z_]+\(.*\)\.await" crates/web/src/components/
rg -n "\.ok\(\);" crates/client/src crates/web/src --glob '!**/tests*' | head -40

# Architecture
rg -n "use anyhow|anyhow::|anyhow!\(" crates/state/src crates/transport/src crates/identity/src crates/messaging/src crates/crypto/src crates/common/src
rg -n "topics:|deps:|participants:|peers:|members:" crates/common/src crates/transport/src crates/state/src/event.rs

# Deps / supply chain
cargo audit -n --ignore $(grep -oE "RUSTSEC-[0-9]+-[0-9]+" .github/workflows/ci.yml | tr '\n' ' ' | sed 's/ /,/g')
grep -rn "cargo install [^-][^- ]*" docker/ .github/workflows/
grep -rn "FROM [^@]*$" docker/        # any unpinned base image?

This makes the audit reproducible across runs without orchestrator memory.

Numbers

• diff since last audit: 37 commits / ~20 PRs / 52 files / +3525 −319
• agents launched: 0
• agents timed out: 0
• findings filed: 2 child + 1 master + 1 lessons = 4 issues
• false-positive findings dropped: 0 (both surviving findings 2nd-pass grep verified)
• dup candidates caught at synthesis: 9 (correctly suppressed)
• runtime: ~minutes
• cargo-audit: clean vs CI ignore list (8 RUSTSEC IDs)
• prior audit findings now confirmed fixed: AUD-1 (docs(web): SAFETY comment for DerivedStateActor #445), AUD-2 (fix(agent): readonly scope must deny join_links resource #450), TD-08 (87c1648), SEC-V-06 (3ee8a98), SEC-V-07 (7ee7e0a), DEP-02 (c479670), [SEC-W-06] Service-worker postMessage payload accepted without kind / origin check #244 (3a230c5/sw bridge extract)

Action

Human (or follow-up routine) to fold suggested edits into .claude/skills/general-audit/SKILL.md. Highest leverage: edits #1 (sweep grep set), #2 (sibling-of-closed pass), #6 (sweep template).

[intendednull]

already-fixed-upstream. suggested edits fold into .claude/skills/general-audit/SKILL.md via commit 8d91a11 docs(skill): fold audit lessons #426 #438 #477 #493 into general-audit.

let _ = .().await sweep entry, sibling-of-closed pass, sub_issue_write linking step, drop 8-agent fan-out example, threshold bump to <100 files / <2000 LOC, sweep grep set in skill body — all addressed. close completed.

closing under /resolving-issues run on claude/friendly-maxwell-cVhhm.

---

Generated by Claude Code