fix(core): reject undefined layer node dependencies

[vidit19sharma]

Issue for this PR

Closes #48819.

Follow-up to #48372, split out at @kernel-oops's suggestion on #48397 ("Agreed that dependency validation in LayerNode.compile would make diagnosis much clearer. Please do open that as a separate follow-up issue/PR rather than expanding the scope here.").

This is diagnostics only. It fixes no cycle on its own — #48397 fixes the filesystem / filesystem/search cycle.

Important

This should land after or together with #48397, not before it.
On a tree where that cycle is still present, the guard fires during startup, so the compiled binary fails on every command — including --version — and script/build.ts fails its own smoke test. I verified both states on macOS arm64: guard alone on current dev fails the smoke test; guard plus #48397 builds, passes the smoke test, and completes a real prompt. Details below.

Type of change

• Bug fix
• New feature
• Refactor / code improvement
• Documentation

What does this PR do?

Layer dependency arrays are built at module-evaluation time, so a circular import can leave an entry undefined instead of a node. Nothing validated that. The failure surfaced much later, while walking the graph, at:

{ cache, resolve: (node) => replacementMap.get(node.name) ?? node }

Reading .name off the undefined entry produced:

TypeError: undefined is not an object (evaluating 'a.name')

In a release build that stack is minified and names no module, and the server turns it into a generic Unexpected server error. In #48372 that meant every prompt failed with no indication of the cause — several people reasonably concluded their credentials were at fault.

This adds a checkDependencies guard:

• make and group — the primary check. It throws at the module that built the bad array, so the stack points at the responsible module rather than at graph-compilation internals.
• compile — Node is a structural interface, so a hand-built or externally-produced node can reach compilation without passing through make.

With the guard, the #48372 case fails as:

Layer node "@opencode/v2/FileSystem" has an undefined dependency at index 2.
This usually means a circular import: the module that provides it has not
finished initializing. Break the cycle, for example by making the import
type-only.

Naming the node and the index is enough to locate the cycle directly.

The check also runs in rewriteReplacementDependencies, which walks the graph independently of walk. Hoisting a tagged subtree reaches that rewrite directly, and location-services.ts always hoists with a non-empty replacement list, so without a check there the production path still produced the original node.name TypeError.

Behaviour change, stated plainly: on a tree that still has an undefined dependency, this fails at startup rather than at the first prompt. That is a real difference in blast radius — an affected build goes from "starts, then fails on every prompt" to "does not start at all", and the build script's smoke test fails. The build is broken in both cases and the early failure is the diagnosable one, but it is why this should not merge ahead of #48397.

Why make, given the suggestion named compile

Checking at construction gives a materially better stack — it names the module whose deps: [...] is wrong, which is the thing you actually need. By compile time that context is gone and only the node name remains. compile is still checked as a backstop for nodes that never went through make, so both paths are covered.

How did you verify your code works?

macOS 15 (Darwin 25.6.0) arm64, Bun 1.4.2, dev at 95daf90.

The guard catches the real reported case. With the filesystem/search cycle still unpatched, bundling and running the actual application modules:

Layer node "@opencode/v2/FileSystem" has an undefined dependency at index 2. ...
    at checkDependencies
    at make24

versus the TypeError: undefined is not an object (evaluating 'a.name') without it.

No false positives. With the same unpatched cycle but the safe import order, the guard stays silent and the graph resolves normally:

deps: [ "@opencode/FileSystem", "@opencode/Location", "@opencode/v2/FileSystem/Search" ]

Test suite and checks:

• bun test in packages/core — 1105 pass, 0 fail, 144 files
• bun test in packages/opencode — 3584 pass, 22 skip, 0 fail, 254 files
• llm 298, tui 193, codemode 263 — 0 fail. schema, enterprise and sdk-next have failures that are byte-identical with and without this change (verified by stashing it), so they are pre-existing and unrelated
• The real locationServices graph (36 top-level nodes) hoists with replacements and compiles cleanly, so the guard does not false-positive on the shipped graph
• script/build.ts --single with fix(core): break filesystem cycle in compiled prompts #48397 applied — smoke test passes, and the built binary completes a real prompt
• bun typecheck (repo root, all 30 packages) — 30 successful, 30 total
• bunx oxlint on both changed files — 0 errors, and 14 warnings, unchanged from the pre-change baseline on those files
• bunx prettier --check on both changed files — clean
• No dependency or lockfile changes

Added tests in test/effect/layer-node/layer-node.test.ts, covering: rejection by make; a correct index for a non-zero position; rejection in group; rejection in compile for a node that bypassed make; hand-built groups at the root and nested (groups are expanded by flatten before a node is visited, so they need their own coverage); hoisting; and the replacement rewrite. Each of these fails against the pre-change source.

Not covered by my testing: Windows and Linux, and other binary targets. The change is platform-independent, but I have only exercised it on macOS arm64.

Screenshots / recordings

Not applicable.

Checklist

• I have tested my changes locally
• I have not included unrelated changes in this PR

[github-actions]

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

1. Open an issue describing the bug/feature (if one doesn't exist)
2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

[kernel-oops]

Thanks for splitting this out. I agree with the scope: this improves diagnosis of #48372; #48397 is still needed to remove the filesystem/search runtime cycle. Checking at construction also makes sense for attributing the captured undefined dependency to its owning node.

One gap in the structural-node backstop, from reading e947236e: compile calls flatten(root) before its new check, and node.dependencies.flatMap(flatten) recursively expands groups without validating their dependencies. A hand-built group therefore bypasses the guard:

const badGroup: LayerNode.Node<never> = {
  kind: "group",
  name: "external-group",
  // @ts-expect-error Deliberately malformed dependency
  dependencies: [undefined],
}
LayerNode.compile(badGroup)

Here flatten reaches undefined.kind rather than the named dependency error; the same applies when that group is nested under an otherwise valid layer. Could you validate group dependencies before recursively flattening them, and add root/nested hand-built-group tests alongside the hand-built-layer test?

This is a source-review finding, not an independent run of your suite or a new platform-validation claim. The normal make/group construction checks do cover the reported filesystem case.

[vidit19sharma]

Good catch — confirmed and fixed in ee2a499. Your reading was exactly right: flatten expanded groups before the check ran, so both the root and nested hand-built cases hit undefined.kind rather than the named error.

Chasing it turned up a third instance of the same class that neither of us had flagged, and it was the one that mattered.

rewriteReplacementDependencies was reproducing the original error on the production path

It walks the graph independently of walk, and hoist reaches it directly for tagged nodes — the tagged branch stores the node via rewriteReplacementDependencies and returns group([]) without calling context.visit, so the whole subtree was only ever traversed by that unguarded recursion. An undefined entry there gave:

TypeError: undefined is not an object (evaluating 'node.name')

which is the original #48372 signature, unchanged.

This is not hypothetical. location-services.ts:96 hoists with allReplacements = replacements.concat([[Location.node, Location.boundNode(ref)]]), which is never empty, so rewriteReplacementDependencies skips its size === 0 early return and always runs. Without a check there, the headline claim of this PR was false for anything under a global-tagged node. Now validated in that recursion too, with a regression test that fails if the check is removed.

Two smaller things from the same pass

• checkDependencies called findIndex unconditionally, so a node with no dependencies field went from tolerated to throwing an undiagnostic TypeError — the exact error class this PR exists to remove, emitted by the guard itself. A missing array is now treated as nothing to check.
• === undefined let null through to die later in flatten; it now rejects both.

I also reworded the comment on the walk check. It claimed every visitor maps recur over the dependencies, which is untrue for tagged and unbound nodes — that inaccuracy is precisely what hid the rewrite gap.

On sequencing

Building the binary surfaced something worth stating explicitly, and I have added it to the PR description: this should not merge ahead of #48397. With that cycle still present, the guard fires during startup, so the compiled binary fails on every command including --version, and script/build.ts fails its own smoke test. Verified both ways on macOS arm64 — guard alone on current dev fails the smoke test; guard plus your search.ts change builds, passes the smoke test, and completes a real prompt.

Verification

• packages/opencode 3584 pass / 22 skip / 0 fail; packages/core 1105 pass / 0 fail
• The real locationServices graph (36 top-level nodes) hoists with replacements and compiles cleanly, so the guard does not false-positive on the shipped graph
• Repo-wide bun typecheck 30/30; oxlint unchanged from baseline on the touched files; prettier --check clean
• schema, enterprise and sdk-next have failures that are byte-identical with and without this change, so they are pre-existing

Added tests for hand-built groups at the root and nested, for hoisting, and for the replacement rewrite. Each fails against the pre-change source.