Skip to content

fix(caller-drift): skip a remediation write whose content is already the canon - #240

Merged
LukasWodka merged 2 commits into
developfrom
fix/1867-skip-identical-remediation-write
Aug 13, 2026
Merged

fix(caller-drift): skip a remediation write whose content is already the canon#240
LukasWodka merged 2 commits into
developfrom
fix/1867-skip-identical-remediation-write

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Bugbot on the staging promotion PR #238. Fixed on develop so the train re-prepares, not pushed onto the promotion PR.

The bug

remediate_copies read each copy's blob sha and then always PUT, never comparing what the branch already carried against the canonical bytes.

So a second --create-prs dispatch re-writes identical content. The API answers 409 (or loses a sha race), remediation records a failure, and the org audit goes red as if the fleet could not be written — while the open PR already carries the canon. The run reports a fleet-wide write failure that never happened.

standards-sync.py already does the right thing:

ifcurrentisnotNoneandcurrent==desired:
return_ensure_pr(full, head, base, issue) # content already pushed; just ensure the PR

That skip is what makes its 422 branch reuse genuinely idempotent. This is the same fix in the same shape.

What changed

  • _read_copy_on_head returns (sha, decoded bytes, error) instead of just the sha, so the caller can distinguish an already-correct copy from a drifted one. Same signature shape as standards-sync's _read_head_file, and for the same reason.
  • The loop skips the PUT when the branch content equals the canon, and still falls through to _ensure_copy_pr — otherwise the first run's work would never become reviewable.
  • import binascii, because a malformed blob now decodes here and b64decode raises binascii.Error, not ValueError alone. Without it the error path would NameError exactly when it was needed.

The selftest was modelling the old call shape

Both remediation stubs returned a bare "existingsha\n" — correct for the old --jq .sha call, but not what the API sends. That is why this skip was untestable: there was no content to compare.

The stubs now return {sha, content} with the branch's current body as a parameter, so a test can say what the branch already carries. New cases:

  • a copy already matching the canon is not re-written
  • the PR is still ensured when every write was skipped

Test plan

  • python3 scripts/tests/caller-drift-selftest.py162 pass / 0 fail (was 160)
  • ruff check --isolated --select E4,E7,E9,F → clean
  • Negative test: removing the skip fails remediation: a copy already matching the canon is not re-written

Note

Low Risk
Scoped to copy remediation in caller-drift.py with expanded selftests; behavior change is skipping no-op writes, reducing API failure risk on re-dispatch.

Overview
Fixes false remediation failures on a second --create-prs dispatch: remediate_copies used to read only the blob sha and always PUT, so re-running against a branch that already had the canonical workflow triggered 409/sha races and marked the fleet as unwritable even when the open PR was already correct.

_read_copy_on_head now returns (sha, decoded bytes, error) from the full Contents API (aligned with standards-sync.py), with binascii handling bad base64 on the error path.

remediate_copiesskips the PUT when decoded branch content equals the canonical bytes and still calls _ensure_copy_pr so the PR remains open/reviewable.

Selftest stubs return real {sha, content} payloads and add cases for no re-write when content matches and PR ensured when all writes are skipped.

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

…the canon
Bugbot on the staging promotion PR #238, fixed on develop so the train
re-prepares rather than pushing onto the promotion PR.
`remediate_copies` read each copy's blob SHA and then always PUT, never comparing
what the branch already carried against the canonical bytes. A second
`--create-prs` dispatch therefore re-writes identical content, which the API
answers with 409 (or loses a sha race), and remediation records a failure -- the
audit goes red as if the fleet could not be written, while the open PR already
carries the canon. standards-sync.py skips the write in exactly this case
(`if current is not None and current == desired`), which is what makes its 422
branch reuse genuinely idempotent; this is the same fix in the same shape.
- `_read_copy_on_head` now returns (sha, decoded bytes, error) instead of just
the sha, so the caller can tell an already-correct copy from a drifted one.
- the loop skips the PUT when the branch content equals the canon, and still
falls through to `_ensure_copy_pr` -- otherwise the first run's work would
never become reviewable.
- `import binascii`, since a malformed blob now decodes here and b64decode
raises binascii.Error rather than ValueError alone.
The selftest stubs returned a bare `"existingsha\n"`, modelling the old
`--jq .sha` call, so the read now JSON-decodes what the real API sends. Both
remediation stubs return `{sha, content}` and take the branch's current body,
which is what made the skip testable at all. New cases: an already-matching copy
is not re-written, and the PR is still ensured when every write was skipped.
Selftest 162 pass / 0 fail (was 160); ruff clean. Removing the skip fails the
new case.
@LukasWodkaLukasWodka self-assigned this Aug 13, 2026
LukasWodka added a commit that referenced this pull request Aug 13, 2026
…lls exemption (#241)
* fix(inventory): drop archived devex-bootstrap, unstale the claude-skills exemption
The caller-drift audit is red on develop with 2 findings, and it is a REQUIRED
check (`gate`) for any PR touching a contract file — so every PR that edits
caller-drift.py, its selftest, repo-inventory.yml or the two workflows inherits
an unrelated red and cannot merge. That is what blocks #240.
Neither finding is about the code:
- devex-bootstrap is in repo-inventory.yml but is no longer an active repo.
Verified: `archived=true`. Its entry existed to hold open the disposition
question in backend#1597 ("joins the train, stays a scratch repo, or is
archived") — that question now has an answer, so the whole 86-line block and
its exemptions go with it. Confirmed with Lukas.
- claude-skills marked `.cursor/BUGBOT.md` exempt via *bugbot_guide_missing, but
the file exists (8,949 bytes). The exemption outlived its reason, which is the
failure mode the audit's stale-exemption check exists to catch: an exemption
nobody revisits reads as "not required here" forever.
Kept deliberately separate from #240's caller-drift.py fix: this is inventory
data, that is script behaviour, and this one unblocks every contract-touching PR
rather than just mine.
* fix: remove every devex-bootstrap remnant, not just the inventory entry
All three Bugbot findings on this PR, and all three the same mistake: I deleted
the entry and left its references.
HIGH -- scripts/standards-sync.py kept an EXEMPT entry naming devex-bootstrap.
Design rule 3 in that file makes a stale exemption a HARD FAILURE ("an exemption
naming a repo the inventory does not know is itself a failure"), so `load_targets`
would have refused and taken the whole scheduled org-standards audit down --
every repo unevaluated, not one repo skipped. EXEMPT is now empty, with the
reason recorded: backend#1597 item 3 was answered by archiving the repo.
Its selftest named devex-bootstrap as the exemplar exempt repo, so emptying
EXEMPT broke two cases. Rewritten to inject their own fixture entry and restore
the real one in a finally: they test the MECHANISM, and a mechanism test should
not break every time the fleet changes -- which is exactly what just happened.
MEDIUM -- the `bugbot_guide_missing` anchor's prose still cited claude-skills
alongside rfcs, after claude-skills stopped aliasing it. Hand-maintained citation
lists rot the moment usage moves; now only rfcs, which is the only alias left.
LOW -- the `devex_bootstrap_undisposed` anchor definition survived with zero
aliases, still asserting the disposition was open. Removed, along with
`no_workflows_directory_at_all` (also zero aliases after the entry went) and the
dangling "Distinct from devex_bootstrap_undisposed" cross-reference it left
behind in the bugbot anchor.
Zero `devex` references remain in the repo.
standards-sync selftest 27 checks / 0 failed; caller-drift selftest 160 / 0;
ruff clean; YAML parses at 19 repos.
@LukasWodka
LukasWodka merged commit c66c246 into developAug 13, 2026
11 checks passed
@LukasWodka
LukasWodka deleted the fix/1867-skip-identical-remediation-write 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.

1 participant

@LukasWodka