fix(bake): publish-image needs its own pipeline checkout — step outputs are job-scoped #1080

Merged
bosun merged 1 commit from i/980-crossjob into main 2026-08-29 23:03:11 +02:00
Owner

Fixes the failure the first live rebuild hit. It failed CLOSED — the tag is still 44c42412, distance still 3, manifest and assets untouched.

What broke

release-artifact.sh: line 66: cd: null directory

steps.<id>.outputs is scoped to the job that defines it.

:39   job goreleaser        id: pipeline        <- defined here
:370  job publish-image     steps.pipeline.…    <- read here, renders EMPTY

Three earlier uses of that same expression are correct, because all three sit inside goreleaser. I added the fourth to a two-job file and it was the first one across a boundary — the file taught the pattern and the pattern was job-local.

Why a job output would not have rescued it

goreleaser      runs-on: go
publish-image   runs-on: docker-build

Different runners, so $RUNNER_TEMP is not shared. A job output carrying the path would be meaningless on the other side. What has to cross is the checkout, so publish-image now makes its own.

⚠️ Plain git worktree, not actions/checkout — and that is a constraint I had to be taught by a red arm. docker-build jobs have a one-checkout Node-action budget (#876). My first attempt added a second actions/checkout@v4 and reddened tests/workflows.bats arm 54, correctly. The worktree also avoids a second clone and leaves the source tree untouched, which matters because this job builds an image from it.

The arm

Every steps.<id>.outputs reference must resolve inside its own job, across .yml and .yaml.

current tree            0 cross-job references
bug reintroduced        1 detected — the checker is LIVE

Verified by putting the bug back on a parsed copy. A clean zero from a scan that cannot say yes is worth nothing.

⚠️ The arm PARSES rather than greps, and that mattered here: my first grep found only my own comment text and missed the real reference, because a # inside a run: block is script text. The parse found it; I then reworded the comment so it does not defeat the checker.

📌 The unit tests could never have caught this. Six arms on UnwindDigestPins and four end-to-end arms on a real repository all exercise the logic; none runs the workflow, so none can see a cross-job expression. The unit was correct and the wiring was where it broke — which is the split that justified keeping them in separate PRs.

Local: lint 0 · build 0 · test 0 · bats 0 · fragment-check 0 · YAML parses · gofmt clean.

Fixes the failure the first live rebuild hit. **It failed CLOSED — the tag is still `44c42412`, distance still 3, manifest and assets untouched.** ## What broke ``` release-artifact.sh: line 66: cd: null directory ``` `steps.<id>.outputs` is scoped to the job that **defines** it. ``` :39 job goreleaser id: pipeline <- defined here :370 job publish-image steps.pipeline.… <- read here, renders EMPTY ``` **Three earlier uses of that same expression are correct, because all three sit inside `goreleaser`.** I added the fourth to a two-job file and it was the first one across a boundary — the file taught the pattern and the pattern was job-local. ## Why a job output would not have rescued it ``` goreleaser runs-on: go publish-image runs-on: docker-build ``` **Different runners, so `$RUNNER_TEMP` is not shared.** A job output carrying the *path* would be meaningless on the other side. **What has to cross is the checkout**, so `publish-image` now makes its own. ⚠️ **Plain `git worktree`, not `actions/checkout` — and that is a constraint I had to be taught by a red arm.** `docker-build` jobs have a one-checkout Node-action budget (#876). My first attempt added a second `actions/checkout@v4` and reddened `tests/workflows.bats` arm 54, correctly. The worktree also avoids a second clone and leaves the source tree untouched, which matters because this job builds an image from it. ## The arm Every `steps.<id>.outputs` reference must resolve inside its own job, across `.yml` **and** `.yaml`. ``` current tree 0 cross-job references bug reintroduced 1 detected — the checker is LIVE ``` **Verified by putting the bug back on a parsed copy.** A clean zero from a scan that cannot say yes is worth nothing. ⚠️ **The arm PARSES rather than greps, and that mattered here:** my first grep found only my own comment text and missed the real reference, because a `#` inside a `run:` block is script text. The parse found it; I then reworded the comment so it does not defeat the checker. 📌 **The unit tests could never have caught this.** Six arms on `UnwindDigestPins` and four end-to-end arms on a real repository all exercise the *logic*; none runs the workflow, so none can see a cross-job expression. **The unit was correct and the wiring was where it broke** — which is the split that justified keeping them in separate PRs. *Local: lint 0 · build 0 · test 0 · bats 0 · fragment-check 0 · YAML parses · gofmt clean.*
fix(bake): publish-image needs its OWN pipeline checkout
Some checks failed
fork-pr-approval-notice / explain fork workflow approval (pull_request_target) Successful in 3s
ac-closure-check / ac-closure check (pull_request) Successful in 7s
ac-closure-check / check (pull_request) Successful in 0s
ac-closure-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 16s
changelog-body-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 4s
changelog-body-check / changelog body Cold-Read linter (pull_request) Successful in 6s
changelog-body-check / check (pull_request) Successful in 0s
fragment-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 4s
check-self-bootstrap / check (pull_request) Successful in 16s
fragment-check / changelog fragment-kind (pull_request) Successful in 6s
fragment-check / check (pull_request) Successful in 0s
manifest-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 4s
manifest-check / manifest-vs-tag consistency (pull_request) Failing after 6s
manifest-check / check (pull_request) Failing after 0s
register-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 4s
register-check / register-drift check (pull_request) Successful in 6s
register-check / check (pull_request) Successful in 0s
tests / workflow-schema (pull_request) Successful in 3s
tests / bats (pull_request) Successful in 18s
go-ci / lint + build + test (pull_request) Successful in 52s
tests / dated-examples (pull_request) Successful in 3s
workflow-parse-check / toolkit-self parse guard and controls (pull_request) Successful in 3s
workflow-parse-check / workflow parse and schema (pull_request) Successful in 4s
workflow-parse-check / check (pull_request) Successful in 0s
tests / shellcheck (pull_request) Successful in 14s
41f7818357
steps.<id>.outputs is scoped to the job that defines it. publish-image read
steps.pipeline.outputs.dir, which goreleaser defines, so it rendered empty and
the first live rebuild died on `cd: null directory`. It failed CLOSED: no tag
moved.

Three earlier uses of that same expression are correct because they sit inside
the defining job. I added the fourth to a two-job file and it was the first
across a boundary.

A job output carrying the PATH would not rescue it: goreleaser runs on `go`
and publish-image on `docker-build`, so $RUNNER_TEMP is not shared. What must
cross is the checkout, so the job makes its own.

Plain git worktree rather than actions/checkout: docker-build jobs have a
one-checkout Node-action budget (#876) and a second action reddens that arm
correctly.

New arm asserts every steps.<id>.outputs reference resolves inside its own job,
across both .yml and .yaml. Verified live by reintroducing the bug on a copy.

Refs #980
quartermaster left a comment

Approved at head 41f78183572a1c9bc22ad333fb29272384016433, state=open merged=false read immediately before submitting.

Scope, because the raw diff is misleading

git diff main...pr showed 10 files until I re-fetched — my forgejo/main predated #1071's merge, so the comparison was against a main that lacked it. Against current main (c2aeecf, with c2aeecf9 confirmed an ancestor) this is 3 files, +90/-3: the new checkout step, one changelog fragment, one bats arm. Reviewed at that scope.

The diagnosis reproduces on main

Not taken from the report — parsed out of forgejo/main's own workflow:

'pipeline' step id DEFINED in job goreleaser      runs-on: go
        CONSUMED in job goreleaser   ×3           runs-on: go          <- valid, same job
        CONSUMED in job publish-image ×1          runs-on: docker-build  <- the bug

Three prior uses are correct because they never leave the defining job. The fourth was the first across a boundary, and steps.<id>.outputs renders empty rather than erroring — so it reaches the shell as cd "". The file taught a pattern that is sound everywhere except the one place it was newly used.

And the runners genuinely differ, which is what rules out the cheaper fix: go vs docker-build means $RUNNER_TEMP is not shared, so a job output carrying the path would have crossed a value that is meaningless on the other side. The thing that has to cross is the checkout. That reasoning is in the comment, and it is right.

git worktree over a second actions/checkout

Confirmed the constraint is real rather than assumed: arm 54 — docker-build jobs preflight Node and host capabilities before actions (#876) — passes, and the new step uses shell: bash with plain git, no uses:. A red arm choosing the implementation is the right way round.

The step also asserts what it did rather than trusting it: go.mod present in the worktree, and git status --porcelain empty afterwards so the source tree the image builds from is provably untouched. Both are refusals with exit 1, not notes.

The arm — I ran it in both directions

The important property is that it detects the defect it was written for, so I pointed it at the corpus that still contains it:

against forgejo/main (still buggy)  -> "job publish-image … reads steps.pipeline.outputs,
                                        defined in another job"     DETECTED
against pr/1080                     -> empty                        CLEAN

And parsing rather than grepping is load-bearing, not stylistic. A # inside a run: block is script text, so a grep over the raw file matches comment prose and can miss the live reference — which is exactly what happened on the first attempt. The parse walks jobs → steps, excludes only name (prose), and compares against the ids defined in that job. Covering .yml and .yaml closes the single-extension hole.

Full suite on the PR: 78 tests, 0 failures, exit 0.

It failed closed, and I verified rather than relayed

v0.56.1            44c4241     unchanged
last_released_sha  c43be07a
distance           3           unchanged

Nothing was moved, so the failed rebuild cost a run and no repair. That is the assert-before-push polarity from #1071 doing its job on the first live exercise.

One observation, not a request

The arm reports the same step twice against main, because that step carries two steps.pipeline.outputs references and each match appends. Duplicate lines in a failure message, nothing more — the detection and the exit status are correct either way.

Nothing blocking.

Approved at head `41f78183572a1c9bc22ad333fb29272384016433`, `state=open merged=false` read immediately before submitting. ## Scope, because the raw diff is misleading `git diff main...pr` showed 10 files until I re-fetched — my `forgejo/main` predated `#1071`'s merge, so the comparison was against a main that lacked it. Against **current** main (`c2aeecf`, with `c2aeecf9` confirmed an ancestor) this is **3 files, +90/-3**: the new checkout step, one changelog fragment, one bats arm. Reviewed at that scope. ## The diagnosis reproduces on main Not taken from the report — parsed out of `forgejo/main`'s own workflow: ``` 'pipeline' step id DEFINED in job goreleaser runs-on: go CONSUMED in job goreleaser ×3 runs-on: go <- valid, same job CONSUMED in job publish-image ×1 runs-on: docker-build <- the bug ``` **Three prior uses are correct because they never leave the defining job.** The fourth was the first across a boundary, and `steps.<id>.outputs` renders empty rather than erroring — so it reaches the shell as `cd ""`. The file taught a pattern that is sound everywhere except the one place it was newly used. **And the runners genuinely differ**, which is what rules out the cheaper fix: `go` vs `docker-build` means `$RUNNER_TEMP` is not shared, so a job output carrying the path would have crossed a value that is meaningless on the other side. The thing that has to cross is the checkout. That reasoning is in the comment, and it is right. ## `git worktree` over a second `actions/checkout` Confirmed the constraint is real rather than assumed: **arm 54 — `docker-build jobs preflight Node and host capabilities before actions (#876)` — passes**, and the new step uses `shell: bash` with plain git, no `uses:`. A red arm choosing the implementation is the right way round. The step also asserts what it did rather than trusting it: `go.mod` present in the worktree, and `git status --porcelain` empty afterwards so the source tree the image builds from is provably untouched. Both are refusals with `exit 1`, not notes. ## The arm — I ran it in both directions The important property is that it **detects the defect it was written for**, so I pointed it at the corpus that still contains it: ``` against forgejo/main (still buggy) -> "job publish-image … reads steps.pipeline.outputs, defined in another job" DETECTED against pr/1080 -> empty CLEAN ``` **And parsing rather than grepping is load-bearing, not stylistic.** A `#` inside a `run:` block is script text, so a grep over the raw file matches comment prose and can miss the live reference — which is exactly what happened on the first attempt. The parse walks `jobs → steps`, excludes only `name` (prose), and compares against the ids defined *in that job*. Covering `.yml` and `.yaml` closes the single-extension hole. Full suite on the PR: **78 tests, 0 failures, exit 0.** ## It failed closed, and I verified rather than relayed ``` v0.56.1 44c4241 unchanged last_released_sha c43be07a distance 3 unchanged ``` Nothing was moved, so the failed rebuild cost a run and no repair. That is the `assert-before-push` polarity from `#1071` doing its job on the first live exercise. ## One observation, not a request The arm reports the same step twice against main, because that step carries two `steps.pipeline.outputs` references and each match appends. Duplicate lines in a failure message, nothing more — the detection and the exit status are correct either way. Nothing blocking.
bosun merged commit cbc416b645 into main 2026-08-29 23:03:11 +02:00
bosun deleted branch i/980-crossjob 2026-08-29 23:03:11 +02:00
Sign in to join this conversation.
No description provided.