Skip to content

fix(engine): the filesystem/search module cycle — the #775 wedge root cause - #1716

Open
aarontrowbridge wants to merge 10 commits into
mainfrom
fix/engine-fs-search-cycle-775
Open

aarontrowbridge wants to merge 10 commits into
mainfrom
fix/engine-fs-search-cycle-775

Conversation

@aarontrowbridge

Copy link
Copy Markdown
Member

Closes #1709. Kills the wedge series behind the 2026-09-03/04 + 2026-10-03/04/05 hub incidents.

The root cause, end to end

FileSystem.node captured FileSystemSearch.node into its deps array at module init, while search.ts imported the FileSystem namespace back from ../filesystem for shapes it uses exclusively inside effect bodies. Under bun/esbuild module-cycle semantics that capture froze to undefined (TDZ in one entry order, silent undefined in the other — both reproduced minimally):

  • every per-directory location build crashed with an anonymous TypeError … 'a.name' inside the layer walk,
  • the builder caught and swallowed it (failed to load project references),
  • the half-built fresh location layer leaked (watchers, snapshot trackers, LLM machinery) on every session prompt,
  • at ~8+ parallel sessions the leak rate wedged the engine: RSS 2.5–3.6 GB in minutes, main thread spinning with GC threads idle, TCP-accepts/HTTP-silent — the watchdog-recovered outages all week.

The twice-per-boot ERROR pair (#1709) was the same crash at the boot-time location build.

The fix, three parts

  1. filesystem/shared.ts (new) — the shared shapes (schema package's Entry/Match/Submatch/FindInput + GlobInput/GrepInput moved from filesystem.ts). Both sides import from it; neither imports the other. The cycle is gone at the source.
  2. layer-node.ts (overlay twin) — the walk names the parent on an undefined dependency instead of crashing anonymously. The next module cycle cannot hide behind a swallowed TypeError. (The instrumented build that diagnosed this produced exactly this naming: undefined dependency under parent=@opencode/v2/FileSystem … deps=[…, UNDEFINED] — the line that located the culprit.)
  3. Regression tests in BOTH module-evaluation orders — the freeze depends on which side of the cycle evaluates first, so each order pins the other (4/4 pass).

Verification

  • bun test test/filesystem/cycle-order-* (materialized core) — 4 passed
  • engine_typecheck_gate.sh — clean (only Base-pin drift: manifest upstream_base_sha stale vs absorbed overlay #1229-tracked base-drift)
  • overlay_typecheck_gate.sh — OK across 7 packages
  • drift_gate.mjs — PASS (manifest refreshed, 865 files)
  • build:binary → a5ef305a, deployed live on the hub (via ops/hub-restart.sh stage/swap/restart): boot run logs ZERO ERROR/WARN where every prior boot logged the a.name pair; /config+/agent for the trigger directory 200 clean; harness/sessions verified through the tunnel.

Incident evidence (erlich)

~/.amico/server/fleet-watchdog/wedge-20261005-040014-wedge/ (smaps, gdb thread states, log tail with the named parent), the sourcemapped diagnostic build decoding #1709 to layer-node.ts:241, and the minimal both-orders bun repro.

Follow-ups

  • The 8+ parallel sessions load that exposed the leak rate is now just load — but the per-prompt caught-and-swallowed error pattern deserves a look generally: silent catches that leak half-built fresh layers are the wedge shape to audit.
  • live-test/harness-main (the in-fleet tree carrying feat(app): the harness switcher in the chat box (#1549) #1550 + this) will want this merged before its next redeploy.

The composer gets a fourth select-slot — the harness — riding the
solver-mode trio end to end:

- engine: GET/POST /amicode/harness (server/amicode/harness.ts). The id
  allowlist IS the extension-published registry view
  (harness-options.json) — the engine never hardcodes harness identity;
  POST validates against it, no-ops when already current, and writes
  {harness, status:"switching"} to the ops dir (same loopback guard +
  fixed value-free refusals as the solver-mode flip).
- extension: harness_switch.ts (the handshake reader/writer + the
  watcher, the solver watcher's poll+busy-latch discipline) and the
  registry layer (harness.ts): harnessMenu — the ONE serialization both
  fronts render — and decideHarnessSwitch — the ONE gate both consult
  (unknown/needs-setup/unentitled blocked with their reason; the
  harness.telaio read-side gate lives here, never as a spawn-time
  failure). The palette command now renders from the same menu and gates
  through the same decision; the watcher persists the setting, republishes
  the menu, restarts, and settles ready — a refused switch settles ready
  at the STILL-CURRENT harness, never a lie.
- composer: the harness select (disabled-with-reason entries carrying the
  disclosure "switching harnesses switches session history" at the point
  of switching), harnessDegraded dimming of agent/model/variant under a
  non-opencode harness (Harness Contract v1 accepts none), and the
  harness switch banner narrating switching → restarting → ready.

Under opencode, byte-for-byte unchanged when untouched: no published
registry → the control renders nothing.
…e composer control's seam (#1549)

Live-test finding on the reference fleet (PR #1550): the app fetches
GET /amicode/harness at its panel origin — the service — whose fork-parity
discipline 404s every unmatched /amicode/* path without proxying, making the
engine-owned route unreachable from the app in EVERY topology, local mode
included. The control rendered nothing, exactly per its own degradation
comment. One named exception: /amicode/harness GET+POST forwards to the
engine when bound (the route's owner — forwarding, not laundering); an
unbound engine or a stock engine keeps the honest failure and the control's
hide-on-absent degradation. Everything else under /amicode/* keeps the
discipline. Tests pin all three seams: the menu fetch, the switch POST, the
narrow-exception 404.
Live-test layout ruling from the reference fleet: agent → model → effort →
harness. The first-in-row placement displaced the trio's reading order and
read as a replacement of the agent select; the harness select now renders
immediately after the degraded wrapper — right of the effort/variant select,
outside the wrapper so it stays clickable under a non-opencode harness (it is
the way back). Manifest refreshed in the same commit (the discipline).
…r select (#1549)

Live-test finding: the two-line flex-col entry fought the menu item's own
inline content slot (span[data-slot=menu-v2-item-content]) and the texts
overlapped on first visual. Single-line entries with the disabled reason
inline + the disclosure in the title tooltip — the pattern the model and
variant selects already use.
… after a 2s grace (#1707)

A bare server.close() waits for every open keep-alive socket to END —
the hub frontdoor holds long-lived pooled backend sockets to the service
origin plus SSE fan-outs, so stops parked the unit in 'deactivating'
anywhere between 6 seconds and systemd's SIGKILL (three wedged hub
stops on 2026-10-04, each an outage window; hub-restart.sh blocks on
them). The engine kill was already bounded (SIGTERM -> 3s -> SIGKILL);
the service close was the unbounded leg.

stop() now: close() first (no new accepts), a 2s grace for in-flight
requests, then closeAllConnections() (Node >= 18.2) and resolve
REGARDLESS — teardown is bounded, never a race against the proxy's
socket lifecycle.

Verified live on the hub: the old runner's final stop took 90s
(systemd SIGKILL); the fixed runner ran three consecutive restarts at
6s / instant / instant with the frontdoor's pooled sockets present.
Regression test pins the contract: a held-open half-request socket
cannot stall stop() past the grace.
… cause

FileSystem.node captured FileSystemSearch.node into its deps array at
module init, while search.ts imported the FileSystem namespace back
from ../filesystem for shapes it uses exclusively inside effect bodies.
Under bun/esbuild module-cycle semantics the capture froze to undefined
(TDZ in one entry order, silent undefined in the other):

  layer walk -> resolve(undefined.name) -> TypeError
  -> caught by every per-directory location builder ('failed to load
     project references') and SWALLOWED
  -> the half-built fresh location layer (watchers, snapshot trackers,
     LLM machinery) leaked on every session prompt
  -> at ~8+ parallel sessions the leak rate wedged the engine: RSS to
     2.5-3.6GB in minutes, main thread spinning (GC threads idle),
     TCP accepts / HTTP silent — the 2026-09-03/04 + 2026-10-03/04/05
     wedge series, and the twice-per-boot ERROR noise (#1709).

The fix, three parts:

1. filesystem/shared.ts (new): the shared shapes (Entry/Match/Submatch/
   FindInput from the schema package + GlobInput/GrepInput moved from
   filesystem.ts). Both sides import from it; neither imports the other.
   The cycle is gone at the source.
2. layer-node.ts (overlay twin): the walk names the parent on an
   undefined dependency instead of crashing anonymously — the next
   module cycle can never hide behind a swallowed TypeError again.
   (The instrumented LAYER-DEBUG build that diagnosed this incident
   produced the parent naming that located the culprit.)
3. Regression tests in BOTH module-evaluation orders — the freeze
   depends on which side of the cycle evaluates first, so each order
   pins the other.

Evidence: incidents/wedge-20261005-040014-wedge on erlich (smaps, gdb,
log tail with the named parent), the #1709 boot errors decoding to
layer-node.ts:241 via the sourcemapped diagnostic build, and the
minimal bun repro importing the pair in both orders.
@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 625461d1-69d2-4bd8-9be3-22658ff4a5c8
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

aarontrowbridge added a commit that referenced this pull request Oct 5, 2026
…river

The module-cycle fix (PR #1716) killed the location-build leak, but the
04:20 EDT wedge recurred on the FIXED engine: a provider turn at 04:16
grew the heap ~10MB/s to 3.1GB with the main thread saturated, HTTP
silent, log silent from the moment the stream began — killed by the
watchdog 4 minutes later. Post-mortem: every streamed delta was pushed
into an UNBOUNDED fragment buffer (joined only at stream end), and the
request carries max_tokens only when generation config sets it — so a
model turn that never stops generating (a looping reasoning block, a
provider stall-loop) accumulates without limit and wedges the engine.
The DB proved it: no part over 10MB ever completed — the growth never
lands; it dies with the engine.

The guard rails, in the publisher's fragment accumulator:

- 32MB per fragment / 64MB per provider turn (4-8x the largest
  legitimate part ever recorded, 7.9MB) — a rail, not a limiter:
  crossing them means the stream is pathological.
- On crossing: the turn DIES with the honest named error
  ('stream overflow: ...') — the session sees a real failure, the
  engine survives, the watchdog stays a watchdog.
- Env-overridable (AMICODE_MAX_STREAM_FRAGMENT_BYTES /
  AMICODE_MAX_STREAM_MESSAGE_BYTES) for tests and diagnosis.

Follow-up tracked separately: a default max_tokens in the llm request
builder (provider-dependent, changes model behavior — deliberately NOT
bundled with this safety net).
aarontrowbridge added a commit that referenced this pull request Oct 5, 2026
…river

The module-cycle fix (PR #1716) killed the location-build leak, but the
04:20 EDT wedge recurred on the FIXED engine: a provider turn at 04:16
grew the heap ~10MB/s to 3.1GB with the main thread saturated, HTTP
silent, log silent from the moment the stream began — killed by the
watchdog 4 minutes later. Post-mortem: every streamed delta was pushed
into an UNBOUNDED fragment buffer (joined only at stream end), and the
request carries max_tokens only when generation config sets it — so a
model turn that never stops generating (a looping reasoning block, a
provider stall-loop) accumulates without limit and wedges the engine.
The DB proved it: no part over 10MB ever completed — the growth never
lands; it dies with the engine.

The guard rails, in the publisher's fragment accumulator:

- 32MB per fragment / 64MB per provider turn (4-8x the largest
  legitimate part ever recorded, 7.9MB) — a rail, not a limiter:
  crossing them means the stream is pathological.
- On crossing: the turn DIES with the honest named error
  ('stream overflow: ...') — the session sees a real failure, the
  engine survives, the watchdog stays a watchdog.
- Env-overridable (AMICODE_MAX_STREAM_FRAGMENT_BYTES /
  AMICODE_MAX_STREAM_MESSAGE_BYTES) for tests and diagnosis.

Follow-up tracked separately: a default max_tokens in the llm request
builder (provider-dependent, changes model behavior — deliberately NOT
bundled with this safety net).
@aarontrowbridge
aarontrowbridge force-pushed the fix/engine-fs-search-cycle-775 branch from d7e6954 to 9bf3efc Compare October 5, 2026 08:32
…river

The module-cycle fix (PR #1716) killed the location-build leak, but the
04:20 EDT wedge recurred on the FIXED engine: a provider turn at 04:16
grew the heap ~10MB/s to 3.1GB with the main thread saturated, HTTP
silent, log silent from the moment the stream began — killed by the
watchdog 4 minutes later. Post-mortem: every streamed delta was pushed
into an UNBOUNDED fragment buffer (joined only at stream end), and the
request carries max_tokens only when generation config sets it — so a
model turn that never stops generating (a looping reasoning block, a
provider stall-loop) accumulates without limit and wedges the engine.
The DB proved it: no part over 10MB ever completed — the growth never
lands; it dies with the engine.

The guard rails, in the publisher's fragment accumulator:

- 32MB per fragment / 64MB per provider turn (4-8x the largest
  legitimate part ever recorded, 7.9MB) — a rail, not a limiter:
  crossing them means the stream is pathological.
- On crossing: the turn DIES with the honest named error
  ('stream overflow: ...') — the session sees a real failure, the
  engine survives, the watchdog stays a watchdog.
- Env-overridable (AMICODE_MAX_STREAM_FRAGMENT_BYTES /
  AMICODE_MAX_STREAM_MESSAGE_BYTES) for tests and diagnosis.

Follow-up tracked separately: a default max_tokens in the llm request
builder (provider-dependent, changes model behavior — deliberately NOT
bundled with this safety net).
@aarontrowbridge
aarontrowbridge force-pushed the fix/engine-fs-search-cycle-775 branch from 9bf3efc to 2ea44c4 Compare October 5, 2026 08:34
@aarontrowbridge

Copy link
Copy Markdown
Member Author

Addendum (same night): the cycle fix killed the location-build leak, but the wedge recurred on the FIXED engine at 04:20 EDT — same signature (silence at stream start, heap ~10 MB/s to 3.1 GB, main thread saturated), zero guard hits, clean location builds. The surviving driver: the provider stream itself. Every streamed delta was pushed into an unbounded fragment buffer (joined only at stream end) and the request carries max_tokens only when generation config sets it — a model turn that never stops generating accumulates without limit and takes the engine down. The DB corroborates: no part over 10 MB has ever completed — the growth never lands anywhere; it dies with the engine.

The commit at the branch tip (now also live-test/harness-main) adds the guard rails to the publisher's fragment accumulator: 32 MB per fragment / 64 MB per turn (4–8× the largest legitimate part ever recorded, 7.9 MB — a rail, not a limiter), crossing them dies with an honest named stream overflow error — the session sees a real failure, the engine survives. Env-overridable + per-turn streamCaps for tests. stream-overflow.test.ts pins both caps; the full core suite (43 tests), engine + overlay typecheck gates, drift gate, all green. Built 70ca1345, deployed live on the hub (the tree's provenance branch pushed).

Follow-up worth its own PR: a default max_tokens in the llm request builder (provider-dependent, changes model behavior — deliberately not bundled with a safety net).

@aarontrowbridge
aarontrowbridge force-pushed the fix/engine-fs-search-cycle-775 branch 2 times, most recently from 67e9e78 to 81f1444 Compare October 5, 2026 09:05
…ge's surviving leg

The publisher-side stream caps (previous commit) never fired — the
wedges recurred on that engine with ZERO frames reaching the publisher.
The surviving leg: the response BODY. The SSE framer buffers by line —
an unterminated SSE line (or any never-terminating provider response)
accumulates without bound at receive speed. The observed wedges: heap
~10MB/s from the moment a turn's stream began, no event ever completed,
main thread saturated, HTTP silent, engine dead until the watchdog.
A plain status-check turn on the same session completed clean — the
pathology is provider-side and stochastic, which is exactly why the
bound must live at the seam every byte crosses.

The cap: 64MB per response (8x the largest legitimate turn ever
recorded), enforced in the http-json transport's frames() with a named
error — an endless body dies in ~6 seconds at observed rates instead of
wedging the engine for the watchdog. Env-overridable
(AMICODE_MAX_LLM_RESPONSE_BYTES). Tests pin both: the endless body dies
at the cap with the named error, a bounded SSE body passes untouched.
@aarontrowbridge
aarontrowbridge force-pushed the fix/engine-fs-search-cycle-775 branch from 81f1444 to 4783551 Compare October 5, 2026 09:06
@aarontrowbridge

Copy link
Copy Markdown
Member Author

Third leg (same night, live-tested): the response-side cap. The publisher caps never fired across two more wedges — zero frames ever reached the publisher. The wedge lives at the transport: the SSE framer buffers by line, so an unterminated SSE line (or any never-terminating response body) accumulates at receive speed with no event ever completing — heap ~10 MB/s from stream start, no deltas, HTTP silent. A status-check turn on the same session completed clean, so the pathology is provider-side and stochastic — which is exactly why the bound must sit at the seam every byte crosses.

010c5838 (deployed live): 64 MB per provider response (8× the largest legitimate turn ever recorded) enforced in the http-json transport's frames(), dying with the named llm response exceeded … error — an endless body dies in ~6 s at observed rates instead of wedging the engine. Env: AMICODE_MAX_LLM_RESPONSE_BYTES. Tests: endless unterminated body trips the cap with the named error; bounded SSE body passes untouched. Full suite green (45 engine tests), both typecheck gates, drift gate.

The three layers now bound the full path: response bytes → fragment accumulation → per-turn total. A runaway anywhere in the pipeline now costs one failed turn with an honest error, never the hub.

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.

Engine boot: TypeError 'a.name' on undefined, twice per boot, ERROR level — both 87bd-era and current-main builds

1 participant