Uh oh!
There was an error while loading. Please reload this page.
chore(promote): develop → main — fr-pass fix + merge-settings drift guard - #108
Merged
Conversation
Two independent defects made /fr-pass unreliable and, worse, indistinguishable from success. On 2026-07-29, 13 genuine /fr-pass comments produced 5 advances, 8 silent skips and zero reactions. 1. The job-level `if` decided policy. It carried `comment.user.login != issue.user.login` (the D30 self-signoff rule). When the reviewer was also the PR author, the whole job was skipped with zero steps — nothing left to react or explain, and a run that reads `skipped`. All 8 drops were Lukas signing off his own PRs; all 5 advances were someone else's. The rule is unchanged, but it now lives in a step that reports it. 2. The reaction never worked, on any path. It used GITHUB_TOKEN, which is read-only org-wide (`default_workflow_permissions: read`), and a called workflow cannot widen the caller job's token. Every POST returned "Resource not accessible by integration (HTTP 403)" and `|| true` swallowed it — so even the 5 successful advances left no visible mark. Writes now go through PROJECTS_KANBAN_TOKEN (already used cross-repo by wip-limit-check). Every terminating path now leaves a signal: 👍 on advance, 👎 plus a short explanatory comment on anything that is a real problem. The only silent outcome is prose that mentions the string without issuing the command. Real errors also fail the run, so a write that cannot reach the thread still shows up red. Also closed while in here: swallowed GraphQL errors that made an expired token look identical to "not on the board"; a truncated projectItems page reported as absence; duplicate cards silently resolved by `head -1`; author_association used as a permission check (a private org membership reads as CONTRIBUTOR); no detection of a Status-column rename; and a stale header describing the pre-RFC-0008 two-gate flow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugbot, correctly: `permissions: contents: read` pins GITHUB_TOKEN to that one scope and zeroes the rest, so the retry could never post a reaction or a comment. It was unreachable code whose ::warning:: claimed a retry that cannot happen. PROJECTS_KANBAN_TOKEN is the only token here that can write, so the writes are now single-attempt with a loud ::error:: and a failed run — same guarantee, no misleading path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…utofix - actionlint/shellcheck SC2086: the repo list went through word splitting. It is now written to a file and read line-by-line, so a name is never split, and gh reads from /dev/null so it cannot swallow the list. - Exclude forks as well as archives: a fork's merge settings are not ours. - FAIL CLOSED on a non-boolean value (Bugbot). The three merge fields are only returned to a principal with admin on the repo, so a token that can LIST but not read settings gets a successful response with the keys absent -- empty strings then matched neither branch and the repo was silently counted as compliant, recreating the false all-clear the zero-repo guard exists to prevent. - Report-only: the autofix path is gone, so the token needs no standing administration:write over every repo.
The report opened with 'found drift' and always recommended allow_rebase_merge=false, which is wrong for two of the three conditions: on an all-methods-disabled repo that PATCH is a no-op that restores nothing, and on an unreadable repo the problem is the token rather than the setting. Each section now carries only the fix for what it detected, and the unreadable section states plainly that those repos are NOT known to be compliant.
fix(fr-pass): never no-op in silence — job `if` swallowed self-signoffs, reaction was 403ing
feat(ci): weekly merge-settings drift guard
LukasWodka
commented
Jul 30, 2026
ContributorAuthor
👋 Heads-up — Code review queue is at 42 / 30 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
Uh oh!
There was an error while loading. Please reload this page.
LukasWodka added a commit
that referenced
this pull request
Aug 24, 2026
…#314) * ci(2364): a PR whose title names a ticket must link it (backend#2364) Merging a fix never closed or advanced its ticket. `closingIssuesReferences` was 0 on 7 of 7 sampled merged PRs (release-train#109/#108, .github#304/#300, backend#2266, client#774, docs#131). The house convention puts the ticket in the PR TITLE; GitHub creates a closing link ONLY from a keyword in the BODY, so a title reference is inert. `kanban-closure-router.yml` fires, finds no linked issue, and correctly does nothing -- every kanban workflow green, every card unmoved. Adds a `closing-ref` job to the EXISTING `set-pr-status.yml` reusable: parse the real title, assert the real `closingIssuesReferences` contains what it names. Derived, not restated (rule 1): two live reads, no list of tickets, repos or authors. The four title forms are measured, not imagined. A bare `#N` outside parentheses is deliberately NOT read as a ticket -- backend#2309's `#2271` is prose about a PR, and scanning loose `#N` would redden a compliant PR. Fails closed (rule 3): a blank title, a GraphQL error, `pullRequest: null`, a null/ownerless node, or `totalCount > len(nodes)` all exit 2 as "cannot tell", never a pass and never a finding against the author. The truncation test is load-bearing beyond pagination -- a link to an issue the token cannot read comes back missing from `nodes` while `totalCount` still counts it, which is indistinguishable from "not linked". The cross-repo trap is its own verdict: `WRONG_REPO` is reported apart from `MISSING` because the remedies differ -- a bare `Closes#304` in `.github` links `.github#304`, closing the wrong issue on merge, and needs the line rewritten rather than added. Fixtures are measured bytes (the backend#2114 lesson), captured with `gh api graphql` and re-verified against the live API before commit. Tests: 102 selftest assertions; 34 mutations, 0 stale, 0 uncaught. The mutation harness edits the real gate and re-runs the real suite -- no inline copy of any rule (rule 9, .github#114/#115). Every anchor must match exactly once, which is the assertion that it actually applied. Refusals are asserted by their own message, never a catch-all (rule 10). The commit-type vocabulary is derived out of org-standards.md and the derivation fails closed if it finds nothing (rule 6). Arming: `closing-ref` is a required status check NOWHERE -- measured across 19 repos x develop/staging/main/master x both classic protection and rulesets -- so a finding blocks no merge (rule 4). Callers trigger on opened/reopened/ready_for_review/converted_to_draft, not `synchronize`, so the 13 open PRs that would report a finding are not reddened by a push. Touches no file in `conformance-gate.yml`'s GUARDED list, and needs no `repo-inventory.yml` row: the inventory tracks callers, one row per reusable, and `set-pr-status.yml` already has its rows. Closestracebloc/backend#2364 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(2364): the remedy stops guessing a repo it cannot know (backend#2364) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(2364): the scope pattern admits a leading dot too, so .github stops depending on a coincidence (backend#2364) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promotes the two fixes that just landed on
develop. Both are reusable/scheduled workflows, so they only take effect frommain.fr-pass-comment.yml — two independent defects (tracebloc/backend#1319)
if:carried the D30 self-signoff rule. A false jobif:skips with zero steps executed, so nothing was left to react or explain — the run just readskipped. Correlation was 13/13: every silent comment was on a PR its own commenter had authored.default_workflow_permissions: read, a called workflow can only narrow the caller's token, and|| trueswallowed the resulting 403.No decision lives in the job
if:any more — it is only a cheap "is this about /fr-pass" filter. Every terminating path now publishes an outcome and reacts, and all writes usePROJECTS_KANBAN_TOKEN. Also closed: swallowed GraphQL errors that made an expired token look identical to "not on the board", a truncated page read as absence,author_associationused as a permission check (private org membership reads asCONTRIBUTOR), andcontains()matching/fr-passport.Release-train promotion PRs are exempt from D30 (Lukas's call): the FR gate on those evaluates the contained items, so the promotion PR itself carries nothing for a second reviewer to assess. The branch refs are looked up and the check fails closed — an unreadable PR is treated as normal work and keeps the rule.
merge-settings-drift.yml — enforce the #1337 sweep (tracebloc/backend#1337)
Rebase-merge was disabled on all 19 active repos, but GitHub enables it by default on new repos and nothing guarded it. Weekly report-only check: enumerates by listing the org (archived and forks excluded), and flags rebase-merge drift, repos with no merge method, and repos whose settings it could not read. Never PATCHes anything — a settings change should be deliberate and visible, and the token then needs no standing
administration:write.Each finding carries only its own remediation: disabling rebase is the wrong advice for an all-methods-disabled repo (no-op) or an unreadable one (a token problem). It fails the run on any finding, so a green run means the settings are actually correct, and it comments on #1337 only when something is wrong — a weekly all-clear would train everyone to ignore it.
README.md
The repo README was a one-line stub. It now carries a new-repo checklist, opening with disabling rebase-merge (reason + command), plus the caller workflows,
developcreation, branch protection, the release-train decision, and private-by-default — so the manual path is covered alongside the automated check.Verification:
actionlint+shellcheckclean at the pinned versions (1.7.12 / 0.11.0); the drift audit executed against the live org (19 repos, zero drift, confirming the sweep held); the fr-pass fix replayed against all 13 real events (the 8 formerly-silent now react, the 5 that worked still reportalready-advanced) and driven through all 14 terminating paths; the exemption verified against real promotion and non-promotion PRs including the fail-closed path. Every Bugbot finding on both PRs fixed and resolved.Note
Medium Risk
Changes org-wide promotion/kanban automation and FR sign-off policy (including release-train exemptions); mistakes could block or mis-advance cards, though behavior is heavily documented and fail-closed on API errors.
Overview
Fixes
/fr-passso reviewers always get thread feedback, and adds a weekly org-wide merge-settings audit so rebase-merge drift cannot silently break the FR gate again.fr-pass-comment.ymlno longer encodes auth, D30 self-signoff, or board logic in the jobif(only a loose/fr-passmention filter), so skipped jobs cannot masquerade as success. Every path records an outcome, posts 👍/👎 viaPROJECTS_KANBAN_TOKEN(not read-onlyGITHUB_TOKEN), and explains failures on the PR/issue. The handler tightens command detection, checks repo write permission (with association fallback), exempts release-train / hotfix-backmerge promotion PRs from D30, surfaces GraphQL/mutation errors, and adds optionalallow-self-signoff.merge-settings-drift.yml(Mondays 06:00 UTC) lists active org repos and flags rebase-merge enabled, no merge method, or unreadable settings; it comments on backend#1337 with per-finding remediation only and fails the run on any finding (report-only, no auto-PATCH).README.mdexpands from a stub into a new-repo checklist (disable rebase-merge, caller workflows, branches, protection, release train) and documents scheduled checks including the new drift job.Reviewed by Cursor Bugbot for commit 495a29a. Bugbot is set up for automated code reviews on this repo. Configure here.