docs: apply skill-review findings to ci-check and the project skills (#1416)

The observation log had accumulated 46 open entries against a last review of
2026-08-14. Seven of them target skills tracked in this repository and were
verified still-unapplied against the current files.

`ci-check` gains four things it was missing. It never mentioned `cargo audit`,
which CI runs pinned at 0.22.1 in `tauri-build` — the one gate that turns red
with zero local changes, because an upstream advisory breaks a branch that was
clean yesterday, and the one a hand-written mirror silently drops because no
edit provokes it. It never mentioned that `release.yml` is tag-triggered and
PR-ungated, so a smoke/sign/strip step added only there first executes on the
release; #1376 shipped a smoke harness whose own bug then blocked a release,
and #1378 fixed it structurally by extracting `Server/scripts/docker-smoke.sh`
for both workflows. And it had no guidance for reading a red check at all: a
new section adds causality-before-forensics triage (diff the changed-file set
against the failing job's input surface before opening a log — a workflow-only
diff cannot cause a Go goroutine leak), the lockfile-fork diagnosis for
dependency bumps (a 1 → 2 entry-count transition means the update forked the
dependency and revoked the features it was borrowing, so aligning versions is
the fix, not setting the feature the new copy demands), and the known-flake
table promoted to a signature-to-recovery index, now including the apt-mirror
hang that cancels `tauri-build` by timeout.

The baseline rule that came with the triage section needed adjusting rather
than transcribing. Its source observation recorded `golangci-lint`'s known-red
complexity baseline as 23 cyclop / 6 dupl / 21 funlen / 12 nestif; #1389
cleared that to zero, so quoting those numbers would have taught the reader to
excuse a failure that is now genuinely theirs. The rule is recorded without
them, stating that the repo currently carries no known-red gate and what to do
if one is ever reintroduced.

`protocol-change` claimed the schema is the source of truth without saying what
it covers. It holds message-type names only, so a payload-field change touches
the Go command/message files, the client types and `docs/protocol.md` and never
the schema — routing one through the regenerate cycle is wasted work. A table
splits the three cases, with the relay-handler caveat: a server that
re-serialises drops unknown fields, so a forwarded field is not backward
compatible with older servers.

`task-observer`'s numbering discipline treated collisions as a parallel-human
accident. They are structural in fan-out workflows, because a dispatched
subagent has the skill active in its own context and writes to the same log.

`bughunt-run` covered findings blocked by a circuit breaker but not findings
that went stale: a later hunt routinely fixes a blocked finding as a side
effect of an overlapping sibling, and a saved debris patch stops applying once
a refactor rewrites its files. Of 6 findings blocked on 2026-08-14, 2 were
already fixed 5 days later.

`docs/contributing.md` gains the commit-body convention that was being followed
without being written down anywhere — reasoning over diff-restatement, a
`Verified:` paragraph proving both directions, and an explicit `Not included:`
line. That last one is what keeps adjacent scope from becoming either silent
drift or an unnecessary blocking question.

Verified: each edit was checked against the live file before applying, which
changed two outcomes. Observation 50 (make the hunt's stop rule measure
coverage, not just quietness) is already implemented — `bughunt-run` documents
`coverage + dry is the real stop`, `stalledCoverage` and
`coverage.uncoveredAtStop`, landed by #1399 — so it is marked actioned rather
than re-applied. Observation 42 looked covered by the same grep and was not:
the existing text handles breaker-blocked findings, a different case from a
finding a sibling fix already closed. Confirmed absent before editing:
`cargo audit` and `release.yml` in ci-check, `payload` in protocol-change,
`subagent` in task-observer. `npm run check:hygiene` passes (prettier clean on
all five files); `npm run check:docs` passes.

Not included: the 21 open observations targeting `superpowers:*` plugin skills,
which live in a versioned plugin cache and are overwritten on update — they are
being routed to a separate user-owned extras skill outside this repository. The
6 targeting `graphify` are deferred pending a decision on whether that skill is
still in use here now that #1413 removed its repository integration. The 5
new-skill candidates are noted only; a review is not permitted to create skills.

Refs skill-observations #25, #35, #39, #41, #42, #43, #45, #58, #59, #63

.claude/skills/bughunt-run/SKILL.md

@@ -185,6 +185,15 @@ points: the fix stage (before any prove agent runs) and inside the prove loop.
 being on the wrong branch, a broken test runner, or ledger coordinates gone stale
 after a rebase. Re-running without fixing the cause just spends the budget again.
 
+**Re-verify a blocked finding against HEAD before fixing it.** A deferred item
+ages against a moving codebase: later hunts routinely fix a blocked finding as a
+side effect of an overlapping sibling, and a saved debris patch stops applying
+once a refactor rewrites the files it touched. Check the _mechanism_ still exists
+at HEAD, not just the line coordinates. If it is already covered, mark it fixed
+with a pointer to the covering commit instead of re-fixing it. Of 6 findings
+blocked on 2026-08-14, 2 were already fixed 5 days later and the debris patch no
+longer applied at all.
+
 Findings from clusters the run never reached come back `blocked` with a rationale
 naming the breaker. Set those back to `open` once the underlying problem is fixed
 — they were never attempted. Their edits are sitting uncommitted in the working

.claude/skills/ci-check/SKILL.md

@@ -9,6 +9,12 @@ description: Run the local mirror of OwnCord's CI gates before pushing. Use when
 
 Run only the sections your change touches. Server and client are independent.
 
+**A step added only to `release.yml` first runs at tag time.** `release.yml` is
+tag-triggered and never gated by a PR, so a smoke/sign/strip step added there is
+untested code on the critical path — its own bugs surface on the release, not on
+a PR. Extract it to a script `ci.yml` also runs (`Server/scripts/docker-smoke.sh`
+is the worked example) or duplicate it into `ci.yml` before merge.
+
 From the repository root, `npm run check` runs all of it, and
 `check:server` / `check:client` / `check:rust` / `check:hygiene` run one stack.
 `node scripts/run.mjs --list` prints the exact command each step runs and the
@@ -95,14 +101,64 @@ it inside `golangci-lint run`, and `.githooks/pre-commit` catches staged files.
 cargo fmt --all -- --check               # runs ahead of clippy in CI
 cargo test --lib                         # CI runs --lib; plain `cargo test` also builds the bin target
 cargo clippy --all-targets -- -D warnings
+cargo install cargo-audit@0.22.1 --quiet && cargo audit   # CI runs this in tauri-build
 ```
 
+`cargo audit` is the one gate here that turns red with **zero** local changes —
+an advisory published upstream breaks a branch that was clean yesterday. Check the
+advisory date before hunting your diff. It is skipped on Dependabot PRs by design
+(it overlaps the scanning that opened them), so a clean Dependabot run does not
+mean the advisory set is clean. The client equivalents, `npm audit --omit=dev
+--audit-level=high` and `knip`, are advisory in CI.
+
 `fallback_crypto` is `cfg(not(windows))`, so its tests compile to nothing on a
 Windows box and only run on the Linux/macOS runners.
 
 Do not attempt `npm run tauri build` locally — the full desktop build runs in
 CI on PRs to `main` and pulls heavy system dependencies.
 
+## Reading a red check
+
+**Causality before forensics.** Before opening a failing job's log, diff the
+PR's changed-file set against that job's input surface and ask whether the change
+could reach it. A diff touching only `.github/workflows/*.yml` cannot cause a Go
+goroutine leak — that failure is pre-existing or flaky by construction. Re-run
+first, and check `dev`/`main` is green to tell "flaky" from "already red". Only
+start log-reading once the change plausibly reaches the job.
+
+**Compare against the baseline, never against zero.** For any gate a repo
+knowingly runs red, the unit of verification is the _delta_ from a recorded
+baseline, not pass/fail — absolute pass/fail only means something when the
+intended state is zero. Get the delta with `git stash && <gate> > /tmp/base &&
+git stash pop && <gate> | diff /tmp/base -`. This repo currently carries **no**
+known-red gate: `golangci-lint`'s complexity backlog was cleared to zero, so a
+red `golangci-lint` is now genuinely yours. If a budget is ever retuned upward,
+record the new baseline here next to the command or the gate reports nothing.
+
+**A dependency bump that breaks the build may be a fork, not a version.** When an
+updated dependency suddenly demands configuration it never needed, suspect it was
+inheriting that configuration from a shared resolution with another dependent.
+Diff the lockfile _entry count_ for that dependency between base and PR: a 1 → 2
+transition means the update forked it into two semver-incompatible copies, feature
+unification stopped crossing the boundary, and the fix is to restore version
+alignment with whatever else requires it — not to set the feature the new copy
+asks for.
+
+### Known infra flakes
+
+Not your change. Match the signature, then recover.
+
+| Signature                                                                                                                                                                                      | Verdict / recovery                                                                                                                                                 |
+| ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
+| `windows-latest` `-race` fault in `ws`: `runtime.scanstack`, `runtime.(*unwinder).next`, or `unexpected fault address 0xffffffffffffffff` / `fatal error: fault` inside ordinary stdlib frames | Go runtime GC fault, not your code — see the Server section. `gh run rerun --job <id>`                                                                             |
+| `##[error]The operation was canceled.` + `Terminate orphan process: ... playwright install --with-deps` + a wall of `Ign:N http://azure.archive.ubuntu.com/...` and no Playwright summary line | Runner apt-mirror outage during "Install Linux system dependencies". The job was **canceled by timeout**, not failed. `gh run cancel` then `gh run rerun --failed` |
+| Red `Lint` step with zero linters actually run                                                                                                                                                 | `golangci-lint`'s network schema fetch failed. Re-run                                                                                                              |
+
+`gh run view --log` refuses while a run is in progress; `gh api
+repos/<owner>/<repo>/actions/jobs/<id>/logs` works. A job cannot be rerun while
+its parent run is still in progress. `tauri-build` has no `timeout-minutes`, so a
+hung apt step can hold a run open for the 6 h default — cancel it rather than wait.
+
 ## Hooks
 
 `npm run hooks:install` (once per clone) points `core.hooksPath` at

.claude/skills/protocol-change/SKILL.md

@@ -8,6 +8,21 @@ description: Add or change a WebSocket message type in OwnCord. Use before editi
 `docs/protocol-schema.json` is the source of truth. Both constant files are
 generated from it by `Server/scripts/genprotocol/`.
 
+**The schema holds message-type NAMES only.** Route by what you are changing —
+most payload work never touches it, and sending a field change through the
+regenerate cycle below is wasted work:
+
+| Change                                                   | What to edit                                                                                                           |
+| -------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------- |
+| New message type                                         | schema + regenerate (steps below)                                                                                      |
+| New or changed payload **field** on an existing type     | `Server/ws/command.go`/`messages.go`, `Client/src/lib/protocolTypes.ts`, `docs/protocol.md` — no schema, no regenerate |
+| Content inside an opaque blob the server relays verbatim | `docs/protocol.md` only; often zero Go change                                                                          |
+
+Before assuming a field needs server work, read the relay handler: if the server
+forwards the message raw, there is nothing to add. If it **re-serialises**, an
+older server drops unknown JSON fields — so a field the server must forward is
+NOT backward compatible with older servers.
+
 1. Edit `docs/protocol-schema.json`.
 2. Run `make protocol-generate` from `Server/`.
 3. Commit **both** outputs — `Server/ws/message_types.go` and

.claude/skills/task-observer/SKILL.md

@@ -176,6 +176,14 @@ checkpoints; piggy-backing the flush onto them means the write happens as a
 side effect of work you were doing anyway, rather than depending on a separate
 act of memory.
 
+**Your own delegates are concurrent writers.** A subagent dispatched into the
+same project has this skill active in its own context and appends to the same
+log, so it consumes numbers between your read and your write. Collisions are
+structural in any fan-out workflow, not a rare parallel-human accident — which
+is exactly why the pre-write assertion below matters most in the workflows that
+spawn helpers. When dispatching, say who owns logging for the session, or two
+writers record the same incident from different angles under different numbers.
+
 **Numbering discipline (mandatory, every append):**
 
 1. _Pre-check:_ read the actual log and find the highest existing number —

docs/contributing.md

@@ -200,6 +200,17 @@ perf: cache role permissions in memory
 ci: add lint step to GitHub Actions
 ```
 
+For anything non-trivial the body carries the reasoning, not a restatement of
+the diff: what was wrong, why the obvious fix is wrong, what was done, concrete
+numbers, and a `Verified:` paragraph proving both directions — that the defect
+was present before and is absent after.
+
+End with an explicit **`Not included:`** line naming adjacent scope you
+deliberately left out, and why. A written deferral is a deliverable: it is what
+separates considered-and-declined from silently-missed, and it means adjacent
+work you spotted mid-change does not have to become either scope creep or a
+blocking question. Put it in the commit that noticed it.
+
 ## Pull Request Process
 
 See [Branch and PR model](#branch-and-pr-model) above for what to branch from