Skip to content

fix(core): break filesystem cycle in compiled prompts - #48397

Open
kernel-oops wants to merge 1 commit into
anomalyco:devfrom
kernel-oops:compiled-filesystem-cycle
Open

kernel-oops wants to merge 1 commit into
anomalyco:devfrom
kernel-oops:compiled-filesystem-cycle

Conversation

@kernel-oops

@kernel-oops kernel-oops commented Sep 10, 2026

Copy link
Copy Markdown

Issue for this PR

Fixes #48372. Related to #44946; this complements that Bun upgrade rather than duplicating its runtime pin change. The upgrade is useful for fixes such as oven-sh/bun#35356 (GC timer CPU usage), but compiled prompt preparation also needs checking.

Type of change

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

What does this PR do?

Remove the runtime import from filesystem search back to the filesystem service. Schema constructors now come directly from the schema package; service input imports remain type-only.

The service depends on the search node. With Bun 1.4.2, the compiled cycle can capture an undefined dependency: health passes, but the first prompt fails in SystemPrompt.environment with TypeError: undefined is not an object (evaluating 'a.name'), before any provider request.

Add an isolated compiled regression and a path-filtered Linux CI job using Bun 1.4.2. The runner exercises authenticated session creation, a streamed request for the actual read tool, completion, persisted tool output and a second turn. All provider responses are deterministic and local.

How did you verify your code works?

Fresh upstream dev at 193de13a88d62a6409c6d385831180f1def527dc, Linux x64/glibc, Bun 1.4.2, native minified/split builds with embedded web UI:

  • Before: health passes; first prompt fails with the error above; zero provider requests.
  • After: compiled regression passes; exactly three provider requests. The embedded runtime is checked by the runner.
  • Frozen-lockfile installation; no lockfile changes.
  • Core filesystem/location/read tests: 46 pass, also 46 pass with FFF disabled.
  • Isolated processor/permission/config/storage tests: 345 pass, 3 platform skips. An initial run inherited local configuration and failed; the clean-environment rerun passed.
  • Core and OpenCode typechecks, formatting, Python syntax, workflow YAML and inline shell checks pass.

Soak update: the operator reports a few days of normal server and TUI use with excellent CPU usage and no noticed issues. This is the custom 1.18.30-kernel-oops-bun142-promptfix1 build with Bun 1.4.2, now used for normal projects, not pristine upstream or a controlled multi-day test. The clean-upstream regression results above are separate.

Windows runner support, other binary targets, real providers and CPU benchmarks were not tested by this PR's isolated regression. Broader platform coverage and upstream review remain outstanding.

Screenshots / recordings

Not applicable.

Checklist

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

@github-actions

Copy link
Copy Markdown
Contributor

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.

@vidit19sharma

Copy link
Copy Markdown

Independent confirmation of this fix on macOS arm64, which I think is the platform gap left by the verification above (Linux x64/glibc).

I hit #48372 after a routine brew upgrade to 1.18.30 and debugged it from scratch before finding this PR; I landed on the same root cause and an equivalent patch, so this is a genuinely independent reproduction rather than a re-run of the same steps.

Environment

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

The undefined dependency is directly observable

Rather than driving a full prompt, the bad dependency can be observed at the graph level. Forcing the hazardous evaluation order (search before filesystem) and bundling is enough:

import "./src/filesystem/search"
import { FileSystem } from "./src/filesystem"
const deps = (FileSystem.node as any).dependencies
console.log(deps.map((d: any) => d?.name ?? "<UNDEFINED>"))
bun build ./probe.ts --target=bun --outfile=probe.js && bun run probe.js

Before (unpatched dev):

[ "@opencode/FileSystem", "@opencode/Location", "<UNDEFINED>" ]

With this PR's search.ts change applied:

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

That <UNDEFINED> is what later reaches resolve in layer-node.ts:

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

node.name on the undefined entry is the minified a.name in the reported stack.

Note this only manifests bundled. Run from source, the same cycle throws the much clearer ReferenceError: Cannot access 'node' before initialization — which is likely why it escaped pre-release testing.

Also checked, with this PR applied

  • tsc --noEmit -p packages/core — exit 0
  • bun test packages/core/test/filesystem/ — 36 pass, 0 fail

This appears to be the only instance of the pattern

Since the failure class is "a deps: array read at module-eval time references a namespace imported from a cycle partner", I scanned for other occurrences: build the runtime import graph (excluding import type), find modules whose top-level deps: [...] dereferences X.node, and flag those where X is mutually reachable with the module.

Across 669 modules under packages/core/src and packages/opencode/src, that reports exactly one hazard — filesystem.tsfilesystem/search.ts — and zero once this patch is applied. So the targeted fix looks complete rather than one instance of a wider pattern.

Possible lighter-weight regression test

The bundle probe above needs no provider, credentials, network, or session, and runs in about a second. It may be worth considering alongside (or instead of) the full compiled runner, since it asserts the actual invariant — no undefined in a compiled layer graph — and would catch any future reintroduction of this cycle anywhere in the graph, not just this module pair.

A related thought for a separate change: LayerNode.compile could validate dependencies and throw undefined dependency in node <name> instead of dereferencing .name. That would have turned this into a one-line diagnosis rather than a silent Unexpected server error. Happy to open that separately if it would be useful.

Impact

Worth noting this is a full outage on the affected build — every prompt fails before any provider request, and the surfaced error is a generic Unexpected server error, so it reads as an auth or provider problem. Several people in #48372 understandably suspected their credentials first. Pinning to 1.18.29 is an effective workaround in the meantime.

@kernel-oops

Copy link
Copy Markdown
Author

Thanks @vidit19sharma — this independent reproduction and before/after validation materially close the macOS arm64 validation gap. The direct graph observation is particularly useful: it matches the compiled-dependency invariant this fix protects — every dependency entry must be a defined node. The source/bundled difference also explains the misleading symptom: source execution catches the premature access with a temporal-dead-zone ReferenceError, whereas the bundled evaluation captures an undefined dependency that only fails later at node.name.

The lighter bundle probe is a strong candidate to complement the full compiled runner: it checks the graph invariant directly and cheaply, while the runner retains end-to-end coverage of the compiled prompt/tool path. Thanks also for your scan across 669 modules finding only this hazard and none after the patch; that is useful additional evidence, though I have not independently reproduced the scan.

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.

bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request Sep 14, 2026
https://build.opensuse.org/request/show/1377875
by user pluskalm + anag_factory
opencode 1.18.30: two fixes, supersedes 1377865 (same patch, one more change).
- Every prompt failed with "undefined is not an object (evaluating 'a.name')" (boo#1280159): circular import between core/src/filesystem.ts and filesystem/search.ts, which bun 1.4.2's bundler orders so a layer dependency is undefined. Backport of upstream PR anomalyco/opencode#48397 (search.ts only). Reproduced on the Factory:ARM RPM in a Tumbleweed container; the patched build answers the same prompt.
- NO_BRP_STRIP_DEBUG=true in %install: brp-15-strip-debug runs binutils strip (not %__strip) on any ELF `file` calls "not stripped", which the compiled binary is whenever the bun it was copied from kept its .symtab (any project without the debuginfo flag); that drops the appended payload and %check prints bun's ver
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SystemPrompt.environment

2 participants