Uh oh!
There was an error while loading. Please reload this page.
fix(stale-backlog): an archived card is not a live Backlog card - #297
Conversation
…end#1979) `projectItems` returns archived items by default (`includeArchived: true`), and archiving a card does not clear its Status field -- `kanban-archive.yml` archives terminal items and leaves the field alone, so a card archived out of `Backlog` reads `Backlog` forever. `status_of` took the first card on the target project as live, so the sweep matched work somebody had deliberately taken OFF the board and auto-closed it. An issue whose card is archived is exactly an issue nobody is watching, and one bulk board tidy-up mints a batch of archived-but-`Backlog` cards at once. ISSUES_Q now asks for `isArchived` and `status_of` steps over archived cards, matching the sibling readers that already had this right (`kanban-archive.yml` selects `.isArchived == false`; `advance-deploy-env.yml` skips archived items). `is not False` is that jq predicate in Python: True, null and a missing field all fail it, so a query regression makes the sweep loudly useless rather than quietly resuming the close. `continue` rather than `return None`, because an issue can hold an archived card AND a live one on the same project; bailing on the first archived node would hide the live card behind it and skip an issue that is genuinely due. 10 new assertions, every one of which fails against the pre-fix script -- the load-bearing one being that an archived card already carrying `stale` returned `close`. Suite: 57 -> 67 passing. Found by Bugbot on the develop->staging promotion mirror .github#292. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LukasWodka
left a comment
There was a problem hiding this comment.
Approving. I verified both of the load-bearing claims rather than taking them, and they hold exactly.
"Every one of the 10 new assertions fails against the pre-fix script." Reproduced: I ran this PR's selftest against develop's stale-backlog.py and got 57 passed, 10 failed — and the 10 are precisely the new archived-card cases, with the pre-existing 57 untouched. That is the strongest form of this claim and it is literally true.
The 10 are well chosen, too, not ten angles on one assertion:
- both destructive paths separately —
main()closes nothing, and--strictfails - the query itself (
ISSUES_Q asks whether each project card is archived), so a future edit that drops the field reddens here rather than silently resuming the close - the fail-closed direction pinned explicitly —
a card with no isArchived field is UNKNOWN, not 'not archived'is what makesis not Falsecorrect rather than incidentally-passing, and it is exactly the assertion a== Trueimplementation would fail - the multi-project combination (
another project's live Backlog does not resurrect an archived target card), which is the case that catches a future fix that filters archived cards while loosening theproject.numbertest
"Matching the sibling readers rather than inventing a second convention." Also verified on develop: kanban-archive.yml queries isArchived in both node selections, and advance-deploy-env.yml selects on .isArchived and skips with a notice. So this script genuinely was the outlier, and the fix converges on the existing convention instead of adding a third.
Two judgement calls I think are right:
continue, notreturn None. An issue can hold an archived card and a live one on the same board; bailing on the first archived node would hide the live card behind it and skip an issue that is genuinely due. The test for that combination exists, so it is pinned rather than just reasoned about.- Writing it up as data-loss rather than Bugbot's Medium. The blast radius is
issue closeon an unattended weekly cron, against issues whose cards somebody deliberately archived — i.e. the ones nobody is watching. And the trigger is one bulk board tidy-up, not an exotic state. Re-rating that up is the honest call.
Nothing outstanding. CI green, no unresolved threads, MERGEABLE.
Uh oh!
There was an error while loading. Please reload this page.
LukasWodka
commented
Aug 21, 2026
/fr-pass |
Fixes a Bugbot finding on the
develop→stagingpromotion mirror .github#292, fixed ondevelopper the train contract. Rolls up under backend#1979 / backend#1597 item 1 — it is a defect in the code those tickets introduced, found in their own review.The bug
ISSUES_Qnever asked forisArchived, andstatus_oftook the first card on the target project as live. Two facts make that destructive rather than cosmetic:projectItemsreturns archived items by default —includeArchiveddefaults totrue, so not asking is asking for them.kanban-archive.ymlarchives terminal items and leavesProd/Cancelledon them; a card archived straight out ofBacklogreadsBacklogforever.So the sweep's allow-list-of-one matched cards that had been deliberately taken off the board, and the consequence was not a wrong log line — it was
issue close. An issue whose card somebody archived is precisely an issue nobody is watching, which is the worst place for a silent auto-close, and the trigger is ordinary rather than exotic: one bulk board tidy-up mints a batch of archived-but-Backlogcards at once.Bugbot rated it Medium. The blast radius is a destructive, unattended weekly cron, so it is written up here as the data-loss bug it is.
The fix
ISSUES_QrequestsisArchivedonprojectItems.status_ofsteps over archived cards, matching the sibling readers rather than inventing a second convention:kanban-archive.ymlselects.isArchived == falsebefore acting, andadvance-deploy-env.ymlskips archived items explicitly.is not Falseis that same jq predicate in Python —true,nulland a missing field all fail it, which is the fail-closed direction this script already takes everywhere else. If the query stops asking for the field, every card reads as un-established, every issue lands onNone, and the sweep goes loudly useless (counted as "no readable Status";--strictfails) rather than quietly resuming the close.continue, notreturn None. An issue can hold an archived card and a live one on the same project; bailing on the first archived node would hide the live card behind it and skip an issue that is genuinely due.On "the first matching" card — the multi-project question
Not a second defect, and worth stating explicitly rather than leaving implied. The loop already filters on
project.number, so it selects the card on the target board, not literally the first card an issue has — aBacklogcard on some other project has never made an issue eligible, and there is an existing case pinning that. What "first matching" was wrong about is narrower: first matching on the target board, archived ones included. Stepping over the archived ones is what makes that selection correct. A new case covers the combination anyway (target card archived, another board still sayingBacklog→ skipped), so a future fix that filters archived cards while loosening the project test gets caught.Also removed one line of unreachable dead code (
return Nonewritten three times at the end ofstatus_of).Tests
10 new assertions in
scripts/tests/stale-backlog-selftest.py, and every one fails against the pre-fix script — verified by running the new suite againstorigin/develop'sstale-backlog.py:The load-bearing one is the second: pre-fix, an archived card already carrying
stalereturnedclose. That is the assertion that names what this prevented.Each is asserted as a pair — archived is skipped and the same card un-archived is still warned/closed — because either half alone is passable by a broken implementation: "archived is skipped" passes if the sweep skips everything, and "live is warned" is the pre-fix behaviour.
main()is driven end to end too, not justdecide(): same function-versus-wiring split this suite keeps catching. A verdict ofNoneis worth nothing ifmain()still logs aCLOSE, and an archived-but-Backlogboard has to surface as "could not tell" rather than as a clean run.The query-text assertion follows the existing
totalCountprecedent: every fixture suppliesisArchived, so all of them stay green ifISSUES_Qstops asking for it — the "test richer than the query" shape this repo has been bitten by before. So it is asserted against the query text, the producer.After: 67 passed, 0 failed.
Has this already closed anything? No.
Checked before writing the fix, because the answer changes what else needs doing:
.github/workflows/stale-backlog.ymlstill pin the olduses: actions/stale@...copy;repo-inventory.ymlrecords the migration asSTAGED, NOT PARKED.workflow_call-only — no cron of its own.ci/1979-arm-the-sweep, so this lands before the sweep is ever armed.Issues closed with the
stalelabel across the org are the oldactions/staleat work — a separate and worse bug (fully column-blind, which is what backend#1597 exists for), not this one. Nothing needs reopening on account of this finding.Verification
make check— green (ruff, shellcheck, house-rules, action-pins, actionlint, every selftest, coverage gate). Output in the session log.An operator can confirm against the live board without closing anything: the script has
--dry-run(surfaced as thedry-run: trueworkflow input), which logs every[DRY] STALE/[DRY] CLOSEit would perform and changes nothing. Pairing it with--strictis the useful combination here — archived cards now land in the "no readable board Status" counter, so--strictturns an archived-but-Backlogboard into a visible non-zero exit instead of a clean-looking run.Note
Medium Risk
Touches unattended issue-close eligibility. The change is fail-closed (skip archived/unknown rather than close), but a mistake here still means silent data loss or a sweep that never acts.
Overview
Stops the stale-backlog sweep from warning or closing issues whose project card is archived but still stamped
Backlog. GitHub returns archivedprojectItemsby default and archiving does not clear Status, so the allow-list previously treated off-board work as eligible and couldissue closeit.ISSUES_Qnow requestsisArchived.status_ofskips any card that is not explicitly live (is not False), then keeps scanning so a live Backlog card behind an archived one is still found. MissingisArchivedis treated as unknown (counted for--strict), matching the existing fail-closed pattern.Selftests cover archived vs live pairs, mixed-card order, other-project cards, query-text presence of
isArchived, andmain()not closing archived items.Reviewed by Cursor Bugbot for commit a6b6ea0. Bugbot is set up for automated code reviews on this repo. Configure here.