Uh oh!
There was an error while loading. Please reload this page.
sec(1528): bake dev's retired posture into the ByEnv defaults (backend#1528) - #900
Conversation
…d#1528) The last code item on #1528, and feasible for the first time as of today: #2738 said it could not be done, client#895 made the re-parent pin derivable from the rotation, and client#897 + client-runtime#439 landed the gate and the REVOKE. dev has been retired since 2026-08-26, so the defaults now record a posture that is real. rotateMysqlRootByEnv.dev, bootstrapDbReparentByEnv.dev and narrowEdgeuserByEnv.dev go true; serviceDbAccounts and perExperimentDbCreds were already true for dev. stg and prod are untouched -- the bake follows RETIREMENT, one fleet at a time, and staging is REVOKEd but not DROPped while prod has none of the posture. WHY IT MATTERS: all three were false for EVERY environment, so a values-resetting upgrade or a fresh-datadir reinstall on dev landed on defaults that put the account mint back on edgeuser. A default dev install now renders the whole retired posture with no --set at all. AND THE BAKE EXPOSED A DESIGN FLAW IN THE GATE I SHIPPED AN HOUR AGO, which is the substance of this PR. With narrowEdgeuserByEnv.dev a flat `true`, turning ONE predecessor off on a dev edge became a HARD RENDER FAILURE: assertNarrowEdgeuserIsSafe asserted over the baked default, so an operator stepping back from the posture -- debugging, a partial rollback -- had to discover a second flag they never set. It broke 5 chart tests and the gate-byenv-resolution guard, which renders each gate OFF by design to prove the map reaches the templates. Fixed in the RESOLVER, not the bake: a DEFAULT tracks the posture, an EXPLICIT request is honoured or refused loudly. Narrowing is a CONSEQUENCE of being retired, not an independent choice, so the ByEnv default is now conditional on the three predecessors. baked default, full posture -> narrows baked default, a predecessor off -> renders, does NOT narrow EXPLICIT true, posture broken -> REFUSED, loudly, as before EXPLICIT false -> overrides the bake Silence is the safe direction here and only here: NOT narrowing leaves edgeuser's grants intact and breaks nothing, while narrowing too early degrades the heartbeat SILENTLY. The dangerous direction is still refused. The assertion stays scoped to the RESOLVED value rather than the explicit override. A mutation showed no test can tell those apart today -- the resolver already declines -- but scoping it to the override would mean that if the resolver were ever made unconditional again, a baked default with a broken posture would render NARROW_EDGEUSER=1 with nothing checking it. One mechanism guarding the other beats two where the second can disable the first. That reasoning is in the helper. Side effect worth naming: scripts/tests/reparent-requires-rotation.sh now has a REACHABLE branch for the first time. I flagged it as structurally vacuous when I wrote it, because no environment baked the re-parent on; it now reports "re-parent defaulted on for [dev], each with the rotation on". Verified: drift 29/29; helm unittest 617/617 (4 cases rewritten that encoded the old defaults or the old design, 3 added); helm lint clean. Behaviour confirmed by render at dev, stg and prod. Mutation-proven: making the baked default a flat `true` again reddens. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…backend#1528) Bugbot, #900: the case titled "the whole retired posture" asserted three of the five gates the dev bake flips. The two omissions -- bootstrapDbReparent and rotateMysqlRoot -- render in files the suite never loaded, so the claim could not be checked by the thing making it. Measured, not assumed: with `rotateMysqlRootByEnv.dev: false` the ORIGINAL suite passes 17/17 green. Reverting the rotation on the bake path changed nothing in a suite whose headline case says it covers the posture. The suite now renders three templates and every case names its own, since rotateMysqlRoot is not a narrowing predecessor and no refusal case reached it by accident. Added: DB_BOOTSTRAP_USER + DB_BOOTSTRAP_PASSWORD on the dev posture case (both-or-neither, per client-runtime#310); the Secret carrying MYSQL_ROOT_PASSWORD and the mysql pod consuming it (a key nothing reads is a rotation nobody performs); and the prod side of both. prod`s mysql case uses notExists on the env key rather than notContains on the entry -- with the gate off that container renders no env at all, and helm-unittest 0.5.2 ERRORS on notContains against a missing path instead of treating absence as satisfaction. Mutation-proved, each anchor asserted to apply exactly once: bootstrapDbReparentByEnv.dev -> false : 4 failed, 1 errored rotateMysqlRootByEnv.dev -> false : 7 failed, 6 errored Suite 22 passed (was 17); full chart suite 36 files / 622 passed; scripts/tests/helm-unittest-error-assertions.sh OK. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 28, 2026
bugbot run |
Uh oh!
There was an error while loading. Please reload this page.
…rd the class (backend#1528) Saqlain on #900: values.yaml comments were updated, three values.schema.json descriptions and three _helpers.tpl doc headers were not -- so the operator-facing contract stated the opposite of what a default dev install renders, with all 29 drift guards green. Also adds the guard, because the finding was the gap not the instances: gate-default-prose-agreement.sh enumerates every *ByEnv gate FROM values.yaml (five today) and refuses any schema description or helper doc that claims a default the chart contradicts. Derived, not listed -- a sixth gate is covered without touching it. Fails closed on unreadable/unparseable/zero-pairs. Mutation-proved against the real defect: on the un-fixed tree it reports exactly the six sites Saqlain found by hand, having derived them independently. drift: all 30 guards green. helm-unittest: 36 suites, 622 tests. shellcheck -S warning -x clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 28, 2026
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9f5d469. Configure here.
Makefile conflict: both sides appended to DRIFT_GUARDS -- develop gained edgeuser-drop-readiness-verdicts.sh (client#896), this branch adds gate-default-prose-agreement.sh. Resolved as the UNION (31 guards), asserting no develop guard was dropped rather than eyeballing it. The two edgeuser-drop-readiness files showed as deletions against develop only because #896 added them after this branch point; the merge restores them. drift: 31/31 green. helm-unittest: 36 suites / 622 tests. edgeuser-drop-readiness-verdicts: 51 passed. Chart 1.9.85 = develop 1.9.84 + 1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 28, 2026
Merged The Makefile conflict was real: both sides appended to The two Verification on the merged tree: I ran #896's incoming suite specifically rather than assuming the merge was inert for it, since this branch touches |
Uh oh!
There was an error while loading. Please reload this page.
aptracebloc
left a comment
There was a problem hiding this comment.
Reviewed at head 0a3a8bb3 (my read was at 9f5d4698; the only change since is a develop merge bringing in the now-merged #896 — it doesn't touch this PR's own files, so these all still stand). The bake itself is clean: dev-only in values.yaml, stg/prod untouched, and the narrowEdgeuser resolver fail-direction is correct — a baked default with a broken predecessor declines silently (safe), an explicit narrowEdgeuser: true over a broken posture still fails loudly, and there's no path that renders NARROW_EDGEUSER=1 without the three predecessors. Chart bump and version==appVersion are right. Three things to address before it ships — all the same default-vs-doc drift class you and Saqlain already agreed belongs in-PR:
stg test coverage — the open Bugbot thread is right (I traced it). The "stg stays unbaked" case only
notContainsNARROW_EDGEUSERon jobs-manager, which is vacuous: the resolver declines stg narrowing regardless, sonarrowEdgeuserByEnv.stg=truewould still pass it. And there are no stg negative cases forrotateMysqlRoot/bootstrapDbReparentthe way prod has — so a stray stg bake of the rotation/re-parent would ship green while an auto-upgrade generates a new root password and re-parents onto a stale datadir. Mirror prod's three negative cases for stg.Two stale runbook docs the sweep missed, now factually wrong for dev (and this is a security posture, so the wrong instruction matters):
client/MIGRATION.md:10("rotateMysqlRoot/rotateMysqlRootByEnv are false for dev, stg and prod, so an upgrade changes nothing") anddocs/migration-tools/rotate-mysql-root.md:65("enable the gate first — rotateMysqlRootByEnv defaults to false for dev/stg/prod").The new guard can't catch #2.
gate-default-prose-agreement.shbills itself as "does any document assert a default this chart contradicts?" but only scansvalues.schema.json+_helpers.tpl— which is why it's green over both stale docs in this very PR. Widen its scanned set to the per-env-default runbooks, or narrow the claim in its comment.
Not blocking on the shipped template/values/version — those are sound. Holding for CI on the new head regardless (it's pending, plus the open thread and no approval yet). Clear #1–#3 and I'll approve once green.
— drafted with Claude Code
…renced (backend#1528) Bugbot, #900 -- and it is a hole in my own previous fix. That fix split the rotation into two assertions (the Secret carries MYSQL_ROOT_PASSWORD, the mysql pod consumes it) on the reasoning that a key nothing reads is a rotation nobody performs. The re-parent has the same asymmetry in the other direction and I only covered one side: jobs-manager was asserted to REFERENCE DB_BOOTSTRAP_PASSWORD via secretKeyRef, with nothing asserting the Secret renders it. A reference to an absent key is not a passing render -- it is a default dev install whose jobs-manager fails on a missing Secret key. Measured, not assumed. Dropping the key from secrets.yaml: pre-fix suite : 22 passed, 0 failed <- blind, exactly as reported with this fix : 1 failed, 21 passed restored : 22 passed prod asserts notExists on both keys, so a one-sided revert cannot pass there either. Full chart suite 36 files / 622 passed; scripts/tests/helm-unittest-error-assertions.sh OK. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 28, 2026
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 51f7712. Configure here.
aptracebloc
left a comment
There was a problem hiding this comment.
CI's green and Bugbot's clear now — nice. The only thing between this and my approval is the change-request I left earlier; your commits since (the Secret re-parent-key test) didn't touch its three items, so they're all still open. The one that actually matters is #2: client/MIGRATION.md:10 and docs/migration-tools/rotate-mysql-root.md:65 still tell operators rotateMysqlRootByEnv "defaults to false for dev" — which this PR makes false, so a rotation runbook now gives the wrong instruction for dev. That's the same in-PR prose-drift class you fixed for the other sites. Fix the two docs (and ideally the stg test-hardening + guard-scope points), and I'll approve — everything else here is sound.
— drafted with Claude Code
…ard that now checks what it claims (backend#1528) Arturo, #900 -- three findings, all the same default-vs-doc drift class. 1. THE stg CASE WAS VACUOUS, and measured: with narrowEdgeuserByEnv.stg flipped to true the suite still passed 22/22. A bare notContains at a plain stg render cannot fail, because the resolver declines stg narrowing on the PREDECESSORS regardless of the map -- the assertion was reading the predecessors and reporting on the map. Replaced with five cases: the narrowing one forces the full posture on (including rotateMysqlRoot, without which secrets.yaml FAILS the render and the case would test the refusal instead of the map), so the ByEnv entry is the only input left; plus stg negatives for the rotation and the re-parent mirroring prod, and prod`s own posture-forced case. Mutation-proved, each anchor asserted to apply exactly once: narrowEdgeuserByEnv.stg -> true : 1 failed rotateMysqlRootByEnv.stg -> true : 2 failed bootstrapDbReparentByEnv.stg -> true : 3 failed, 3 errored 2. Two runbooks were factually wrong for dev, and this is a security posture: client/MIGRATION.md said an upgrade "changes nothing on its own" (on dev it now adds MYSQL_ROOT_PASSWORD and rolls the mysql pod), and docs/migration-tools/rotate-mysql-root.md told the operator to enable a gate that is already on for dev -- so a zero read there means a broken install, not an off gate. Both corrected, stating dev vs stg/prod. 3. gate-default-prose-agreement.sh claimed to answer "does ANY document assert a default this chart contradicts?" while scanning two files -- it was green over both stale runbooks in this very PR. Widened to every markdown file under client/ and docs/, GLOBBED not listed, scanned per paragraph and only paragraphs naming the gate. Markdown is NORMALISED before matching, and that is load-bearing: the sentence that shipped reads "they are `false` for `dev`" -- backticks sit inside the span the patterns must cross, so raw matching finds nothing and the widening would have been a bigger corpus that cannot see the sentence it was widened for. Proved by re-inserting BOTH original stale sentences: MIGRATION.md#p5 -> FAIL docs/migration-tools/rotate-mysql-root.md#p11 -> FAIL Fails closed on an empty glob or an unreadable file. Suite 26 passed (was 22); full chart suite 36 files / 626 passed; guard 5 gates / 84 pairs (was 2 sources); shellcheck -S warning -x clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 28, 2026
All three addressed at 1 — the stg case was vacuous, and I measured how badly. With Replaced with five cases. The narrowing one forces the full posture on so the ByEnv entry is the only remaining input — including 2 — both runbooks corrected. 3 — the guard now checks what it claims. Widened to every markdown file under One thing worth flagging, because widening it naively would have been theatre: markdown has to be normalised before matching. The sentence that shipped reads ``they are Fails closed on an empty glob or an unreadable file. Suite 26 passed (was 22); full chart suite 36 files / 626 passed; guard now 5 gates / 84 pairs; |
LukasWodka
commented
Aug 28, 2026
bugbot run |
aptracebloc
left a comment
There was a problem hiding this comment.
All three addressed — verified at d52b1039, and cleanly:
- stg negatives now exist and can actually fail — "stg bakes no root rotation (no Secret key, nothing consuming one)", "stg's mysql pod injects no MYSQL_ROOT_PASSWORD", "stg's jobs-manager is not re-parented", and the ByEnv-declines case forces
rotateMysqlRoot/bootstrapDbReparenton so it isn't vacuous. A stray stg bake would now redden. - the two runbooks (
MIGRATION.md,rotate-mysql-root.md) now correctly staterotateMysqlRootByEnv.devistrue(baked under #1528 S3),falsefor stg/prod. - the guard now globs every
.mdunderclient/anddocs/, per-paragraph and gate-named, and fails closed on zero files — so it can't be green over a stale runbook again.
Nothing left on my side but CI — it's still running on this head. I'll approve the moment it's green; no need to re-request.
— drafted with Claude Code
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
LukasWodka
commented
Aug 28, 2026
@aptracebloc all three items are addressed at Independent verification of |
| mutation | result |
|---|---|
narrowEdgeuserByEnv.stg=true | 1 failed — "stg does not narrow, and it is the ByEnv map saying so — not the posture" |
rotateMysqlRootByEnv.stg=true | 2 failed |
bootstrapDbReparentByEnv.stg=true | 3 failed, 3 errored |
All three would have shipped green before. Your read of the old case was exact: it passes with narrowEdgeuserByEnv.stg=true because the resolver declines stg narrowing regardless, and the replacement sets every predecessor so the ByEnv default is the only thing left holding narrowing off.
Item 3 — the widened guard catches a re-stale. I re-introduced `false` for `dev` into MIGRATION.md:
gate-default-prose-agreement: FAIL
- client/MIGRATION.md#p5: says 'false' for dev (matched 'false for dev') but values.yaml ships rotateMysqlRootByEnv.dev = true
Worth noting why this was subtle: the runbooks write `false` for `dev` and false for dev/stg/prod, so inline code spans sit between exactly the words the patterns join. A widening that scans the files but doesn't normalise markdown still reports OK — I hit that myself before normalising. d52b103 handles it and also strips [text](url), which mine did not.
Item 2 — both runbooks correct. The MIGRATION.md wording is better than what I had: it names the upgrade consequence (the Secret gains MYSQL_ROOT_PASSWORD, the mysql pod rolls once) rather than just the new default, which is the part an operator acts on.
drift 31/31 · helm-unittest 36 suites / 626 tests · guard OK at 84 gate/env pairs.
One process note, since it cost real work
Two sessions fixed your three items simultaneously on the same four files (149 vs 150 changed lines). The per-PR lock that is supposed to prevent this expired — its TTL is 6 minutes and the other session's heartbeat lapsed while it worked, so my pass stole the lock as stale and started duplicating. The non-fast-forward push abort caught it, which is the backstop working correctly and one full cycle too late.
I discarded mine rather than merging over it: d52b103 was already pushed and is equal or better on every point I checked, so superseding it would have been a discard of the better version. Recording the lock-expiry mechanism because it will recur — a long fix is exactly when the heartbeat is most likely to lapse.
Your item 3 was the sharpest of the three, incidentally: the guard's own comment claimed "any document" while scanning two files, so it sat green over two docs the same PR made false. A guard over-claiming its inputs is the defect that suite exists to catch, and it was in the guard.
…reads values.yaml too (backend#1528) Two Bugbot findings. HIGH -- an unrotated dev edge crash-loops on the first auto-upgrade. The bake turns the re-parent and the rotation on together. On an edge whose Secret has no MYSQL_ROOT_PASSWORD the chart GENERATES one; the mysql entrypoint applies it only at FRESH datadir init, so an existing datadir keeps the image-baked password; and DB_BOOTSTRAP_PASSWORD is DERIVED from the rotation value (backend#2738), so jobs-manager authenticates as root with a password the live account does not have. Auto-upgrade uses --reset-then-reuse-values, so it picks the new defaults up on its own. Documented in MIGRATION.md with the check to run and two remedies. NOT changed in the chart: secrets.yaml`s own derivation comment records that dev rotated before the re-parent was enabled, which is why this has not bitten, and whether any dev edge is actually in the unrotated state is a live-fleet question this diff cannot answer. Flagged rather than assumed. MEDIUM -- gate-default-prose-agreement.sh did not read values.yaml`s own comments, and its `false everywhere` pattern would not have matched `OFF everywhere` anyway. Both halves fixed: the vocabulary now carries off/on synonyms in both polarities, and each comment run is compared against the gate it introduces. ATTRIBUTION IS SCOPED, because widening it naively made it NOISY. Charging every gate named anywhere in a block produced four false positives -- the re-parent`s header cites serviceDbAccounts as a precondition, so its own "OFF for stg" was charged to serviceDbAccounts (stg=true); and docs/SECURITY.md`s identity TABLE is one paragraph, so a correct row about perExperimentDbCreds was charged three rows away. A guard that cries wolf gets skipped (rule 4). So: a values.yaml comment run is attributed to the key it INTRODUCES, and markdown is matched per LINE. Three real stale comments surfaced by the widening and fixed: bootstrapDbReparent "OFF everywhere by default", and two "Default false:" rollout lines that predate dev/stg being flipped on. Mutation-proved: restore "OFF everywhere" -> values.yaml#L1616 FAIL restore the MIGRATION.md sentence -> MIGRATION.md:10 FAIL SECURITY.md`s correct table row -> still OK (no false positive) Guard 5 gates / 129 pairs; full chart suite 36 files / 626 passed; shellcheck -S warning -x clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 28, 2026
bugbot run |
Uh oh!
There was an error while loading. Please reload this page.
aptracebloc
left a comment
There was a problem hiding this comment.
CI's green now and all three of my original items are addressed at b56f2ae4 — I re-verified each: the stg negatives are in edgeuser_narrowing_test.yaml, both stale runbooks (MIGRATION.md, rotate-mysql-root.md) now read correctly for dev (plus the new unrotated-dev upgrade-hazard block, which I read — documenting it with the fleet-already-rotated reasoning + an operator pre-upgrade check is a fair resolution for a dev-only bake), and the guard now globs every .md under client/ and docs/. Good follow-through.
One thing left, and it's the open Bugbot thread — I traced it, it's correct, not noise. The guard header at scripts/tests/gate-default-prose-agreement.sh:45 says "the mutation test in this suite re-inserts the original stale sentence to prove the finding is reachable" — but there's no such test: not inside the file (it has no self-test block) and no sibling in scripts/tests/. That leaves the load-bearing part unverified — _MD_STRIP stripping the backticks/bold out of they are `false` for `dev` is the entire reason widening the corpus wasn't vacuous, and nothing proves it still matches. If normalisation or a claim pattern silently stops matching that span, make drift goes green over the exact drift this guard was added to catch, and the header would still promise a test that was never there.
Best fix is to make the claim true: add the mutation case the header already describes — re-insert the original stale sentence verbatim (backticks and bold intact) into a scanned doc in a fixture/temp copy and assert the guard fails on it. That pins _MD_STRIP + the patterns against the real shipped drift. Failing that, drop the sentence from the header. Either way it's the last thing between this and my approval — resolve that thread and I'll approve (everything else is green and settled).
— drafted with Claude Code
…ackend#1528)
Found by running the full drift suite locally on the current head, which CI
reported green.
THE BUG. gate-default-prose-agreement.sh built its markdown list with
`MDFILES=$(find ...)` and passed it UNQUOTED as `$MDFILES`, so the list
word-split on whitespace. The primary dev checkout lives under
`.../Claude File System/...`, so the guard tried to read
`/Users/lukas/Documents/Claude` and failed closed -- "cannot tell, which is a
finding" -- on EVERY local run. Fixed by reading the list into an ARRAY and
expanding it as "${MDFILES[@]}".
NEITHER EXISTING CHECK COULD SEE IT, and that is the interesting part:
* CI cannot reproduce it. GitHub runners check out to
/home/runner/work/client/client -- no spaces, ever. So the guard was green in
the required job and permanently red on the one machine most likely to break
what it guards.
* shellcheck does not gate on it. SC2086 ("double quote to prevent word
splitting") is severity INFO and this repo gates at `-S warning`. Verified:
`shellcheck -S warning` exits 0 on the broken version.
So a guard written to catch prose drift was itself incapable of running, and both
of the mechanisms that should have said so were structurally blind.
scripts/tests/guards-survive-spaced-paths.sh closes that: it copies client/,
docs/ and scripts/ into a temp path containing a space and runs the
filesystem-walking guards from there, so the class is exercised IN CI where the
path otherwise never has one. It fails closed on an empty guard list, a copy that
did not land, or a temp path that somehow has no space -- "could not set the
scenario up" must not read as "the guards are fine". Registered in the single
DRIFT_GUARDS declaration.
Scope is deliberately narrow: only guards that ENUMERATE files from the
filesystem can exhibit this, and sweeping all 32 into a copied tree would trade a
precise check for a slow one.
Checked the class rather than the instance: `shellcheck -S info` finds SC2086 in
3 of 53 shell files here, but all 32 guards now pass from the spaced path, so
only this one was path-bearing. Left the other three alone rather than churning
them without evidence.
Verified: drift 32/32 green from a path WITH a space; helm unittest 626/626;
shellcheck -S warning + bash -n clean. Mutation-proven: restoring the unquoted
expansion reddens the new test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>LukasWodka
commented
Aug 28, 2026
Everything on this PR is addressed. Summary of where each item landed, and one bug I found that neither CI nor @aptracebloc's change-request — all three, verified by him at |
| Secret | MYSQL_ROOT_PASSWORD | means |
|---|---|---|
| absent | — | fresh install — safe |
| present | present | already rotated — safe |
| present | absent | upgrade that never rotated — the hazard, exactly |
So a render-time refusal was implementable. The real reason not to add one: I checked, and lookup returns nothing under helm template, so the positive case can never be reached by helm unittest. A fail keyed on it would sit on the upgrade path with no test able to exercise it — and if the condition were subtly wrong, everyhelm upgrade would break with nothing in CI saying so. This PR has spent its life removing checks that couldn't fail; adding one that can't be tested, on the path that upgrades edges hourly, is a worse trade.
Two facts that narrow the residual risk: dev rotated root on 2026-08-25 per #1528's table, so the edge this bake was written for cannot crash-loop. And the open question is only whether any otherCLIENT_ENV=dev edge has a pre-existing datadir and no stored rotation — one kubectl command per namespace, posted on the thread.
A bug I found that CI could not
Running the full drift suite locally on the current head — CI said green, local said red.gate-default-prose-agreement.sh passed its markdown list unquoted ($MDFILES), so it word-split on the space in .../Claude File System/... and failed closed on every local run.
Both mechanisms that should have caught it were structurally blind:
- CI cannot reproduce it — runners check out to
/home/runner/work/client/client, no spaces, ever. shellcheckdoesn't gate on it — SC2086 is severity info, and this repo gates at-S warning. Verified:-S warningexits 0 on the broken version.
So a guard written to catch prose drift was itself unable to run, and green in the required job. Fixed with an array, plus scripts/tests/guards-survive-spaced-paths.sh, which copies the tree into a temp path containing a space and runs the filesystem-walking guards from there — so the class is exercised in CI, where the path otherwise never has one. It fails closed on an empty guard list or a copy that didn't land.
Checked the class: shellcheck -S info finds SC2086 in 3 of 53 shell files, but all 32 guards now pass from the spaced path, so only this one was path-bearing. Left the others alone rather than churning them without evidence.
Verification on the current head
make drift32/32 from a path with a space · helm unittest626/626 · helm lint clean · shellcheck -S warning + bash -n clean · mutation-proven (restoring the unquoted expansion reddens the new test; making the baked default a flat true reddens the suite).
Chart 1.9.85, one patch above develop.
…med (backend#1528) Bugbot, Low, and it is this guard`s own class one level up: the header said "the mutation test in this suite re-inserts the original stale sentence to prove the finding is reachable". No such test existed -- the mutations had been run by hand in a shell and never committed. A docstring asserting a check nobody can run is exactly what gate-default-prose-agreement.sh was written to stop (rule 7). scripts/tests/gate-default-prose-mutations.sh, armed in DRIFT_GUARDS beside the guard so it gates rather than advises (rule 2). Eight cases, each asserting the SPECIFIC finding text rather than a bare non-zero exit (rule 10): the tree as shipped agrees with its own defaults (the baseline the rest are differences against) a stale runbook sentence is caught, backticks and all a stale values.yaml comment is caught, OFF counts as false WITHOUT normalisation the same sentence is MISSED <- the anti-proof a correct row about one gate is not charged to another <- no false positive a checkout path containing a space still resolves <- regression, d626a15 zero markdown files is a FINDING, not agreement an unreadable schema is a FINDING, not agreement Two cases earn their place by proving a NEGATIVE. Case 4 disables the markdown normaliser and asserts the guard then goes GREEN over the same stale sentence -- so "stripping backticks is load-bearing, not cosmetic" is demonstrated rather than asserted. Case 6 covers d626a15`s word-splitting bug: every other case builds its fixture under mktemp, which has no spaces, so the suite would otherwise have been blind to exactly the bug that was live on the primary dev checkout. It calls the real guard, never a copy of its rule (rule 9). Verified by breaking it twice: dropping `off` from the false-claim vocabulary reddens case 3 by name, and reverting the MDFILES array to the word-splitting string reddens case 6 by name. Both restore to 8/8. The header now names a file that exists. Guard 5 gates / 129 pairs; mutations 8 passed; chart suite 626 passed; shellcheck -S warning -x clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 28, 2026
bugbot run |
LukasWodka
commented
Aug 28, 2026
|
…il (backend#1528) The required `quality / pipefail early-close` check, and both are in files this PR adds. gate-default-prose-mutations.sh:56 printf ... | grep -qF -- "$want" guards-survive-spaced-paths.sh:70 printf ... | sed ... | head -6 `grep -q` and `head` close their input on match / on the Nth line, which SIGPIPEs the writer; under `set -o errexit -o pipefail` that fails the whole pipeline. The consequences are worse than a lint nit in both cases: in the first a PASSING assertion would have reported as a failure, and in the second the diagnostic printer for a failing case would itself fail, replacing a legible excerpt with a second misleading error. Fixed as the checker advises -- a here-string for the grep, capture-then-slice for the excerpt. Verified under the CI condition rather than the default shell: both suites run to completion with `set -o errexit -o pipefail`. gate-default-prose-mutations 8 passed; guards-survive-spaced-paths OK; shellcheck -S warning -x clean on both. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 28, 2026
CI red fixed —
One thing I did not change, and want to flag rather than silently fix. But its header says "the drift guards must work from a checkout whose path contains a space", and I left it alone because this PR is one green run from approval and a 30-guard sweep is exactly the kind of thing that adds flakiness at the wrong moment. Either narrow the header to what it runs, or do the derived version as a follow-up — your call. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
aptracebloc
left a comment
There was a problem hiding this comment.
Two Bugbot threads open right now; here's where I land on each after tracing them:
The new one — "True-claim patterns omit list form" (gate-default-prose-agreement.sh:230) — is real, and I'd fix it.FALSE_CLAIMS carries the list form (_OFF for [\w,\s]*{env}) and default off; TRUE_CLAIMS has neither — just the single-env _ON for {env} and baked on for {env}. So a doc that says true for dev, stg and prod while stg/prod are still off wouldn't redden. That's not a hypothetical asymmetry: it's the exact direction this rollout travels — envs flip ON one at a time, so the drift you'll actually generate next is a premature "on for [list]" claim, and that's the polarity the guard is currently blind to. Mirror the two missing patterns into TRUE_CLAIMS, and add the true-polarity list-form case to gate-default-prose-mutations.sh (Bugbot's right that the sibling is untested — right now only the false list-form is exercised).
The other one — "spaced-path guard dies on long output" — is already fixed at 4cf0085d. The early-closing head -6 is gone, replaced with capture-then-sed -n '1,6p' (reads to EOF, no SIGPIPE), so fails increments correctly now. That thread is Bugbot's, posted against 13faf29d; it should clear once the re-review on this head finishes — nothing left to do there.
Everything I raised originally is settled. Once TRUE_CLAIMS is symmetric (with its mutation case), CI's green, and both threads are cleared, I'll approve.
— drafted with Claude Code
saqlainsyed007
left a comment
There was a problem hiding this comment.
Re-review on 4cf0085. My earlier finding is fully resolved — thanks for syncing all six documents plus adding gate-default-prose-agreement.sh; the guard derives gates from values.yaml, fails closed, and is mutation-proved against the real drift rather than a synthetic one. Not approving yet, for two reasons:
@aptracebloc's CHANGES_REQUESTED (from 0a3a8bb) is still the standing review decision and hasn't been dismissed or re-approved — that's his call to clear, not mine.
One open Bugbot thread is a valid, unaddressed gap on this head: gate-default-prose-agreement.sh TRUE_CLAIMS is asymmetric with FALSE_CLAIMS — it lacks the list form (_ON ... for [words]{env}) and
default on, so a future stale "on"-polarity claim in list form naming a still-off env would not redden, and the mutations suite doesn't exercise that sibling. Worth closing the symmetry (and adding the mirror mutation case) before merge.
The other Bugbot thread (guards-survive-spaced-paths.sh / head -6) reads as already fixed on this head — you switched to capture-then-slice with sed -n '1,6p', which doesn't early-close. Can be resolved.
The High-severity crash-loop hazard is well handled: documented in MIGRATION.md with the check and both remedies, and dev's 08-25 root rotation means the targeted fleet edge is safe.
…nd#1528) saqlainsyed007 on #900: TRUE_CLAIMS was asymmetric with FALSE_CLAIMS -- it lacked the LIST form and `default on`, so a stale on-polarity claim naming a still-off env did not redden while its off-polarity twin did. Fixed by DERIVING both lists from one _POLARITY_SHAPES table rather than adding the two missing entries, so a new shape reaches both polarities and this asymmetry cannot return. The symmetry is asserted, not assumed. Completing the on-side surfaced TWO LATENT BUGS already present in the shipped off-side patterns, unexercised rather than absent: 1. The list span `[\w,\s]*` crossed the OTHER polarity, so "true for dev, false for stg" reported "says true for stg". Every one of these documents states both polarities in one sentence -- 18 false findings appeared the moment the on-side was completed. The span now refuses to cross the opposite polarity word or a second `for`. 2. A bare `default <pol>` named no environment, so it fired for every env whose value differed -- harmless while everything was false everywhere, wrong once one env was baked on. `default`/`defaults to`/`baked` are now a prefix on the two SCOPED shapes, so a claim must say everywhere or name an env. Mutations 8 -> 11. Measured against the pre-fix guard: both ON-polarity list-form cases FAIL there (exit 0, wanted 1) -- Saqlain gap, demonstrated. The both-polarities-correct case PASSES pre-fix, so it guards THIS fix rather than a pre-existing hole; recorded as such. Also re-established the zero-markdown fixture adjacent to its own run_case: my insertion had repointed the shared $D, and the case ran against a fixture nobody built for it. drift 33/33. helm-unittest 36 suites / 626 tests. shellcheck -S warning -x clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 28, 2026
@saqlainsyed007 your point 2 is fixed in 13b94fb, and completing the symmetry surfaced two latent bugs that were already in the shipped patterns. The asymmetryYou were exactly right: Two latent bugs the on-side exposedBoth were present in the shipped off-side patterns — unexercised, not absent — and both appeared the instant the on-direction became reachable:
Worth naming: had the on-side been completed by adding two entries, both bugs would have landed as a wall of false findings on a green-looking guard. Mutations 8 → 11, and what each provesMeasured against the pre-fix guard rather than asserted:
The third one is honest bookkeeping: it does not demonstrate a pre-existing hole — it guards my fix against regressing into the greedy span. I'd rather label it that way than present three cases as three caught gaps. I also re-established the zero-markdown fixture next to its own VerificationOn your other two points: the |
LukasWodka
commented
Aug 28, 2026
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 13b94fb. Configure here.
aptracebloc
left a comment
There was a problem hiding this comment.
Approving at 13b94fbb — CI's green, both Bugbot threads are cleared, and everything I raised across this review is addressed and verified against the code, not just thread-closed.
Walking my original three: the stg negative cases are in edgeuser_narrowing_test.yaml; both stale runbooks (MIGRATION.md, rotate-mysql-root.md) now read correctly for dev, with the unrotated-dev upgrade hazard documented and given an operator pre-upgrade check; and the prose guard globs every .md under client/ and docs/. Good.
The guard hardening that came out of the Bugbot rounds is the part I want to credit, because you took each finding to its root rather than patching the instance:
- The missing mutation test is now
gate-default-prose-mutations.sh, armed inDRIFT_GUARDS— and case 4 mutates_MD_STRIPto prove the normalisation is load-bearing, which was my actual worry. It also found and fixed a real one: the guard's unquoted$MDFILESword-split on a spaced checkout path (invisible to CI and toshellcheck -S warning), now an array, withguards-survive-spaced-paths.shto keep it fixed. - The
TRUE_CLAIMSasymmetry (my last comment) you solved better than I asked: instead of hand-adding the two missing patterns, you derived both polarities from one_POLARITY_SHAPEStable with a symmetry self-check that fails closed if they ever diverge — so the two lists can't drift apart again. The mutations suite pins it three ways: the ON-polarity list form, thebaked onprefix variant, and a both-polarities-correct sentence that must NOT false-positive. - The pipefail early-close nits are gone too (here-string +
sed -n '1,6p'for the readers).
This is exactly the follow-through a dev-only, prod-irreversible-adjacent bake wants: the safety rail is now hardened against the silent-vacuous failure it exists to prevent. LGTM.
— drafted with Claude Code
Uh oh!
There was an error while loading. Please reload this page.

Part of tracebloc/backend#1528 — the last code item on that ticket.
Why this is possible only now
Three things landed today that this depended on:
#2738said bakingbootstrapDbReparentByEnvcould not be done,client#895made the re-parent pin derivable fromrotateMysqlRoot, andclient#897+client-runtime#439landed the gate and the actualREVOKE. dev has been retired since 2026-08-26, so the defaults can finally record a posture that is real.rotateMysqlRootByEnv.dev,bootstrapDbReparentByEnv.devandnarrowEdgeuserByEnv.dev→true.serviceDbAccountsandperExperimentDbCredswere already true for dev.stgandprodare untouched. The bake follows retirement, one fleet at a time — staging isREVOKEd but notDROPped, and prod has none of the posture.What it buys: all three were
falsefor every environment, so a values-resetting upgrade or a fresh-datadir reinstall on dev landed on defaults that put the account mint back onedgeuser— the exact risk #1528 names. A default dev install now renders the whole retired posture with no--setat all.The bake exposed a design flaw in the gate I shipped an hour ago
This is the real substance, and it's why the diff touches
_helpers.tpl.With
narrowEdgeuserByEnv.deva flattrue, turning one predecessor off on a dev edge became a hard render failure —assertNarrowEdgeuserIsSafeasserted over the baked default, so an operator stepping back from the posture (debugging, a partial rollback) had to discover a second flag they never set. It broke 5 chart tests and thegate-byenv-resolutionguard, which renders each gate OFF by design to prove the map reaches the templates.Fixed in the resolver, not the bake: a default tracks the posture, an explicit request is honoured or refused loudly. Narrowing is a consequence of being retired, not an independent choice.
CLIENT_ENV=devnarrowEdgeuser: true, posture brokennarrowEdgeuser: falseSilence is the safe direction here and only here: not narrowing leaves
edgeuser's grants intact and breaks nothing, while narrowing too early degrades the heartbeat silently. The dangerous direction is still refused.One deliberate non-change, because a mutation asked the question
I initially also scoped the assertion to the explicit override. A mutation showed no test can tell that apart from the resolved value — the resolver already declines — so it was redundant. I reverted it, and the reasoning is in the helper: scoping it to the override would mean that if the resolver were ever made unconditional again, a baked default with a broken posture would render
NARROW_EDGEUSER=1with nothing checking it. One mechanism guarding the other beats two where the second can disable the first.Side effect worth naming
scripts/tests/reparent-requires-rotation.shnow has a reachable branch for the first time. I flagged it as structurally vacuous when I wrote it, because no environment baked the re-parent on — it could only be exercised by synthetic self-cases. It now reports:Verification
make drift— 29/29helm unittest ./client— 617/617. Four cases rewritten because they encoded the old defaults or the old design; three added, including the two that pin the new behaviour.helm lint— cleandev,stgandprod— dev rendersDB_BOOTSTRAP_USER,SERVICE_DB_ACCOUNTS,PER_EXPERIMENT_DB_CREDS,NARROW_EDGEUSERandMYSQL_ROOT_PASSWORD; stg and prod render none of the three new gates.trueagain reddens the suite.Chart 1.9.85.
🤖 Generated with Claude Code
Note
High Risk
Dev fleet upgrades change MySQL Secret wiring, bootstrap auth, and narrowing by default; existing dev edges without prior root rotation can hit a documented CrashLoop unless operators follow the migration warning.
Overview
Bakes backend#1528’s retired posture into chart defaults for
CLIENT_ENV=devonly —rotateMysqlRootByEnv,bootstrapDbReparentByEnv, andnarrowEdgeuserByEnvflip totruefor dev; stg/prod stay off. A default dev install now renders root rotation, bootstrap re-parent, service DB accounts, per-experiment creds, andNARROW_EDGEUSERwithout extra--set.Fixes
tracebloc.narrowEdgeuserfor baked dev: a ByEnv default narrows only when all three predecessor gates resolve on; turning one gate off for debugging no longer hard-fails the render. ExplicitnarrowEdgeuser: truewith a broken posture still fails at template time viaassertNarrowEdgeuserIsSafe.Docs and ops:
MIGRATION.mdand the MySQL root runbook now describe dev’s baked rotation and warn about jobs-manager CrashLoop on dev edges with an existing datadir that never rotated root before auto-upgrade. Schema/helper prose is aligned with the new defaults.Tests and CI:
edgeuser_narrowing_test.yamlis expanded acrosssecrets.yamlandmysql-deployment.yaml, with stg/prod negative cases that can actually fail. Three new drift guards keep gate-default prose in sync withvalues.yaml, prove the guard can fail, and run file-walking guards from paths containing spaces.Reviewed by Cursor Bugbot for commit 13b94fb. Bugbot is set up for automated code reviews on this repo. Configure here.