Skip to content

fix(2242): Done is a Status an override may name, so a repo that deploys nothing can say so - #304

Merged
LukasWodka merged 3 commits into
developfrom
fix/2242-done-is-an-override-status
Aug 22, 2026
Merged

fix(2242): Done is a Status an override may name, so a repo that deploys nothing can say so#304
LukasWodka merged 3 commits into
developfrom
fix/2242-done-is-an-override-status

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Unblocks backend#2242. Prerequisite for tracebloc/rfcs#41 — that PR cannot merge
until this has reached this repo's main.

The blocker, measured

backend#2242's option 2 — give rfcs a .kanban.yml mapping its branches to Done
— was recorded as unblocked by backend#2243 (one shared mapping both writers read).
It unblocked half of it. backend#2324, landing the same day, made an undeclared
Status a refusal
, and ENV_FOR_STATUS declared the five deploy columns only —
so the semantically correct override was the one shape the new guard rejected:

# the mapper live on this repo's `main` today, given rfcs' .kanban.yml
develop -> REFUSED (exit 1)
::error::.kanban.yml maps `develop` to 'Done', which names a Status this mapping
does not declare. ...

Both refusals are right on their own terms; together they leave a repo that deploys
nothing with no way to say so. The accept list should be the set of Statuses a
merge can legitimately mean
, not the set of stages a deploy passes through.

The change

One row: "Done": "none".

none is a real option on the board's Deploy environment field (measured on
project #2, 2026-08-22 — none, Active, dev, staging, prod, Cancelled), and it is
the one that agrees with Done. The dict's contract is that acceptance carries its
own environment, so a deploy env here would leave the two board fields contradicting
each other — a card in Done stamped dev, the backend#1277 shape.

Three consumers checked first, because Done is terminal and nothing else here is

consumerverdict
advance-deploy-env.ymlrank() already scores Done 11, above Prod's 10 — the monotonic guard advances into it and nothing demotes out. A push re-carrying an old commit is a no-op, not an un-shipping.
kanban-closure-router.ymlits STATUS_NAME = "Done" guard refuses to write Doneover a deploy state. A merged PR's card sits in Code review, which classify_column places below On dev and returns no for — so the write lands, and the guard still protects the case it was built for.
kanban-reconcile.ymlits weekly sweep pulls non-terminal columns only, so it never sees these cards.

The first two are already asserted by kanban-deploy-state-selftest.py (Code reviewno, Doneno), so this PR adds no case for them — it cites them.

The third one was load-bearing and unasserted, so it is a check now

Reconcile's drift-to-prod arm writes Prod for any merged PR whose sha reached the
prod branch, and consults no .kanban.yml at all — deliberately, because
resolve_prod_branch refuses to trust a repo-controlled file (D27-L4). The sweep
filter is the only thing standing between that arm and every overridden card, once a
week, silently. Done is outside the filter today; nothing said it had to be.

branch-status-map-selftest.py now:

  • derives the filter by parsing it out of kanban-reconcile.yml — a hand-copied
    list here would agree with itself while the workflow moved (rule 1);
  • fails loudly on a filter it cannot recognise, because an empty set makes the
    assertion vacuously true, and that is the one outcome indistinguishable from a
    clean tree (rule 3); plus an anchor assertion so a partial parse is caught too;
  • keys the invariant on the environment, not on a hand-listed set of terminal
    names: a Status declaring none deployed nothing, so a sweep arm that writes a
    deploy column must never be able to reach it. Deploy-stage Statuses stay in the
    sweep on purpose — On dev and Ready for prod are in it and must be;
  • runs the assertion and both mutation directions through the same predicate
    (rule 9), so a guard that returns [] for every input cannot pass as a satisfied
    invariant.

Mutation-proved: adding "Done" to reconcile's sweep list — anchor verified
applied, grep -c = 1 — gives 168 passed, 1 failed, naming Done. Restored.

Evidence

scripts/tests/branch-status-map-selftest.py 169 passed, 0 failed
scripts/tests/kanban-deploy-state-selftest.py 27 passed, 0 failed
scripts/kanban-columns-check.py All 12 written Status name(s) exist
on the board (Done <- branch_status_map.py)
make check green

Deliberately not in this PR

repo-inventory.yml. rfcs' entries for these two callers are still exempt, and
that file's own note says to land the caller first and flip the entry after — its
audit branch for rfcs is the default branch (main, since rfcs is not on the
train), so the flip has to wait until rfcs#41 has been promoted there. Its
advance-deploy-env exemption text is separately wrong today — it reads "this repo
has no develop", and rfcs has one that receives merges (#32#36, #38) — and
correcting that belongs with the flip rather than with a change that would put the
inventory out of step with reality and redden the conformance gate.

Refs backend#2242, backend#2243, backend#2324


Note

Medium Risk
Changes weekly kanban reconcile writes for closed issues, including a new path that can move cards to terminal Done. Guards keep it off deploy columns, but a mapping or case-arm mistake would stall or misplace cards.

Overview
Lets a repo that deploys nothing (e.g. rfcs) map a merge to Done instead of a deploy column. ENV_FOR_STATUS now accepts "Done": "none", so the unknown-Status refusal no longer rejects the only correct override.

Reconcile’s router-miss backstop gains a "Done" arm so those cards are not left in an active column when the closure router misses. Selftests now parse the weekly sweep filter and DEST case from kanban-reconcile.yml and assert: no none Status is swept (so drift-to-prod cannot overwrite the override), and every declared Status that has an option id also has a write arm.

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

…ploys nothing can say so
`rfcs` ships no artifact and has no environment, so RFC-BACKEND-1405 D8 puts its
merged work in `Done` -- completed, nothing deployed. backend#2242 is blocked on
saying that: `ENV_FOR_STATUS` declared the five DEPLOY columns, and backend#2324
(correctly) made an undeclared Status a refusal -- so the semantically right
override was the one shape the new guard rejected.
The accept list is the set of Statuses a MERGE can legitimately mean, not the set
of stages a deploy passes through. `Done` -> `none`, and `none` is a real option on
the board's `Deploy environment` field (measured on project #2, 2026-08-22), so
acceptance still carries an environment that AGREES with the Status.
Three consumers were checked before adding it, because `Done` is terminal and
nothing else in the table is:
advance-deploy-env `rank()` already scores Done 11, above Prod -- the monotonic
guard advances into it and nothing demotes out.
kanban-closure-router its Done guard refuses to write Done OVER a deploy state; a
merged PR's card sits in `Code review`, which
`classify_column` returns `no` for, so the write lands.
Both facts are already asserted by
kanban-deploy-state-selftest.py.
kanban-reconcile its weekly sweep pulls NON-TERMINAL columns only, so it
never sees these cards.
That last one was load-bearing and unasserted. Reconcile's `drift-to-prod` arm
writes `Prod` for any merged PR whose sha reached the prod branch and consults no
`.kanban.yml` at all -- deliberately, because `resolve_prod_branch` refuses to
trust a repo-controlled file (D27-L4). The sweep filter is the only thing standing
between that arm and every overridden card, so it is a machine check now:
`branch-status-map-selftest.py` parses the filter out of the workflow and asserts
no Status declaring `none` appears in it. Derived, not restated; fails loudly on a
filter it cannot parse; and the predicate is exercised in both directions so a
guard that returns [] for everything cannot pass as a satisfied invariant.
Mutation-proved by adding "Done" to that list -- 1 failed, restored.
Refs backend#2242, backend#2243, backend#2324
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@LukasWodkaLukasWodka self-assigned this Aug 22, 2026
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

Comment threadscripts/branch_status_map.py
… must
Bugbot (.github#304, Medium) was right, and my comment was wrong. It said
kanban-reconcile's router-miss arm has "no option id for Done and skips" -- but
`DONE_OPT` is resolved in that very step and already written by the arm beside it,
so the skip was an omission, not a limit.
The consequence is the shape this PR would otherwise have opened rather than found:
widening the accept list without widening reconcile's `case "$DEST"` leaves exactly
the repos the override exists for with no weekly backstop. A router miss parks the
card in an active column, archive only takes terminal ones, and it stays there
forever -- the invariant .github#127 fixed for every other mapping.
Adding the arm is safe for the same reason writing DONE_OPT beside it is: the
enclosing `_skip` guard has already run `classify_column "$COL"` and required `no`,
so the card is in neither a deploy state nor an unplaceable column.
AND THE INVARIANT IS A CHECK, keyed on the OPTION ID rather than on "every declared
Status needs an arm" -- checking is what made that distinction (rule 8). Two
declared Statuses have no arm on purpose: nothing writes `Staging (agent review)`
yet (RFC-BACKEND-1552 D5, read-only until backend#1578) and `Ready for prod` is a
human `/fr-pass` act (D6), and reconcile resolves no option id for either. So the
real defect shape is "the job knows the id but cannot write it", and that is what
`branch-status-map-selftest.py` now asserts: both the arms and the resolved ids are
parsed out of the workflow, the parse fails closed with anchor assertions, and one
predicate serves the assertion and both mutation directions.
Mutation-proved by deleting the new arm (anchor verified applied, grep = 0):
173 passed, 1 failed, naming `Done`. Restored -> 174 passed, 0 failed.
actionlint clean; `make check` green.
Refs backend#2242
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

Comment threadscripts/tests/branch-status-map-selftest.py
…rm was unguarded
Bugbot (.github#304, High) again, and again right. `OPT_ID`'s non-greedy
`\$\(opt(?:_either)?\s+(.+?)\)` stops at the FIRST `)` -- and the one argument list
that matters contains one:
$(opt_either 'Staging (human review)' 'FR on staging')
OLD regex captured: ["'Staging (human review"]
So `FR on staging` was never collected. The invariant went green with that arm
deleted, and the anchor assertion did not notice, because `Done` -- which has no
parens -- was present. An under-collecting extractor reporting a clean sweep of a
SUBSET is exactly the shape `kanban-columns-check.py`'s `cross_check` exists for,
reproduced one file over, one commit after I wrote a docstring about deriving rather
than restating.
Three changes, and the third is the one that would have caught it:
* SCANNED, NOT REGEXED. `_arglist` walks the `$(...)` with a depth counter and is
quote-aware, so a `)` inside a quoted column name is part of the name. An
unterminated call returns "" -- a parse failure, not "no arguments".
* THE ANCHOR NAMES THE HARD CASE. It now requires `FR on staging` and
`Staging (human review)` -- reachable only through `opt_either` -- and >= 8 ids.
An anchor a broken parser can pass is not an anchor.
* THE MUTATION COVERS THAT PATH. Deleting the `FR on staging` arm is now a case.
With only `Done` mutated, the guard was vacuous for every parenthesised name.
Mutation-proved on the real workflow (anchor verified, grep = 0): deleting the
`FR on staging` arm gives 172 passed, 3 failed, the primary assertion naming
`FR on staging`. Restored -> 175 passed, 0 failed. `make check` green.
Refs backend#2242
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 8e569cf. Configure here.

@saqlainsyed007saqlainsyed007 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.

Reviewed all three files against the code — and the change is more than the "one row" the summary suggests, in the right way. It's the prerequisite for rfcs#41 (which needs Done to be an accept-list Status), and it lands the row together with the two things that make the row safe.

  • branch_status_map.py"Done": "none". The accept list was the five deploy columns; the board's vocabulary is wider, and a repo that ships nothing (rfcs, RFC-BACKEND-1405 D8) merges to Done, "completed, nothing deployed". none is a real Deploy environment option that agrees with Done, so the two board fields don't contradict (backend#1277). The three terminal-status consumers are each checked — advance-deploy-env's rank scores Done above Prod (monotonic, no un-shipping), the closure-router's guard won't write Done over a deploy state, and reconcile's sweep excludes it.
  • kanban-reconcile.yml — adds the "Done") OPT="$DONE_OPT" arm to the router-miss backstop. This is the half Bugbot caught: widening the accept list without this arm would drop exactly the override repos out of the weekly backstop (router miss → skip → card stuck in an active column), the invariant .github#127 fixed everywhere else. $DONE_OPT was already in scope; Staging (agent review) / Ready for prod are deliberately armless (no option id resolved — read-only / human /fr-pass).
  • branch-status-map-selftest.py — two invariants, each derived from the workflow, both-direction mutation-tested, and failing loud on an unrecognized parse (an empty set would pass vacuously): (1) no none-env Status sits inside reconcile's sweep, keyed on the environment so the drift-to-prod arm can't overwrite an override weekly; (2) every Status reconcile has an option id for has a DEST arm. The option-id extractor is a depth-counted, quote-aware scanner rather than a regex, precisely because Bugbot (High) showed the naive regex stopped at the first ) and silently missed FR on staging — the anchor assertion now names that opt_either case so a broken parser can't pass it.

Green, no open threads. Approving.

@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.

Approving — and the selftest is the part worth saying something about, because it clears the bar the accept-list change needs it to.

Both new invariants are derived from kanban-reconcile.yml rather than restated beside it, both fail loudly on a parse they no longer describe (the alternative being two empty sets that look exactly like a clean tree), and both run their assertion and their mutations through one shared predicate — so a guard that has stopped working can't present as a pass. The anchors are the good bit: picking FR on staging because it is reachable only through opt_either, whose argument list is the one containing a ), means the anchor is one the broken extractor could not have satisfied. An anchor a broken parser passes isn't an anchor, and Done alone was exactly that.

I also checked the thing that would have made me push back — armless keyed on "declared and has an option id" rather than "declared", so Staging (agent review) and Ready for prod having no arm stays silent. That's the deliberate case, and it's asserted as silent rather than left to be inferred, which is the difference between a guard and a coincidence.

I traced _arglist by hand: depth counting with a quote flag, ) inside a quoted name is part of the name, unterminated call and end-of-line both return "" and are treated as a parse failure. Correct for every shape in that file. The one thing it doesn't model is a backslash-escaped quote inside a quoted argument ("a\"b") — the flag would clear early and a later ) could be miscounted. Not reachable with the board's column names, so I'd leave it; worth knowing only because this file's whole job is extractor correctness.

One sequencing note, which is really about rfcs#41 rather than this PR: ENV_FOR_STATUS on main does not declare Done yet, and rfcs#41's .kanban.yml names it. This PR has to land first or that one's runs refuse. I've said so over there.

@LukasWodka
LukasWodka merged commit d32fd23 into developAug 22, 2026
14 checks passed
@LukasWodka
LukasWodka deleted the fix/2242-done-is-an-override-status branch August 22, 2026 20:04
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

/fr-pass

LukasWodka added a commit that referenced this pull request Aug 24, 2026
…#314)
* ci(2364): a PR whose title names a ticket must link it (backend#2364)
Merging a fix never closed or advanced its ticket. `closingIssuesReferences`
was 0 on 7 of 7 sampled merged PRs (release-train#109/#108, .github#304/#300,
backend#2266, client#774, docs#131). The house convention puts the ticket in
the PR TITLE; GitHub creates a closing link ONLY from a keyword in the BODY, so
a title reference is inert. `kanban-closure-router.yml` fires, finds no linked
issue, and correctly does nothing -- every kanban workflow green, every card
unmoved.
Adds a `closing-ref` job to the EXISTING `set-pr-status.yml` reusable: parse the
real title, assert the real `closingIssuesReferences` contains what it names.
Derived, not restated (rule 1): two live reads, no list of tickets, repos or
authors. The four title forms are measured, not imagined. A bare `#N` outside
parentheses is deliberately NOT read as a ticket -- backend#2309's `#2271` is
prose about a PR, and scanning loose `#N` would redden a compliant PR.
Fails closed (rule 3): a blank title, a GraphQL error, `pullRequest: null`, a
null/ownerless node, or `totalCount > len(nodes)` all exit 2 as "cannot tell",
never a pass and never a finding against the author. The truncation test is
load-bearing beyond pagination -- a link to an issue the token cannot read comes
back missing from `nodes` while `totalCount` still counts it, which is
indistinguishable from "not linked".
The cross-repo trap is its own verdict: `WRONG_REPO` is reported apart from
`MISSING` because the remedies differ -- a bare `Closes#304` in `.github` links
`.github#304`, closing the wrong issue on merge, and needs the line rewritten
rather than added.
Fixtures are measured bytes (the backend#2114 lesson), captured with
`gh api graphql` and re-verified against the live API before commit.
Tests: 102 selftest assertions; 34 mutations, 0 stale, 0 uncaught. The mutation
harness edits the real gate and re-runs the real suite -- no inline copy of any
rule (rule 9, .github#114/#115). Every anchor must match exactly once, which is
the assertion that it actually applied. Refusals are asserted by their own
message, never a catch-all (rule 10). The commit-type vocabulary is derived out
of org-standards.md and the derivation fails closed if it finds nothing (rule 6).
Arming: `closing-ref` is a required status check NOWHERE -- measured across 19
repos x develop/staging/main/master x both classic protection and rulesets -- so
a finding blocks no merge (rule 4). Callers trigger on
opened/reopened/ready_for_review/converted_to_draft, not `synchronize`, so the
13 open PRs that would report a finding are not reddened by a push.
Touches no file in `conformance-gate.yml`'s GUARDED list, and needs no
`repo-inventory.yml` row: the inventory tracks callers, one row per reusable,
and `set-pr-status.yml` already has its rows.
Closestracebloc/backend#2364
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(2364): the remedy stops guessing a repo it cannot know (backend#2364)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(2364): the scope pattern admits a leading dot too, so .github stops depending on a coincidence (backend#2364)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
LukasWodka added a commit that referenced this pull request Aug 27, 2026
…e truthful (backend#2556) (#360)
* fix(closing-ref): re-run when the title changes, and let a child PR be truthful (backend#2556, backend#2616)
Two defects in one gate, fixed together because either alone leaves the other's
remedy unusable.
BACKEND#2556 -- THE GATE READS THE TITLE AND NEVER RE-RUNS WHEN IT CHANGES.
`closing-ref` decides its verdict from the PR title and body; the callers
triggered on `[opened, reopened, ready_for_review, converted_to_draft]`, and
`edited` is the only event GitHub fires when either field changes. So the two
inputs the gate reads were the two inputs that could change without re-running
it. That is a bypass, not a gap: open a PR titled `chore: tidy up`, the gate
records NOTHING_NAMED and goes green, then retitle it to `fix(1234): ...` with
nothing linked -- no event fires, the green stands. It also made the remediation
unusable in the other direction: 20 sync PRs had to be cleared with 20 manual
`gh run rerun` calls (backend#2555). Same one-word fix, same reason, as
`fr-gate-caller.yml` (backend#1945).
A CONSEQUENCE THAT HAD TO BE GUARDED, not discovered later. Every trigger this
workflow had was reachable only on an OPEN PR, so `set-status` writing Status
unconditionally was always correct. `edited` is not: GitHub fires it when a
MERGED PR's title or body is edited, and this is the one board writer in the
fleet that does not compare pipeline rank. Without the new guard, fixing a typo
in a shipped PR's description would drag its card from `Prod` back to
`Code review`. Both jobs are now `if: ...state == 'open'`.
BACKEND#2616 -- ONE SATISFYING FORM, AND A CHILD PR CANNOT TRUTHFULLY USE IT.
The checker read `closingIssuesReferences` and nothing else, and that field is
populated only by a closing keyword. So a PR whose title named a ticket had
exactly one satisfying form: promise to close it. For this org's most common
shape -- a parent ticket with several child PRs titled `type(NNNN): ...`, none
of which finishes it -- the only remedies were a FALSE `Closes`, or deleting the
number from the title, which passes the check by removing the traceability the
check exists to enforce. Four PRs in one day took the second (.github#349, #350,
#352, #353) and #354 wrote that workaround into the canon.
A declared NON-CLOSING body reference now satisfies the title, reported as its
own state (`MENTIONED`) so a green run still says which promise was made. The
closing link stays the stronger form, and `WRONG_REPO` is decided BEFORE any
body reference is consulted -- a truthful `Part of tracebloc/backend#304` must
not mask a `Closes#304` that closes `.github#304` on merge.
DERIVED, NOT RESTATED (rule 1). The non-closing vocabulary is PARSED out of
`org-standards.md`, which already declares the org's partial-work form. Add
`Refs <owner>/<repo>#N` to the canon and the gate accepts it with no code
change. The derivation fails closed in both directions: an unreadable canon and
a canon declaring no non-closing form are separate cannot-tells (exit 2), never
a silent reversion to closing-only -- reverting is the defect itself. GitHub's
closing keywords are the one list here that cannot be derived from this org, so
they are named, cited, and used only to subtract.
The canon and CLAUDE.md carried two statements this change makes false -- that
the check fails unless the body carries `Closes`, and to keep the number out of
the subject. Both are rewritten; classify() reports IN_SYNC.
EVIDENCE
closing-ref-gate-selftest 156 assertions, all passed (was 109)
closing-ref-gate-mutations 53 mutations, 0 stale, 0 uncaught (was 36)
ruff / actionlint / shellcheck / house-rules clean
standards-sync selftest 60 checks, 0 failed
The mutation harness now rewrites the two WORKFLOW files as well as the checker:
`edited` and the open-state guards are guarantees declared in YAML, and rule 5
does not exempt a guard for living in a different language.
Part of tracebloc/backend#2556
Part of tracebloc/backend#2616
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(closing-ref): an escaped Unreadable reddens a case instead of killing the suite
The multi-word-keyword mutation was scored UNCAUGHT for the right reason: it made
`evaluate` raise, which killed the suite mid-run, and the harness refuses to
count a broken harness as coverage. `evaluate` grew a path that can raise for a
NEW reason (the derivation from org-standards.md coming back empty), so the
positive cases now go through a wrapper, exactly as `value` already does for
every other call.
Part of tracebloc/backend#2616
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(closing-ref): the derived-vocabulary cases report a refusal instead of crashing
`value` returns the exception TEXT on a refusal, and a string is iterable -- so a
case that walked the derived list iterated characters and raised IndexError on the
space in "RAISED Unreadable: ...". Normalised once. Found by the multi-word-keyword
mutation, which is the job: it was scored UNCAUGHT for breaking the harness rather
than being caught by a case.
Part of tracebloc/backend#2616
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.

3 participants

@LukasWodka@saadqbal@saqlainsyed007