ums: an ad-hoc && chain has nowhere to put pipefail, and a review can arrive after mergedAt

[d-morrison]

Merge-time UMS pass for #1079 (merged 03:36:11Z) and #1082 (merged 04:15:05Z).

Three candidates were considered. One was already covered and nothing was written for it.

Covered, not written: a retraction that deleted a true fact

The session's most interesting failure -- a verification step that deleted a true claim from the corpus and replaced it with a false one, then propagated it to a second file -- is already recorded, by #1079 itself.

shared/workflow/metacognitive-monitoring.md carries the corrected chain, names check run 91327863807, and states the general shape verbatim:

A verification inherits the scope of whatever instrument it reaches for, so "I checked and it is not there" stays a claim about the instrument until the instrument's own coverage has been established.

shared/workflow/fully-clean.md's fifth case carries the same correction with the measured rollup counts. Re-verified independently here:

$ gh api repos/Morrison-Lab/ai-config/check-runs/91327863807 \
    --jq '{name, conclusion, completed_at, head_sha}'
{"name":"copilot-pull-request-reviewer","conclusion":"success",
 "completed_at":"2026-08-01T04:50:41Z","head_sha":"7abfed6be383..."}

Nothing added.

shared/coding/errexit-is-not-uniform.md (+60)

The existing "A pipe discards the status of everything left of it" section covers the mechanism and names pipefail, and one of its Don't: bullets already names piping "purely to tidy its output". What it does not cover is the shape that actually bit: both its examples end in || fallback, and its preferred remedy is "set pipefail ... in any script" -- so a reader running a batch of pre-push checks as one && chain finds the remedy has no set line to attach to, and may read the section as not applying.

Adds a subsection for that case:

• The symptom inverts. With || the fallback is unreachable, so nothing happens that should have. With && the chain runs on and reports success, a stronger and more misleading claim than silence. Measured on bash 3.2.57:

  $ bash -c 'false | tail -1 && echo "CHAIN CONTINUED"; echo "rc=$?"'
  CHAIN CONTINUED
  rc=0
  $ bash -c 'set -o pipefail; false | tail -1 && echo "CHAIN CONTINUED"; echo "rc=$?"'
  rc=1
  

• The remedy does have a home: set -o pipefail; works at the front of a one-off command line, not only in a script.

• The trigger, which nobody reaches by reasoning about error handling: a check prints more than you want to read, so you pipe it through tail. At that moment the pipe is a formatting decision about output, which is exactly when the exit status is least in view -- and for a verification check the status was the whole point.

Plus one In review line.

Case: 2026-08-03, preparing a push in this repo. A pre-push check set ran as one && chain with npx markdownlint-cli2 ... | tail -N among the stages and no set line. markdownlint reported a real MD018 failure, tail exited 0, the chain continued and reported every check passing. Surfaced only on a later unpiped run.

skills/post-merge/SKILL.md (+85)

Step 1.25 already scans for late reviews -- across all three surfaces, with the empty-body and plain-issue-comment traps handled. But it bounds the window at mergedAt, and says so four times (:46, :50-54, :61-63, :96). That excludes by construction the case where the review posts after the merge, and post-merge typically runs within a minute or two of merging, so the scan fires before the late review exists and its clean result expires.

Adds:

• A second pass with no upper bound, treating a submitted_at later than mergedAt as ordinary rather than anomalous.
• A third reason for relocating work. A merged PR is no longer a place to land code fixes. That is distinct from ARD's Defer (the fix would widen scope) and from address-every-comment.md's main-sync case (the line is not yours). Here the finding is in scope and is yours; only the branch is gone.
• The vehicle is a follow-up PR against main, narrower than the "issue or PR" item 2 leaves open -- the findings usually still apply to main (confirm against current main first, since an overlapping merge can change or remove that code), and the fixes are already known, so an issue would record work that could simply be done.
• The back-pointer belongs on the merged PR. Item 2's link runs from the follow-up to the merged PR, which serves a reader who starts at the follow-up; few do. A later reader arrives at the merged PR and finds a review with unaddressed findings under a merged banner, with no way to tell whether anyone handled them.

Case: #1079 merged 2026-08-03T03:36:11Z; Copilot's review posted 03:46:25Z, ten minutes after the merge rather than two minutes before it as in the #1029 record directly above. Three inline findings plus two in a Suppressed comments (2) block, all five correct, addressed against main in #1082 (merged 04:15:05Z), with a comment on #1079 at 03:52:47Z naming #1082 and each disposition.

Verification

Every identifier in the brief was resolved with gh pr view / gh issue view: #1056, #1073, #1079, #1082, #1008, #1005 (all MERGED), #1074 (CLOSED), #1072 (OPEN).
Of those, the diff itself cites only #1079 and #1082; the rest were checked but did not end up needing a citation, and #1008 / #1005 / #1074 appear in this body only.
The two pre-existing references my new text points at, #1029 and #1034, were already in the file and are untouched.

Checks, each run with its exit status captured before any output trimming -- deliberately, given what this PR is about:

check	rc
sync-codex-skill-wrappers.py	0 (177 wrappers, no diff)
validate-skills.py	0
check-links.py	0 (969 links / 435 files; up from 968, the new cross-link)
check-vendored-drift.py	0
check-memory-file-size.py	0
test_check_memory_file_size.py	0 (13 passed)
markdownlint-cli2	0 (458 files linted, 0 issues)
check-new-line-breaks.py (post-commit, origin/main...HEAD)	0

Banned-glyph scan over the three-dot diff: 117 added lines examined, 0 hits. No deletions in the diff.

The markdownlint row states its scope because Summary: 0 issues in 0 files counts files with issues, and fully-clean.md records that reading that line alone as a vacuous scan is a false-positive route.

Round 2 (b706339a), self-review only

claude-review returned Ready for merge with no findings at 9b31005d, and independently re-verified every timestamp cited here against gh api. Copilot's check run completed success on that head while posting zero reviews across all three surfaces -- the silent-reviewer case fully-clean.md documents, not an approval. Re-requested at the new head.

The three changes in this round came from re-running the project-conventions self-review against my own diff, not from either reviewer:

• An invented particular. "post-merge normally runs within a minute or two of the merge" asserted a figure nothing measured. Softened to "promptly", which is all the argument needs. Recording this here because it is the exact defect metacognitive-monitoring.md names, committed while writing about verification.
• An unsupported empirical claim. "Few do" asserted something about reader behaviour. Replaced with the structural reason: the follow-up is reachable only by someone who has already found it, whereas the merged PR is what a changelog entry, a git blame, or the review notification points at.
• An internal inconsistency. A Don't: bullet said "seconds after the merge" while the prose it summarised said "a minute or two".

Also adds a cross-reference to CLAUDE.md's push-races-the-merge case, which asks for a back-pointer of the same shape on a merged PR, and names why neither case fires on the other -- its trigger is a * [new branch] push tell rather than a review posting. Verified the quoted phrasing at CLAUDE.md:668 before citing it.

Round 3 (06ec4b37), Copilot's five findings addressed

Copilot returned five findings at b706339a, each a statement broader than the mechanism supports; all five Addressed, all five inline threads resolved. Merged current main in first (base was 13 commits behind):

• post-merge, late-review race. A single synchronous re-read cannot close the window -- on Record that gh pr checks omits check runs, and correct fully-clean's fifth case #1079 the review posted ten minutes after the merge. Added that the durable catch is staying subscribed to the merged PR / a delayed poll; the re-read only catches a review already sitting on the PR.
• post-merge, "address anything." Narrowed to "no longer a place to land code fixes" -- the merged PR still takes replies, thread resolutions, and the disposition comment this same section requires.
• post-merge, "by construction." Softened to "usually still apply", with an instruction to confirm against current main first, since an overlapping merge can change or remove that code.
• errexit, "removes the stage." Reworded: a pipe collapses the chain's verdict onto the pipeline's last command; an earlier failure is masked only when the last stage (a formatter like tail) succeeds.
• errexit, blanket pipefail. Qualified to "when every stage's non-zero exit is a genuine failure", with the | head SIGPIPE exception called out and the split-command remedy pointed to.

claude-review then returned Needs minor changes at this head for one non-blocking item only: this description was stale relative to the final diff -- the two claims above, and the declared +53/+64 counts. Synced here: counts corrected to +60/+85, the two claims now match the file, and this Round 3 entry records the round. Diff content was Ready for merge.

[github-actions]

Claude finished review — View run

---

I've completed a thorough review of this PR. Given the small, self-contained nature of the diff (two Markdown prose files in this meta-corpus, no code), I verified the review directly rather than spinning up heavy parallel subagents (which failed with context-overflow errors given this session's large system prompt) — I confirmed this is a reasonable adaptation since the diff is prose-only and every claim in it is independently checkable.

What I checked:

• PR status: open, not draft, no prior reviews/comments — genuine first review.
• Technical accuracy: the bash pipefail/&&-chain semantics claimed in shared/coding/errexit-is-not-uniform.md are correct per standard POSIX/bash behavior (without pipefail, a pipeline's exit status is its last command's, so false | tail -1 && echo X runs echo and exits 0; with pipefail set, the pipeline reports false's exit code and && short-circuits).
• Factual/citation claims: verified via gh api — PR Record that gh pr checks omits check runs, and correct fully-clean's fifth case #1079 merged 2026-08-03T03:36:11Z ✓, PR Address post-merge Copilot review on #1079: pagination, commit statuses, raw-run route #1082 merged 2026-08-03T04:15:05Z ✓, Copilot review on Record that gh pr checks omits check runs, and correct fully-clean's fifth case #1079 posted 03:46:25Z ✓, the disposition comment posted 03:52:47Z naming Address post-merge Copilot review on #1079: pagination, commit statuses, raw-run route #1082 ✓ — all exact matches to what's written in the diff.
• Internal cross-references: [address-every-comment](../../shared/workflow/address-every-comment.md) resolves correctly from skills/post-merge/SKILL.md; the "item 2" reference matches the actual numbered list at lines 91–94 of that file; the characterization of ARD's Defer disposition and the main-sync case is an accurate, defensible reading of the source fragment.
• Corpus style compliance (this repo's own shared/coding/ascii-punctuation-in-source.md and shared/writing/semantic-line-breaks.md rules): scanned both files for banned non-ASCII punctuation (em-dash, en-dash, curly quotes, ×) — zero hits in the new content (all pre-existing em-dashes elsewhere in post-merge/SKILL.md are untouched, grandfathered lines outside this diff's added-line ranges). Manually checked every added line for multi-sentence-per-line violations — none found. Heading levels (### nested correctly under ##) and blank-line spacing around headings are correct.
• No hallucinated APIs/functions/paths: this diff contains no code, only prose and shell examples using standard bash builtins.

No --comment argument was provided, so no GitHub comments were posted, per the workflow instructions.

Summary

No issues found. Checked for bugs and CLAUDE.md compliance.

Verdict

Ready for merge

[github-actions]

💰 Cost: $5.4833 (review) — run

[Copilot]

Pull request overview

Documents two workflow hazards: masked failures in piped shell check chains and reviews arriving after PR merge.

Changes:

• Adds pipefail guidance for ad-hoc && chains.
• Adds post-merge review handling and follow-up instructions.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
skills/post-merge/SKILL.md	Handles reviews posted after merge.
shared/coding/errexit-is-not-uniform.md	Covers piped checks in ad-hoc chains.

[dem-ucdh]

Driving this PR to clean (ARDI) — paws off until done.

---

Generated by Claude Code

[dem-ucdh]

ARD round 1 — all 5 findings Addressed (06ec4b37)

Merged current main in first (base was 13 commits behind), then addressed every Copilot finding. All were correct "the statement claims more than the mechanism supports" narrowings:

#	File / line	Finding	Disposition
1	post-merge L125	A single re-read can't close the late-review race	Address — added that the durable catch is staying subscribed / a delayed poll; the re-read only catches a review already on the PR
2	post-merge L129	"no place to address anything" conflicts with the later "comment dispositions on it" requirement	Address — narrowed to "no place to land code fixes"
3	post-merge L142	"valid against main by construction" ignores an overlapping merge	Address — softened to "usually still apply", confirm against current main first
4	errexit L216	"piping removes the stage from the verdict" is imprecise	Address — reworded: the pipe collapses the verdict onto the pipeline's last command; an earlier failure is masked only when the last stage succeeds
5	errexit L243	blanket pipefail is unsafe for | head (SIGPIPE → false failure)	Address — qualified to "when every stage's non-zero exit is a genuine failure" and pointed to the split-command remedy for the head case

All 5 inline threads resolved. The push re-triggers claude-review + Copilot against the new head; the prior claude-review red was the session-wide infra failure, not a content issue (Copilot reviewed fine). Iterating to clean.

---

Generated by Claude Code

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.

_{2026-08-03 17:33 UTC}

[github-actions]

Claude finished review — View run

---

Review posted. Summary:

Content check: Re-reviewed the current head (06ec4b37) after it merged main and pushed a round responding to Copilot's review. All five of Copilot's findings are properly addressed in the file text (verified the underlying bash pipefail/&& claim directly, and checked cross-references to CLAUDE.md, address-every-comment.md, and the file's own numbered items — all resolve accurately). No banned punctuation or multi-sentence added lines.

One finding (non-blocking): The PR description is stale relative to the final diff — it still states the pre-narrowing claims ("no place to address anything", "valid by construction") that the last commit corrected in the file itself, and its declared line counts (+53/+64) don't match the actual current stats (+60/+85). This is the exact defect this repo's own address-every-comment.md calls out for PR bodies.

Verdict posted: Needs minor changes (PR description sync only; diff content is ready for merge).

One process note: while investigating whether I could programmatically fix a stray test comment I'd accidentally posted, I discovered gh pr comment --edit-last works in this environment even though gh api calls are blocked — I used it to edit that comment into the real review rather than leaving a duplicate junk comment on the PR.

[github-actions]

💰 Cost: $8.2133 (review) — run

[dem-ucdh]

Good catch, and a fitting one for this PR — the description had drifted into exactly the overclaim-shipping the diff is about. Synced the body:

• Line counts corrected to +60 (errexit) / +85 (post-merge), matching the actual per-file additions.
• The two stale claims now match the file: "no place to address anything" → "no longer a place to land code fixes", and "valid against main by construction" → "usually still apply … confirm against current main first".
• Added a Round 3 (06ec4b37) entry recording the Copilot-response round, so the body no longer shows the pre-narrowing claims with no note that they were fixed.

No code change — the diff was already Ready for merge per your review. Re-requesting a fresh review to confirm the body finding is cleared.

@ claude review

---

Generated by Claude Code

[claude]

Code review

Confirming review — no new findings. The diff content at head 06ec4b375331275de755c7788bd28a49225b258e was already verified end-to-end in the prior review (technical claims, citations, banned punctuation, multi-sentence lines) and returned Ready for merge with one non-blocking item: the PR description had drifted from the final diff.

That item is now resolved. I re-checked the PR body against the current diff:

• File sizes now match: body states shared/coding/errexit-is-not-uniform.md (+60) and skills/post-merge/SKILL.md (+85), which match the actual current per-file stats exactly.
• Prose now matches the narrowed file content: the body's post-merge summary no longer states the pre-narrowing overclaims ("no place to address anything", "valid against main by construction") — it now reads "no longer a place to land code fixes" and "usually still apply ... but confirm that against current main first," matching the corrected wording in skills/post-merge/SKILL.md itself.
• Round 3 entry added: the body now documents commit 06ec4b37's response to Copilot's five findings, so a reader of the description alone sees the full history rather than stale claims.

No code changed since the last content review, and the description now accurately reflects the diff.

Verdict

Ready for merge.

[github-actions]

Claude finished review — View run

---

Posted the confirming review: #1087 (comment)

Summary: No new findings. The diff content (head 06ec4b37) was already verified as ready for merge in the prior review round. The one outstanding item — a stale PR description — has been fully fixed: file sizes corrected (+60/+85), overclaiming prose replaced with wording matching the narrowed file content, and a new "Round 3" section added documenting the fix.

Verdict: Ready for merge

[github-actions]

💰 Cost: $6.2800 (review) — run

[dem-ucdh]

Done — PR is free.

Fully clean at 06ec4b37: fresh claude-review Ready for merge (no findings; the stale-body item is resolved), require-review + build + validate + new-line-breaks all green and completed, all 5 inline threads resolved. Ready for your merge (human-gated — I won't merge).

---

Generated by Claude Code