Skip to content

fix(worker-image): resolve base digest from the local image when a registry pull fails (MNG-1731)#1478

Merged
zbigniewsobiecki merged 1 commit into
devfrom
fix/mng-1731-base-digest-local-fallback
Jul 3, 2026
Merged

fix(worker-image): resolve base digest from the local image when a registry pull fails (MNG-1731)#1478
zbigniewsobiecki merged 1 commit into
devfrom
fix/mng-1731-base-digest-local-fallback

Conversation

@aaight

@aaight aaight commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

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:

build error: (HTTP code 401) unexpected - Head "https://ghcr.io/v2/.../manifests/dev": unauthorized · build failed

Root causedefaultResolveBaseDigest in src/router/worker-image-build.ts unconditionally 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 rewired defaultResolveBaseDigest to wire the real Docker pull/inspect into it. The composed-FROM + pull: false build path is unchanged — only how the base digest/ref is obtained changes.

Resolution order:

  1. Best-effort registry pull() for the freshest digest.
  2. 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, greppable cannot obtain base image … error (fail-closed — no silent fallback to a wrong/moving image).
  3. Primary path (unchanged): the immutable registry digest from RepoDigests.
  4. Self-hosted / locally-built base (no RepoDigests): pin by the immutable local image ID (inspect().Id) rather than failing. The build already uses pull: false, so a local reference resolves without a registry call. Never falls back to the moving tag — the pin stays immutable.

An observability logger.warn fires 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:

  • pull SUCCEEDS + RepoDigests present → returns the registry digest (primary path unchanged, no warn).
  • pull THROWS (401) + local WITH RepoDigests → returns the local registry digest, does not rethrow, logs the fallback warn. (the dev-incident regression)
  • pull THROWS + local with NO / undefined RepoDigests → returns the local image-ID pin (self-hosted path).
  • pull THROWS + image ABSENT locally → rethrows the clear, greppable cannot obtain base image … error (folds in the 401).
  • pull SUCCEEDS but inspect throws → surfaces the inspect error, not the cannot-obtain error.
  • local image has neither RepoDigests nor an image ID → clear cannot-pin error.
  • delegates repo-matching to resolveDigestFromRepoDigests.

The helper is fully covered; defaultResolveBaseDigest remains inside the existing /* v8 ignore */ un-mockable-Docker-glue block.

Acceptance criteria

  • ✅ With a base image already present locally, base-digest resolution SUCCEEDS even when a fresh registry pull fails (401/network) — no build failure on that path.
  • ✅ The primary path (registry pull works) is byte-for-byte unchanged.
  • ✅ A base image that is neither pullable nor locally present still fails with a clear, greppable error (no silent fallback).
  • ✅ Unit tests cover all four cases (incl. the pull-fails-but-local-with-RepoDigests dev regression).
  • npm run build, npm test (router: 61 files / 1156 tests green), npm run lint, npm run typecheck all 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

@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@nhopeatall nhopeatall 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.

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: pullImageOnce 401 is now caught; when the base is present locally with RepoDigests (the dev-incident shape), resolveDigestFromRepoDigests returns 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 existing worker-image-build.test.ts (which injects resolveBaseDigest), so the refactor doesn't disturb the existing suite. ✔
  • Coverage: every branch of the extracted helper is exercised, and defaultResolveBaseDigest stays inside the /* v8 ignore */ un-mockable-glue block — codecov/patch should 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.ts rejects no-RepoDigests images — that validator pins a launch-by-digest ref, whereas here the ref is only a FROM in a local pull: false build.

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;

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.

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.

@zbigniewsobiecki
zbigniewsobiecki merged commit b058084 into dev Jul 3, 2026
9 checks passed
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.

3 participants