fix(mcp): ship only the runtime closure in the -mcp image (prod deps + tsx layer)

[itsmiso-ai]

Closes #1173

What

The mcp stage copied node_modules from the dev-inclusive deps stage (npm ci, devDependencies included) because its entrypoint runs tsx. That shipped the entire lint/test toolchain — including the accepted dev-only advisory chain eslint-config-next -> @next/eslint-plugin-next -> fast-glob -> micromatch -> braces (GHSA-vfj7-8cjw-p6xm) — in the published ghcr.io/misospace/dispatch-mcp image.

This adds a mcp-deps stage: npm ci --omit=dev plus a layer-local rewrite of package.json (tsx moved into dependencies, devDependencies dropped — the range is still read from devDependencies, so Renovate stays the source of truth; the rewrite fails the build if the tsx range disappears, tolerates a manifest without a dependencies field, and writes a trailing newline) followed by npm install --no-save, which adds only tsx/esbuild and prunes the stragglers this lock reifies even under --omit=dev (typescript et al.: dev:false in the lock via optional-peer refs). The mcp stage now copies its node_modules and its shipped package.json from mcp-deps, and the docker-mcp CI job gains an "Assert MCP image ships no dev toolchain" step after the existing handshake validation.

Both direct npm install shapes were empirically rejected against npm 11.19 (documented in the Dockerfile comment): a bare tsx@range install without --omit=dev reifies the whole tree and reinstalls every devDependency, and with --omit=dev npm skips an explicit package that package.json lists as dev-only.

Verification

• Full simulation against the repo's real package.json/package-lock.json (npm 11.19 / node 24): after the stage recipe, the CI assert loop passes clean (no eslint chain, no vitest/typescript/ts-node, one nesting level scanned, scoped hosts included), and the MCP stdio initialize handshake returns serverInfo under tsx from the resulting tree, before and after the rewritten package.json is placed in the image.
• New CI step planted-failure tested (a planted node_modules/eslint makes it exit 1); image.yaml parses as valid YAML.
• Dockerfile.test.ts (5 tests incl. the new MCP-wiring pins) passes locally under vitest; lint/typecheck clean locally.
• Second review round: the rewrite guard was exercised directly (missing dependencies field now yields a clean rewrite, not a TypeError); assert step poison-tested (top-level eslint, nested duplicate under a plain and a scoped host, top-level typescript — all detected under dash with set -euo pipefail).
• Earlier Docker Build (MCP) failures on 6fdf72f were transient CI infra (runner ENOTFOUND nodejs.org; GHCR "No server is currently available") — the job is green on 20e4c83 and on the current head with a byte-identical MCP diff.
• No Docker daemon in the run lane, so the authoritative docker build + handshake is the CI docker-mcp job on this PR.

Invariants this change must keep (and the other code paths enforcing the same rules)

1. builder installs the full dev tree (deps), runner ships the production-only tree (prod-deps). Untouched: Dockerfile still has COPY --from=deps in builder and COPY --from=prod-deps in runner; the deps and prod-deps stages are byte-identical to before. The only stage retargeted is mcp.
2. ENV DATABASE_URL stays strictly inside builder (issue Dockerfile builds with hardcoded build-time databaseurl #533). Kept: the new mcp-deps stage sets no DATABASE_URL; Dockerfile.test.ts enforces this over every stage by parsing all FROM ... AS sections and still passes (plus a direct mcp-deps belt from the new tests).
3. The runner image must not gain tsx/dev tooling. Kept: tsx remains a devDependency in package.json (unchanged in git); it is added only inside the mcp-deps layer's rewritten copy, which prod-deps never sees. npm run audit (--omit=dev) and security-audit.yaml are unaffected. A comment in mcp-deps warns against copying .npmrc (its include=dev would override --omit=dev, per CI: Security Audit failing on the default branch (npm audit) #1166).
4. The -mcp image keeps its entrypoint, file layout, and non-root user. Kept: ENTRYPOINT ["./node_modules/.bin/tsx", "src/mcp/server.ts"], the import-closure COPY lines, USER mcp, and tag/push logic in the docker-mcp job are unchanged; only the dependency-tree source and the shipped manifest changed. The shipped package.json keeps every runtime-relevant field (dependencies, overrides, allowScripts); only devDependencies is removed, and src/mcp/server.ts hardcodes its server version rather than reading the manifest. The layer-local rewrite now throws at build time if tsx ever disappears from devDependencies (and tolerates a manifest with no dependencies field), instead of silently shipping an image whose entrypoint binary is missing.
5. No dev toolchain in the -mcp image, now or later. Enforced four ways: the build recipe itself, Dockerfile.test.ts pinning mcp to COPY --from=mcp-deps (node_modules + package.json) and forbidding COPY --from=deps in that stage and pinning the mcp-deps recipe (npm ci --omit=dev, the rewrite reading the tsx range from devDependencies, npm install --no-save, no ENV DATABASE_URL), the new assert step in .github/workflows/image.yaml (advisory-chain packages scanned at top level and one nesting level deep, including under scoped host packages), and the existing handshake step which proves tsx + @modelcontextprotocol/sdk + zod still work from that tree.
6. package.json / package-lock.json are unchanged in git. Kept: the manifest rewrite happens only inside the layer; --no-save writes nothing back. Confirmed by the diff (branch-vs-main delta contains no manifest/lockfile changes; lockfile movement in the branch history is only merges from main).
7. Main-image validation (Validate Prisma CLI runtime), Trivy, and tag/cache policy are untouched. Kept: only the docker-mcp job changed (one appended step); the docker job is byte-identical.

SECURITY-ACCEPTED-RISKS.md gains a resolution-history row: the GHSA-vfj7-8cjw-p6xm chain now ships in no published image (dev installs only), with the CI assert as the standing guard.

[itsmiso-ai]

Rebased onto latest main and resolved the SECURITY-ACCEPTED-RISKS.md conflict with #1172: kept both resolution-history rows and rewrote the "Shipping surface" bullet in the new Dev-Only Advisories section, which still described the -mcp image as shipping the lint chain (no longer true after this PR). No code changes beyond the merge.

[itsmiso-ai]

Review round pushed in be22380 (all findings from an independent review pass): (1) the mcp-deps package.json rewrite now fails the build if the tsx range disappears from devDependencies and writes a trailing newline; (2) advisory-chain scan in the image.yaml assert step also covers duplicates nested under scoped host packages; (3) Dockerfile.test.ts now pins the mcp stage to COPY --from=mcp-deps (node_modules + package.json), forbids COPY --from=deps there, and pins mcp-deps to an npm ci --omit=dev install with no ENV DATABASE_URL; (4) comment in mcp-deps warns against copying .npmrc (include=dev overrides --omit=dev, per #1166). Local: vitest Dockerfile.test.ts 5/5 pass, lint and typecheck clean. Rebased state unchanged; PR body invariants updated to match.

[itsmiso-ai]

Replying to the CHANGES_REQUESTED blocker (review on 6fdf72f, "Docker Build (MCP) failing on this head"):

The failure was transient CI infra, not this change — both runs failed before the MCP build/assert steps ever executed:

• 6fdf72f run: runner network failure, ENOTFOUND nodejs.org at the "Install Node" step.
• Next attempt: GHCR No server is currently available at "Extract metadata" on an identical tree.

Both were re-run as no-op commits (6fdf72f, 20e4c83) rather than masking anything with code changes, and the evidence backs the infra diagnosis:

• On 20e4c83 every check passed, including Docker Build (MCP) (build + initialize handshake + the new "Assert MCP image ships no dev toolchain" step) — run 37157411758, job 111303571709, conclusion success.
• The full diff for the MCP change is byte-identical between 6fdf72f and 20e4c83 (only the two empty re-run commits differ), so no code fix was possible or needed.

Head is now 93357ae (merge of latest main: jsdom 30.1.1 → 30.1.2 from #1177, package-lock only; no changes to Dockerfile, image.yaml, or the MCP code). A fresh CI run is in progress on it.

No action items remain from the review: the blocker is green CI on the current head (the AI-reviewer check on 20e4c83 ran against a tree with zero MCP-related delta), and the non-blocking note about the npm-11.19 empirical rejection claims is documented in the mcp-deps Dockerfile comment as empirical rather than asserted as upstream-documented.

[itsmiso-ai]

Nit-fix round pushed in f47f6a5, from an independent review pass over the post-merge diff (no blockers found; recipe and CI guard verified by empirical replication):

1. Dockerfile mcp-deps rewrite now guards p.dependencies (p.dependencies = p.dependencies || {}): a manifest without a dependencies field previously crashed with a cryptic TypeError instead of the clean rewrite; verified with a direct node -e run against such a manifest.
2. Fixed the inverted comment: typescript leaks into --omit=dev precisely because it is not dev-flagged in the lock (dev:false via optional-peer refs), not because it is a "dev-flagged straggler". The pruning behavior described is empirically correct.
3. Dockerfile.test.ts now pins the tsx-layer recipe itself (rewrite reads the range from devDependencies + npm install --no-save), so deleting the rewrite fails the unit test, not only the CI handshake step. 14/14 tests pass locally; lint (only the pre-existing login/page.tsx warning) and typecheck clean.

Also rebased-in main via merge 93357ae (jsdom 30.1.1 → 30.1.2 from #1177, package-lock only). PR body updated to match the diff (rewrite guard, corrected straggler wording, expanded test-pin list in invariant 5). Follow-up issue filed for the pre-existing splitIntoStages --platform parsing weakness found by the review: #1178 (out of scope here).

Superseded by a newer automated review for this pull request.

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: MiniMax-M3 (anthropic) — primary route · pr-reviewer-action v3.2.0

Partial coverage: required-check coverage is incomplete — this review did not resolve every required check and must not be read as a complete pass.

Recommendation

Approve. The PR delivers what issue PR 1173 asks for: the -mcp image is rebuilt from a production-only install plus a layer-local tsx shim, the existing Validate MCP handshake step is followed by a new Assert MCP image ships no dev toolchain step, and regression tests in Dockerfile.test.ts pin the new wiring. CI is green on the head (f47f6a5); the prior CHANGES_REQUESTED blocker was diagnosed by the author as transient runner infra, supported by an identical-tree rerun that passed Docker Build (MCP) end-to-end.

Change-by-change findings

• Dockerfile — new mcp-deps stage. Verified the stage exists between prod-deps and builder; it copies package.json + package-lock.json* only (no .npmrc), runs npm ci --omit=dev, then performs a node -e rewrite of package.json (moves tsx from devDependencies into dependencies, deletes devDependencies) followed by npm install --no-save --no-audit --no-fund. The comment correctly cites PR 1166 as the reason .npmrc must not be COPYd (its include=dev overrides --omit=dev), and the rewrite throws at build time if tsx ever disappears from devDependencies (verified at Dockerfile:37). This is the #1173 task's core fix.
• Dockerfile — retargeted mcp stage. Verified mcp now COPY --from=mcp-deps for both node_modules and package.json; the old COPY --from=deps /app/node_modules and COPY package.json lines are gone. The entrypoint, import closure, and USER mcp are unchanged, matching invariant PR 4 from the PR. ENV NODE_ENV=production is preserved; ENV DATABASE_URL is not added (belt for PR 533, covered by test).
• Dockerfile — builder/runner/prod-deps/deps stages. Verified byte-identical to before: builder still does COPY --from=deps, runner still does COPY --from=prod-deps, and mcp-deps adds no DATABASE_URL. Claim 4 and the corresponding invariant hold.
• .github/workflows/image.yaml — new assert step. Verified the new step sits after Validate MCP handshake in the docker-mcp job, runs docker run --rm --entrypoint sh $IMAGE -c '…' on the same IMAGE (handles pull_request vs push the same way the handshake does), and exits 1 on any of the advisory-chain packages or top-level vitest/typescript/ts-node. The nested-scope scan (node_modules/@*/*/node_modules/...) catches scoped duplicates hiding under a host package — matches the author's claim in the comment thread.
• Dockerfile.test.ts — new Dockerfile MCP image wiring describe block. Verified both new tests: mcp stage must contain COPY --from=mcp-deps /app/node_modules ./node_modules AND COPY --from=mcp-deps /app/package.json ./package.json, must NOT contain COPY --from=deps ; mcp-deps must contain npm ci --omit=dev, devDependencies.tsx, npm install --no-save, and must not match ^ENV\s+DATABASE_URL\b. The splitIntoStages regex (FROM\s+\S+\s+(?:AS\s+)?(\S+)) captures both FROM base AS mcp-deps and FROM base AS mcp, so all four stages are wired into the assertion map. These are real predicates (contain/not-contain/not-match), not just assignments.
• SECURITY-ACCEPTED-RISKS.md. Verified the Shipping surface bullet for the braces advisory was rewritten to claim "none" with the same mcp-deps justification, and a new row was added under retired risks with resolution #1173. The text correctly references .github/workflows/image.yaml and the Assert MCP image ships no dev toolchain step.

Claims disposition (hypotheses)

• Claim 1 (lint chain shipped in image; ghcr.io/misospace/dispatch-mcp). Held for every listed item: the diff removes the dev chain from Dockerfile (mcp no longer copies from deps), adds the explicit assert in image.yaml that walks the advisory-chain list, and updates SECURITY-ACCEPTED-RISKS.md to mark the chain as absent. (8/8)
• Claim 2 (mcp copies node_modules + package.json from mcp-deps; docker-mcp gains a step). Held. (12/12)
• Claim 3 (both npm install shapes empirically rejected against npm 11.19). The Dockerfile comment and the --no-save recipe match the claim; the empirical-rejection note is documented as observed rather than asserted as upstream contract, consistent with the author's reply in the PR thread. Held as far as the implementation can verify — I cannot independently reproduce the npm 11.19 dry runs from the corpus, but the recipe and its rationale are coherent and the --no-save path is the standard escape hatch. (12/12)
• Claim 4 (builder installs the dev tree, runner ships production-only; deps/prod-deps byte-identical; only mcp retargeted). Held. The added lines live strictly inside the new mcp-deps block and the mcp retarget lines; builder, runner, deps, and prod-deps are untouched. (12/12)
• Claim 5 (ENV DATABASE_URL stays in builder; mcp-deps has none; Dockerfile.test.ts enforces). Held. The new Dockerfile MCP image wiring describe adds an explicit ^ENV\s+DATABASE_URL\b belt on mcp-deps, and the existing keeps ENV DATABASE_URL strictly inside the builder stage test still iterates every non-builder stage and would fail if mcp-deps ever set it. (12/12)

Standards Compliance

• Docker conventions (AGENTS.md). Multi-stage pattern preserved; base is node:24-bookworm-slim; OpenSSL/ca-certificates install path unchanged; the docker-mcp job's Validate MCP handshake step is still the live MCP signal. ✓
• Lint/typecheck blocks CI. Lint, Build, Typecheck are green on the head per the CI status block. ✓ (req-e589bfe1ad7c)
• Token secrecy. No tokens are echoed, logged, or persisted by the new step; the docker run --rm --entrypoint sh form passes no secrets. ✓ (req-ad902b2a08da)
• runner must not gain tsx/dev tooling. tsx remains a devDependency in package.json; only the layer-local copy is rewritten, and prod-deps does not touch tsx; npm run audit and security-audit.yaml are unchanged. ✓ (req-b0d8b27b3b0e)

Linked Issue Fit

Issue PR 1173 asks for: build the -mcp image from a production-only install plus a tsx layer; verify the MCP handshake still passes. The PR does exactly this: a new mcp-deps stage performs npm ci --omit=dev then a layer-local package.json rewrite + npm install --no-save to install only tsx/esbuild; the mcp stage copies from mcp-deps; the existing Validate MCP handshake step is preserved (and is green on this head); Dockerfile.test.ts and the new CI assert step guard the regression. Acceptance criteria satisfied.

Tool Harness Findings

• .npmrc could not be read directly (sensitive-file block). Its include=dev content was inferred from the consistent cross-file evidence: package.json's "//" audit block, package-audit.test.ts:6, and SECURITY-ACCEPTED-RISKS.md:74 all describe .npmrc as setting include=dev and overriding --omit=dev; the new mcp-deps comment correctly warns against copying .npmrc into the stage.
• All other file reads succeeded. The splitIntoStages parser in Dockerfile.test.ts correctly captures both FROM base AS mcp-deps and FROM base AS mcp (the regex's optional (?:AS\s+)? plus the AS capture works for both forms used in this Dockerfile).
• git log Dockerfile confirms the three-commit history (afebaaa, be22380, f47f6a5) with the most recent commit pinning the tsx-layer recipe in tests, matching the comment thread.

Unknowns or Needs Verification

• The empirical claim "both direct npm install shapes were empirically rejected against npm 11.19" cannot be independently reproduced from the corpus; I treat the Dockerfile-side recipe as evidence the author actually iterated on this rather than invented the rationale. The new CI assert is the durable guard if npm behavior shifts in the future.
• The .npmrc content was not directly read (sensitive-file block); the inference from the surrounding code is consistent, but a reviewer who wants absolute certainty could re-read it locally.
• The hand-shake-validates behavior is asserted by Dockerfile.test.ts + the CI assert step, but the actual initialize handshake round-trip is verified by the live docker-mcp job (green on this head).

Verdict

Approve.

Requirement trace

3 of 3 requirement(s) not fully traced to enforcement and a test:

• req-e589bfe1ad7c — unverifiable (no valid enforcement location)
• req-ad902b2a08da — unverifiable (no valid enforcement location)
• req-b0d8b27b3b0e — unverifiable (no valid enforcement location)

Approval withheld: this review's coverage is incomplete — required-check coverage or the tool-loop investigation did not finish, so it is publishing as an advisory comment rather than an approval.

[itsmiso-ai]

Closing the loop on the advisory review of f47f6a5 (its three "could not verify" items):

1. .npmrc content confirmed directly (repo file, read at this head): it contains exactly include=dev with a comment "Overrides global omit=dev so npm ci installs everything needed for npm run typecheck and npm run test" — so the mcp-deps warning against COPY .npmrc and the CI: Security Audit failing on the default branch (npm audit) #1166 audit-gate history are corroborated by the file itself, not inference.
2. The npm-11.19 shape rejections are recorded in the Dockerfile comment explicitly as empirical observations from this run (npm 11.19 / node 24), not upstream-documented behavior; agree the CI assert + handshake are the durable guards, which is why both were added.
3. Live handshake is verified by the green docker-mcp job on this head (run 37214887060, job 111473273576: build + initialize handshake + "Assert MCP image ships no dev toolchain", all success). The "unverifiable requirement trace" the review cites is its own tool-loop coverage limit, not a gap in the change: each requirement's enforcement location is named in the PR body's invariants section (build recipe, Dockerfile.test.ts pins, CI assert step, CI handshake step).

State: head f47f6a5, all 13 checks green (only the reviewer check itself is advisory), prior CHANGES_REQUESTED dismissed, no open line-review threads, mergeable clean. Left open for human review per the no-merge rule.

[itsmiso-ai]

Courier: already addressed (run courier-0d332351100d97a86f8fe8b7c6646e2e)

PR #1175 is already review-ready at head f47f6a5 (local = origin/courier/misospace/dispatch/issue-1173 = PR head; base 01b6bad = origin/main, no drift). All 13 checks green on the head incl. Docker Build (MCP) run 37214887060 job 111473273576 (build + initialize handshake + 'Assert MCP image ships no dev toolchain'), plus Lint/Typecheck/Build/Tests/Coverage/npm audit/smoke/scheduler jobs. The only prior CHANGES_REQUESTED (transient CI infra on 6fdf72f) is DISMISSED; mergeable_state clean. Zero open review threads; the latest AI review on f47f6a5 is a verdict 'Approve' (advisory partial-coverage; its 3 'unverifiable' requirement-trace items are its own tool-loop coverage limits and were answered in issue comment 5981921422). All review feedback rounds (be22380, f47f6a5) are pushed and replied to; PR body invariants match the diff; human reviewer (joryirving) pending per the no-merge rule.

[itsmiso-ai]

Courier: already addressed (run courier-6172c14b9e8687a14bd16ded6c3d0c31)

PR #1175 is review-ready at head f47f6a5 (local = origin/courier/misospace/dispatch/issue-1173 = PR head; origin/main still 01b6bad = PR base, no drift). Verified live this run: all 13 checks on the head are completed/success, including Docker Build (MCP) run 37214887060 job 111473273576 (build + initialize handshake + Assert MCP image ships no dev toolchain), plus Lint/Typecheck/Build/Tests/Coverage/Database integration/Database migrations/Workflow lint/npm audit/smoke. The only prior CHANGES_REQUESTED (transient CI infra on 6fdf72f) is DISMISSED; latest its-saffron review on f47f6a5 is COMMENTED (advisory approve) and its 3 unverifiable items were answered in issue comment 5981921422; get_review_comments returns zero open threads. mergeable_state clean; PR body invariants section matches the diff (4 files, +116/-4). All review-feedback rounds (be22380, f47f6a5) are pushed and replied to. Human reviewer joryirving pending per the no-merge rule; nothing actionable remains.

[itsmiso-ai]

⚠️ This PR fix item has been marked as BLOCKED and needs human attention.

Reason: PR review: CHANGES_REQUESTED

Total attempts: 76

Attempts by lane:

• NORMAL: 76 attempts

Failing run(s):

• https://ghcr.io/token?scope=repository:${GITHUB_REPOSITORY}:pull&service=ghcr.io
• https://ghcr.io/v2/${GITHUB_REPOSITORY}/manifests/${TAG}
• https://github.com/misospace/dispatch/actions/runs/37156187905/job/111299929877
• https://github.com/.extraheader
• https://github.com/misospace/dispatch/actions/runs/37156187940/job/111299929869

Last attempt:

AI Automated Review

Full PR review.

Analysis engine: MiniMax-M3 (anthropic) — primary route · pr-reviewer-action v3.2.0

Partial coverage: required-check coverage is incomplete — this review did not resolve every required check and must not be read as a complete pass.

Findings (1 blocker, 1 info)

Severity	Location	Finding
🛑 Blocker	.github/workflows/image.yaml	CI reports `Docker Build (MCP)` as failing on this exact head (commit 6fdf72f), and this PR's only behavioral change is in the MCP build — the failure log is required before this PR can be approved.
Info	.npmrc	The PR comment in `mcp-deps` warns against copying `.npmrc` because it sets `include=dev`; this fact is corroborated by `package.json`'s `"//".audit` block, `package-audit.test.ts`, and `SECURITY-ACCEPTED-RISKS.md`, but the direct read of `.npmrc` is blocked by the harness — not an issue with this P

Recommendation

I cannot approve this PR in its current state. The full CI result is reported as a failure for Docker Build (MCP) at commit 6fdf72f, which is exactly the change this PR introduces. The PR description claims the change is verified by the existing CI handshake step but does not address why the MCP build/check is failing on this exact head. I want to see the failing step's log before the build is trusted.

In addition, the only available CI signal is the harness's reported status; I did not see the failing log content. The lint/typecheck job is reported green (Lint: success, Typecheck: success), so the AGENTS.md invariant Lint/typecheck blocks CI; must pass appears satisfied in this lane, but Docker Build (MCP) (the only relevant job for this PR) is reported as failing. Without seeing the step output I cannot tell whether the failure is the new Assert MCP image ships no dev toolchain step (in which case the dev tree is back in the MCP image and the central claim of the PR is violated), a build/cache problem on the runner, or something else. This is a known-unknowns situation, but the failing status itself is decisive: this PR's entire purpose is the MCP image build/contents, and that job is red.

I have also separately verified the rest of the diff against the linked issue and repository conventions, and the code-level claims hold up well; the failing CI is the only blocker I'm raising.

Change-by-change findings

Dockerfile — new mcp-deps stage

• The new stage correctly runs npm ci --omit=dev first and then a Node one-liner that rewrites this layer's copy of package.json: it reads p.devDependencies.tsx, throws if it is missing, moves that range into p.dependencies, deletes devDependencies, and writes JSON.stringify(p,null,2)+"\n". The throw provides a real predicate (if(!range)throw new Error(...)) and is a build-time guard, not just a property rename.
• The recipe documents the two empirically-rejected shapes (tsx@range without --omit=dev reifies the tree, with --omit=dev skips explicit dev-only packages), and the comment in mcp-deps correctly warns against COPY .npmrc because the repo .npmrc is documented (in package.json's "//".audit block and package-audit.test.ts) as setting include=dev, which overrides --omit=dev in npm's config — that matches the PR 1166 finding and is the right thing to warn against.
• The mcp stage now COPY --from=mcp-deps /app/node_modules ./node_modules and COPY --from=mcp-deps /app/package.json ./package.json (no longer COPY --from=deps). The old COPY package.json tsconfig.json ./ is split: tsconfig.json still comes from the build context (fine — no dev deps there), and the rewritten manifest is shipped. The PR's note that tsx does not read package.json at runtime is corroborated by src/mcp/server.ts (the server version is hardcoded as 1.0.0, not read from the manifest), and there are no other package.json/require('..package..') callers in src/mcp.
• The entrypoint ./node_modules/.bin/tsx src/mcp/server.ts and USER mcp are unchanged, matching the linked-issue requirement that the -mcp image keep its entrypoint, layout, and non-root user.
• ENV DATABASE_URL remains scoped to the builder stage. The mcp-deps stage does not introduce one; the existing test that walks every FROM ... AS and forbids ENV DATABASE_URL outside builder still passes (and the new belt in the test pins the mcp-deps stage directly).
• builder still does COPY --from=deps /app/node_modules ./node_modules, and runner still does COPY --from=prod-deps ... ./node_modules. Both stages are byte-identical to before — invariant 1 holds.

Dockerfile.test.ts — new MCP-wiring tests

• splitIntoStages already exists and works correctly for multi-stage FROM ... AS parsing; it is reused.
• The first new test pins the mcp stage to COPY --from=mcp-deps ... for both node_modules and package.json, and asserts COPY --from=deps is not present (it preserves a trailing space so it won't match --from=deps-something).
• The second new test pins mcp-deps to npm ci --omit=dev and asserts ENV DATABASE_URL is not set there.
• Both tests have valid expect(..., "...").toBeDefined() guards (the predicate is the expect matcher, not just an assignment). They are real tests, not just property copies.

.github/workflows/image.yaml — new Assert MCP image ships no dev toolchain step

• The step runs after the existing MCP handshake validation in the docker-mcp job, matching the PR's claim that the new step is added after the handshake.
• It uses set -euo pipefail, picks the first tag, docker pulls on non-PR events (consistent with the handshake step's pattern), and runs a docker run --rm --entrypoint sh that scans for the full advisory chain plus a dev-tool smoke list. The chain check includes eslint, eslint-config-next, @next/eslint-plugin-next, fast-glob, micromatch, braces, and the dev smoke list includes vitest, typescript, ts-node.
• The chain check scans top-level and one nesting level under node_modules/*/node_modules and the scoped form node_modules/@*/*/node_modules (covering @next/eslint-plugin-next and @testing-library/...-style hosts). That covers the duplicate-under-host-package concern called out in the second commit (be22380).
• One subtle point: when the script finds a violating path it echos a line and sets rc=1, then exit "$rc". That is correct under set -e because the assignment is not the last command in the loop and rc=1 (a bare assignment) is not a failing command.
• If this step is the one failing CI on this head, it means the MCP image being produced by this PR contains one of those packages — directly contradicting the PR's central claim. That's why I want to see the failing log.

SECURITY-ACCEPTED-RISKS.md

• The Dev-Only Advisories section's "Shipping surface" bullet is correctly rewritten to state that the chain is absent from both published images and to point at the new CI assert.
• A new retired-risk row is added tying the chain's image-scope resolution to PR 1173.

Claims-to-falsify

1. Claim 1 (image shipped the dev lint chain before this PR; ghcr.io/misospace/dispatch-mcp is the published target). The code change retargets the mcp stage's node_modules and package.json to the new mcp-deps stage. Cannot be fully verified from the corpus because the authoritative evidence is the new build's published image, and that build (Docker Build (MCP)) is reported as failing on this head. Unverified: 0/8 items individually violated; the entire claim is dependent on a green Docker Build (MCP) which is currently red.
2. Claim 2 (mcp copies node_modules and package.json from mcp-deps; CI adds the assert after handshake). Held: Dockerfile L105–L108 do exactly that, and image.yaml adds the new step directly after the existing handshake step. Held: 12/12 line items in the diff match the claim.
3. Claim 3 (both npm install shapes empirically rejected). The comment in mcp-deps documents the rejection. The third commit (be22380) added a tsx missing from devDependencies (#1173) throw, satisfying the stronger review-round claim. I cannot independently re-run npm 11.19 against the lockfile to confirm the empirical claim itself (no 11.19 references found in the search). Held from comments/test, unverified empirically.
4. Claim 4 (builder still installs full dev tree from deps; runner ships prod-only from prod-deps; deps and prod-deps stages unchanged). Held: COPY --from=deps /app/node_modules ./node_modules is in builder; COPY --from=prod-deps ... ./node_modules is in runner; the deps and prod-deps stages both do the same single-line npm ci and npm ci --omit=dev as before, with no tsx/rewriting touching them. The mcp-deps stage sits between prod-deps and builder but does not feed builder or runner. Held: all 12 line items match.
5. Claim 5 (mcp is the only retargeted stage; ENV DATABASE_URL stays inside builder). Held: only mcp rewires its COPY --from= source; mcp-deps does not set ENV DATABASE_URL; the existing Dockerfile.test.ts enforcement walks every stage and forbids ENV DATABASE_URL outside builder. Held: 12/12 line items match.

Linked Issue Fit

Linked issue PR 1173 calls for the -mcp image to be built from a production-only install plus tsx, with handshake verification retained. The PR delivers exactly that:

• The -mcp image's node_modules now comes from a mcp-deps stage that uses npm ci --omit=dev plus a layer-local tsx install (achieved via rewriting this layer's package.json to add tsx to dependencies and drop devDependencies, then a plain npm install --no-save). ✅
• The mcp stage's existing handshake validation is preserved (image.yaml Validate MCP handshake), and a stronger assertion is added. ✅
• Files touched match the expected files: Dockerfile (deps/mcp stages, with a new mcp-deps stage inserted), .github/workflows/image.yaml (docker-mcp job). ✅
• The mcp stage retains ENTRYPOINT ["./node_modules/.bin/tsx", "src/mcp/server.ts"] and USER mcp. ✅
• The --no-save recipe and the tsx missing from devDependencies build-time throw are not specifically mentioned in the issue, but they are documented as the answer to the implicit "how do you install tsx without --omit=dev re-admitting the dev tree" question. This is a defensible implementation choice that the issue does not foreclose.

The only open issue here is that the CI lane that would prove this fix — Docker Build (MCP) — is reported as failing on this exact head.

Standards Compliance

The relevant repository conventions are in AGENTS.md (Docker section). The PR follows them: node:24-bookworm-slim base is preserved, multi-stage structure is preserved, and tsx remains a devDependency in package.json so the main runner image never ships it. The Dockerfile does not introduce any new package or OS dependency. No convention in AGENTS.md is violated by the diff.

Tool Harness Findings

• read_file Dockerfile, Dockerfile.test.ts, .github/workflows/image.yaml, SECURITY-ACCEPTED-RISKS.md, package.json, package-audit.test.ts, src/mcp/server.ts — all succeeded and match the diff.
• read_file .npmrc and git_grep against .npmrc — both blocked by the harness as a sensitive file. This blocks a direct read of .npmrc to confirm the include=dev line, but the same fact is established by three independent sources already in the corpus: package.json's "//".audit comment, package-audit.test.ts line 6 ("include=dev (added in PR 428)"), and SECURITY-ACCEPTED-RISKS.md "Retired Risks" entry for PR 1166. So the PR's mcp-deps warning against copying .npmrc is well-grounded.
• read_file .dockerignore — confirmed: the repo's .dockerignore only excludes local-only files (.env.local, node_modules, .next, etc.). It does not exclude .npmrc, so the explicit "do not COPY .npmrc" comment in mcp-deps is necessary, not redundant with .dockerignore.
• git_grep serverInfo|version src/mcp — confirmed src/mcp/server.ts hardcodes version: "1.0.0" in the createServer() call. Supports the PR's claim that removing package.json's devDependencies does not affect the MCP handshake.

Unknowns / Needs Verification

• Failing Docker Build (MCP) step output. The PR's central claim is that the -mcp image no longer ships the dev toolchain. The CI lane that proves it is reported as failing on this exact head. I cannot tell from the corpus alone whether the failure is the new assert (which would mean the fix did not work), a build/cache problem, or a workflow YAML issue. I need the failing step's log to disposition this PR.
• Empirical npm 11.19 rejection of the two direct tsx@range install shapes. Documented in the Dockerfile comment and corroborated by the second commit's added tsx missing from devDependencies throw, but not re-verifiable from the corpus without running the lockfile against npm 11.19. Not blocking on its own, but worth noting.
• No git_diff_stat output was returned by run_command (the harness returned an empty stdout). Not material to the review.

Requirement Coverage

• req-e589bfe1ad7c (Lint/typecheck blocks CI; must pass). Lint: success, Typecheck: success per the CI Check Results summary. Test file: Dockerfile.test.ts. The repo-wide lint/typecheck lane is green; this requirement appears met at the CI level. disposition: met with the caveat that the relevant MCP lane is failing for an unrelated (or possibly related) reason.
• req-ad902b2a08da (DISPATCH_AGENT_TOKEN / GITHUB_TOKEN must never be logged/echoed/persisted). The MCP handshake step echoes DISPATCH_AGENT_TOKEN=ci (a constant placeholder, not a real secret) into docker run. The CI assert step does not echo any token. No new secret-handling surface is introduced. disposition: not_applicable — this requirement applies to secret management, and the PR does not change how secrets are handled.
• req-b0d8b27b3b0e (runner image must not gain tsx/dev tooling; tsx remains a devDependency; mcp-deps rewrites only its layer-local copy; warning against copying .npmrc). Enforcement: Dockerfile mcp stage does COPY --from=mcp-deps /app/node_modules ./node_modules; mcp-deps is a distinct stage that does not feed runner or prod-deps; tsx is still in package.json devDependencies; the inline comment in mcp-deps warns against COPY .npmrc. Test: Dockerfile.test.ts "ships the MCP image from the production-only mcp-deps tree" and "installs mcp-deps without dev dependencies". Both files appear in the diff. disposition: met. The Docker test pins the wiring; the npm-empirical part (that --no-save then npm install adds only tsx + esbuild) is asserted by the image.yaml CI step, which is currently failing on this head — so the enforcement-line is sound but the runtime evidence is inconclusive pending the CI failure log.

Requirement trace

3 of 3 requirement(s) not fully traced to enforcement and a test:

• req-e589bfe1ad7c — unverifiable (no valid enforcement location)
• req-ad902b2a08da — unverifiable (no valid enforcement location)
• req-b0d8b27b3b0e — unverifiable (no valid enforcement location)

Posted automatically by Dispatch on 2026-10-04T16:13:53.889Z

[joryirving]

Re-reviewed current head f47f6a5. Approve.

I independently checked the current 4-file diff against #1173 rather than relying on the earlier review rounds.

• mcp now sources both node_modules and the shipped manifest from the production-only mcp-deps stage; the main runner path remains untouched.
• The layer-local manifest rewrite keeps tsx sourced from the repo's devDependency range while preventing the repo dev tree from becoming the MCP runtime tree.
• The root postinstall is harmless in this stage because prisma/schema.prisma is not present here; the subsequent install therefore does not introduce a hidden build/runtime dependency.
• The authoritative Docker Build (MCP) job is green on this exact head: image build, live MCP initialize handshake, and Assert MCP image ships no dev toolchain all succeeded.
• The accepted-risk documentation now matches the built-image evidence.
• The splitIntoStages FROM --platform weakness is real but pre-existing and correctly tracked separately in #1178; the current Dockerfile does not use that form, so it does not invalidate this PR's tests.

The stale CHANGES_REQUESTED review was tied to the older transient red build, not this head. I found no remaining merge blocker or nit worth holding #1175 for.