Skip to content

fix: stop delegation cleanup from leaking unhandled rejections - #18

Open
Tyagiquamar wants to merge 1 commit into
openprose:mainfrom
Tyagiquamar:fix/delegation-unhandled-rejection
Open

fix: stop delegation cleanup from leaking unhandled rejections#18
Tyagiquamar wants to merge 1 commit into
openprose:mainfrom
Tyagiquamar:fix/delegation-unhandled-rejection

Conversation

@Tyagiquamar

Copy link
Copy Markdown

Problem

pressFn tracks spawned delegations in pendingRlmCalls using:

promise.finally(() => pendingRlmCalls.delete(promise));

Promise.prototype.finally() returns a new promise that rejects with the same reason when the original rejects. Nothing ever handles that derived promise, so every failed delegation leaks an unhandledRejection even when the caller handled the error correctly.

Two concrete consequences:

  1. Phantom errors in the parent loop. While a parent exec is in flight, the engine registers a temporary process.on("unhandledRejection") handler (see JsEnvironment.exec) to surface stray async failures to the model. The leaked rejection from the cleanup chain gets captured there, so sandbox code that correctly recovers via try { await press(...) } catch { ... } still sees the error appended to its iteration output — teaching the model that its error handling failed when it didn't.
  2. For unawaited fire-and-forget delegations that reject after the exec window closes, the rejection reaches Node's default handler (process exit on Node ≥ 15).

Provenance: found by code inspection while reading the delegation lifecycle; no matching open issue at the time of writing. Reproduced with a regression test.

Fix

Attach both fulfillment and rejection handlers so no dangling promise is created; the original promise's handling semantics are unchanged:

const removeFromPending = () => pendingRlmCalls.delete(promise);
promise.then(removeFromPending, removeFromPending);

The .then(...) result always fulfills, so nothing rejects unhandled.

Testing

Added caught delegation failure does not surface as a phantom error in test/rlm.test.ts: a child whose LLM driver throws is awaited inside a sandbox try/catch; asserts the run completes and that root-iteration events carry no error.

  • Pre-fix: fails with expected 'child boom' to be null (the caught child error leaked into the parent's iteration output).
  • Post-fix: passes.
  • npx vitest --run test/rlm.test.ts — 56/57 pass on Windows; the single failure (auto-resolve file paths > resolves .md file paths...) is pre-existing on main and is addressed by fix: resolve Windows-style backslash paths in input resolution #17.
  • npm run build — clean compile.

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.

1 participant