PRs: apply the quality findings, and make /quality require it

.agents/skills/quality/SKILL.md

@@ -4,8 +4,10 @@ description: >-
   Make the code correct, clean, and current. A thermo dual-review: a
   correctness/security track and a maintainability/code-judo track run in
   parallel, then a synthesis step dedupes, severity-ranks (Blocker/High/Medium/
-  Low), verifies each finding against the real code, auto-applies the safe
-  behavior-preserving fixes, re-reviews, and gates the rest. Grounded in ADE's
+  Low), verifies each finding against the real code, and FIXES EVERY VERIFIED
+  FINDING at any severity — re-reviewing until clean. Only findings needing a
+  product decision, or a behavior change this branch was not asked to make,
+  reach the merge-blocking gate. Grounded in ADE's
   own bug classes (runtime-backed null services, daemon action-domain wiring,
   cr-sqlite CRR, IPC contract drift, fast-tier loading).
 ---
@@ -107,15 +109,37 @@ are handled by the synthesis step below, not a separate phase.
 5. **Sweep the bug class.** When an accepted finding is a repeated pattern, scan
    the diff scope for sibling instances and fix them together — stop at touched
    surfaces and owner boundaries; no refactor beyond the class.
-6. **Apply** the fixes that are **unambiguous and behavior-preserving** — safe
-   correctness fixes and Track B judo moves. Every applied change must be
-   verifiable by reading the diff; do not change behavior.
-7. **Re-review until clean.** If step 6 changed code, re-run Track A on the *new*
-   diff. New accepted findings → verify (4), apply (6), re-check. Stop when a
-   pass yields no new accepted findings (cap 2 extra passes; anything still open
-   goes to the gate). Catches fix-induced regressions before `/test` or `/ship`.
-8. **Gate** — do NOT auto-apply Blockers or judgment-call findings. Surface them
-   in the report for the author and for `/ship` to gate the merge on.
+6. **Apply every finding you accepted in step 4 — all of them, whatever the
+   severity.** Verified means valid; valid means fix it. Medium and Low are not a
+   backlog, and "behavior-preserving" describes *how* you apply a fix, not which
+   findings earn one. This is the entire point of the skill: a run that surfaces
+   real problems and leaves them in the code has cost the user tokens and
+   returned nothing.
+
+   Fix correctness findings and Track B judo moves alike. If a fix is genuinely
+   large (a multi-file extraction, a schema migration), it is still yours to do —
+   do it here, in this run, not "as a follow-up".
+7. **Re-review until clean.** If step 6 changed code, re-run **both mandatory
+   tracks, A and B,** on the *new* diff. New accepted findings → verify (4),
+   apply (6), re-check with both tracks again. Stop only when the same pass
+   yields no new accepted findings from either track. A re-review count is never
+   a reason to defer a verified finding or move it to the gate. This catches
+   fix-induced correctness regressions and maintainability debt before `/test`
+   or `/ship`.
+8. **Gate — the narrow exception, not the escape hatch.** Only two kinds of
+   accepted finding may go to the gate unfixed:
+   - it needs a **product decision you cannot make** (which of two valid
+     behaviors the user wants), or
+   - the fix is **not behavior-preserving** and changing behavior is not what
+     this branch was asked to do.
+
+   "Structural", "large", "risky", "pre-existing", "out of scope for this PR",
+   and "worth doing deliberately" are **not** gate reasons — those are fixes you
+   owe. If you gate a finding, the report must say which of the two reasons
+   applies and what decision you need. Anything in the Gate table blocks the
+   merge until the author resolves it; `/ship` treats a non-empty gate as a stop.
+
+   A finding you neither fixed nor gated is a bug in your run.
 9. **Reconcile (optional)** — if a PR exists, *after* the independent audit, read
    the PR discussion and review-bot comments (`gh pr view --comments`, or the
    `ade-pr-workflows` skill). ADE's review bots are `@copilot` (first push) and
@@ -126,8 +150,9 @@ are handled by the synthesis step below, not a separate phase.
 
 ## Completion
 
-Output a summary. The **Gate** section is what `/test` and `/ship` consume — list
-every Blocker and High finding that was NOT auto-fixed.
+Output a summary. The **Gate** section is what `/test` and `/ship` consume, and a
+non-empty gate blocks the merge. List only findings you could not fix for one of
+the two permitted reasons — not findings you chose to defer.
 
 ```markdown
 ## Quality Summary
@@ -137,11 +162,24 @@ every Blocker and High finding that was NOT auto-fixed.
 - Auto-applied: [count] (safe correctness fixes + structural judo moves)
 - Re-review passes: [n]
 
-### Gate (not auto-fixed — for /test regression targets and /ship merge gate)
-| Severity | file:line | Finding | Why not auto-fixed |
-|---|---|---|---|
-| Blocker | ... | ... | needs human judgment / product intent |
-| High | ... | ... | ... |
+### Gate (MERGE-BLOCKING — every row needs an author decision)
+Only two reasons belong here: a product decision you cannot make, or a fix that
+is not behavior-preserving on a branch that was not asked to change behavior.
+Empty is the expected outcome. "Structural / large / out of scope" is not a
+gate reason — those get fixed above.
 
-Next: /test (turn each Blocker/High above into a named regression test).
+When empty, print exactly:
+
+- Empty.
+
+Do not print a table. When non-empty, replace `- Empty.` with a table containing
+only real findings and these columns: Severity, file:line, Finding, Which gate
+reason, Decision needed. Never leave an example or placeholder row that another
+skill could mistake for a live gate.
+
+Next: /test (itemize every accepted correctness finding and give each a named
+regression test or explicit alternate verification).
+
+**Before you print this:** every accepted finding is either in "Auto-applied" or
+the Gate section. If one is in neither, go back to step 6 and fix it.
 ```

.agents/skills/ship/SKILL.md

@@ -4,8 +4,11 @@ description: >-
   Autonomous PR-to-merge loop. Polls CI and review bots, fixes failures, rebases
   only on real conflicts, and lands the PR on main. Soft cap of 5 normal
   iterations plus one force-finalize iteration that bypasses review and fixes
-  only CI. Pure loop — it does NOT run /quality or /test; run those first. Full
-  phase logic lives in docs/playbooks/ship-lane.md.
+  only CI. Pure loop — it does not replace the baseline /quality or /test runs;
+  run those first. It does revalidate quality after any ship-loop mutation so
+  the final result is bound to the exact reviewed PR head and content tree.
+  Full phase logic lives
+  in docs/playbooks/ship-lane.md.
 ---
 
 # Ship Skill — Autonomous Merge Loop
@@ -34,9 +37,46 @@ runtime-neutral entrypoint and the ADE-specific deltas below. If re-invoked by a
 scheduled wake, read the state file first; if `status == running`, skip Phase 0
 and go to Phase 1.
 
-The playbook's Phase 0 is **commit → push → open PR** only. Test generation and
-the local-CI gate are NOT part of ship — that's `/test` (and optionally
-`/finalize`) before you reach this skill.
+The playbook's Phase 0 is **checkpoint → commit-bound quality revalidation →
+push → open PR**. Baseline test generation and the local-CI gate are NOT part
+of ship — that's `/test` (and optionally `/finalize`) before you reach this
+skill.
+
+## Precondition: `/quality` must be empty and bound to the final tree
+
+Before Phase 0, require a completed `/quality` result with an empty gate. Before
+Phase 3c, run the playbook's single canonical **Validate the current quality
+binding** procedure. It binds the reviewed head, content tree, and base so
+GitHub's squash/merge/rebase result has the reviewed tree. Green CI on a later
+head or base does not preserve this binding.
+
+A non-empty gate **blocks the merge** — every row in it is a finding that was
+verified as real and left unfixed, and by `/quality`'s contract the only two
+things that may be there are a product decision the author owes, or a behavior
+change this branch was not asked to make. Both need the author.
+
+- Gate rows exist → do not merge. Surface them, state the decision needed, and
+  stop with `blocked`. Do not merge and mention them afterwards.
+- If `/quality` was never run on this lane, or its final gate result is not
+  available in the lane handoff, stop with `blocked`; unknown is not empty.
+- Any base movement, rebase, conflict resolution, Phase 3b edit, or
+  force-finalize edit clears all three quality binding fields. Run the
+  playbook's single canonical **Commit-bound
+  quality revalidation** procedure before pushing that mutation.
+- Never enter Phase 3c with a missing or mismatched binding. Revalidate first;
+  do not merge and disclose stale quality evidence afterwards.
+- Bind every normal or admin merge attempt with
+  `--match-head-commit "$QUALITY_VALIDATED_SHA"`. Persistent auto-merge is not
+  allowed because a later push can replace the validated head while it remains
+  armed.
+- GitHub creates a new commit for squash/merge/rebase. The validation claim is
+  deliberately about its exact content tree, not its not-yet-created commit
+  OID. After merge, run the playbook's canonical **Confirm the validated merge
+  result** procedure; a mismatch is never `done-clean`.
+
+Severity is irrelevant here: a Medium in the gate blocks exactly as hard as a
+Blocker, because presence in the gate means it needed a human, not that it was
+minor.
 
 ---
 
@@ -95,11 +135,11 @@ terminal-neutral, and continue. Record it under `inactiveReviewBots`, never
 If branch protection requires an absent check, Phase 3c will surface that as a
 merge-policy block.
 
-**Rebase only on real conflicts.** `behindMain` alone does NOT trigger a rebase.
-Only rebase/merge `main` when there is an actual conflict (`mergeStateStatus`
-shows the PR is dirty/conflicting). If the branch is merely behind but cleanly
-mergeable, skip the rebase and let the merge handle it — needless rebases burn
-iterations and CI.
+**Rebase only on real conflicts or a stale quality base.** `behindMain` alone
+does not normally trigger a rebase. The one safety exception is base movement
+after quality validation: the final tree is no longer the reviewed head tree,
+so rebase and rerun the canonical quality procedure even when GitHub reports a
+clean merge. Otherwise, skip needless rebases.
 
 **Bot pings by iteration.** Never ping GitHub Copilot and never treat Copilot as
 an expected review signal; quota exhaustion otherwise leaves the loop waiting
@@ -109,11 +149,15 @@ fix-iteration re-pushes → `@codex review`. For a >250-file diff, also ping
 expected review signals to settle before fixing. This is the playbook's Phase 4
 rule — defer to it for exact bodies.
 
-**Merge needs admin.** `main` is ruleset-guarded — `gh pr merge --squash` will
-show BLOCKED. Retry with `gh pr merge --admin --squash`; the ruleset's
-non-linear-history rule can still reject `--admin`, in which case fall back to a
-local merge + admin-bypass push (per AGENTS.md). Do NOT pass `--delete-branch`
-(it fails from a worktree); delete the head ref server-side via
+**Merge needs admin.** `main` is ruleset-guarded —
+`gh pr merge --squash --match-head-commit "$QUALITY_VALIDATED_SHA"` will show
+BLOCKED. Retry with
+`gh pr merge --admin --squash --match-head-commit "$QUALITY_VALIDATED_SHA"`;
+the ruleset's non-linear-history rule can still reject `--admin`. Do not fall
+back to a locally-created commit: it would not be the reviewed and CI-tested PR
+merge result. Exit blocked if both direct `gh` paths fail. After a successful
+merge, run **Confirm the validated merge result**. Do NOT
+pass `--delete-branch` (it fails from a worktree); delete the head ref server-side via
 `gh api -X DELETE "repos/{owner}/{repo}/git/refs/heads/<branch>"`.
 
 **Fix discipline (every fix agent must follow):** (1) Fix CI and review together
@@ -160,14 +204,16 @@ self-resume signal. Either:
 ## The loop (summary — full detail in the playbook)
 
 - **Phase 0 (first run):** safety rails (clean tree, GitHub origin, refuse
-  `main`) → commit → push → open PR (`ade`, gh fallback) → write state →
+  `main`) → checkpoint → canonical commit-bound quality revalidation → push →
+  open PR (`ade`, gh fallback) → verify the provisional binding → write state →
   schedule first wake.
 - **Phase 1 — Poll:** wait for CI terminal and every bot that actually started to
   become terminal. After one 12-minute grace window, classify bots with zero
   evidence as inactive/terminal-neutral. Return a structured summary (merged /
   conflicting / ciFailed / newComments). Don't fix on a partial signal.
-- **Phase 2 — Decide:** merged → `done-clean`. Real conflict → Phase 3a rebase
-  (rebate). CI or bots running → reschedule. Both terminal, no work → 3c merge.
+- **Phase 2 — Decide:** merged → run **Confirm the validated merge result** and
+  only then set `done-clean`. Real conflict → Phase 3a rebase (rebate). CI or
+  bots running → reschedule. Both terminal, no work → 3c merge.
   Both terminal, work exists, `iter < 5` → 3b fix. `iter >= 5`, not merged → 3d
   force-finalize.
 - **Phase 3a Rebase / 3b Fix / 3c Merge / 3d Force-finalize** — per the playbook.
@@ -184,7 +230,7 @@ self-resume signal. Either:
 |--------|---------|
 | `done-clean` | PR merged on main |
 | `done-max` | 5 normal + 1 force-finalize exhausted, merge genuinely blocked |
-| `blocked` | Unrecoverable conflict, gate failure, API error, or force-finalize CI failed |
+| `blocked` | Unrecoverable conflict, gate failure, API error, force-finalize CI failed, or a non-empty `/quality` gate awaiting an author decision |
 
 Always print the final summary (PR, branch, iterations, status, reason,
 per-iteration log, unaddressed items) on exit. Do NOT schedule a wake when

.agents/skills/test/SKILL.md

@@ -1,6 +1,6 @@
 ---
 name: test
-description: 'Prove the new code works: enforce the logging/PostHog ground truth, prune dead tests, consolidate fragments, add only tests that prove new contracts, turn each /quality Blocker/High finding into a named regression test, then run CI-mirrored shards. Also keeps docs/mobile/CLI/TUI parity in lockstep.'
+description: 'Prove the new code works: enforce the logging/PostHog ground truth, prune dead tests, consolidate fragments, add only tests that prove new contracts, turn accepted /quality correctness findings into named regression tests, then run CI-mirrored shards. Also keeps docs/mobile/CLI/TUI parity in lockstep.'
 ---
 
 # /test — Test Suite Steward
@@ -17,7 +17,27 @@ The suite has bloated for three reasons. You exist to fight all three:
 
 Every run does three passes in this order: **PRUNE → CONSOLIDATE → ADD**. You may finish at any pass — adding is optional.
 
-**Consume the `/quality` gate.** If `/quality` ran on this lane, take its Summary's **Gate** section — every Blocker/High it surfaced but did not auto-fix. Each is a named regression-test target for the ADD pass: a test that fails on the bug and passes once it's fixed. A finding isn't "handled" until a test pins it. No gate available → derive the same targets from the diff.
+**Consume the `/quality` result.** A non-empty `/quality` gate blocks this skill:
+the branch still contains a verified finding awaiting an author decision, and
+committing a knowingly failing test is not a substitute for fixing it. Resume
+after the decision and the corresponding `/quality` fix.
+
+Build a correctness inventory from the completed quality summary. Itemize
+**every accepted correctness finding** by a stable finding name and original
+`file:line`; aggregate counts such as "5 findings covered" are not sufficient.
+For each item, provide exactly one of:
+
+- a named regression test that pins the public contract and would fail on the
+  pre-fix behavior, or
+- an explicit alternate verification: the exact command/check, the observed
+  evidence, and why a regression test is not appropriate.
+
+Existing coverage counts only when you name the specific test and confirm that
+it exercises the finding's failure mode. Structural maintainability findings do
+not require artificial tests when existing coverage already proves the
+behavior-preserving move. No quality result available → derive the same
+itemized correctness inventory from the diff and state that quality evidence
+was unavailable.
 
 **Run the way CI would.** After the suite work, run only the affected shards, never the full suite (that's `/finalize`'s local gate and `/ship`'s remote CI). Verify every new/edited test file matches a vitest workspace glob so CI actually picks it up.
 
@@ -571,6 +591,11 @@ Added:
 - <new file or extended file> — <N tests covering: contract A, contract B>
 - Or "none — feature was visual / fully covered by consolidation"
 
+Quality correctness findings:
+- <stable finding name> (`file:line`) — regression: `<test file> :: <test name>`
+- <stable finding name> (`file:line`) — alternate verification: `<exact command/check>` → `<observed evidence>`; no regression test because <specific reason>
+- Or "none — /quality accepted no correctness findings"
+
 Parity:
 - Logging/PostHog: <instrumentation/docs/dashboard changes, or explicit not-applicable reason> — privacy + cost gate PASS / blocked
 - Docs: <files updated, or "none required"> — validation PASS / blocked
@@ -604,3 +629,5 @@ Mark **completed** only if all of:
 5. Every new test file matches a vitest workspace glob.
 6. The summary is the *only* thing you output.
 7. `docs/logging.md` exists, was read, and every analytics-applicable change is covered or has an explicit not-applicable rationale.
+8. Every accepted correctness finding from `/quality` appears individually in
+   the summary with a named regression test or explicit alternate verification.

AGENTS.md

@@ -17,15 +17,15 @@ Day-to-day work follows a five-stage loop, each stage an agent-folder skill unde
 `/context` → work → `/quality` → `/test` → `/ship`
 
 - **/context** — session primer: detects the lane's area and loads only the matching docs + perf skill (never a broad dump).
-- **/quality** — dual-track review (correctness/security + maintainability/code-judo); auto-fixes the safe findings, gates Blockers. The loop's bug-finding and cleanup engine.
-- **/test** — test steward: prune/consolidate/add + docs/mobile/CLI/TUI parity + CI-mirrored shards; turns each `/quality` gate finding into a named regression test.
-- **/ship** — pure autonomous PR→merge loop (poll → fix → rebase → merge). Does NOT run quality/test — run those first. Wraps `docs/playbooks/ship-lane.md`.
+- **/quality** — dual-track review (correctness/security + maintainability/code-judo); fixes every verified finding at every severity. It gates only a product decision the agent cannot make or a behavior change the branch was not authorized to make.
+- **/test** — test steward: prune/consolidate/add + docs/mobile/CLI/TUI parity + CI-mirrored shards; records a named regression test or exact alternate verification for every accepted correctness finding.
+- **/ship** — autonomous PR→merge loop (poll → fix → rebase → merge). Run baseline `/quality` and `/test` first; after any ship-loop mutation, ship reruns commit-bound `/quality` revalidation before pushing or merging. Wraps `docs/playbooks/ship-lane.md`.
 
 Utilities (run when relevant, not part of the core loop): **/audit** (targeted bug hunt), **/finalize** (optional pre-push local-CI gate), **/optimize** (perf profiling), **/release** (cut a release).
 
 ## Playbooks
 
-- `docs/playbooks/ship-lane.md` — autonomous PR-to-merge driver (poll → fix → rebase → merge; `/quality` and `/test` run *before* it, not inside it). Any agent CLI can follow it directly; Claude Code invokes it via the `/ship` skill.
+- `docs/playbooks/ship-lane.md` — autonomous PR-to-merge driver (poll → fix → rebase → merge). Baseline `/quality` and `/test` run before it; mutation-specific commit-bound quality revalidation runs inside it. Any agent CLI can follow it directly; Claude Code invokes it via the `/ship` skill.
 
 ## Working norms