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,10 @@ 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 commit merged. Full phase logic lives
+  in docs/playbooks/ship-lane.md.
 ---
 
 # Ship Skill — Autonomous Merge Loop
@@ -38,6 +40,36 @@ The playbook's Phase 0 is **commit → push → open PR** only. Test generation
 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 merge commit
+
+Before Phase 0, require a completed `/quality` result with an empty gate. Before
+Phase 3c, also require `qualityValidatedSha` in the ship state to equal both
+`git rev-parse HEAD` and the PR's current `headRefOid`. The result is valid only
+for that exact commit; green CI on a later commit does not preserve it.
+
+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 rebase, conflict resolution, Phase 3b edit, or force-finalize edit clears
+  `qualityValidatedSha`. 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.
+
+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.
+
 ---
 
 ## Execution Mode: Autonomous
@@ -109,8 +141,11 @@ 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
+**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`, 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
@@ -184,7 +219,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.

apps/desktop/src/main/services/prs/prRowMetadata.test.ts

@@ -0,0 +1,81 @@
+import { describe, expect, it } from "vitest";
+import {
+  deriveGithubSnapshotLaneLink,
+  deriveGithubSnapshotMergeFacts,
+  type PullRequestRowMetadata,
+} from "./prRowMetadata";
+
+const LANE_ID = "lane-42";
+
+function makePrRowMetadata(
+  overrides: Partial<PullRequestRowMetadata> = {},
+): PullRequestRowMetadata {
+  return {
+    lane_id: LANE_ID,
+    merged_at: null,
+    additions: null,
+    deletions: null,
+    ...overrides,
+  };
+}
+
+describe("PR row metadata mappers", () => {
+  it("separates a live lane mapping from frozen detached-lane provenance", () => {
+    const laneById = new Map([[LANE_ID, { name: "my-feature" }]]);
+
+    expect(deriveGithubSnapshotLaneLink(makePrRowMetadata(), laneById)).toEqual({
+      linkedLaneId: LANE_ID,
+      linkedLaneName: "my-feature",
+      detached: null,
+    });
+
+    expect(deriveGithubSnapshotLaneLink(makePrRowMetadata({
+      detached_at: "2026-07-30T00:00:00Z",
+      detached_lane_name: "retired-lane",
+      detached_lane_color: "#4ADE80",
+      detached_provenance: JSON.stringify({ chats: 3, artifacts: 2, checkpoints: 5 }),
+    }), laneById)).toEqual({
+      linkedLaneId: null,
+      linkedLaneName: null,
+      detached: {
+        at: "2026-07-30T00:00:00Z",
+        laneName: "retired-lane",
+        laneColor: "#4ADE80",
+        chats: 3,
+        artifacts: 2,
+        checkpoints: 5,
+      },
+    });
+  });
+
+  it("normalizes persisted merge facts for a GitHub list row", () => {
+    expect(deriveGithubSnapshotMergeFacts(makePrRowMetadata({
+      merged_at: "2026-07-29T00:00:00Z",
+      merged_by_login: " octocat ",
+      merged_by_avatar_url: "https://example.com/octocat.png",
+      merge_method: "squash",
+      additions: 12,
+      deletions: 4,
+      commit_count: 3,
+      changed_files: 2,
+    }))).toEqual({
+      mergedAt: "2026-07-29T00:00:00Z",
+      mergedBy: { login: "octocat", avatarUrl: "https://example.com/octocat.png" },
+      mergeMethod: "squash",
+      additions: 12,
+      deletions: 4,
+      commitCount: 3,
+      changedFiles: 2,
+    });
+
+    expect(deriveGithubSnapshotMergeFacts(makePrRowMetadata({
+      merge_method: "unsupported",
+      additions: -1,
+      commit_count: Number.NaN,
+    }))).toMatchObject({
+      mergeMethod: null,
+      additions: null,
+      commitCount: null,
+    });
+  });
+});