Skip to content

fix(kanban): a completed issue is terminal, not a deploy state (backend#2722) - #361

Merged
LukasWodka merged 3 commits into
developfrom
fix/2722-closed-issues-terminalise
Aug 27, 2026
Merged

fix(kanban): a completed issue is terminal, not a deploy state (backend#2722)#361
LukasWodka merged 3 commits into
developfrom
fix/2722-closed-issues-terminalise

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Closed issues were landing in deploy columns because two mechanisms deliberately put them there — not because either was failing.

  1. kanban-closure-router.yml mirrored the closing PR's Status onto the issue, so a PR-closed issue inherited On dev / FR on staging / Prod. Its own declared contract said exactly that.
  2. advance-deploy-env.yml then marched those cards onward (backend#1600), because issues parked at On dev never advanced when the code shipped and drifted permanently — "2026-08-06: all 20 drifted cards were closed issues, 0 PRs."

#1600 was right that a closed issue must not be stranded, and wrong about where to put it.Done is terminal, so there is nothing left to drift, and kanban-archive sweeps it off the board daily — which answers #1600 more completely than advancing ever did.

What changed

  • Router: a completed issue routes to Done, regardless of how it was closed. not_plannedCancelled unchanged.
  • advance-deploy-env: the closing-issue advancement block is removed (126 lines → 24 lines of rationale).

Both halves, deliberately. Fixing only the router looks fixed while advance-deploy-env drags the cards straight back in.

The closer lookup goes with it

It existed only to tell a hand-close from a PR-close so the two could route differently; with one destination there is nothing to tell apart. It was also the origin of two fail-wrong defects — a transient GraphQL error reading as a hand-close (.github#126), and five distinct situations collapsing into one NONE token (.github#127) — and a call that is never made cannot fail.

Verified before removing: CLOSER_TYPE and CLOSING_PR_BASE were read nowhere outside that branch. permission-issues also drops from advance-deploy-env, since the only issue reads were in the removed block.

Checked, not assumed

Terminalising issues could plausibly block promotions, since fr-gate evaluates every contained item. It does not — fr-gate.yml:390 ranks Done11 against On dev6, so a terminal card satisfies both the staging and prod gates. Its own comment records this being learned the hard way, when strict equality meant "a single Done card blocked every prod" promotion.

Cost of the status quo, measured

117 closed issues cleared out of deploy columns by hand in one session (2026-08-27): 18 + 55 from FR on staging, 44 from On dev. Two of those passes were functional-review batches — had those 73 gone to Ready for prod with the PRs, the next prod payload would have read 73 items larger than the work it contained. And Ready for prod is not terminal, so kanban-archive never sweeps it.

Type

fix

Test plan

make selftests: all suites green (was 171 passed / 2 failed mid-change, now 0 failed). actionlint clean on both workflows; both parse under yaml.safe_load.

Two existing assertions were updated, not loosened.branch-status-map-selftest expected 2 override-consulting call sites and 2 holding-state writes in the router; the second of each lived in the completed-issue arm, which no longer maps a base. Both are now 1, with the reason recorded inline — and kept as exact counts rather than >= 1, because a floor would let a new unchecked call site hide behind an old good one.

This behaviour was completely unguarded, which is why it drifted. Flipping the router's destination from Done to On dev left all 29 selftest suites green. So the guard is new, in kanban-deploy-state-selftest.py, and it reads the completed arm out of the workflow rather than restating it:

mutationexpectedresult
router DoneOn devred2 FAILs
reinstate permission-issuesred1 FAIL
(restored)green0 failed

Each mutation asserted its anchor applied before running, so an inert edit cannot pass for coverage.

What was deleted, recorded so it is reused rather than rewritten

The removed loop was hard-won and its rationale is preserved in a comment at the deletion site: cross-repo closingIssuesReferences resolution, fail-closed on a lookup error so a rate limit could not read as "closes no issues" (.github#166) while still skipping a non-PR number quietly (.github#181), and a refusal to advance a still-OPEN closing issue (.github#168).

Follow-up deliberately not done here

repositories: on advance-deploy-env's token stays unnarrowed. Its stated reason was the cross-repo closing-issue advancement, which is now gone — so narrowing is plausible, but that is a token-scope change wanting its own measurement rather than riding a behavioural fix. The stale reason is corrected in place so nobody re-cites it.

Checklist

  • Targets develop
  • Self-contained: two workflows + their tests, no unrelated changes
  • Assignee set, one reviewer requested
  • No secrets, tokens or customer data

Closes tracebloc/backend#2722


Note

Medium Risk
Changes fleet-wide kanban automation on issue close and deploy pushes; misconfiguration could strand cards or briefly disagree with reconcile until the weekly run, but behavior is idempotent for PR merges and guarded by new selftests.

Overview
Completed closed issues are treated as terminal work, not as cards that should sit in or march through deploy columns (On dev, FR on staging, Prod). Deploy status stays on PRs; issues that are finished go to Done.

In kanban-closure-router.yml, the issues + completed path now sets STATUS="Done only. The GraphQL closer / timeline lookup and branch→Status mirroring from the closing PR are removed, along with the failure modes that mis-routed hand-closes vs PR-closes.

advance-deploy-env.yml drops the large loop that resolved closingIssuesReferences (including cross-repo) and advanced those issues with the same monotonic deploy logic as PRs. permission-issues is removed from the App token scopes since nothing in that workflow reads issues anymore. Comments clarify that unnarrowed repositories: is no longer justified by cross-repo issue advancement.

kanban-reconcile.yml aligns the weekly backstop: closed completed issues with a PR/Commit closer are written to Done via DONE_OPT, not derived through branch_status_map.py into deploy columns—avoiding the slower job undoing the router.

Selftests flip from “every reconcile $DEST arm must exist” to “no deploy derivation for closed issues,” tighten mapper call-site counts, and add kanban-deploy-state-selftest.py checks so regressions (e.g. router DoneOn dev) fail CI.

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

…nd#2722)
Closed issues were landing in deploy columns because two mechanisms deliberately
put them there, not because either was failing.
1. kanban-closure-router.yml mirrored the closing PR's Status onto the issue, so
a PR-closed issue inherited On dev / FR on staging / Prod. Its own declared
contract said so.
2. advance-deploy-env.yml then marched those cards onward (backend#1600), because
issues parked at On dev never advanced when the code shipped and drifted
permanently -- all 20 drifted cards on 2026-08-06 were closed issues, 0 PRs.
#1600 was right that a closed issue must not be stranded and wrong about where to
put it. `Done` is terminal, so nothing can drift, and kanban-archive sweeps it off
the board daily -- which answers #1600 more completely than advancing did.
So: the router routes a completed issue to Done regardless of how it was closed,
and the closing-issue block in advance-deploy-env is removed. Both halves, because
fixing only one looks fixed while the other drags the cards straight back in.
The closer lookup goes too. It existed only to tell a hand-close from a PR-close
so they could route differently; with one destination there is nothing to tell
apart. It was also the source of two fail-wrong defects (.github#126, #127), and a
call never made cannot fail. CLOSER_TYPE / CLOSING_PR_BASE were read nowhere else.
`permission-issues` drops from advance-deploy-env with the only reads that needed it.
Checked rather than assumed: fr-gate ranks Done 11 against On dev 6, so terminal
cards satisfy both gates instead of blocking promotions.
Cost of the status quo, measured: 117 closed issues cleared out of deploy columns
by hand in one session (18 + 55 from FR on staging, 44 from On dev). Two of those
passes were functional-review batches, so the next prod payload would have read 73
items larger than the work it contained.
Guarded now, because it was not: flipping the router's destination left all 29
selftest suites green. kanban-deploy-state-selftest asserts both halves, reading
the completed arm out of the workflow rather than restating it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodkaLukasWodka self-assigned this Aug 27, 2026
Comment thread.github/workflows/kanban-closure-router.yml
Comment threadscripts/tests/kanban-deploy-state-selftest.py
LukasWodkaand others added 2 commits August 27, 2026 17:37
Bugbot, High. The router now sends every completed issue to Done, but
kanban-reconcile.yml still derived a deploy stage from the closer`s base branch
and wrote On dev / FR on staging / Prod for exactly those cards. Fixing one
writer and not the other is worse than fixing neither: the weekly sweep would
have put every card back, the slower job silently undoing the faster one --
which is the failure .github#295`s own comment was written to prevent, arriving
from the other direction.
A deploy column is a property of a PR. A finished ISSUE belongs in Done whether
or not its fix shipped, and issue cards parked in deploy columns are what stops
kanban-archive clearing them.
The closer lookup stays, because having one still distinguishes the two cases in
the LOG -- but both arms now reach the same column, which is the rule.
Second finding, same shape one layer out: the selftest read only the router and
advance-deploy-env, so a green run confirmed the two EDITED files rather than
the invariant. It now extracts the reconcile`s closer arm too and asserts it
writes only DONE_OPT, names no deploy column, and reaches no branch->stage
mapper. Mutation-proved: restoring the On dev write reddens it.
Also removes two now-dead option ids shellcheck flagged, and the comment that
justified them, which my own change had made false.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ssue (backend#2722)
The selftest already encoded #2722 for the router -- one call site, not two --
but still required kanban-reconcile.yml to keep one. Removing reconcile`s
router-miss derivation made that false, so three assertions had to follow:
* reconcile`s mapper call sites: 1 -> 0. Its only one was the closed-completed
issue arm. The merged-PR lookback still places PR cards in deploy columns but
always mapped its branches INLINE, so the count did not move because of it.
* the DEST-arm invariant is inverted rather than deleted. It asserted every
Status with an option id needed an arm, or the backstop silently skipped the
repos an override exists for (.github#304) -- true while reconcile DERIVED a
column, meaningless now there is no $DEST. It now asserts there are no arms,
plus that the extractor can still find one in a fixture, so "no arms" cannot
be confused with "the reader broke".
* the stderr-preservation loop drops reconcile, which has no stderr to preserve.
Its absence is asserted above rather than left implicit.
All 15 selftests pass and actionlint reports 0 across every workflow.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

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

@LukasWodka
LukasWodka requested review from aptracebloc and saadqbal and removed request for aptracebloc and saadqbalAugust 27, 2026 15:53

@saadqbalsaadqbal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diagnosis is the valuable part: closed issues in deploy columns were not a malfunction, two mechanisms deliberately put them there and one of them said so in its own declared contract. That reframing is what makes the fix a deletion rather than a patch.

"#1600 was right that a closed issue must not be stranded, and wrong about where to put it" is the right way to overturn a prior decision — keep what it established, correct the placement, and show the new answer subsumes the old goal. Done being terminal means there is nothing left to drift, and kanban-archive sweeping it daily answers #1600 more completely than advancing ever did.

Doing both halves is the part I'd have gone looking for."Fixing only the router looks fixed while advance-deploy-env drags the cards straight back in" — two mechanisms, one symptom, and a half-fix that presents as a whole one. I have been on the wrong side of exactly this three times in the last week, so it is good to see it handled in the PR rather than in a follow-up.

Deleting the closer lookup rather than hardening it is the strongest available fix for its class. It existed only to distinguish a hand-close from a PR-close so the two could route differently; with one destination there is nothing to distinguish. And it was the origin of two fail-wrong defects — a transient GraphQL error reading as a hand-close, and five distinct situations collapsing into one NONE token. "A call that is never made cannot fail" is the right conclusion: removing the surface beats adding a guard to it.

Checking CLOSER_TYPE and CLOSING_PR_BASE were read nowhere else before removing, and dropping permission-issues because the only issue reads were in the deleted block, is the mint-scope discipline applied without being asked — the grant follows the code rather than outliving it.

And the "checked, not assumed" section asks the question that mattered most, which I verified independently. Terminalising issues could plausibly have blocked every promotion carrying one, since fr-gate evaluates every contained item. It cannot: Done and Cancelled both rank 11, above Prod at 10 and far above On dev at 6, so a terminal card satisfies both the staging and prod gates. The comment at that line also records the incident where both fell through to "" and the strict-equality path, so "a single Done card blocked every prod promotion that carried it" — which is precisely the failure this change would have re-created had the ranking not already been fixed.

Green, no threads. 126 lines of mechanism replaced by 24 of rationale is the right trade. 👍

@LukasWodka
LukasWodka merged commit 831e8b4 into developAug 27, 2026
15 checks passed
@LukasWodka
LukasWodka deleted the fix/2722-closed-issues-terminalise branch August 27, 2026 16:09
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.

2 participants

@LukasWodka@saadqbal