Skip to content

docs(multi-repo): gate a template file-sync on a purely additive diff - #161

Merged
CybotTM merged 2 commits into
mainfrom
docs/fleet-sync-additive-gate
Aug 22, 2026
Merged

docs(multi-repo): gate a template file-sync on a purely additive diff#161
CybotTM merged 2 commits into
mainfrom
docs/fleet-sync-additive-gate

Conversation

@CybotTM

@CybotTM CybotTM commented Aug 22, 2026

Copy link
Copy Markdown
Member

Writing a shared template file over each consumer destroys repo-specific content silently. Nothing errors, nothing is reported, and the loss surfaces later when someone misses what used to be there. This adds a per-repo gate to the dry-run rule: apply where the diff only adds, report the rest for a human.

The two details that are load-bearing

Both are in the text because the first version of this change got them wrong and an adversarial pass caught it before the PR opened.

Strip the header; do not exclude a second character. grep -cE '^-[^-]' looks right — it skips the --- file header — but it also skips every removed blank line and every removed line whose own first character is -. That is exactly what a YAML list item at column 0 looks like:

cur='on:\n- push\n- local\n'  vs  tpl='on:\n- push\n'
  grep -cE '^-[^-]'        → +0/-0     ← "already current", never flagged
  tail -n +3 | grep -c '^-' → +0/-1     ← MANUAL, as intended

Both counts landing at zero puts the repo in the first branch, so it is reported as already synced — the gate failing in precisely the case it exists for.

Read diff's exit status before trusting the counts. With the file absent from the consumer, diff exits 2 with empty stdout, both counts are 0, and || true swallows the failure. The repositories missing the shared file are then the ones reported as synced. || true is still required on the grep -c — no matches exits 1, fatal under set -e, and under set -eo pipefail even the ordinary differing-files case aborts because diff returns 1 — it simply must not be the only status handling.

Two things the gate must not skip

Declared exemptions are read as a key, not grepped: intentional-drift: lives in .github/template.yaml, and those manifests also mention managed files in prose. One such comment reads "byte-identical, drift-enforced checks.yml" while the key beside it lists other files entirely — a grep for the filename matches the comment and returns the opposite of the truth.

And an implausible classification rate is a broken query. A dry run reporting that most of a fleet has deliberately opted out of one shared file is not a state of the world; read one matched item in full before believing the aggregate.

Scope of the observation

The 21-repository sweep cited in the section ran the short pattern, so "the other twenty removed nothing" is established only for removals that pattern can see. The text says so rather than presenting the hit rate as evidence the gate works.

Assisted by claude-code:claude-opus-5 — Session

Writing a shared file over each consumer destroys repo-specific content
silently. Nothing errors and nothing is reported; the loss surfaces when someone
misses what used to be there. The sweep should decide per repo, and a removal
should stop it rather than be applied.

Two details in the snippet are load-bearing, and both obvious shorter forms are
wrong. They are in the text because the first version of this change got them
wrong and an adversarial pass caught it:

`grep -cE '^-[^-]'` looks correct — it skips the `--- file` header — but it also
skips every removed BLANK line and every removed line whose own first character
is `-`, which is what a YAML list item at column 0 looks like. Both then count
as zero removals and the repo lands in the FIRST branch: reported as already
current, never flagged, never opened. Measured: cur='on:\n- push\n- local\n'
against tpl='on:\n- push\n' scores +0/-0 under the short form and +0/-1 under
`tail -n +3 | grep -c '^-'`.

And `diff`'s exit status has to be read before the counts are trusted. With the
file absent from the consumer, diff exits 2 with empty stdout, both counts are
0, and `|| true` swallows it — so the repositories MISSING the shared file are
exactly the ones reported as synced. `|| true` is still needed on `grep -c`
(no matches exits 1, fatal under set -e); it just must not be the only status
handling.

The section also carries the two things the gate must not skip: read
`intentional-drift:` as a key rather than grepping the manifest, since those
manifests mention managed files in prose and one such comment reads
"byte-identical, drift-enforced checks.yml" while the key lists other files
entirely; and distrust an implausible classification rate, because a dry run
claiming most of a fleet has opted out of one shared file is a broken query.

The 21-repository observation is scoped to what was actually measured: that
sweep ran the short pattern, so "the other twenty removed nothing" holds only
for removals that pattern can see.

Assisted-by: claude-code:claude-opus-5
Agent-Session: https://claude.ai/code/session_0144iD1P22LotW8rxmxrNGro
Agent-Host: 0493f0
Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
@github-actions github-actions Bot added documentation Improvements or additions to documentation skill labels Aug 22, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@CybotTM
CybotTM marked this pull request as ready for review August 22, 2026 16:31
Copilot AI lite review requested due to automatic review settings August 22, 2026 16:31

Copilot AI 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.

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

The gate uses an early `return`, which needs a function. Shown as a bare block
it breaks when pasted into a loop, where `continue` is the right word. Wrapped
as `sync_one()` with the two inputs named, and the alternative stated.

Verified by running the published snippet verbatim against five fixtures:
removed blank lines and a removed YAML list item both reach MANUAL (+0/-2 and
+0/-1), a purely additive diff reaches the apply branch, identical files report
already current, and an absent consumer file reports CANNOT COMPARE.

Assisted-by: claude-code:claude-opus-5
Agent-Session: https://claude.ai/code/session_0144iD1P22LotW8rxmxrNGro
Agent-Host: 0493f0
Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
@sonarqubecloud

Copy link
Copy Markdown

@CybotTM

CybotTM commented Aug 22, 2026

Copy link
Copy Markdown
Member Author

The bot review was unavailable: Copilot is out of quota, account-wide and monthly. The one entry on this PR sits on an older commit and is an error body; there is no approval on 4b8ab52f. What follows is what I checked.

The snippet was executed verbatim as published, wrapped exactly as the file now shows it, against five fixtures:

removed blank lines          → probe: MANUAL +0/-2
removed YAML list item       → probe: MANUAL +0/-1
purely additive              → probe: WOULD SYNC +1/-0
identical                    → probe: already current
consumer file absent         → probe: CANNOT COMPARE (file missing?)

The first two are the cases the original version of this snippet got wrong: grep -cE '^-[^-]' scored both +0/-0 and put them in the "already current" branch — a gate that silently reports unsynced repositories as done. That is why those two bullets are in the text rather than only the corrected code.

The return was a real defect in the published example, caught on re-reading rather than by the verification pass: it needs a function, and a paste into a bare loop breaks. Wrapped as sync_one(), with continue named as the alternative.

Scope stated rather than glossed: the 21-repository sweep cited in the section ran the short pattern, so "the other twenty removed nothing" holds only for removals that pattern can see. The earlier draft presented that hit rate as evidence the gate worked; it isn't, and an editorializing sentence to that effect was removed.

Verified independently: intentional-drift: is a real key in .github/template.yaml, and the prose trap is literal — t3x-nr-textdb's manifest contains the comment "byte-identical, drift-enforced checks.yml" while its intentional-drift: lists ci.yml and release.yml only.

Repo gates: validate-skill.sh reports 0 errors, warning count unchanged from baseline. Fence parity checked.

Assisted by claude-code:claude-opus-5 — Session

@CybotTM

CybotTM commented Aug 22, 2026

Copy link
Copy Markdown
Member Author

Self-review: 4b8ab52

The review this pull request demands is unsatisfiable (Copilot quota wall or repeated bot failures on this head). Per the documented fallback, the diff on this head was reviewed by the PR author; this comment is the on-the-record attestation the merge gate reads back. It stops matching on the next push.

@CybotTM
CybotTM merged commit cdc6fe0 into main Aug 22, 2026
20 checks passed
@CybotTM
CybotTM deleted the docs/fleet-sync-additive-gate branch August 22, 2026 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation skill

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants