Uh oh!
There was an error while loading. Please reload this page.
chore(promote): develop → main — fr-pass exit codes, fail-closed mutators, caller comment (backend#1412, #1413) - #124
Merged
Merged
Conversation
… itself Two defects, one file. 1. A correct refusal was indistinguishable from a success. Nine paths ended `exit 0`; only three of them had advanced anything or had nothing to advance. The six that lied now exit 1: not-authorised no write access to the repo self-signoff D30 second-pair-of-eyes refusal not-on-project item is not on the kanban at all ambiguous-item resolves to several cards; will not guess no-status on the board, Status empty wrong-column not in "FR on staging" The three that stay 0 are `advanced`, `already-advanced` (the gate is already satisfied) and `not-a-command` (both the first step's early return and the reporting step's silent path). The five error paths already exited 1 and are untouched. Why it matters: `gh run list` is where anyone reads many threads at once, and green there is taken to mean the card moved. Twenty-six green runs hid three cards that had not moved, the refusal was diagnosed as a bug in the advance logic, and the cards were then advanced by hand through the project API — overriding a governance control that had worked correctly. The 👎 and its note are unchanged and remain the real explanation; the exit status is only the signal that one exists. No refusal wording and no decision changed. 2. The handler answered itself. The job filter matches any comment containing `/fr-pass` and every refusal note contains that literal string, so 6 of those 26 runs were the bot reading its own comment. It terminated only because the step's anchored grep rejects a line starting with an emoji — luck, not design, and lost as soon as a note is reflowed. The reporting step now prepends `<!-- fr-pass-handler -->` to every note it posts and the job `if` rejects a body starting with it, so the guard holds regardless of how the wording is later reflowed. startsWith rather than contains, and marker-first, so a quote-reply (prefixed "> ") still gets in: a human who quotes a refusal and issues the command underneath must still be heard. A `comment.user.type != 'Bot'` clause goes in alongside it. It catches nothing today and is not the fix: PROJECTS_KANBAN_TOKEN is a PAT belonging to a human, so the handler's comments arrive under that person's login and are indistinguishable from their real reviews. Keyed on account type rather than a login for that reason — pinning the login would lock out the busiest reviewer — and it becomes sufficient on its own if the token ever moves to a GitHub App. Refs tracebloc/backend#1413 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four call sites counted their failures, printed them, and exited 0. A
green run therefore meant "the workflow finished", not "the board is
correct" -- and because every one of these is a scheduled daily pass,
a persistent cause (revoked project write, deleted Status option,
token scope loss) failed identically every day behind an unbroken
green history. Ports the RUN_FAILED pattern already used in
advance-deploy-env.yml rather than inventing a second one.
What fails closed now that did not before:
- kanban-archive.yml "Archive them": a failed archiveProjectV2Item
left terminal work on the board -- the one thing the workflow
exists to prevent -- and reported success. Now asserts fail -eq 0
after the loop. The error text was also destroyed by
`> /dev/null 2>&1`; stderr is now captured and printed with the
warning. Step summary is still written before the exit.
- kanban-reconcile.yml "Apply moves": same shape, both the ARCHIVE
and the Status-update branch. Every [FAIL] was a card left in the
column the pass had just decided was wrong. Now asserts fail -eq 0
and captures stderr into the [FAIL] line.
- kanban-reconcile.yml membership add loop: [FAIL] ("could not add")
and [WARN] ("Status set failed") were printed and never counted, so
the step could fail on every item and still go green. Both now
increment add_fail and the step exits non-zero. [WARN] counts as a
failure because a Status-less card renders in no column and no pass
repairs it: classify reads null Status as "No status" -> no-action,
and the sweep skips it because it is now on the board.
- kanban-reconcile.yml scope discovery: the add-to-kanban.yml probe
used gh's exit code only, making 404 ("not tracked") identical to
403/429/5xx ("could not determine"), so a repo silently dropped out
of scope and its drift became invisible. Now reads the HTTP status
and fails on anything that is neither 2xx nor 404. Adds a MIN_SCOPE
floor because the per-repo sweep emits "Missing from board: 0"
identically whether nothing is missing or nothing was examined --
MAX_ADDS only caught the opposite failure.
Success paths are unchanged; this is failure handling only. Verified
with actionlint 1.7.12 + shellcheck 0.11.0 (0 findings) and by
simulating each failure mode against stubbed gh calls on bash 5.
Refs tracebloc/backend#1412
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>The caller's header described a "FR on dev" -> "Ready for staging" transition that does not exist. RFC-BACKEND-1405 D6 retired the dev-side review -- "On dev" is set automatically when the train merges to develop -- so /fr-pass applies at exactly one gate: FR on staging -> Ready for prod. The reusable workflow it calls has only ever implemented the staging hop and documents that correctly at the top of the file, so this was the comment drifting away from the code, not a behaviour change. Comment-only; no logic touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(fr-pass): exit non-zero on every refusal; stop the bot triggering itself (backend#1413)
fix(kanban): fail closed in the three mutators that exit 0 on failure (backend#1412)
docs(ci): /fr-pass advances staging → prod only (backend#1405 D6)
LukasWodka
commented
Aug 1, 2026
ContributorAuthor
👋 Heads-up — Code review queue is at 39 / 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.) |
This was referenced Aug 1, 2026
Removes the reusable itself. MUST land after every caller is gone: a caller pointing at a deleted reusable fails hard, turning a soft nudge into a red check on every PR in the fleet. Refs: tracebloc/backend#1405
Removes the reusable itself. MUST land after every caller is gone: a caller pointing at a deleted reusable fails hard, turning a soft nudge into a red check on every PR in the fleet. Refs: tracebloc/backend#1405
Deleting it would break every caller still present on staging and the prod branches - a caller pointing at a missing reusable fails hard. Measured 2026-08-02: the caller is still on staging in all 10 train repos and on the prod branch in 14. Those copies need a staging hop AND a prod hop to clear. A tombstone keeps them green and removes the ordering hazard entirely. Refs: tracebloc/backend#1405
chore(ci): retire the WIP-limit nudge (no-op tombstone, not a deletion)
saadqbal
approved these changes
Aug 3, 2026
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 3, 2026
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
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 three fixes merged to
developtoday. Until this lands, none of them do anything — all 122 caller references across the org pin@main..githubhas nostagingbranch, sodevelop → mainis the whole promotion.What goes live
fr-pass-comment.ymlbackend#1413. 6 of 9 exit paths nowexit 1instead ofexit 0, so a refusal stops being indistinguishable from a success ingh run list. Plus a self-recognition marker so the bot can no longer answer its own refusal note.kanban-archive.yml,kanban-reconcile.ymlbackend#1412. Three mutators that counted failures, printed them and exited 0 now fail closed. Stderr is captured instead of discarded, a 404 is distinguished from a 403/rate-limit, and an emptySCOPEtrips a floor assertion instead of reporting "Missing from board: 0" after sweeping nothing.fr-pass-comment-caller.ymlFR on dev → Ready for stagingtransition retired by RFC-BACKEND-1405 D6. This is the file the 14 per-repo callers are copied from, so leaving it stale reintroduces the drift on the next repo enrolled.Blast radius — read before merging
This is the repo D1 calls "the highest-leverage repo we own": 122 callers across 16 repos consume these workflows at
@main, and they pick this up on their next run. There is no staged rollout.Expect things to go red that were previously green — that is the point of #122. Those three mutators have been swallowing failures. Any condition they were quietly tolerating will now surface as a red run, most likely on the nightly
kanban-reconcilecron. A red cron after this merge is the guard working, not a regression from it — but it is worth being ready to read the first one rather than assuming it is this PR's fault.#121is lower risk in the same direction:/fr-passrefusals that used to report success will now report failure. Same reasoning — those runs were already refusing, they were just lying about it.Ordering
backend#1411(wiring theDonecolumn) must land after this. The work order sequences it that way deliberately: arming these assertions afterDoneis wired enlarges the mutation population on the first run and turns a warning into a red cron. With this promoted first,#1411lands against already-armed guards.Verification
All three PRs passed
actionlint(the required check here) pluscheck,set-status,add-to-projectand Cursor Bugbot ondevelop.#122was additionally verified in abash:5container rather than local bash 3.2, because the runner is Ubuntu andmapfiledoes not exist on macOS bash — the local shell was the wrong oracle for it.🤖 Generated with Claude Code
Note
High Risk
Changes apply immediately to all org callers on
@mainand will turn previously green cron/comment runs red when underlying API or permission issues exist; mistakes affect production kanban state and release-train PR checks.Overview
Org-wide reusable workflows at
@mainget stricter signal hygiene for kanban automation and functional review./fr-pass(fr-pass-comment.yml) — Deliberate refusals (not-authorised,self-signoff,not-on-project,ambiguous-item,no-status,wrong-column) nowexit 1sogh run listgreen no longer means “card moved.” Onlyadvanced,already-advanced, andnot-a-commandstay exit 0. The jobifalso skips bot comments and bodies starting with<!-- fr-pass-handler -->; refusal notes prepend that marker when posted so the handler does not re-trigger on its own text.fr-pass-comment-caller.ymlcomments now describe the single gate FR on staging → Ready for prod (no retired “FR on dev” path).Kanban archive & reconcile — Archive mutations and reconcile apply / membership add steps capture GraphQL stderr, print it on failure, and
exit 1if any item fails (including cards added without a Status). Membership scope probing uses HTTP status (404 vs 403/5xx) instead of exit-code-only, andMIN_SCOPEaborts an empty tracked-repo sweep.WIP limit —
wip-limit-check.ymlis a documented no-op tombstone (inputs preserved for old callers);wip-limit-caller.ymlis removed from this repo. Lingering@mainreferences on older branches still resolve without a hard workflow failure.Reviewed by Cursor Bugbot for commit 74ffb7f. Bugbot is set up for automated code reviews on this repo. Configure here.