fix(installer): gate the arch on the adopt and dev-mode install paths (backend#2146)

[aptracebloc]

Closes tracebloc/backend#2146

The defect

_assert_engine_runs_on_this_arch ran on only the normal values-generation path. Two other paths reach a Helm deploy without it:

• adopt-reconcile (TRACEBLOC_CLIENT_ADOPTED=1) — _reconcile_adopted_client runs helm upgrade before the assert was ever reachable.
• TRACEBLOC_VALUES_FILE (dev-mode) — takes the early then branch and skips the whole else where the assert lived.

So on an arm64 host with no amd64 emulation, preflight classified the machine as fresh-8.4 (no existing_id yet; node-local has no host datadir) and its early gate optimistically waved it through — the late backstop that skip depends on never ran. The install reported success while Helm kept the amd64-only MySQL 5.7 image and the pod CrashLooped.

The fix

Each bypassing path now learns the engine that will actually run from its own source of truth, then calls _assert_engine_runs_on_this_archunchanged, before any deploy:

Path	Engine source	Reason
adopt	the live release's stored values (helm get values) — --reuse-values keeps whatever it already pins, so a fresh resolution would be wrong	existing-release
dev-mode	the engine the supplied values file declares (it is installed as-is)	values-file
normal	unchanged — _resolve_mysql_engine + assert	(as before)

_values_pin_mysql_84 is the one shared reader for "does this pin 8.4", matching both the quoted heredoc form (tag: "8.4") and the quote-stripped helm get values form (tag: 8.4). The sticky rule in _mysql_engine_decision now uses it too, so the three callers cannot drift. The adopt read is bounded (namespace probe) and fail-closed: an unreadable release degrades to the 5.7 gate rather than hanging or being assumed 8.4.

Why not a one-liner

Neither bypassing path set TB_MYSQL_ENGINE_RESOLVED, so simply calling the assert there is a no-op (its first line is [[ … == "5.7" ]] || return 0). And hoisting _resolve_mysql_engine earlier is wrong — its decision helper probes ${HOST_DATA_DIR}/${TB_NAMESPACE}/mysql, and TB_NAMESPACE is not assigned until later. Each path needs its own engine source.

Every deploy point is now gated

• _reconcile_adopted_client's helm upgrade ← adopt gate
• the shared helm upgrade --install ← dev-mode gate and normal-path assert

Testing

bats scripts/tests/install-client-helm.bats — 219 pass (210 existing + 9 new). make lint (parse + shellcheck --severity=error) clean; advisory shellcheck clean. preflight.bats, setup-macos-arch.bats, installer-parity.bats all green (the sticky-rule refactor is consumed by preflight).

New coverage:

• adopt on arm64 + 5.7 release + no emulation → refuses before reconcile
• adopt on arm64 + 8.4 release (quote-stripped tag: 8.4) + no emulation → reconciles
• adopt on arm64 + 5.7 release + emulation present → reconciles
• dev-mode file pinning 5.7 on arm64 + no emulation → refuses before helm
• dev-mode file pinning 8.4 on arm64 + no emulation → installs
• unit tests for _values_pin_mysql_84 (quoted/unquoted/none/proximity) and _release_pins_mysql_84 (fail-closed on unreadable)

Scope

Bash installer only. The PowerShell installer gates Windows/arm64 via Docker Desktop emulation (a different mechanism) and has no equivalent MySQL-engine adopt/values-file arch gate.

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes installer gating on adopt and dev-mode paths—wrong 8.4/5.7 detection could refuse valid installs or allow CrashLoops, but behavior mirrors the existing normal path with extensive bats coverage.

Overview
Fixes arm64 installs that reported success then CrashLooped when adopt-reconcile or TRACEBLOC_VALUES_FILE deployed the amd64-only MySQL 5.7 image without running the late arch gate that the normal path already had.

Shared engine detection adds _values_pin_mysql_84 (structural mysqlClient block parse from stdin—comments ignored, exact tag: 8.4 with empty digest only) and _release_pins_mysql_84 (bounded helm get values, fail-closed to 5.7). Sticky 8.4 detection in _mysql_engine_decision uses the same reader instead of a fragile grep -A window.

Before any Helm deploy on adopt and dev-mode paths, the installer sets TB_MYSQL_ENGINE_RESOLVED from the release’s stored values or the supplied file (not a fresh auto resolution), then calls _assert_engine_runs_on_this_arch unchanged—refusing 5.7 on arm64 without emulation before reconcile or upgrade.

Tests add unit coverage for the reader and end-to-end adopt/dev-mode refuse vs allow cases; manifest hash for install-client-helm.sh is updated.

^{Reviewed by Cursor Bugbot for commit 630e0d6. Bugbot is set up for automated code reviews on this repo. Configure here.}

[saadqbal]

Good bug and the right fix. Two paths reaching a Helm deploy without the arch gate is the kind of thing that only shows up as "install succeeded, pod CrashLooped", and the preflight optimism on a fresh-8.4 classification is what made the late backstop load-bearing in the first place.

Checked rather than credited: _assert_engine_runs_on_this_arch is defined once at :1495 and now called from three places — :1087 (adopt), :1682 (dev-mode), :1911 (normal). Called, not reimplemented, which is what keeps the three paths from drifting into three different notions of "runs here". Same for _values_pin_mysql_84: one definition at :1369, three consumers, and it streams rather than slurps.

Reading the adopt engine from helm get values rather than re-resolving is the part I'd have flagged if it were the other way round.--reuse-values preserves the stored values and does not pick up new chart defaults, so a fresh resolution would tell you what a new install would pin while the upgrade keeps what the release already has. Getting that backwards is how you assert against an engine that isn't the one about to run.

And the tests pin the bypass rather than the assert. adopt on arm64 with a 5.7 release + no emulation -> refuses BEFORE reconcile with [[ "$output" != *"VERIFY_CALLED"* ]] proves it aborted in adopt instead of falling through to the normal connect — delete the new call site and that reddens. The 8.4 counterpart stops it being a blanket refusal, and an unreadable release ... is fail-closed to NOT 8.4 puts the unknown on the safe side.

One residual, and you clearly already know about it since there's a test for the misread direction: _values_pin_mysql_84's -A 3 window is a formatting assumption. A legitimately-8.4 mysqlClient block with comments or extra keys pushing tag: past three lines reads as not-8.4, and because unknown fails closed that produces a spurious refusal rather than a wrong proceed. Safe by direction, and worth knowing it's the direction it fails in.

One check still pending, so nothing to approve against yet.

[saadqbal]

Green — 43 pass, 5 skipped, nothing pending, no open threads. Approving.

Signing what I verified last pass: the assert is defined once and called from all three paths rather than reimplemented, _values_pin_mysql_84 is one reader with three consumers, and the adopt path reads the engine from helm get values because --reuse-values keeps what the release already pins — asserting against a fresh resolution would have checked an engine that wasn't the one about to run.

And the tests pin the bypass, not the assert: adopt on arm64 with a 5.7 release -> refuses BEFORE reconcile, with the VERIFY_CALLED absence proving it aborted in adopt rather than falling through to the normal connect. Delete either new call site and that reddens.

The -A 3 window in _values_pin_mysql_84 stays as I described it — a formatting assumption that fails toward a spurious refusal rather than a wrong proceed, with a test for the misread direction already there. Not a blocker, just the direction it fails in.

[saadqbal]

Retracting my approval two minutes ago — it was wrong on both counts.

Procedurally: I ran the thread check and the approve in one batch instead of gating on the result, so I approved over an open Bugbot thread. That is exactly the signal the gate exists to prevent, and the approval said "ready" on a PR that isn't.

Substantively, the finding is a sharper version of the residual I waved through as "safe by direction", and my framing was too generous. _values_pin_mysql_84's three-line window fits the installer's compact heredoc but not client/values.yaml, where comments push the real tag: outside it. So a TRACEBLOC_VALUES_FILE that follows the chart's own documented 8.4 opt-in classifies as 5.7, and on arm64 the dev-mode gate this PR adds then refuses a native 8.4 install. That is not a theoretical spurious refusal — it is a refusal on the supported path, introduced by this change.

And the obvious fix is the trap: those same mysqlClient comments carry a decoy tag: "8.4", so widening -A reads the default 5.7 pin as 8.4 and skips the gate. Fail-closed becomes fail-open. The two directions are not symmetric and picking either window is choosing which one to be wrong in.

The tests can't see it because they feed compact snippets only. A case built from the real client/values.yaml — both the default and the 8.4 opt-in — is what would have caught this, and is what makes any fix here checkable rather than argued.

The rest of the PR stands as I reviewed it: the assert called once from three paths, the one shared reader, helm get values for the adopt engine, and tests that pin the bypass rather than the assert. This is one reader with a shape assumption, not a problem with the fix's design.

[aptracebloc]

@saadqbal thanks — you were right on both counts, and the retraction reasoning is exactly the shape of the bug.

Fixed in f8b09fb (the reader) + 207b484 (the manifest hash):

• The reader is now structural, not a line window._values_pin_mysql_84 walks the indent-delimited mysqlClient block, drops comment/blank lines, and matches the real tag: by exact value. That closes both directions you named: the decoy # tag: "8.4" is a comment so it is dropped (no fail-open on the 5.7 default), and the real tag: is found wherever it sits under the block (no false refusal of a documented 8.4 opt-in on arm64). 8.40/8.4.1 are no longer read as 8.4 either.
• Tests are built from the real client/values.yaml, per your ask — not compact snippets: the default (tag: "prod" + the decoy) reads as NOT 8.4, and the chart’s own documented opt-in applied to that file reads as 8.4. Plus block-scoping and exact-value cases. Full suite 222/222; drift/lint/shellcheck green.

The rest is unchanged from what you approved (assert called once from three paths, one shared reader, helm get values for the adopt engine, tests that pin the bypass). Ready for another look when you have a minute.

— drafted with Claude Code

[LukasWodka]

Picking this up as reviewer while Asad is out. Reviewed f8b09fb9.

The structural rewrite is a real improvement and I'd have asked for it too._values_pin_mysql_84 now walks the YAML by indent — finds the mysqlClient: block, tracks base indent, exits the block on dedent, and only then reads tag:. It handles the things a line window gets wrong: comment lines (so the decoy # tag: 8.4 can't fool it), blank lines, inline trailing comments, and quoted-or-unquoted values. Re-entering on a second mysqlClient: at the same indent is a nice touch. That's parsing the declaration rather than pattern-matching near it.

Reading the engine from the file as-is rather than re-resolving is also the right call, and the comment says why in one line: a fresh resolution could pick 8.4 while the file pins the amd64-only 5.7, and the file is what will actually run.

On Bugbot's open "Arch gate ignores digest-wins pin" — I couldn't verify the mechanism, and I'd rather say that than guess.

The finding is coherent on its face: if the chart's image helper lets a set digest win over tag, then a values file carrying tag: 8.4 plus a digest pointing at the amd64-only 5.7 image passes the gate and CrashLoops on arm64 — the exact operator-visible failure this PR exists to prevent.

What I could not confirm is the premise. Searching the repo for a mysqlClient digest path turns up nothing; the digest handling I can find is images.ingestor.prodDigest, which is a different image and a different flow. That is not evidence Bugbot is wrong — the code search index has been visibly stale for this org today — it just means I can't stand behind it either way.

So, concretely: does the chart template honour images.mysqlClient.digest, and does it take precedence over tag? If yes, the gate needs to read whichever field actually decides the image, and _values_pin_mysql_84 should probably refuse (or warn) when a digest is set at all, since a digest is opaque to a tag-based engine check. If no — if mysqlClient has no digest path — then the finding doesn't apply and it's worth saying so on the thread so it can be closed with a reason rather than left hanging.

You know this chart far better than I do; a one-line answer either way settles it.

---

Process note, because it will block you and isn't your fault. Asad's CHANGES_REQUESTED from 2026-08-25 was made against 207b4849, and you've since pushed f8b09fb9. GitHub keeps that block until he re-reviews or someone dismisses it, and he's away today. So even with the Bugbot thread closed and CI green, this won't merge on the strength of my approval alone — it needs @LukasWodka to dismiss the stale review, or Asad on his return. Flagging now rather than at merge time.

Nothing further from me until the digest question is answered.

[aptracebloc]

bugbot run

[aptracebloc]

@LukasWodka thanks for the careful review — and you asked exactly the right question. Yes, the chart honours images.mysqlClient.digest, and the digest wins over the tag. Your search index was stale; here is the path:

• client/templates/mysql-deployment.yaml:123 renders the mysql image via include "tracebloc.image" (dict … "tag" (…mysqlClient.tag) "digest" .Values.images.mysqlClient.digest …).
• tracebloc.image (client/templates/_helpers.tpl:507-514): {{- if $digest -}}{{ repository }}@{{ $digest }}{{- else -}}{{ repository }}:{{ tag }}{{- end -}} — a set digest is emitted and the tag is ignored.
• The default pin (client/values.yaml:882) is a single-arch amd64 5.7 digest.

So Bugbot was correct: tag: "8.4" + a non-empty (5.7) digest actually runs 5.7, and a tag-only gate would wave it through → arm64 CrashLoop.

Fixed in 630e0d6 (pushed just after your review), which does what you proposed: _values_pin_mysql_84 now reads the digest too and reports 8.4 only when tag is 8.4 AND the digest is empty/absent — the documented opt-in is exactly tag: "8.4" + digest: "", which is how our heredoc writes it. A set digest is an opaque sha256 we cannot decode, so it is treated as not-provably-8.4 and the 5.7 arch gate runs (fail closed — refuse rather than CrashLoop). Two new tests cover it: tag: 8.4 + a non-empty digest, and the real values.yaml with only the tag flipped (5.7 digest kept) — both read as NOT 8.4. I replied on and resolved the Bugbot thread.

Re-requesting your review of 630e0d6. And thanks for the process note on Asad’s stale CHANGES_REQUESTED — understood that it needs your dismissal or Asad on his return; I won’t touch his review.

— drafted with Claude Code

[cursor]

✅ 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 630e0d6. Configure here.}

[aptracebloc]

@LukasWodka — flagging the one thing standing between this and merge, for you to decide as the covering reviewer.

Everything actionable is resolved:

• Your digest question — answered above with the file:line evidence; fixed in 630e0d6 (reads the digest, reports 8.4 only when tag: 8.4 + empty digest, fail-closed otherwise).
• All three Bugbot findings — fixed and their threads resolved (0 open).
• CI green, 0 unresolved review threads, no conflicts. Full bats suite 224/224.

The only remaining blocker is a stale review, not code: Asad’s CHANGES_REQUESTED was filed against 207b48494; head is now 630e0d6 (two commits past it, and those two commits are exactly what his review asked for). GitHub keeps reviewDecision = CHANGES_REQUESTED until that review is dismissed or Asad re-reviews — so even with your approval and green CI, it won’t merge on its own.

Since you’re covering for Asad and his points are addressed: if you’re comfortable, dismissing his stale review (Files changed → his review → "Dismiss review", reason e.g. "addressed in 630e0d6; author out") and approving would unblock the merge. Entirely your call — I won’t dismiss another reviewer’s review myself, and if you’d rather wait for Asad to return that’s fine too. Just flagging so it doesn’t sit unnoticed until he’s back.

Once it’s approved + the stale review cleared, it’ll merge automatically on my side.

— drafted with Claude Code

[LukasWodka]

Re-reviewed 630e0d6f. That answers my question, and the fix is right. I asked whether the chart honours images.mysqlClient.digest over tag; it does, so Bugbot's finding was real and reading the tag alone would have skipped the gate.

The resolution is conservative in the correct direction. The whole decision is one line:

functionflush() { if (inblk && tag =="8.4"&& digest =="") found =1 }

A set digest means "I can't confirm this is 8.4", which falls through to 5.7 and runs the arch gate rather than skipping it. That's the right way round: an opaque digest is unknowable from a values file, so the safe reading is the one that checks. I traced the block bookkeeping too — flush() fires both on dedent and at END, and a second mysqlClient: resets tag/digest, so a block at EOF and repeated blocks both decide correctly.

Two edges I checked rather than assumed: digest: "" is treated as unset, which matches Helm; digest: null would read as set and gate unnecessarily — wrong-but-safe, and not worth code.

The second test is the one that earns its keep:

sed 's/tag: "prod"/tag: "8.4"/'"$src">"$vf"# tag flipped, digest untouched

It flips the tag on the real shipped client/values.yaml rather than a synthetic fixture, so it can't drift from what the chart actually pins — if someone changes the default image, this test changes with it. The synthetic tag: "8.4" + digest: "sha256:deadbeef" case covers the rule directly, and the pair together is exactly the derived/independent split that makes a guard hold.

I'm satisfied on the merits and the gate is otherwise clean: 45 checks pass, 4 skip, zero open threads from anyone, no conflict.

---

The only thing blocking is a stale review.reviewDecision is still CHANGES_REQUESTED from Asad's review on 207b4849 — three commits back, before both the structural read and this digest fix. He's out today, so it won't clear on its own, and an approval from me wouldn't override it either.

@LukasWodka — this one is ready apart from that; it needs the stale review dismissed, or Asad on his return.

[saadqbal]

This is a better fix than what I asked for, and I checked the implementation against its own comment rather than trusting the message — which matters here, because "the comment describes behaviour the code doesn't have" has been the recurring defect in this area.

It matches, line for line. Comment and blank lines are dropped first, so the decoy # tag: "8.4" in client/values.yaml goes with them rather than being read as the pin. The block is indent-delimited with flush() called on dedent, so sibling mysqlClient: blocks are each decided instead of bleeding into one another. The value match is exact — tag == "8.4" — which closes something I hadn't raised: 8.40 and 8.4.1 are no longer prefix-matches. And END { flush(); exit(...) } means it reads to EOF and never early-exits, so a producer piping in (helm get values) can't be SIGPIPE'd, preserving the backend#1778 property the old grep -A 3 had for a different reason.

Naming the failure in both directions in the comment is the part worth keeping: too narrow refuses a legitimate 8.4 opt-in on arm64, too wide reads the decoy and skips the gate on a 5.7 default. The second is the fail-open, and stating that a widened window fails open is what stops someone "fixing" this later by bumping -A 3 to -A 10.

630e0d6f making a pinned digest suppress the tag-based verdict is right and conservative in the correct direction: a digest could resolve to anything, so "I cannot tell" answers no rather than yes, and the gate applies. That's the same instinct as the rest of this — an absence doesn't approve.

Lifting my change-request. Everything green, no open threads.

[LukasWodka]

Approving 630e0d6f — 45 checks pass, zero open threads, mergeStateStatus=CLEAN.

I said on my last pass that I was satisfied on the merits and that the only thing standing in the way was a stale CHANGES_REQUESTED from three commits back. @saadqbal has now re-reviewed and approved, so that's cleared and this closes my side of it. Head is unchanged from the commit I reviewed.

Recording what settled it, since the thread is long:

The digest question was the real one, and the answer changed the fix. I asked whether the chart honours images.mysqlClient.digest over tag; it does, so reading the tag alone would have skipped the arch gate on exactly the configuration that needed it. The resolution is one line and fails in the safe direction:

functionflush() { if (inblk && tag =="8.4"&& digest =="") found =1 }

A set digest means "can't confirm this is 8.4", which falls through to 5.7 and runs the gate rather than skipping it. An opaque digest is unknowable from a values file, so the only safe reading is the one that checks.

The structural YAML read replaced a line window, which is what made the whole thing trustworthy — indent-tracked block entry and exit, comment and blank lines dropped, inline comments and quotes stripped, and a second mysqlClient: resetting state. I traced the bookkeeping: flush() fires both on dedent and at END, so a block at EOF and repeated blocks both decide correctly.

And the test that earns its keep flips the tag on the real shipped client/values.yaml rather than a synthetic fixture — so if the default image ever changes, the test changes with it instead of quietly passing against a stale copy. Paired with the synthetic tag: "8.4" + digest: "sha256:deadbeef" case, that's the derived/independent split that makes a guard hold.

Value: an arm64 operator adopting an amd64-only 5.7 release, or installing a values file that pins one, now gets told before the install rather than after a CrashLoop.