feat(router): worker-image build engine (compose, build, pin, smoke-test) [MNG-1723]#1474
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
APPROVE — a clean, fail-closed, well-tested build engine that faithfully extends the spec-022 validation pattern. I independently verified the two riskiest novel pieces (the hand-rolled tar and the dockerode labels path) and the fail-closed / no-strand / GC invariants all hold. All 98 tests across the touched files pass locally; CI is 7/7 green.
What I verified
- Hand-rolled ustar tar (
createSingleFileTar) — header field offsets (name/mode/uid/gid/size/mtime/chksum/typeflag/magic/version), the space-filled checksum computed over all 512 bytes, 512-block body padding, and the 1024-byte zero trailer are all correct.dockerode'sprepareBuildContextpasses a rawReadablestraight through (no re-tarring), andReadable.from(buffer)emits the whole Buffer as one chunk (Node special-cases Buffer/string — confirmed empirically), so the build context is not corrupted byte-by-byte. cascade.build_hashlabel via thebuildImage({ labels })option — the reuse fast-path depends on this label actually landing on the image.dockerodedoesn't special-caselabels, butdocker-modem.buildQuerystringJSON-stringifies any non-array object option, solabelsreachesPOST /buildas the JSON map the Engine API expects.cascade.managed=trueis separately a composedLABELline, so GC matching does not depend on the option path.- GC claims — the superseded built image is untagged-but-labeled (
cascade.managed=truefrom the composedLABEL), matching the existingdangling=true AND label=cascade.managed=truereaper filter; the active:latesttag is structurally never dangling. No new loop needed. Confirmed againstdangling-image-cleanup.ts. pull: falsecorrectness — the base is pulled by tag indefaultResolveBaseDigestfirst, so the composedFROM repo@sha256:…digest ref resolves locally without a build-time pull.- No-strand / fail-closed —
keepActiveis derived from the pre-build snapshot; the verified/first-fail/keep-active branches inrecordWorkerImageBuildResultmatch the four integration cases exactly; the handler never rejects and Sentry-captures unexpected throws. The early-throw default ofkeepActive=falseis the correct fail-closed choice (guarantees a terminalfailedrather than a stuckbuilding). - Distinct reasons (
build failed:vsruntime requirement missing …), smoke-test against the immutable pin (not the retag-able tag), schema columns present from ticket 1/5, and thedefaultRunImageCheck → runWorkerImageSmokeTestrename has no dangling references.
Optional (non-blocking, low severity)
One defense-in-depth nit on the pin step — see inline comment. Not a merge blocker.
🕵️ claude-code · claude-opus-4-8 · run details
| } | ||
|
|
||
| // 6. Pin — inspect the freshly built image for its immutable local ID. | ||
| const built = await deps.inspectBuiltImage(opts.tag); |
There was a problem hiding this comment.
Optional hardening (low severity, non-blocking): the pin step takes built.id without re-asserting built.fullHash === opts.fullHash, whereas the reuse check on line 326 does compare it. In the narrow window where a timeout-abandoned zombie build (dockerode has no cancel hook, as the comment on withBuildTimeout notes) or a concurrent supersede retags cascade-built-<proj>:latest between buildImage() resolving and this inspect, the pin could point at a different image than the one just built, and it would still be recorded verified. Re-checking the label here — if (!built?.id || built.fullHash !== opts.fullHash) return { ok: false, reason: 'build failed: …' } — converts that silent wrong-pin into a clean fail-closed record, mirroring the reuse path. Very unlikely in practice (requires a >workerBuildTimeoutMs build racing a re-enqueue), so purely a defense-in-depth suggestion.
Worker Dockerfile 3/5 — router-side build engine
Implements ticket 3/5 of the worker-Dockerfile sequence: a router-side
worker-image-builddashboard job that composes an operator's extra layers onto the pinned worker base, builds a local image, pins it by immutable local image ID, runs the existing runtime smoke-test, and recordsverified/failedfail-closed. It is the first-everdocker.buildImage()in the tree and a strict superset of the spec-022 validation handler.Ships operator-unreachable-but-tested: no set surface enqueues a build yet (ticket 4/5). Tested by invoking
handleWorkerImageBuilddirectly with injected deps against fabricateddockerfile-sourced rows — Docker sits behind the injectable dep struct, so no real daemon is needed.Linear: https://linear.app/mongrel/issue/MNG-1723/worker-dockerfile-35-build-engine-compose-build-pin-smoke-test-content
Engine contract (in order)
worker_image_build_hashno longer equals the job's hash (superseded).routerConfig.workerImageto an immutablerepo@sha256:….FROM <base>@<digest>·USER root·<operator lines>·USER node·LABEL cascade.managed=true; defensively throws on a self-declaredFROM.cascade-built-<projectId>:latestwhosecascade.build_hashlabel equals the full-hash (a changed base digest ⇒ different full-hash ⇒ rebuild); skipsdocker build.docker.buildImage()from an in-memory single-file tar, tagged per-project, bounded by a wall-clockWORKER_BUILD_TIMEOUT_MS.buildWorkerImageCheckScriptone-shotdocker.runthe validator uses, run against the pin.Two hashes (kept distinct)
sha256(raw operator content)→ theworker_image_build_hashcolumn + job identity + supersede guard (computed by the set surface in ticket 4).sha256(composed + base digest)→ stamped asLABEL cascade.build_hash, used only for the step-4 reuse fast path.Fail-closed & no-strand
FROM, build error, wall-clock timeout, smoke-test non-zero, unexpected throw) recordsfailedwith a precise reason and never leaves the project inbuilding. Build failures startbuild failed:; smoke-test failures start a distinctruntime requirement missing:. Unexpected throws are additionally Sentry-captured under tagworker_image_build. The handler never rejects.worker_image_status = verifiedon the old pin (keepActive) and records onlyworker_image_build_status = failed+ reason. A first build failure (no prior verified image) setsworker_image_status = failed.GC (no new loop)
Built images carry
LABEL cascade.managed=true+ a stable per-project:latesttag. A successful rebuild retags:latest→ the superseded digest becomes dangling → reclaimed by the existing 30-mindangling-image-cleanuploop. The active tagged image is never dangling, so a sweep can never remove the last-good image. A failed rebuild does not retag.Secrets
Builds inject no build-args and no project credentials; the credential master key is never in scope. Public-only builds by design.
Files
New
src/router/worker-dockerfile-compose.ts— purecomposeDockerfile(throws onFROM),computeContentHash,computeFullBuildHash.src/router/worker-image-build.ts—WorkerImageBuildDeps+handleWorkerImageBuild(mirrorsWorkerImageValidationDeps), plus default Docker deps (in-memory ustar tar builder,docker.buildImageprogress-error handling, base-digest resolution, built-image inspect).Touched
src/db/repositories/projectsRepository.ts—recordWorkerImageBuildResult(build-hash-guarded,keepActiveno-strand) +readWorkerImageBuildInputs.src/queue/client.ts—WorkerImageBuildJobin theDashboardJobunion +enqueueWorkerImageBuildJob+workerImageBuildJobId(deterministic per-project id; re-enqueue supersedes).src/router/worker-manager.ts—processDashboardJobworker-image-buildbranch (dispatched directly, no worker slot), beside validation.src/router/config.ts—workerBuildTimeoutMsfromWORKER_BUILD_TIMEOUT_MS(default 10 min).src/router/worker-image-validation.ts— extracted/exportedrunWorkerImageSmokeTestso the build engine reuses the exact same smoke-test.Tests
tests/unit/router/worker-dockerfile-compose.test.ts— compose shape, reject-FROM(case/whitespace/comment tolerant), CRLF, both hashes.tests/unit/router/worker-image-build.test.ts— superseded drop, happy path (verified + local pin +:latesttag), build failure (build failed:), smoke-test non-zero (runtime requirement missing:), wall-clock timeout, keep-active vs first-build failure, content-hash reuse skips build, unexpected throw caught + Sentry-captured + never rejects, stale-result drop, tar/tag helpers.tests/unit/queue/worker-image-build-job.test.ts— enqueue + deterministic jobId + distinct from validation.tests/unit/router/worker-manager.test.ts(extend) — build branch invokeshandleWorkerImageBuild, notguardedSpawn.tests/unit/router/dangling-image-cleanup.test.ts(extend) — superseded built digest reclaimed via the existing filter; active tagged image structurally exempt.tests/integration/db/projectsRepository.test.ts(extend) — build-hash guard, keep-active on failed rebuild with prior verified, first-build failure setsstatus=failed,readWorkerImageBuildInputs.Verification
npm run build,npm test(586 files / 10595 tests), the projectsRepository integration suite (60 tests),npm run lint(biome check --error-on-warningson changed files), andnpm run typecheckall green.Out of scope (later tickets)
Set-mutation enqueue + reject-
FROMat set time + mutual exclusivity + audit + manual-rebuild trigger (4/5); dashboard rendering + rebuild button + operator docs (5/5); build-time secrets / BuildKit / registry push / base-bump fan-out.🤖 Generated with Claude Code
🕵️ claude-code · claude-opus-4-8 · run details