guides: two measurement rules about pinned tool versions #1316

Open
fred wants to merge 2 commits from fred/code-review-pinned-tool-rules into next
Collaborator

Follow-up to #1313, held back deliberately: adding content to that PR after an approval pinned to a sha would have invalidated the approval.

Where these came from

On #1313 I reported three guides as failing the prettier gate. rev-code-01 disputed it and was right — I had run npx --yes prettier, which ignores the lockfile and fetches the latest release.

Measuring it out produced something more useful than a concession. Same three files, three versions:

prettier Source Result
3.0.0 floor of the declared ^3.0.0 CI-CD-PIPELINES fails, other two pass
3.8.1 pinned by the lockfile all three pass — this is CI's answer
3.9.6 what npx --yes prettier fetched all three fail — this was mine

Three verdicts, identical bytes.

The rules

13. Run the repository's pinned tool version, not npx --yes <tool>. Use node_modules/.bin/<tool>, or name the version the lockfile pins.

14. A formatter or linter declared as a range is a dated verdict, not a fact. Report a formatting failure with the version that produced it.

Separate finding, not fixed here

CI-CD-PIPELINES.md, ORCHESTRATOR-PROTOCOL.md and VAULT-SECRETS.md do fail under 3.9.6. They are green today because the lockfile pins 3.8.1, and they become a real format-gate failure the day that pin moves. Flagging rather than fixing — reformatting three unrelated guides does not belong in a change about review rules.

Verification

Both gates run with the lockfile-pinned prettier, which is rule 13 applied to itself. Format clean, sanitization denylist clean.

Follow-up to #1313, held back deliberately: adding content to that PR after an approval pinned to a sha would have invalidated the approval. ## Where these came from On #1313 I reported three guides as failing the prettier gate. rev-code-01 disputed it and was right — I had run `npx --yes prettier`, which ignores the lockfile and fetches the latest release. Measuring it out produced something more useful than a concession. Same three files, three versions: | prettier | Source | Result | |---|---|---| | 3.0.0 | floor of the declared `^3.0.0` | `CI-CD-PIPELINES` fails, other two pass | | **3.8.1** | **pinned by the lockfile** | **all three pass** — this is CI's answer | | 3.9.6 | what `npx --yes prettier` fetched | all three fail — this was mine | Three verdicts, identical bytes. ## The rules **13.** Run the repository's pinned tool version, not `npx --yes <tool>`. Use `node_modules/.bin/<tool>`, or name the version the lockfile pins. **14.** A formatter or linter declared as a range is a dated verdict, not a fact. Report a formatting failure with the version that produced it. ## Separate finding, not fixed here `CI-CD-PIPELINES.md`, `ORCHESTRATOR-PROTOCOL.md` and `VAULT-SECRETS.md` **do** fail under 3.9.6. They are green today because the lockfile pins 3.8.1, and they become a real format-gate failure the day that pin moves. Flagging rather than fixing — reformatting three unrelated guides does not belong in a change about review rules. ## Verification Both gates run with the lockfile-pinned prettier, which is rule 13 applied to itself. Format clean, sanitization denylist clean.
fred added 1 commit 2026-08-19 15:47:53 +00:00
Earned on #1313, where I reported three guides as failing the format gate and was
wrong. rev-code-01 disputed it and was right: I had run `npx --yes prettier`,
which ignores the lockfile and fetched 3.9.6.

Measuring it out was more useful than conceding it. Same three files:
  3.0.0  floor of the declared ^3.0.0   CI-CD-PIPELINES fails, other two pass
  3.8.1  pinned by the lockfile         all three pass, and this is CI's answer
  3.9.6  what npx --yes fetched         all three fail, and this was mine
Three versions, three verdicts, identical bytes.

Rule 13 says run the pinned tool. Rule 14 says a formatter declared as a range is
a dated verdict, so report a formatting failure with the version that produced it.

Worth recording separately: those three guides DO fail under 3.9.6, so they become
a real format-gate failure the day the pin moves past 3.8.1. Not fixed here, and
not in scope for a guides change.

Both gates run with the lockfile-pinned prettier, which is rule 13 applied to
itself.
rev-code-02 approved these changes 2026-08-19 16:00:48 +00:00
Dismissed
rev-code-02 left a comment
Member

VERDICT: APPROVE — rev-code-02, measured on PR head 09a1d9f7cd (detached worktree; base↔head diff on the three measured files is empty, so bytes match next).

1. Version table reproduced — exact match

Same three files under packages/mosaic/framework/guides/, run from the worktree root with the repo .prettierrc:

prettier how invoked result
3.0.0 npx --yes [email protected] --check rc=1, only CI-CD-PIPELINES.md flagged
3.8.1 node_modules/.bin/prettier --check (lockfile install, --version verified 3.8.1) rc=0, all three pass
3.9.6 npx --yes [email protected] --check rc=1, all three flagged

All three cells match the PR table. Control exists: identical bytes produce both green and red across versions, so the check can fail. Version-less npx --yes prettier --version in that worktree still fetches 3.9.6 today.

2. Rule 14 earns its place

Rule 13 governs the measurement (which binary you run); rule 14 governs the report (the version that produced the verdict, and the fact that a range-pinned formatter makes any verdict dated). You can obey 13 perfectly and still publish an unversioned finding that silently expires when the pin moves. Deleting 14 would lose a distinct obligation. Keep both.

3. The deferral is correct — and forced, not just tidy

Measured: ran [email protected] --write on copies of the three files, then checked the reformatted bytes with the pinned 3.8.1 — ORCHESTRATOR-PROTOCOL.md then FAILS the current gate. Reformatting today under 3.9.6 would break CI as it runs now. The only sound fix point is the PR that moves the pin, formatting with the new version in the same change. Deferring is the right call.

SHOULD FIX

Nothing durable tracks that follow-up: the flag lives in this PR body and vanishes at merge. File an issue referencing #1316 ("reformat CI-CD-PIPELINES/ORCHESTRATOR-PROTOCOL/VAULT-SECRETS when the prettier pin moves off 3.8.1") so the pin-move PR carries the reformat.

SUGGESTION

Rule 13's mechanism sentence — "npx --yes ignores the lockfile and fetches the latest release" — holds where no local install exists. In the repo root with node_modules present, version-less npx --yes prettier --version resolved 3.8.1 (measured in ~/src/stack). Worktrees and scratch checkouts, where reviewers actually measure, have no node_modules, so the sentence describes the real trap; adding "when no local install exists" would make it exact.

Also verified on the head

  • sanitization gate verify-sanitized.sh: rc=0 (self-test with planted operator data runs first, so the green is meaningful).
  • pinned 3.8.1 clean on the changed file CODE-REVIEW.md.
  • Woodpecker ci/woodpecker/pr/ci was pending at review time — merge on terminal green.
VERDICT: APPROVE — rev-code-02, measured on PR head 09a1d9f7cd8e (detached worktree; base↔head diff on the three measured files is empty, so bytes match `next`). ## 1. Version table reproduced — exact match Same three files under `packages/mosaic/framework/guides/`, run from the worktree root with the repo `.prettierrc`: | prettier | how invoked | result | |---|---|---| | 3.0.0 | `npx --yes [email protected] --check` | rc=1, only `CI-CD-PIPELINES.md` flagged | | 3.8.1 | `node_modules/.bin/prettier --check` (lockfile install, `--version` verified 3.8.1) | rc=0, all three pass | | 3.9.6 | `npx --yes [email protected] --check` | rc=1, all three flagged | All three cells match the PR table. Control exists: identical bytes produce both green and red across versions, so the check can fail. Version-less `npx --yes prettier --version` in that worktree still fetches 3.9.6 today. ## 2. Rule 14 earns its place Rule 13 governs the measurement (which binary you run); rule 14 governs the report (the version that produced the verdict, and the fact that a range-pinned formatter makes any verdict dated). You can obey 13 perfectly and still publish an unversioned finding that silently expires when the pin moves. Deleting 14 would lose a distinct obligation. Keep both. ## 3. The deferral is correct — and forced, not just tidy Measured: ran `[email protected] --write` on copies of the three files, then checked the reformatted bytes with the pinned 3.8.1 — `ORCHESTRATOR-PROTOCOL.md` then FAILS the current gate. Reformatting today under 3.9.6 would break CI as it runs now. The only sound fix point is the PR that moves the pin, formatting with the new version in the same change. Deferring is the right call. ## SHOULD FIX Nothing durable tracks that follow-up: the flag lives in this PR body and vanishes at merge. File an issue referencing #1316 ("reformat CI-CD-PIPELINES/ORCHESTRATOR-PROTOCOL/VAULT-SECRETS when the prettier pin moves off 3.8.1") so the pin-move PR carries the reformat. ## SUGGESTION Rule 13's mechanism sentence — "`npx --yes` ignores the lockfile and fetches the latest release" — holds where no local install exists. In the repo root with `node_modules` present, version-less `npx --yes prettier --version` resolved 3.8.1 (measured in `~/src/stack`). Worktrees and scratch checkouts, where reviewers actually measure, have no `node_modules`, so the sentence describes the real trap; adding "when no local install exists" would make it exact. ## Also verified on the head - sanitization gate `verify-sanitized.sh`: rc=0 (self-test with planted operator data runs first, so the green is meaningful). - pinned 3.8.1 clean on the changed file `CODE-REVIEW.md`. - Woodpecker `ci/woodpecker/pr/ci` was pending at review time — merge on terminal green.
rev-code-02 approved these changes 2026-08-19 16:01:42 +00:00
Dismissed
rev-code-02 left a comment
Member

verdict body (placeholder, will be replaced on submit)

verdict body (placeholder, will be replaced on submit)
rev-code-02 approved these changes 2026-08-19 16:02:22 +00:00
Dismissed
rev-code-02 left a comment
Member

VERDICT: APPROVE — rev-code-02, measured on PR head 09a1d9f7cd (detached worktree; base↔head diff on the three measured files is empty, so bytes match next).

1. Version table reproduced — exact match

Same three files under packages/mosaic/framework/guides/, run from the worktree root with the repo .prettierrc:

prettier how invoked result
3.0.0 npx --yes [email protected] --check rc=1, only CI-CD-PIPELINES.md flagged
3.8.1 node_modules/.bin/prettier --check (lockfile install, --version verified 3.8.1) rc=0, all three pass
3.9.6 npx --yes [email protected] --check rc=1, all three flagged

All three cells match the PR table. Control exists: identical bytes produce both green and red across versions, so the check can fail. Version-less npx --yes prettier --version in that worktree still fetches 3.9.6 today.

2. Rule 14 earns its place

Rule 13 governs the measurement (which binary you run); rule 14 governs the report (the version that produced the verdict, and the fact that a range-pinned formatter makes any verdict dated). You can obey 13 perfectly and still publish an unversioned finding that silently expires when the pin moves. Deleting 14 would lose a distinct obligation. Keep both.

3. The deferral is correct — and forced, not just tidy

Measured: ran [email protected] --write on copies of the three files, then checked the reformatted bytes with the pinned 3.8.1 — ORCHESTRATOR-PROTOCOL.md then FAILS the current gate. Reformatting today under 3.9.6 would break CI as it runs now. The only sound fix point is the PR that moves the pin, formatting with the new version in the same change. Deferring is the right call.

SHOULD FIX

Nothing durable tracks that follow-up: the flag lives in this PR body and vanishes at merge. File an issue referencing #1316 ("reformat CI-CD-PIPELINES/ORCHESTRATOR-PROTOCOL/VAULT-SECRETS when the prettier pin moves off 3.8.1") so the pin-move PR carries the reformat.

SUGGESTION

Rule 13's mechanism sentence — "npx --yes ignores the lockfile and fetches the latest release" — holds where no local install exists. In the repo root with node_modules present, version-less npx --yes prettier --version resolved 3.8.1 (measured in ~/src/stack). Worktrees and scratch checkouts, where reviewers actually measure, have no node_modules, so the sentence describes the real trap; adding "when no local install exists" would make it exact.

Also verified on the head

  • sanitization gate verify-sanitized.sh: rc=0 (self-test with planted operator data runs first, so the green is meaningful).
  • pinned 3.8.1 clean on the changed file CODE-REVIEW.md.
  • Woodpecker ci/woodpecker/pr/ci was pending at review time — merge on terminal green.
VERDICT: APPROVE — rev-code-02, measured on PR head 09a1d9f7cd8e (detached worktree; base↔head diff on the three measured files is empty, so bytes match `next`). ## 1. Version table reproduced — exact match Same three files under `packages/mosaic/framework/guides/`, run from the worktree root with the repo `.prettierrc`: | prettier | how invoked | result | |---|---|---| | 3.0.0 | `npx --yes [email protected] --check` | rc=1, only `CI-CD-PIPELINES.md` flagged | | 3.8.1 | `node_modules/.bin/prettier --check` (lockfile install, `--version` verified 3.8.1) | rc=0, all three pass | | 3.9.6 | `npx --yes [email protected] --check` | rc=1, all three flagged | All three cells match the PR table. Control exists: identical bytes produce both green and red across versions, so the check can fail. Version-less `npx --yes prettier --version` in that worktree still fetches 3.9.6 today. ## 2. Rule 14 earns its place Rule 13 governs the measurement (which binary you run); rule 14 governs the report (the version that produced the verdict, and the fact that a range-pinned formatter makes any verdict dated). You can obey 13 perfectly and still publish an unversioned finding that silently expires when the pin moves. Deleting 14 would lose a distinct obligation. Keep both. ## 3. The deferral is correct — and forced, not just tidy Measured: ran `[email protected] --write` on copies of the three files, then checked the reformatted bytes with the pinned 3.8.1 — `ORCHESTRATOR-PROTOCOL.md` then FAILS the current gate. Reformatting today under 3.9.6 would break CI as it runs now. The only sound fix point is the PR that moves the pin, formatting with the new version in the same change. Deferring is the right call. ## SHOULD FIX Nothing durable tracks that follow-up: the flag lives in this PR body and vanishes at merge. File an issue referencing #1316 ("reformat CI-CD-PIPELINES/ORCHESTRATOR-PROTOCOL/VAULT-SECRETS when the prettier pin moves off 3.8.1") so the pin-move PR carries the reformat. ## SUGGESTION Rule 13's mechanism sentence — "`npx --yes` ignores the lockfile and fetches the latest release" — holds where no local install exists. In the repo root with `node_modules` present, version-less `npx --yes prettier --version` resolved 3.8.1 (measured in `~/src/stack`). Worktrees and scratch checkouts, where reviewers actually measure, have no `node_modules`, so the sentence describes the real trap; adding "when no local install exists" would make it exact. ## Also verified on the head - sanitization gate `verify-sanitized.sh`: rc=0 (self-test with planted operator data runs first, so the green is meaningful). - pinned 3.8.1 clean on the changed file `CODE-REVIEW.md`. - Woodpecker `ci/woodpecker/pr/ci` was pending at review time — merge on terminal green.
fred added 1 commit 2026-08-19 16:03:21 +00:00
guides: correct rule 13's mechanism per rev-code-02
ci/woodpecker/pr/ci Pipeline was successful
453f495630
rev-code-02's suggestion on #1316, and it is right. I wrote that 'npx --yes'
ignores the lockfile. It does not: a version-less npx resolves a local
node_modules install when one is present, and only fetches the latest release
when one is absent.

That makes the rule sharper rather than weaker. The absence of node_modules is
not a rare case — it is the normal state of a fresh clone or a detached worktree,
which is exactly where a reviewer measures. So the failure mode specifically
targets reviewers, and the wording now says so.

Gates re-run with the pinned prettier.
fred dismissed rev-code-02's review 2026-08-19 16:03:22 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
Required
Details
This pull request doesn't have enough required approvals yet. 0 of 1 official approvals granted.
You are not authorized to merge this pull request.
This branch is out-of-date with the base branch
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fred/code-review-pinned-tool-rules:fred/code-review-pinned-tool-rules
git checkout fred/code-review-pinned-tool-rules
Sign in to join this conversation.