fix(signals): optimistic settle verdicts — combined isPending on the child lane, onSettled after the revert re-derive (#3409, #3411)

.changeset/fix-optimistic-settle-verdicts.md

@@ -0,0 +1,8 @@
+---
+"@solidjs/signals": patch
+---
+
+Optimistic settle verdicts (#3409, #3411).
+
+- `isPending(() => [a(), b()])` over two async siblings of an optimistic value reports pending as soon as either read is (#3409). `assignOrMergeLane` now follows a merged lane to its root and runs the parent/child check on it; the old "merged lane is stale, take the source lane" shortcut moved the combined probe's effect onto the held parent lane, where its verdict waited on the async it reports.
+- An unowned `onSettled` callback (event handler, action body) reads a settled world (#3411). The settle only enqueues the reverted subscribers for the next pass, so a callback fired in the commit pass saw the optimistic source reverted beside a sync memo of it still holding the optimistic value. The fire now waits for the heap to drain.

packages/signals/docs/INTERNALS-ASYNC-STATE.md

@@ -83,6 +83,7 @@ Semantics of the `(_pendingValue, _overrideValue)` pair for an optimistic node
 
 - One lane per optimistic _source signal_ (`signalLanes` WeakMap), reused across writes to the same signal. Union-find merging (`_mergedInto`); merges move `_pendingAsync` and effect queues into the root.
 - `_parentLane`: companion nodes (`_pendingSignal`/`_latestValueComputed`) get _child_ lanes that intentionally do **not** merge with the parent (`assignOrMergeLane` parent/child carve-out) so `isPending` effects can flush before the parent's async settles. The parent is read from the owner's `_optimisticLane` when the companion lane is created (`getOrCreateLane`), so every path that assigns a node's lane and pokes its companions must assign first: `notifyStatus` does (#3379 — a memo made pending by propagation before it rode the lane got a parentless companion lane, which the indicator's other dep merged into the held lane; `isPending` then waited on the async it reports).
+- A subscriber whose lane was merged is followed to its root (`findLane`) like any other and goes through the same parent/child check (#3409). The earlier shortcut — a merged `_optimisticLane` is stale, take the source lane — skipped that check: an `isPending(() => [a(), b()])` reader of two async siblings of one optimistic value got the two companion child lanes (siblings, so they merge), and the parent lane's next notification then moved it onto the held parent, where its verdict waited on the async it reports while `isPending(a)` and `isPending(b)`, each on one unmerged child lane, flipped at once.
 - Lane lifecycle: created on optimistic write → nodes join via `insertSubs(node, true)` → `assignOrMergeLane` → lane-routed effects run when the lane is not held (`runLaneEffects` → `laneHeld`) → cleaned up by `cleanupCompletedLanes` when the owning transition completes (or when orphaned, `_transition === null`).
 - `_pendingAsync` add/delete sites: added in `recompute`'s async catch under a lane (core.ts ~264), removed on async resolution (`asyncWrite`, async.ts ~214) and on lane-corrected recompute (core.ts ~254). The set records the async the lane _owns_, not what holds it.
 - Replay gating (#3330): `laneReadsCommitted` hands a lane reader the committed `_value` of a staged node and records the reader in the batch's `_gatedSubs` for a re-run at commit — only when `_pendingValue !== _value`. A lane recompute that already published the value (INV-11) leaves the two equal; recording the reader anyway replayed its effects against an unchanged frame.
@@ -99,6 +100,7 @@ Semantics of the `(_pendingValue, _overrideValue)` pair for an optimistic node
 - `_optimisticNodes` — nodes whose override reverts at completion (`resolveOptimisticNodes`).
 - Incomplete-transition flush stashes queues (`stashQueues`) and continues with a fresh view; completion restores them, commits pending, reverts optimistic, replays `_gatedSubs`, cleans lanes.
 - `_contested` — effects whose single value slot was written under this transaction and then overwritten by another live transaction or by mainline (#3322). Effects are not shared state, so a shared effect never merges transactions (memos do, via their `_transition` stamp); instead `Effect._valueTransition` records which view produced `_value`, `recompute`, when that owner changes, registers the effect on every owed live transaction, and `finalizePureQueue` re-dirties them **before** its heap run so the re-derive and the effect phase land in the same pass — the other view's value is never published. Exception: a settle that reverts optimism (a non-empty `_optimisticNodes`) re-dirties them **after** `_resolveOptimistic`, with the gated replay — between `commitPendingNodes` and the revert the truth is committed but the overrides still display, and a re-derive there composes the two (the #3164 tear; the reveal wake sits post-revert for the same reason). The slot meanwhile holds the frame already on screen, so nothing new is published early. Rules that fall out: a stale (render) reader with no transaction active is mainline and sees a foreign transaction's staged signal as committed (`read`'s fast path and `readNodeFast` apply `stale && el._transition !== null`, matching the slow path); a value computed mainline needs no protection (mainline publishes what it computes, and a transaction whose writes never touched the effect finds it still correct at commit).
+- `onSettled` and the revert re-derive (#3411): the settle drops the overrides in the commit pass and only _enqueues_ their subscribers (the revert's `insertSubs`, the contested and gated replays, the store clears); the pass after re-derives them, and reads do not pull (`prepareComputed(el, false)`). An unowned `onSettled` callback is a one-shot in the commit pass's user phase, so it read the optimistic source already reverted next to a sync memo of it still holding the optimistic value. The fire waits for the heap to drain instead — while `dirtyQueue` has work it re-enqueues itself (`run` swaps the queue, so the re-enqueue lands in the next pass; `enqueue` keeps the drain alive) — which is what settled means: no derivation outstanding. Forcing the re-derive into the commit pass is not an option: an optimistic write completing in its own flush shows its lane frame (applied by `cleanupCompletedLanes`) before the truth, and the store clears compose a torn shape when their readers re-derive inside finalize (#3164).
 - Finalize re-entry (#3319): `finalizePureQueue` can _enter_ a held transaction partway through — a store commit hook (`bumpDeep` on a node the transaction owns), a boundary `_checkSources` write, or a stamped recompute in its heap — and `initTransition` then adopts the batch being finalized. Two rules keep that consistent. **State:** finalize captures the batch it started with and, if `currentBatch` changed, commits/reverts nothing batch-derived (the entered transaction owns it now); a _completing_ transaction whose ambient batch was separate (the #2916 shape) still settles its own containers, since adoption never touched them. **Effects:** ownership. A run applies with the commit of the transaction that computed its value (`Effect._valueTransition`). The ordinary effect phase runs with `activeTransition` set only in a flush whose finalize entered one, so `runEffect` leaves runs owned by a still-held transaction queued — `_modified` stays set — for the next gate to stash with the owner, while everything computed mainline (the write that caused the flush) applies now. Lanes are exempt by construction: they apply their own effects ahead of their transaction (the optimistic view) and their runner ORs `LANE_RUN` into the `type` it passes; the creation-time immediate run in `effect()` passes it too. The exemption is keyed on the _effect_ still having a lane, not on the runner: after a supersession demotes the cascade (#3331, §1), a `LANE_RUN` runner reaching a now lane-less effect whose value was computed under a still-held transaction leaves it queued like any owned run — otherwise the lane would apply the corrected derivation ahead of the commit that is supposed to reveal it. Known residue: writes staged by a hook _before_ the entry are adopted (held) and, because finalize's heap runs after its hooks, their dependents recompute owner-stamped and park with them; an entry that happens _inside_ that heap can leave an earlier mainline-computed effect applied over an adopted source — narrow, and inherited from adoption rather than from this rule.
 - `transitionComplete`: prunes dead reporters (`reporterBlocksSource`), transition is done when no live reporter still blocks a source whose **own flight is up** and no active-override node is blocked on someone else's async. "Own flight is up" is the source's self entry in its `_pendingSources` (#3375) — not `_error.source`, which a later-pending input overwrites on propagation while the flight is still in the air.
 - Fallback-caught async holds nothing — in both orders (ruled 2026-09-12, #3375). A collecting boundary consumes the notification, so a reader under a fallback never registers. A reader registered while its boundary showed content (forwarded) stays registered when the boundary's `on` changes and it flips to the fallback; `reporterBlocksSource` therefore walks the reporter's `_queue._parent` chain and treats a reporter behind a collecting pending-type boundary (`_collectionType & STATUS_PENDING && !_initialized`) as not live. If nothing outside the boundary consumes the flight, the hold is over; a reader outside it still holds. The reset itself calls `wakeParked()` so the re-judgement happens in the same drain.

packages/signals/src/core/lanes.ts

@@ -178,14 +178,12 @@ export function assignOrMergeLane(
   const sourceRoot = findLane(sourceLane);
   const existing = el._x?._optimisticLane;
   if (existing) {
-    // If the subscriber's lane was merged into another lane, it's stale —
-    // replace it with the new source lane instead of following the merge chain
-    // (which would incorrectly merge the new lane into the old group)
-    if (existing._mergedInto) {
-      ext(el)._optimisticLane = sourceLane;
-      (el as any)._config |= CONFIG_HAS_LANE;
-      return;
-    }
+    // A merged lane is followed to its root like any other: the root is where
+    // the subscriber's affinity lives now. Replacing it with the source lane
+    // outright (as this once did) skipped the parent/child check below — an
+    // isPending reader of two async siblings had their two companion lanes
+    // merge, and the next parent-lane notification then moved it onto the
+    // held parent, where its verdict waited on the async it reports (#3409).
     const existingRoot = findLane(existing);
     if (activeLanes.has(existingRoot)) {
       if (existingRoot !== sourceRoot && !hasActiveOverride(el)) {

packages/signals/src/signals.ts

@@ -32,6 +32,7 @@ import { emitDiagnostic, registerGraph, reportDiagnostic } from "./core/dev.js";
 import { installOptimisticEngine } from "./core/optimistic.js";
 import {
   activeTransition,
+  dirtyQueue,
   entangleConfirmingTransitions,
   globalQueue,
   Queue
@@ -1179,7 +1180,15 @@ export function onSettled(callback: () => void | (() => void)): void {
   const owner = getOwner();
   owner && !(owner._config & CONFIG_CHILDREN_FORBIDDEN)
     ? trackedEffect(() => untrack(callback), __OBSERVE__ ? { name: "onSettled" } : undefined)
-    : globalQueue.enqueue(EFFECT_USER, () => {
+    : globalQueue.enqueue(EFFECT_USER, function fire() {
+        // Settled means derived. A settle that reverts optimism (or replays
+        // gated reads) only enqueues the affected subscribers; the pass after
+        // the commit re-derives them. Fired in the commit pass, the callback
+        // read the optimistic source already reverted beside a sync memo of it
+        // still holding the optimistic value — reads do not pull (#3411). Fall
+        // to the next pass while the heap has work; `run` swapped the queue,
+        // so this lands there, and `enqueue` keeps the drain alive.
+        if (dirtyQueue._max >= dirtyQueue._min) return globalQueue.enqueue(EFFECT_USER, fire);
         // Unowned, out-of-band fire (no owner, or a children-forbidden one this
         // one-shot must not bind to): a returned cleanup has no lifecycle to
         // attach to. Reject it in dev; in production the return is simply

packages/signals/tests/optimistic-settle-verdicts.test.ts

@@ -0,0 +1,160 @@
+import { describe, expect, it } from "vitest";
+import {
+  action,
+  createMemo,
+  createOptimistic,
+  createRenderEffect,
+  createRoot,
+  createSignal,
+  flush,
+  isPending,
+  onSettled
+} from "../src/index.js";
+
+// Manual clock (see async-chain-supersession.test.ts).
+let now = 0;
+let timers: { at: number; run: () => void }[] = [];
+function delay<T>(ms: number, value?: T): Promise<T> {
+  return new Promise<T>(r => timers.push({ at: now + ms, run: () => r(value as T) }));
+}
+async function settle() {
+  for (let r = 0; r < 3; r++) {
+    for (let i = 0; i < 10; i++) await Promise.resolve();
+    flush();
+  }
+}
+async function advanceTo(t: number) {
+  while (true) {
+    timers.sort((a, b) => a.at - b.at);
+    const next = timers[0];
+    if (!next || next.at > t) break;
+    timers.shift();
+    now = next.at;
+    next.run();
+    await settle();
+  }
+  now = t;
+  await settle();
+}
+function reset() {
+  now = 0;
+  timers = [];
+}
+function frames(log: string[], when: number[]): string[] {
+  const byTime = new Map<number, string[]>();
+  log.forEach((v, i) => (byTime.get(when[i]) ?? byTime.set(when[i], []).get(when[i])!).push(v));
+  return [...byTime].map(([t, vs]) => `${t}: ${vs.sort().join(" | ")}`);
+}
+function text(fn: () => string, log: string[], when: number[]) {
+  let last: string | undefined;
+  createRenderEffect(fn, v => {
+    if (v !== last) {
+      last = v;
+      log.push(v);
+      when.push(now);
+    }
+  });
+}
+
+describe("onSettled during an optimistic revert", () => {
+  // The settle drops the override in the commit pass and only enqueues the
+  // subscribers; the pass after re-derives them. An unowned onSettled fired in
+  // the commit pass read `value` reverted beside `copy` still holding 1 (reads
+  // do not pull). The fire now waits for the heap to drain.
+  it("#3411 reads the optimistic source and a sync memo of it consistently", async () => {
+    reset();
+    const log: string[] = [];
+    const when: number[] = [];
+    let run!: () => Promise<void>;
+    createRoot(() => {
+      const [value, setValue] = createOptimistic(0);
+      const [snapshot, setSnapshot] = createSignal("not called");
+      const copy = createMemo(() => value());
+      run = action(function* () {
+        setValue(1);
+        onSettled(() => {
+          setSnapshot(`Value: ${value()}, Copy: ${copy()}`);
+        });
+        yield delay(1000);
+      });
+      text(() => `Value: ${value()}`, log, when);
+      text(() => `Copy: ${copy()}`, log, when);
+      text(() => `Snap: ${snapshot()}`, log, when);
+    });
+    flush();
+    await settle();
+    run();
+    await settle();
+    await advanceTo(3000);
+    expect(frames(log, when)).toEqual([
+      "0: Copy: 0 | Copy: 1 | Snap: not called | Value: 0 | Value: 1",
+      "1000: Copy: 0 | Snap: Value: 0, Copy: 0 | Value: 0"
+    ]);
+  });
+
+  // Same tear without an action: the ambient optimistic write reverts at the
+  // end of its own flush and re-derives on the next pass.
+  it("#3411 also without an action", () => {
+    reset();
+    const log: string[] = [];
+    const when: number[] = [];
+    let write!: () => void;
+    createRoot(() => {
+      const [value, setValue] = createOptimistic(0);
+      const [snapshot, setSnapshot] = createSignal("not called");
+      const copy = createMemo(() => value());
+      write = () => {
+        setValue(1);
+        onSettled(() => {
+          setSnapshot(`Value: ${value()}, Copy: ${copy()}`);
+        });
+      };
+      text(() => `Value: ${value()}`, log, when);
+      text(() => `Copy: ${copy()}`, log, when);
+      text(() => `Snap: ${snapshot()}`, log, when);
+    });
+    flush();
+    write();
+    flush();
+    expect(log.at(-1)).toBe("Snap: Value: 0, Copy: 0");
+  });
+});
+
+describe("isPending() over several async siblings of an optimistic value", () => {
+  it("#3409 a combined probe reports pending while either read is pending", async () => {
+    reset();
+    const log: string[] = [];
+    const when: number[] = [];
+    let run!: () => Promise<void>;
+    createRoot(() => {
+      const [value, setValue] = createOptimistic(0);
+      const a = createMemo(() => delay(1000, value()));
+      const b = createMemo(() => delay(2000, value()));
+      run = action(function* () {
+        setValue(1);
+        yield delay(3000);
+      });
+      text(() => `A: ${isPending(a)}`, log, when);
+      text(() => `B: ${isPending(b)}`, log, when);
+      text(() => `Combined: ${isPending(() => [a(), b()])}`, log, when);
+    });
+    flush();
+    await settle();
+    await advanceTo(3000);
+    run();
+    await settle();
+    await advanceTo(9000);
+    // Optimistic phase (3000–5000): every indicator flips with its own
+    // reads. The revert (6000) reloads the truth as an ordinary held
+    // transaction, so the indicators clear together when it commits (8000),
+    // exactly as they do for a plain signal write.
+    expect(frames(log, when)).toEqual([
+      "2000: A: false | B: false | Combined: false",
+      "3000: A: true | B: true | Combined: true",
+      "4000: A: false",
+      "5000: B: false | Combined: false",
+      "6000: A: true | B: true | Combined: true",
+      "8000: A: false | B: false | Combined: false"
+    ]);
+  });
+});

packages/signals/tests/treeshake.test.ts

@@ -258,6 +258,10 @@ describe("pay-for-use tree-shaking (#2883)", () => {
     // transaction-owned node's committed children previously died on the
     // spot — and a contested effect's mainline pass releases its zombies
     // itself. +102 B (22,969 → 23,071).
+    // Settle verdicts (#3409, #3411): `assignOrMergeLane` follows a merged
+    // lane to its root like any other (the stale-lane shortcut that skipped
+    // the parent/child check is gone), -33 B (23,091 → 23,058); the
+    // unowned `onSettled` fire's heap-drain wait shakes out with onSettled.
     expect(minifiedBytes).toBeLessThan(23_150);
   });