docs(review-checklist): rule 35 — a published package's src change needs a version bump

[lilyshen0722]

What this is

Rule 34 for docs/development/review-checklist.md, the incident-derived reviewer checklist. Docs only — one file, no code, no test changes.

Why it is worth a file

The version guard already documents itself, at length and well — in .github/workflows/package-version-guard.yml, which is exactly the file someone editing commonly-mcp/src/tools.js has no reason to open. The rule therefore exists in the repo and reaches none of the people it applies to. That is the same shape as the defects the guard was written for: knowledge present, reader absent.

It cost a red run today. #1876 fixed commonly-mcp/src/tools.js; the tool suite was green, lint:ts was clean, and CI went red on this guard alone. The fix was one commit (0.3.12 → 0.3.13), which is the right size — but I only knew that because the guard's message happened to be specific.

What the rule says, and what it deliberately does not

Three ways the guard fails a PR, each predictable from the checklist now:

1. No bump — $pkg/src moved and the version still equals base's.
2. A bump below base — a long-lived branch that bumped while main moved further ahead. A bare equality test passes this; merging then walks the published version backwards. The guard's own comment records two open PRs sitting in this state, both showing green.
3. A version an older open PR already claimed for the same package. Two branches both taking cli 0.1.30 → 0.1.31 touch different files, merge with no conflict, and npm ends up with one 0.1.31 built from both PRs' source. Older PR keeps the number; the newer picks the next.

What it does not require is also stated, because a reviewer pattern-matching on "version guard" will otherwise demand bumps for tests: tests, docs, and anything outside $pkg/src need none. A backend-test-only change needs none. The guard says so by name (· cli: no src changes).

Two operator facts included because they change how a red run should be read:

• A pair involving a prerelease makes the guard refuse rather than guess — sort -V orders 1.0.0 before 1.0.0-beta.1, inverting in both directions, so 0.1.0-beta.1 → 0.1.0 would be rejected as a backwards move. Compare such a PR by hand.
• Every gh call in the guard's second job is fatal on failure on purpose: an unchecked failure yields an empty version list, which reads as "this PR bumps nothing" and passes. A guard going blind must not look like a clean run.

Verification

Re-read the guard at main (a2913955) rather than citing it from memory: on.pull_request.types includes edited for base retargeting; the source test is git diff --name-only "$BASE"...HEAD -- "$pkg/src" for pkg in cli commonly-mcp; the "below base" branch is the newest != head_v case; the prerelease case refuses; the second job's proposed() returns non-zero on a failed gh api. Each of those is what the rule text claims.

No test is added: the artifact is prose about an existing guard, and the guard's own behaviour is what CI verifies. The reviewer gate for this PR is "is any sentence here false of the guard as it stands", which is why the two awkward cases (prerelease, and the deliberate fatality) are stated rather than smoothed over.

Rebased: this is rule 35, not 34

#1877 landed its own rule 34 on main while this was open, in the same file. GitHub reported the PR DIRTY, and the measurement agrees: git merge-tree --write-tree HEAD origin/main exits 1 with three conflict stages on docs/development/review-checklist.md. Rebased onto e3a32f32 and renumbered to 35 so the file's numbering stays ascending; the rule's content is unchanged.

Recorded because the shape recurs: a numbered list is a shared counter, and each author sees only their own next number. Here git caught it as a textual conflict, which is the lucky version — the same class as the guard's second job, where two branches claim one version and merge clean.