fix(kanban): the deploy-state guard was matching a column that no longer exists - #237
Conversation
promote: develop -> main (D30 self-signoff relaxation)
chore(promote): develop → main — fr-pass exit codes, fail-closed mutators, caller comment (backend#1412, #1413)
chore(promote): develop → main — wire the Done column, and give the closer lookup a token (backend#1411)
release-train: develop -> staging
release-train: develop -> staging
release-train: staging -> main
release-train: develop -> staging
release-train: develop -> staging
release-train: develop -> staging
release-train: staging -> main
release-train: develop -> staging
release-train: develop -> staging
release-train: develop -> staging
release-train: staging -> main
release-train: develop -> staging
release-train: staging -> main
release-train: develop -> staging
release-train: staging -> main
release-train: develop -> staging
release-train: develop -> staging
release-train: staging -> main
release-train: develop -> staging
release-train: develop -> staging
release-train: develop -> staging
release-train: staging -> main
release-train: develop -> staging
release-train: develop -> staging
release-train: develop -> staging
release-train: staging -> main
…ger exists `CURRENT_COL` is read from the BOARD, whose column is "Staging (agent review)". The guard's list carried only the pre-rename "Staging (human review)", so an issue hand-closed while sitting in the agent-review column did not match, `Done` overwrote its deploy state, and kanban-archive.yml then hid the card entirely -- precisely the sequence this guard was added to prevent (Bugbot, .github#126), reopened by a string that quietly stopped being true. kanban-reconcile.yml's equivalent guard already lists both names, which is what makes this a drift rather than a design gap: one list was updated at the rename and its copy was not. The guard also stranded silently. Refusing is right -- the two real cases needed OPPOSITE answers (backend#1493 had shipped via cli#452 and belonged in Prod; data-ingestors#488 was reverted and belonged in Done), so no default is correct and only the person closing the issue knows which. But a `::notice::` in a run log is invisible by the time anyone looks at the board, so the card sat in a deploy state with nothing anywhere saying why. It now comments on the issue with the two options, and warns if even that fails. Verified: YAML parses, the embedded shell parses, house-rules clean, the version-bump-gate selftest still 51/0. Refs tracebloc/backend#1846
|
bugbot run |
There was a problem hiding this comment.
✅ 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 894499a. Configure here.
|
The red This PR originally targeted It does not block. Confirmed independently by the watcher in #239, run against this repo: I am not re-running it on purpose. Re-running a stale run makes it the newest entry for its context and can re-block a PR — trap #2 in the runbook this epic just wrote (release-train#70). The honest fix is to leave it and say why, which is this comment. |
shujaatTracebloc
left a comment
There was a problem hiding this comment.
The fix is correct, and I verified the diagnosis against the live board rather than the diff.
engineering kanban Status options:
6 On dev
7 Staging (agent review)
8 FR on staging
9 Ready for prod
10 Prod
Staging (agent review) is real and Staging (human review) does not exist on the board at all. So the guard genuinely was blind to the column cards actually sit in, and the failure chain described — guard misses → Done overwrites the deploy state → kanban-archive.yml hides the card — is exactly right. Also confirmed the two-copies claim: kanban-reconcile.yml:367 already carried both names in the identical case shape, so this really was one list updated at the rename and one not.
Commenting on the issue is the right call. Refusing silently parks a card in a deploy state with no self-heal, and a run-log notice is invisible by the time anyone looks. The two cited cases needing opposite answers (backend#1493 shipped → Prod; data-ingestors#488 reverted → Done) is the argument for why no default is correct — that belongs in the comment text, and it's there.
The step runs under PROJECTS_KANBAN_TOKEN, so no permissions: block governs it; if the PAT lacks issue-comment scope this degrades to the ::warning:: rather than breaking the router. Correctly handled.
⚠️ A bigger instance of this exact bug, in this same file, that the PR doesn't touch
The premise behind "when the rename window closes, drop the legacy name in BOTH places" no longer holds — the window is already closed, and three sites still hardcode the dead name:
:71staging) STATUS="Staging (human review)"— the pull_request writer:156same mapping on the issue-closure path:232if [ "$STATUS_NAME" = "Staging (human review)" ]— the rename fallback
Traced end to end, today's behaviour on a merge into staging:
STATUS = "Staging (human review)"
-> option lookup returns null (column gone)
-> :232 fallback (backend#1592) rewrites it to "FR on staging"
=> the card SKIPS "Staging (agent review)" and lands in FR review
The fallback was written when the rename was human-review → FR-on-staging. A later rename inserted Staging (agent review) between On dev and FR on staging, so the fallback now jumps a stage instead of translating one. Every staging promotion appears to bypass agent review on the board.
Not blocking — that predates this PR, #1846 is about the Done guard, and this change is strictly an improvement. But it's the same stale-string class, in the same file, on the write path rather than the guard path, so it's worth its own fix rather than waiting to be rediscovered. Flagging separately.
On the red check — stale, not real
gate / gate is failing for a reason unrelated to your code:
FR gate run 31680582303 created 08:06Z with BASE: main
PR updated (retargeted to develop) 10:17Z
The run predates the retarget, so it evaluated the "manual promotion PRs are retired" rule against main. Against develop that rule doesn't apply. Root cause: fr-gate-caller.yml:24 lists types: [opened, reopened, synchronize, ready_for_review, labeled, unlabeled] — no edited, so a base change never re-fires the gate and the stale verdict sticks.
I tried to clear it and can't — gh run rerun needs admin, and I lack label permissions on this repo. Toggling gate-nudge ("Toggled by the release train to (re-)fire the fr-gate") will do it in one step on your side.
Worth noting the irony: a required check stuck on a verdict that can never update is precisely what your #239 watcher is built to catch — and adding edited to that types list would prevent the whole class.
Approving on the code; the gate needs a nudge it won't take from me.
saadqbal
left a comment
There was a problem hiding this comment.
Clean fix. Verified Staging (agent review) is a real board column (it's rank 7 in fr-gate's rank(), added in #1577) and this now matches kanban-reconcile.yml's guard list exactly (line 367) — so the two guards are back in sync. Guard fires on the agent-review deploy state, and the issue-comment-with-::warning::-fallback is a good answer to the invisible-::notice:: problem.
Heads up on the red gate / gate — it's not this diff. The FR gate ran at 08:06 while the PR was still targeting main and correctly tripped the promotion-shape guard (manual promotions to main are retired); you retargeted to develop at 09:02, but fr-gate-caller.yml doesn't listen for base-change events, so that failed run never got superseded. A fresh push (or close/reopen) clears it; on a develop base the gate is a no-op anyway.
One thing worth naming (non-blocking): this restores parity but doesn't close the class of bug. An unrecognized CURRENT_COL still falls through and lets Done overwrite, so the next rename lands right back in this same silent no-op — now across two hand-maintained lists (here + reconcile) plus fr-gate's rank(). The comment's 'drop the legacy name in both places' is a manual reminder, not a guard. A shared list or a rank-based match would make the next rename unable to reopen it. Fine to leave for the deferred half of #1846.
Refs tracebloc/backend#1846 · epic tracebloc/backend#1646
A guard that stopped guarding, through a stale string
kanban-closure-router.ymlrefuses to overwrite a deploy state withDone(RFC-BACKEND-1405 D8).CURRENT_COLis read from the board, and the board's column isStaging (agent review)— while the guard's list carried only the pre-renameStaging (human review).So an issue hand-closed while sitting in the agent-review column did not match the guard,
Doneoverwrote its deploy state, andkanban-archive.ymlthen hid the card entirely. That is exactly the sequence the guard was added to prevent (Bugbot, .github#126), reopened by a string that quietly stopped being true.This is drift, not a design gap.
kanban-reconcile.yml's equivalent guard already lists both names — one copy of the list was updated at the rename and the other was not. When the rename window closes, the legacy name must be dropped in both places; there is now a comment saying so at the site.The guard also stranded cards silently
Refusing is right, and the two real cases prove no default would be:
main)ProdDoneOpposite answers, and only the person closing the issue knows which. But the refusal was a
::notice::in a run log — invisible by the time anyone looks at the board — so the card sat in a deploy state with nothing anywhere explaining why. It now comments on the issue with the two options, and emits a::warning::if even that fails.Scope
This is the half of backend#1846 that needed no decision from anyone. The other half — non-train repos like
claude-skillsparking atOn devforever, because nothing ever promotes them — is a per-repo.kanban.ymlmapping and stays open on the ticket.Evidence
YAML parses · the embedded shell parses under
bash -n·house-rulesclean ·version-bump-gateselftest still 51/0.🤖 Generated with Claude Code
Note
Low Risk
Workflow-only change to kanban closure routing and issue comments; no application runtime or auth/data paths.
Overview
Fixes the D8 deploy-state guard in
kanban-closure-router.ymlso hand-closing an issue inStaging (agent review)is treated like other deploy columns andDoneno longer overwrites that state (matchingkanban-reconcile.yml, which already listed both staging column names).When the router refuses setting
Donebecause the card is still in a deploy column, it now comments on the issue with the two manual outcomes (it shipped → move to the right column; nothing deployed → clear deploy state thenDone), and logs a::warning::if the comment cannot be posted—instead of only a run-log notice that leaves the board unexplained.Reviewed by Cursor Bugbot for commit 9ad6db0. Bugbot is set up for automated code reviews on this repo. Configure here.