feat(kanban): reconcile asks the board too — the second copy of the list is gone - #252
Conversation
…ist is gone .github#249 replaced the router's hand-maintained list of deploy-state columns with the board's own ORDER. This is its twin, and the reason the class stayed open: there were TWO copies that had to agree, and one of them rotted. The router carried the pre-rename "Staging (human review)" while the board's column is "Staging (agent review)", so a card hand-closed there lost its deploy state and kanban-archive.yml hid it (.github#237). Both readers now derive the answer from `$PROJ`, which they already fetch: a deploy state is any column at or after "On dev" and at or before "Prod". A column INSERTED between them -- exactly how "Staging (agent review)" arrived -- is classified correctly with no edit in either file, and a renamed intermediate column keeps working because its POSITION is what matters. UNKNOWN NO LONGER FALLS OPEN here either, and it leans the opposite way to the router's on purpose: this loop decides whether to ASSERT Done, so an unplaceable column is treated AS a deploy state and skipped. Leaving a card where it is costs nothing; asserting "nothing shipped" over work that did erases the fact, which is the failure both halves of #1846 are about. Verified by driving the extracted `col_index` against a fixture board, the same way .github#249's selftest does: every deploy state skips, everything before On dev and both terminal columns are eligible for Done, an unknown column skips, and a NEWLY INSERTED column between the anchors skips with no edit. house-rules clean; the router's selftest still passes 15/15. Closes tracebloc/backend#1846
Two reported, and a third the first fix exposed. All mine. 1. col_index WAS DEFINED IN A DIFFERENT SHELL. It lived in the `ids` step and was called from `plan`; every `run:` block is a new shell, so the function and the `$PROJ` it read were both gone. The first closed-completed issue would have aborted classify with `command not found` and the weekly backstop would have applied no moves at all. `bash -n` cannot see this -- each block parses perfectly on its own -- which is exactly why it shipped. The board ORDER is now published as a step output from the step that HAS `$PROJ`, and the helper lives beside its caller. 2. "NO STATUS" IS NOT AN UNPLACEABLE COLUMN. `$COL` is the literal "No status" when the field is null, so the new guard skipped it as unplaceable -- and those are exactly the cards this backstop exists to terminalize. They would have stayed closed, non-terminal and unarchived forever. The ABSENCE of a column is evidence nothing deployed; an UNRECOGNISED column is not. 3. AND THE FIX FOR (1) CARRIED THE set -e TRAP AGAIN. `grep -nxF | head | cut` fails the pipeline when the column is not found, and `_ci=$(col_index ...)` is a bare assignment -- so an unrecognised column ABORTED the classify step instead of being handled as unplaceable. Verified directly: the line after the assignment never ran. That is release-train#73's trap, which I documented at length, written again here. A helper whose job is to report "not found" must not do it BY FAILING. awk always exits 0 and prints -1. Driven end to end against a fixture board, 13 cases: every deploy state skips, everything before On dev and both terminal columns stay eligible for Done, an unknown column skips, "No status" and empty are eligible, and a NEWLY INSERTED column skips with no edit. house-rules clean; the router's selftest still 15/15.
|
Both fixed in 1.
|
…#252) This change created a SECOND `col_index`, and the selftest extracted only the router's -- with a `paths:` filter that omitted kanban-reconcile.yml entirely. A PR touching only the new copy would never have run the check. That is the same two-copy rot this change exists to close, one level up: the guard could stay green while the thing it guards changed underneath it. TWO COPIES ARE FORCED, so the guarantee had to change rather than the count. kanban-closure-router.yml is a REUSABLE workflow that runs in the CALLER's checkout, so a shared script in this repo is not on disk for it; kanban-reconcile runs here and works from step outputs rather than `$PROJ`. They cannot share code, and their inputs differ -- jq over a project document, awk over a tab-separated order. So the selftest now extracts BOTH, runs every case through each, and requires them to AGREE. A divergence is reported as a disagreement rather than silently taking one answer: DISAGREE: router=DEPLOY_STATE, reconcile=ANCHORS_MISSING Verified by breaking only the reconcile copy -- every case turns into that line. Restored, 15/15 pass across both. The `paths:` filter now lists kanban-reconcile.yml on both triggers, so the check actually fires when the copy it now covers changes. ruff and house-rules clean.
|
Fixed in This PR created a second Two copies are forced, so the guarantee had to change rather than the count
So the selftest now extracts both, runs every case through each, and requires them to agree: Verified by breaking only the reconcile copy — every case turns into that line. Restored: 15/15 across both. And the Running total on this PR, since it is worth being straight about
Four rounds, four real defects, none visible in the diff. bugbot run |
Bugbot, .github#252: the selftest extracted col_index from both
workflows but then ran the ROUTER's comparison for both. Reconcile's
production decision -- unknown and a missing anchor become skip, and
"No status" is forced placeable -- never executed, so a regression in
that `if` stayed green while the test reported agreement.
Split the two halves so each is tested where it actually lives:
# selftest:classify-* the verdict. Byte-identical in both files and
asserted so -- it must not depend on which
workflow is asking. Only col_index differs,
because only their inputs do.
# selftest:policy-* what each file DOES with the verdict, run
verbatim from each file. These lean opposite
ways by design: the router refuses to WRITE
Done on unknown, reconcile refuses to ASSERT
it, and substituting one for the other is the
defect above.
classify_column is stubbed in the policy cases so all four verdicts are
driven, including ones no live board can produce.
Mutation-proved, four ways, each reddening the case that names it:
reconcile loses the "No status" bypass -> 3 fail (identity + 2 cases)
BOTH copies lose it (identity blind) -> 2 fail
reconcile policy `!= no` -> `= yes` -> 2 fail (noboard, unknown)
router policy `yes` stops protecting -> 1 fail
25 passed, 0 failed on the restored tree.
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 61ac077. Configure here.
Closes tracebloc/backend#1846 · epic tracebloc/backend#1646 · twin of #249
Why the class stayed open after #249
#249 replaced the router's hand-maintained list of deploy-state columns with the board's own order. There were two copies that had to agree, and one of them rotted: the router carried the pre-rename
Staging (human review)while the board's column isStaging (agent review), so a card hand-closed there lost its deploy state andkanban-archive.ymlhid it (#237).@saadqbal's point on that PR was that fixing one copy leaves the mechanism intact. This removes the other.
Both readers now ask the board
A deploy state is any column at or after
On devand at or beforeProd, taken from$PROJ— which both workflows already fetch, so this costs no extra call.A column inserted between the anchors (exactly how
Staging (agent review)arrived) is classified correctly with no edit in either file, and a renamed intermediate column keeps working because its position is what matters.Unknown leans the opposite way here, deliberately
The router decides whether to overwrite a deploy state, so unknown → refuse to write.
This loop decides whether to assert Done, so unknown → treat as a deploy state and skip. Leaving a card where it is costs nothing; asserting "nothing shipped" over work that did erases the fact — which is what both halves of #1846 are about.
Evidence
Driven by extracting
col_indexfrom the YAML and exercising the real comparison, the same way #249's selftest does:house-rulesclean; the router's selftest still passes 15/15; every embeddedrun:block parses underbash -n.🤖 Generated with Claude Code
Note
Medium Risk
Changes how weekly reconcile decides Done vs leaving deploy states—wrong classification could strand or mis-terminalize closed issues—but behavior is heavily guarded by extracted selftests and mirrors the already-shipped router approach.
Overview
kanban-reconcile.ymlreplaces the hard-coded deploy-state column list with the same board-order rule as the closure router: any Status between On dev and Prod is a deploy state. It publishes tab-separatedstatus_orderfrom the ids step (since$PROJdoes not survive shells), implementscol_indexvia awk so misses do not abort underset -euo pipefail, and uses sharedclassify_column()plusselftest:policy-*markers so closed-completed issues are only eligible for Done when classification isno—unknown / noboard / deploy columns are skipped (opposite lean from the router, which refuses to write Done).kanban-closure-router.ymlrefactors the Done overwrite guard to the sameclassify_column()+ policy block instead of inline index comparisons.CI / tests:
kanban-deploy-state-selftestnow runs whenkanban-reconcile.ymlchanges.kanban-deploy-state-selftest.pyextracts logic from both workflows, asserts byte-identicalclassify_column, runs bothcol_indexpreambles, checks cross-workflow agreement, and exercises each file’s policy verbatim (router errors on noboard, reconcile skips on unknown).Reviewed by Cursor Bugbot for commit 61ac077. Bugbot is set up for automated code reviews on this repo. Configure here.