Repository navigation
fix(dockerfile): parse FROM --platform flag in splitIntoStages (#1178) - #1240
Conversation
The test helper's FROM regex captured the first token after FROM as the base image and made the AS keyword optional, so a line like 'FROM --platform=linux/amd64 node:24 AS foo' registered stage foo under the platform flag (or the base image) instead of its real name. Capture the image separately from an optional 'AS <name>' clause, skipping any leading --flags, and add regression tests for the --platform shape.
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: MiniMax-M3 (anthropic) — primary route · pr-reviewer-action v3.4.1
Recommendation
Approve. This PR correctly fixes the splitIntoStages test helper to handle FROM --platform=... base AS name lines per issue PR 1178. The change is scoped entirely to Dockerfile.test.ts (the only caller of the helper), the new regex preserves the 7 named stages currently in Dockerfile, and the three new tests cover the regression paths including the downstream FROM <prev> AS <next> reference shape.
Change-by-change findings
fromReregex (Dockerfile.test.ts:27) — Old pattern/^\s*FROM\s+\S+\s+(?:AS\s+)?(\S+)/imadeASoptional and captured the first non-space token afterFROMas the stage name. New pattern/^\s*FROM\s+(?:--\S+\s+)*(\S+)(?:\s+AS\s+(\S+))?/icorrectly strips any leading--flagtokens, captures the image in group 1, and the optionalAS <name>in group 2. On the currentDockerfile(no flags, named stages) the resulting map keys are identical to before; only the previously-misparsed--platformshape changes. Held.currentName = match[2] ?? null(Dockerfile.test.ts:41) — Uses the new AS-name capture group, with?? nullcorrectly handling unnamed FROM lines so they still flush the prior stage but are not themselves keyed. Consistent with the docstring. Held.flush()and body accumulation — Untouched; the FROM line is not part of the body, matching prior behavior. Held.- Three new tests — Cover single-stage
--platformshape, downstream stage referencing a--platformbase, and a regression test that all 7 named stages (base,deps,prod-deps,mcp-deps,builder,runner,mcp) still resolve from the real Dockerfile. Held. - Existing PR 533 / PR 1173 stage-scoped assertions — Unchanged; they use
splitIntoStagesto readbuilder,mcp,mcp-depsand depend only on the named stages remaining keyed by their AS clause, which the third new test pins. Held.
Claim verification
- Claim 1 (FROM --platform mis-key bug): Held. Old regex made the stage keyword match
match[1]from the first non-space token afterFROM; the new regex moves the AS-name capture to group 2 and skips leading--flagtokens. - Claim 2 (no production code / Dockerfile changes): Held. Only
Dockerfile.test.tsmodified;Dockerfileand production source are untouched.grep --platformshows zero occurrences outside the new docstring and tests in this file. - Claim 3 (all 7 named stages still keyed): Held. Confirmed by manual regex walk against the 7
FROM ... AS <name>lines inDockerfileand by the third new test ("still parses plain named stages without a flag"). - Claim 4 (regex invariants / byte-for-byte identical on current Dockerfile): Held.
flush()+buffer = []untouched; old regex would produce identical keys for the plain-shape FROM lines; new regex produces the same keys for the same input. - Claim 5 (same 7 keys as old on Dockerfile, differs only on --platform shape): Held.
Standards Compliance
- No secrets, credentials, or tokens touched;
DISPATCH_AGENT_TOKEN/GITHUB_TOKENpolicy is not engaged. - No production code paths change; the helper is file-local and only invoked from tests within the same file.
- Lint and Typecheck CI checks reported as success on the head SHA. Local
npx tsc --noEmitandnpx eslint .reported clean by the author; CI matches. - Multi-stage Docker conventions and the existing
ARG DATABASE_URL/ENV DATABASE_URLrules (issue PR 533) are unaffected by the test regex.
Linked Issue Fit
Issue PR 1178 asked for:
- A fix to
splitIntoStagessoFROM --platform=... base AS nameparses to stagenamekeyed correctly — delivered (regex + capture-group shift). - A unit test covering
FROM --platform=linux/amd64 node:24 AS foo— delivered as the first new test. - Scope: only
Dockerfile.test.ts— held.
The author additionally added a downstream-stage test and a 7-stage regression test, both reasonable extensions of the requested coverage.
Tool Harness Findings
read_file Dockerfile.test.ts— Full file confirmed; onlyfromReandcurrentName = match[2] ?? nullchange in the helper, plus three new tests appended after the existingdescribe("Dockerfile MCP image wiring")block.read_file Dockerfile— Confirmed no--platformusage and exactly 7 named stages (base,deps,prod-deps,mcp-deps,builder,runner,mcp).git_grep splitIntoStages— All 8 references are insideDockerfile.test.ts(definition + 5 existing call sites + 3 new test call sites). No production callers.git_grep --platform— All matches are inDockerfile.test.ts(docstring, comment, test inputs/asserts). Zero inDockerfileor production source.
CI Check Results
The PR head SHA a8758f8c shows all checks green (Docker Build, Docker Build (MCP), smoke, npm audit, Lint, Workflow lint, Database migrations, Coverage, Build, Tests, Database integration, Typecheck). No CI evidence to dispute.
No findings warrant a request for changes.
What
Fixes #1178. The
splitIntoStageshelper inDockerfile.test.tsparsedFROMlines asFROM <image> [AS <name>], capturing the first token afterFROMas the image and treatingASas optional. AFROM --platform=... base AS nameline therefore registered the stage under the wrong key — the--platform=...flag (or the base image) — so every stage-scoped assertion silently read the wrong section and could give false pass/fail results.The regex now skips leading
--flagtokens and captures the image and the optionalAS <name>separately:/^\s*FROM\s+\S+\s+(?:AS\s+)?(\S+)/i(name = capture 1)/^\s*FROM\s+(?:--\S+\s+)*(\S+)(?:\s+AS\s+(\S+))?/i(name = capture 2)The current
Dockerfilehas no--platformusage, so the bug was latent; the fix is scoped to the test helper only. No production code and noDockerfileinstructions change.Invariants this change keeps
Dockerfileis still keyed by itsAS <name>. Kept by the third new test (base,deps,prod-deps,mcp-deps,builder,runner,mcpall resolve), and the existing#533/#1173stage-scoped assertions still pass unchanged.ASclause — the base-image token is never used as a key. Kept by(?:\s+AS\s+(\S+))?; name capture moved from group 1 to group 2.--flagsnever shift the image/name tokens. Kept by(?:--\S+\s+)*.FROM(theFROMline is not part of the body). Unchanged:flush()+buffer = []reset is untouched.Dockerfileis byte-for-byte identical to before (no regression). Verified: new regex yields the same 7 keys as the old one onDockerfile; differs only on the previously-misparsed--platformshape.Other code paths enforcing the same rule
splitIntoStagesis a file-local helper with no production callers, and there is no otherFROMparser in the repo. The other regexes in this same file (# Dockerfile.test.tsARG DATABASE_URL/ENV DATABASE_URL) matchARG/ENV, notFROM, so they are unaffected and need no change. Stage membership for the#533and#1173assertions depends only onsplitIntoStages, which is kept correct by invariant 1.Local validation (run against the integrated change on this branch)
npx vitest run Dockerfile.test.ts→ 8 passed (5 existing + 3 new).npx vitest run(full suite) → 4305 passed | 16 skipped | 1 todo; 0 failed.npx tsc --noEmit→ clean.npx eslint .→ 0 errors (1 pre-existing warning insrc/app/login/page.tsx, unrelated to this change).FROM --platform=linux/amd64 node:24 AS fooasnode:24while the fix keys itfoo, and the plain-Dockerfilekey set is unchanged.CI is left as-is for the operator to observe; nothing here is claimed green beyond the local runs above.