mirendev/runtime#980

My first concern — WebCommand() returning npx next start even for yarn projects — has been fixed. Lines 282–283 of node.go now return "yarn next start -p $PORT" when the package manager is yarn, and the "next with yarn" test case at line 434 correctly expects that value. That's the right fix and the comment at lines 279–280 makes the intent explicit.

My second concern — no LLB-level test for the Next.js build step — is still open. TestNodeNextjs (lines 408–476) exercises detection, frameworkBuildCommand(), and WebCommand(), but it never calls GenerateLLB(). The new code that chains build.AddEnv(k, v) in a loop and then calls build.Run(llb.Shlex(buildCmd), ...) before applyOnBuild is entirely untested at the graph-construction level. A mistake there — wrong ordering, a stale state variable, a nil-map env injection — would not be caught by the current suite.

This is a draft, so I'm judging readiness to hand to human review. The detection logic and command-string layer are solid and now consistent for both package managers. The unverified LLB wiring is a concrete gap: either add a GenerateLLB call to TestNodeNextjs (even just require.NoError(t, err) after the call, with checkDocker() guard if you want to skip buildLLB) to confirm the graph marshals without error, or accept that risk consciously.

Inline comments

• pkg/stackbuild/stackbuild_test.go:476 — TestNodeNextjs still never calls GenerateLLB, so the new branch in GenerateLLB that loops over opts.EnvVars and chains a build.Run(...) step is not exercised at all. Consider adding a call after stack.Init(BuildOptions{}) — something like:

state, err := stack.GenerateLLB(dir, BuildOptions{})
require.NoError(t, err)
_ = state

That alone (no Docker required) would catch ordering bugs or panics in the LLB construction path. You can gate a full buildLLB call behind checkDocker() if you want deeper validation.

---

Verdict: caveats

mirendev/runtime#980

This is a draft, so I'm judging readiness to hand off to human review. The change is small, well-motivated, and mostly correct — but there's one concrete correctness gap and a couple of smaller things worth calling out before it moves forward.

What the PR does

Init() now checks whether next is in either dependencies or devDependencies (via the new hasDependency helper). If it is, frameworkBuildCommand() returns yarn build or npm run build as appropriate, and WebCommand() short-circuits to npx next start -p $PORT instead of falling through to the script/entry-point resolution logic.

The TestNodeNextjs table test covers: Next.js under npm, under yarn, Next.js under devDependencies, and a plain Express app. That's good coverage for the detection and command-string logic.

Concern 1 — next start always uses npx, even when yarn is the package manager

WebCommand() returns "npx next start -p $PORT" regardless of which package manager was detected. The frameworkBuildCommand() correctly uses yarn build for yarn projects, but the start command doesn't follow suit. For a yarn project the natural equivalent would be yarn next start -p $PORT (or at minimum yarn dlx), not npx. The TestNodeNextjs "next with yarn" case accepts npx next start -p $PORT as wantWeb, so the test is written to match the implementation rather than to challenge it. If this is an intentional choice (e.g. to avoid depending on whether next is installed in the project's node_modules/.bin), it deserves a comment; if not, it should be fixed.

Concern 2 — No integration test for the Next.js build step in GenerateLLB

TestNodeNextjs exercises hasNext, frameworkBuildCommand(), and WebCommand() but never calls GenerateLLB. The new code path that injects env vars and emits build.Run(llb.Shlex(buildCmd), ...) is untested at the LLB level. The existing TestNode does call buildLLB, but that fixture has no next dependency, so it never enters the new branch. An integration test for a minimal Next.js project (even a stub one, similar to the Rails/Ruby pattern) would catch issues like bad LLB ordering or environment injection mistakes.

Concern 3 — hasDependency is called in Init, but Init calls parsePackageJSON first only when invoked directly

This is fine as written — Init always calls parsePackageJSON before hasDependency, so s.dependencies and s.devDependencies are populated. The guard if s.dependencies[name] doesn't crash on a nil map (Go treats a nil map read as a zero-value hit). No bug here, just worth being aware that hasDependency is only safe after parsePackageJSON runs.

Minor

wantWeb in the "next listed under devDependencies" test case is "npx next start -p $PORT". That's fine if Concern 1 is intentional, but it means this case doesn't actually verify that devDependencies detection works end-to-end through WebCommand differently from dependencies — it would pass regardless.

---

Bottom line: the core detection and command-string logic is sound and I like the clean hasDependency helper. The npx vs yarn mismatch in WebCommand is a concrete correctness question that should be resolved before merging, and the missing LLB-level integration test means the build-step wiring is unverified. Either address these or consciously accept them.

Inline comments

• pkg/stackbuild/node.go:280 — WebCommand() returns "npx next start -p $PORT" unconditionally, but frameworkBuildCommand() already distinguishes yarn vs npm. For a yarn project, yarn manages the local next binary; relying on npx (an npm tool) could fail or use a different binary resolution path. Either use yarn next start -p $PORT when s.packageManager == nodePkgYarn, or leave a comment explaining why npx is intentional here.
• pkg/stackbuild/stackbuild_test.go:400 — TestNodeNextjs never calls GenerateLLB, so the new branch in GenerateLLB that emits the build step and injects env vars is untested at the LLB level. Consider adding a lightweight integration test (guarded by checkDocker(), like TestNode) that creates a minimal Next.js-shaped project (the package.json stub is already here), calls GenerateLLB, and verifies at minimum that the returned *llb.State marshals without error — and ideally runs buildLLB to confirm the step sequence is valid.

---

Verdict: caveats