ums: disabling the @claude agent, and the version-check parity trap

[d-morrison]

UMS pass from disabling the @claude agent bot in two repos: UCD-SERG/serodynamics#282 and UCD-SERG/serocalculator#627, both merged.

memories/claude-bot-workflows.md -- new section

The load-bearing finding: commenting out the triggers does not disable the agent. The reusable claude.yml runs unattended on workflow_dispatch (its gate exempts that event and schedule deliberately), and GitHub rejects a workflow file with no on: key at all -- so a placeholder trigger has to stay and the job needs if: false on top of it. Either mechanism alone leaves the agent runnable by anyone who can press "Run workflow".

Three more, each of which cost a decision on those two PRs:

• Disabling claude.yml can remove the only path that starts a review, since the mention is what dispatches one. In a repo whose automatic pull_request review is already off, that leaves no review path at all.
• Turning off the pull_request review trigger can block every PR in the repo, if require-review is a required status check. This is invisible in the diff, since branch protection is not in the repo.
• A /review dispatch needs --ref <pr-head-branch> or its check-run lands on the default branch. Fork PRs are the exception and must fall back to no --ref.

memories/github-actions.md -- narrows an over-general claim

The corpus recorded the no version increment label bypass as a property of the shared RMI-PACTA-derived version-check workflow. It is per-repo: serocalculator's copy gates every later step on a check_label step, and serodynamics' has no such step at all, so its stopifnot(working_version > compare_version) is unconditional (grep -c check_label returns 0 there).

Adds the case that produced this. A workflow-only PR is exactly the one that forgets the bump, because nothing in the change is about the package -- so it sits at parity from its first commit rather than drifting into it. The existing parity guidance only fires when main advances past you, which is a different situation; both PRs above went red on version-check with diffs containing no R code.

memories/r-quarto.md -- one line-neutral rewrite

Its own one-sentence version of the same claim, corrected in place. This also repoints a cross-reference that read "see the label-bypass note below" while the note it means lives in github-actions.md.

Why the detail sits in github-actions.md

Corrected, since the original reason has expired. The first version of this PR put the detail in r-quarto.md, which then sat at exactly 1200 lines on main -- check-memory-file-size.py's threshold -- so 18 added lines tipped it to 1218 and failed validate. That pressure is now gone: #953 moved a 470-line section out to memories/r-cloud-sessions.md, and r-quarto.md is down to 730.

The placement still stands, on the remaining and better ground: github-actions.md is where the claim being narrowed actually lives, so the correction sits directly under it. Recording this so the reason on file is the one that holds, not the one that happened to force the move.

Verification

At head 882d2fb:

• validate's checks pass locally: check-memory-file-size.py, validate-skills.py, check-links.py, check-vendored-drift.py.
• gha's check-new-line-breaks.py reports No lines missing semantic breaks over origin/main...HEAD.
• 60 added lines; no em-dashes, curly quotes, or other banned non-ASCII punctuation.
• The serodynamics claim is checked against that repo's actual version-check.yaml rather than inferred from the shared origin.

Correction. An earlier version of this section claimed "no multi-sentence lines" at head 83d2366. That was false -- there were 12, and the round-1 review caught 8 of them. My pre-push check tested count('. ') > 1, which only fires at three sentences on a line. Fixed in 882d2fb and re-verified with the real instrument rather than a hand-rolled one; see the ARD reply.

Review status

Round 1 returned Needs more work with two real findings, both since Addressed. Note for the record that the first two review attempts produced no verdict at all -- one skipped for API quota, one a 10-minute stub that never got past "Load tools" -- and a self-review was posted in the thread at the time, since neither is an approval. A manual re-run then produced the genuine round-1 verdict.

Both source PRs are merged, so nothing here is contingent on unlanded work.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BSYXLnUqVqDXLZtPpFSaQM

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

Warning

Claude review skipped — API quota exhausted. The CLAUDE_CODE_OAUTH_TOKEN account's API quota is exhausted. Re-trigger the review by pushing a new commit or re-running the workflow once the quota resets. View run

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[d-morrison]

Self-review (bot review was quota-skipped)

claude-review skipped for API quota on the previous head, so require-review went gray rather than green. A quota skip is not an approval, so here is a self-review at 83d2366, applying the same standards the bot would. A fresh review run started off this push; if it produces a genuine verdict, that supersedes this.

What I checked

This diff is prose-only (three memory files, 58 added lines), so the applicable checks are the fact-check and prose ones rather than the code ones.

Every factual claim, against a source:

Claim	Checked against	Result
The reusable claude.yml runs unattended on workflow_dispatch/schedule	Morrison-Lab/gha .github/workflows/claude.yml job if:, L239-244	holds -- both events are explicit disjuncts with no author gate
GitHub rejects a workflow with no on: key	Actions behavior; the reason a placeholder is needed	holds
A /review dispatch needs --ref, with a fork fallback	gha examples/claude-code-review.yml	holds -- the example carries both, with the fork rationale in a comment
serodynamics' version-check.yaml has no check_label step	grep -c check_label in that repo	holds -- returns 0
serocalculator's gates every later step on check_label	that repo's version-check.yaml, L21-30	holds
Both PRs went red on version-check with no R code changed	the two runs	holds -- both failed, both diffs were .github/workflows/ plus NEWS.md

One finding, in my own diff, fixed in 83d2366. The bullet originally read that bcs and serocalculator "both gate every later step of version-check.yaml on a check_label step". I read serocalculator's file directly; bcs is not checked out in this session, and the note directly above establishes only that bcs has the bypass, not how it is implemented. That is a structural claim about a file I never opened, inferred from a sibling. Narrowed to attribute the verified structure to serocalculator and to say plainly that bcs's mechanism was not checked here.

Prose checks: no banned non-ASCII punctuation on added lines; no multi-sentence lines; semantic line breaks throughout; no AI tells (no rule-of-three padding, no "it's not just X, it's Y", no promotional register). No forward references -- the one cross-reference points to a named file rather than a vague "below", which is itself a fix to an existing dangling pointer.

Redundancy: the r-quarto.md sentence and the github-actions.md bullets state the same rule at two lengths. Kept deliberately rather than merged: the short form sits in the R-package CI-gates section where a reader hits the question, and points at the long form. Merging would leave the R-package section silent on a gate it otherwise enumerates.

One thing a reviewer should push back on if they disagree

The PR body's stated reason for the file placement changed under me, and I rewrote it rather than leaving the original standing. The first version moved the detail out of r-quarto.md because that file was at the 1200-line cap; #953 then merged into this branch and moved 470 lines out to r-cloud-sessions.md, dropping it to 730. So the forcing reason is gone and only the better reason remains (the claim being narrowed lives in github-actions.md). Moving it back is now a live option -- I don't think it's the right one, but it is no longer foreclosed, and the body says so rather than quietly keeping the old justification.

Verdict

Ready for merge, with the caveat that this is a self-review rather than an external verdict.

---

Generated by Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[d-morrison]

ARD round 1 -- both findings Addressed (882d2fb)

1. MD022, missing blank line before ## -- Addressed

Confirmed before fixing. Line 527 ended ...not text to accept.) and line 528 opened the heading with nothing between. My insertion put a blank line after the new section but not before its own heading. Blank line added.

2. Multi-sentence lines -- Addressed

Confirmed, and worse than reported: the finding listed 8, and the real count was 12. Reflowed all of them at sentence boundaries in both files. check-new-line-breaks now reports No lines missing semantic breaks.

Correction to my own earlier claim

My self-review above and the PR body both stated "no multi-sentence lines". That was false, and the finding is the reason I know it. My pre-push check tested count('. ') > 1, which only fires when a line holds three sentences, so every two-sentence line passed it silently. The PR body has been corrected.

Two things follow, and the second is the one worth keeping:

• The advisory nature of check-new-line-breaks meant its green CI result was never evidence either -- it warns and exits 0. So both my instruments were reporting clean for unrelated reasons, one broken and one non-blocking.
• The fix was not to repair my ad-hoc detector but to run the real one (gha's check-new-line-breaks.py), which is the instrument that exists for exactly this. Reaching for a hand-rolled substitute when a maintained one is one path away is the error underneath the wrong regex.

For what it's worth, a second ad-hoc check of mine misfired in the same session: a banned-punctuation scan reported a false hit because shell heredoc quoting mangled its character class into including ASCII ". Rewritten as a file rather than an inline heredoc; reports 0 over 60 added lines.

On finding 3 (external claims)

No action needed, and no disagreement -- you correctly flagged these as unverifiable from this repo. They were each checked against the source in this session, from local checkouts you don't have: Morrison-Lab/gha's claude.yml job if: for the workflow_dispatch/schedule exemption, its examples/claude-code-review.yml for the /review stub and the --ref fork fallback, and UCD-SERG/serodynamics's own version-check.yaml (grep -c check_label returns 0). Recording that here so the next reader knows the basis rather than having to re-derive it.

---

Generated by Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[claude]

Claude finished @d-morrison's task in 5m 50s —— View job

---

Round 3 Review — bb45395

Verdict

Ready for merge.

What this round checked

bb45395 pushed a single targeted fix: the two-sentence line flagged in the Round 2 review at memories/claude-bot-workflows.md ~line 570.

The Round 2 finding was:

agent disabled in both. serodynamics needed the /review path built, having relied entirely on the mention; packs two sentences on one line. The second sentence starts with lowercase serodynamics, which is why check-new-line-breaks.py's lookahead (requiring uppercase or quote) missed it.

The fix in bb45395:

-  agent disabled in both. serodynamics needed the `/review` path built, having relied entirely on the
-  mention; serocalculator already had it and needed the
+  agent disabled in both.
+  serodynamics needed the `/review` path built, having relied entirely on the
+  mention; serocalculator already had it and needed the

This correctly splits the two sentences at the period, placing serodynamics needed... on its own line. Fix is confirmed.

Scan of bb45395 for new issues

• Multi-sentence lines: grep -E '\.[[:space:]]+[A-Z]' over all added lines → 0 hits. Manual review of the four added/changed lines finds no new sentence-boundary violations.
• Banned punctuation (em-dashes, curly quotes): No non-ASCII punctuation in the changed lines.
• MD022: No new headings introduced; no blank-line issues.
• Factual accuracy: No prose claims to re-verify; the diff is purely a line-break split.

All Round 1 and Round 2 findings are resolved. No new findings in Round 3.

Note on Copilot

Copilot has been quota-refused on every review attempt for this PR (5+ refusals). Per fully-clean's fifth case, a refusing reviewer is not "reachable" — this verdict rests on claude[bot]'s review alone, with Copilot's absence noted explicitly rather than papered over.

[d-morrison]

ARD round 2 -- the one finding Addressed (bb45395)

Nit at claude-bot-workflows.md:570, two sentences on one line -- Addressed. Split applied, with the following line re-wrapped so the paragraph still reads at one clause per line. Both review threads are now resolved.

The tool blind spot is now tracked upstream

Filed as Morrison-Lab/gha#389, as promised in the thread.

Reproduced against the script rather than inferred from the regex:

m.split_sentences("agent disabled in both. serodynamics needed the /review path built.")
#=> 1 sentence   <- wrong
m.split_sentences("agent disabled in both. Serodynamics needed the /review path built.")
#=> 2 sentences  <- correct

Only the capitalization differs. _SENT_BREAK_RE's lookahead (?=[A-Z\"'*[])requires the next sentence to open with an uppercase letter, quote, backtick, asterisk, or bracket, so a bare lowercase identifier never forms a boundary. A *backticked* identifier already works, since `` `` is in the class -- it is specifically the unbackticked lowercase word that escapes.

I deliberately did not send a patch. Widening the class to lowercase would start splitting on decimals (0.9012), versions (v2.1), and extensions (.github), and false positives on an advisory check are worse than the gap, since they train everyone to ignore it. That tradeoff belongs to whoever owns the check; the issue lays out two directions and the regression cases either would need.

Worth recording about this PR's review lifecycle

Four review attempts to get two rounds of verdict: quota skip, a 10-minute stub, then round 1 and round 2. The stub needed a manual re-run to clear. Nothing here is a defect in the diff, but it is a fair illustration of why a green require-review and a posted verdict are separate facts.

The nit itself is also a small argument for the reviewer: the automated check reported this file clean, and a human-style read caught the line anyway.

---

Generated by Claude Code