LearnNewsExamplesServices
Frontmatter
id18208
titleThe Workstation overwrites the engine''s item-only refresh, so #18152''s lock fix never reaches the demo host
stateClosed
labels
bugaiperformancedashboard
assigneesneo-opus-grace
createdAtSep 3, 2026, 3:53 PM
updatedAtSep 15, 2026, 7:58 PM
githubUrlhttps://github.com/neomjs/neo/issues/18208
authorneo-opus-grace
commentsCount5
parentIssuenull
subIssues[]
subIssuesCompleted0
subIssuesTotal0
contentTrust
projected
quarantined0
signals[]
blockedBy[]
blocking[]
closedAtSep 15, 2026, 7:58 PM

The Workstation overwrites the engine's item-only refresh, so #18152's lock fix never reaches the demo host

Closed Backlog/active-chunk-20 bugaiperformancedashboard
neo-opus-grace
neo-opus-grace commented on Sep 3, 2026, 3:53 PM

Status (2026-09-14) — live symptom, retired prescription

Reopened at @neo-gpt's audit after @tobiu challenged my closure. He was right and so is the audit: closing the proposed implementation is not the same disposition as retiring the problem, and I conflated them. The prescription below is blocked; the defect is not fixed.

Live symptom, verified at dev@933dfc67d3. The host still writes retainTopology unconditionally, now at apps/workstation/view/VesselWorkspace.mjs:897-905, since #18677 moved getRefreshOptions out of Workspace.mjs. Same descriptor and receiver, {operation: 'setItemLocked', itemId: 'metrics', locked: true}:

{"engine": {"retainTopology": true}, "host": {"geometryOnly": false, "retainTopology": false}}

That is the refresh-option mismatch. No fresh browser measurement of the visible flicker is claimed.

Retired prescription. Both PRs on this line (#18213, #18254) are closed unmerged, and the directive against adding to the dock projection layer has never been interpreted for a host-side change that REMOVES a replacement rather than adding a derivation. Reopening reinstates neither, and their source assumptions predate the move above.

What closes this: a demonstrated fix, an explicit transfer to a named open successor with matching scope, or an explicit retirement of the problem itself. A maintainer moving to another lane is none of those.

Owner: @neo-opus-grace.

Context

#18152 fixed the lock flicker the operator reported: setItemLocked assigns one boolean on one item and touches document.nodes nowhere, so the engine now derives {retainTopology: true} for it instead of restaging the shell and re-parenting every header in the app.

It does not reach apps/workstation — the host the operator demoed multi-window docking with.

Measured

Same descriptor, same instance, engine default versus the host's override:

engine says: {"retainTopology":true}
host   says: {"geometryOnly":false,"retainTopology":false}

The host's override writes the key unconditionally (apps/workstation/view/Workspace.mjs):

retainTopology: operation === 'detachItem' || operation === 'transferNode',

For setItemLocked that expression is false, and because it is an unconditional key it overwrites the engine's true rather than falling back to it. So a lock click in the Workstation still takes the full staged transaction, and the flicker #18152 measured and removed is still live there.

Why this is structural, not one missing operation

The clause shadows the whole itemFlags class, and it will shadow whatever the engine derives next, because the host answers retainTopology for every commit whether or not it has an opinion. #18206 hit the same shape on the geometry half and fixed it by delegating (base.geometryOnly === true); this is the same fix on the other key, and it is the last one in this override.

That is worth stating plainly: after this, the host's getRefreshOptions contributes only what is genuinely its own — an explicit geometryOnly on its options-object commits, and its commit-scoped preserveItemIds. Which is what epic #18151 is for.

Proposed change

Delegate instead of overwriting, the way the geometry key now does: take the engine's retainTopology and add the host's two operations to it rather than replacing it.

The railed-item guard needs care and is the reason this is not a one-liner: the engine's itemFlags answer is {} for an item currently on an edge rail (a pinned or auto-hidden pane projects outside the shell, so an item-only refresh would leave a stale copy on the rail). That guard reads isDockRailedItem, which needs the host's live dockModel. A naive || merge would resurrect exactly the bug #18152's railed arm was added to prevent, so the arm must be an item that IS railed, not just any locked item.

⛔ Two AC corrections, measured 2026-09-15 at dev@e1f1d00307

Both describe machinery that is not in the tree, so they cannot be satisfied as written. #18746 implements everything else.

AC-3's railed guard does not exist. This body says "the engine's itemFlags answer is {} for an item currently on an edge rail… that guard reads isDockRailedItem, which needs the host's live dockModel." Measured:

  • src/dashboard/dock/Workspace.mjs:2375 returns {retainTopology: true} for itemFlags unconditionally — no railed branch.
  • isDockRailedItem appears nowhere in src/ or apps/.

So there is no pre-check for a naive || to resurrect, and the warning that shaped this prescription was describing a guard that had already gone.

What actually protects a railed item is downstream, and getRefreshOptions's own JSDoc says so: the key is an admission REQUEST, and "the reconciler validates it, falls back to the staged transaction on any structural delta." Rail handling lives in LayoutAdapter's railedItemIds pass, and the behavioural guarantee is held at component tier by DockLock.spec.mjs:154 — "locked rail remains revealable while inert, and the unlock reaches the revealed pane in place" — green on the fix branch.

AC-5 is not met and I am not claiming it. DockLock.spec.mjs exercises the dashboard host; extending the witness to apps/workstation needs the Neural Link runtime and is separate work. The unit arms prove the option now delegates. They do not prove the repaint is gone in this host — this body's own status line already said no fresh browser measurement was claimed, and that stays true.

Acceptance Criteria

  • apps/workstation's getRefreshOptions derives retainTopology from the engine and contributes only detachItem/transferNode on top; no key is written unconditionally.
  • A red-first arm: a lock on a shell item in this host returns retainTopology: true. It fails on today's code.
  • A railed arm: a lock on a pinned or auto-hidden item in this host does NOT take the item-only path. It fails if the merge is a naive ||.
  • The detachItem/transferNode mapping and the geometryOnly passthrough are unchanged — measured, not assumed.
  • The component-tier flicker witness (DockLock.spec.mjs, mutations outside the locked pane's own header) is extended to this host, or the ticket records why the unit arms are sufficient.

Non-goals

  • detachItem / transferNode versus the class map. The host requests retainTopology for both while operationChangeClass declares them topology. Not a contradiction — the map is deliberately conservative and retainTopology is a different axis (items may change, structure holds) — but the host knows something the engine does not, and moving that knowledge into the engine needs its own measurement. Still open after this.
  • Any change to the engine's derivation or to the class map.

Related

#18151 (epic — this is the same re-derivation on the other key) · #18152 (the fix that does not reach this host) · #18206 / PR #18207 (the geometry half, where the delegation pattern is established) · #18188 (landedInPlace as the outcome witness)

Found while writing #18206's delegation arm — the arm passed, and the neighbouring line was the answer to a question I had not asked.

Sweep before filing: retainTopology full-text over open+closed (no equivalent; #18152 closed on the engine default, no host follow-up), live latest-20 open, own open issues.

Authored by Grace (Claude Opus 5, Claude Code). Session 8d936ec9-b816-4ded-a91e-6e91ef6a3325.


Update 2026-09-03 21:30 UTC (@neo-opus-grace, author) — the prescription above is RULED OUT. The defect is not.

PR #18213 implemented the "Proposed change" section exactly, was reviewed and APPROVED by @neo-opus-ada at 15:34, and I closed it myself at 17:34 on an operator directive: stop adding to this layer. Nothing was wrong with the diff. The layer it improves is what the directive is about.

Recording it in the body rather than leaving it in a closed PR's comment, because a reader who finds this ticket by its title will otherwise re-implement the delegation and re-derive the approval. That is not hypothetical — it is the shape a peer hit today (#18222 dropped, #18217 closed NOT_PLANNED, "I re-implemented a forbidden boundary the operator ruled out"), and I nearly did it here: I picked this ticket up as an unclaimed lane and only found the ruling by checking why an existing branch had no PR.

The reasoning, so it is judgable and not just an instruction. The change-class machinery this ticket extends into apps/workstation exists because a dock projection re-derives every tab header's chrome on every commit — which is why one setItemLocked, a single boolean on one item, produced 32 DOM mutations, 25 of them outside any tab header. Making that re-derivation cheaper per commit is optimising a layer whose existence is the defect. The named alternative is that a tab header toolbar owns its own chrome and the dock owns placement only: with item.locked as a config, object permanence makes a lock click one config write on one component and a genuine no-op for every unrelated header — no change class, no retainTopology, no geometryOnly, no railed-item guard.

Measurement, re-run at dev@274b63beac rather than quoted from the closure. src/dashboard/dock/Workspace.mjs is 4317 lines / 107 class members. Sixteen of those members are tab-header chrome — eleven syncDock* sweeps and five handleDock*Action handlers — totalling 761 body lines + 261 JSDoc = 1022 lines, 23.7% of the file. Method extent is measured decl-line to the line before the next 4-space member decl, trailing blanks trimmed; the JSDoc column is the contiguous block above each decl. My closure comment said "862 (19%)" and "13 sweeps" — both were loose: the sweep count missed three async members, and I did not state the boundary the line figure used. The counts above are the ones to cite.

syncDockLockItemPresentation (86 body lines) is the single clearest instance: it reaches into a button's wrapperCls and a pane's vdom.inert from the workspace.

What this means for this ticket. The measured defect stands — a lock click in apps/workstation still takes the full staged transaction, and #18152's fix still does not reach the host the multi-window dock is demoed with. What is void is the Proposed change and its ACs. I am not rewriting them into a 1022-line relocation: that is a different thesis at a different layer, it is high-blast, and it wants a shaped question rather than an AC list minted from a heartbeat run. Left open and assigned to me as the symptom record until the chrome-ownership question has a surface.

Not filed as a ticket tonight, and why — the relocation is Discussion-shaped under ticket-create §9, and §6.2 family-keyed quorum is not reachable while every active seat is claude-family (the same constraint recorded on #17416 at 20:11). Filing it as an architecture ticket from a heartbeat would create the premature center-of-gravity §1d exists to prevent. Named here and on #18151 so it is visible to whoever opens that Discussion; unowned.

The branch is not lost: agent/18208-workstation-lock-delegation still holds the implementation and its two mutation-verified arms at 719b0b86ec, should the directive ever be revisited.


Update 2026-09-04 02:30 UTC (@neo-opus-grace, author) — the note above failed on the reader it was written for: me.

At 02:13 a later session of mine opened PR #18254 against this ticket, re-implementing the same one-line delegation from the ruled-out prescription. @neo-opus-ada declined it as a Drop+Supersede rather than approve the change a second time, and I closed it. Verified, not inferred: #18213 closed 2026-09-03T17:34:05Z at 719b0b86ec; #18254's head eca5010420 produces the identical retainTopology line over the same two files.

The note worked — on a reviewer. It was written so that "a reader who finds this ticket by its title will otherwise re-implement the delegation and re-derive the approval". A reviewer read the body before the diff and caught it. The author never read it, because nothing put it in front of them: a ticket can carry a directive in its body and a PR can be opened against that ticket with nothing surfacing it. @neo-opus-ada recorded that as a [TOOLING_GAP] on the review; it is the reusable finding here, and it is worth more than the PR was.

Not lost. #18254's test rework is better than #18213's: the arms drive the engine's real isDockRailedItem through a dockModel document instead of a hand-held boolean. Branches agent/18208-host-retain-topology and agent/18208-workstation-lock-delegation both stand.

Open question for @tobiu, unresolved by either of us: does "stop adding to this layer" admit a change that subtracts from it? #18254 removes getRefreshOptions' last unconditional key. A reviewer narrowing a directive is how a ruling erodes, so neither Ada nor I took that call. On a lift the head is intact.

The measured defect is unchanged: a lock click in apps/workstation still takes the full staged transaction, and this stays open as the symptom record until the chrome-ownership question has a surface.

tobiu referenced in commit 57c3efe - "fix(workstation): the host adds to the engine's in-place admission instead of replacing it (#18208) (#18746) on Sep 15, 2026, 7:58 PM
tobiu closed this issue on Sep 15, 2026, 7:58 PM