Skip to content

fix(standards-sync): a create failure disarms the writes, it does not truncate the audit (backend#2735) - #366

Closed
LukasWodka wants to merge 2 commits into
developfrom
fix/2735-disarm-not-truncate
Closed

fix(standards-sync): a create failure disarms the writes, it does not truncate the audit (backend#2735)#366
LukasWodka wants to merge 2 commits into
developfrom
fix/2735-disarm-not-truncate

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

What

standards-sync.py's fleet loop breaks when pr create fails, so the report covers only the repos before the failure. This makes it disarm remediation and keep classifying instead — matching what the pre-flight identity refusal already does.

Closes tracebloc/backend#2735

Why it is real, not a style point

Two paths reach one condition — this credential cannot open PRs:

where it is foundwritesaudit
pre-flight check_author_identitydisarmedcompletes (#348)
mid-loop pr createstoppedstopped too ← the bug

Which one fires depends only on when the fact becomes knowable, and check_author_identity cannot know it: a token with the wrong fine-grained scopes, or one never SSO-authorized for the org, passes every read-only check and fails at pr create. The selftest says so itself at the _ensure_pr case. So the truncating path is the reachable one, and the fleet report silently depended on timing.

standards-sync.yml argues against exactly this twenty lines above the secret: aborting before the audit turns "PRs could not be opened" into "fleet state unknown", which is strictly less information.

What is deliberately preserved

The writes still stop dead at the first failure. remediate() cuts the branch ref (POST git/refs) before it calls pr create, so continuing to remediate really would leave a branch on every drifted repo and a PR on none — the half-rollout the main() gate exists to prevent. The bound the selftest pins, at most one repo with a branch and no PR, still holds; a new check asserts remediate is called exactly once.

The row text also had to stop assuming the pre-flight cause — NOT REMEDIATED: … refused is wrong wording for a create failure, so both causes now name themselves.

Evidence

  • standards-sync-selftest.py: 66 checks, 0 failed (60 before, 6 added).
  • Mutation-proved: restoring the break reddens exactly the two audit-completeness checks, and nothing else.
  • Non-vacuous twin: the same 3-repo fleet with a working credential remediates all three and exits 0 — so the new checks cannot pass over a script that merely stopped remediating.
  • Added to standards-sync-mutations.py; full run 12 mutations, 0 stale, 0 malformed, 0 uncaught.

The new checks were inserted above the summary computation — that file documents its own past defect where checks appended below it printed FAIL and did not count.


Note

Medium Risk
Changes remediation control flow and CI exit/report semantics for org-wide sync runs when the author PAT fails at PR creation—high operational impact but bounded writes and stronger audit completeness.

Overview
When pr create fails with AuthorUnusable during --create-prs, standards-sync.py no longer breaks the fleet loop. It now matches pre-flight PAT refusal (#348): turn off further remediation, continue classifying every target, and exit 2 with a full table.

Behavior preserved: only the first repo still gets a remediate() attempt (avoids branches on every repo with no PRs). Row text and report footer distinguish pre-flight “refused” vs mid-loop “cannot open PRs” via remediation_off; the footer says REMEDIATION DISARMED instead of ABORTED, clarifying the audit covered the whole fleet.

Tests:standards-sync-selftest.py adds a 3-repo scenario (audit all targets, one remediate call, correct later-row wording). standards-sync-mutations.py pins restoring break as a regression.

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

… truncate the audit (backend#2735)
Bugbot (High) on the staging->main promotion, .github#363.
Two paths reach one condition -- "this credential cannot open PRs". The
pre-flight identity refusal disarms remediation and lets the read-only
audit finish (#348). The mid-loop `pr create` failure used to `break` the
fleet loop, so the table covered only the repos before the failure.
Which path fires depends only on WHEN the fact becomes knowable, and
check_author_identity cannot know it: a token with the wrong fine-grained
scopes, or one never SSO-authorized, passes every read-only check and
fails at `pr create`. So the truncating path was the reachable one.
The writes still stop dead -- remediate() cuts the branch ref before it
calls `pr create`, so continuing to remediate really would leave a branch
on every drifted repo and a PR on none. Only the classification continues.
Six selftest checks, incl. a non-vacuous twin, plus a mutation entry that
restores the `break`. Verified: 66 checks 0 failed; under the restored
`break` exactly the two audit-completeness checks redden.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodkaLukasWodka self-assigned this Aug 27, 2026
Comment threadscripts/tests/standards-sync-selftest.py Outdated
…d#2735)
Bugbot on .github#366. The check searched the whole report for "cannot
open PRs", a phrase the failed row and the DISARMED footer both already
carry -- so every later row could still say the pre-flight "refused" and
the check would pass. It could not fail for its own reason.
Now asserted on the bravo/charlie rows themselves, and that they do NOT
say "refused".
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

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

@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 a95d357. Configure here.

@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

Closing as the duplicate — .github#368 carries this fix and is the one to merge.

Both PRs were written in parallel against the same defect from two tickets (backend#2735 here, backend#2690 there — the same Bugbot finding recorded at the prod and staging hops). Verified before closing that #368 is not missing anything this PR had:

#368
breakremediating = False + continue✅ (standards-sync.py:684,690)
tracking variable renamedhalted_at
footer rewritten**REMEDIATION HALTED at …**
later rows name the create-failure cause, not the pre-flight refused✅ (cause = "cannot open PRs …" if halted_at)
row-scoped (non-vacuous) assertion
selftest + mutation entries✅ 13 mutations, 0 uncaught
checks✅ 18 green, 0 failed, 0 unresolved threads

Chose this direction rather than the reverse because an AI session may only close a PR it opened itself in the same session, and #368 came from a parallel one. On merit the two are equivalent.

backend#2735 is being closed as a duplicate of backend#2690.

One thing that must not be lost with this PR: the Bugbot thread on the promotion .github#363 is deliberately left unresolved and is still blocking the staging → main hop. Merging #368 puts the fix on develop — it does not clear that thread. The hop stays blocked until the fix reaches main through the train (.github#365 develop → staging first, then a fresh staging → main), or until someone resolves the thread on #363 knowing the fix is inbound.

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