# `affects()`: Fable audit - 4 bugs

[brenelz]

Branch: next (2.0.0-beta.18, post-#2888) · Package: @solidjs/signals

A focused bug hunt around the question-scoped pending model turned up four high-severity bugs in affects(). Each is pinned by failing repro tests (left in-tree as packages/solid-signals/tests/__hunt-{readpath,txn,store}.test.ts); all existing spec suites pass, so none of this is covered behavior. Root causes are distinct, but bugs 2–4 share a theme: marks ride the async status rails but skip the mechanisms that make those rails self-healing (dirtying on landing, re-throw on read), so states that real async recovers from are terminal for marks.

---

Bug 1 — addPendingSource illegal dual state → isPending() stuck true forever after settle

The pending-source container migrates _pendingSource (singular) → _pendingSources (Set) at two entries, leaving the singular slot empty. addPendingSource (src/core/async.ts:37) checks only the singular slot:

if (!el._pendingSource) {      // true once the Set exists!
  el._pendingSource = source;  // 3rd source lands here, next to Set{#1, #2}
  return true;
}

A third source creates dual state {_pendingSource: #3, _pendingSources: Set{#1, #2}}, and a fourth would silently drop #3 (el._pendingSource = undefined at the end of the set branch). In dual state, removePendingSource refuses to touch Set members while the singular slot is occupied, and settlePendingSource's walk early-returns on that refusal — so mark release strands the two Set sentinels along with STATUS_PENDING.

Real async escapes this because landings dirty the reader and clearStatus wipes everything on recompute. Marks are value-transparent (#2886): nothing ever dirties the holder, so the strand is permanent.

The deterministic trigger is the flagship <For>/mapArray shape: mapArray's internal computed subscribes to exactly three nodes covered by a keyless store mark (rows leaf + length + $TRACK).

const [state, setState] = createStore({ rows: [{ name: "a" }] });
const mapped = createMemo(mapArray(() => state.rows, r => r.name));
createRenderEffect(mapped, () => {});
flush();

const save = action(function* () {
  affects(state);              // keyless mark covers 3 nodes mapArray reads
  yield api.save();
});
await save();
flush();

isPending(() => mapped());     // true — and stays true forever

Instrumentation confirms: post-release the internal computed still holds _pendingSources.size === 2 with every _affectsCount at 0; manually calling settlePendingSource(node, node._affectsSentinel, true) clears it and the verdict flips.

Fix: if (!el._pendingSource && !el._pendingSources) — note this container bug is reachable by anything accumulating 3+ pending sources, not just marks; marks are merely the only client that can't self-heal.

Repros: __hunt-store.test.ts — the mapArray/keyless-mark tests and the two DISSECT/ROOT CAUSE tests.

---

Bug 2 — mark propagation captures downstream subscribers into the marking action's transaction

propagateAffectsMark (src/affects.ts:74) and the notifyStatus descent call queuePendingNode(sub) for every subscriber the mark pends. When the flush stashes the still-in-flight transaction, reassignPendingTransition stamps sub._transition = T_mark on all of them. From then on, any write that dirties one of those subscribers is pulled into the marking action's transaction (recompute/setSignal re-activate or merge into el._transition):

• a concurrent action's transaction merges into the marker's, deferring its settle/revert until the marker finishes (both settle orders repro'd);
• a plain write to the marked signal is transition-held — value unreadable, effects deferred — until the action settles;
• worst: a plain write to a completely unmarked signal that merely shares a downstream memo with the marked one freezes too, and even reads a false isPending() === true.

const [count] = createSignal(1);
const [other, setOther] = createSignal(10);
const sum = createMemo(() => count() + other());
createRenderEffect(sum, () => {});
flush();

const act = action(function* () {
  affects(count);              // mark on count ONLY
  yield inFlight;
});
act();
flush();

setOther(20);                  // plain write, unmarked, unrelated signal
flush();
other();                       // 10 — write is dark until the action settles
isPending(() => other());      // true — false pending on unmarked data

A control with the identical shape minus affects() commits immediately.

This contradicts the existing design note at src/core/scheduler.ts:550 ("marks don't hijack the node's _transition — a mark on a plain signal must not entangle unrelated writes to it") — that fix covered the marked node itself; the subscriber-capture channel does the hijacking one hop downstream. It also breaks mark value-transparency and the "optimistic writes display instantly" pins.

Fix direction: don't queuePendingNode (or don't _transition-stamp in reassignPendingTransition) subscribers whose only new pending source is a mark sentinel — they carry no held value needing a transition-scheduled commit, and the sentinel already never blocks settlement.

Repros: __hunt-txn.test.ts H24–H27 (H28 is the no-mark control).

---

Bug 3 — mark pendingness doesn't survive recomputation beyond one derivation level; the isPending() probe itself destroys it

Registration-time propagation reaches the whole downstream graph, but re-acquisition after clearStatus is gated on el._affectsCount (src/core/core.ts:734, :760) — only direct readers of the marked node collect into affectsReads. A node ≥2 derivation steps away sheds pending on any mid-window recompute and can never get it back.

Worse, the probe is destructive: the sentinel's NotReadyError sits in _error, which updateIfNecessary treats as retryable once the clock advances — so isPending() on a second-level memo triggers the recompute that strips the status it's reporting on.

const [s] = createSignal(1);
const b = createMemo(() => s() * 2);      // depth 1 — fine
const c = createMemo(() => b() + 1);      // depth 2 — broken
createRenderEffect(c, () => {});
flush();

const act = action(function* () {
  affects(s);
  yield inFlight;
});
act();
flush();
// rails delivered STATUS_PENDING to c at registration…
isPending(() => c());   // true — but this probe recomputes c and strips it
isPending(() => c());   // false, while the mark is still live
isPending(() => b());   // true — direct reader re-acquires fine

Consequences repro'd: shed on unrelated-dep recompute (value-changing and equality-bailout), memo chains created during the window never acquiring past depth 1, diamond joins shedding, and a reactive isPending badge memo disagreeing with a direct probe mid-window (the badge stays true only because its recomputed false verdict is transition-held).

This violates the API's own contract (affects() JSDoc: "the named slot(s) — and everything DERIVED from them — read as pending … until the surrounding transaction settles") and the design comment in affects.ts claiming re-acquisition is "the same shape as real async". For real async it is transitive (the re-throw on read re-establishes the source at every level); the value-transparency bypass (core.ts:786, 1dc0b45) removed the re-throw for marks without a replacement.

Fix direction: at the mark-only suspension bypass, a tracked read of a mark-pended node should feed the reader's affectsReads (with the sentinels' _affectsFor nodes, or the sentinels directly) so applyAffectsReads re-establishes transitively.

Repros: __hunt-readpath.test.ts H1, H2, H4, H5, H12b, H13.

---

Bug 4 — a memo that throws a real error under a mark gets it replaced by NotReadyError

recompute applies markedReads unconditionally after the commit block (src/core/core.ts:363) — the if (!el._error) guard protects the commit, not this line. For a memo that read marked data and then threw, applyAffectsReads sets STATUS_PENDING on top of STATUS_ERROR and setPendingError clobbers _error with the sentinel's NotReadyError. The user's error is gone; readers get a NotReadyError from a mark-only-pended node — exactly what value-transparency promises can never happen.

const [s] = createSignal(1);
const [boom, setBoom] = createSignal(false);
const m = createMemo(() => {
  s();
  if (boom()) throw new Error("boom");
  return 1;
});
// … action: affects(s); yield inFlight; …
setBoom(true);
flush();

m();   // throws NotReadyError — "boom" is swallowed
// m's node: _statusFlags === ERROR | PENDING, _error instanceof NotReadyError

Control without the mark surfaces "boom" correctly.

Fix direction: skip applyAffectsReads (or make it error-preserving) when the recompute ended with a real (non-NotReady) _error.

Repros: __hunt-readpath.test.ts H6 (H6b control).

---

Suggested order

1 is a one-line container fix with blast radius beyond marks. 2 and 3 overlap mechanically (both are "marks skipped the rails' self-healing half") and are best designed together. 4 is a local guard.

Two intentional-looking divergences were also observed and deliberately not filed here (may deserve a design decision + spec pin): untracked reads of marked nodes are never pended, and effects reading marked data run mid-window with fresh values instead of deferring like real async.

[ryansolid]

All four fixed on next in 655b614, pinned by tests/affects-audit-2893.test.ts:

• Bug 1 — addPendingSource now checks both container slots before using the singular one, so a third overlapping source joins the Set instead of corrupting it. The flagship keyless-store-mark-over-mapArray shape settles and isPending releases.
• Bug 2 — mark propagation is transaction-inert: pended subscribers are no longer queued as pending nodes (in propagateAffectsMark, the notifyStatus descent, and recompute's needsPendingCommit), so plain writes to marked data or to unmarked signals sharing a downstream memo commit and render immediately.
• Bug 3 — read-path re-establishment is transitive: a tracked read of a mark-pended owner feeds the reader's affectsReads with the sentinels' still-live _affectsFor nodes (collectMarkSources), so pendingness survives mid-window recomputes at every derivation depth — including the recompute the isPending() probe itself triggers.
• Bug 4 — real errors outrank marks: notifyStatus drops mark-sourced propagation onto STATUS_ERROR holders and recompute skips applyAffectsReads after an errored compute, so a thrown error is never clobbered by the sentinel's NotReadyError.

A24 corollaries documented in SPEC-ASYNC-SEMANTICS.md; mechanics in the affects-on-rails section of INTERNALS-ASYNC-STATE.md. The two deliberately-unfiled divergences (untracked reads never pended; effects run mid-window with fresh values) remain open design questions — they follow from mark value-transparency and are currently intended behavior.