refactor(signals): store node reads select by readerSeesCommitted (Rule 1, one implementation)

.changeset/shared-read-predicates.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/signals": patch
+---
+
+Consolidate value-selection predicates: `readerSeesCommitted` (the full committed-vs-staged arm read()'s slow tail used to inline) and `visibleOverride` / `hasActiveOverride` (one definition each, previously duplicated between the core, lanes, verdict channels and the store) — a zero-semantic-change refactor toward one implementation per rule (DESIGN-CONSOLIDATION, move 3b).

.changeset/store-node-rule1.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/signals": patch
+---
+
+Store node reads select committed-vs-staged by the core's `readerSeesCommitted` (one Rule 1 implementation for signals and store nodes). Fixes a render effect's untracked read of a store key held by a foreign action never replaying at that action's commit — the store's hand-restated stale-of-foreign clause served the committed value but skipped the replay registration the signal path performs, leaving the effect on the pre-action value permanently.

packages/signals/src/core/core.ts

@@ -1626,6 +1626,50 @@ export function enterStagedRead(
   globalQueue.initTransition(t);
 }
 
+/**
+ * Rule 1 (value selection), the full arm: does this reader see a STAGED
+ * node's COMMITTED value? One implementation of the rule the fast paths
+ * (readNodeFast, read's fast block) carry as their trivial ternary and that
+ * every slow site — read's tail, the store's backing selection, the lane and
+ * verdict arms — used to restate by hand (DESIGN-CONSOLIDATION, move 3b). In order:
+ * - no reader at all (an untracked read) — the committed frame;
+ * - a reader under an optimistic lane the engine says reads committed
+ *   (laneReadsCommitted: another lane's hold, #3460);
+ * - nothing staged;
+ * - a children-forbidden reader (createTrackedEffect / onSettled: the frame,
+ *   never the graph — A32);
+ * - a stale reader (render effect) of a FOREIGN transaction's staged write —
+ *   committed, no entanglement (heldFromStale registers the replay; a node
+ *   born held has no committed frame to fall back to, `noCommitted`);
+ * - A17 for HELD truth (#3164, CONFIG_HELD_TRUTH): staged confirming truth —
+ *   fold-staged onto an armed family, or entangle-stolen by an awaited
+ *   until() — is masked from ordinary readers until its transaction's
+ *   reveal, the retaining transaction's own speculative recomputes included
+ *   (partial override coverage would otherwise compose override + staged
+ *   truth into a state no timeline contains). Authoritative readers
+ *   (until()'s predicate) and latest() see the staged truth — the tunnel that
+ *   keeps the hold deadlock-free.
+ * False means the reader derives from the staged value and enters its
+ * transaction (enterStagedRead, A29).
+ */
+export function readerSeesCommitted(
+  el: Signal<any> | Computed<any>,
+  c: Computed<any> | null,
+  owner: Signal<any> | Computed<any>,
+  noCommitted: boolean
+): boolean {
+  return !!(
+    !c ||
+    (currentOptimisticLane !== null && GlobalQueue._laneReadsCommitted!(el, owner, c)) ||
+    el._pendingValue === NOT_PENDING ||
+    c._config & CONFIG_CHILDREN_FORBIDDEN ||
+    (stale && !noCommitted && heldFromStale(el, c)) ||
+    (el._config & CONFIG_HELD_TRUTH &&
+      !latestReadActive &&
+      !(c._config & CONFIG_AUTHORITATIVE_READ))
+  );
+}
+
 /** A28 — set when a node is staged (queuePendingNode) or a held node rewritten
  * (stashHeldRewrite) OUTSIDE a flush; cleared when the next flush begins. The
  * read sites test this one module boolean instead of `globalQueue._running`:
@@ -1680,6 +1724,22 @@ export function unflushedOverride(el: Signal<any> | Computed<any>): boolean {
   // Companions are optimistic signals written by the engine (see unflushed).
   return !globalQueue._running && el._x?._overrideTime === clock && !el._x?._parentSource;
 }
+/** Active optimistic override on an armed node (an armed slot idles at
+ * NOT_PENDING; undefined = unarmed plain node). The writer's own channels —
+ * the draft, `in`/keys inside the setter — compose on this regardless of
+ * flush state. */
+export function hasActiveOverride(el: Signal<any> | Computed<any>): boolean {
+  const x = el._x;
+  return x !== null && x._overrideValue !== undefined && x._overrideValue !== NOT_PENDING;
+}
+/** The override a READER sees: installed, and carried by a flush (A28 (5) —
+ * an optimistic write is a write; until its flush no reader sees it). One
+ * implementation for read()'s override arm, the verdict channels
+ * (latestRead, computePendingState) and the store's selection
+ * (DESIGN-CONSOLIDATION, move 3b). */
+export function visibleOverride(el: Signal<any> | Computed<any>): boolean {
+  return hasActiveOverride(el) && !unflushedOverride(el);
+}
 /** A derivation served the committed value because of an unflushed write
  * (A28) must run again in the flush that carries it — the late-linker case
  * (#3337's reason to defer the walk): it linked after the write walked. */
@@ -1928,7 +1988,7 @@ export function read<T>(el: Signal<T> | Computed<T>): T {
       nodeName: (owner as any)?._name
     });
 
-  if (el._x?._overrideValue !== undefined && el._x?._overrideValue !== NOT_PENDING) {
+  if (hasActiveOverride(el)) {
     // A17: the override IS the value for every reader — except an authoritative
     // reader (until()'s predicate carries CONFIG_AUTHORITATIVE_READ): it must
     // observe independently-arriving truth, and serving it the caller's own
@@ -1992,26 +2052,9 @@ export function read<T>(el: Signal<T> | Computed<T>): T {
     if (pendingCheckActive) GlobalQueue._recordFresh!(el, u);
     return u as T;
   }
-  const value =
-    !c ||
-    (currentOptimisticLane !== null &&
-      GlobalQueue._laneReadsCommitted!(el, owner, c as Computed<any>)) ||
-    el._pendingValue === NOT_PENDING ||
-    c._config & CONFIG_CHILDREN_FORBIDDEN ||
-    (stale && !noCommitted && heldFromStale(el, c as Computed<any>)) ||
-    // A17 for HELD truth (#3164, see CONFIG_HELD_TRUTH): staged confirming
-    // truth — fold-staged onto an armed family, or entangle-stolen by an
-    // awaited until() — is masked from ordinary readers until its
-    // transaction's reveal; the retaining transaction's own speculative
-    // recomputes included (partial override coverage would otherwise
-    // compose override + staged truth into a state no timeline contains).
-    // Authoritative readers (until()'s predicate) and latest() see the
-    // staged truth — the tunnel that keeps the hold deadlock-free.
-    (el._config & CONFIG_HELD_TRUTH &&
-      !latestReadActive &&
-      !((c as Computed<any>)._config & CONFIG_AUTHORITATIVE_READ))
-      ? el._value
-      : (enterStagedRead(el), el._pendingValue as T);
+  const value = readerSeesCommitted(el, c as Computed<any> | null, owner, noCommitted)
+    ? el._value
+    : (enterStagedRead(el), el._pendingValue as T);
   // Record that this isPending() probe observed the fresh pending value, so
   // the probe doesn't pair "pending" with the new value (#2831).
   if (pendingCheckActive) GlobalQueue._recordFresh!(el, value);

packages/signals/src/core/lanes.ts

@@ -4,7 +4,8 @@ import {
   NOT_PENDING,
   REACTIVE_DISPOSED
 } from "./constants.js";
-import { currentOptimisticLane, ext } from "./core.js";
+import { currentOptimisticLane, ext, hasActiveOverride } from "./core.js";
+export { hasActiveOverride };
 import { enqueueSub } from "./heap.js";
 import {
   activeTransition,
@@ -201,14 +202,6 @@ export function resolveTransition(el: Signal<any> | Computed<any>): Transition |
   return resolveLane(el)?._transition ?? el._transition;
 }
 
-/**
- * Check if a node has an active optimistic override.
- */
-export function hasActiveOverride(el: Signal<any> | Computed<any>): boolean {
-  const x = el._x;
-  return x !== null && x._overrideValue !== undefined && x._overrideValue !== NOT_PENDING;
-}
-
 /**
  * Assign or merge a lane onto a node. At convergence points (node already has
  * a different active lane), merge unless the node has an active override.

packages/signals/src/core/verdict.ts

@@ -37,7 +37,7 @@ import {
   setSignal,
   unflushed,
   unflushedCompanions,
-  unflushedOverride,
+  visibleOverride,
   unflushedValue,
   setStrictRead,
   stale,
@@ -292,8 +292,7 @@ function computePendingState(el: Signal<any> | Computed<any>): boolean {
   if (
     el._config & CONFIG_OVERRIDE_SUPERSEDED &&
     el._pendingValue === NOT_PENDING &&
-    hasActiveOverride(el) &&
-    !unflushedOverride(el)
+    visibleOverride(el)
   )
     return !el._equals || !el._equals(el._value as any, unwrapOverride(el._x?._overrideValue));
   // A28 (2): an unflushed write is not yet observable — the verdict answers
@@ -306,7 +305,7 @@ function computePendingState(el: Signal<any> | Computed<any>): boolean {
     // non-final"; an override is one (a node whose first landing was held
     // by a reveal it never got to commit, then superseded under its
     // override, read false here).
-    if (hasActiveOverride(el) && !unflushedOverride(el))
+    if (visibleOverride(el))
       return !el._equals || !el._equals(staged as any, unwrapOverride(el._x?._overrideValue));
     // A quiet re-ask's held landing still answers the same question: the
     // classification survives the landing (asyncWrite) and dies with the
@@ -503,9 +502,7 @@ function latestRead<T>(el: Signal<T> | Computed<T>): T {
   const prevPending = latestReadActive;
   setLatestReadActive(false);
   const visibleValue = (
-    hasActiveOverride(el) && !unflushedOverride(el)
-      ? unwrapOverride(el._x?._overrideValue)
-      : el._value
+    visibleOverride(el) ? unwrapOverride(el._x?._overrideValue) : el._value
   ) as T;
   // A28: an unflushed write is not the staged value latest() serves. The
   // shadow was written at the source's write to mirror it (A8) — consult it

packages/signals/src/store/next/store.ts

@@ -36,7 +36,9 @@ import {
   isEqual,
   latestReadActive,
   stale,
-  unflushedOverride,
+  hasActiveOverride,
+  visibleOverride,
+  readerSeesCommitted,
   prepareComputed,
   read as readNode,
   READ_SLOW,
@@ -65,7 +67,7 @@ import {
   setProjectionWriteActive,
   setStoreCommitHook
 } from "../../core/scheduler.js";
-import type { Owner, Signal } from "../../core/types.js";
+import type { Computed, Owner, Signal } from "../../core/types.js";
 import { pendingCheckActive, strictRead } from "../../core/core.js";
 import {
   DEV,
@@ -1607,11 +1609,17 @@ const UNSAFE_KEYS = new Set<PropertyKey>(["__proto__", "prototype", "constructor
  * computed (#2687 — untracked reads inside mapArray Roots see in-flight
  * values mid-flush). CHILDREN_FORBIDDEN execution scopes (createTrackedEffect
  * / onSettled callbacks) get COMMITTED visibility (#3006), same as core. */
-function inOwnerContext(): boolean {
+/** Core read()'s reader: the current computation, a root reading as its
+ * parent computed (`context` persists under untrack — an untracked read
+ * inside an effect is still that effect's read). */
+function readerContext(): Computed<any> | null {
   const c: any = getOwner();
-  if (c === null) return false;
-  const eff = c._root ? c._parentComputed : c;
-  return eff != null && !(eff._config & CONFIG_CHILDREN_FORBIDDEN);
+  return c === null ? null : c._root ? (c._parentComputed ?? null) : c;
+}
+
+function inOwnerContext(): boolean {
+  const eff = readerContext();
+  return eff !== null && !(eff._config & CONFIG_CHILDREN_FORBIDDEN);
 }
 
 /** CHILDREN_FORBIDDEN execution scope (createTrackedEffect / onSettled
@@ -1752,18 +1760,7 @@ export function runAuthoritative<T>(fn: () => T): T {
   }
 }
 
-/** Active optimistic override on an armed node (armed slot idles at
- * NOT_PENDING; undefined = unarmed plain node). */
-export function hasActiveOverride(node: Signal<any>): boolean {
-  return node._x?._overrideValue !== undefined && node._x?._overrideValue !== NOT_PENDING;
-}
-/** The override a READER sees: installed, and carried by a flush (A28 (5) —
- * an optimistic write is a write; until its flush no reader sees it). The
- * writer's own channels (the draft, `in`/keys inside the setter) compose on
- * the installed override regardless — they use hasActiveOverride. */
-export function visibleOverride(node: Signal<any>): boolean {
-  return hasActiveOverride(node) && !unflushedOverride(node);
-}
+export { hasActiveOverride, visibleOverride };
 
 /** The reading computation is until()'s authoritative-view predicate — same
  * source of truth as core read()'s A17 carve-out (`context`, which persists
@@ -1801,23 +1798,20 @@ function nodeValue(node: Signal<any>, backing: any): any {
   // only: staged pending values are authoritative, overrides are the
   // caller's optimism.
   const v =
-    !authoritativeServe() && hasActiveOverride(node) && !unflushedOverride(node)
+    !authoritativeServe() && visibleOverride(node)
       ? unwrapOverride(node._x?._overrideValue)
       : node._pendingValue !== NOT_PENDING &&
+          // Store-only tunnels first: latest() reaches this untracked path for
+          // store keys (#3075) and truth authors (authoritativeServe: the
+          // projection derive's draft, write-override) see staged truth
+          // unconditionally. Then Rule 1's committed-vs-staged arm — the
+          // same readerSeesCommitted core read() serves tracked reads by
+          // (owner context, children-forbidden, stale-of-foreign, HELD truth,
+          // lanes) — with core's context selection (a root reads as its
+          // parent computed).
           (latestReadActive ||
-            // Owner-context pending visibility — except HELD truth (#3164,
-            // see CONFIG_HELD_TRUTH: fold-staged or entangle-stolen
-            // confirming truth), which only authoritative/latest readers
-            // see (core read()'s A17-for-held-truth twin; ordinary readers
-            // keep committed until the transaction's reveal — latest() is
-            // exempted by the leading arm above).
-            // — and core read()'s stale-reader clause: a render effect's
-            // untracked read of a FOREIGN transaction's write sees committed
-            // (#3336; the tracked read reaches core read() and already does).
-            (((inOwnerContext() &&
-              !(stale && node._transition !== null && foreignHold(node._transition))) ||
-              authoritativeServe()) &&
-              !(node._config & CONFIG_HELD_TRUTH && !authoritativeServe())))
+            authoritativeServe() ||
+            !readerSeesCommitted(node, readerContext(), (node as any)._firewall || node, false))
         ? node._pendingValue
         : backing;
   return v === (FORCE as any) ? backing : v;