Uh oh!
There was an error while loading. Please reload this page.
release-train: staging -> main - #301
Merged
Merged
Conversation
* feat(2114): a missing Bugbot review is a finding, not a clean PR (backend#2114)
Cursor Bugbot's auto-trigger silently drops PRs. Measured 2026-08-17: five open PRs
across cli, release-train and .github carried no `Cursor Bugbot` check at all, while
PRs opened minutes before and hours after were reviewed normally.
No discriminator survived. Not diff size -- a 1-file/66-line PR was reviewed and a
1-file/29-line one was not. Not file type: two 3-file Go diffs in the same repo, one
reviewed, one not. Not repo, not time (10:54 absent, 10:55 present), not quota (PRs
hours later were fine), not author or draft state. Posting `bugbot run` started a
review within a minute and both came back clean -- the reviews were not failing,
they were never starting.
WHY IT BELONGS IN THIS WATCHER. bricked-prs.py exists because a required check that
never reports "is the one CI failure mode with NO red signal at all -- nobody is
notified, and no reviewer sees a problem, because there is no failure, only an
absence. So it needs a watcher; nothing inside a PR can detect it." That paragraph
describes this bug exactly, one class over: checks then, the reviewer now. A per-PR
required check would be wrong here -- it would sit pending for the minutes Bugbot
legitimately takes, which is the thing this repo refuses to ship.
Four rules, each earned from the measurement:
- a SKIPPED Bugbot counts as PRESENT. It ran and decided; that is a verdict. Only
total absence is the silent drop.
- bot authors are exempt, keyed on GitHub's `[bot]` suffix rather than a list of
names that would go stale. dependabot[bot]'s absence was the one legitimate case
in the sample.
- the same young-head rule as the required-check case, and an unreadable age reads
as young: a false finding is what makes a report ignorable.
- reported EVEN WHEN every required check is present, because that is precisely
the case that renders as a completely clean PR.
AND ITS OWN LABEL. The renderer sent anything not `conflicted` to "BRICKED", so a
missing review would have been announced as a bricked PR -- sending the reader to
branch protection for something one comment fixes. It renders as UNREVIEWED, names
`bugbot run` as the remedy, and the summary now says the quiet part: a bricked PR
cannot merge, an unreviewed one merges perfectly well, which is the worse of the two.
That rendering is asserted from the source, because it lives in main() and no
decision-table case reaches it.
Six mutations, all caught: the check removed, the bot exemption disabled, the
young-head guard dropped, the cause collapsed into `never-reported`, the label
collapsed to BRICKED, and the remedy text removed.
Verified: bricked-prs selftest 21 passed (was 14); `make selftests` 41 passed;
actionlint clean.
Closes backend#2114
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(1979): stale-backlog becomes a board-aware reusable (backend#1979, #1597 item 1)
THE BUG. `actions/stale` exempts on LABELS ONLY. It has no concept of the board, so
an item in `North Stars`, `Ready for prod` or `In progress` went stale and auto-closed
after 8 weeks of silence exactly like a Backlog item -- a strategic priority archived
because nobody commented on it. `kanban-reconcile.yml` already carries a "Shield
North Stars from the stale sweep" step, which is the workaround admitting it.
WHY THE COPY MODEL HAD TO GO FIRST. Board awareness cannot be expressed as a label,
so it cannot be a config knob on an action -- it needs a script, and a script cannot
be maintained as sixteen byte-identical copies. Fixing the copy model was the
prerequisite, not the goal. That is the whole content of #1979.
THE RULE IS AN ALLOW-LIST OF ONE: eligible only when the board Status is exactly
`Backlog`. Every other column, an issue with no card, a Status that is not a string,
and a card on a different project are all skipped. A deny-list of the columns we
happened to think of would make a column added next year eligible by default, and
nobody would find out until something was closed.
FAIL CLOSED HERE MEANS SKIP, NOT ACT. Every other guard in this org fails closed by
refusing to report clean. This one is destructive, so the direction reverses: an
unreadable board, an unparseable response, an absent card and an unreadable
updatedAt all mean "leave it alone". `--strict` additionally exits non-zero so an
outage is visible rather than merely harmless -- silence that means "I could not
tell" must not look like silence that means "nothing was due".
A MAX_ACTIONS ceiling refuses rather than executes: the only ways to reach 40 due
issues are a bug in the eligibility rule or a board outage that made everything read
as Backlog, and mass-closing is the wrong response to either.
App-native from the start (backend#2036) -- it never had a PAT to migrate off. The
selftest runs as a GATE inside the reusable, before the sweep: a destructive
unattended job whose rule has regressed must not run at all.
STAGED INVENTORY, per this ticket's own sequencing. `stale-backlog.yml` moves from
`copies:` to `reusables:`, and all 17 repos get a caller entry declared `exempt`
with a written reason while the caller PRs are in flight. Declaring it `required`
now would redden the org audit for every repo whose caller has not landed -- the
drift window made to look permanent. The final PR of the sweep flips them.
TWO THINGS THE MIGRATION SURFACED, both fixed here rather than carried:
- `cli` was recorded `divergent` for pinning actions/stale@v11 where everyone else
pinned v9. A thin caller has no action pin, so the divergence is resolved by
construction rather than by a realignment PR.
- `release-train`'s exemption reason was rendered FALSE by this change. It read
"stale-backlog.yml is column-blind and would close live pipeline work". The
column-blindness is what this fixes. The exemption is RETAINED -- enrolling the
repo that drives promotions into an unattended closing sweep is a decision
someone should take deliberately -- but it is now recorded as UNDECIDED, citing
the absence of a basis rather than a defect that has been fixed.
Seven mutations, all caught: board-awareness removed (20 cases fail), the allow-list
turned into a deny-list of known columns (19), an unknown Status defaulted to
Backlog (2), the project number ignored (1), exempt labels dropped (2), an
unreadable date treated as ancient (crashes -- fail-closed by construction), and the
close grace period removed (1).
Verified: 30-case decision table; `make selftests` runs it and `selftests-cover`
confirms all 9 suites are wired; inventory schema validates; actionlint clean.
Refs backend#1979
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(1979): scope the new App mint before it ships, and drop the merge contamination
Two things this branch needed before it could go out.
1. THE DIFF CARRIED SOMEONE ELSE'S WORK. This branch was cut from the #2114
branch, so `scripts/bricked-prs.py` and its selftest showed as modified. #2114
has since merged (.github#282) with three rounds of review changes on top of the
snapshot here. Resolved by taking develop's version of both files outright --
verified `git diff --cached origin/develop` on them is EMPTY -- so the diff is
now #1979's own six files and nothing else.
2. IT ADDED A NEW UNSCOPED APP MINT. `stale-backlog.yml` minted with `owner:` and
no `permission-*`, i.e. the App's full installation grant, in the same week
backend#2157 swept 19 files to remove exactly that. Shipping a new one while a
guard against them is in review (.github#287) would have been the twentieth mint
arriving the way the first nineteen did.
Scoped from what the sweep actually CALLS, not from what the App holds:
issues: write `issue edit --add-label`, `issue comment`,
`issue close` (stale-backlog.py:188-192)
organization-projects: read ISSUES_Q reads `projectItems .. Status`; the
sweep never writes a board field
No `pull-requests` -- this workflow does not touch PRs by design (backend#1408).
No `contents` -- the App token is only ever `GH_TOKEN` for the sweep; the
`.github` checkout uses the workflow token with `persist-credentials: false`.
`repositories:` narrows the issue writes to the caller's own repo, which is
precisely the sweep's scope (`--repo "${{ github.repository }}"`). Without it a
DESTRUCTIVE, UNATTENDED sweep would hold issue-close rights across all 19 repos
while only ever closing in one.
Org-projects being unaffected by repo scoping is MEASURED rather than assumed:
backend#2181's verification run 32255581084 wrote the org project with
`repositories:` set. That was the load-bearing unknown and it is settled.
PROVED AGAINST THE GUARD ITSELF, not against my reading of it. Running #287's
mint-scope.py over this tree: 14 mint steps, 12 unscoped (all pre-existing and
exempted), 0 findings. So this branch passes the gate that is still in review --
which is the whole point of adding the gate before the burn-down rather than after.
make check green: 41 selftests, coverage gate, action-pins 34/0, actionlint 0.
Refs backend#1979, backend#1597 item 1, backend#2157.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(1979): truncation must not close an exempt issue, and .github joins the wave
Two Bugbot findings, both correct.
HIGH -- `exempt` PLUS A CALLER IS ITSELF A FINDING. This PR added
stale-backlog-caller.yml to .github while marking .github's stale-backlog.yml
`exempt`. caller-drift.py:2260 reports exactly that pair ("the exemption is stale").
It went green pre-merge only because .github's OWN callers are read from develop
over the API rather than from the PR checkout -- the awkwardness caller-drift's
docstring already documents -- so it would have merged green and reddened the very
next audit. That is the outage the in-flight exemption exists to prevent, produced
by the exemption's own PR.
The caller is removed. .github joins the caller wave like every other repo, and the
wave's final PR flips every `exempt` to `required` -- which is what the exemption
anchor already says the plan is. Recorded there, with the cost stated: .github is
unswept until its caller lands. The sweep is weekly and the reusable has never run,
so nothing regresses.
The alternative -- flip .github to `required` here -- is the direction caller-drift
explicitly refuses: the caller is not on develop yet, so the PR would go red.
"Land the caller first, flip the entry in a follow-up" is its own instruction.
MEDIUM, AND THE WORSE ONE IN CONSEQUENCE -- `labels(first:40)` never read
`totalCount`, so a `keep-open` label past the page read as ABSENT and the issue was
labelled `stale` and later closed. Every other unreadable input in this script
causes a SKIP; this one PROCEEDED. Both connections now request `totalCount` and a
truncated read refuses.
The shape of the fix is what reddened when `totalCount` was added to the fixture:
28 passed, 2 failed
FAIL an idle Backlog issue past 42d is warned
FAIL a warned issue silent for 14d+ is closed
Exactly the two cases that WRITE. Every skip case stayed green.
projectItems(first:10) is handled by the same rule. It fails SAFE -- no card lands
on "not Backlog" -- so it was never destructive; it is checked so both connections
obey one rule rather than one by accident.
5 mutations, all applied and caught. Two of them found my own errors:
* `total >= len(nodes)` would call an exactly-full page truncated and silently
leave every such issue unswept -- the opposite failure, equally invisible. Now
pinned by a case.
* the comment claimed the truncation test is "checked FIRST" and nothing pinned
it: OR-ing it with the exempt test left all 35 cases green, because both paths
skip. The order only changes the REASON -- but a `keep-open` issue reported as
"label list truncated" sends someone to fix the wrong thing.
AND THAT ORDER CASE WAS INERT ON ITS FIRST ATTEMPT. I asserted `"exempt label" in
why`, and the truncation message CONTAINS those words ("cannot rule out an exempt
label"), so it passed on the wrong reason. Now
`why.startswith("exempt label") and "truncated" not in why`. Same substring-
collision shape as an earlier inert assertion in this repo, which is why the
mutation run is the thing that decides, not review.
36 cases green. make check green.
Refs backend#1979, backend#1597 item 1.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(1979): a skip nobody counts is a clean-looking sweep
Two Bugbot findings, both consequences of the truncation fix in the previous
commit. Closing the fail-open created a silent-nothing, which is the failure this
repo names most often.
HIGH -- TRUNCATION SKIPPED AND REPORTED SUCCESS. The skip was real; nothing counted
it. `--strict` tested only `unknown` (no readable Status), and truncation set no
counter at all. So if ISSUES_Q ever stopped asking for `totalCount`, EVERY issue
would read as cut, every issue would skip, and the sweep would print `0 due` and
exit 0 -- the same signal as a genuinely empty backlog, on an unattended weekly
cron that nobody watches when it is green.
Three changes, in increasing order of how loud they are:
* decide() returns a TRUNCATED sentinel rather than folding into None. None means
"no card", which is ordinary; TRUNCATED means "could not tell", which is a
defect in the read. An object(), not a string, so it can never equal
ELIGIBLE_STATUS or a column somebody adds later.
* main() counts them separately and names them in the summary line.
* `::warning::` unconditionally, and `--strict` now FAILS on them. Truncation was
outside the strict test entirely -- the one mode that exists to make an
incomplete sweep visible could not see the most complete way to be incomplete.
MEDIUM -- THE SUITE DID NOT PIN THE QUERY. Every case built its own `totalCount`
payload, so dropping the field from ISSUES_Q left all fixtures green while
production treated every label list as cut. Now asserted against the query TEXT --
`totalCount` present, and present on the labels and projectItems connections
specifically. Same restate-instead-of-derive shape as the meta fixture on
.github#289, in the other direction: there the fixture was richer than production,
here it was richer than the query.
AND main() IS NOW ACTUALLY DRIVEN, because two mutations lived where no case looked.
Stopping the `cut` increment, and removing truncation from `--strict`, both left
every one of the 42 decide()-level cases green -- the same wiring-versus-function
gap as .github#289's High, in the same epic, two PRs apart. fetch_issues and apply
are the only network seams, so stubbing them runs the real parsing, the real
counters and the real exit codes. Behavioural, not a source assertion.
8 mutations, all applied and caught:
truncation reports None again 42/4
the cut counter never increments 43/3 <- was uncaught before main() cases
--strict stops failing on truncation 45/1 <- was uncaught before main() cases
the warning is removed 45/1
TRUNCATED becomes None 45/1
ISSUES_Q drops totalCount 45/1 <- was uncaught before query cases
the truncation check is removed 39/7
truncated() uses >= 37/9
46 cases green. `make check` exit 0 -- captured, not piped, after the last commit
went out red because `make check | tail -3 && git push` returns tail's status.
Refs backend#1979.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(1979): create the `stale` label -- actions/stale did, gh issue edit does not
Bugbot, Medium, and it would have broken the sweep on the first repo it ran in.
`gh issue edit --add-label stale` FAILS when the repo has never defined that label.
`actions/stale` created it; this script replaced actions/stale and did not. Measured
across the org 2026-08-20:
`stale` label: 11/19 repos have it
MISSING: .github, claude-skills, data-ingestors, design-system, docs,
release-train, rfcs, start-training
The eight without include `.github`, where this reusable lives. The eleven WITH it
have it only because actions/stale happened to create it there -- so the sweep would
have worked in the repos that had already been swept and failed in every repo that
had not, which is the least useful possible distribution of a bug.
Without the fix: every warn fails, the run goes red, and no issue ever enters the
14-day close window. A sweep that cannot pass its own first stage.
ensure_label() probes and creates once per run, and only when a warn is actually due
-- a close-only run needs nothing created, and a repo with nothing due should not
acquire a label it never uses. Idempotent by ASKING rather than by `--force`, because
`--force` rewrites colour and description on every weekly run: eleven repos would
take a no-op edit forever to spare eight a one-line create.
FAIL CLOSED, at the run level rather than per issue: if the label cannot be had, the
warn loop is refused with a message, instead of every `--add-label` failing
separately and reddening the run while warning nobody.
FOUR OF MY OWN ERRORS IN THIS COMMIT, each found by the suite or a mutation:
* ensure_label was an UNSTUBBED NETWORK SEAM in run_main, so the clean-sweep case
started returning 2 by shelling out to a real `gh` against the fixture repo. A
stub list one short is a test measuring the machine it runs on.
* a mutation making ensure_label report success on a failed create left all 49
green, because every case stubbed the function and pinned only WHEN it is called.
Third instance of the function-versus-wiring split in this epic (.github#289's
High, the `cut` counter earlier on this PR, this). The real function is now driven
with subprocess.run stubbed.
* that same mutation's first anchor -- `if dry_run:\n return True,` --
matched apply()'s early return instead, a different function that run_main stubs.
So it landed on code no case drives and reported "uncaught" for the wrong reason.
An imprecise anchor lies in both directions.
* `make check` was piped again on the previous push; exit status is captured now.
7 mutations across the label work, all applied and caught:
the label is never ensured (the original bug) 47/2
a failed ensure is ignored and warns anyway 48/1
the label is ensured on every run 48/1
ensure_label reports success on a failed create 52/1
dry-run creates the label anyway 52/1
the probe result is ignored 52/1
...plus the 8 truncation mutations, still green
53 cases green. make check exit 0.
Refs backend#1979.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>…evelop (#289) * fix(2214): audit the branch a repo ships from, not any branch named develop `caller-drift` preferred `develop` wherever that branch existed. `rfcs` is the only repo in the org whose default is `main`, so the backend#2157 sweep landed there -- where rfcs actually ships -- and the audit read a `develop` that lags, then reported drift against a change that was correctly in place: rfcs/main 2a3a432 the default branch, where the change landed rfcs/develop 07283e3 what the audit read IT AGREED BY COINCIDENCE FOR MONTHS, and that is the part worth recording. Both refs held the same blob because nothing had ever changed that file, so a wrong ref and a right ref were indistinguishable. A resolver pointing at the wrong branch produces a correct-looking result for exactly as long as the two branches match, then emits a false finding the first time real work lands. THE RULE IS NOW DERIVED, NOT LISTED. Develop-first exists because TRAIN repos default to main/master while work lands on develop, so a default-branch audit under-reports work in flight. A NON-TRAIN repo has no promotion pipeline: its default branch IS where it ships, and preferring a stray `develop` audits a branch nobody merges to. The discriminator is `release_train`, which the inventory already carries and which `load_release_train` already verifies against release-train/repos.yml -- so the audited ref is derived from a fact this guard independently checks, rather than from a hand-maintained `audit_ref` field or an exception row for rfcs. A second place to be wrong is what an override would buy. Measured across all 19 repos before writing it: the new rule changes exactly ONE answer, rfcs develop -> main. Nothing else moves. `audit_branch` is renamed develop-first -> develop-first-on-train in the inventory AND in SUPPORTED_AUDIT_BRANCH, because the schema check compares them and the semantics changed. The one-line description above the key said "develop where that branch exists, else the default" -- exactly the behaviour being removed -- and now states the rule and names the discriminator. BOTH DIRECTIONS ARE PINNED, because neither case can fail alone: assert only the train side and the non-train path is untested; assert only the non-train side and a resolver that always uses the default branch passes. Each reddens a different mutation. 4 mutations, all applied and caught: reverts to plain develop-first 193/1 the non-train case always the default branch 192/2 the train case the train flag read inverted 191/3 the train case + two more audit_branch no longer enforced 193/1 the inventory positive control AND ONE FIX TO THE SUITE ITSELF. The `_good` positive control RAISED AssertionError when the tree was fetched off develop, which aborts the whole run -- so two of those mutations first reported "CRASH" and hid every other case they also broke. It now returns an empty tree, failing cleanly. A suite that dies on the first surprise cannot tell you the shape of a regression. The fixture META also gained `release_train: True`, which is now load-bearing: without it every existing case would have silently exercised the non-train path while being written about develop-first. NOT FIXED HERE, because it is not in this repo: the workspace CLAUDE.md states "Verified 2026-08-06: every active repo's default branch is now `develop` (all 20)". That is false for rfcs and is the line a reader would use to conclude this guard reads the right branch. It lives outside any git repo, so it cannot ride a PR; flagged on backend#2214. make check green: 194 caller-drift cases, all selftests, coverage gate, actionlint. Closes backend#2214. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(2214): the train flag never reached the resolver Bugbot, High, and it makes the previous commit worse than a no-op. `read_repo` read `meta.get("release_train")`, but production `meta` comes from `list_active_repos`, which returns ONLY: {"visibility": ..., "default_branch": ...} No `release_train`. So the flag was always None -> False, EVERY repo was audited on its default branch, and develop-first was silently deleted for the train repos it exists for. Today that happens to be harmless -- every train repo currently defaults to `develop`, measured -- but the policy was gone, and the next repo to default to main/master would have under-reported work in flight with nothing saying so. THE SELFTEST PASSED BECAUSE THE FIXTURE CARRIED A KEY PRODUCTION NEVER SETS. I added `release_train: True` to META in the same commit that started reading it from there. A fixture richer than the real payload is a test asserting its author's assumption, and this is the THIRD time that shape has appeared in this epic -- `is_bot` on #282, the look-alike action name on #287, this. THE FIX IS A PARAMETER, NOT A LOOKUP. `on_train` is required and positional, so a caller that forgets it raises TypeError; defaulting it to False would reproduce the bug with better manners. `main()` passes `bool(entry.get("release_train"))` from the INVENTORY row -- schema-required, and cross-checked against release-train/repos.yml by load_release_train -- so the ref this guard reads is derived from a fact it independently verifies. META IS NOW EXACTLY THE PRODUCER'S SHAPE, and a case asserts that by reading `list_active_repos`'s own source rather than a list written in the test: it writes `visibility` and `default_branch` and nothing else. That case reddens if the producer starts setting `release_train`, which is the only honest way to keep the two in step. AND THE WIRING IS PINNED SEPARATELY, because the behavioural cases could not see it. They call read_repo directly, so mutations hardcoding the argument to True or False left all 196 green -- and #289's bug WAS the wiring, not the resolver. A source assertion on main() closes it. Weaker than behavioural and said so in the comment: driving main() needs the org listing, the inventory and the train file stubbed together, which this suite has no harness for. That assertion was ALSO wrong on its first attempt: a paren-matching regex allowing one level of nesting could not match `bool(entry.get(...))` across two lines, so it found nothing and failed on the correct code while every mutation "passed". An extractor that cannot find the thing reports the same as a defect. Line-based now. 7 mutations, all applied and caught: on_train read from meta again 194/2 the train-branch case call site passes False 196/1 the wiring case call site passes True 196/1 the wiring case call site reverts to meta 196/1 the wiring case plain develop-first restored 193/1 the non-train case always the default branch 192/2 the train case list_active_repos sets release_train 194/2 the producer-shape case 197 cases green. make check green. Addresses Bugbot on .github#289. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(2214): ruff — the previous push was red, and I masked it Three ruff errors (E402 import-not-at-top, two E741 ambiguous `l`) shipped in the commit before this one. They shipped because I ran make check 2>&1 | tail -3 && git commit && git push and a pipeline exits with the status of its LAST command. `tail` returned 0, so the `&&` chain treated a failed `make check` as a pass and pushed anyway. The output was even visible -- "make: *** [ruff] Error 1" was in the three lines I printed -- and the chain ran on regardless. Recorded rather than quietly fixed, because it is the same defect this repo keeps finding in its own guards: a check whose result nothing actually reads. Mine was in the shell, one layer out from the code. `make check` now run with its exit status captured (exit=0), not piped. 197 selftest cases green, 41 suites green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…rsarial pass (#291) * test(1788): house-rules.sh gets a suite, a mutation tier, and an adversarial pass 852 lines of quote-aware, heredoc-aware, command-position-aware shell lexing, with `quality / house-rules` a REQUIRED status check on develop, staging AND prod across 16 repos, and no test of any kind. A false positive here does not make noise; it blocks every merge in the org, including the release train's own promotion PRs, so the fix would travel through the pipeline it just blocked. 41 CASES. Four rules x fires/silent -- neither half alone is worth having: assert only FIRES and a matcher that flags everything passes; assert only SILENT and one that flags nothing does. Plus the lexer's six documented precision promises (each a reason a rule does NOT fire, and therefore a way it could start crying wolf), all three pragma forms, both repo-wide stand-downs, six config directives, the exit contract including --soft-fail, and `--base` over a diff with a deleted file. Every case pins the rule ID, not just a non-zero exit: four rules share an exit code. THE ADVERSARIAL PASS, which is #1788's own argument. The checker matches arbitrary source content, including comments that DISCUSS the patterns being matched. Run over 103 real shell files in five repos -- release-train's 19 prose-dense scripts that discuss `set -o pipefail` and SIGPIPE at length, client's 44 installers with real curl and helm, cli, e2e-test-agent, client-runtime: release-train 19 -> 0 client 45 -> 0 e2e-test-agent 25 -> 0 cli 11 -> 0 client-runtime 3 -> 0 AND A CONTROL, because "zero findings" and "the matcher stopped working" print the same thing. A violation injected into that same corpus fires all four rules. Both facts are cases; the control is what makes the clean result mean anything. A MUTATION TIER, because .github had none at all. It applies each mutation to the REAL script and runs the REAL suite -- no re-implemented rule inline, which reads as satisfied while breaking the real thing reddens nothing. An anchor that matches other than exactly once, or changes no text, is STALE and fails the run: an inert mutation and good coverage are indistinguishable in a log. IT EARNED ITS KEEP ON THE FIRST RUN. Two mutations passed the entire suite: * loosening `--tlsv1\.[23]` to `--tlsv1` -- every case used a COMPLIANT flag, so the version FLOOR, which is the actual rule, was never exercised; * reading the raw line instead of the masked one, re-introducing a documented, already-fixed bug where `set -o pipefail` inside a string marks a whole file safe. Nothing pinned that fix. Both are cases now. The second took three attempts: `echo "set -o pipefail"` does NOT reproduce it, because the detector anchors on `(^|[[:space:];])set` and there `set` follows a quote. I applied the mutation and enumerated which forms do -- `echo "please set -o pipefail here"`, `MSG="run set -o pipefail first"`, `printf "x; set -o pipefail\n"` -- and a comment does not. A case written for a bug it cannot construct is worth less than no case, because it reads like coverage. TWO NEAR-MISSES, recorded because both were one commit from being wrong: * The flag-var case failed and I read it for a minute as "the documented feature is inert". It is not: the table is built with `git grep`, so a /tmp fixture cannot reach it. The harness now runs inside a throwaway git repo with fixtures `git add`ed -- load-bearing, since the sourced-file list is repo-wide too. * Three fixtures appeared to show a live fail-open in the pipefail detector. Re-run on the pristine script, all three fire correctly; the "finding" was an artefact of a badly-escaped sed that had corrupted the awk program. Verifying on a clean file is the only reason it did not become a wrong ticket. ONE BEHAVIOUR PINNED RATHER THAN FIXED. A named path that does not exist reports "no shell files to check" and exits 0. That looks like the fail-open this ticket is about, and it is NOT REACHABLE FROM CI: code-quality.yml only ever builds `--all` or `--base "$BASE_SHA"` (code-quality.yml:755-758). Asserted as current behaviour with the reason, per the rule that a fix for an unreachable path costs more than filing nothing. The reachable neighbour -- `--base` over a diff that DELETED a shell file -- is asserted to still check the survivors. WIRING. `selftest-house-rules` joins the REQUIRED `selftests` context, and the mutation run gets its own step there rather than a new context that would sit unrequired until somebody armed it. `make check` resolves the anchors only (~ms); the full ~58s run is in `check-all` and in CI. Measured, and the numbers are in the Makefile next to the choice. AND THE COVERAGE GUARD LEARNED A SECOND FAMILY. `selftests-cover` rejected the mutation runner for not matching `*-selftest.{py,sh}` -- correctly, since a file no wildcard sees makes its assertion pass vacuously. Renaming it would have been a lie that also nested the 58s tier inside the 3s one, so `*-mutations.py` is now its own family with its own coverage assertion: a mutation runner nobody runs is the same dead weight as an unwired selftest, with the extra cost that it LOOKS like the tier exists. make check exit 0 (captured, not piped). 41 cases, 5 mutations, 0 stale, 0 uncaught. Refs backend#1788. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(1788): assert WHICH rules fired, not that one appeared somewhere Bugbot, Medium, and it is a dead parameter in the suite I wrote to prove the matcher works. `expect` took an expected finding COUNT as $3, computed `n`, and never compared them. So a non-zero case checked only rc=1 plus "the named rule appears in the output" -- and extra rules passed silently. BUGBOT'S EXAMPLE IS THE ONE THAT MATTERED: the scoped-pragma case `# house-rules: ignore=curl-tls` stayed green even if the scoping were a NO-OP, because `curl-timeout` fires either way and the test only asked whether `curl-timeout` appeared. A case whose entire purpose is to prove `ignore=` narrows to ONE rule, which could not have failed if it didn't. `n` was not even a finding count: its grep alternation included the word `house-rules`, which matches the summary line. THE FIX IS A SET, not a wired-up count. The expected value is now the exact comma-separated rule ids (`-` for clean), so extra findings fail, missing findings fail, and the wrong rule fails. Strictly more informative than a number, and it kills the parameter that read as an assertion and was not one. REGISTERED AS A MUTATION, so the case cannot go vacuous again: disabling the `ignore=` match now reddens exactly `a SCOPED pragma silences only the named rule` (40/1). Six mutations, 0 stale, 0 uncaught. THE STRICTER ASSERTION IMMEDIATELY CAUGHT TWO OF MY OWN CASES OVERSTATING WHAT THEY PINNED. A bare `curl -fsSL "$url"` has neither a TLS floor nor a time bound, so it violates BOTH rules; two cases claimed only `curl-tls` and passed under the loose helper. Corrected to `curl-timeout,curl-tls`. Same defect as Bugbot's, one level out: my expectation was looser than it read. AND THE FIRST EXTRACTION WAS NOT PORTABLE. I parsed the rule ids with `sed -n 's/.*\[\(a\|b\)\].*/\1/p'` -- `\|` is a GNU extension, so on BSD sed it matched nothing and every firing case reported "-", failing 11 cases at once. `grep -oE` instead. Portability is this script's whole design constraint (POSIX sh + awk, "a runner, a Mac, or a minimal container with equal results"), and the suite has to honour it too or it tests one platform. 41 cases green. make check exit 0. Refs backend#1788. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(1788): here-strings, and a trap that cleans the checkout Two Bugbot findings. The first is a defect I have spent this session writing about in another repo and then wrote fresh here. `printf … | grep -q` UNDER pipefail. `grep -q` exits on its first match and SIGPIPEs the writer; the PIPELINE can then take status 141, and the negated form (`! … | grep -q`) reads 141 as "absent" -- so a `disable:` directive that did nothing would have recorded a PASS. Eight sites, all converted to here-strings. Honest about reach, because overclaiming it would be its own defect: `printf` of a short string usually completes before `grep` leaves, so 141 is unlikely here rather than impossible. That is the same measurement release-train's fr-evidence.sh records -- SIGPIPE needs the writer ALIVE when the reader exits, which for a fetch-write-exit process means output past the 64K pipe buffer. Changed anyway, and for release-train's stated reason: a here-string is simply the smaller construction, with no pipe and therefore no pipefail interaction for the next reader to re-derive. The rule is now written where someone would otherwise put a pipe back. Bugbot's own framing is the sharper one: "sibling tests already use a here-string instead of a pipe." The idiom existed in this org and I did not follow it. THE TRAP DID NOT CLEAN THE CHECKOUT. The adversarial control writes a deliberate `curl` violation INTO THIS REPO and `git add`s it -- necessarily, because `--all` enumerates with `git ls-files` and cannot see an untracked file. The EXIT trap only removed $WORK, so an interrupt between the add and the cleanup left `.hr-control.sh` STAGED. That file is itself a house-rules hit, so the next `make check` would have failed on leftover test state and pointed at the wrong thing -- a test that breaks the build it is meant to protect. Now a `cleanup()` on EXIT INT TERM that unstages and deletes it, with the path in one variable so the two call sites cannot drift. Verified by sending SIGINT mid-run: no leftover, `git status` clean. While fixing it the variable rename swept one assertion onto the wrong output buffer ($OUT where it wanted $OUT2, the adversarial control's own run). Caught by reading the diff rather than by the suite, which would have passed either way because both buffers contained the string. 41 cases green, 6 mutations 0 stale 0 uncaught, make check exit 0, tree clean. Refs backend#1788. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(1788): the timeout-wrapper case could not fail Bugbot, Medium, and it is the vacuous-pin shape in the case written to prove the directive works -- the third variation of that defect this suite has produced. The fixture put a COMPLIANT curl behind the wrapper and recorded success from a clean exit alone: directive honoured `guard` is a wrapper, so curl is in COMMAND position and the *-timeout rules stand down -> clean, rc 0 directive IGNORED `guard` is not a wrapper, so curl is in ARGUMENT position and is not a command at all -> clean, rc 0 Identical outcome. A no-op `timeout-wrapper:` was indistinguishable from a working one, exactly as Bugbot puts it. DROPPING `--tlsv1.2` SPLITS THEM, because `timeout-wrapper:` waives only the *-timeout rules and leaves curl-tls live. Measured both ways before writing the assertion: with the directive rc=1 rules=[curl-tls] curl IS a command; timeout waived without the directive rc=0 rules=[-] curl is just an argument So the case now asserts the exact rule set, which also makes it say something stronger than "clean": that the directive waives the timeout rules AND ONLY THOSE. REGISTERED AS A MUTATION -- parse `timeout-wrapper:` and throw the value away. It reddens exactly this case. Seven mutations, 0 stale, 0 uncaught. Worth naming the pattern, because it has now happened three times in one file and each time the case LOOKED like coverage: an assertion that passes for two different reasons is not an assertion. `expect` asking "does this rule appear" rather than "which rules fired"; the scoped-pragma case leaning on a rule that fired anyway; this one reading a clean exit that both branches produce. All three were found by review or by mutation, none by reading the case. 41 cases green, make check exit 0. Refs backend#1788. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* sec(2157): make "no unscoped App-token mint" a checkable property
The 19 add-to-kanban copies are scoped and the audit is clean (run 32322964366,
0 findings). Nothing stops the twentieth mint arriving the way the first nineteen
did -- nine workflows acquired a full-permission mint in one day because the first
one did and the rest were copied from it. Least privilege applied by hand is a
STATE; this is the part that makes it a property.
WHAT THE GUARD DECIDES, AND WHAT IT REFUSES TO GUESS. A mint with no
`permission-*` input carries the App's FULL installation grant -- contents +
pull-requests + issues + org-projects write across every installed repo, and the
App holds `bypass_reviews` on staging and prod fleet-wide, so the blast radius is
merge-past-review rather than merely write. Whether the scopes a workflow DOES
name are the right ones is a question only its own API calls can answer, and a
guard that claimed to know would be restating rather than deriving. "Asked for
nothing at all" is decidable, and is what was actually going wrong.
DERIVED, NOT RESTATED. The workflow list is the directory; mint steps are found by
parsing YAML and comparing the `uses:` value up to the `@`. Add a workflow and it
is covered. Bump the action's pin and it is still checked -- there is a test and a
mutation for exactly that, because an exact-string match including the version is
the obvious wrong implementation.
FAIL CLOSED, INCLUDING ON ZERO. An unparseable workflow, a missing directory and
ZERO MINTS FOUND are all hard errors. The last one matters most: this file's
premise is that mints exist, so finding none means the matcher broke, not that the
fleet got clean.
ARMED GREEN OVER 12 PRE-EXISTING OFFENDERS. Each is exempted BY NAME WITH A
REASON, so this lands green instead of as a red gate that trains people to skip
the tier. `fr-gate` is a required check on every promotion branch fleet-wide and
three others read branch protection, where a narrower token is known to return
LESS data rather than an error -- that is why caller-drift keeps a PAT at all
(ruleset `bypass_actors` is returned only to a write-access caller). Downscoping
those needs measurement per workflow; it is not a thing to guess at across 13 files
in one commit.
AND A STALE EXEMPTION IS ALSO A FINDING. Without that half the list stops being a
burn-down and becomes cover: a full-grant mint re-introduced into an exempted file
would be admitted by a row written years earlier for a different reason.
13 cases. 8 mutations, each verified applied AND caught. TWO OF THOSE MUTATIONS
WERE UNCAUGHT ON THE FIRST RUN, and the run is how I found out rather than review:
* "a missing `with:` counts as scoped" survived the whole suite, because every
fixture happened to supply a `with:` block;
* "the action match becomes a substring test" survived, because the look-alike
fixture (`evil/actions-create-github-app-token`) does not contain the real
name -- the hyphen breaks it -- so it could not distinguish `in` from
`split("@")[0] ==`. Replaced with `myorg/actions/create-github-app-token`,
which a substring test does wrongly match.
Both gaps are now cases. Also: two suite cases initially failed by measuring
PRODUCTION state -- without an explicit exemption override the live 12 rows all
read as stale against a fixture directory. That failure was the stale-exemption
check working on the wrong input, and the helper now says so where someone would
otherwise repeat it.
ALSO IN THIS COMMIT: repo-inventory.yml's `divergent` reason for release-train's
add-to-kanban copy said its header "is now the ONLY difference". That was false in
two ways -- the copy also lacked the least-privilege scoping until
release-train#93, and it carries a second wanted comment (the Dependabot note) the
reason never mentioned. Rewritten to name both, and to state the claim in a form
someone can check: strip every comment and blank line from that copy and from the
canonical one and they are byte-identical.
make check green: lint (incl. the new gate), 13 mint-scope cases, all selftests,
coverage gate, actionlint.
Refs backend#2157.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(2157): put the guard in a REQUIRED context, not just `make lint`
Bugbot, High, and it is the failure mode this repo's own rule 2 names -- shipped in
the PR that added the guard.
`mint-scope` was wired only into `make lint`, which CI does not run. The required
contexts on develop are actionlint, gate, quality/{gitleaks,house-rules,ruff,
shellcheck,action-pins} and selftests -- and `selftests` runs the FIXTURE suite,
never the production audit. So a new unscoped `create-github-app-token` mint could
merge with everything green. A guard in a non-required job is advice; this one was
not even in a job.
`action-pins` was cited in the code as the peer that asks a structurally identical
question, and Bugbot's sharpest point is that action-pins IS enforced in CI, which
is exactly what made the comparison misleading rather than merely incomplete.
ADDED TO THE EXISTING `selftests` JOB rather than as a new context, and that is the
deliberate half: `selftests` is already required on develop/staging/main, so this
arms the gate with NO branch-protection edit. A new context would sit unrequired
until somebody armed it -- which is precisely how a guard ends up advisory, and
"never land a red gate" is the other half of the same rule.
BOTH HALVES NOW RUN IN THAT CONTEXT, and they answer different questions:
make selftests the rule CATCHES 13 fixture cases, 8 mutations
make mint-scope the fleet COMPLIES 13 mints, 12 exempted, 0 findings
Either can pass while the other fails, in both directions, so neither substitutes
for the other. The Makefile's CI-job map now carries that line, since that map is
what a reader uses to conclude a local pass predicts CI.
make check green.
Refs backend#2157.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(2157): my conflict resolution kept a schema key #288 had removed
Bugbot High, and CI agreed: `audit` and `gate` both red. The conformance audit
could not load the contract at all -- exit 2 before auditing anything -- so this PR
was disabling the guard it exists to add.
WHAT I DID WRONG. Resolving this branch against develop, I took OURS wholesale for
repo-inventory.yml because our side had the corrected `divergent` reason for
release-train's add-to-kanban copy. But develop's side also carried a STRUCTURAL
change from #288: `stale-backlog.yml` moved from top-level `copies:` to
`reusables:`. Keeping ours preserved a `repos.release-train.copies.stale-backlog.yml`
key whose family no longer lists that file, and caller-drift.py rejects any extra
copies key as a schema error.
"Keep ours" is only ever right about the hunk you looked at. I read the reason text,
saw it was the version I wanted, and did not ask what else that side of the file had
stopped saying.
THE FIX is the narrow one: drop the orphaned `copies:` entry. release-train's
stale-backlog exemption already lives under `callers:` on develop, with the reason
#288 rewrote -- the one that says the original basis (column-blindness) no longer
holds and the exemption is retained as an UNDECIDED question rather than a
justification. Bugbot's second sentence is what pointed at that: the callers cell
still referenced the newer anchor, so the reason I was preserving applied to nothing.
The corrected `divergent` reason for add-to-kanban.yml -- the part I actually wanted
from our side -- is untouched and still says the checkable thing: strip every comment
and blank line from both copies and they are byte-identical.
Verified: the inventory loads, `make check` exit 0, mint-scope 12 exempted 0
findings, 41 house-rules cases, 13 mint-scope cases.
Refs backend#2157.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>…it is (backend#2264) (#296) The #2264 fleet sweep reported exactly one offender in this repo: house-rules-selftest.sh:474 NF=$(printf '%s' "$OUT" | grep -oE 'across [0-9]+ file' | ... | head -1) It is not a hazard. This file runs under `set -uo pipefail` (line 37) with NO errexit, so a 141 could not abort anything -- and two `grep -oE` filters over one line of output could not SIGPIPE in the first place. The scanner reports it because it reads the `set -euo pipefail` lines inside this suite's QUOTED FIXTURE SCRIPTS -- whole scripts passed as multi-line arguments to `expect` -- as this file's own options. That limitation is documented and pinned by a test in tracebloc/client (client#777); tracking quoted regions by counting apostrophes was tried and rejected, because prose apostrophes desynchronise the count and a desynchronised count HIDES real offenders. So the marker carries the reason, not just a silencer -- and this repo now reads clean for when the shared gate is armed. Verified: the selftest suite still passes (41 passed, 0 failed); shellcheck and bash -n clean; the scanner reports zero for the repo. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
First tier of backend#2157, taken on the two workflows whose requirement is DERIVABLE from what they run and whose result is VERIFIABLE by dispatch -- not on the ones whose scopes are still unmeasured. kanban-columns runs `kanban-columns-check.py`, which READS the project field vocabulary and writes nothing: `organization-projects: read` is the whole requirement. kanban-archive mutation surface is exactly `archiveProjectV2Item`: `organization-projects: write`, no issues, no pull-requests, no contents. Both previously minted the App FULL install -- contents+PR write across all 19 repos -- for a read of one project field list and one archive call. `repositories:` narrows the repo-level half. It does not touch the board: `organization_projects` is an ORG permission, measured unaffected by repo scoping in backend#2181 run 32255581084. That was the load-bearing unknown and it is already settled, so this does not re-litigate it. Both exemption rows come out of mint-scope.py in the same commit, so the guard now reports 10 unscoped rather than 12 and has no exemption for a workflow that no longer needs one -- a stale exemption is a licence nobody is using, which the guard itself reports. Deliberately NOT in this PR: the six caller-triggered board writers and the three protection readers. A narrower token on a protection read returns LESS DATA rather than erroring, so those need their own measurement window, not a plausible-looking guess. backend#2157 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…end#1979) (#297) `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>
…#2243) (#298) `caller-drift.py` reported two stale exemptions on .github#292's audit, and both are real -- the caller exists in the live org while the inventory still claims it is absent: claude-skills advance-deploy-env.yml -> caller advance-deploy-env.yml release-train fr-pass-comment.yml -> caller fr-pass-comment-caller.yml Verified each against the live org rather than trusting the report: `gh api repos/<r>/contents/.github/workflows` lists both callers. Promoted to `required` with the caller filename in the trailing comment, which is the remediation the checker itself prescribes and the convention every other tracked entry in this file already uses. This is the failure mode .github#295 names as a Medium -- "an exemption can outlive its reason while the check stays green" -- except here a different checker caught it, so it was loud rather than silent. The two findings blocked the staging promotion mirror (#292) at `release`, which is how they surfaced. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…does not exist (#295) * fix(2243): one branch to Status mapping, and it surfaced a column that does not exist Two workflows decide a card Status from the branch a merge landed on, and they held THREE copies of the rule between them: advance-deploy-env had one and READ the per-repo `.kanban.yml` override; kanban-closure-router had two and IGNORED it. Both write Status, and a PR merged to develop fires both -- the router on pull_request closed, advance on the push -- so with a `.kanban.yml` present they would write DIFFERENT statuses for the same merge and run ordering decided which stuck. The router could not have honoured the override even in principle: it never checks the caller out. That is why the fix is one shared mapping rather than a second copy of the yq read -- the override has to be FETCHED. WHAT THIS FOUND, which is the part worth reading. `Staging (human review)` was an accepted override value in advance-deploy-env and is NOT a column on the board -- measured against project #2, whose Status options are Backlog, North Stars, Ready, In progress, Code review, On dev, Staging (agent review), FR on staging, Ready for prod, Prod, Done, Cancelled. A repo that had used it would have had its write rejected for naming a column that does not exist. Nothing caught it because the vocabulary lived in a shell `case` that kanban-columns-check.py never read. So kanban-columns-check now IMPORTS the mapping instead of regex-scraping two workflows for it. That is strictly stronger -- it reads the data structure rather than a rendering of it -- and it SHRINKS the regex surface rather than growing it. Both rewired workflows come out of WRITERS with the reason stated, since their literals are legitimately gone. The fold happens in `written_names`, not in `main`: the selftest substitutes `written_names` to control its input against a fake board, and folding into main silently widened what the selftest could not see. Caught by that selftest going red. A GUARD SO THE MAPPING CANNOT BE RE-COPIED. Its first version matched any `STATUS="On dev"` and flagged three innocent sites -- a sibling-merge holding state and two no-base-ref floors. Those are policy defaults for cases where there IS no branch, not copies of the mapping, and a guard that cannot tell the difference gets argued with and then switched off. Narrowed to a branch-keyed case arm, with both directions asserted. 22 selftest cases, 10/10 kanban-columns, all 10 .github selftests green, ruff clean, actionlint clean, make check green. backend#2243 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(2243): five findings, and two of them defeated the feature entirely Bugbot on .github#295. Two Highs, and both meant the override still never applied -- so the PR would have shipped a refactor with the bug intact. HIGH 1 -- PyYAML is not on the runner. `read_override` imported `yaml` lazily and treated ImportError exactly like a parse miss: a warning and an empty override. Neither rewired workflow installs it, so a real `.kanban.yml` would have been silently ignored and both writers would have kept the defaults -- failing open in the one environment that matters. Parsed with `yq` now, which is what advance-deploy-env used before this refactor, so it is a dependency this path already had. A present file with no parser REFUSES rather than defaulting: an unread override is not "no override". HIGH 2 -- the router `case` clobbered any non-stock override. Both sites called the mapper and then a `case` over the four stock branch names overwrote ANY other result with the On dev floor. So a `.kanban.yml` key for a branch outside those four was computed and discarded on the same run -- including the rfcs override this change exists to unblock. Keyed on whether the mapper ANSWERED now, which also removes the last branch-name list from that workflow. MEDIUM -- kanban-reconcile held the second copy, justified by "identical, deliberately" (.github#126). backend#2243 made that justification unsafe: once the router honours an override, an identical copy is no longer identical, and the weekly sweep would move an overridden card BACK onto a deploy column -- the slower one silently undoing the faster one, exactly as that comment predicted, through the mechanism it was written to prevent. It calls the mapper now; the Status-name to option-id case that remains is a different relation, not a second copy of the rule. MEDIUM -- a mapping-only PR never ran the board-name check, because `branch_status_map.py` was not in kanban-columns.yml `paths:` even though the check now imports it. Added, with its own selftest assertion: WRITERS does not name it (it is not a workflow), so the existing WRITERS-vs-paths case could never have covered it. MEDIUM -- I dropped kanban-closure-router from WRITERS on the claim its literals were gone. Only its BRANCH MAPPING moved; it still writes Cancelled, Done and On dev, and Cancelled/Done are not in the imported mapping -- so the conformance check could have gone green while those two named columns the board no longer has. Restored, with the reason. advance-deploy-env stays out because it now writes no literal at all, which is checkable rather than asserted. Also fixed a shell syntax break of my own: replacing the `case` heads left their arms and `esac` behind. actionlint caught it. 11/11 kanban-columns, 22/22 branch-status-map, all 10 .github selftests green, ruff and actionlint clean. backend#2243 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(2243): two Highs, both mine, both the same defect through a different door Bugbot on .github#295 round two. HIGH -- reconcile had no checkout. My own note said "branch_status_map.py is local: this job only ever runs in tracebloc/.github" -- true of the REPOSITORY and false of the WORKSPACE. A hosted runner starts empty, so the stage-derivation step would have failed on a missing file and taken the weekly backstop down mid-sweep. A comment that is confidently right about the wrong noun. HIGH -- read_override failed open on a fetch failure. It caught every `gh api` error and returned an empty map, identical to "no .kanban.yml". So a present override behind a 403, a 5xx or a rate limit was silently ignored and both writers applied the defaults: the exact override-ignore defect this change exists to close, reached by a different door. The parse and missing-`yq` paths already refused; this one did not. 404 is now the only failure that means "no override", matched on the message the way promote-repo.sh does it since `gh` exits non-zero for both. Everything else refuses. Four cases, and they assert `SystemExit` specifically rather than "it raised" -- a different exception would mean a different path, which is the rule this repo has on bare assertRaises. One of those cases failed on its first run for a reason worth keeping: the loop set the stub AFTER the call, so the 403 iteration ran against the 404 stub from the case above and reported a false failure. The test caught its own off-by-one. 26/26 branch-status-map, all 10 .github selftests green, ruff and actionlint clean. backend#2243 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(2243): the override was read from the wrong ref, and reconcile could not survive it Bugbot on .github#295, two Highs. HIGH -- `read_override` defaulted to the API HEAD, i.e. the repo DEFAULT branch, and no caller passed a ref. So an override present on `develop` but not yet on `main` was silently ignored by both writers. And that is a REGRESSION rather than a new limitation: advance-deploy-env previously read `.kanban.yml` off the checked-out PUSHED branch. My own docstring argued for the default on the grounds that a closed PR head may be gone -- true, and irrelevant, because every branch these callers map is a BASE branch that persists. Right reasoning, wrong input. All four call sites pass the ref now, and the selftest asserts it by reading the call sites: the failure is a MISSING argument, which no unit call can show. Verified non-vacuous by dropping one ref and watching it redden. HIGH -- the mapper call could take the weekly sweep down. Making read_override REFUSE on an unreadable `.kanban.yml` was right for a single card and wrong inside a loop: a bare command substitution under `set -euo pipefail` let ONE unreadable repo truncate the whole backstop mid-sweep, while the sibling closer lookups beside it already soft-fail. It soft-fails now and SKIPS the item rather than defaulting. Falling back to the default mapping would be the fail-open this PR removed, one layer out -- the router already moved that card correctly, and this sweep exists to fix misses, not to overrule with a guess. 33/33 branch-status-map, all 10 .github selftests green, actionlint and ruff clean. backend#2243 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(2243): the one arm that was silently empty while the docstring said otherwise Bugbot Medium, and saadqbal made the argument better than Bugbot did: this is not merely an inconsistency with the other arms, it CONTRADICTS the contract documented two lines above it. The docstring claimed "a malformed one is also empty, and SAYS SO on stderr". True of a YAML/JSON parse failure, which raises with ::error::. False of a `branch_status_map` that parses fine and is a list: no message, no exit code, {} returned. So the file had three refuse-loudly paths, one legitimate empty, and one that was silently empty while the docs said it announced itself. A documented contract that one path quietly breaks is worse than an undocumented one, because the next reader trusts it -- and I was that reader, twice, on this PR. The arm refuses now, and the docstring is a TABLE of all five outcomes rather than prose that drifted from three of them. The cases assert the five as a SET, not one at a time, because the defect was an inconsistency BETWEEN them. Plus the inverse -- a real mapping still comes back -- so the guard cannot degenerate into "refuse anything". 37/37, all 10 .github selftests green, ruff clean. backend#2243 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(2243): WRITERS was wrong twice, so it is checked now instead of trusted saadqbal on .github#295, and his framing is stronger than Bugbot Medium. Bugbot said a NEW literal in advance-deploy-env.yml would slip past. The fact is thirteen Status names are in that file RIGHT NOW, in `rank()`, and dropping it from WRITERS took all thirteen outside the check. The file documents its own failure mode: an unknown Status returns "" from rank(), the guard falls through to strict equality, and the card then BLOCKS every prod promotion carrying it. A read-side name that does not resolve breaks at least as loudly as a write-side one -- so "WRITERS is a write-side name" was technically defensible and wrong about what the check is for. That was the SECOND wrong removal on this PR; the router was the first. And the naive fix would have failed the check, which is worth knowing before someone tries it: twelve of the thirteen are live board options, and the thirteenth is `Staging (human review)` -- the phantom column this PR own headline finding identified. So, in order: 1. RETIRE THE SHIM. It ranked alongside `FR on staging` to keep the monotonic guard stable across the #1592 rename INSTANT, and that instant has passed -- measured against project #2. Keeping a shim for a completed rename is how this same file came to accept an override naming a nonexistent column. 2. Restore the file to WRITERS, where its remaining twelve get checked. Two hand-removals, two errors, is enough: `unlisted_namers()` derives against the tuple now. Any workflow naming a board column on a CODE line while absent from WRITERS is a finding, with a two-row exemption list carrying reasons. It found two on its first run, which is the argument for it existing: kanban-archive.yml:104 selects the three terminal columns to archive, and wip-limit-check.yml:47 defaults its column input to `Code review`. Neither writes a Status; neither name was checked. A rename would have left the archiver archiving nothing and the WIP check counting an empty column -- both indistinguishable from a quiet board. Three things the tests caught while doing this, all mine: the guard fired inside the stubbed-board cases and short-circuited main() (stubbed in run(), like cross_check, with the reason written down); my own cases got that stub because run() patches permanently, so they use kcc_fresh as the cross_check cases already did; and the paths-filter regex matched only CONSECUTIVE list items, so my interleaved comment truncated the block and produced a false "uncovered" that took two rounds to read as a parser artefact. 16/16, all 10 selftests green, ruff and actionlint clean. backend#2243 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(2243): restoring the file to WRITERS did not cover its rank() names Bugbot High + Medium on .github#295, and both are about claims I wrote one commit earlier. HIGH -- putting advance-deploy-env.yml back in WRITERS covered NONE of rank()s twelve names. `LITERAL` matches an assignment idiom and a `case` arm has no `=`; WRITERS membership also makes `unlisted_namers` SKIP the file. So all twelve stayed invisible -- and `Backlog`, `North Stars` and `Ready` were collected from nowhere at all -- while the comment I had just added claimed they were covered. Measured, not argued: 12 of 12 unattributed before, 0 after. `written_names` now collects every board column a WRITERS file NAMES, not only the ones it assigns, through one `names_in` helper that `unlisted_namers` also uses. Two copies of "does this file name a column" is the drift this file keeps finding in itself. MEDIUM -- the stale-exemption check the docstring promised did not exist. `unlisted_namers` said stale entries "are reported by the caller for the same reason mint-scope.py reports its own", and main() never diffed them. It does now, with three expiry conditions asserted separately because they are different facts: the file is gone, it joined WRITERS, or it names no column any more. Plus the inverse, so a live exemption is not a permanent red. The exempt dict moved to module level so both functions read ONE copy. AND FIXING THE HIGH NEARLY CREATED ITS TWIN. The staleness guard tested `if not found`, and `found` now has three contributors -- so case-arm names alone could have satisfied it while `LITERAL` matched nothing: the guard disarmed by the very change that broadened the collection. It is keyed on the assignment pass now, and I broke the pattern to watch it fire. 7 cases (23/23), all 10 selftests green, make check 41/41, ruff clean. backend#2243 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(2243): a refused override must not cost the router its write Bugbot High on .github#295, and it is NEW -- the old in-file `case` could not fail this way. `read_override` refuses on an unreadable `.kanban.yml`, and a bare command substitution under `set -euo pipefail` then exits the target step before any Status is published. The update step non-empty guard skips, and the project built-in "Item closed" automation sets `Cancelled` and archives shipped-via-parent work -- the .github#157 no-write path this same file documents, reached by a door I opened. And it fires exactly when the first repo adopts the override this change exists to unblock. THE POLICY DIFFERS BY CALLER, and that is the substance rather than a detail. reconcile SKIPS the item: it is a backstop that fixes misses, so writing a guessed column would overrule a router that already got it right. The router writes the DEFAULT: publishing nothing is strictly worse than ignoring an override for one run, because one is recoverable and the other leaves a card Cancelled. So `read_override` still refuses, and `--no-override` is how a caller asks for the answer it can safely fall back to. Neither caller gets a silent default -- the router logs a ::warning:: naming the repo, the branch and the reason. The fallback is proven to consult NOTHING by running it with `gh` removed from PATH, and the same call WITHOUT the flag is asserted to refuse -- so the flag is doing the work rather than a silent default. The call-site assertion failed on its own first run and the fix is the interesting part: it counted the new fallback calls and demanded a ref of them. A `--no-override` call consults no file by definition, so demanding a ref would demand the opposite of its purpose. It now distinguishes the two shapes, and additionally asserts the router has a fallback in BOTH arms while reconcile has NONE -- the per-caller policy, pinned. 47/47, all 10 selftests green, make check 41/41, actionlint and ruff clean. backend#2243 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(2243): refusing has to be a WRITE where another system supplies the default saadqbal on .github#295, correcting my own fix from one commit ago -- and the qualifier he adds is the part worth keeping: "Refuse rather than guess" assumes that doing nothing is safe. At a decision point whose default is supplied by ANOTHER system, refusing has to be expressed AS A WRITE, not as an absence. My first fix wrote the DEFAULT MAPPING on an unreadable override. That denies the built-in Item-closed automation its opening, which was the point -- but it claims a promotion happened on a read that failed, which is exactly the "silently apply the default mapping" this PR removed everywhere else. The router writes the non-terminal HOLDING STATE now (`On dev`) and labels the card `override-unreadable` for the weekly pass, modelled on the sibling-merge arm three lines down, which keeps its Status write for precisely this reason. The holding state claims nothing: it is an explicit WE COULD NOT TELL. And it is still a write, because the built-in automation acts on the close independently of this workflow -- a loud red run does not protect the card, the automation wins the race. `--no-override` is gone from the router. It stays in the CLI because the flag itself is right and tested; nothing calls it today, which the selftest now asserts rather than leaving to be discovered. THE THREE CONSUMERS NO-WRITE PATHS ARE PINNED AS A SET, because that is what the finding is about -- three callers, three different correct answers, each for a stated reason: router writes the holding state + label; another system would decide reconcile skips the item; it fixes misses, nothing else acts on its silence advance no fallback; a push has no competing automation Reverting one arm to the old default-mapping fallback drops the suite to 48/2, so the distinction is checked rather than described. 50/50, all 10 selftests green, make check 41/41, actionlint clean. backend#2243 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…count (#299) Finishing the cleanup .github#298 deliberately left for the reviewer, which Bugbot then decided on the staging mirror (#292). Both findings are real, counted rather than trusted. ORPHANED ANCHOR DEFINITIONS, REMOVED. An anchor with zero aliases is inert to YAML and dangerous as documentation: the name is still there for the next person to reach for, and reaching for it asserts a stale UNREMEDIATED claim nothing has re-measured. advance_deploy_env_caller_missing 0 aliases. #298 flipped claude-skills to `required` -- verified live, `gh api repos/tracebloc/claude-skills/contents /.github/workflows` lists advance-deploy-env.yml -- which removed the last citation and left the definition behind. set_pr_status_caller_missing 0 aliases, and NEVER had one: `git log -S'*set_pr_status_caller_missing'` is empty back to cd640e8, where the definition was born. Found by sweeping the block rather than reported. Worse than orphaned -- it claims a gap that is false in every repo: set-pr-status.yml is `required` in all 19 entries. STALE COUNT, CORRECTED. `fr_pass_comment_caller_missing` said "5 repos". It has ONE citation (claude-skills) and has only ever been aliased twice -- 2 at cd640e8, 2 at 8a516f2, 1 after #298 -- so the 5 was never this anchor's reach; it was the org-wide sweep's count, worn as if it were the alias count. The new text separates the two, re-measures the one remaining citation (2026-08-21: no fr-pass-comment caller in claude-skills), and points at the grep instead of a hand-maintained list, which is what `code_quality_caller_missing` above already learned to do. rfcs' inline reason cross-referenced "the fr_pass_comment_caller_missing repos" in the plural; it names the one repo now. SWEPT, NOT FIXED. Every other anchor's count claim was checked against its real alias set and is accurate: code_quality (THREE: claude-skills, release-train, rfcs) and not_a_pipeline_prod_branch (the same three) both match exactly. The block header's "Reasons cited by more than one repo" is now false for six single-alias anchors, but reconciling that means either inlining them or softening the block's stated design -- a decision, not a count fix, so it is reported rather than taken here. Verified: 16 shared_reasons, 0 orphans, post-parse on resolved values. A structural diff of the parsed tree against origin/develop shows exactly TWO leaf changes -- claude-skills' and rfcs' fr-pass-comment reason strings -- and every other top-level key byte-identical, so no repo's policy moved. `make check PYTHON=<venv>` green: ruff, shellcheck, house-rules, mint-scope (14 mints, no findings), actionlint 0, 11/11 selftests, caller-drift selftest 197 pass / 0 fail. `make audit` against the live org reports zero caller or exemption findings (it exits 2 on 16 ruleset reads this token cannot see `bypass_actors` for -- pre-existing, token scope, not this diff). Rolls up under backend#2243 per CLAUDE.md: a Bugbot drive-by inside an already-tracked task, not a new one. Mechanical detection of orphaned anchors is .github#295's territory and deliberately not attempted here. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
release-train: develop -> staging
LukasWodka
commented
Aug 21, 2026
ContributorAuthor
bugbot run |
Uh oh!
There was an error while loading. Please reload this page.
…kend#2264) (#300) * feat(code-quality): arm the pipefail early-close gate fleet-wide (backend#2264) `client` has carried the only copy of this rule for a month. This moves it to the shared workflow so every repo is covered by ONE implementation, which is the whole point -- six copies of a scanner is the drift this rule exists to prevent. NO PER-REPO CALLER CHANGES. Every repo already calls code-quality.yml, so the gate arrives as a new job inside it. That also sidesteps caller-drift.py's sequencing constraint entirely: there is no 16-repo wave to land in order. ARMED GREEN, measured, not assumed. Across 18 repos at their default branches: 134 shell files, 68 of them running under errexit+pipefail, 0 offenders. The file counts are reported because "0 offenders" and "scanned nothing" print the same thing -- five repos have no shell at all and exit 0 on "nothing in scope", which is honest rather than vacuous. WHOLE-TREE, and here that is CORRECTNESS not policy. Whether a line is hazardous depends on whether its FILE runs under both options, and a library that sets neither inherits them from its sourcer. A diff-scoped scan resolves inheritance against a partial tree: edit lib/foo.sh without touching its sourcer and the gate calls it safe. `rc=2` (cannot tell) is fatal REGARDLESS of soft-fail. A gate that could not read the tree has not reported a clean tree, and letting soft-fail swallow that is how a gate becomes decoration (#1729 rule 3). Ported from client with its three reviewer-found fixes intact -- `||` is not a pipe, the stand-in must also be a boundary, and `|&` is a pipe. End-to-end proof that the shared copy matches the local one: run against client at the commit BEFORE its #1778 cleanup, it finds exactly the instances that cleanup converted, including the ones inside scripts/lib/*.sh that only inheritance resolution can see. Tests, in this repo's conventions rather than client's bats: scripts/tests/pipefail-early-close-selftest.sh 40 cases, both directions scripts/tests/pipefail-early-close-mutations.py 17 mutations, 0 uncaught wired into SELFTEST_TARGETS / MUTATION_TARGETS, so selftests-cover sees them (13 selftests, 2 mutation runners), and the --dry anchor check joins `lint` The mutation harness earned its place immediately: "the inheritance fixpoint is skipped" came back UNCAUGHT, because my fixture was one level deep and one loop iteration resolves that. Real installers are deeper. Added a transitive two-level case; the mutation is caught now. That is the same fixture-only-covers-what-the-author-imagined trap client#781 is about, caught here by the harness instead of by a reviewer. Follow-up, deliberately NOT in this PR: retire client's local copy once this is on main. Doing it here would leave client ungated in between. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(selftest): the gate flagged its own fixtures — keep them off column 0 CI caught this and my local run could not, which is the interesting part. `git ls-files` enumerates TRACKED files. When I ran `make check` the selftest was still untracked, so the gate never scanned it and reported clean. The commit made it tracked; CI scanned it and found ~20 findings in it. The findings were real, by the rule's own documented limitation: written as multi-line quoted strings, the fixtures put `set -euo pipefail` at COLUMN 0 of this file, and the scanner cannot tell a quoted string from code (client#777, where that limitation is documented and pinned by a test). It therefore read its own test suite as a script enabling errexit and flagged every fixture pipe. Fix: fixtures are one-line printf FORMATS, so no fixture line sits at column 0. `scan_raw` now takes a format rather than a literal, with the reason written at the helper so the next person does not "tidy" them back into heredocs. Verified in the state CI actually runs: - the file is tracked, confirmed with `git ls-files`, so the green run below genuinely scanned it rather than skipping it as before - gate on this repo: 0 findings - selftest 40 passed / 0 failed; mutations 17, 0 stale, 0 uncaught - `make check` green The suite's last case -- "tracebloc/.github is itself clean under the rule" -- was VACUOUS for this file until now, for the same tracked-vs-untracked reason. It is live: mutating the comment-skip rule reddens it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(gate): three fail-open holes, all found by Bugbot, all reproduced first Every one was a way for this gate to report green without having checked. 1. THE SCANNER'S OWN STATUS WAS IGNORED (scripts/pipefail-early-close.sh). The wrapper runs under `set -uo pipefail` WITHOUT errexit -- deliberately, so it can classify and report rather than die -- so a failing `awk` did not stop it, and `[ -n "$out" ]` read a CRASHED scanner as a clean tree. Reproduced by corrupting the awk program: rc was 0, is now 2. 2. SOFT-FAIL SWALLOWED INTEGRITY FAILURES (code-quality.yml). Only rc=2 was forced fatal, so a missing or non-executable script (127), or a signal death, fell through to the soft-fail branch and reported green. Now only 0 and 1 are VERDICTS -- clean and findings -- and anything else is fatal regardless of soft-fail. Whitelisting the verdicts is the load-bearing change; blacklisting rc=2 is what left the hole. 3. THE MUTATION TIER NEVER RAN IN CI (Makefile, selftests.yml). The new runner was in MUTATION_TARGETS and satisfied `selftests-cover` -- which asks make what that list would run -- while `selftests.yml` named ONE MEMBER of the list, `make mutation-house-rules`. Covered on paper, unrun in fact. Fixed twice over. `make mutations` is the list, and CI runs the list, so the next runner is picked up by adding one word to MUTATION_TARGETS. And `selftests-cover` now ALSO asserts the workflow runs both tiers, because the root cause was that nothing checked CI executes what the Makefile declares -- being wired to a target is only half of it. Mutation-proved: pointing the workflow back at one member reddens the guard. Finding 3 is the one worth remembering. It is this repo's own catalogued shape -- a mechanism that looks connected and is not -- inside the coverage guard built to catch exactly that. Also pins finding 1 with a case: "a scanner that CRASHES is exit 2, never a clean tree", driving the real gate with a corrupted scanner. Verified: selftest 41 passed / 0 failed; both mutation runners 0 stale, 0 uncaught (7 + 17); make check green; shellcheck and actionlint clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(gate): the seed missed the split form `set -eu -o pipefail` (Bugbot) The hazardous-file seed required the option to be the FIRST cluster after `set`, so three ordinary spellings never seeded: set -eu -o pipefail missed set -e -o pipefail missed set -o errexit -o pipefail missed house-rules.sh already treats the split form as first-class, so the two disagreed about what "this file enables pipefail" means. The DAMAGE IS CONFINED to the half this wrapper exists for. The awk gets the direct file right from its own positional state machine either way -- but a split-form script's SOURCED LIBRARIES were never marked inherited, which is precisely the case the wrapper was written to cover. A scanner-level test could not have seen it; the new cases drive the wrapper. RE-MEASURED THE CLAIM I ALREADY MADE. No repo in the fleet uses a split form today, so the "18 repos, 0 offenders" figure in this PR is unaffected -- I re-ran it with the fixed seed and it is still 0. Recording that explicitly because the honest answer to "did your green measurement miss something" is a measurement, not a reassurance. "No instance today" is not a property, which is why it is fixed rather than noted. Fix: allow the cluster anywhere on the line (`.*` before it). The SIGN check is unchanged and still load-bearing -- the `-` is required, so `set +o pipefail` cannot satisfy it. Verified across all seven spellings: four on-forms seed, `+o pipefail` / `-uo pipefail` / `-eu` do not. Three selftest cases, one per split spelling, driving the INHERITANCE path. A mutation pins it: restoring the first-cluster anchor reddens exactly those. 44 selftest cases, 0 failed. Mutation tier 7 + 18, 0 stale, 0 uncaught. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(make): check-all and lint named a list MEMBER too (Bugbot, 2nd time) Same defect as the CI one, in the same PR, found after I fixed the CI half: check-all: ... mutation-house-rules -> skipped the new runner lint: ... mutation-house-rules-dry -> same, on the dry tier I pointed `selftests.yml` at `make mutations` and left both Makefile entry points naming one member of MUTATION_TARGETS. That is precisely the paired-construct shape client#781 encodes -- change one half of something that must move together and the other half is now a bug -- committed by me one day after writing the rule, and for the THIRD time this week. Fixed properly rather than pointwise: - `mutations` and `mutations-dry` are the only sanctioned entry points, and every consumer (CI, check-all, lint) depends on one of them. - `mutations-dry` is DERIVED, `$(addsuffix -dry,$(MUTATION_TARGETS))`. A hand-written second list is the same drift one level down. - `selftests-cover` now REFUSES a Makefile where check-all or lint names an individual runner, with the reason inline. The guard is what stops the fourth occurrence; my own attention plainly does not. Mutation-proved both arms: restoring `mutation-house-rules` in check-all, and `mutation-house-rules-dry` in lint, each redden the guard. Adding the next runner is now one word in MUTATION_TARGETS and it cannot be half-wired -- CI, check-all and lint all pick it up, and the guard fails if anyone reintroduces a member reference. make lint / make check green; 44 selftest cases; mutation tier 7 + 18, 0 stale, 0 uncaught. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(gate): a basename-only `source "worker.sh"` kept its quote (Bugbot) The inheritance extractor ran `s|.*/||; s|^.*[[:space:]]||`. On a quoted target with a PATH that works, because stripping through the last `/` takes the opening quote with it. On a quoted BASENAME there is no slash, so the quote survived, the basename compare never matched, and the library was never marked inherited. Measured before fixing: source "${LIB_DIR}/worker.sh" -> worker.sh ok source "worker.sh" -> "worker.sh BROKEN . "worker.sh" -> "worker.sh BROKEN source worker.sh -> worker.sh ok So the form that worked did so BY ACCIDENT, which is why the gap survived: the case I tested with was the one where an unrelated substitution happened to clean up after the missing one. Fix is `s|^"||`. Four spellings now asserted through the real gate -- quoted-with-path, quoted-basename, bare-with-path, and `.` in place of `source` -- and a mutation removing the new substitution reddens them. Re-measured the PR's headline claim again with the fixed extractor: 18 repos, still 0 offenders, no rc>1. Every fix in this PR that could widen what the gate sees gets the fleet sweep re-run, because "the number was 0 before" stops being evidence the moment the scanner's reach changes. 48 selftest cases, 0 failed. Mutation tier 7 + 19, 0 stale, 0 uncaught. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(gate): close Asad's three remaining items, and a bash 3.2 regression FROM THE REVIEW --------------- 1. SOURCE EXTRACTION missed `source "$(dirname "$0")/lib.sh"` and `source "${DIR}"/file.sh` -- fail-OPEN, in the half this wrapper exists for. An embedded quote ended the match early. Quotes are now STRIPPED before extraction rather than tolerated inside the pattern, and `(`/`)` are excluded from the token so a `$(dirname …)` prefix ends it. Six spellings asserted through the real gate. 2. `haz` WAS A SPACE-SEPARATED STRING iterated unquoted, so a path containing a space split into two nonexistent paths and the file was never marked hazardous. Now an array. Zero such paths fleet-wide -- and after the split form, "no instance today" is not a reason to leave it. 3. THE SEED SPANNED A TRAILING COMMENT (`set -e # …-o pipefail…` seeded). Fail-closed, so never dangerous, but the awk already strips comments and the two halves of one rule should agree deliberately. Both strip now. AND ONE THE REVIEW COULD NOT HAVE SEEN -------------------------------------- Converting `haz` to an array introduced a portability regression that CI WOULD HAVE PASSED. bash 3.2 -- still /bin/bash on every macOS -- treats an empty array's `[@]` as unbound under `set -u`: scripts/pipefail-early-close.sh: line 168: haz[@]: unbound variable The runners are bash 5, where it is fine. So the suite would have been green in CI and broken for every developer running it locally, on exactly the trees where nothing is hazardous. Expansions are now `${haz[@]+"${haz[@]}"}`. Caught only because this suite gets run on macOS before pushing. MUTATION HARNESS DID ITS JOB TWICE The seed rewrite left two anchors stale and made one mutation inert -- the quote-tolerance mutation could not redden anything once the sed strips quotes first. Anchors are now generated FROM the file rather than retyped, the inert one is replaced by removing the strip itself, and the two duplicates are gone. 52 selftest cases, 0 failed. Mutation tier 7 + 20, 0 stale, 0 uncaught. Fleet re-measured after both widenings: 18 repos, 0 offenders, no rc>1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(selftest): the early-close suite piped into grep -q, eight times (Bugbot) The test suite for the early-close rule committed the early-close hazard, in its own assertions: if [ "$RC" = 1 ] && printf '%s' "$OUT" | grep -q 'worker.sh'; then This file runs `set -uo pipefail`. On a large enough $OUT the `grep -q` closes early, the producer takes SIGPIPE, the pipeline returns 141 -- and a real match reads as ABSENT, so the assertion passes or fails for the wrong reason. Eight of them, including a negated form where the inversion makes it worse. house-rules-selftest.sh already used here-strings for exactly this reason, so the two sibling suites disagreed about the rule one of them enforces. Note the gate did NOT flag this file, correctly: no errexit, so no abort is possible. The hazard here is a WRONG ANSWER rather than a dead script, which is the failure mode the rule's prose has always described and the scanner cannot see. Worth stating plainly -- "the gate is green" was never the same claim as "the file is right". All eight are here-strings now, and the reason is in the header so the next person does not reintroduce them. 52 selftest cases, 0 failed. Mutation tier 7 + 20, 0 stale, 0 uncaught. make check green; shellcheck clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(gate): a set-line comment could disarm the option state (Bugbot + Asad) `apply_set` ran on the RAW line, so whitespace-splitting turned comment tokens into real option updates: set -euo pipefail # note: +e would be bad -> errexit CLEARED set -euo pipefail # never +o pipefail here -> pipefail CLEARED Every hazard below such a line was then skipped. Fail-open on the positional state machine, and silent. Found independently by Bugbot and by Asad. AND IT RUNS BOTH WAYS, WITH A LIVE INSTANCE IN THE FLEET. e2e-test-agent's scripts/federated/04_ingest_fixtures.sh line 17 reads: set -uo pipefail # NOT -e — a per-modality failure must not abort the rest. The comment says NOT -e. The old scanner read that `-e` as errexit ON. Demonstrated by appending a hazard line to a copy: old awk 1 finding, new awk 0. It never mattered only because that file happens to contain no early-close line -- the mechanism was live in the tree the whole time. Fix is one `sub()` at the top of `apply_set`, where the hole is, rather than at the dispatch: the `^[[:space:]]*#` skip only handles WHOLE-line comments, and the existing strip is for function-body detection. Unconditional is safe -- the only legitimate `#` on a set line is inside a positional argument (`set -- "a#b"`), and the loop ignores tokens not starting with `-`/`+`. I ALSO HAD TO CORRECT A FALSE CLAIM OF MY OWN. The wrapper's comment said the seed strips comments "because the awk already strips comments before deciding". That was not true, and it was quoted back approvingly in review before anyone checked it. A comment asserting what ANOTHER file does is a claim to verify, not to quote -- which is the entire subject of this rule family. The comment now says what the code does and records why it was wrong. Three cases, per the pattern on this PR: `+e` in a comment does not disarm, `+o pipefail` in a comment does not disarm, and -- the discrimination -- a REAL `set +e` still does, so the first two cannot pass under a scanner that just ignores every `+`. Mutation-proved: removing the `sub()` reddens exactly those. 55 selftest cases, 0 failed. Mutation tier 7 + 21, 0 stale, 0 uncaught. Fleet re-measured after this widening: 18 repos, 0 offenders, no rc>1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(gate): a discarded status covers its own pipeline, not the whole line (Bugbot) The `|| true` spare matched anywhere on the line and `next`ed out of it, so a live hazard sharing the line was skipped: foo || true && producer | grep -q x -> missed The `;` form was already caught (the old terminator class did not admit `;`), so the live half of this was the `&&` form. Lines are now split on `;` and `&&` and each segment judged on its own: spared only if IT ends in `|| true` / `|| :`, flagged if IT holds the hazard. THE MUTATION HARNESS CAUGHT WHAT THE REFACTOR BROKE, and this is the part worth recording. Splitting on `&&` SUBSUMED two existing cases -- the `||`-span tests both used `&&`, so after segmentation the span could no longer occur and their mutations came back UNCAUGHT. Two tests that had been load-bearing since client#777 were made vacuous by a change three files away, and nothing but the harness would have said so. Re-pinned with the same span inside ONE segment. Also needed a real discriminator for the END-ANCHOR on the spare, since segmentation alone does not imply it: printf %s "$(get || true)" | grep -q needle `|| true` there discards the status of `get`, not of the `grep -q` pipeline that follows. Anchored: flagged. Unanchored: skipped. That case is now the pin. Six cases added: the two live forms, two "the real idiom is still spared" discriminations, the both-segments-spared case, and the anchor discriminator. 63 selftest cases, 0 failed. Mutation tier 7 + 21, 0 stale, 0 uncaught. Fleet re-measured after this widening: 18 repos, 0 offenders. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(gate): pin Asad's table, including the false positive we avoided His review measured four spellings and recommended the cheaper fix (an end-anchored regex) with its cost named honestly: producer | head -1 || true; echo done would be a FALSE POSITIVE The segment-wise evaluation already shipped is the version he described as "more work" and said to reach for only if that false positive bit someone. It did not need to bite: all seven rows of his table now read correctly, including the one the cheaper fix would have cost. rm -f x || true; producer | head -1 FLAGGED rm -f x || true ; producer | head -1 FLAGGED rm -f x || true && producer | head -1 FLAGGED rm -f x || : ; producer | head -1 FLAGGED producer | head -1 || true spared ( producer | head -1 || true ) spared producer | head -1 || true; echo done spared <- no precision loss His diagnosis is the part worth keeping: `([[:space:]]|$|\))` was a HALF-IMPLEMENTED boundary -- it refused `true;` because `;` is not in the class, but accepted `true ` followed by anything, and a space is not a boundary between commands. So whitespace before the `;` decided the verdict, and the spelling people actually write was the one that failed open. Same shape as the `\001` stand-in finding three rounds earlier, at the other end of the same line. Four cases added: the two whitespace spellings, and the two spare-forms that would have regressed under the cheaper fix -- the latter recorded as the REASON this approach was chosen, so nobody simplifies it back. 67 selftest cases, 0 failed. Mutation tier 7 + 21, 0 stale, 0 uncaught. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(gate): a multi-line function opener skipped its own body (Bugbot) f() { producer | head -1 more_stuff } matched the function-opener pattern, failed the self-closing test, recorded the scope and `next`ed -- skipping the hazard on that very line. Fail-open. THIS IS THE SAME DEFECT AS THE ONE-LINER CASE THE COMMENT ABOVE IT DOCUMENTS. Bugbot #763 removed an UNCONDITIONAL `next` from this branch so one-line helpers would be scanned. The conditional `next` on the other arm was left in place, and it has the identical consequence for a multi-line opener that carries code. Changing half of a paired construct -- fourth time this week, and the third on this PR. The rule I wrote in client#781 keeps being right about me. Fix: record the scope and FALL THROUGH. Two cases, because the fall-through must not cost what the branch exists for: - a hazard ON a multi-line opener is flagged - function SCOPING still ends at the closing brace: `set -e` inside f() does not leak to the line after `}` (asserts exactly one finding, at line 5) Mutation-proved: restoring the `next` reddens the first, and only the first. 69 selftest cases, 0 failed. Mutation tier 7 + 22, 0 stale, 0 uncaught. Fleet re-measured after this widening: 18 repos, 0 offenders. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(gate): strip trailing comments quote-aware, in ONE place (Bugbot) The segment/hazard path ran on the RAW line, so two false positives: producer | head -1 || true # explains why FLAGGED (spare broken) do_thing # NOT producer | head -1, see above FLAGGED (prose as code) `apply_set` already stripped comments; this path did not. Same asymmetry, third occurrence on this PR. A REGEX IS THE WRONG FIX HERE, and measuring showed it. `sub(/[[:space:]]*#.*$/, ...)` also cuts a `#` living inside a string, so x="a # b"; producer | head -1 loses its real hazard -- trading two loud false positives for a silent false negative. So `strip_trailing_comment()` walks the line tracking quote state and cuts only at a `#` that is outside quotes AND preceded by whitespace. Both paths call it, so there is one implementation rather than two spellings of it. Verified, all directions: || true + comment spared (was flagged) prose in trailing comment spared (was flagged) # inside "..." FLAGGED (would regress under a regex) # inside '...' FLAGGED (same, other quote) x=a#b; producer | head -1 FLAGGED (no space -> not a comment) allow marker spared whole-line comment spared AND IT MADE A GUARD DEAD, WHICH THE HARNESS REPORTED HONESTLY. With the strip in place, `if (line ~ /^[[:space:]]*#/) next` can no longer change any verdict -- a whole-line comment strips to the empty string. Its mutation came back UNCAUGHT. A surviving mutation on a guard that cannot fail is dead code announcing itself, so the guard is REMOVED rather than annotated, and the case it covered now runs through the strip. The other UNCAUGHT was my own bad mutation: I removed the whitespace-precedence condition, which changes nothing for `x="a # b"` (that `#` IS space-preceded). The real discriminator is `x=a#b; producer | head -1` -- with the condition, flagged; without, the strip eats the hazard. That is the pin now. 76 selftest cases, 0 failed. Mutation tier 7 + 23, 0 stale, 0 uncaught. Fleet re-measured: 18 repos, 0 offenders. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(gate): the set-line dispatch skipped its own line, and `pipefail;` never registered TWO fail-opens, the second found while fixing the first. 1. `if (... set ...) { apply_set(line); next }` -- the unconditional `next` meant the rest of the PHYSICAL line was never judged: set -euo pipefail; producer | head -1 -> missed Third `next`-shaped miss on this file, after the one-liner function and the multi-line opener. Fall through instead; the segmentation added earlier is what makes that safe, exactly as Asad said: `set -euo pipefail` becomes its own segment, carries no pipe, cannot produce a false positive. 2. AND THE BIGGER ONE. With the `next` gone the case STILL did not fire, because `apply_set` split on whitespace only: `set -euo pipefail; cd /tmp` tokenises as `pipefail;`, which never equals `pipefail`, so the `-o` handler missed it and p_on stayed 0 FOR THE WHOLE FILE. Every hazard in such a file was skipped, not just the one sharing the line. Now splits on `[[:space:];]+`. Worth stating: fixing #1 alone would have left a green test and a still-broken gate. The only reason it surfaced is that I ran the case rather than assuming the fix worked. Four cases: the same-line hazard, the whole-file version, and -- the discriminations -- `set +e; cmd` and `set +o pipefail; cmd` must STILL disarm, or falling through would have traded a fail-open for a fail-closed. Mutation-proved both: restoring the `next` and reverting the split each redden their own case, nothing else. ON THE REST OF ASAD'S REVIEW: all three directions of the `:220` finding, and the `echo '# not a comment' | head -1` fail-open he warned a naive strip would cause, are already correct on this branch -- the quote-aware `strip_trailing_comment` landed before his review. Measured all four again. 80 selftest cases, 0 failed. Mutation tier 7 + 25, 0 stale, 0 uncaught. Fleet re-measured after both widenings: 18 repos, 0 offenders. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…rd (#302) * fix(2324): refuse an unknown Status in .kanban.yml, not Cancel the card `resolve()` returned a `.kanban.yml` override's Status verbatim. An unknown BRANCH was refused; an unknown STATUS was not. That asymmetry is what made a typo destructive: the name reaches kanban-closure-router's update step, resolves to no ProjectV2 option id, and the step aborts WITHOUT WRITING -- at which point the project's built-in "Item closed" automation sets `Cancelled` and kanban-archive.yml takes the card off the board within a day. It fell straight through the `unreadable_override` holding state that .github#295 added for the neighbouring case. An override that cannot be READ was parked; one that read fine and named a column that does not EXIST was silently destroyed. Both now refuse, so both reach every caller as the non-zero exit all three already handle. Membership of ENV_FOR_STATUS is the accept list -- one dict supplies both the acceptance and the environment, so they cannot drift apart, and kanban-columns-check.py already asserts every key exists on the board. Also: * the mapper's stderr is no longer discarded at any of the three call sites. The refusal names the branch, the bad value and the accepted vocabulary, and that message is the only thing telling an operator which line to fix. * `override-unreadable` -> `override-unusable`. Two causes reach the holding state now; a label claiming a read failure that never happened sends the operator to the wrong place. No card has ever carried the old name. Refs tracebloc/backend#2324, .github#295, backend#2243 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(2324): the new label description was 133 chars, and the API caps it at 100 Bugbot on #302, and it is right. GitHub's label API 422s over 100 characters. The create runs under `set -euo pipefail`, so an over-long description aborts the step and the parked card never gets the marker -- the holding state losing the one thing that makes it visible to the weekly pass, which is the failure this PR exists to prevent, reintroduced by its own wording. The baseline description was 80 and mine was 133; the replacement is 83. Measured now rather than eyeballed: the selftest extracts every `-f description=` in the router and asserts the cap, and fails closed when it can extract none. Refs tracebloc/backend#2324 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
release-train: develop -> staging
LukasWodka
commented
Aug 21, 2026
ContributorAuthor
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 bde473f. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated promotion by the release train (RFC-0008 D14). Head is the train-managed
release-train/to-mainbranch (a mirror ofstaging), so it never collides with a human PR. Merged only when the fr-gate is green.Note
High Risk
Touches fleet-wide GitHub App tokens, unattended issue close, and kanban Status writes. A mapper or stale-sweep bug can archive shipped work or close live issues.
Overview
Promotes a large
.githubbatch: one shared branch→Status mapper, a board-aware stale-issue sweep, and two new CI gates (pipefail early-close and App-token mint scope).Kanban.
branch_status_map.pyis now the only branch→Status/env mapping. Advance, the closure router, and reconcile all call it (including.kanban.ymlfetched over the API). Unknown Status names refuse instead of writing a name the board cannot resolve (which used to let “Item closed” setCancelledand archive the card). The router parks those cards onOn devand labelsoverride-unusable. Column-check now imports the mapping and flags workflows that name columns but are not inWRITERS.Stale backlog.
stale-backlog.ymlis a reusable that runsstale-backlog.pyinstead ofactions/stale. Only live, unarchivedBacklogcards are eligible; unreadable board state skips. Inventory treats it as a caller (migration still in flight).Quality / audit.
code-qualitygains a whole-tree pipefail early-close job.mint-scoperequires explicitpermission-*on App-token mints (with a burn-down exemption list); a few workflows already scope down. Caller-drift auditsdeveloponly for train repos (develop-first-on-train). Requiredselftestsnow also run mint-scope and the mutation tier.Reviewed by Cursor Bugbot for commit bde473f. Bugbot is set up for automated code reviews on this repo. Configure here.