Uh oh!
There was an error while loading. Please reload this page.
fix(fr-pass): exit non-zero on every refusal; stop the bot triggering itself (backend#1413) - #121
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>
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.
Fixes the two defects in
tracebloc/backend#1413. Touches only.github/workflows/fr-pass-comment.yml.Line numbers in the ticket's correction pass were exact — 9
exit 0sites at 143, 175, 220, 313, 323, 354, 361, 368, 442, file 498 lines. The original body's 7-site list was the stale one.1 · Exit codes
Rule applied: only advanced, already advanced (no-op) and not a /fr-pass command may exit 0. Everything else exits 1, so a refusal is distinguishable from a success in
gh run list.not-a-command0→0/fr-passbut never issues it. The one deliberately silent path; nothing was asked for, so nothing failed.not-authorised0→1self-signoff0→1not-on-project0→1ambiguous-item0→1already-advanced0→0Ready for prod/Prod. The gate is satisfied and there is nothing for a human to do — a genuine no-op, so it stays green.no-status0→1Statusempty, so there is no column to advance from.wrong-column0→1FR on staging. Nothing advanced.not-a-command(reporting step)0→06 of 9 changed, matching the corrected count. The two unlisted sites (354, 442) both turned out to be legitimately green — one already advanced, one not a command — so the ticket's "6 of 9" numerator holds without adjustment.
The five paths that already exited 1 (274, 288, 307, 340, 397 — API and mutation errors) are untouched.
Refusal wording is unchanged, and no decision the workflow makes has changed. Verified that the
trap publish EXITstill writessteps.assess.outputs.resulton anexit 1, so the reporting step (if: always()) still posts the 👎 and its note exactly as before. Only the job's final status differs.2 · The bot triggering itself
Every refusal note contains the literal
`/fr-pass`, and the job filter matched any body containing it — hence 6 of 26 runs being the bot answering itself. It terminated only because the step's anchored grep rejects a line starting with an emoji; reflow a note and it loops.The reporting step now prepends
<!-- fr-pass-handler -->to every note it posts, and the jobifrejects a body starting with it. Added at the single place a note is posted rather than in eachdecidecall, so no future note can forget it, and it holds however the wording is reflowed.startsWithrather thancontains, marker first — GitHub's Quote reply prefixes>, so a human who quotes a refusal and issues the command underneath is still heard, which the step's precise grep then resolves correctly.On the requested actor guard. A
comment.user.type != 'Bot'clause is included, but it is not what fixes this, and the ticket's framing needs a correction: there is no separate bot identity to guard on.PROJECTS_KANBAN_TOKENis a PAT belonging to a human — the self-signoff notes on backend#1353 are authored byLukasWodka,type: User,performed_via_github_app: null, i.e. the same login as that person's genuine reviews. An actor guard on the login would lock out the busiest reviewer. It is keyed on account type instead, and becomes sufficient on its own the moment the token moves to a GitHub App.Note wording was left alone deliberately: the marker guard is already reflow-proof, and rewriting the messages to avoid the literal string would have worsened them for no additional robustness.
Verify
actionlintrun locally and passed — v1.7.12 with shellcheck, clean on the changed file and across all workflows in the repo.exit 1+ EXIT-trap interaction were both exercised locally (see above).issue_comment: [created]only, so there is no edited-comment path around the guard.FR on staging: comment/fr-pass→ expect 👎, the same explanatory note, plus a red run. Previously green.FR on staging: comment/fr-pass→ 👍, card advances, run green. Unchanged.not-a-command.Two notes for the reviewer
@main, so this has no effect untildevelopis promoted tomain.allow-self-signoffdefaults totrueand the callers pass no inputs, so path ci: add reusable advance-deploy-env workflow #3 is unreachable in practice today, as the ticket says. The logic is retained — it is still reachable when a caller sets the input tofalse.Refs tracebloc/backend#1413
🤖 Generated with Claude Code
Note
Medium Risk
Changes org-wide FR gate CI signaling and job filtering only in GitHub Actions; thread behavior is unchanged, but green runs on refusals were masking governance outcomes.
Overview
Fixes backend#1413 in the reusable
/fr-passkanban handler so workflow run color matches whether a card actually moved.Refusal exit codes: Deliberate no-ops that still post 👎 and an explanation (
not-authorised,self-signoff,not-on-project,ambiguous-item,no-status,wrong-column) nowexit 1instead of0, sogh run listno longer shows green when the card stayed put. Onlyadvanced,already-advanced, andnot-a-commandremain success exits; thread messaging and the EXITtrapreporting path are unchanged.Self-trigger loop: The job
ifnow also skips bot commenters and bodies thatstartsWith<!-- fr-pass-handler -->. Every posted refusal note prepends that HTML comment at the single comment-post site, so notes that still contain/fr-passdo not re-fire the workflow (quote-reply with>prefix is still allowed for real commands).Reviewed by Cursor Bugbot for commit b41db6a. Bugbot is set up for automated code reviews on this repo. Configure here.