Pull-request CI never builds the image, so Dockerfile faults only surface on main #129

Closed
opened 2026-08-12 02:19:24 +00:00 by coilyco-ops · 2 comments
Member

The gap

.forgejo/workflows/ci.yml runs two jobs. test runs on every pull request: build, policy-check, vet, test, pre-commit. publish-echo-image is gated:

if: >-
  github.event_name == 'push' &&
  github.ref == 'refs/heads/main'

Nothing builds the image on a pull request. A fault in the Dockerfile, in .dockerignore, or in any script the build runs is therefore invisible until the merge commit, at which point main goes red and the Telegram alert fires.

How it bit

#124 was green across every pull-request check and turned main red on merge, run 148. Two faults, both in the compose stage:

  • The stage copied agent/compose and scripts/stage-compose-sources.sh only, while the script builds cmd/sirens-echo-compose, which needs go.mod, go.sum, cmd, and internal.
  • .dockerignore excludes scripts/*, so the stage never received the script it runs.

The second fault predated the pull request entirely. It had simply never been executed, because until that merge nothing had ever run the compose stage.

Fixed in #128, but the fix carries the same blind spot: that pull request going green proves nothing about the image either.

Why it matters more now

The compose stage clones an external catalogue, runs an expander over an allowlist, and materializes bundles the runtime refuses to start without. That is real logic with real failure modes, and none of it is reachable from a pull-request check today.

Options

  • Build the image in the test job on pull requests without publishing. Costs build minutes on every pull request and catches everything above.
  • Add a cheaper docker build --target compose on pull requests, exercising only the stage that carries logic.
  • Keep publishing gated to main either way. This is about building, not publishing.

ward exec image already builds locally, so the verb exists; it is the pull-request wiring that is missing.

Complete when

A Dockerfile, .dockerignore, or build-script fault fails a pull-request check rather than the merge.

## The gap `.forgejo/workflows/ci.yml` runs two jobs. `test` runs on every pull request: build, policy-check, vet, test, pre-commit. `publish-echo-image` is gated: ```yaml if: >- github.event_name == 'push' && github.ref == 'refs/heads/main' ``` Nothing builds the image on a pull request. A fault in the `Dockerfile`, in `.dockerignore`, or in any script the build runs is therefore invisible until the merge commit, at which point `main` goes red and the Telegram alert fires. ## How it bit https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/124 was green across every pull-request check and turned `main` red on merge, run 148. Two faults, both in the compose stage: * The stage copied `agent/compose` and `scripts/stage-compose-sources.sh` only, while the script builds `cmd/sirens-echo-compose`, which needs `go.mod`, `go.sum`, `cmd`, and `internal`. * `.dockerignore` excludes `scripts/*`, so the stage never received the script it runs. The second fault predated the pull request entirely. It had simply never been executed, because until that merge nothing had ever run the compose stage. Fixed in https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/128, but the fix carries the same blind spot: that pull request going green proves nothing about the image either. ## Why it matters more now The compose stage clones an external catalogue, runs an expander over an allowlist, and materializes bundles the runtime refuses to start without. That is real logic with real failure modes, and none of it is reachable from a pull-request check today. ## Options * Build the image in the `test` job on pull requests without publishing. Costs build minutes on every pull request and catches everything above. * Add a cheaper `docker build --target compose` on pull requests, exercising only the stage that carries logic. * Keep publishing gated to `main` either way. This is about building, not publishing. `ward exec image` already builds locally, so the verb exists; it is the pull-request wiring that is missing. ## Complete when A Dockerfile, `.dockerignore`, or build-script fault fails a pull-request check rather than the merge.
Author
Member

Partial, deliberately

#140 landed a static guard that catches the exact fault class both this issue and #91 describe, without needing a daemon: a Go test parses the Dockerfile's context COPY sources and evaluates each against .dockerignore with Docker's ordering, negation, and directory-descendant semantics, failing on a copied path that is excluded or missing.

Both historical faults were reintroduced against the checkout and confirmed to fail the check:

  • docs excluded while the build stage copies it, the #91 fault
  • the !scripts/stage-compose-sources.sh negation removed under scripts/*, the #129 fault

The hand-maintained imageContextPaths mirror is now checked against the parsed COPY set too, so a rename cannot drift it silently.

It runs in the existing test job, so it gates every pull request today.

Why this issue stays open

A static guard is not the image build. Still invisible until a merge:

  • a base-image change in agentic-os:release
  • a RUN step that fails, including the compose stage's clone and expander run
  • a build input that is present but insufficient, which was the first of the two faults in this issue's own description

That last one matters. The compose stage copied agent/compose and the script but not go.mod, go.sum, cmd, internal. Every one of those paths is present in the repository and not excluded, so the static guard cannot see the fault. Only running the build can.

What the remaining half needs

Not code. It needs a runner arrangement where docker build reaches a daemon, and per #91 the first run has to be read from the web UI because this Forgejo predates the Actions log endpoints. I am deliberately not pushing a speculative CI job to main, since an unverifiable job turning main red is the failure mode this issue exists to remove.

#91's ordered hypotheses are still the right list to try, and its acceptance criterion "the first run of the new job is checked against real logs before it is relied on" is the part that is not mine to satisfy.

Next owner

Ops or Kai, for one supervised attempt with the log read. The cheap option in this issue, docker build --target compose on pull requests, is the one worth trying first: it exercises the stage that carries the logic and skips the Go build the test job already covers.

## Partial, deliberately #140 landed a static guard that catches the exact fault class both this issue and #91 describe, without needing a daemon: a Go test parses the Dockerfile's context `COPY` sources and evaluates each against `.dockerignore` with Docker's ordering, negation, and directory-descendant semantics, failing on a copied path that is excluded or missing. Both historical faults were reintroduced against the checkout and confirmed to fail the check: * `docs` excluded while the build stage copies it, the #91 fault * the `!scripts/stage-compose-sources.sh` negation removed under `scripts/*`, the #129 fault The hand-maintained `imageContextPaths` mirror is now checked against the parsed `COPY` set too, so a rename cannot drift it silently. It runs in the existing `test` job, so it gates every pull request today. ## Why this issue stays open A static guard is not the image build. Still invisible until a merge: * a base-image change in `agentic-os:release` * a `RUN` step that fails, including the compose stage's clone and expander run * a build input that is present but insufficient, which was the *first* of the two faults in this issue's own description That last one matters. The compose stage copied `agent/compose` and the script but not `go.mod`, `go.sum`, `cmd`, `internal`. Every one of those paths is present in the repository and not excluded, so the static guard cannot see the fault. Only running the build can. ## What the remaining half needs Not code. It needs a runner arrangement where `docker build` reaches a daemon, and per #91 the first run has to be read from the web UI because this Forgejo predates the Actions log endpoints. I am deliberately not pushing a speculative CI job to `main`, since an unverifiable job turning `main` red is the failure mode this issue exists to remove. #91's ordered hypotheses are still the right list to try, and its acceptance criterion "the first run of the new job is checked against real logs before it is relied on" is the part that is not mine to satisfy. ## Next owner Ops or Kai, for one supervised attempt with the log read. The cheap option in this issue, `docker build --target compose` on pull requests, is the one worth trying first: it exercises the stage that carries the logic and skips the Go build the `test` job already covers.
Author
Member

Complete

Merged as #142, fedeb997. The Forgejo 16 upgrade serves the Actions log endpoints, which is what let the job be written from evidence instead of guessed at. Full diagnosis is on #91.

Of the three options in this issue, the first landed: build the image in a pull-request job without publishing. The cheaper --target compose was not needed, because the compose stage depends on the build stage anyway, so the saving would have been the final assembly only.

Complete when

A Dockerfile, .dockerignore, or build-script fault fails a pull-request check rather than the merge.

All three classes are now covered, and each was demonstrated rather than asserted:

  • .dockerignore - #140's static guard. Both historical exclusions reintroduced and confirmed to fail.
  • Dockerfile - the image build. Reproduced the first of the two faults described above, an input present in the repository but missing from the stage, by removing the compose stage's COPY --from=build of the expander binary. The static guard passes on it, since that class is invisible to a path check. The image build fails, exit 127 at Dockerfile line 33.
  • Build script - same job. scripts/stage-compose-sources.sh runs inside the build, so a fault in it fails the same check. That is the mechanism the fault above actually tripped.

On the concern this issue raised about its own fix

Fixed in [#128], but the fix carries the same blind spot: that pull request going green proves nothing about the image either.

No longer true. Run 17781 on the pull request built all 35 Dockerfile steps including the compose stage's catalogue clone and expander run, which this issue correctly identified as real logic that nothing was exercising. Run 17782 on main after the merge is green across test, image-build, and publish-echo-image.

Publishing stays gated to main and on the deploy runner, unchanged. The new job holds no registry credential and its tag ships nowhere.

## Complete Merged as #142, `fedeb997`. The Forgejo 16 upgrade serves the Actions log endpoints, which is what let the job be written from evidence instead of guessed at. Full diagnosis is on #91. Of the three options in this issue, the first landed: build the image in a pull-request job without publishing. The cheaper `--target compose` was not needed, because the compose stage depends on the build stage anyway, so the saving would have been the final assembly only. ## Complete when > A Dockerfile, `.dockerignore`, or build-script fault fails a pull-request check rather than the merge. All three classes are now covered, and each was demonstrated rather than asserted: * **`.dockerignore`** - #140's static guard. Both historical exclusions reintroduced and confirmed to fail. * **Dockerfile** - the image build. Reproduced the *first* of the two faults described above, an input present in the repository but missing from the stage, by removing the compose stage's `COPY --from=build` of the expander binary. The static guard passes on it, since that class is invisible to a path check. The image build fails, exit 127 at Dockerfile line 33. * **Build script** - same job. `scripts/stage-compose-sources.sh` runs inside the build, so a fault in it fails the same check. That is the mechanism the fault above actually tripped. ## On the concern this issue raised about its own fix > Fixed in [#128], but the fix carries the same blind spot: that pull request going green proves nothing about the image either. No longer true. Run 17781 on the pull request built all 35 Dockerfile steps including the compose stage's catalogue clone and expander run, which this issue correctly identified as real logic that nothing was exercising. Run 17782 on `main` after the merge is green across `test`, `image-build`, and `publish-echo-image`. Publishing stays gated to `main` and on the `deploy` runner, unchanged. The new job holds no registry credential and its tag ships nowhere.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
coilyco-gaming/sirens-echo#129
No description provided.