review-pr git:20261019.0d602b7A

Immutable. This exact content never changes and is served at /api/v1/blob/eee1016d479a4c41.

---
name: review-pr
description: Takes a pull request of this repository from start to finish — read the issue and the diff, decide what is ours and what goes back to the author, cover what is missing in tests, measure, run the checks the diff calls for and integrate. Use when asked to "work on PR N", when reviewing a third-party contribution, or when resuming a stalled branch.
---

# Working on a pull request

This skill is the **backbone**: it says *when* each thing enters and *why*. The
*how* is in the skills it points to, and is not repeated here — a step that fits
in one sentence plus a pointer is one sentence plus a pointer.

**Not every PR ends in a release, and merge is not release.** The four legitimate
exits are in step 4; the straight line to integration is one of them, not the
only one. Steps 6 and 7 belong to that line alone and step 8 holds for all four;
step 5 serves both sides — on the straight line it covers the gaps, and when
handing the PR back it turns the reproductions into tests of the project. A
plausible verdict also crosses the line: it sends the PR to `golden-run`,
`python-matrix` or `rebuild-vis` before any exit is closed.

## Where each signal leads

| What appears in the diff or on the branch | What enters |
|---|---|
| `pyproject.toml`, `uv.lock`, `src/frontend/package*.json`, `.github/workflows/**` | `dependencies` |
| A Dependabot PR | `dependencies`: never merged as it stands; a maintainer turns its branch into the tested commit (below) |
| `src/frontend/**` | `rebuild-vis` (a browser for a view that changed) |
| `src/noworkflow/now/vis/**` | `rebuild-vis` ("Smoke"), then `golden-run`, step 4 |
| `src/noworkflow/now/collection/**`, or an issue that names a Python version | `python-matrix`; also `golden-run` when what is recorded changes |
| `src/noworkflow/now/persistence/**`, `src/noworkflow/now/models/**`, `src/noworkflow/now/cmd/**`, `src/noworkflow/patterns/**`, `src/noworkflow/resources/**` | `golden-run` |
| A new or renamed command or option | `README.md` and, once it exists, `skills/noworkflow/SKILL.md`; for `run`, `restore` and `schema`, also a manual probe of the magic that inherits the parser (below) |
| `src/noworkflow/now/ipython/**`, `src/noworkflow/kernel/**` | A blind spot of the suite: a manual probe, recorded in the PR |
| Branch behind `main` | `merge-downstream` |
| A suspected defect | `prove-candidate` |
| An alert of the `CodeQL` check (how to read it, and the route for a fork: step 6) | `prove-candidate` with the other candidates of step 3, then step 4: an alert the PR introduces is discussed in it |
| A red `SAST (bandit)` step (how to read it: step 6) | `prove-candidate` with the other candidates of step 3: an intended site gets `# nosec Bxxx` with the reason on the line above, in the PR; a real one gets an issue and its entry in `bandit-baseline.json`, never a `nosec` |
| A PR that depends on an unpublished `pyposast` | `dependencies`: blocked until that release |
| Any change that must be measured | `safe-refactor` |
| Green, measured and integrated | `release`, when the maintainer decides |

Two rows need a word more. **A Dependabot PR is never merged as it
stands**: `main` is the baseline, and only what a maintainer tested reaches
it (`AGENTS.md`, "Dependencies"). The PR is its own record and needs no
issue: a maintainer turns its branch into the commit that will land —
rebased on `main`, the bundle rebuilt in it for npm, the lock redone for uv —
pushes it to the PR's branch so that CI tests that commit, and integrates it
as step 7 says (`dependencies` has the steps per ecosystem). What decides whether the
bump is accepted is the series rule of `AGENTS.md` ("Dependencies"): a patch
of the same series goes in, a minor or major bump is a decision for the
maintainer, never a side effect. **A new option of `now run`, `now
restore` or `now schema` is also a new option of `%now_run`, `%now_restore` or
`%now_schema`**: the magics in `src/noworkflow/now/ipython/magics/` inherit
the command's parser, so the diff never touches `now/ipython/**` and the
blind-spot row does not fire; probe the magic by hand in an IPython shell and
record it in the PR.

## 1. Reconnaissance

`gh pr checkout` switches branches: with a modification of yours in a file the
PR touches, git refuses the whole switch, so start with a clean tree. If there
is uncommitted work there, **stop and ask the maintainer** before switching: by
`AGENTS.md` it is the maintainer who commits, and an uncommitted change has no
reflog to come back from. On an older branch, what only `main`'s `.gitignore`
ignores (the files the audit and the build generate) shows up as untracked:
that is nobody's work.

`--force` is not optional: without it, on a PR that was force-pushed, the
fast-forward that gh runs inside fails and the local branch stays at the old
head, and you spend the whole cycle measuring a stale branch. `--force`
resets the local branch to the PR's head — check first that no commit of
yours is there.

```bash
gh pr view <N> --json title,body,state,headRefName,baseRefName,mergeable,commits,statusCheckRollup,isCrossRepository,maintainerCanModify,closingIssuesReferences
git fetch --all --prune
gh pr checkout <N> --force                  # everything from here on reads the tree
git log --oneline origin/main..HEAD         # what the PR brings
git log --oneline HEAD..origin/main         # what is missing from main
```

The second list empty is what you want. If it is not, `merge-downstream` before
any measurement: a stale base measures the lag, not the PR.

`closingIssuesReferences` lists the issues the merge would close. Anything
there goes against the closing rule of `AGENTS.md`: ask the author to drop
the `Fixes`/`Closes`. A body that names the issue without a closing keyword
does not appear there, and needs nothing.

**"Everything from here on reads the tree" includes this skill and the tools.**
A branch that predates `uv.lock`, the root `conftest.py`, the pytest
configuration or `.claude/skills/` no longer has them on disk after the
checkout: the skills this one points to are read from `main` (`git show
origin/main:.claude/skills/<name>/SKILL.md`), and **nothing goes through `uv`
on that tree — `/code-review` included — until `merge-downstream` has brought
the lock in.** On a tree without `uv.lock`, every `uv run` re-resolves, writes
an untracked `uv.lock`, syncs `.venv` to a set without pytest, and the
untracked lock then aborts the catch-up merge. If it already happened: `rm
uv.lock` before the merge, `uv sync --locked` after it. Meanwhile the probes
and the reproductions run on a worktree of `main` (step 3).

A PR from a fork brings the branch in the author's remote: `maintainerCanModify`
above says whether the maintainer can push there the reproduction that step 3
prepares, and whether a catch-up merge can be pushed at all. Without it, the
reproduction only reaches the author as a comment, `merge-downstream` serves
only to measure, and the sync of step 3 of `CONTRIBUTING.md` goes back to the
author with the rest of the round (step 4).

**Read the issue and its comments, not only the title**, and also those of the
related and closed issues — found the way `AGENTS.md` prescribes before
opening an issue (issuefinder, then `git log --all --grep="#NNN"`). That is
where the product decision already taken, the reverted
attempt and what was already verified live; without it, it is easy to treat as
new a part that was already settled, or to reopen a closed discussion.

Fire `/code-review max <N>` and follow step 2 while it runs — the PR number
also gives it the author's comments.

- **`max`, not less:** below `xhigh` it may fall into a single-pass variant,
  without the final sweep for gaps, and `max` is the ceiling.
- **Never `--comment` nor `--fix`.** The first publishes on the PR; the second
  applies to the tree outside the checkpoints, and `/rewind` does not undo it.
- It does not verify what it returns, and the end of the list is usually a
  floor of findings, not discovery. Who triages is step 3.
- **It measures: it runs the suite.** Overlapping with the measurement of step
  6 contaminates the round — a failure shows up as a regression of the PR and
  is two runs sharing the machine. Wait for it to finish before measuring, or
  measure before firing it.
- **It cleans the tree when it closes, and does not distinguish whose file it
  is.** An untracked file that appeared during the round disappears — the
  reproduction you just wrote included, without warning: you only find out
  when you run the test. Write it in the session scratchpad and copy it into
  `src/noworkflow/tests/` when it is time to run.
- **"Finished" may not be the end.** The completion notice arrives even with a
  background sweep of its still alive: it comes back, sends an addendum — and
  cleans the tree again. Check that it is `completed` before treating the list
  as closed.

The alerts of the `CodeQL` check join its findings as candidates of step 3;
step 6 says how to read them and when the list is complete. A PR from a fork
has no `CodeQL` check: start its analysis now (step 6), so that its alerts
reach the triage and the single round of step 4.

## 2. Read the diff

**Say first what kind of PR this is** and declare the expected differences
before running any measurement. The criteria are in step 8 of `safe-refactor`
— bug fix, feature and refactoring — and **an upgrade is read as a
refactoring**: no difference is expected from it either.

**Read the diff yourself, and note your own candidates** — do not delegate
this to `/code-review`. It crosses files and confronts the diff with the
written conventions, but it does not leave the repository: four classes of
candidate **only come out of your reading**, because they depend on what the
repository does not keep:

- **The effect on existing provenance stores.** `AGENTS.md` says why it cannot
  be claimed without opening a store created by the previous version; the
  store-compatibility check of `golden-run` is the instrument. The same
  question for the data already recorded: a field whose meaning changes is
  read by `now show`, by every export and by the vis, over trials that were
  collected before the change.
- **The coverage the issue asked for**, not the one that exists: the machine
  only complains about a missing test as a nit, and does not know what the
  issue promised.
- **A product decision**, including about the request you yourself made to the
  author. It is not a finding, it is judgement, and retracting in public is
  part of the job.
- **What the merge will run into** — which test on `main` stops passing
  against the new model, which export or Prolog rule stops matching a model
  that the branch changed, which handler the transformer refactoring on `main`
  no longer calls. The two sides pass green separately, and only the full
  suite after the merge tells; the detail is in `merge-downstream`. It also
  holds with the branch up to date, so do not wait for the routing of step 1
  to look. `git merge-tree --write-tree origin/main HEAD` shows the conflicts,
  and `git show <tree>:<file>` the merged file, without touching the tree.

**A change to a vis route, a content engine, a subprocess call or a workflow
is read for security as if CodeQL were not there**: those are where it sees
least, and step 6 lists what it does not see.

When the PR touches `pyproject.toml` or `uv.lock`, what decides the review is
what changed **in the dependency**, not what the author wrote: the notes of
step 1 of `safe-refactor` apply. What is specific to the review is checking
**the author's adjustment against the library's migration guide** — a
migration instruction comes in prose, and it is the item that goes unnoticed
and only breaks on the user's machine.

The checklist of the PR template is what the author claims; step 6 is where
each box is verified.

## 3. Triage the candidates

Yours and those of `/code-review`, all through `prove-candidate`: the
escalation, the three verdicts and the rules of proof are there.

What is specific to a PR review is **where the "before" is: at the
merge-base, not on `main`** — a stale branch would measure the PR plus what
`main` moved. A worktree outside the checkout — `<outside>` is the session
scratchpad, or the checkout's parent:

```bash
base=$(git merge-base origin/main HEAD)
git worktree add <outside>/nw-base "$base"
cd <outside>/nw-base && uv sync && uv run pytest   # at an old base, its run_tests.py (below)
```

**The merge-base may predate the pytest configuration and the lock**: a
`pyproject.toml` without `[tool.pytest.ini_options]` and no `uv.lock`. There,
`uv sync` resolves fresh and installs no pytest (`uv run pytest` fails to
spawn), and `uv run --with pytest pytest` would collect without the doctests
and without the scratch directory, so its count is not comparable. The runner
at such a commit is its own `run_tests.py` (`uv run python run_tests.py -v`
names each test it runs). It ran only the suites assembled by hand in
`tests/__init__.py`, so compare its tests one by one with the same tests
here, never its `Ran N tests` with pytest's count.

It is also how you find out that **a candidate already held before the PR**:
it is not a regression, and the message to the author changes from "you broke
it" to "this got worse here — fix it now or does it become an issue?".

The merge-base worktree is for when the **environment** differs — a PR that touches
`pyproject.toml`, a different lock. When only the code changed, the cheaper
**and more precise** control is `git checkout "$base" -- <file>` and run
again: it isolates the hunk, not the whole commit, and skips `uv sync`. The
tree is clean after the checkout of step 1, so `git checkout HEAD -- <file>`
puts it back without losing anything.

**When the branch predates the lock, the probes run on a worktree of
`main`.** Until `merge-downstream`, nothing on the PR's tree goes through
`uv` (step 1), and the merge-base has no pytest. A worktree of `origin/main`
synced from the lock takes the PR's files one by one:

```bash
git worktree add --detach <outside>/nw-main origin/main
cd <outside>/nw-main && uv sync --locked
git checkout <branch> -- <file>                    # a file main did not change
git diff "$base" <branch> -- <file> | git apply    # only the PR's hunk, for one it did
git diff --stat "$base" origin/main -- <files>     # what main changed since the base
```

It stands for the "before" only for the files that the last command shows
unchanged; say so in the report. Bringing the whole file of one that `main`
changed undoes `main` in the measurement. `--detach` matters: a worktree on
the `main` branch stops the checkout from switching back to `main`.

Create either worktree **outside** the repository and remove it with `git
worktree remove --force` when step 6 closes (`--force` because `uv sync`
leaves `.venv/`, and at an old base also `uv.lock`, untracked there; `git
worktree prune` if the directory is already gone): inside the checkout it
breaks the clean-tree precondition of `release`, and forgotten it accumulates
one per PR — `release` checks `git status`, never `git worktree list`.

## 4. Whose fix is it?

It is the central decision of the skill, and it has four exits.

**We take it over, and carry on** when the PR is mostly adequate: the
author's fixes attack the cause, what is missing is **additive** (a test
nobody wrote, coverage of a new path) and there is no product decision in
dispute. Commenting on the PR in that case only postpones by days work that is
already done in the tree.

**Comment on the PR and hand it back** when there is rework: the fix treats
the symptom instead of the cause, the number of adjustments would rewrite the
author's commits, or context is missing that only the author has.

**Stop and ask the maintainer** when the doubt is about the product, not the
code.

**Do not go ahead** when the PR contradicts a decision already taken,
duplicates a closed issue, or is stale to the point that the merge costs more
than redoing it. It comes out of the reading of step 1, and it is the most
valuable conclusion a review produces — the one that avoids the whole rest of
the cycle. Closing a third party's PR is the maintainer's decision: bring the
reason and what supports it.

**When handing back, convert the PR to a draft** (`gh pr ready --undo <N>`).
A PR marked ready for review is how the author signals that the checklist is
complete, and it is by that mark that the maintainer finds the queue; a PR
handed back that stays ready comes back to the queue as if nothing had
happened. Who marks it ready again is the author, when the round is done.

**When handing back, send the reproductions along — not only the prose.**
Those of step 3 already assert the correct behavior and fail while the defect
exists, and on the author's branch they would leave the suite red until the
fix. That is more useful than a description, and it is not an accusation: the
reproduction **is the specification of the fix**, and it is worth saying so in
the comment. Commit and push belong to the maintainer — leave the reproduction
ready in the tree and ask. On a branch of this repository the maintainer
pushes it; on a fork, only if `maintainerCanModify` allows.

Two conditions. The reproduction goes clean, with the name and the place of a
test of the project, with no remains of the exploratory script that produced
it. And **feedback in one round**: gather everything — confirmed, refuted,
what is not a regression and what stayed open — instead of commenting finding
by finding, which makes the author chase a moving target.

**A vulnerability the PR itself introduces is discussed in it**, because the
defect is born at the merge. One found in passing, already on `main`, becomes
its own issue like any other bug: noWorkflow runs on the user's own machine,
and `AGENTS.md` has the cycle.

Two things that do **not** justify handing back, and become a written note: a
deviation from the commit-message convention, and an adjacent finding outside
the PR's scope — that one becomes its own issue (search the existing ones
first, as `AGENTS.md` prescribes).

## 5. Cover the gaps

The new coverage comes after step 4, because only there you know where it
goes. Handing the PR back does **not** throw it away: on a branch of this
repository it travels with the reproductions, in the same commit, and the
author receives both. On a fork without `maintainerCanModify` only the prose
arrives — then writing it before the decision is wasted work. The
reproductions of step 3 are another thing — those already exist, and they are
what travels; when handing back, this is where they take the form of a test
of the project, by the rules below.

**The commit goes red, and the message says so.** A reproduction that travels
leaves the branch's CI red until the fix — it is the design, not a new break.
Without that sentence in the body of the commit, the next round reads the red
as a regression and goes fixing the test.

**Measure what the PR's own coverage reaches before writing yours.** Revert
each part of the change and run the tests the PR adds: the part whose
reversal brings down no test is not covered — and it is where the regression
passes without the suite noticing. In a four-part PR, three may come without a
single test, with the suite closing green.

What the change touches and the suite does not execute becomes a test, by
`safe-refactor` — including the check that the test is not empty. Where the
test lives is in step 3 of `safe-refactor`. Tests added raise `min-tests` in
`.github/workflows/ci.yml` and in `experimental.yml` in the same PR. What not
even the suite reaches goes to `golden-run`, `python-matrix` or `rebuild-vis`
by the table, and the blind spots that none of them covers (`AGENTS.md`) are
probed by hand and recorded in the PR.

A review inverts the order of `safe-refactor`: the change **already exists**,
so the test cannot be written against the old code. The way out is to write it
against the new behavior and prove the red **by simulating the previous
version** — with its code, never with a hand-written approximation. The
cheapest control is the file-level one of step 3: `git checkout "$base" --
<file>`, run the new test and see it red, `git checkout HEAD -- <file>`. The
worktree of step 3 when the environment differs; `uv run --with
"<pkg>==<old>" pytest` for a dependency.

**The reproduction must also go green when the defect leaves.** Today's red
proves it reaches the defect; only the control proves it is satisfiable —
disable the culprit line and check that it passes. Without that, an assertion
travels that no fix satisfies, and the author loses the round trying: an
output compared character by character, a UUID typed in, an order that the
store does not guarantee, an environment attribute that only exists on your
machine.

**A reproduction from a previous round is suspect again.** Run the old ones
before reading the new diff, and for each one that passed ask *why* it
passed: was the defect fixed, or did the fix open a path that returns before
reaching the point being measured? A precondition guard, an early `return`
on an empty store and a condition on the Python version leave the test green
without touching what it guarded. The test is the usual one: disable the line
of the fix and check that the test falls. One that passed for the wrong
reason is rewritten — and with it the comment that described the defect in
the present tense, otherwise the next round reads the description as if the
defect were still there.

## 6. Measure

The full suite by `safe-refactor`, compared with the baseline.

Check also the CI at the tip of the branch: it runs what the local machine
does not — every supported Python on Linux, the frontend guard, lint, SAST,
the audits — and it is the measurement that reaches the blind spots
`AGENTS.md` lists. A push does not run macOS: for a change that touches
paths, encodings or the file system, run `pre-release.yml` on the branch
(`gh workflow run pre-release.yml --ref <branch>`) and read it too.

**The `SAST (bandit)` step fails three ways**, each with its own message:
`-b` prints a finding whose kind the baseline lacks in that file; the `diff`
prints `> <file> <test>` for a new finding of a kind the baseline already
has there (bandit's `-b` alone would hide it), and `< <file> <test>` for an
entry whose finding is gone, which the commit of the fix removes by
regenerating the file (the command is next to the step in `ci.yml`); the
`grep` refuses a `# nosec` without a test id. A `nosec` on a line with nested
calls makes bandit warn "nosec encountered ... but no failed test" on every
run, without failing: that warning is the only sign of a stale `nosec`, so
keep it meaningful (put the `nosec` where no nested call shares its line, or
say it in code, as `hashlib.sha1(..., usedforsecurity=False)` does).

**CodeQL is not a job of `ci.yml`.** Its default setup, in the repository
settings, analyzes every PR to `main` from a branch of this repository and
posts the `CodeQL` check. The check annotates each alert the PR adds in the
lines it changes, and fails only on one of severity error or of security
severity high or critical; a lower one leaves it green and is still a
candidate. `gh api "repos/gems-uff/noworkflow/code-scanning/alerts?pr=<N>&state=open"`
lists them. **The check reports as each language finishes**: it can show
completed, with only the workflows analyzed, while Python and TypeScript still
run. A summary that warns of configurations "not found" is partial; read it
once the analysis run has finished (its `success` says the analysis ran, not
that it found nothing — the verdict stays the check):

```bash
gh api "repos/gems-uff/noworkflow/actions/runs?head_sha=$(gh pr view <N> --json headRefOid -q .headRefOid)" \
  --jq '.workflow_runs[] | select(.path == "dynamic/github-code-scanning/codeql") | "\(.status) \(.conclusion)"'
```

Each alert is a candidate like any other: `prove-candidate`, then fixed in the
PR, or explained in it for a maintainer to dismiss with that reason. The
ruleset on `main` requires code-scanning results and blocks on an alert at
those same severities, so the merge button refuses a PR with such an alert,
or whose analysis is missing or still running. Maintainers integrate by a
fast-forward push, which the ruleset lets repository admins bypass, so for
them the check is read, not enforced.

**No analysis is not "no alert".** A PR whose last push predates the setup
has no `CodeQL` check until its branch gets a new push (the catch-up merge of
`merge-downstream` is one). **A PR from a fork never gets one**: default setup
skips forks. Its code is analyzed through a branch of this repository, as
soon as the checkout of step 1 (and the catch-up merge, if any, so that what
is analyzed is what would land) allows. The push, the branch and the PR are
the maintainer's, like every push (`AGENTS.md`): prepare the tree and ask, as
step 4 does with a reproduction. The commands:

```bash
git diff --name-only origin/main...HEAD -- .github/   # read these first (below)
git push origin HEAD:refs/heads/codeql-pr-<N>
gh pr create --base main --head codeql-pr-<N> \
  --title "Do not merge: CodeQL analysis of #<N>" --body "CodeQL analysis of #<N>, closed once the analysis finishes."
# <M>: the number at the end of the URL that gh pr create printed. Wait until the
# runs query above, with head_sha=$(git rev-parse HEAD), prints "completed".
gh api "repos/gems-uff/noworkflow/code-scanning/alerts?pr=<M>&state=open"
gh pr close <M> --delete-branch
```

**Close the mirror PR as soon as its alerts are read, before step 7.** Once
`main` contains its head, GitHub marks it merged and deletes its branch by
itself, and `gh pr close <M> --delete-branch` fails.

The push runs this repository's workflows **as the fork wrote them**, with
this repository's token and permissions, which the fork's own PR never has: a
change under `.github/` is read, and judged harmless, before the push, never
after.

**The fork's own CI run may wait in `action_required`**: GitHub asks for
approval before running workflows for a first-time contributor, also when the
push to the fork's branch was a maintainer's. The runs of `codeql-pr-<N>`
(its push and its PR) test the same SHA: read the CI at the tip there, and
approving the fork's run is optional.

**A green `CodeQL` is not a security review.** CodeQL sees untrusted input
only where its models recognize it, and the vis's own code mostly escapes
them: the route's URL parameters are not a source, because CodeQL does not
follow `app = WebServer().app` back to Flask (only `flask.request` is,
whatever part of the request is read from it); nor is data read back from
the store; and calls through `__getattr__` (`content.get`) or `args.func`,
SQL run through `relational.engine.raw_connection()`, a missing Host/Origin
check and a state-changing GET are outside what it follows. In a workflow, an
event field interpolated into a `run` step (`${{ github.event.* }}`) is
reported only when the job can write: a job with `contents: read` gets no
alert for it. That is why step 2 reads a change to a vis route, a content
engine, a subprocess call or a workflow as if CodeQL were not there.

**`gh pr checks <N>` exits non-zero while a check is pending, when one
failed, and when the branch has no checks at all** (a PR older than the
workflow), so treating a non-zero exit as a transient error to swallow makes
the waiting loop spin silently, and the silence is indistinguishable from
"still running". Pending *is* the state, not a failure to query it: decide by
the text of the output, never by the exit status. The status the ruleset
requires is `ci-ok`; the `test-experimental (...)` legs come from
`experimental.yml` and are not required — a red there is compared with the
same leg on `main` (`gh run list --workflow experimental.yml --branch main
--limit 1`) before being charged to the PR. While that workflow is suspended
(its header says until when) the PR has no such legs: when the diff touches
paths or file encodings, run it by hand on the PR's branch (`gh workflow run
experimental.yml --ref <branch>`). A PR from a fork has no `coverage`
job, by the guard in `ci.yml`; that is not a failure either.

Then what the table routes: `dependencies` for the lock and the workflows,
`python-matrix` when the collector changed (and `golden-run` when that
changes what is recorded), `rebuild-vis` for the frontend, `rebuild-vis` and
`golden-run` for the vis server, `golden-run` for persistence, models,
commands, patterns and resources. A green suite says nothing about any of
them.

## 7. Integrate

Before this, all of it has to be true: the full suite green on the branch, the
checks the table routed done, `main` synchronized, working tree clean, CI
green at the tip, the `CodeQL` analysis finished with each alert fixed or
dismissed (step 6; for a fork, through its branch of this repository, whose
mirror PR is already closed), and the
second list of step 1 empty — otherwise
`merge-downstream` and measure again. The worktrees of step 3 are removed.
Your part of this step ends with that list, checked; the integration itself
is the maintainer's, and these are the commands they run:

```bash
git checkout main && git pull --ff-only
git merge --ff-only <branch>                # fails if the branch is behind
git push origin main
git push origin --delete <branch>           # a branch of this repository only
git branch -d <branch>
```

After the fast-forward the branch points to a commit `main` already
contains, so deleting it loses nothing. A fork's branch lives in the
author's remote and is theirs to delete; only the local copy goes.

Then the maintainer closes the issue (`gh issue close <issue> --reason
completed`), unless the PR delivered only part of it: integrated into `main`
and finished is done, and the release finds it later through the
`Issue #NNN:` prefix of its commits. The integration leaves a pending item
for the release when persistence, exports or commands changed: the
store-compatibility check of `golden-run` is the maintainer's input for the
version number.

## 8. Close the trail

Each thing learned has a place, and the criterion is the one of `AGENTS.md`,
in the section "A skill is a procedure, not a logbook": the mechanism goes to
the skill, the measurements and the path of the round go to a comment on the
issue.

One obligation before considering the PR closed: **a conclusion of yours that
fell to new evidence is corrected where it was written.** If it became a commit
message not yet pushed, ask the maintainer to rewrite the commit — the test
may survive with another framing, the wrong justification cannot become
history.