Skip to content

fix(standards): retry fresh-branch reads; refresh sha on rejected writes (backend#1602) - #197

Merged
saadqbal merged 4 commits into
developfrom
fix/1602-sync-fresh-branch-race
Aug 11, 2026
Merged

fix(standards): retry fresh-branch reads; refresh sha on rejected writes (backend#1602)#197
saadqbal merged 4 commits into
developfrom
fix/1602-sync-fresh-branch-race

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Hardens scripts/standards-sync.py against GitHub's fresh-ref consistency window (backend#1602 follow-up, found live on run 31373298821): remediate() created a branch and immediately read contents/CLAUDE.md?ref=<head>; the read transiently 404'd, the 404 was believed, and the sha-less PUT against an existing path failed with "Invalid request" — one flaky read turned a fleet run red (an immediate re-dispatch succeeded).

Fix, per the script's design rules (new rule 5 in the header):

  • _read_head_file() — the caller now tells it whether the base branch has the file. If the base proves the file exists, a 404 on the fresh ref cannot be true yet: retry with backoff (5 attempts) and fail closed if it never appears — never conclude "no file" and send a sha-less write. Genuine absence (base has no file) is still confirmed by one re-read rather than trusted from a single answer.
  • _write_head_file() — a PUT rejected with 409/422 refreshes the sha from the ref and retries exactly once; a second rejection is a real conflict and fails closed.
  • remediate() decomposed to use both; behavior otherwise unchanged (same commit message, same PR ensure).

Type

Bug fix (CI tooling).

Test plan

  • Offline selftest extended with five race checks using a scripted gh stub (no network): transient-404-retried-to-success, persistent-404-fails-closed-after-all-attempts, genuine-absence-confirmed-by-re-read, rejected-write-refreshes-sha-and-retries-once (asserts the retried PUT carries the refreshed sha), second-rejection-fails-closed. 25/25 locally; runs as the selftest job on this PR.
  • python -m py_compile on both files.
  • Organic: next create-prs dispatch exercises the path fleet-wide.

Part of tracebloc/backend#1602.

🤖 Generated with Claude Code


Note

Medium Risk
Changes fleet-wide org-standards remediation and GitHub API write behavior; mistakes could block sync PRs or mis-handle file creation, but scope is CI tooling with strong offline tests and fail-closed semantics.

Overview
Hardens standards-sync.py remediation against GitHub’s fresh-ref consistency window (design rule 5): transient 404s on a just-created sync branch no longer lead to sha-less PUTs that fail, and rejected writes get one sha-refresh retry before failing closed.

remediate() is split into _read_head_file() (backoff retries when the base has CLAUDE.md and the branch is fresh; one re-read when absence is plausible) and _write_head_file() (single retry after 409/422 with an updated sha, plus an explicit “racing this branch” error on a second rejection). expect_file is now file_on_base and branch_is_fresh so reused sync branches that legitimately lack CLAUDE.md can still get a sha-less create instead of retrying into a permanent failure (#197).

Adds .gitignore for Python bytecode and local noise, and extends the offline selftest with scripted gh cases for read/write races and reused-branch read counts.

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

…tes (backend#1602)
Design rule 5: fresh refs are eventually consistent. On run 31373298821
(frontend-app leg) the contents read on a just-created branch transiently
404'd, the 404 was believed, and the sha-less PUT was rejected as
'Invalid request' — one flaky read failed a fleet run. remediate() now
knows whether the base has the file: reads the base proves must succeed
retry with backoff and fail closed if they never do; genuine absence is
confirmed by a re-read; a 409/422-rejected write refreshes the sha and
retries exactly once. Five new offline selftest checks pin the contract
with a scripted gh stub (25 checks total).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LukasWodkaLukasWodka self-assigned this Aug 10, 2026
Comment threadscripts/standards-sync.py
Comment threadscripts/standards-sync.py Outdated
LukasWodkaand others added 2 commits August 10, 2026 17:31
…a real conflict
Two Bugbot findings on #197.
1. `expect_file` MEANS "a 404 here cannot be true" -- which holds only when the
ref was cut moments ago from a base that has the file. That is the eventual
consistency window the retry exists for.
A REUSED branch (the 422 path) is a different situation: it may have been cut
BEFORE CLAUDE.md existed on the base, so a 404 is honest and permanent.
Passing file_on_base alone made the read retry five times and fail closed, so
the sha-less create could never run and that repo was stuck forever. The old
code read the 404 as absence and created the file, which was right here.
expect_file is now `file_on_base and branch_is_fresh`.
2. The second-rejection message was UNREACHABLE. On attempt 2 the flow fell
through to the generic return inside the loop, so the one failure worth
distinguishing -- refreshed the sha and was rejected anyway, i.e. another
writer is racing the branch -- was the one nobody could see. It now returns
its own message, and the post-loop return is documented as a fail-closed
backstop rather than dead code that would return None (i.e. SUCCESS).
Selftest 25 -> 27. Mutation-verified both: reverting expect_file, and making the
second-rejection branch unreachable again -- both CAUGHT.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…oved nothing
Mutation-checking the previous commit caught its own test. Reverting the
expect_file fix left the selftest GREEN, because the check asserted only "the
file got created".
With the bug present the read retries, swallows the scripted PUT response, reads
its `{}` body as the file, and the run reaches the same end state by a different
path. Same outcome, wrong reason -- so the check could not distinguish fixed from
broken, in either direction.
Assert the READ COUNT instead: 2 when a 404 on a reused branch is believed as
absence, 5 when it is wrongly treated as the eventual consistency window. That is
the thing the fix actually changes. Also catches the stub's exhaustion assertion
and records it as a failure rather than letting it abort the whole selftest.
Re-verified: reverting expect_file is now CAUGHT (reads=3, expected 2).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@cursorcursorBot 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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 3fb6fa7. Configure here.

Same artifact as .github#204, removed on this branch as well. This branch was cut
after #200, so it carries the file; without this, merging #197 after #204 would
put it straight back and the fix would look like it had held.
The .gitignore is identical to #204's, so whichever merges first the other
resolves cleanly.
Selftest: 27 checks, 0 failed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@cursorcursorBot 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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit f877494. Configure here.

LukasWodka added a commit that referenced this pull request Aug 10, 2026
`scripts/__pycache__/standards-sync.cpython-314.pyc` was committed by a
`git add -A` in #200 and reached `develop`. It is now riding the
develop -> staging promotion in .github#203, which is how I noticed it.
It is a build artifact and does not belong in the tree: version- and
platform-specific (cpython-314, while CI runs 3.12), regenerated on every local
run of the selftest, and a stale one silently shadows the source it was built
from.
Added a .gitignore covering the whole class rather than the one file that got
caught -- `__pycache__/`, `*.py[cod]`, plus .DS_Store and .pytest_cache, which
the same careless `add -A` would sweep in next.
The file is also present on the #197 branch, which branched after #200; removed
there too, so whichever lands first the artifact is gone and the other cannot
reintroduce it.
Selftest still passes (20 checks on this branch; #197 takes it to 27).
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@saadqbal
saadqbal merged commit 1f753e4 into developAug 11, 2026
11 checks passed
LukasWodka added a commit that referenced this pull request Aug 12, 2026
… failure a read failure
Two Bugbot findings on #227, both real.
1. THE COMMONEST DISPATCH WOULD HAVE FAILED. remediate_copies read the file on a
just-created branch and took a 404 as proof of absence, then issued a sha-less
PUT. For a DRIFTED copy the file exists on the base, so that 404 is the
eventual-consistency window standards-sync.py already pays for -- and the
sha-less write is rejected 422. Fresh branch + drifted file is the single most
likely way this feature is ever invoked, and it would have failed every time.
Ported _read_head_file's contract as _read_copy_on_head: `remediable` now
records, per copy, whether the file exists on the base (drifted yes, missing
no). When the base has it AND the ref was just created, a 404 cannot be true
-- retry with backoff, fail closed if it never appears. When the base does not
have it, one confirming re-read still guards against a single blip.
A REUSED branch is deliberately NOT treated as fresh: it may have been cut
before the file existed on the base, so its 404 is honest and permanent, and
retrying-then-failing-closed would strand that repo forever. That is
standards-sync's own #197, avoided rather than rediscovered.
Found while fixing it: `branch_is_fresh` was referenced and never assigned.
The new test raised it as a NameError rather than a reviewer finding it later.
2. EXIT 2 HAD TWO CAUSES AND ONE VOCABULARY. Remediation failures already exited
2, but every exit-2 message describes unread repos or schema failure, so a
dispatch whose PRs failed to open was headlined on the conformance issue as
"repos that could not be read are NOT known to comply" -- a true sentence
about something that did not happen, sending the reader at the wrong problem.
The count is now its own step output and its own verdict line.
Evidence: selftest 136 pass / 0 fail (from 132). Two mutations: treat every 404
as absence -> 2 red (both retry cases); treat a reused branch as fresh -> 1 red
(the strand-forever case). All four exit-2/1/0 verdict paths rendered and
checked. Pinned ruff 0.15.20 --select E4,E7,E9,F clean, actionlint and shellcheck
clean. Live re-verify against the fleet still selects exactly cli's drifted
stale-backlog.yml, now correctly tagged as present-on-base.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka added a commit that referenced this pull request Aug 12, 2026
…est (backend#1608 items 3+4) (#227)
* feat(conformance): publish the matrix as a standing screen, not only on drift (backend#1608)
Item 3 of the harness: "one screen". The matrix has existed since .github#223,
but nothing published it when the fleet was GREEN — the drift comment fires only
on findings (deliberately: an all-clear per run trains people to ignore the
issue, backend#1344), so a conformant fleet produced a step summary on a run
nobody opens. "Where does the fleet stand?" was still answered by running the
script by hand, which is the thing the ticket set out to remove.
Split the two roles: the issue BODY is the current state, rewritten every
scheduled/manual run; the COMMENTS stay drift-only and remain the history.
The body is rewritten regardless of outcome, which is what makes staleness mean
anything. The audit is weekly, so a timestamp older than ~8 days means the audit
itself stopped and conformance is UNKNOWN — the backend#1530 cron-watchdog
contract. That only holds if a RED run rewrites the body too; otherwise a fleet
that broke in January still shows January's green and merely looks stale-ish.
Hence always(), not gated on exit_code.
Also fixed while here: the drift comment was posting to backend#1415, which is
CLOSED. Comments on a closed issue still post, so it looked like it worked — but
a closed issue cannot be pinned, drops out of default issue views, and notifies
nobody not already subscribed. Drift has been reporting into a drawer. Now
backend#1781, open and pinnable.
The verdict is DERIVED from the counts rather than from the exit code alone: a
clean exit alongside a non-zero unreadable/findings count renders "Inconsistent
result — treat as UNKNOWN". The script does not produce that pair today, which
is why it is worth pinning — "every repo read" printed directly above "3
unreadable" is the most confidently wrong thing this screen could say.
Report heading "Repo conformance drift" -> "Repo conformance": it now renders
under a verdict line that is often "Conformant", and a heading asserting drift
above a green matrix contradicts itself on the one screen people should trust.
Evidence: selftest 123 pass / 0 fail; actionlint and shellcheck clean; the body
rendered against a LIVE fleet audit (20/20 evaluated, 0 unreadable, 0 findings);
all five verdict paths exercised (clean / drift / unevaluable / empty exit code /
inconsistent), and the >60k truncation guard verified to keep the body under
GitHub's 65536 limit — a rejected edit would leave the previous body in place,
which is a stale green presented as current.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(conformance): remediate drifted copies by PR, and only where a PR is honest (backend#1608)
Item 4: the self-healing half. Detection alone still leaves the manual work; it
only makes it visible.
WHAT IT WILL NOT DO, which is most of the design. The ask was "open the
missing-piece PR per repo", but a pull request is the right instrument for
exactly one of the five families:
copies REMEDIABLE. Byte-identical by definition, and the guard
already holds the canonical bytes, so the fix is exact
rather than generated.
callers NOT generated. Caller content is repo-specific — measured
2026-08-12, all EIGHT sampled repos have a different
code-quality-caller.yml because each passes its own
toolchain inputs. A generated caller would be a plausible
file that is wrong for that repo, which is worse than an
absent one that at least reports as a finding.
protection, NOT remediable BY PR at all. These are API settings, not
required_checks, files in the tree; no commit can change them, so a PR
rulesets claiming to fix them would be theatre.
And within copies, only entries marked `required`. `divergent` records a written
reason why a repo differs — cli pins actions/stale@v11 where canon pins v9, and
the newer pin may well be the better one. Silently overwriting a recorded
decision would destroy the judgement the inventory exists to hold. Same for
`exempt`. Both are reported, never rewritten.
Dispatch-only, gated on `github.event_name == 'workflow_dispatch' && inputs
.create-prs == true` — the same expression standards-sync.yml uses, whose
remediation path this mirrors throughout (422-means-reuse, sha-refresh, PR
reuse, actor assignment). Writing to twenty repos is not something a cron may
decide to do, and a PR-triggered audit that wrote to the fleet would be a
supply-chain hole.
Remediation failures go in their OWN list, not `unreadable`: the exit path
derives "caller/copy state UNKNOWN" by subtracting the protection and ruleset
lists from `unreadable`, so a failed WRITE pushed in there is reported as a
failed READ — the wrong diagnosis on the line an operator acts from. They exit
2, because "I tried to fix it and could not" is not the same as "there was
drift".
Landed on the item-3 branch deliberately: it touches the same two files, so a
separate PR would either conflict or be a stacked PR, and the standard forbids
stacking.
Evidence: selftest 132 pass / 0 fail (up from 123). Five mutations, each caught
by its intended case and only that case — enqueue a `divergent` copy (the safety
property, caught structurally via AST rather than grep); return None on a failed
write; treat any branch-create failure as "already exists"; always send `sha=`;
skip the existing-PR check. Against the LIVE fleet: --create-prs reports
"nothing to remediate" and writes nothing, and with cli's genuinely-drifted
stale-backlog.yml flipped to `required` it selects exactly that one file
(blob 4e4246398130 vs canonical 14d689e) — verified with the writer
replaced by a recorder, so nothing was written. shellcheck and actionlint clean.
The workflow header said REPORT-ONLY and no longer is; updated in the same
commit, along with the token scopes that implies.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(selftest): hoist the remediation-test imports to the top (E402)
ruff E402: the new remediation cases imported ast/inspect/textwrap/os mid-file.
Hoisted and the underscore aliases dropped with them.
Verified against the pinned toolchain rather than a local one: ruff 0.15.20 with
--select E4,E7,E9,F, which is what code-quality.yml runs. My first local pass used
a newer ruff with default rules and reported UP037 on pre-existing lines while
missing this — the wrong version answering a different question.
That is backend#1606 in miniature: `.github` has no `make check`, so there is no
local command that runs what CI runs, and the first honest answer arrives red on
a PR. Selftest still 132 pass / 0 fail.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(conformance): survive the fresh-ref 404, and stop calling a write failure a read failure
Two Bugbot findings on #227, both real.
1. THE COMMONEST DISPATCH WOULD HAVE FAILED. remediate_copies read the file on a
just-created branch and took a 404 as proof of absence, then issued a sha-less
PUT. For a DRIFTED copy the file exists on the base, so that 404 is the
eventual-consistency window standards-sync.py already pays for -- and the
sha-less write is rejected 422. Fresh branch + drifted file is the single most
likely way this feature is ever invoked, and it would have failed every time.
Ported _read_head_file's contract as _read_copy_on_head: `remediable` now
records, per copy, whether the file exists on the base (drifted yes, missing
no). When the base has it AND the ref was just created, a 404 cannot be true
-- retry with backoff, fail closed if it never appears. When the base does not
have it, one confirming re-read still guards against a single blip.
A REUSED branch is deliberately NOT treated as fresh: it may have been cut
before the file existed on the base, so its 404 is honest and permanent, and
retrying-then-failing-closed would strand that repo forever. That is
standards-sync's own #197, avoided rather than rediscovered.
Found while fixing it: `branch_is_fresh` was referenced and never assigned.
The new test raised it as a NameError rather than a reviewer finding it later.
2. EXIT 2 HAD TWO CAUSES AND ONE VOCABULARY. Remediation failures already exited
2, but every exit-2 message describes unread repos or schema failure, so a
dispatch whose PRs failed to open was headlined on the conformance issue as
"repos that could not be read are NOT known to comply" -- a true sentence
about something that did not happen, sending the reader at the wrong problem.
The count is now its own step output and its own verdict line.
Evidence: selftest 136 pass / 0 fail (from 132). Two mutations: treat every 404
as absence -> 2 red (both retry cases); treat a reused branch as fresh -> 1 red
(the strand-forever case). All four exit-2/1/0 verdict paths rendered and
checked. Pinned ruff 0.15.20 --select E4,E7,E9,F clean, actionlint and shellcheck
clean. Live re-verify against the fleet still selects exactly cli's drifted
stale-backlog.yml, now correctly tagged as present-on-base.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* chore: drop the runtime scratch file that was committed by accident
`wd-body.md` is written into the workspace by caller-drift.yml's watchdog step at
runtime. It got picked up by a `git add -A` while I was rendering the body locally
to test it. Harmless on a runner, but it would have been a checked-in file that
looks like a report and is stale the moment it lands.
Caught by another session diffing #227's file list against theirs, not by any check
here — .gitignore now covers it so the next local render cannot repeat it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(caller-drift): honest verdict + exit-2 wording for remediation failures (Bugbot #227)
- Remediation-failure verdict no longer claims 'the fleet was read successfully'
when UNREADABLE>0 (both counts can be set together); it names the unreadable
tally so the headline can't contradict the numbers beside it.
- Exit-2 tracking-issue comment and the final fail step now name remediation
failure as a cause of exit 2, not just inventory/read failure, so a failed
create-prs dispatch isn't misdiagnosed as 'repos could not be read'.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(caller-drift): create-prs token needs the workflow scope for .github/workflows/ writes (Bugbot #227)
The callers create-prs writes live under .github/workflows/, which GitHub refuses
to write with Contents:RW alone — it needs the separate 'workflow' scope (classic
PAT) or fine-grained Workflows:write. The note claimed the same scopes as
standards-sync.yml, but that writes CLAUDE.md, not workflow files. Without this
the documented token fails every remediation PUT closed and opens no PR.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(caller-drift): exit-2 verdict names the real cause, not just unread repos (Bugbot #227)
die() exits 2 for a bad inventory or a failed org enumeration too, where
UNREADABLE is 0 — the old code-2 verdict always blamed 'repos that could not be
read', misdirecting the reader. Now the base verdict is cause-agnostic and a
refinement names the actual cause from the UNREADABLE count (read failure vs a
die-path that couldn't run to completion).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka
LukasWodka deleted the fix/1602-sync-fresh-branch-race branch August 14, 2026 13:53
Sign up for freeto 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

@LukasWodka@saadqbal