fix(worker-image): resolve base digest from the local image when a registry pull fails (MNG-1731)#1478
Conversation
…gistry pull fails (MNG-1731)
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
APPROVE — this correctly fixes the ghcr 401 build failure (MNG-1731) by making the base-image pull best-effort and falling back to the locally-present base. The primary path (pull succeeds + RepoDigests present) is byte-for-byte unchanged, and the extracted resolveBaseImageRef helper is fully unit-covered.
What I verified
- Fix addresses the incident:
pullImageOnce401 is now caught; when the base is present locally withRepoDigests(the dev-incident shape),resolveDigestFromRepoDigestsreturns the same immutable registry digest a successful pull would yield → build proceeds. ✔ - Primary path unchanged: pull-success + RepoDigests returns the identical registry digest and emits no warn (asserted by the test). ✔
- Fail-closed preserved: pull-fails + image-absent throws a clear, greppable
cannot obtain base image …error that folds in the 401; pull-succeeds + inspect-throws surfaces the inspect error rather than masking it as a cannot-obtain. ✔ - No stale coupling: the removed
could not resolve an immutable digest …message isn't referenced by any other code, test, or the existingworker-image-build.test.ts(which injectsresolveBaseDigest), so the refactor doesn't disturb the existing suite. ✔ - Coverage: every branch of the extracted helper is exercised, and
defaultResolveBaseDigeststays inside the/* v8 ignore */un-mockable-glue block —codecov/patchshould stay green. ✔ - Context reconciled: using a bare local image ID for the base FROM in the build path is legitimate even though
worker-image-validation.tsrejects no-RepoDigestsimages — that validator pins a launch-by-digest ref, whereas here the ref is only aFROMin a localpull: falsebuild.
One non-blocking confirmation question inline. Nice work — the helper extraction mirrors the file's existing injectable-deps pattern and the tests are precise.
🕵️ claude-code · claude-opus-4-8 · run details
|
|
||
| // 4. Self-hosted / locally-built base (no RepoDigests): pin by the immutable | ||
| // local image ID rather than failing. | ||
| if (info.Id) return info.Id; |
There was a problem hiding this comment.
Non-blocking confirmation. This self-hosted branch returns the bare local image ID (sha256:<hex>), which composeDockerfile then emits verbatim as FROM sha256:<hex>. The ticket explicitly flagged that a bare image-ID FROM might not be build-valid ("if a bare image-ID FROM isn't build-valid, tag a stable local ref and use that").
Reading the build path, this should be fine: defaultBuildImage drives the classic builder (docker.buildImage + modem.followProgress, pull: false), and moby's reference.ParseAnyReference parses sha256:<64hex> as a digest reference, which GetImage resolves to the local image by ID — so FROM sha256:<hex> resolves without a registry call. The gap is only that no unit/integration test proves the end-to-end docker build accepts it — the helper tests assert the returned string, not a real build. Since the path is fail-closed (a wrong assumption just records build failed:), the risk is bounded, but given the ticket called it out, a one-time manual confirmation on a self-hosted / no-RepoDigests base would lock it down.
Summary
Fixes MNG-1731 — worker Dockerfile base-digest resolution now uses the local base image when a fresh registry pull fails.
Issue: https://linear.app/mongrel/issue/MNG-1731/worker-dockerfile-base-digest-resolution-must-use-the-local-base-image
Problem (found live on the dev deployment, 2026-07-03)
Setting a per-project worker Dockerfile and letting the router build it failed on the dev deployment:
Root cause —
defaultResolveBaseDigestinsrc/router/worker-image-build.tsunconditionally pulled the global base image from the registry to read its immutable digest. The dev router's Docker daemon has no standing ghcr credentials for on-demand pulls — but the base image is already present locally (the router runs every worker from it), so its immutable digest is obtainable locally without any registry call. The unconditional pull is the only thing that broke it. This also blocked self-hosted deployments whose base is present locally but not freshly pullable.Fix
Extracted the base-ref decision logic into a small, injectable, unit-testable helper
resolveBaseImageRef(ref, { pull, inspectLocal })and rewireddefaultResolveBaseDigestto wire the real Dockerpull/inspectinto it. The composed-FROM+pull: falsebuild path is unchanged — only how the base digest/ref is obtained changes.Resolution order:
pull()for the freshest digest.inspectLocal(). A pull failure (401/auth, network,ENOTFOUND) is tolerated when the base is already present locally. If the image is genuinely absent AND the pull failed → throw a clear, greppablecannot obtain base image …error (fail-closed — no silent fallback to a wrong/moving image).RepoDigests.RepoDigests): pin by the immutable local image ID (inspect().Id) rather than failing. The build already usespull: false, so a local reference resolves without a registry call. Never falls back to the moving tag — the pin stays immutable.An observability
logger.warnfires when the pull fails but the local base is used, so the fallback is visible in router logs.Tests
New
tests/unit/router/worker-image-base-digest.test.ts(8 tests) covering all four ticket cases plus edge branches:cannot obtain base image …error (folds in the 401).resolveDigestFromRepoDigests.The helper is fully covered;
defaultResolveBaseDigestremains inside the existing/* v8 ignore */un-mockable-Docker-glue block.Acceptance criteria
npm run build,npm test(router: 61 files / 1156 tests green),npm run lint,npm run typecheckall green.Out of scope
No change to compose / build / pin / smoke-test / spawn behavior beyond how the base ref is obtained. No standing registry credentials, per-project registry creds, registry push, or multi-host.
🤖 Generated with Claude Code
🕵️ claude-code · claude-opus-4-8 · run details