chore(tooling): automate ship review and validation (#2176)

* chore(tooling): automate ship review and validation

* refactor(tooling): tighten ship review guidance

* fix(tooling): commit ship review fixes before confirmation

* fix(tooling): isolate ship review context

Author: tyler-dane

.codex/skills/ship/SKILL.md

@@ -30,10 +30,12 @@ For browser-observable changes, read and use the available Chrome-control skill.
 1. Open a fresh local tab and exercise the golden path.
 2. Derive 2–4 edge cases from the diff: boundaries, new conditionals, state transitions, and the exact bug reproduction. Avoid generic cases unrelated to the change.
 3. Check browser console and network failures when exposed.
-4. Record reviewer-facing steps while testing: URL/view, user action, and observable result. Use these unchanged as unchecked PR manual-test steps.
+4. Record completed validation evidence: URL/view, user action, observable result, and console or network status when relevant. Evidence is a record of work already performed, not a checklist for the user.
 
 For changes not observable in the browser, run and record an equivalent CLI or API scenario. Never silently skip this validation record. Re-run the affected scenario after a user-visible fix.
 
+Do not ask the user to repeat validation per PR. The owner's normal daily use of staging is a separate confidence loop, not a merge gate or an itemized acceptance pass.
+
 ## 3. Review correctness
 
 Review the full branch diff, not only unstaged files. Look for state/race/cleanup errors, boundaries, keyboard/pointer/scroll/focus regressions, accessibility, security/privacy/auth/data-loss risks, and missing regression coverage.
@@ -58,7 +60,18 @@ Use `codex-simplify-code` on the diff from the base branch. Reduce duplication,
 - Revalidate user-visible behavior after behavior-adjacent cleanup.
 - Capture what changed, or that no simplification opportunity existed, and justify any retained or new `useEffect`, `useRef`, or `useState` for the PR.
 
-## 6. Final local gate
+## 6. Independent review and final local gate
+
+Run a fresh, read-only reviewer agent against the final branch diff after simplification. Spawn it without inherited conversation context (`fork_turns: "none"`) and provide a self-contained task with the worktree path, base ref, task intent, and repository instructions. This review is internal to `ship`: the user should not need another command, approval, or review step.
+
+Give the reviewer the task intent, applicable `AGENTS.md`, and the complete base-to-head diff without the implementation agent's conclusions. Direct it to:
+
+1. Start from changed lines and form concrete risk questions.
+2. Narrow with `rg` or file discovery before reading the smallest relevant caller, contract, and test ranges.
+3. Report only confirmed, actionable defects with severity, path/line, impact, and supporting evidence; omit style preferences and speculative findings.
+4. Cover correctness, regression coverage, performance, accessibility, security/privacy/data loss, simplicity, and maintainability as applicable.
+
+Fix confirmed findings, revalidate them, and commit the review fixes separately with a conventional message before running a fresh confirmation review. Repeat only when a confirmed finding changed the diff. Stop if a fresh reviewer is unavailable, a finding is uncertain, or confidence remains incomplete; do not substitute self-review or hand validation back to the user.
 
 Run checks chosen from the actual diff and repository guidance. At minimum run focused regression tests, affected type checking, and lint; run broader package tests for shared contracts. Use `bun run verify` only after inspecting its selected checks. Confirm the diff and worktree are clean except for intentional, committed work; distinguish new failures from existing warnings.
 
@@ -73,14 +86,14 @@ Use these sections in order:
 
 ## Simplicity
 
-## Manual Testing Steps
-- [ ] Reviewer-visible golden-path and diff-derived edge-case steps
+## Automated validation
+
+## Independent review
 
 ## Test plan
-- [x] Commands actually run
 ```
 
-Write manual steps in user-observable language—never function, variable, or file names. Close the originating issue only when unambiguous.
+Under automated validation, record the completed browser, CLI, or API scenarios and their observed results. Under independent review, record the final reviewer result and any confirmed findings fixed. Under test plan, list commands actually run. Do not include manual-testing sections, unchecked boxes, or tasks for the user. Close the originating issue only when unambiguous.
 
 ## 8. Monitor CI and feedback
 
@@ -90,7 +103,7 @@ Inspect unresolved review threads or requested changes. Address actionable feedb
 
 ## 9. Squash merge
 
-Merge only when required checks and reviews are satisfied, the PR targets the expected base, and local validation supports confidence. Compass normally squash-merges; use the repository’s observed strategy and delete the remote branch only when normal. Never merge on a guess.
+Merge only when required checks and the independent review gate are satisfied, the PR targets the expected base, and local validation supports confidence. Compass normally squash-merges; use the repository’s observed strategy and delete the remote branch only when normal. Never merge on a guess.
 
 ## 10. Monitor main release
 
@@ -100,4 +113,4 @@ Treat tag, publication, deployment, migration, or health-check failures as incid
 
 ## Report
 
-Lead with the shipped result. Include the PR and merge result, implementation and simplification commits, local/manual validation, PR CI, post-merge release/deployment result and tag, plus pre-existing warnings or follow-up risks. Emit git directives only for actions that actually succeeded.
+Lead with the shipped result. Include the PR and merge result, implementation and simplification commits, automated/browser validation, independent review, PR CI, post-merge release/deployment result and tag, plus pre-existing warnings or follow-up risks. Emit git directives only for actions that actually succeeded.

.github/PULL_REQUEST_TEMPLATE.md

@@ -2,23 +2,18 @@
 
 <!-- What changed and why. -->
 
-## Test plan
+## Simplicity
+
+<!-- What was simplified, or why the implementation is already minimal. -->
+
+## Automated validation
 
-<!-- How you verified this: commands run, manual steps, screenshots/recordings. -->
+<!-- Completed browser, CLI, or API scenarios and their observed results. -->
 
-## Accessibility checklist
+## Independent review
 
-Only for UI changes. Skip anything not touched by this PR - see the
-"Accessibility Testing" section of
-[docs/development/testing-playbook.md](../docs/development/testing-playbook.md)
-for what automation already covers.
+<!-- Final result from the fresh diff-first reviewer, including confirmed findings fixed. -->
+
+## Test plan
 
-- [ ] Keyboard-only: reached and operated the changed UI with Tab/Shift+Tab
-      and arrow keys, with a visible focus indicator throughout
-- [ ] Exercised dynamic content the changed flow introduces (dialogs, menus,
-      validation/error states) rather than just the resting page
-- [ ] Checked browser zoom/reflow at 200% and 400% if layout changed
-- [ ] Checked screen-reader names/announcements (e.g. VoiceOver) if roles,
-      labels, or live regions changed
-- [ ] Checked reduced-motion / forced-colors behavior if this PR touches
-      animation or relies on color alone
+<!-- Commands actually run. Do not add manual tasks or unchecked boxes. -->

docs/development/testing-playbook.md

@@ -328,7 +328,7 @@ conformance:
 | React Testing Library | Roles, accessible names, state, focus, and keyboard behavior | Focused web tests and `bun run test:web` |
 | Playwright with axe | Automatically detectable issues in realistic rendered states | `bun run test:a11y`, `bun run verify web`, and E2E CI |
 | Targeted browser assertions | Regressions generic rules cannot reliably model, such as transient contrast or focus behavior | The relevant E2E regression test |
-| Manual review | Keyboard usability, zoom/reflow, screen-reader experience, and other judgment-based WCAG criteria | Major UI changes and periodic audits |
+| Daily staging use and periodic audits | Broad usability regressions and judgment-based WCAG criteria | Once-daily normal use and separately scoped audits, never per-PR checklists |
 
 #### How Axe Coverage Works
 
@@ -393,7 +393,8 @@ at the call site. Unexplained `incomplete` results seen in this suite today
 are `color-contrast` (axe cannot compute a background across an
 absolutely-positioned overlap or a CSS gradient, and cannot reliably sample
 single-character date-cell text) - these are exactly the states the
-datepicker's targeted contrast test and manual review already cover.
+datepicker's targeted contrast test covers or periodic accessibility audits
+must inspect.
 
 #### Representative Checkpoints
 
@@ -443,22 +444,18 @@ Test meaningful outcomes such as focus movement, updated `aria-expanded` or
 A successful `getByRole` query is useful pressure toward semantic markup, but
 it is not an accessibility audit by itself.
 
-#### Manual Review
+#### Daily Staging Confidence
 
-For a major UI change, verify the affected flow with:
+Once per day, use the accumulated staging build as a normal Compass user. This
+is an unstructured product-confidence pass, not a PR checklist: do not replay
+individual changes or map observations back to every merged PR. File any
+regression observed during normal use as a bug.
 
-- keyboard-only navigation, including visible focus and a logical focus order
-- browser zoom and reflow at 200% and 400%
-- VoiceOver on macOS for names, roles, state changes, and announcements
-- reduced-motion and forced-colors/high-contrast settings when relevant
-- a free browser audit such as Accessibility Insights for Web
-
-Record the states reviewed in the PR - `.github/PULL_REQUEST_TEMPLATE.md` has a
-short accessibility checklist for exactly the judgment calls automation can't
-make (keyboard reach, dynamic content exercised, zoom/reflow, screen-reader
-names, reduced-motion/forced-colors). Automated checks passing means no tested
-automated violation was found; it does not mean the page conforms to every WCAG
-success criterion.
+Per-PR confidence comes from automated tests, agent-driven browser validation,
+and the independent diff-first review. If those gates cannot establish
+correctness, `ship` pauses instead of delegating manual verification to the
+owner. Separately scoped accessibility audits can still examine judgment-based
+WCAG criteria without becoming a release requirement for each PR.
 
 #### Workflow For UI Changes
 
@@ -469,7 +466,8 @@ success criterion.
 4. Add a targeted browser regression only for behavior the general scan does
    not protect.
 5. Run `bun run test:web`, `bun run lint`, and `bun run verify web`.
-6. For major UI changes, complete and document the relevant manual checks.
+6. Record completed agent-driven browser validation in the PR. Do not add
+   manual tasks; pause `ship` if the behavior cannot be confirmed.
 
 Browser-based accessibility checks are appropriate for CI and pre-push
 verification. Keep commit hooks limited to fast static checks so normal commits