Skip to content

docs: write the merge-ready signal and two verification rules into the agreement - #110

Merged
dborgards merged 5 commits into
mainfrom
docs/merge-ready-signal
Sep 13, 2026
Merged

dborgards merged 5 commits into
mainfrom
docs/merge-ready-signal

Conversation

@dborgards

Copy link
Copy Markdown
Owner

What does this change?

Writes three rules into CLAUDE.md that the last two pull requests earned, and fixes the three places in that file which the new rules immediately condemn.

1. „FERTIG — mergebar" is what says a branch is finished

The convention you chose, now where it applies rather than only in the retro. A status message naming the new head is not the signal: 9b5248f and 9f172d6 were merged past on #104, 8ebced1 on #107 — all three pushed shortly before a merge and all three announced here. The gap is between the push and the merge, not between us, so being quicker cannot close it. Only a state that says no further push is coming can.

Writing it turned up two rules it collides with, and both are reconciled in place rather than left to compete:

  • A red leg the branch did not cause does not block the signal — otherwise the rule would be unusable on this repository, where docs: add the working-practice retro for the 12/13 September issue wave #109 saw five macOS runs and no two alike. But it has to be named in the same breath, with what is failing and where it is recorded, so the merge decision rests on facts and not on a green tick.
  • A finding arriving after the signal still gets its thread answer — a pull request is finished when every thread is closed does not lapse. What waits is the push, which becomes a decision for you, put as a question with options, rather than a fix quietly racing your merge.

2. A figure belongs in one place; everywhere else points at it

This one cost the most on #109 and is the most reusable. A summary that restates a count proved elsewhere goes stale on the next change to the evidence, and silently, because each of the two places stays internally consistent. Keeping them in sync failed three times in one review, twice in the correction of the previous failure — so the rule is to delete the duplicate rather than sync it again.

Bounded deliberately: it binds living text, not a dated record. A retro states its figures as of its date and is read that way; a rules file, a description or a summary is read as current, and that is where a stale number misleads.

3. A claim about where something came from is checkable, so check it

Provenance is in the history and I wrote it from memory: two of three examples in a list on #109 were wrong — one finding credited to a review that had not made it, and a pair of sentences offered as a contradiction when the commit resolving them had explicitly classified them as not one. Cite the commit rather than paraphrasing it, and read it before citing.

What applying the new rules to this file found

Rules 2 and 3 are only worth having if they survive contact with the file that states them. They did not, three times:

place defect now
Dependabot paragraph restated the per-ecosystem limits from .github/dependabot.yml, and omitted github-actions — which produces pull requests too, on the default limit points at the config, which is where those numbers belong
merge-queue aside restated a merge_group run count that becomes wrong the moment #106 lands „It has never run", with #106 named as the evidence
clock rule „five distinct tests in three packages" — already wrong; #92 records eight across four points at #92, which counts them

The first is a correctness fix independent of the rule: the fan-out argument omitted an entire ecosystem.

Two further defects came out of drafting and are fixed in the same commit: the new rule pointed at „the next rule" for something two sections away, and described the red-leg procedure as being „above" when it is below. Both are the cross-reference form of the same class.

Also

Removes the retro's open item for this convention, which this change makes stale — the retro now points at CLAUDE.md for the rule and keeps only its derivation. A defect this branch causes, closed in this branch.

Type of change

  • feat — new behaviour (minor release)
  • fix / perf — bug or performance fix (patch release)
  • docs / test / refactor / chore / ci — no release
  • Breaking change

Checklist

Checked instead: every § *…* cross-reference in CLAUDE.md resolves to a real heading, the Dependabot figures were read out of .github/dependabot.yml rather than recalled, and the whole file was read against the new rules rather than only the diff — which is what found the three defects above.

macos-latest may be red as on any pull request; #92 records the population, and this diff is markdown only.

🤖 Generated with Claude Code

https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj


Generated by Claude Code

…e agreement

Three rules the last two pull requests earned, plus the corrections that
writing them turned up.

**"FERTIG — mergebar" is what says a branch is finished.** A status message
naming the new head is not: 9b5248f and 9f172d6 were merged past on #104 and
8ebced1 on #107, all pushed shortly before a merge and all announced. The gap
is between the push and the merge, so promptness cannot close it - only a state
that says no further push is coming. Reconciled with the two rules it touches:
a leg red for a failure the branch did not cause does not block the signal but
must be named alongside it, and a finding arriving after the signal still gets
its thread answer - what waits is the push, which becomes the maintainer's
decision rather than a fix racing their merge.

**A figure belongs in one place; everywhere else points at it.** Keeping a
restated count in sync failed three times during the review of #109, twice in
the correction of the previous failure, because both places stay internally
consistent while one goes stale. Bounded to living text: a dated retro states
its figures as of its date, a rules file or a description is read as current.

**A claim about where something came from is checkable, so check it.** Writing
provenance from memory put two wrong entries into one three-item list on #109 -
a finding credited to a review that had not made it, and a pair of sentences
offered as a contradiction that the commit resolving them had explicitly
classified as not one.

Applying the first two to this file found three defects in it, all fixed here:
the Dependabot paragraph restated per-ecosystem limits from the config and
omitted github-actions entirely; the merge-queue aside restated a run count
that the moment #106 lands would be wrong; and the clock rule restated a test
count from #92 that is already out of date. All three now point at their
source. Closes the retro's open item for this convention, which this change
makes stale.

Markdown only; no code, project or workflow file touched, so no build, test or
format result is claimed.

Refs #104, #106, #107, #109.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj
@cursor

cursor Bot commented Sep 13, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Markdown-only updates to contributor agreements and a retro; no runtime, API, or CI behavior changes.

Overview
Documents three working-agreement rules in CLAUDE.md that came out of recent merge races and review mistakes, and applies them to fix stale or duplicated claims in the same file.

The largest addition is the „FERTIG — mergebar" procedure: use that phrase only when threads are closed and branch checks are settled; name unrelated red CI legs in the same message; withdraw the signal immediately if a finding or push falsifies the state; after the signal, pushes are maintainer decisions, not silent fixes. PR description edits do not revoke it.

Two verification rules join Before claiming something is true: one canonical place for figures (summaries point at evidence, e.g. #92 for flaky clock tests) and check provenance against git/PR history before citing it.

Housekeeping in the same pass: Dependabot and merge-queue asides no longer restate counts from config or CI; github-actions is included in the Dependabot note. The 2026-09-13 working-practice retro now cites PR timelines for “lost” pre-merge commits, links the merge signal to CLAUDE.md, and drops the open todo that the convention was missing there.

Reviewed by Cursor Bugbot for commit 6484fa4. Bugbot is set up for automated code reviews on this repo. Configure here.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7c4622a4a1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CLAUDE.md Outdated
@codecov

codecov Bot commented Sep 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

The review found the hole, and it is the one that matters: the procedure said
what happens to a finding after the signal - it becomes the maintainer's
decision rather than a push - but not what happens once that push is
authorised and made. The signal would then outlive the head it described, so
the maintainer could merge a commit whose checks and threads had not settled.
That is the original race, re-entered through the procedure written to prevent
it.

So an authorised push now revokes the signal explicitly, is announced as doing
so, and requires a fresh "FERTIG - mergebar" once the new head settles. Editing
the description still does not, and the paragraph now says revoke rather than
break so the two read against the same verb.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 07fe5e0302

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CLAUDE.md Outdated
The review is right that the three cited hashes are poor evidence, though not
for quite the reason given: they do still resolve in a full clone today,
because their branches were never deleted. But none is reachable from `main`
- which is what "merged past" means - so they survive only on those branch
tips and stop resolving the day either branch is cleaned up. That is exactly
the durability the new provenance rule is supposed to buy.

So CLAUDE.md now points at #104 and #107, whose timelines are permanent, and
the provenance rule says which record to cite rather than assuming a commit is
always the durable one: the commit where it is reachable from `main`, the pull
request or issue where it is not. As written it would have mandated the weaker
reference in precisely this case.

The retro keeps the hashes - it is a dated record, which the same rule
exempts - and now says where they live and why, so a reader who cannot resolve
them knows that is the finding rather than a broken citation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 50c7c02b1a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CLAUDE.md Outdated
The review is right, and it found the same hole one step earlier than the last
round did. Revoking only on an authorised push leaves the maintainer holding a
signal that stopped being true the moment a finding landed: the new thread
makes "every thread is closed" false immediately, and they can merge on the
stale signal during the investigation, or after a finding that needed no code
change at all - a case the push-triggered wording never reaches.

So the signal is now a statement about a state, and anything falsifying that
state revokes it when it happens: a finding arriving revokes it before the
investigation starts, a push revokes it, and it is re-issued once threads are
closed and any new head has settled. Editing the description remains the one
exemption, and the paragraph now says why rather than just asserting it - it
changes no thread and no head.

Two rounds on the same rule with the same shape of gap, which is its own
lesson: a rule that names one trigger for a state change has probably missed
the others. The question to ask of the next one is not "what did I forget to
handle" but "what else makes this claim false".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj

Copy link
Copy Markdown
Owner Author

macOS is red here too, and it is not this pull request's

One comment for the whole macOS situation on this branch, per the rule; I will not repeat it per head.

What failed. On 50c7c02, macos-latest was red 613/614 with IsoTpStminTimingTests.Sender_Paces_Consecutive_Frames_According_To_Peer_Stmin — IsoTpTimeoutException: N_Cr timer expired waiting for next Consecutive Frame. Every other check was green, ubuntu-latest and windows-latest included.

Why it is not this branch's. The diff is two markdown files: CLAUDE.md and a document in docs/reviews/, a directory mkdocs.yml excludes from the site. No .cs, no project file, no workflow, no build input. It cannot reach an ISO-TP receive timer.

It is also the oldest entry in the ledger. This test is row 1 of the table in #92, filed on 12 September, and it failed again on #109 earlier today. Nothing about it is new, so there is nothing to add there.

Nothing to port. The prepared fix on fix/periodic-test-sample-count addresses the J1939 single-frame periodic test, not this one. #92 step 2 — moving these assertions onto a virtual clock — is the fix, and it is a suite-wide change that has no business inside a documentation pull request.

No re-run on 50c7c02: it is already superseded by 6484fa4, whose own run is the observation that matters. If macOS is red there too, that is the same finding and still not this branch's.

For the record, the branch's macOS ledger so far is the same pattern #109 showed — a different member of the #92 population each time, over a diff that changes no code.


Generated by Claude Code

@dborgards
dborgards merged commit 8afa92f into main Sep 13, 2026
25 of 27 checks passed
@dborgards
dborgards deleted the docs/merge-ready-signal branch September 13, 2026 14:58
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