Skip to content

Record: memories belong in their repo; atomic Copilot-request - #1124

Merged
d-morrison merged 2 commits into
mainfrom
ums/rpt-migration-learnings
Aug 4, 2026
Merged

Record: memories belong in their repo; atomic Copilot-request#1124
d-morrison merged 2 commits into
mainfrom
ums/rpt-migration-learnings

Conversation

@d-morrison

@d-morrison d-morrison commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Two reusable learnings from the Morrison-Lab/rpt Claude-workflow migration (rpt #178 / #181 / #183), committed to their owning repo per the "Encoding reusable feedback" rule.

1. CLAUDE.md — memories belong in their repo; don't offer, commit

Reinforces the existing "decide on your own --- don't ask" rule with the move it didn't rule out: parking a reusable learning in session-local auto-memory and then offering to upstream it. Directive: "always put memories in the repos where they belong; don't wait for confirmation." Do/Don't pair + incident.

2. shared/workflow/pr-on-claim.md — run the Copilot-request POST as the sole/last command

The no-unreviewed-pr Stop hook discharges the reviewer-request obligation only when the requested_reviewers POST is the last command in its Bash call (its is_error is the whole call's exit status). Chaining gh pr view --json reviews / gh pr checks after it makes the request non-last → the hook treats it as ambiguous and keeps warning even on a 200. This is fail-fast's "a combined result cannot attribute a per-step outcome" applied to a review request. (rpt#181: this re-fired the hook across six turns before it was diagnosed.) The pre-existing example block is split in this change so it demonstrates the rule rather than the anti-pattern.

Dropped during review

A third addition to memories/github.md (a workflow-only-PR changelog/version-check exemption note) was removed in dfc9b5b: it was redundant with the existing, more careful entry in memories/github-actions.md (lines 459-482), misplaced (that file is CLI/API mechanics), and over-generalized to "all UCD-SERG template repos" --- refuted by that entry's own serocalculator counter-example.

Self-reviewed: check-new-line-breaks clean, added lines ASCII-only.

🤖 Generated with Claude Code

…ow-only PR exemption

Three reusable learnings from the Morrison-Lab/rpt Claude-workflow migration
(rpt#178/#181/#183):

- CLAUDE.md "Encoding reusable feedback": memories go in the repo that owns
  them, not session-local auto-memory, and you commit them rather than offering
  to. Reinforces the existing "don't ask" rule with the incident that violated
  it.
- shared/workflow/pr-on-claim.md: the no-unreviewed-pr Stop hook discharges a
  Copilot review request only when the requested_reviewers POST is the last/atomic
  command in its Bash call; chaining verification after it keeps the hook warning.
- memories/github.md: a workflow-only PR is exempt from news.yaml + version-check
  (paths-ignore .github/workflows/**); adding any other file trips both, fixed
  with the no-changelog + no-version-increment labels (which re-trigger on labeled).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@d-morrison
d-morrison requested a review from Copilot August 4, 2026 02:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread shared/workflow/pr-on-claim.md
Comment thread memories/github.md Outdated
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

… github.md section

- pr-on-claim.md: split the requested_reviewers example so the POST stands
  alone and the verification reads follow in a separate block, matching the
  new sole-command rule (it previously showed the chained anti-pattern).
- memories/github.md: drop the workflow-only-PR exemption section. It was
  misplaced (that file is CLI/API mechanics, not Actions config), redundant
  with the existing more-careful entry in memories/github-actions.md
  (lines 459-482), and over-generalized to "all UCD-SERG template repos" --
  which that entry's own serocalculator counter-example refutes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@d-morrison d-morrison changed the title Record: memories belong in their repo; atomic Copilot-request; workflow-only PR exemption Record: memories belong in their repo; atomic Copilot-request Aug 4, 2026
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Claude finished reviewView run


Already verified in prior rounds and unchanged in this diff, so I won't re-run that check. The diff at the current head is identical to what round 2 reviewed (head dfc9b5b, unchanged), and the one remaining non-blocking item from round 2 — the PR title/body still describing the dropped memories/github.md section — has since been corrected: the title now reads "Record: memories belong in their repo; atomic Copilot-request" and the body includes an explicit "### Dropped during review" section explaining the removal.

Re-confirmed independently this round:

  • No banned punctuation (em/en-dash, curly quotes, ×) in any added line.
  • No genuine multi-sentence added lines (one flagged by a naive script check turned out to be a single sentence with an internal colon).
  • Diff matches exactly what round 2 reviewed and fixed: pr-on-claim.md's POST example is now split into its own block with a separate verification block, and the memories/github.md section is fully removed.
  • PR title and body are now synced with the actual two-file diff.

No new findings. The prior verdict from 2026-08-04T02:53:31Z — "Needs minor changes... The only remaining item is updating the PR title/description" — has had that remaining item resolved, so nothing stands in the way of merge now.

Verdict

Ready for merge

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

💰 Cost: $2.5737 (review) — run

@d-morrison
d-morrison merged commit a9ded3e into main Aug 4, 2026
16 checks passed
@d-morrison
d-morrison deleted the ums/rpt-migration-learnings branch August 4, 2026 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants