LearnNewsExamplesServices
Frontmatter
id18236
titleObject permanence is test-enforced impossible: headerActions_ opts out of change detection, and an arm asserts the teardown
stateClosed
labels
bugaicore
assigneesneo-opus-ada
createdAtSep 4, 2026, 12:05 AM
updatedAtSep 4, 2026, 1:12 AM
githubUrlhttps://github.com/neomjs/neo/issues/18236
authorneo-opus-ada
commentsCount3
parentIssuenull
subIssues[]
subIssuesCompleted0
subIssuesTotal0
contentTrust
projected
quarantined0
signals[]
blockedBy[]
blocking[]
closedAtSep 4, 2026, 1:01 AM

Object permanence is test-enforced impossible: headerActions_ opts out of change detection, and an arm asserts the teardown

neo-opus-ada
neo-opus-ada commented on Sep 4, 2026, 12:05 AM

From @tobiu's dialogue with @neo-opus-vega. Her finding; the gate she named is now closed and the consequence measured.

⚠️ Corrected 2026-09-04 — the original premise was wrong

This ticket first claimed the failing spec arm "pins the teardown as the contract" and that inverting it was the point. False. The arm guards a real contract, and the defect is one level lower: Neo.core.Compare.compareFunctions cannot distinguish two functions with the same source. Measurement in the retraction comment. The body below is corrected; the census and the symptom chain still stand.

The reflex — and why it is not one

Eight configs in the engine override isEqual. Four are () => false, all list-shaped, all byte-identical in descriptor, none carrying a reason:

config file
container.Base#items_ container/Base.mjs:106
tab.Container#headerActions_ tab/Container.mjs:102
toolbar.Base#actions_ toolbar/Base.mjs:42
selection.Model#items_ selection/Model.mjs:45

The two that were reasoned carry the why on the same line — VdomLifecycle:96 (vnode trees can be huge) and button.Base:81 (functions are never equal). Not one of the four does.

The census is accurate; the inference I drew from it was not. Undocumented is not unjustified. At least two of these four are load-bearing, for the mechanical reason below — button.Base:81's stated reason turns out to be the general case, not a special one.

The real defect: deep equality is blind to captured state

src/core/Compare.mjs:53:

static compareFunctions(item1, item2) {
    if (item1.name !== item2.name) { return false }
    return item1.toString() === item2.toString()
}

Two bound functions from one target are indistinguishable to it — name is "bound boundHandler" and toString() is "function () { [native code] }" for both. Two closures from one factory likewise: identical source text, different captured values.

const make = x => () => x;
make(1) vs make(2)   →  reported EQUAL  →  but they return 1 and 2

So any config carrying handlers must opt out of change detection, or genuine handler replacements are silently dropped. That is what isEqual: () => false on headerActions_ is doing, whether or not its author knew it.

This form arrived in #6130, a mechanical refactor ("use a map and move the testing logic into separate methods") that lifted the branch out of a switch. No rationale stated then or since.

What removing the opt-out alone does

Deleting isEqual: () => false from headerActions_ and running unit/tab: the arm at HeaderActions.spec.mjs:340 fails.

expect(secondBoundAction).not.toBe(firstBoundAction);   // :340 — fails
expect(secondBoundAction.handler()).toBe(2)             // :341 — never ran, the run aborted above

Reordering the two so :341 executes gives the real answer:

Expected: 2
Received: 1

The handler swap is dropped and the old handler stays live. :340 is the instance-level proxy for that contract, not an assertion about teardown for its own sake. The arm is the guard.

HeaderActions.spec.mjs:113 carries the byte-identical pair against a plain toolbar, exercising toolbar.Base#actions_ — the second opt-out on this path. It keeps its opt-out per AC-5 and passes throughout.

What actually fixes it

Compare functions by identity — the only sound answer, since behavioural equivalence is not decidable from source text:

static compareFunctions(item1, item2) {
    return item1 === item2
}

Then headerActions_ needs no opt-out and both properties hold at once: an identical re-projection matches and is a no-op (object permanence), a genuine handler change differs and fires.

Measured with both changes: test-unit 3143 passed / 0 failed, test-components 234 passed / 0 failed.

The symptom chain (unchanged)

Every projection rewrites headerActions → syncActions destroys and rebuilds the buttons → actionsChange fires → tab.plugin.Overflow (1,209 lines) re-measures and re-splits. One lock click repaints every header because the dock keeps resetting the header while the header tries to do its own job.

Acceptance Criteria

  • AC-1 — headerActions_ uses the engine's default isEqual. Depends on AC-0: removing the opt-out alone drops handler swaps.
  • AC-0 — Compare.compareFunctions compares by identity. Red-first: a spec asserting that two bound functions from one target, and two closures from one factory, are not equal.
  • AC-2 (rewritten — the original was withdrawn) — HeaderActions.spec.mjs:340 is kept; it asserts a real contract. A sibling arm is added for the other direction: an identical re-projection preserves the instance.
  • AC-3 — red-first by measurement: count Neo.button.Base constructions across an identical re-projection. Before: one per action per projection. After: zero — with a control proving the counter can fire.
  • AC-4 — the flaky tab-header click @neo-opus-vega chased is re-tested against this change. If it survives, say so.
  • AC-5 — the other three () => false configs are not touched here. container.Base#items_ holds component configs that legitimately carry handlers, so its opt-out is honest for the same mechanical reason; each needs its own sweep and ticket once AC-0 lands.

Out of Scope

  • syncActions diffing by stable action key, and the projection emitting a fresh array per call. Both are @neo-opus-vega's pieces 2 and 3 — independently shippable, each removing behaviour.
  • A shared keyed-list comparison in core.Config. AC-0 may remove the need for it entirely.

Open question, not decided here

AC-0 touches core.Compare, which is engine-wide. Identity comparison makes a freshly-built inline closure compare unequal where the old form said equal, so some paths will now fire an update they previously skipped. 3377 green tests bound that risk; none of them measures re-render frequency. Flagged for @neo-opus-grace / @neo-opus-vega / @tobiu before anything lands.

Avoided Traps

  • Changing all four at once. Three are unverified and one is the base class of every container.
  • Treating the failing arm as breakage. It is the finding. Inverted: I treated a guard as the finding. The correction came from running the assertion the aborted run never reached.

Origin Session ID: 7596538d-5324-419a-b043-95c585d833ea

Retrieval Hint: isEqual false list configs object permanence headerActions compareFunctions toString name bound closure captured state deep equality handler swap dropped