test: add cleanup regression test for 4.0.1

[taras]

This PR was generated by Opus 4.5. I do not expect this PR to be merged. I just wanted to capture the context that accumulated while troubleshooting the @effectionx/websocket extension.

Summary

This PR adds a test that reproduces a regression introduced in 4.0.1 and includes the fix.

Test Results

Version	Result
v4.0.0	✅ PASS
v4.0.1 (before fix)	❌ FAIL (cleanup deadlocks)
This PR	✅ PASS

The Pattern

yield* resource(function* (provide) {
  let { resolve: closeStream, operation: streamClosed } = withResolvers<void>();
  let { resolve: confirmClosed, operation: closed } = withResolvers<void>();

  // Spawned consumer task
  yield* spawn(function* () {
    yield* streamClosed;  // Wait for signal from finally block
    confirmClosed();      // Signal back that we processed it
  });

  try {
    yield* provide(undefined);
  } finally {
    closeStream();        // Signal the spawned task
    yield* closed;        // Wait for spawned task to confirm
  }
});

This pattern:

1. Resource spawns a consumer task that waits on a signal
2. Finally block resolves the signal to notify consumer
3. Finally block waits for consumer to confirm it processed the signal

Works in 4.0.0, deadlocks in 4.0.1

Real-World Impact

This pattern is used by `@effectionx/websocket`'s `useWebSocket`, where:

• A spawned task consumes the WebSocket message stream
• Finally block calls `socket.close()` (triggering close event)
• Finally block waits for the consumer to see the close event

See: thefrontside/effectionx#134

Root Cause

The regression was introduced by PRs #1081 and #1085 which changed the task finalization order. In 4.0.1:

• `destroy()` runs destructors in reverse order
• Child task destructors (added via spawn) run BEFORE the parent's delimiter closes
• So by the time the parent's finally block runs, children are already destroyed

Before (4.0.0):

1. Parent's `top.close()` runs (triggers finally block)
2. Finally block signals children, children respond
3. Children get destroyed

After (4.0.1, broken):

1. Children get destroyed first (they were added to destructors last → run first in reverse)
2. Parent's `top.close()` runs
3. Finally block tries to signal children → deadlock (children are gone)

The Fix

Added a `setFinalizer()` mechanism to `ScopeInternal` that runs BEFORE regular destructors during `destroy()`.

Changes

lib/scope-internal.ts:

• Added `finalizer` variable and `setFinalizer()` method
• `destroy()` now runs the finalizer first, then regular destructors

lib/task.ts:

• Changed from `scope.ensure()` to `scope.setFinalizer()` for delimiter closing

Why This Works

The finalizer ensures the task's own delimiter closes (running its finally block) before any child destructors run. This restores the 4.0.0 behavior where:

1. Parent's finally block executes while children are still alive
2. Finally block can communicate with children
3. Children are destroyed after the parent's finally block completes

Diff

// scope-internal.ts
+  let finalizer: (() => Operation<void>) | undefined = undefined;

   // In scope object:
+  setFinalizer(op: () => Operation<void>): void {
+    finalizer = op;
+  },

   // In destroy():
+  // Run the finalizer first (e.g., close the task's delimiter).
+  if (finalizer) {
+    try {
+      yield* finalizer();
+    } catch (error) {
+      outcome = Err(error as Error);
+    }
+    finalizer = undefined;
+  }
+  // Then run regular destructors in reverse order (children first)

// task.ts
-  scope.ensure(function* () {
+  scope.setFinalizer(function* () {
     try {
       yield* top.close();
     } finally {

Expected Behavior

When a task is halted, spawned tasks within that scope should still be able to:

1. Receive signals from the parent's finally block
2. Complete work in response to those signals
3. Signal back to the parent before being torn down

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/thefrontside/effection@1090

commit: 0b37b5f

[taras]

Analysis of the Root Cause

After investigating the commits between 4.0.0 and 4.0.1, I identified the regression was introduced by:

• PR 🐛 fix: destroy task scope when halting to prevent memory leak #1081 (153e4200): Changed task.halt() to delegate to scope.destroy() instead of directly closing the delimiter
• PR 🐛 fix: move all task finalization into the scope destructor #1085 (dd1c9cb0): Moved task finalization logic from the coroutine's finally block into scope.ensure()

The Problem

The key change is in the order of operations during cleanup:

Before (4.0.0):

1. task.halt() → closes the delimiter → task's finally block runs
2. Spawned tasks are still alive at this point
3. Parent's finally block can signal spawned tasks and wait for response
4. Then spawned tasks are cleaned up

After (4.0.1):

1. task.halt() → scope.destroy()
2. destroy() runs all destructors in reverse order of registration
3. Spawned task destructors run FIRST (they were registered after the parent's top.close())
4. Spawned tasks are already gone by the time parent's finally block tries to signal them
5. Parent's finally block runs but can't communicate with children → deadlock

Code Flow

In lib/task.ts, the current structure is:

scope.ensure(function* () {
  try {
    yield* top.close();  // Runs parent's finally block
  } finally {
    // ... resolve/reject future
  }
});

let routine = createCoroutine({
  scope,
  *operation() {
    try {
      yield* top;
    } finally {
      yield* destroy();  // Destroys spawned tasks, then runs ensure callbacks
    }
  },
});

When destroy() is called, it runs destructors in reverse order. Since spawned tasks register their cleanup after the parent's scope.ensure(), the spawned task cleanup runs before top.close().

---

Proposed Fix

The fix needs to ensure that parent's finally block runs while spawned tasks are still alive.

Option 1: Run top.close() before scope destruction (Recommended)

Change destroy() in lib/scope-internal.ts to have the task explicitly close its delimiter before running other destructors. Or, restructure the task to not use scope.ensure() for top.close():

// In lib/task.ts - don't register top.close() via ensure
// Instead, have the coroutine's finally block handle it:

let routine = createCoroutine({
  scope,
  *operation() {
    try {
      yield* top;
    } finally {
      // Close delimiter FIRST (parent's finally block runs, children still alive)
      yield* top.close();
      // THEN destroy scope (clean up spawned tasks)
      yield* destroy();
    }
  },
});

// And move the future resolution to a separate ensure callback
scope.ensure(function* () {
  group.delete(task);
  let { outcome } = top;
  if (outcome!.exists) {
    let result = outcome!.value;
    if (result.ok) {
      future.resolve(result.value);
    } else {
      let { error } = result;
      future.reject(error);
      boundary.raise(error);
    }
  } else {
    future.reject(new Error("halted"));
  }
});

Option 2: Two-phase destruction

Introduce two phases in scope destruction:

1. Phase 1: Signal all tasks to close (run their finally blocks)
2. Phase 2: Destroy scopes and clean up resources

This would allow parent→child signaling to complete before teardown.

Option 3: Priority-based destructors

Allow destructors to have priorities so top.close() runs with higher priority (later in reverse order) than child cleanup.

---

Recommendation

Option 1 is the simplest and most backwards-compatible. It essentially restores the 4.0.0 behavior where the parent's finally block runs before spawned tasks are destroyed, while still preserving the memory leak fix from PR #1081 (ensuring scope.destroy() is called).

The key insight is that top.close() should NOT be registered via scope.ensure() because it needs to run BEFORE child destructors, not after them in reverse registration order.

Edge Cases to Verify

1. Normal completion: Task completes → finally block runs → top.close() → destroy() ✓
2. Task.halt(): Task is halted → destroy() interrupts coroutine → finally block runs → top.close() → destroy() (re-entrant, no-op) ✓
3. Error in task: Task throws → finally block runs → top.close() → destroy() ✓
4. Memory leak prevention: Scope is destroyed on halt (PR 🐛 fix: destroy task scope when halting to prevent memory leak #1081's goal) ✓

[taras]

Refined Fix Proposal (Preserving Memory Leak Fix)

After further consideration, the fix must preserve the memory leak fix from PR #1081 where task.halt() must call scope.destroy().

The Core Problem

The issue is the order of operations inside destroy(), not that destroy() is called. Currently:

destroy() → runs destructors in reverse registration order
         → spawned task destructors run first (registered later)
         → top.close() runs last (registered earlier)
         → parent's finally block runs but children are already gone

We need:

destroy() → top.close() runs first (while children alive)
         → parent's finally block can signal children
         → children respond
         → then spawned task destructors run (child cleanup)

Recommended Fix: Explicit delimiter close in destroy()

In lib/scope-internal.ts, modify destroy() to explicitly close the scope's delimiter before running other destructors:

function* destroy(): Operation<void> {
  if (destruction) {
    return yield* destruction.operation;
  }
  destruction = withResolvers<void>();
  parent?.expect(Children).delete(scope);
  unbind();
  
  // NEW: Close this scope's delimiter first (runs finally block while children alive)
  let delimiter = scope.get(DelimiterContext);
  if (delimiter && !delimiter.finalized) {
    yield* delimiter.close();
  }
  
  // Then run destructors (child cleanup) - existing logic
  let outcome = Ok();
  try {
    for (let destructor of [...destructors].reverse()) {
      try {
        destructors.delete(destructor);
        yield* destructor();
      } catch (error) {
        outcome = Err(error as Error);
      }
    }
  } finally {
    if (outcome.ok) {
      destruction.resolve();
    } else {
      destruction.reject(outcome.error);
    }
  }

  unbox(outcome);
}

And in lib/task.ts, remove top.close() from scope.ensure() since it's now handled by destroy():

// Remove this:
scope.ensure(function* () {
  try {
    yield* top.close();  // <-- Remove this, now handled in destroy()
  } finally {
    // ... future resolution stays
  }
});

// Keep just the future resolution:
scope.ensure(function* () {
  group.delete(task);
  let { outcome } = top;
  if (outcome!.exists) {
    let result = outcome!.value;
    if (result.ok) {
      future.resolve(result.value);
    } else {
      let { error } = result;
      future.reject(error);
      boundary.raise(error);
    }
  } else {
    future.reject(new Error("halted"));
  }
});

Why This Preserves the Memory Leak Fix

1. task.halt() still calls destroy() - the entry point from PR 🐛 fix: destroy task scope when halting to prevent memory leak #1081 is unchanged
2. scope.destroy() is still called - ensuring all resources are cleaned up
3. All destructors still run - no resources leak

Why This Fixes the Regression

1. destroy() explicitly closes the delimiter first
2. Parent's finally block runs while spawned tasks are still alive
3. Spawned tasks can respond to signals
4. Then spawned task destructors run (child cleanup)

Edge Cases

Scenario	Behavior
Normal completion	Coroutine ends → destroy() → delimiter already finalized (no-op) → destructors run ✓
task.halt()	destroy() → delimiter closes (finally runs) → destructors run ✓
Error in task	Coroutine throws → destroy() → delimiter closes → destructors run ✓
Memory leak	destroy() still called → scope cleaned up ✓
Nested resources	Each scope's destroy() closes its delimiter before children ✓

Import Required

lib/scope-internal.ts will need to import DelimiterContext:

import { DelimiterContext } from "./delimiter.ts";

This creates a dependency from scope-internal to delimiter, but this seems acceptable since scopes and delimiters are already tightly coupled in the task system.