diff --git a/.agents/skills/no-mistakes/SKILL.md b/.agents/skills/no-mistakes/SKILL.md index d2fe5bcafd0..6e3e7569cbd 100644 --- a/.agents/skills/no-mistakes/SKILL.md +++ b/.agents/skills/no-mistakes/SKILL.md @@ -131,9 +131,15 @@ Run the pipeline and decide on its findings as they come up: outcomes are: - `checks-passed` - the change is validated and CI is green, but the PR is not merged yet. **You are done driving the pipeline.** Do not wait for the - merge: tell the user the PR is ready and ask them to review and merge it - (the PR link is in the `help` line). no-mistakes keeps monitoring the PR - in the background, so a human can watch it in the TUI. + merge: before telling the user or a maintainer that the PR is ready, run + the `pr-readiness` skill if this repo has it. + It refreshes live GitHub/base + facts, checks mergeability and overlap with current base, verifies public + PR text, and blocks stale, conflicting, unverified, or overly broad PRs. + If that audit passes, tell the user the PR is ready and ask them to review + and merge it (the PR link is in the `help` line). + no-mistakes keeps + monitoring the PR in the background, so a human can watch it in the TUI. - `passed` - the changes cleared the gate and the PR was merged or closed. - `failed` or `cancelled` - they did not; read the output and address it. Fix whatever the output points at (a failing test, a lint error, a finding diff --git a/.agents/skills/pr-readiness/SKILL.md b/.agents/skills/pr-readiness/SKILL.md new file mode 100644 index 00000000000..55dfe022a9b --- /dev/null +++ b/.agents/skills/pr-readiness/SKILL.md @@ -0,0 +1,124 @@ +--- +name: pr-readiness +description: >- + Prepare, audit, or rewrite a pull request before it is opened, updated, + commented on, or presented to a maintainer. + Use when a user asks to create a PR, ask someone to merge a PR, replace an + upstream PR from a fork, comment on a PR, publish branch work, or make sure a + PR is maintainer-ready. +--- + +# pr-readiness + +Use this before any public PR action: opening a PR, updating a PR body, asking a +maintainer to merge, commenting with a replacement branch, or presenting a PR as +ready. +The goal is to avoid wasting maintainer attention with stale branches, +missing research, internal transcript language, or weak validation. + +## Required checks + +1. Refresh the base facts for the actual action. + + For an existing PR, capture both the base branch and the base repository + before comparing or fetching: + ```sh + gh-axi pr view --json number,title,author,mergeStateStatus,statusCheckRollup,changedFiles,additions,deletions,url + gh-axi api repos///pulls/ --jq '{mergeable,mergeable_state,rebaseable,head:{repo:.head.repo.full_name,ref:.head.ref,sha:.head.sha},base:{repo:.base.repo.full_name,ref:.base.ref,sha:.base.sha}}' + BASE_REPO="$(gh-axi api repos///pulls/ --jq '.base.repo.full_name')" + BASE_REF="$(gh-axi api repos///pulls/ --jq '.base.ref')" + BASE_REMOTE="refs/remotes/pr-base/${BASE_REF}" + git fetch "https://github.com/${BASE_REPO}.git" "refs/heads/${BASE_REF}:${BASE_REMOTE}" + ``` + + Before a PR exists, derive the intended base from the local branch and fetch + that base directly. + For fork or replacement workflows, set + `TARGET_BASE_REPO` to the intended upstream repository rather than assuming + `origin`. + ```sh + BRANCH="$(git branch --show-current)" + BASE_REPO="${TARGET_BASE_REPO:-$(gh-axi repo view --json nameWithOwner --jq '.nameWithOwner')}" + BASE_REF="${TARGET_BASE_REF:-$(git config "branch.${BRANCH}.gh-merge-base" || true)}" + BASE_REF="${BASE_REF:-$(gh-axi repo view "${BASE_REPO}" --json defaultBranchRef --jq '.defaultBranchRef.name')}" + BASE_REMOTE="refs/remotes/pr-base/${BASE_REF}" + git fetch "https://github.com/${BASE_REPO}.git" "refs/heads/${BASE_REF}:${BASE_REMOTE}" + git status --short + git branch -vv + ``` + Use `gh-axi`, not raw `gh`, in this repo. + +2. Compare against current base, not the branch's old base. + ```sh + git log --oneline --cherry-pick --right-only "${BASE_REMOTE}"...HEAD + git diff --name-status "${BASE_REMOTE}"...HEAD + git merge-tree "$(git merge-base "${BASE_REMOTE}" HEAD)" "${BASE_REMOTE}" HEAD + ``` + If the branch is dirty/conflicting, rebase or rebuild before asking anyone to + review or merge it. + +3. Check overlap and supersession. + - Inspect recently merged commits on `${BASE_REMOTE}`. + - Inspect adjacent open PRs that touch the same files or subsystem. + - Decide whether the change is still needed, partly superseded, or should be + split into smaller PRs. + +4. Verify the change. + - Run focused tests for the changed behavior. + - Run lint/syntax checks for touched languages. + - For code changes in firstmate, run the no-mistakes gate once the work is + committed unless the user explicitly chooses a lighter path. + - Do not claim GitHub CI passed unless GitHub actually shows passing checks. + +5. Inspect public text. + Remove local/internal language before opening, updating, or commenting: + - no "Captain" address; + - no local transcript/intake details; + - no firstmate operational chatter unless it is relevant to the upstream repo; + - no "validated locally" claim without the exact commands and outcome; + - no request to merge a conflicting, stale, or unverified branch. + +## Maintainer-facing format + +Prefer this structure: + +```markdown +## What + +One short paragraph describing the bug or capability. + +## Why + +The concrete failure mode or maintainer-relevant motivation. + +## Changes + +- Specific implementation points. +- Any deliberate scope limits. + +## Validation + +- `command` - outcome +- GitHub checks: passed / unavailable with reason + +## Notes + +Compatibility, risk, or follow-up work if relevant. +``` + +For replacement PRs from a fork, also state what happened to the original PR: + +```markdown +This is a rebased/reworked replacement for #. +I kept , dropped , and retested against current base. +``` + +## Stop conditions + +Do not proceed publicly without telling the captain if any of these are true: + +- GitHub reports `mergeable=false`, `mergeable_state=dirty`, or not rebaseable. +- The PR has no GitHub checks and the repo normally expects checks. +- The PR body still contains internal transcript language. +- The branch includes unrelated fixes that should be split. +- Recent upstream commits may have already solved the same problem. diff --git a/AGENTS.md b/AGENTS.md index f268e4e6210..0a0269e9098 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -309,7 +309,7 @@ Do not eagerly backfill every project. **Delivery mode (choose at add).** `` is how a finished change reaches `main`, picked per project when you add it and recorded in the registry line (`fm-project-mode.sh` parses it; `fm-spawn` records it into each task's meta): - `no-mistakes` (default; `[...]` may be omitted) - full pipeline -> PR -> captain merge. Highest assurance. -- `direct-PR` - push + open a PR via `gh-axi`, no pipeline -> captain merge. +- `direct-PR` - run `pr-readiness`, push + open a PR via `gh-axi`, no pipeline -> captain merge. - `local-only` - local branch, no remote, no PR; firstmate reviews the diff, the captain approves, firstmate merges to local `main` (section 7). Orthogonal to mode is an optional `+yolo` flag (`[direct-PR +yolo]`), default off and **not recommended**: with `yolo` on, firstmate makes the approval decisions itself instead of asking the captain (section 7). When the captain adds a project without saying, default to `no-mistakes` with yolo off; only set a faster mode or `+yolo` on the captain's explicit say-so. @@ -411,8 +411,15 @@ Its charter retargets escalation to the main firstmate's status file, so routine A ship task's path from `done` to landed on `main` is set by the project's `mode` (recorded in meta; section 6); `yolo` decides who approves. The Validate / PR ready / Ship teardown stages below are written for the `no-mistakes` path; the other modes diverge: - **no-mistakes** - the stages below as written: no-mistakes validation pipeline -> PR -> captain merge. -- **direct-PR** - no pipeline. The crewmate pushes and opens the PR itself (its brief says so) and reports `done: PR `. Skip the Validate step and go straight to PR ready (run `fm-pr-check`, relay the PR). Teardown uses the normal pushed-branch check. -- **local-only** - no remote, no PR. The crewmate stops at `done: ready in branch fm/`. Review the diff with `bin/fm-review-diff.sh `, relay a one-paragraph summary to the captain, and on approval run `bin/fm-merge-local.sh ` to fast-forward local `main` (it refuses anything but a clean fast-forward - if it does, have the crewmate rebase). No `fm-pr-check`. Then teardown, whose safety check requires the branch already merged into local `main`, OR the work pushed to any remote (a fork counts - relevant for upstream-contribution PRs on a local-only-registered project). +- **direct-PR** - no pipeline. + The crewmate runs the `pr-readiness` audit, then pushes and opens the PR itself (its brief says so) and reports `done: PR `. + Skip the Validate step and go straight to PR ready (run the same readiness audit on the opened PR if facts may have changed, then `fm-pr-check`, then relay the PR). + Teardown uses the normal pushed-branch check. +- **local-only** - no remote, no PR. + The crewmate stops at `done: ready in branch fm/`. + Review the diff with `bin/fm-review-diff.sh `, relay a one-paragraph summary to the captain, and on approval run `bin/fm-merge-local.sh ` to fast-forward local `main` (it refuses anything but a clean fast-forward - if it does, have the crewmate rebase). + No `fm-pr-check`. + Then teardown, whose safety check requires the branch already merged into local `main`, OR the work pushed to any remote (a fork counts - relevant for upstream-contribution PRs on a local-only-registered project). When reviewing any crewmate branch diff, use `bin/fm-review-diff.sh ` rather than `git diff ...branch` directly. Pooled clones keep their local default refs frozen at clone time and can lag `origin`; the helper always compares against the authoritative base. @@ -437,11 +444,14 @@ Use chat for yes/no decisions; use lavish-axi when there are multiple findings o ### PR ready For PR-based ship tasks, the ready signal depends on mode: `no-mistakes` reports `done: PR checks green` after CI is green, while `direct-PR` reports `done: PR ` after opening the PR. +Before telling the captain the PR is ready, asking a maintainer to review or merge, updating PR public text, or commenting with a replacement branch, use the `pr-readiness` skill to audit live GitHub/base facts, mergeability, current-base overlap, validation, and maintainer-facing text. +If that audit finds stale base, conflicts, missing checks, internal transcript language, or broad unrelated scope, stop and resolve it before any public PR action. Run `bin/fm-pr-check.sh ` - it records `pr=` in the task's meta and arms the watcher's merge poll. Tell the captain: the PR's full URL (always the complete `https://...` link, never a bare `#number` - the captain's terminal makes a full URL clickable), a one-paragraph summary, and, for `no-mistakes`, the risk level it emitted. (The check contract, for any custom `state/.check.sh` you write yourself: print one line only when firstmate should wake, print nothing otherwise, and finish before `FM_CHECK_TIMEOUT`.) -If the captain says "merge it", run `gh-axi pr merge` yourself; that instruction is the explicit approval. If `yolo=on`, merge a green/approved PR yourself and post the required FYI. +If the captain says "merge it", run the `pr-readiness` audit again if the ready facts may be stale, then run `gh-axi pr merge` yourself; that instruction is the explicit approval. +If `yolo=on`, merge a green/approved PR yourself only after the same readiness audit passes, then post the required FYI. ### Ship teardown (only after merge is confirmed) @@ -681,7 +691,7 @@ Secondmates inherit this automatically: each secondmate home carries the same `A ## 11. Crewmate briefs Scaffold with `bin/fm-brief.sh ` - it writes `data//brief.md` with the standard contract (branch setup, status-reporting protocol, push/merge rules, definition of done) and all paths filled in. -For a ship task the definition of done is shaped by the project's delivery mode (section 6): `no-mistakes` ends in the harness-appropriate no-mistakes validation pipeline, `direct-PR` has the crewmate push and open the PR itself, `local-only` has it stop at "ready in branch" for firstmate to review and merge locally. +For a ship task the definition of done is shaped by the project's delivery mode (section 6): `no-mistakes` ends in the harness-appropriate no-mistakes validation pipeline, `direct-PR` has the crewmate run `pr-readiness` before pushing and opening the PR itself, `local-only` has it stop at "ready in branch" for firstmate to review and merge locally. The scaffold reads the mode via `fm-project-mode.sh`, so you do not pass it. Ship briefs also include the project-memory contract: run `bin/fm-ensure-agents-md.sh` when the project already has agent-memory files or when the task produced durable project-intrinsic knowledge, then record proportionate learnings in `AGENTS.md`. For scout tasks add `--scout`: the scaffold swaps the definition of done for the report contract (findings to `data//report.md`, no branch, no push, no PR) and declares the worktree scratch; scout is mode-agnostic. diff --git a/README.md b/README.md index 9ab98ff25fd..d7e84dc4dd5 100644 --- a/README.md +++ b/README.md @@ -134,7 +134,7 @@ firstmate works from any terminal - outside tmux, crewmates land in a detached ` After seeding a secondmate, `fm-backlog-handoff.sh` moves already-judged in-scope queued items from the main backlog into that secondmate home so the domain queue starts in the right place. Idle secondmate panes are healthy; teardown is explicit and refuses while the secondmate home has in-flight work unless the captain has approved discard with `--force`. - **Project modes are explicit** - `data/projects.md` records each project's delivery mode and optional `+yolo` autonomy flag. - `no-mistakes` projects run the full validation pipeline, `direct-PR` projects open PRs without that pipeline, and `local-only` projects stay local until firstmate performs an approved fast-forward merge. + `no-mistakes` projects run the full validation pipeline, `direct-PR` projects run the PR-readiness audit before opening PRs without that pipeline, and `local-only` projects stay local until firstmate performs an approved fast-forward merge. - **Project memory belongs to projects** - durable project-intrinsic agent knowledge lives in each project's committed `AGENTS.md`, with `CLAUDE.md` as a symlink. Ship briefs prompt crewmates to create or update those files through the normal delivery path; `data/projects.md` stays a thin private registry. - **Local clones stay fresh** - bootstrap and PR-based teardown refresh remote-backed project clones with clean default-branch fast-forwards when the clone is on the default branch and has no local work, and prune local branches whose remote is gone and that no worktree still needs. @@ -231,6 +231,7 @@ Human-authored pull requests targeting `main` must be raised through `git push n Local `.no-mistakes/` state and test evidence stay out of this repo; `.no-mistakes.yaml` keeps evidence in a temp directory instead. The current watcher reliability work keeps the one-shot process model and adds a durable queue plus singleton lock. The presence-gated sub-supervisor (`bin/fm-supervise-daemon.sh`) provides proactive wake routing for walk-away supervision via the `/afk` skill; a blocking-waiter split remains a deferred follow-up phase. +The `pr-readiness` skill audits live GitHub/base facts, current-base overlap, validation, and public PR text before firstmate opens, updates, comments on, presents, or asks maintainers to merge a PR. ```sh bash -n bin/*.sh # syntax-check the toolbelt