diff --git a/.agents/skills/app-test-review/SKILL.md b/.agents/skills/app-test-review/SKILL.md new file mode 100644 index 0000000000..85847ffa98 --- /dev/null +++ b/.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//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//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//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//e2e-review/` evidence means there is nothing to build; skip this silently. + +## Where + +Build the page at `.lavish//-/review.html` inside this firstmate home. +`` is the project the task shipped against; `` 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//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//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 ` 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. diff --git a/.agents/skills/e2e-visual-review/SKILL.md b/.agents/skills/e2e-visual-review/SKILL.md new file mode 100644 index 0000000000..cdd0bc6ca4 --- /dev/null +++ b/.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//e2e-review/` in the firstmate home. +This is a sibling location to the already-established `data//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//e2e-review/` path in your `done`/status report so firstmate knows where to find it. diff --git a/AGENTS.md b/AGENTS.md index b1c7ccff19..20355a19ea 100644 --- a/AGENTS.md +++ b/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) /brief.md per-task crewmate brief, or per-secondmate charter brief when kind=secondmate /report.md scout task deliverable, written by the crewmate; survives teardown + /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 .status appended by crewmates: ": " 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//-/review.html built captain review pages (`` = feature-review or bug-detection) from crewmate-captured app-test evidence; LOCAL, gitignored, durable across worktree teardown; see app-test-review skill ``` A `state/.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 ` - 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/.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 ` before the watcher may execute it. +When the reporting task left evidence at `data//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//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 ` `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. diff --git a/bin/fm-brief.sh b/bin/fm-brief.sh index 07fac122d6..d6b04680f3 100755 --- a/bin/fm-brief.sh +++ b/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})" diff --git a/docs/documentation-audiences.json b/docs/documentation-audiences.json index 10967832bd..d253bada7f 100644 --- a/docs/documentation-audiences.json +++ b/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" diff --git a/tests/fm-brief.test.sh b/tests/fm-brief.test.sh index 74690eab42..346478b0d0 100755 --- a/tests/fm-brief.test.sh +++ b/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