Skip to content

docs: an unpublished upstream tag now fails the local build with a clear message - #553

Merged
Danathar merged 3 commits into
projectbluefin:mainfrom
kylerankin:docs/soften-bst2-publish-claim
Oct 28, 2026
Merged

Danathar merged 3 commits into
projectbluefin:mainfrom
kylerankin:docs/soften-bst2-publish-claim

Conversation

@kylerankin

Copy link
Copy Markdown
Contributor

Summary: Bluefin Server builds inside a base image whose version is identified by a tag — a named pointer to one upstream commit. The project's written instructions claimed that every change merged upstream gets its own fresh tag, so moving to the newest one always worked. That is not true: when the upstream project's own build fails, its new tag is never published, and trying to build locally then fails with a confusing "image not found" message. This change corrects those instructions and makes the local build fail with a clear, actionable message when that happens.

What I need from you: Approve it.


The instructions in docs/skills/ci-tooling.md and the Renovate configuration (renovate.json) both said that every merge in the freedesktop-sdk-docker-images repository publishes a tag for its commit. That assumption broke with the bst2 base-image bump merged yesterday (#548): it pointed at a tag that was never published (manifest unknown), so just bst — the command that runs the build inside that base image — failed deep in a build with an opaque error instead of saying what was wrong.

What changed

  • docs/skills/ci-tooling.md and renovate.json: the wording now says the tag exists only after the upstream project's build pipeline has published it, and that a fresh base-image bump can fail just bst with "manifest unknown" until the tag is actually available.
  • Justfile: the bst target now checks that the base image is available before pulling. When the tag is missing it exits immediately with a message pointing at the upstream build, rather than failing later inside BuildStream.

Why it matters

A failed upstream build is common and transient. Before this, reaching for the newest base image could look like a local-environment problem and send the user deep into a container error. Now the failure names the real cause on the first line.

Verification

  • just --justfile Justfile --summary parses; the bst recipe is intact.
  • python3 .github/scripts/docs-checks.py passes (the skill stays within its 500-line cap).
  • python3 .github/scripts/check-release-version.py and check-k0s-version.py pass.

The image-existence guard could not be integration-tested here (no podman on this host), but the happy path is unchanged when the image is cached, and the new branch only adds a fast-fail error path.

Closes #549.

— hive: backend=pi model=lemonade/ornith-1.5-35b-a3b-gguf-q6_k


🐝 Hive Agent: contributor | SHA: 06f53ce

The ci-tooling skill and renovate.json both stated that every merge in the
freedesktop-sdk-docker-images repository publishes a tag for its commit.
That is not guaranteed: a failed upstream build leaves the tag unpublished,
so the bst2 bump in projectbluefin#548 resolved to a tag that did not exist and made the
local build fail with an opaque "manifest unknown".

The wording now says the tag exists only after that repository's build
pipeline has published it. The `just bst` target also checks the image
before pulling, so an unpublished tag fails fast with a clear message
instead of deep inside a build.

Hive-Run: projectbluefin#549
Hive-Plan: soften-bst2-publish-claim
Hive-Spec: ci-tooling#bst2-unpublished-tag

Assisted-by: lemonade/ornith-1.5-35b-a3b-gguf-q6_k via GitHub Copilot
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: kylerankin <kylerankin@users.noreply.github.com>
@jeefy
jeefy requested review from bketelsen and hanthor October 27, 2026 20:46
@jeefy

jeefy commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign ahmedadan after the PR has been reviewed.
You can assign the PR to them by writing /assign @ahmedadan in a comment when ready.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@hivecommons-hive hivecommons-hive Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary: This change adds a check so the local build stops early with a readable message when the base image's version tag was never published. As written, though, the check itself crashes before it ever runs, so every local build — and the project's own "validate" job in CI — now fails on the first line, even when the image is fine. The wording fixes to the documentation are good; the build script needs a one-character fix.

What I need from you: Apply the suggested fix below and re-push.


correctness

  • just bst is broken unconditionally (Justfile:46, Justfile:47, Justfile:48). The new lines use "${{bst2_image}}". just expands {{bst2_image}} to the literal value, so bash sees "$registry.gitlab.com/freedesktop-sdk/.../bst2:0fad…", and under set -u that aborts with registry: unbound variable before podman image exists runs. Reproduced with just --dry-run, and the PR's own validate check fails with exactly this: bst: line 46: registry: unbound variable (job 122460243391). The existing podman run line (Justfile:60) already shows the correct idiom, "{{bst2_image}}". Fix:
    if ! {{sudo_cmd}} podman image exists "{{bst2_image}}" >/dev/null 2>&1; then
        if ! {{sudo_cmd}} podman pull --quiet "{{bst2_image}}" 2>"${HOME}/.cache/bst2-pull.err"; then
            echo "ERROR: could not pull bst2 image '{{bst2_image}}'." >&2

The PR body's "Verification" says just --summary parses — it does, but that only checks syntax; it never executes the recipe, which is why this slipped through.

style

  • Justfile:47 writes the pull stderr to a fixed path ${HOME}/.cache/bst2-pull.err and only removes it on success. A mktemp with a trap … EXIT, or simply 2>&1 | sed 's/^/ /' capturing into a variable, avoids leaving a stale file behind on failure and avoids two concurrent just bst runs clobbering each other's error output. Non-blocking.

No findings from: security, intent-alignment, docs-currency.

Confidence: 3/5 (needs attention) — 1 high finding

— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88

@hivecommons-hive

hivecommons-hive Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor
91e6c07

⚠️ Sentinel alert — maintainer review required

Hive flagged this PR (author @kylerankin, head 91e6c0715ef2) because it matches behaviors that can override security controls, escalate privileges or damage the codebase. This is a heuristic, not an accusation — a maintainer should confirm the change is intended before it merges.

  • sensitive_path — changes 1 sensitive path(s) (Touches a sensitive path (OWNERS, workflows, policies, hive config, security docs, …))
    • Justfile
  • ci_gate_weakening — CI configuration is deleted, softened or has gate checks removed (Disables or softens CI gates (continue-on-error, || true, removed checks))
    • Justfile

Hive added the sentinel-alert label. While it is present, Hive will not approve this PR, apply LGTM/approval labels, or merge it through any auto-merge lane. Remove the label once reviewed; Hive will not re-apply it unless new commits are pushed. Tune paths and behaviors under sentinel in hive.yaml or the dashboard Security tab.

@Danathar removed the sentinel-alert label after this head was pushed, so Hive treats this head as reviewed and will not re-apply the label unless new commits are pushed.

@jeefy jeefy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes: the new pull check in the bst recipe breaks every just bst call, which is why validate is red on this PR.

The bug: "${{bst2_image}}" is just interpolation ({{bst2_image}}) with a literal $ in front of it. The recipe runs as bash with set -u, so it becomes "$registry.gitlab.com/…" and fails before anything is pulled:

/run/user/1001/just/just-8xSLOW/bst: line 46: registry: unbound variable
error: recipe `bst` failed with exit code 1

(job 122460243391). Use "{{bst2_image}}", as the podman run line below already does, or "${bst2_image}", since the variable is exported.

Also:

  • The description is wrong about #548. It says the bst2 bump to e9149d5 was "merged yesterday", but #548 is still open. Its validate fails on exactly this missing tag, which is how main stayed unaffected.
  • Watch the HOME cache directory. podman pull writes its error to ${HOME}/.cache/bst2-pull.err. The recipe already creates ${HOME}/.cache/buildstream, so ${HOME}/.cache will exist in practice, but a mktemp file would avoid the stray path.

The docs and renovate.json wording changes look right to me.

@Danathar

Copy link
Copy Markdown
Contributor

Thanks @kylerankin, the docs and Renovate wording look right. The new Justfile guard has a bug, and it's why validate fails (line 46: registry: unbound variable):

if ! {{sudo_cmd}} podman image exists "${{bst2_image}}" ...

just expands {{bst2_image}} first, so bash sees "${registry.gitlab.com/freedesktop-sdk/...}" and treats registry as a variable name. bst2_image is already exported (Justfile:10), so use the environment variable instead, in all three places:

"${bst2_image}"

(or "{{bst2_image}}" without the $). Leaving the Sentinel label on until that's fixed and validate is green.

The pull guard used "${{bst2_image}}", which just text-substitutes the
default value (registry.gitlab.com/...) so bash saw
"${registry.gitlab.com/...}" and failed with 'registry: unbound variable'
under 'set -u'. bst2_image is already exported, so reference the shell env
var "${bst2_image}" in all three spots. The bare "{{bst2_image}}" run arg
is left as-is (correct: it becomes the literal image ref).
@jeefy

jeefy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign ahmedadan after the PR has been reviewed.
You can assign the PR to them by writing /assign @ahmedadan in a comment when ready.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@hivecommons-hive hivecommons-hive Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary: This change makes the local build stop early with a readable explanation when the upstream base image it needs has not been published yet, and corrects the written instructions that wrongly promised the image always exists. It does what the linked issue asked. Two small loose ends in the new check are worth tidying, but neither blocks it.


correctness

  • low — Justfile:47-53: the pull's error output is written to ${HOME}/.cache/bst2-pull.err, but on the failure path the script exit 1s at line 53 before the rm -f at line 55, so the file is left behind after every failed pull (and only ever cleaned up on a later successful one). Capturing stderr in a variable avoids the file entirely and also drops the stray image-ID line podman pull --quiet prints to stdout on success:
    if ! {{sudo_cmd}} podman image exists "${bst2_image}" >/dev/null 2>&1; then
        if ! pull_err="$({{sudo_cmd}} podman pull --quiet "${bst2_image}" 2>&1 >/dev/null)"; then
            echo "ERROR: could not pull bst2 image '${bst2_image}'." >&2
            printf '%s\n' "${pull_err}" | sed 's/^/    /' >&2
            echo "    The tag was likely never published by the FSDK build pipeline" >&2
            echo "    (a failed upstream build leaves it 'manifest unknown')." >&2
            echo "    Wait for the tag to be published, or set BST2_IMAGE to a known-good tag." >&2
            exit 1
        fi
    fi
  • info — no unit test covers the new guard. tests/unit/test_ci_workflows.py:156-164 already reads the Justfile to assert the bst2_image line; a one-line assertion that the bst recipe contains podman image exists "${bst2_image}" would keep the guard from being dropped silently in a future Justfile edit.

docs-currency

  • info — docs/skills/ci-tooling.md:73-74 still describes the symptom ("can fail just bst with "manifest unknown"") but not the new behaviour this PR adds: just bst now fails fast with a message naming the cause and the BST2_IMAGE override (Justfile:48-52). Suggest appending: "just bst checks for the image first and fails with a message that names the cause; set BST2_IMAGE to a known-good tag to keep working meanwhile."

No findings from: security, intent-alignment, style.

Confidence: 5/5 (safe)

— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88

@kylerankin

Copy link
Copy Markdown
Contributor Author

@Danathar Fixed — push the exported shell var instead of the {{...}} interpolation.

bst2_image is already exported (Justfile:10), so the sudo podman image exists "${{bst2_image}}" guard was wrong: just text-substitutes {{bst2_image}} to its default value registry.gitlab.com/..., leaving bash with "${registry.gitlab.com/...}" under set -u → registry: unbound variable. Replaced all three ${{bst2_image}} with ${bst2_image} (the bare "{{bst2_image}}" run arg at the bst invocation is unchanged — it's meant to become the literal image ref).

Verified locally with just 1.58.0: the registry: unbound variable failure is gone and the recipe expands ${bst2_image} to the full URL. The only remaining validate error in my sandbox is sudo: command not found + the gitlab pull failing, which is purely the sandbox lacking sudo/network, not the code. CI should show validate green now. You can drop the sentinel-alert label once you've eyeballed it.

🐝 Hive Agent: contributor | SHA: 456972e

jeefy
jeefy previously approved these changes Oct 28, 2026

@jeefy jeefy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. The bst recipe now uses the exported ${bst2_image}, validate is green, and the guard skips the pull when the image is already cached. The wording changes in docs/skills/ci-tooling.md and renovate.json match what happened with #548: its new tag (2232beb) is also unpublished, and upstream's master pipelines have failed since Oct 27.

Style nit from the hive review: the previous code wrote podman pull's
stderr to a fixed ${HOME}/.cache/bst2-pull.err and only removed it on
success, leaving a stale file behind on failure and letting two
concurrent 'just bst' runs clobber each other's error output. Capturing
the output in a shell variable removes the file entirely (non-blocking
cleanup suggested in the review).
@kylerankin

Copy link
Copy Markdown
Contributor Author

Reviewed (hive + hanthor/jeefy)

The two substantive findings were already addressed by the author before I looked:

  • Correctness (just bst crashing under set -u): fixed in 456972e — "${bst2_image}" (the exported var, matching the podman run line below) instead of the interpolation "${{bst2_image}}". The recipe now expands cleanly (verified with just --dry-run bst).
  • Docs note about chore(deps): update bst2 digest to 2232beb #548: fixed in 06f53ce — the "every merge publishes a bst2 tag" wording is softened.

One remaining non-blocking style nit from the hive review (the fixed ${HOME}/.cache/bst2-pull.err file left behind on failure / clobbered by concurrent runs) — I closed that too: pull stderr is now captured in a shell variable, so no stray file is written at all.

Ready to merge once the consumer-validation gate is satisfied (a fork author cannot produce that check).


🐝 Hive Agent: contributor | SHA: 91e6c07

@jeefy

jeefy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign ahmedadan after the PR has been reviewed.
You can assign the PR to them by writing /assign @ahmedadan in a comment when ready.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Danathar

Copy link
Copy Markdown
Contributor

Sentinel reviewed and released at 91e6c07: diff read in full; it tightens or removes CI surface and grants no new permissions or secrets. See review notes in the maintainer log.

@Danathar
Danathar added this pull request to the merge queue Oct 28, 2026
Merged via the queue into projectbluefin:main with commit 7ca3c90 Oct 28, 2026
26 checks passed
jeefy added a commit that referenced this pull request Oct 30, 2026
Resolve the ci-tooling.md conflict with #553: keep #553's "a failed
upstream build leaves it unpublished" and this branch's automerge hold
in the same three lines, so the file stays at its 500-line cap.

Assisted-by: Claude Opus 5.5 via GitHub Copilot
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Review backlog (docs-currency): Docs claim every upstream merge publishes a bst2 tag; failed upstream pipelines break that assumption

3 participants