feat: broaden E2E visual review trigger and add review-page build skill

.agents/skills/app-test-review/SKILL.md

@@ -0,0 +1,40 @@
+---
+name: app-test-review
+description: >-
+  Firstmate's own procedure for building the captain's interactive review page from visual evidence a crewmate captured under data/<task-id>/e2e-review/.
+  Covers when to build it, where the built page lives, what it must contain, and the lavish-axi mechanism used to build it.
+  Load whenever a task's PR-ready or done report points at that evidence, before that captain-facing checkpoint.
+icon: 'i-heroicons-play-circle'
+user-invocable: false
+metadata:
+  internal: true
+---
+
+# app-test-review
+
+Load this whenever a task's PR-ready or `done` report points at evidence under `data/<task-id>/e2e-review/`.
+It defines when firstmate builds the captain's review page from that evidence, where the built page lives, what it must contain, and how it gets built.
+Building the page is firstmate's own job; the crewmate skill `e2e-visual-review` never does this itself - see its "Your responsibility boundary" section.
+
+## When
+
+Build the review page whenever a task that left evidence at `data/<task-id>/e2e-review/` reaches a captain-facing checkpoint: PR ready or `done`.
+This is the default review artifact for that change, not an optional extra reserved for explicit requests.
+A task with no `data/<task-id>/e2e-review/` evidence means there is nothing to build; skip this silently.
+
+## Where
+
+Build the page at `.lavish/<project>/<type>-<task-id>/review.html` inside this firstmate home.
+`<project>` is the project the task shipped against; `<type>` is `feature-review` or `bug-detection`, whichever best describes the nature of the change.
+This location is captain-private, gitignored, and durable: it survives the crewmate's worktree teardown, unlike `data/<task-id>/e2e-review/` itself, which does not survive `/stow`-style long-term cleanup the same way.
+
+## Contents
+
+Embed every screenshot and video the crewmate captured directly in the page - inline as `data:` URIs or otherwise self-contained per Lavish's own constraints.
+Never merely link to the evidence files in `data/<task-id>/e2e-review/`; that directory is not guaranteed to persist alongside the built page.
+Read the crewmate's `manifest.md` for what each file shows and at what point in the run, and use it to caption and order the evidence.
+
+## Mechanism
+
+Build the page with `lavish-axi`, following its own conventions: run `lavish-axi playbook <playbook_id>` for relevant guidance (the `code`, `diagram`, or `comparison` playbooks may apply depending on what the evidence shows) and consult its help output rather than reinventing structure by hand.
+This gives the captain the normal interactive Lavish review surface - annotation, queued feedback, and `lavish-axi poll` - rather than a static dump.

.agents/skills/e2e-visual-review/SKILL.md

@@ -0,0 +1,46 @@
+---
+name: e2e-visual-review
+description: >-
+  Fleet-wide convention for capturing visual evidence whenever a task runs a test suite that exercises the running application - E2E suites, UI/browser-driven tests, or visual regression checks - as distinct from plain unit or logic tests.
+  Covers what to capture, where it goes as a named exception to worktree isolation, and the crewmate's responsibility boundary against firstmate's own review-page tooling.
+  Loaded via the ship and scout brief pointer whenever that scope applies.
+icon: 'i-heroicons-film'
+user-invocable: false
+metadata:
+  internal: true
+---
+
+# e2e-visual-review
+
+Load this on any task, ship or scout, whose test run exercises the running application - an E2E suite, UI/browser-driven tests, or a visual regression check.
+It defines what visual evidence to capture from that run, where to leave it, and where your responsibility ends.
+
+## Scope
+
+Applies whenever your task runs a test suite that exercises the running application: E2E suites, UI/browser-driven tests, and visual regression checks.
+Plain unit or logic tests never trigger this - they have nothing meaningful to screenshot, and keep reporting pass/fail exactly as before.
+A project with no app-facing test suite means this section is a no-op: do not add one, invent one, or run an unrelated test suite just to trigger this protocol.
+Do not start a separate, redundant run solely to record it; capture happens as part of the same run you were already performing for the task.
+
+## What to capture
+
+Framework-agnostic, using whatever the test framework already supports natively:
+
+- Screenshots at meaningful checkpoints during the run (most frameworks already support this, e.g. Playwright's `page.screenshot()`).
+- A full video recording of the run, if the framework has native support for it (e.g. Playwright's `recordVideo` option on `launch()`/`launchPersistentContext()`, Cypress's built-in video capture).
+- If the framework has no native recording capability, screenshots alone are acceptable.
+  Do not build a custom recording mechanism from scratch.
+
+## Where it goes
+
+Your project worktree is disposable and torn down after the task, so evidence must not be left only there.
+
+Write every captured screenshot and video, plus a short `manifest.md` describing what each file shows and at what point in the run, to `data/<task-id>/e2e-review/` in the firstmate home.
+This is a sibling location to the already-established `data/<task-id>/report.md` and the status file, both of which are named exceptions to the ship/scout brief's worktree-isolation rule.
+This is a third such named exception, not a general license to write elsewhere.
+
+## Your responsibility boundary
+
+You do not invoke `lavish-axi` yourself or otherwise manage a Lavish review-session lifecycle.
+Building or opening the actual review page from your evidence is firstmate's own operational job, not yours - see `app-test-review` for that side of the contract.
+Your scope ends at producing well-located, well-labeled evidence, plus a one-line mention of the `data/<task-id>/e2e-review/` path in your `done`/status report so firstmate knows where to find it.

AGENTS.md

@@ -82,6 +82,7 @@ data/                personal fleet records; LOCAL, gitignored as a whole
   secondmates.md      secondmate routing table; firstmate-private, maintained by fm-home-seed.sh (section 6)
   <id>/brief.md      per-task crewmate brief, or per-secondmate charter brief when kind=secondmate
   <id>/report.md     scout task deliverable, written by the crewmate; survives teardown
+  <id>/e2e-review/   visual evidence (screenshots, video, manifest.md) from a task whose test run exercised the running app (E2E, UI/browser-driven, or visual regression); conditional, absent otherwise; see e2e-visual-review skill
 projects/            cloned repos; gitignored; READ-ONLY for you
 state/               volatile runtime signals; gitignored
   <id>.status        appended by crewmates: "<state>: <note>" wake-event lines, not current-state truth
@@ -112,6 +113,7 @@ state/               volatile runtime signals; gitignored
   .last-watcher-beat watcher liveness beacon, touched every poll (including while absorbing benign wakes); guard scripts read it
   .subsuper-* .supervise-daemon.*   sub-supervisor internals; never touch
 .no-mistakes/        local validation state and evidence; gitignored
+.lavish/<project>/<type>-<task-id>/review.html   built captain review pages (`<type>` = feature-review or bug-detection) from crewmate-captured app-test evidence; LOCAL, gitignored, durable across worktree teardown; see app-test-review skill
 ```
 
 A `state/<id>.status` line is a wake event, not current-state truth; `bin/fm-crew-state.sh` owns current-state reconciliation.
@@ -278,6 +280,7 @@ The path's worker, automated gates, and captain approval remain authoritative:
 
 Documentation is required on all three tiers; the ship brief's `# Documentation` section and `project-docs` skill own the contract, and missing docs are never acceptable on any path.
 Versioning follows the same automatic-bump/escalate-on-breaking-change split; the ship brief's `# Versioning` section and `version-management` skill own that contract.
+Any task whose test run exercises the running application (E2E, UI/browser-driven, or visual regression - not plain unit/logic tests) captures visual evidence for captain review; the ship brief's `# E2E Visual Review` section and `e2e-visual-review` skill own that contract.
 
 Delivery mode and `yolo` are orthogonal.
 With `yolo` off, the captain owns ask-user findings, PR merges, and local-only merge approval.
@@ -312,6 +315,7 @@ Run `bin/fm-pr-check.sh <id> <PR url>` - it records `pr=` and the forge's `pr_he
 Tell the captain the PR's full URL, always the complete `https://...` link rather than a bare `#number`, a concise outcome summary, and the no-mistakes risk level when applicable.
 A captain instruction to merge is explicit authority; `yolo` is the only standing routine authority.
 For any custom `state/<id>.check.sh` you write yourself, keep it an ordinary single-link mode-`0700` file, print one line only when firstmate should wake, print nothing otherwise, finish before `FM_CHECK_TIMEOUT`, then bind its current bytes with `bin/fm-check-register.sh <id>` before the watcher may execute it.
+When the reporting task left evidence at `data/<task-id>/e2e-review/`, load `app-test-review` and build the captain's review page before or alongside that PR-ready or `done` report; this is the default review artifact for that change, not an optional extra.
 
 Tear down a ship task only after landing is confirmed.
 A teardown refusal for uncommitted or unlanded work is a stop-and-investigate result, never an obstacle to bypass.
@@ -480,6 +484,7 @@ These skills are not captain-invocable; load them only at their precise triggers
 - `stuck-crewmate-recovery` - load when the session-start digest reports an ordinary direct report's endpoint dead or its metadata has no window, or after a stale wake, looping pane, repeated confusion, an answered-by-brief question, an unresponsive crewmate, or a failed steer.
 - `secondmate-provisioning` - load before creating, seeding, validating, launching, handing backlog to, recovering, pushing inherited local material into, or retiring a secondmate home, and before editing `data/secondmates.md`.
 - `decision-hold-lifecycle` - load before treating an investigation or visual review as complete, before ending a visual review that exposed a decision, and when recording or routing the captain's answer.
+- `app-test-review` - load whenever a task's PR-ready or `done` report points at evidence under `data/<task-id>/e2e-review/`, before that captain-facing checkpoint; builds the interactive Lavish review page from the crewmate's captured evidence.
 - `fmx-respond` - load on an `x-mention <request_id>` `check:` wake to handle the mention, on an `x-mode-error ...` `check:` wake to report the X-mode configuration blocker, and on any milestone or terminal wake for an X-mode-linked task before posting its completion follow-up; relevant only when X mode is on.
 - `firstmate-codexapp` - load before coordinating a visible Codex Desktop thread, evaluating a Codex App backend request, or reconciling Codex Desktop host-tool smoke evidence for Firstmate work.
 - `firstmate-coding-guidelines` - load before changing firstmate's shared, tracked material, as defined by section 1's list, whether editing directly or briefing a crewmate for a firstmate-repo task.

bin/fm-brief.sh

@@ -243,7 +243,7 @@ The report is the only thing that survives, so anything worth keeping must be in
 
 # Rules
 1. Never push to any remote and never open a PR.
-2. Stay inside this worktree; the only files you may write outside it are the report and the status file below.
+2. Stay inside this worktree; the only files you may write outside it are the report, the status file below, and the E2E visual-review evidence path when that section below applies.
 3. Use gh-axi for GitHub operations and chrome-devtools-axi for browser operations.
 4. Report status by appending one line:
    \`echo "{state}: {one short line}" >> $STATUS_FILE\`
@@ -263,6 +263,10 @@ The report is the only thing that survives, so anything worth keeping must be in
    every lane/home, so restarting it kills other lanes' in-flight pipeline runs. On ANY no-mistakes
    daemon error, append \`blocked: {the daemon error}\` and stop; only firstmate manages the daemon.
 
+# E2E Visual Review
+Only if this task runs a test suite that exercises the running application - E2E, UI/browser-driven, or visual regression, never plain unit/logic tests: load \`$FM_ROOT/.agents/skills/e2e-visual-review/SKILL.md\` for the full capture-and-placement protocol.
+\`$DATA/$ID/e2e-review/\` is a permitted exception to "stay inside this worktree", alongside the report and the status file.
+
 # Definition of done
 Write your findings to \`$DATA/$ID/report.md\`.
 The report must stand alone: what you did, what you found, the evidence (commands run, output, file:line references), and what you recommend.
@@ -352,7 +356,7 @@ If the top-level path is the primary checkout or not the worktree you were launc
 
 # Rules
 $RULE1
-2. Stay inside this worktree; modify nothing outside it.
+2. Stay inside this worktree; the only files you may write outside it are the status file below and the E2E visual-review evidence path when that section below applies.
 3. Use gh-axi for GitHub operations and chrome-devtools-axi for browser operations.
 4. Report status by appending one line:
    \`echo "{state}: {one short line}" >> $STATUS_FILE\`
@@ -393,6 +397,10 @@ Documentation is required on every delivery tier - no-mistakes, direct-PR, and l
 Load \`$FM_ROOT/.agents/skills/version-management/SKILL.md\` for the full semantic-versioning protocol.
 Bump PATCH or MINOR automatically from the branch's Conventional Commit types, in the same commit as the change; never bump MAJOR yourself - a breaking change is a \`needs-decision:\` finding.
 
+# E2E Visual Review
+Only if this task runs a test suite that exercises the running application - E2E, UI/browser-driven, or visual regression, never plain unit/logic tests: load \`$FM_ROOT/.agents/skills/e2e-visual-review/SKILL.md\` for the full capture-and-placement protocol.
+\`$DATA/$ID/e2e-review/\` is a permitted exception to "stay inside this worktree", alongside the status file.
+
 $DOD
 EOF
 echo "scaffolded: $BRIEF (ship, mode=$MODE; replace {TASK})"

docs/documentation-audiences.json

@@ -111,6 +111,10 @@
       "path": ".agents/skills/ahoy/SKILL.md",
       "audience": "agent-runtime"
     },
+    {
+      "path": ".agents/skills/app-test-review/SKILL.md",
+      "audience": "agent-runtime"
+    },
     {
       "path": ".agents/skills/ask-user-authority/SKILL.md",
       "audience": "agent-runtime"
@@ -131,6 +135,10 @@
       "path": ".agents/skills/diagnostic-reasoning/SKILL.md",
       "audience": "agent-runtime"
     },
+    {
+      "path": ".agents/skills/e2e-visual-review/SKILL.md",
+      "audience": "agent-runtime"
+    },
     {
       "path": ".agents/skills/firstmate-codexapp/SKILL.md",
       "audience": "agent-runtime"

tests/fm-brief.test.sh

@@ -136,6 +136,50 @@ test_ship_project_memory_wording() {
   pass "fm-brief.sh: ship project-memory wording carries the AGENTS.md authoring bar"
 }
 
+test_ship_e2e_visual_review_pointer() {
+  local home id brief
+  home="$TMP_ROOT/e2e-visual-review-home"
+  mkdir -p "$home/data"
+  id="brief-e2e-visual-review-c2"
+  FM_HOME="$home" FM_ROOT_OVERRIDE="$ROOT" "$ROOT/bin/fm-brief.sh" "$id" some-proj >/dev/null 2>&1
+  brief="$home/data/$id/brief.md"
+  assert_present "$brief" "brief was not scaffolded"
+  assert_grep "# E2E Visual Review" "$brief" \
+    "ship brief missing the E2E Visual Review section"
+  assert_grep "$ROOT/.agents/skills/e2e-visual-review/SKILL.md" "$brief" \
+    "ship brief did not point at the e2e-visual-review skill"
+  assert_grep "$home/data/$id/e2e-review/" "$brief" \
+    "ship brief did not name the e2e-review evidence path"
+  assert_grep "permitted exception to \"stay inside this worktree\", alongside the status file" "$brief" \
+    "ship brief did not name the e2e-review path as a worktree-isolation exception"
+  assert_grep "test suite that exercises the running application - E2E, UI/browser-driven, or visual regression, never plain unit/logic tests" "$brief" \
+    "ship brief did not carry the broadened app-facing-test trigger scope"
+  assert_grep "the only files you may write outside it are the status file below and the E2E visual-review evidence path" "$brief" \
+    "ship brief Rule 2 still contradicts its own E2E Visual Review worktree exception"
+  pass "fm-brief.sh: ship briefs point at the e2e-visual-review protocol"
+}
+
+test_scout_e2e_visual_review_pointer() {
+  local home id brief
+  home="$TMP_ROOT/e2e-visual-review-scout-home"
+  mkdir -p "$home/data"
+  id="brief-e2e-visual-review-scout-c3"
+  FM_HOME="$home" FM_ROOT_OVERRIDE="$ROOT" "$ROOT/bin/fm-brief.sh" "$id" some-proj --scout >/dev/null 2>&1
+  brief="$home/data/$id/brief.md"
+  assert_present "$brief" "scout brief was not scaffolded"
+  assert_grep "# E2E Visual Review" "$brief" \
+    "scout brief missing the E2E Visual Review section"
+  assert_grep "$ROOT/.agents/skills/e2e-visual-review/SKILL.md" "$brief" \
+    "scout brief did not point at the e2e-visual-review skill"
+  assert_grep "$home/data/$id/e2e-review/" "$brief" \
+    "scout brief did not name the e2e-review evidence path"
+  assert_grep "permitted exception to \"stay inside this worktree\", alongside the report and the status file" "$brief" \
+    "scout brief did not name the e2e-review path as a worktree-isolation exception"
+  assert_grep "test suite that exercises the running application - E2E, UI/browser-driven, or visual regression, never plain unit/logic tests" "$brief" \
+    "scout brief did not carry the broadened app-facing-test trigger scope"
+  pass "fm-brief.sh: scout briefs point at the e2e-visual-review protocol"
+}
+
 test_herdr_lab_contract_is_explicit_and_complete() {
   local home id brief
   home="$TMP_ROOT/herdr-lab-home"
@@ -388,6 +432,8 @@ test_ship_modes_generate_clean_briefs
 test_faster_paths_use_configured_authority_without_stacked_review
 test_no_mistakes_dod_wording
 test_ship_project_memory_wording
+test_ship_e2e_visual_review_pointer
+test_scout_e2e_visual_review_pointer
 test_herdr_lab_contract_is_explicit_and_complete
 test_herdr_lab_contract_quotes_foreign_firstmate_path
 test_herdr_lab_omission_is_loud_for_ship_and_scout