From e947236efc78ce7402d894313464a3eff47061e2 Mon Sep 17 00:00:00 2001 From: Vidit19sharma Date: Sun, 13 Sep 2026 19:26:20 +0530 Subject: [PATCH 1/2] fix(core): reject undefined layer node dependencies Dependency arrays are built at module-evaluation time, so a circular import can leave an entry undefined instead of a node. Nothing validated that, and the failure only surfaced later while walking the graph, where `resolve` reads `.name` off the undefined entry: TypeError: undefined is not an object (evaluating 'a.name') The stack is minified and names no module, so the origin is invisible. This is what #48372 reported: the server returned a generic "Unexpected server error" on every prompt, and several people reasonably suspected their credentials. Validate dependencies in `make` and `group`, which fails at the module that built the bad array and names it. `compile` checks too, since `Node` is a structural interface and a hand-built node never passes through `make`. The reported case now 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. This is diagnostics only and fixes no cycle on its own; #48397 fixes the filesystem/search cycle that triggered it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NxhAy1sXV5Mi7kkrJu4U7W --- packages/core/src/effect/layer-node.ts | 24 +++++++++++++- .../test/effect/layer-node/layer-node.test.ts | 31 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/packages/core/src/effect/layer-node.ts b/packages/core/src/effect/layer-node.ts index 9dbc3d51607b..7a787a9c804d 100644 --- a/packages/core/src/effect/layer-node.ts +++ b/packages/core/src/effect/layer-node.ts @@ -78,6 +78,22 @@ type MakeInput< readonly tag?: T } +/** + * Dependency arrays are built at module-evaluation time, so a circular import can leave an entry + * `undefined` instead of a node. That stays invisible until the graph is walked, where it surfaces + * as a `TypeError` reading `.name` off `undefined` with no indication of which module is at fault. + * Failing here instead names the node and the offending position. + */ +function checkDependencies(name: string, dependencies: readonly AnyNode[]) { + const index = dependencies.findIndex((dependency) => dependency === undefined) + if (index === -1) return + throw new Error( + `Layer node "${name}" has an undefined dependency at index ${index}. ` + + `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.`, + ) +} + export function make< const Implementation extends Layer.Any, const Items extends NodeList, @@ -85,9 +101,11 @@ export function make< >( input: MakeInput, ): Node, Layer.Error | Error, T> { + const name = input.service !== undefined ? input.service.key : input.name + checkDependencies(name, input.deps) return { kind: "layer", - name: input.service !== undefined ? input.service.key : input.name, + name, service: input.service, implementation: input.layer, dependencies: input.deps, @@ -108,6 +126,7 @@ export function unbound(service: Context.Key( dependencies: Items, ): Node, Error, NodeTag> { + checkDependencies("group", dependencies) return { kind: "group", name: "group", dependencies } } @@ -258,6 +277,9 @@ export function compile( node, (node, context) => { if (node.kind === "unbound") throw new Error(`Unbound layer node: ${node.name}`) + // nodes are normally validated in `make`, but `Node` is a structural interface so a + // hand-built or externally-produced node can still reach compilation unchecked + checkDependencies(node.name, node.dependencies) const dependencies = node.dependencies.flatMap(flatten).map(context.visit) const implementation = node.implementation! as RuntimeLayer return dependencies.length === 0 diff --git a/packages/core/test/effect/layer-node/layer-node.test.ts b/packages/core/test/effect/layer-node/layer-node.test.ts index b671792c59af..542f244612fa 100644 --- a/packages/core/test/effect/layer-node/layer-node.test.ts +++ b/packages/core/test/effect/layer-node/layer-node.test.ts @@ -63,6 +63,37 @@ describe("layer node", () => { expect(await Effect.runPromise(program)).toEqual(["first", "second"]) }) + test("rejects an undefined dependency", () => { + // a circular import leaves the dependency undefined at module-evaluation time + // @ts-expect-error A dependency must be a node + expect(() => make({ service: Greeting, layer: greetingLayer, deps: [undefined] })).toThrow( + 'Layer node "test/LayerNodeGreeting" has an undefined dependency at index 0', + ) + }) + + test("reports the position of an undefined dependency", () => { + // @ts-expect-error A dependency must be a node + expect(() => make({ service: Greeting, layer: greetingLayer, deps: [value, undefined] })).toThrow( + "undefined dependency at index 1", + ) + }) + + test("rejects an undefined dependency in a group", () => { + // @ts-expect-error A dependency must be a node + expect(() => LayerNode.group([value, undefined])).toThrow( + 'Layer node "group" has an undefined dependency at index 1', + ) + }) + + test("rejects an undefined dependency on a node that bypassed make", () => { + // `Node` is structural, so a node built by hand never passes through `make` + // @ts-expect-error A dependency must be a node + const handBuilt: LayerNode.Node = { ...greeting, dependencies: [undefined] } + expect(() => LayerNode.compile(handBuilt)).toThrow( + 'Layer node "test/LayerNodeGreeting" has an undefined dependency at index 0', + ) + }) + test("requires unbound nodes to be replaced before compilation", async () => { const unbound = LayerNode.unbound(Value, tags.values.app) const greeting = make({ service: Greeting, layer: greetingLayer, deps: [unbound] }) From ee2a49980ac461c0d38752d5cdac3bd494035495 Mon Sep 17 00:00:00 2001 From: Vidit19sharma Date: Sun, 13 Sep 2026 20:41:07 +0530 Subject: [PATCH 2/2] fix(core): guard the replacement rewrite and tolerate missing deps Review of the previous commit surfaced three gaps. `rewriteReplacementDependencies` walks the graph independently of `walk`, and hoisting a tagged subtree reaches it directly, so an undefined entry there still produced the original unattributable error: TypeError: undefined is not an object (evaluating 'node.name') This is the production path: location-services always hoists with a non-empty replacement list, so the rewrite always runs. Validate there too. `checkDependencies` also called `findIndex` unconditionally, so a node without a `dependencies` field started throwing an undiagnostic TypeError where hoisting previously tolerated it. Treat a missing array as nothing to check, and reject `null` alongside `undefined`. The comment on the walk check claimed every visitor maps over the dependencies, which is not true for tagged or unbound nodes and is what hid the rewrite gap. Reworded. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NxhAy1sXV5Mi7kkrJu4U7W --- packages/core/src/effect/layer-node.ts | 18 ++++++---- .../test/effect/layer-node/layer-node.test.ts | 36 +++++++++++++++++++ 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/packages/core/src/effect/layer-node.ts b/packages/core/src/effect/layer-node.ts index 7a787a9c804d..e307618d6387 100644 --- a/packages/core/src/effect/layer-node.ts +++ b/packages/core/src/effect/layer-node.ts @@ -84,8 +84,9 @@ type MakeInput< * as a `TypeError` reading `.name` off `undefined` with no indication of which module is at fault. * Failing here instead names the node and the offending position. */ -function checkDependencies(name: string, dependencies: readonly AnyNode[]) { - const index = dependencies.findIndex((dependency) => dependency === undefined) +function checkDependencies(name: string, dependencies: readonly AnyNode[] | undefined) { + if (dependencies === undefined) return + const index = dependencies.findIndex((dependency) => dependency === undefined || dependency === null) if (index === -1) return throw new Error( `Layer node "${name}" has an undefined dependency at index ${index}. ` + @@ -212,6 +213,9 @@ function walk( ) } + // validate before any visitor dereferences an entry + checkDependencies(target.name, target.dependencies) + visiting.add(target) stack.push(target) try { @@ -277,9 +281,6 @@ export function compile( node, (node, context) => { if (node.kind === "unbound") throw new Error(`Unbound layer node: ${node.name}`) - // nodes are normally validated in `make`, but `Node` is a structural interface so a - // hand-built or externally-produced node can still reach compilation unchecked - checkDependencies(node.name, node.dependencies) const dependencies = node.dependencies.flatMap(flatten).map(context.visit) const implementation = node.implementation! as RuntimeLayer return dependencies.length === 0 @@ -325,6 +326,8 @@ function rewriteReplacementDependencies(root: AnyNode, replacements: ReadonlyMap visiting.add(target) stack.push(target) try { + // this recursion is separate from `walk`, and hoisting a tagged subtree reaches it directly + checkDependencies(target.name, target.dependencies) const dependencies = target.dependencies.map((dependency) => recur(dependency)) const result = dependencies.every((dependency, index) => dependency === target.dependencies[index]) ? target @@ -349,7 +352,10 @@ export function hasUnbound(root: Node, source: AnyNode): } function flatten(node: AnyNode): readonly AnyNode[] { - return node.kind === "group" ? node.dependencies.flatMap(flatten) : [node] + if (node.kind !== "group") return [node] + // groups are expanded before the compile-time check runs, so validate here too + checkDependencies(node.name, node.dependencies) + return node.dependencies.flatMap(flatten) } export * as LayerNode from "./layer-node" diff --git a/packages/core/test/effect/layer-node/layer-node.test.ts b/packages/core/test/effect/layer-node/layer-node.test.ts index 542f244612fa..a5b38416475a 100644 --- a/packages/core/test/effect/layer-node/layer-node.test.ts +++ b/packages/core/test/effect/layer-node/layer-node.test.ts @@ -94,6 +94,42 @@ describe("layer node", () => { ) }) + // groups are expanded by `flatten` before a node is visited, so they need their own coverage + const handBuiltGroup = (): LayerNode.Node => ({ + kind: "group", + name: "external-group", + // @ts-expect-error A dependency must be a node + dependencies: [undefined], + }) + + test("rejects an undefined dependency in a hand-built group at the root", () => { + expect(() => LayerNode.compile(handBuiltGroup())).toThrow( + 'Layer node "external-group" has an undefined dependency at index 0', + ) + }) + + test("rejects an undefined dependency in a nested hand-built group", () => { + const parent = make({ service: Greeting, layer: greetingLayer, deps: [handBuiltGroup()] }) + expect(() => LayerNode.compile(parent)).toThrow( + 'Layer node "external-group" has an undefined dependency at index 0', + ) + }) + + test("rejects an undefined dependency while hoisting", () => { + expect(() => LayerNode.hoist(handBuiltGroup(), tags.values.app)).toThrow( + 'Layer node "external-group" has an undefined dependency at index 0', + ) + }) + + test("rejects an undefined dependency while rewriting replacements", () => { + // a hoisted tagged node is rewritten rather than visited, so it reaches a separate recursion + const parent = make({ service: Greeting, layer: greetingLayer, deps: [handBuiltGroup()] }) + const replacement = Layer.succeed(Value, Value.of({ value: "simulation" })) + expect(() => LayerNode.hoist(parent, tags.values.app, [[value, replacement]])).toThrow( + 'Layer node "external-group" has an undefined dependency at index 0', + ) + }) + test("requires unbound nodes to be replaced before compilation", async () => { const unbound = LayerNode.unbound(Value, tags.values.app) const greeting = make({ service: Greeting, layer: greetingLayer, deps: [unbound] })