Skip to content

fix(standards-sync): a PR refusal stops the writes, not the audit (backend#2690) - #368

Open
LukasWodka wants to merge 2 commits into
developfrom
fix/2690-audit-survives-write-refusal
Open

fix(standards-sync): a PR refusal stops the writes, not the audit (backend#2690)#368
LukasWodka wants to merge 2 commits into
developfrom
fix/2690-audit-survives-write-refusal

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

This unblocks the prod promotion. The same defect was re-found at the staging→main hop as a High (backend#2735, thread on #363) and is currently stopping that promotion; its thread is deliberately left unresolved until this lands. One finding, two cards — Medium at the develop→staging hop, High at the prod hop — so this PR closes both.

Summary

remediate() raising AuthorUnusable used to break the target loop. A credential GitHub stopped taking on repo 1 of 16 therefore left the report covering a prefix of the fleet — while the summary line went on printing {len(targets)} targets and the drift counts described only the repos the sweep had reached. A mechanism reporting a result it never established.

Stopping the writes is correct and stays. #348's reasoning holds: the same credential opens every one of these, so carrying on remediating would push a branch to all of them and open a PR on none.

Stopping the audit was never correct. Classification is a read that needs no credential, and standards-sync.yml argues exactly this twenty lines above the secret — aborting before the audit "would turn 'PRs could not be opened' into 'fleet state unknown' — strictly less information". The author_refusal path already obeyed that; this path contradicted it.

So the refusal now sets remediating = False and carries on classifying:

  • every target gets a row;
  • repos after the halt read NOT REMEDIATED, and say which of the two causes applies (refused up-front vs. cannot open PRs mid-run);
  • the footer says REMEDIATION HALTED at betaand that the audit covers all N targets, so "halted" and "audited" can be told apart.

Test plan

scripts/tests/standards-sync-selftest.py60 → 67 checks, 0 failed._run_main gained targets= / remediate= parameters because this is a multi-repo property: a one-repo fleet has no "rest of the fleet" for the abort to have eaten.

New checks assert the finding per repo (not by counting rows, so a report that lost gamma cannot pass by gaining a footer line), that gamma is never attempted, the footer wording, exit 2, and non-vacuity (an unrefused three-repo fleet still gets three PRs and is green).

scripts/tests/standards-sync-mutations.py13 mutations, 0 stale, 0 malformed, 0 uncaught. Two new ones, not one, because the two properties are independent and a single mutation cannot prove both:

mutationcaught by
restores the breakmain: a mid-fleet PR refusal still audits EVERY target
keeps the audit but never disarms remediationmain: the refusal disarms remediation for the REST of the fleet

Both mutations edit the real scripts/standards-sync.py and re-run the real suite — no rule is re-implemented in the harness.

Identity of the two cards

Verified rather than assumed — they are the same finding, not two:

backend#2690backend#2735
severity / hopMedium, develop→staging (.github#341)High, staging→main (.github#363)
path / linescripts/standards-sync.py L651-665scripts/standards-sync.py L651-665
findingthe break on AuthorUnusable aborts the remaining auditthe break on AuthorUnusable aborts the remaining classification

The High-hop description names the same asymmetry this PR removes: "The identity-refusal path in the same function already disarms writes and keeps the read-only audit; this path stops both."

Closes tracebloc/backend#2690
Closes tracebloc/backend#2735

🤖 Generated with Claude Code


Note

Medium Risk
Touches org-wide standards-sync remediation and reporting in CI; behavior change is intentional (complete audit on credential failure) while preserving the no-half-rollout write gate.

Overview
When --create-prs remediation hits AuthorUnusable (PAT resolves but GitHub refuses pr create), the sync used to break out of the target loop. That stopped classification for the rest of the fleet while the summary still claimed all len(targets) repos were audited—a partial sweep dressed as a full one.

Remediation still stops fleet-wide (#348): remediating is turned off and no further branches are pushed after the failure repo. The change is that the read-only audit keeps going—same principle as the upfront author_refusal path and what standards-sync.yml argues for.

Reporting is tightened: halted_at replaces aborted_after; drifted rows after a halt say NOT REMEDIATED with whether the cause was upfront refusal vs mid-run PR failure; the footer uses REMEDIATION HALTED and states the audit still covers all targets.

Tests extend _run_main for multi-repo fleets and add two mutation cases so “audit continues” and “remediation disarms” stay independently pinned.

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

…ckend#2690)
`remediate()` raising `AuthorUnusable` used to `break` the target loop, so a
credential that GitHub stopped taking on repo 1 of 16 left the report covering
a PREFIX of the fleet -- while the summary line went on printing
`{len(targets)} targets` and the drift counts described only the repos it had
reached. A mechanism reporting a result it never established.
Stopping the WRITES is correct and stays (#348: the same credential opens every
one of these, so carrying on would push a branch to all of them and open a PR
on none). Stopping the AUDIT was never correct -- classification is a read that
needs no credential, and `standards-sync.yml` argues exactly this twenty lines
above the secret: aborting before the audit "would turn 'PRs could not be
opened' into 'fleet state unknown' -- strictly less information". The
`author_refusal` path already obeyed that. This path contradicted it.
So the refusal now sets `remediating = False` and carries on classifying: every
target gets a row, repos after the halt read NOT REMEDIATED, and the footer says
which of the two things happened -- remediation halted at `beta`, audit complete
for all N.
Six selftest checks (60 -> 67), and two mutations rather than one: the properties
"the audit continues" and "no further branch is pushed" are independent, and a
single mutation cannot prove both.
Closestracebloc/backend#2690
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodkaLukasWodka self-assigned this Aug 27, 2026
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

⚠️ Duplicate: this and .github#368 are the same fix

Two parallel sessions landed on the same defect from two different tickets, five minutes apart:

PRclosesticket framing
#366backend#2735Bugbot High, blocking the prod hop (.github#363, staging → main)
#368backend#2690Bugbot Medium/Low, shipped at the staging hop

backend#2690 and backend#2735 are the same Bugbot finding recorded at two hops of the same promotion chain, so the two tickets are duplicates of each other as well.

Both PRs touch the identical three files and make the identical change:

They will conflict. Whichever merges second gets a conflict in standards-sync.py and in both test files.

Recommendation

Merge one, close the other, and close the losing ticket as a duplicate. I have no strong preference on which — they are equivalent in behaviour. Marginal differences:

I am not closing either — the org rule is that an AI session does not close PRs without an explicit instruction, and this one is LukasWodka's on both sides. @LukasWodka / @saadqbal, one call closes it out.

Comment threadscripts/tests/standards-sync-selftest.py Outdated
…nd#2690)
Bugbot on .github#368. The footer this change adds contains the words
"NOT REMEDIATED", so searching the whole report was satisfied before any
row was consulted -- gamma could be a bare drifted classification and the
check still passed. The missing-repo check beside it already matches
`| {r} |` per target; this one now does too.
Co-Authored-By: Claude Opus 4.8 <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 128956e. Configure here.

@LukasWodka
LukasWodka requested review from saqlainsyed007 and removed request for saadqbalAugust 28, 2026 06:13
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