merge-and-release's Procedure section holds 33 verified defects across four whole-unit passes #1477

A carried-content review pass over the canonical unit .agents/skills/merge-and-release/SKILL.md > The Procedure, run while retiring the publish-on-merge release trigger (#1467), read the unit whole and raised 14 findings, and three later passes over the same unit raised 19 more. None of the 33 is about that change, so all of them are pre-existing under local-strict-review "Disposing of Findings" and blocked nothing. Filing them here so they survive the session.

Four passes read this unit whole, each at a different digest as the one sentence the change touched was rewritten and then cut back. The findings below are the union, deduplicated, and each was verified against the tree or the live fleet by the pass that raised it. What the count implies about the unit is at the end.

Each was verified against the tree at cf7be2b (jq 1.8.2, git 2.47.3, gh 2.100.0). Grouped by what following the step literally does.

Loses a failure instead of reporting it

1. Step 5's release-trigger command exits 0 with empty output when its first stage fails, which is the outcome the same sentence forbids. The sentence requires "on a non-1 count exit non-zero rather than returning empty with success", but the guard lives inside jq, so a git show origin/main:registry/repos.json failure (an unfetched origin/main, a checkout with no registry/, a differently named default branch) writes to stderr and hands jq empty stdin. Verified: git show origin/main:registry/no-such-file.json piped into that same filter prints nothing and exits 0. The unit prescribes no set -o pipefail and no separate capture of the git show result.

2. Step 5's // fallback resolves to null with exit 0 when the registry declares no default, and the step then routes that into a dispatch. ($m[0].releaseTrigger // .defaults.releaseTrigger) followed by "Two cases, none versus anything else." registry/repos.schema.json does not require defaults, and spec/validate.py:559-563 errors only on an invalid value, so an absent defaults.releaseTrigger is valid. Three entries (DiskSpeedTest, GEM-Echo-Server, GoogleTo1Password) carry no releaseTrigger of their own and depend entirely on that fallback. Verified: with the key removed the filter prints null and exits 0, and null is not none, so it falls into "anything else" and the procedure dispatches a release for a repository whose trigger could not be resolved.

Destroys work

3. Step 8's worktree sweep has no test for the property it says bounds it. It says "Never apply this sweep to develop or main themselves, only to feature branches a drive-pr loop created", but the qualifying checks given are merged-with-baseRefName develop, matching tips, and not being develop/main. Nothing distinguishes a drive-pr branch from any other merged-to-develop feature branch, so a branch and worktree belonging to a different live task, merged but deliberately kept, passes every check and is git worktree removed and git branch -D'd.

4. A bracketed placeholder reads as optional, and on step 3 it silently succeeds when omitted. gh pr merge [number] --merge. Every other placeholder in the unit is angle-bracketed, and square brackets are the CLI convention for optional. gh pr merge with no number resolves the current branch's PR, so the reading the brackets invite merges a different PR rather than erroring. (pr_review.py's number is a required positional, so step 2 fails loudly instead.)

5. Step 7 does not say which checkout git checkout main runs in. "In the hub, when the chosen scope includes a release, bring this checkout to the merged content." "This checkout" is not resolved against step 5's "the hub checkout used for this procedure", and the session's own working tree is a third candidate. Following it in the session's tree switches that tree off its branch, and the preflight in the same step is scoped to .agents/skills/ and .claude-plugin/ only, so it does not catch that.

Cannot be followed as written

6. Step 6 matches on a value no prior step tells you to capture. "matched by headSha against the dispatched ref's tip ... and by createdAt against the dispatch time". Step 5 issues gh workflow run and never says to record the dispatch time, so the createdAt half cannot be evaluated by a reader following the steps in order. The headSha half is captured, in step 4.

7. Step 6's polling bound is unspecified while the wait immediately after it is exact. "Poll the list itself, within a bounded interval" and "Report and stop rather than guessing once the interval elapses", in a paragraph that then specifies 2700 seconds for the subsequent watch. Incomplete rather than wrong.

8. Step 6 names no bounded-wait mechanism for two hosts it says must be covered. "or the equivalent bounded-wait mechanism enforcing the same bound on a host without it (macOS without coreutils, native Windows)". The instruction cannot be followed literally on either named host.

9. Step 2 requires a hub checkout and never says how to get one. "From a hub checkout, scripts/ is not carried into downstream repos". Steps 5 and 7 depend on that checkout too. A reader carrying this skill in a downstream repository, which is the case the clause exists for, gets the constraint and no way to satisfy it.

10. Step 2 invokes a script bare where step 7 invokes one through python3. scripts/pr_review.py status ... against python3 scripts/skills_install.py --report. Both files carry #!/usr/bin/env python3 and mode 0755, so the bare form works on Linux and macOS but not on native Windows, which step 6 of this same unit names as a host the procedure must run on.

11. Step 8's first bullet gives no commands and no branch for its own absent case. "The promotion PR's own worktree: fetch and prune, remove the worktree, then fast-forward the base clone to develop." It is the only step prescribing actions with no command form, and it presumes a promotion worktree exists, where step 1 says "stop and report if none is open" for its analogous case.

States something false about the tree

12. Step 7's success criterion for the Skills refresh does not exist in the tool's output. "confirm --report now reads current". scripts/skills_install.py's report path prints a JSON object of stamp, currentCommit and stale, and returns 1 if stale else 0. It never emits the word "current". Relatedly, the unit orders --report before the install without noting that the first call is expected to exit 1, which a reader treating non-zero as failure stops on.

13. Step 7's stated reason for checkout over switch is false against the tree. "checkout rather than switch since the fleet's own git floor is undeclared". docs/host-setup.md:19 declares git's floor in the host-tool table, as none. The conclusion survives, the premise does not.

14. Step 8's rationale for the headRepository check describes something gh pr list cannot return. "a same-owner PR against an identically named branch in a different repository must never pass this check either". A gh pr list scoped to one repository returns only pull requests in that repository, so a different repository's PR can never appear regardless of the check. What the field actually rejects is a fork-head (cross-repository) PR. The check is worth keeping, the stated reason misleads about what it defends against.

Raised by the two later passes over the same unit

Two further passes ran over the unit after the publish-on-merge sentence was rewritten, and each read the whole text again. Four more pre-existing findings came out of them.

15. The Skills preflight's pathspecs are cwd-relative, so it passes vacuously from any subdirectory. "First assert git status --porcelain --untracked-files=all --ignored -- .agents/skills/ .claude-plugin/ is empty". The unit never says to run it at the checkout root or with git -C <root>. Verified: run from docs/, the command prints nothing and exits 0 however dirty those two directories are, so a stray file rides into the install, which is the exact failure the same sentence cites skills_install.py as guarding against.

16. Step 7 and step 8 contradict each other when the target repository is the hub. Step 7 does git checkout main, git merge --ff-only origin/main, and verifies the install stamp against that checkout, while step 8's first bullet fast-forwards "the base clone" to develop. Those are the same clone in the hub, so step 8 moves it off the main tip whose stamp step 7 just verified, and the unit never says the two may be the same checkout.

17. Step 6's final reporting instruction cannot be followed with the command the step supplies. "Report a timeout separately from a completed run's own conclusion, the tag or version it produced." The only command given is gh run watch <run-id> --exit-status, which reports status and conclusion and nothing about a tag or version.

18. --ref develop is stated unconditionally and at least one fleet publisher refuses it by design. KiCadLibrary's setup job fails fast when the event is a workflow_dispatch and the ref is not the default branch, emitting "Dispatch publish-release from the default branch ...; the matrix builds both branches", and its dispatch path publishes both branches regardless of ref. A --ref develop dispatch there produces a failed run rather than a prerelease.

19. Step 6's mid-dispatch-ref-change report fires on the ordinary case. "A run whose headSha does not match the expected tip at all, rather than simply being absent, means the dispatched ref moved between step 4's confirmation and the dispatch itself, report that distinctly." gh run list returns history at a default limit of 20, so the candidate list is normally full of older dispatch runs at other SHAs. Verified live: that exact query returned three runs with three different headSha values from three different days. Every poll before the new run registers therefore has the shape "list non-empty, nothing matches", which this sentence says to report as a ref change, during the very window the preceding sentence tells the reader to keep polling through.

20. Step 6's develop correlation value has no source and no moment. "or develop's current tip for a prerelease". The main case is pinned to a value step 4 confirmed. gh workflow run --ref develop resolves the ref server-side, while a reader's natural git rev-parse develop reads a local clone that can lag origin/develop, and the stale value then yields a non-matching headSha and, per finding 19, a false ref-change report.

21. The Skills preflight is scoped to two paths while the operations it guards are whole-tree. It asserts .agents/skills/ and .claude-plugin/ are clean and then runs git checkout main and git merge --ff-only origin/main. A modified tracked file anywhere else in the hub checkout passes the preflight and then either rides across the branch switch or aborts the checkout. The step names only two refusal modes on top of "what the preflight already ruled out", so the reader is told the preflight ruled out what would stop this step when it did not, and carrying another task's uncommitted work across branches is what the boundaries law exists to prevent.

22. Step 5's registry query never defines its <repo-name> placeholder, and it is not the spelling every other command in the step uses. It must be the registry's exact case-sensitive .name, where the surrounding commands take the owner/repo form that gh accepts in several spellings. A reader substituting the value that works for gh, or the wrong case, gets zero matches, the deliberate error(...) path, and a halted release step for a repository that is fine. The same step's "read this repo's releaseTrigger" compounds it, since the step runs in the hub checkout against a possibly different target repository, so "this repo" has two readings.

23. The quoting-safety rule mislocates the arguments it binds. "Every branch or worktree-path placeholder below is the real value, substituted as its own quoted argument." The placeholders that rule must govern include the branch in gh pr list --head and in the git ls-remote refs/heads/ guidance, both of which appear above that sentence in the same bullet. It is the one place the direction word matters.

24. Step 8's exactly-one guard aborts on the case the same bullet calls normal. The gh pr list --head query errors unless exactly one merged PR matches, three lines before "a different merged pull request can share the same head branch name against a different base, and that is never this sweep's target". The command carries no --base filter, so the baseRefName check it prescribes is unreachable whenever it would matter, and a branch that did finish against develop can never be cleaned once any second merged PR shares its head name.

25. The --exit-code 2 diagnosis is backwards for the only cleanup path the unit defines. It reads "most likely a prior cleanup attempt got interrupted after the remote delete but before the local one", but the procedure deletes the local branch first and the remote last, so an interrupted run leaves the remote present and the local gone.

26. Two tip-match preconditions state no disposition on failure. The remote and local tips "must match that exact pull request's headRefOid" before each delete, and the unit never says whether a mismatch stops the procedure, stops the sweep, or skips that branch, in a unit that spells out "stop and report rather than guessing" for every other inconclusive case.

27. Step 4 confirms a merge with no command, no source, and no fetch. "Confirm the merge landed, mergedAt set, main's tip matching the merge commit." The first git fetch origin main appears in step 5 and only when the scope includes a release, so read against an unfetched main this confirmation fails on a merge that did land. Step 3's gh pr merge output is never captured, so the merge commit's SHA has no source either.

28. Step 1's selection is the one the unit does not legislate. "Identify the open develop -> main promotion PR for this repo, stop and report if none is open" gives no command, while steps 2 and 3 immediately consume a PR number, and the unit elsewhere legislates exactly-one selection and fail-loud-on-a-non-1-count twice over.

29. Step 6 carries no scope guard, so a merge-only run polls for a dispatch nobody made. Steps 5 and 7 both gate on "when the chosen scope includes a release" and step 6 does not, so following the unit literally under a merge-only scope ends in step 6's bounded-interval "Report and stop" as though a release had failed.

30. Identical "stop and report" wording covers two different outcomes. Step 2's stop must end the procedure, while step 8 says steps 5 through 7's stops do not, listing "no release configured, a dispatch failure, an ambiguous run match, a timeout, a failed run, or a hub Skills refresh" as all still reaching cleanup. Step 7's own preflight stop is absent from that enumeration, so whether it ends the procedure is unresolved.

31. Step 6's concurrent-run warning names a collision its own query excludes. "A concurrent run of a different event on the same branch must never be mistaken for this one" guards a query that already carries --event workflow_dispatch. The collision that actually produces more than one candidate is a second dispatch run on the same branch, which the sentence does not name.

32. The -- end-of-options rule is applied everywhere except one command that supports it. The unit uses -- on git ls-remote, git branch -D, and git push origin --delete, states an explicit exception for git rev-parse, and then prescribes git worktree remove "<worktree-path>" with neither, though git worktree remove -- is supported. The unit's own justification for the surrounding quoting rule is exactly the case quoting does not cover and -- does.

33. Step 8's sweep and step 4's tip check run against a checkout the unit never establishes. The unit establishes only "a hub checkout" in step 2, and step 8's second bullet then runs git worktree list, git branch -vv, git ls-remote, git rev-parse --verify, git branch -D, and git push origin --delete against a clone of the target repository. For any target other than the hub, origin in the established checkout is the hub, so the sweep reads, and would delete, against the wrong remote.

What the count suggests

Four passes read this unit whole at four digests and raised 33 distinct findings, none of them introduced by the change that occasioned the passes, and each pass found roughly ten the previous one had not. That distribution is the argument for restructuring the procedure rather than landing 33 point fixes on it: the failures cluster into a few classes (a selection the unit does not legislate, a precondition with no stated disposition, a checkout with no antecedent, and a rationale sentence that has drifted from the command beside it), and fixing the classes is what stops the next pass finding ten more.

Verified clean in the same pass

git ls-remote --heads --exit-code, git rev-parse -- resolving no SHA yet exiting 0, the gh pr list single-match jq guard, --timeout 2700 in scripts/pr_review.py, the watched list and copy mechanics in scripts/skills_install.py, the plain-porcelain-is-silent-on-ignored-paths claim, git status --porcelain --untracked-files=all --ignored over two paths, the releaseTrigger enum, the human-merge-never-auto-publishes gate in publish-plan-task.yml, delete_branch_on_merge: false in repo-config/settings.json, docs/host-setup.md "Fleet Skills Install", publish-release.yml's dispatch trigger and ref assertion, gh run list's flags and JSON fields, gh run watch --exit-status, and git push accepting --.