fix(ujust): use bootc status as source of truth for live image tag - #942
fix(ujust): use bootc status as source of truth for live image tag#942castrojo wants to merge 2 commits into
Conversation
image-info.json is baked at build time and describes the image the filesystem was built as, not what is booted. After a rebase it goes stale, so ujust reported a different stream than bootc status. Add /usr/libexec/ublue-image-resolve, which reads .status.booted.image.image.image from `bootc status --json` and falls back to image-info.json when bootc is absent, exits non-zero, emits unusable JSON, or reports no booted image. Ref parsing splits on the final path component so registry ports and @sha256 digests are handled. ublue-image-info.sh, changelog.just, and toggle-testing now consume the resolver instead of parsing image-info.json directly. The ublue-image-info.sh output contract (name:tag + lock glyph) is unchanged. Closes #820 Assisted-by: Claude Opus 5 via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tatus # Conflicts: # docs/TESTING.md
hanthor
left a comment
There was a problem hiding this comment.
Approving — the diagnosis is right, the centralization is the correct shape, and I traced the ref-parsing logic case by case rather than trusting the description. Three notes, one of which I'd like confirmed before this lands.
Parsing verified. I walked ref_path / ref_basename / strip_transport through the awkward inputs:
| input | result |
|---|---|
registry:5000/foo/bar:tag |
path registry:5000/foo/bar, tag tag ✅ |
registry:5000/foo/bar (no tag) |
path unchanged, port intact ✅ |
repo:tag@sha256:… |
digest stripped before split ✅ |
bluefin:latest (no slash) |
path bluefin, tag latest ✅ |
ostree-image-signed:docker://ghcr.io/… |
##*:// handles it ✅ |
ostree-unverified-registry:ghcr.io/foo |
transport regex strips it ✅ |
The [^0-9] guard in strip_transport is what keeps registry:5000/… from being eaten — that's a subtle case and it's handled correctly. Agreed this is a real improvement on the old s|^[a-z-]+:||.
1. Please confirm bootc status --json works unprivileged — the whole fix rests on it.
The PR body says it "requires root". If that's true on a real host, the headline benefit doesn't reach the most visible consumer: fastfetch.jsonc:23 calls /usr/bin/ublue-image-info.sh in the user's shell, so bootc status would fail there, silently fall through to the baked file, and post-rebase fastfetch would keep reporting the stale stream — exactly the bug being fixed.
For what it's worth, bootc-status.8 documents no root requirement and explicitly recommends bootc status --json as the way to programmatically detect a bootc deployment, which suggests it is unprivileged-safe. But since you verified the schema from source specifically because you couldn't run it, this is the one thing I can't confirm from here. A single bootc status --json as a normal user on a Bluefin box settles it.
2. Two resolver invocations means two bootc status --json per shell.
IMAGE_NAME="$("${IMAGE_RESOLVE}" image-name ...)"
IMAGE_TAG="$("${IMAGE_RESOLVE}" image-tag ...)"Each call independently runs bootc status --json and pipes through jq. Since fastfetch runs this on every new terminal, that's 2× bootc + 2× jq per shell on top of the existing rpm-ostree status --booted. Worth collapsing — either a combined mode (ublue-image-resolve image-name-tag emitting name:tag), or have the resolver cache the JSON in an env var the caller can pass through. Not blocking, but it's cheap to fix now and annoying to notice later as "terminals got slower".
3. Minor: half-resolved output.
if [[ -n "${IMAGE_NAME}" || -n "${IMAGE_TAG}" ]]; then
echo -n "${IMAGE_NAME}:${IMAGE_TAG}"|| means one empty side still prints, yielding bluefin: or :stable. Reachable with a digest-pinned bootc ref on a host with no baked file (tag unresolvable, name fine). && — or emitting just the non-empty part — would be tidier.
Good call flagging bonedigger-report as the same staleness class rather than scope-creeping into it, and the test_changelog.bats bctl drive-by is a real fix (brew ships one, so that filter was wrong for anyone with brew — which is everyone on Bluefin).
Housekeeping: currently DIRTY. Your branch-state note anticipates the docs/TESTING.md collision with #941, so that's understood — just needs the resolve.
…fin#943) ## Root cause — confirmed against bash-preexec 0.6.0 source Verified by reading the tagged 0.6.0 source directly, not from memory: - L74: the installer is deferred by appending a string to `PROMPT_COMMAND`: `__bp_trap_string="$(trap -p DEBUG)"; trap - DEBUG; __bp_install` - L289-293: `__bp_install()` **returns early** if `"${PROMPT_COMMAND[*]}"` already contains `__bp_precmd_invoke_cmd` - The self-removal code reads `existing_prompt_command="${PROMPT_COMMAND:-}"` and later assigns `PROMPT_COMMAND='__bp_precmd_invoke_cmd'` On Fedora, `PROMPT_COMMAND` is an **array** (bash ≥ 5.1). `${PROMPT_COMMAND}` and bare assignment address **element `[0]` only**. So once any other hook (mise, direnv, starship, vte, systemd, zoxide) pushes the installer out of element `[0]`, it is **never removed** — and runs on every prompt. Its first act is `trap - DEBUG`; `__bp_install` then returns early. From prompt 2 onward the DEBUG trap is permanently empty. That is exactly the reporter's `declare -p` output: `__bp_precmd_invoke_cmd` in `[0]`, installer stranded in `[2]`, atuin's hook absent. Reproduced empirically in a real prompt loop: `cycle1 trap: [trap -- '__bp_preexec_invoke_exec "$_"' DEBUG]`, `cycle2: []`, `cycle3: []`. With this fix, all three cycles retain the trap. Matches upstream rcaloras/bash-preexec#188 (fixed upstream but insufficient alone) and #186 (still open). ## Non-obvious discovery Bash does not inherit the DEBUG trap into functions without `functrace`: `trap -p DEBUG` **inside a function always returns empty**, and `trap - DEBUG` inside a function is a no-op — but `trap ... DEBUG` inside a function *does* set it globally. Two consequences: a detector-guard is impossible (hence the unconditional re-arm), and it invalidated the first simulation, which passed **vacuously**. Written up in `docs/skills/shell-scripts.md`. ## Changes - New `system_files/shared/.../bling/bash-preexec-rearm.sh` — re-arms the exact trap bash-preexec installs, once per prompt - `bling.sh` sources it **last**, under the bash guard, so the hook is the final `PROMPT_COMMAND` entry `bling.sh` stays POSIX (it is under an sh-dialect shellcheck gate); the bash-only logic lives in the sibling file. `BLING_DIR` allows bats to point at the repo tree. No vendored copy of bash-preexec. ## Tests — 14 cases, all pass Tests **1 and 4 are negative controls** that pin the bash-preexec stand-in to the real defect, so the positive cases cannot pass vacuously. Covered: array `PROMPT_COMMAND` trap survival, atuin-style hook still firing, idempotency on double-source, scalar `PROMPT_COMMAND` (no regression), bash-preexec absent, readonly `PROMPT_COMMAND`, missing helper, zsh source-safety. Also green: `bash -n` / `sh -n`, both CI shellcheck invocations, `just check`, `pre-commit run --all-files`. ## Confidence and limits High that the DEBUG trap now survives — the failure and its repair are both directly reproduced. Two things only real hardware can prove: 1. atuin actually writing records end-to-end (tests use a preexec stub, not atuin 18.18.1) 2. that no downstream hook on a real session appends its own `trap - DEBUG` **after** our entry Ordering is safe for everything bling itself initializes. ## Reviewer note The re-install is **unconditional** per prompt, forced by the functrace scoping above; it matches the reporter's validated workaround. A third party's own raw DEBUG trap set after bling would be overwritten each prompt — but such a trap already breaks bash-preexec today. ## Blast radius — highest in this batch `bling.sh` is sourced by **every interactive bash shell** on bluefin, bluefin-lts, and dakota. Recommend lab validation before merge. Closes projectbluefin#869 --- ## Branch state Updated onto `main` after projectbluefin#926 landed (via `gh pr update-branch`). No conflict — this PR's `docs/TESTING.md` row and `Justfile` entry merged cleanly. ## Overlap with the sibling PRs in this batch **projectbluefin#942** also edits `docs/skills/shell-scripts.md`, but adds different sections (this PR: `Bash DEBUG traps are invisible inside functions`, `POSIX-sh files cannot hold bash array code`; projectbluefin#942: `image-info.json is build-time state`). Verified non-conflicting with a real sequential merge. No file overlap with projectbluefin#941. ## Pre-merge checklist for the reviewer - [ ] Lab-validate on real hardware — the tests use a preexec stub, not atuin 18.18.1, so end-to-end recording is unproven - [ ] Confirm the unconditional per-prompt trap re-install is acceptable (see reviewer note above; a detector-guard is impossible due to bash's functrace scoping) Co-authored-by: Jorge Castro <jorge@projectbluefin.io> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Problem
/usr/share/ublue-os/image-info.jsonis baked at build time and describes the image the filesystem was built as. After a rebase it no longer matches what is booted, so every consumer reports the build-time stream instead of the live one.Confirmed on this host: the baked file reads
{"image-name":"dakota-gaming","image-tag":"latest",...}and itsimage-refcarries no tag at all.Changes
New
system_files/shared/usr/libexec/ublue-image-resolve <image-name|image-tag|image-path>— bootc-first, baked-file fallback. Consumers updated:ublue-image-info.sh,changelog.just,system.just(toggle-testing).Centralizing avoids three divergent copies of bootc-JSON parsing.
Schema verification
bootc status --jsonrequires root, so the schema was verified from bootc source (crates/lib/src/spec.rs, v1.16.6):HostStatus.booted→BootEntry.image→ImageStatus.image→ImageReference.image. The jq path is.status.booted.image.image.image.Note: the existing
bonedigger-reportuses.status.booted.imageDigest, a different field — not a usable precedent for the ref.Ref parsing
Splits on the final path component, so both
registry:5000/foo/bar:tagandrepo:tag@sha256:...parse correctly. This also fixes a latent bug in the oldsedtransport-strip (s|^[a-z-]+:||would have eatenregistry:).Output contract — unchanged
ublue-image-info.shstill emitsname:tagplus the lock glyph; verified live (dakota-gaming:latest 🔓). Consumers checked:fastfetch.jsonc,tests/test_ublue_image_info.bats,docs/TESTING.md,docs/contributing/style-guide.md. Only stderr noise on a missing file/jq changed.Tests
tests/test_ublue_image_resolve.bats— 23 tests: live-beats-stale, tag+digest, digest-only, ported registry, bootc absent / non-zero / empty / invalid JSON /booted:null, transport stripping, jq absent, bad field. Plus an integration test intest_ublue_image_info.bats. 43/43 pass across affected files. Wired intoJustfile,unit-tests.yml, anddocs/TESTING.md.Reviewer notes
image-pathinstead ofimage-ref. bootc refs include a tag; the bakedimage-refdoes not. Feeding a bootc ref through the old sed would produce…/bluefin-lts:stable:testinginbootc switch.image-pathreturns a tagless registry path and centralizes that.test_changelog.batsfiltered only~/.local/binfor a hostbctl, but brew ships one too — all 15 tests were failing onmainlocally. Now filters any PATH dir providingbctl.tests/test_update_just.bats(fail identically onmain).bonedigger-reportreads the same baked file and has the same staleness class, affecting bug-repo routing.Blast radius
system_files/shared/→ bluefin + bluefin-lts + dakota.Closes #820
Branch state
Merged
mainatd5b56d85after #926 landed. The only collision was aone-line conflict in the
docs/TESTING.mdtest table — both PRs append a row.Resolved by keeping both rows.
Re-verified after that merge:
test_ublue_image_resolve.bats23/23 passand
test_theming_hook.bats(from #926) 5/5 pass.Overlap with the sibling PRs in this batch
brew tap --trustwithbrew tap+brew trust#941 touchessystem_files/bluefin/.../just/system.justat lines 149/152(
brew tap); this PR touches line 249 (toggle-testing). Git auto-mergesthose hunks cleanly — verified with a real sequential merge, not
merge-tree.docs/skills/shell-scripts.md, but adds differentsections (
Bash DEBUG traps…,POSIX-sh files…vs this PR'simage-info.json is build-time state…). No semantic overlap.Whichever of #941/#942 merges second will need one more
docs/TESTING.mdresolve. That is inherent to the shared table, not a defect in either branch.