fix(dashboard): stop trusting pointercancel end coordinates in drag hook

[javenciu]

Problem / Motivation

usePointerDrag derives its onEnd payload from a position tracker so that terminal events carrying sentinel coordinates cannot corrupt the reported delta (#8958). The tracker-refresh guard, however, is a deny-list that excludes only lostpointercapture:

if (e.type !== 'lostpointercapture') {
  s.lastX = e.clientX
  s.lastY = e.clientY
}

pointercancel passes this guard. A cancel is platform-fired (touch scroll takeover, pen leaving digitizer range, palm rejection, screen-orientation change) — the user never chose its position — and engines have shipped it with default-initialized coordinates (0,0). Pointer Events Level 3 had to add an explicit clarification that pointercancel coordinates must match the last dispatched pointer event precisely because behavior diverged. On an engine delivering the 0,0 shape, the guard refreshes the tracker to 0,0 and onEnd reports dx ≈ -startX.

The in-code comment claims "pointerup/pointercancel carry real coordinates" — for pointercancel that is only true on engines conforming to the L3 clarification, and the hook's own harvested rule from #8958 ("platform-fired ends can carry sentinel coordinates") describes pointercancel exactly.

Why it matters

Resizer consumers run persist(apply(sign * dx)) in onEnd. A mid-drag pointercancel with sentinel coordinates commits a clamped-extreme or collapsed pane size and writes it to localStorage — a persisted wrong layout from an end the user never performed. This is the same harm class #8958 closed for lostpointercapture, reachable through the one platform-fired end type the deny-list still trusts.

What changed (motivation → approach → change)

Observed symptom: at the unfixed tree, a synthetic pointercancel with clientX: 0, clientY: 0 after mid-drag moves produces onEnd({ dx: -100, dy: -100, x: 0, … }) instead of the last tracked position (reproducer output below).

Root cause: the coordinate-refresh guard classifies end events by denying known-bad types instead of allowing known-good ones. Any platform-fired end type not on the deny-list is trusted by default — pointercancel today, and any future terminal type tomorrow.

Change: invert the guard to an allow-list. Only pointerup — the user-driven end whose coordinates are spec-defined — refreshes the tracker; every platform-fired end commits the last tracked position:

if (e.type === 'pointerup') {
  s.lastX = e.clientX
  s.lastY = e.clientY
}

On a conformant engine the excluded cancel carries the last dispatched coordinates — exactly what the tracker already holds — so the inversion is lossless there and fail-safe on engines that deliver sentinels (worst case: one coalesced-move stale, never a corrupt delta). The contract comment and the DragInternal docstring are synced to the repaired behavior in the same commit.

Alternatives rejected:

• Extending the deny-list to pointercancel: repeats the structural mistake; a future platform-fired terminal type would be trusted by default again.
• Sanitizing 0,0 specifically (if (x || y)): 0,0 is a legal on-screen position for a real pointerup; guarding by value corrupts a genuine top-left release.

Lineage (one topic per PR): #8904 made the hook terminate on capture loss; #8958 derived end coordinates from the tracker for lostpointercapture; this PR closes the remaining cancel-class window that #8958's own review round named as a residual. Each change is a distinct guard on the same seam.

Tests

Two attack pins, one conformant control, one idempotence pin (all in website/src/hooks/usePointerDrag.test.tsx):

• a pointercancel with sentinel 0,0 coordinates ends from the last tracked position — mid-drag cancel at 0,0 must report the moved-to position (fails before, passes after).
• a pointercancel before any move ends at the drag origin (dx 0), not at 0,0 — pre-move cancel must report the origin (fails before, passes after).
• a spec-conformant pointercancel (coordinates match the last dispatched event) ends identically — proves the allow-list is lossless on conformant engines (passes both sides; regression control, stated honestly).
• does not double-fire onEnd when lostpointercapture follows a pointercancel — the spec's implicit-release sequence fires lostpointercapture right after pointercancel; end stays single-fire (passes both sides).

Reproducer output at the unfixed tree (base 0d65dc969):

× a pointercancel with sentinel 0,0 coordinates ends from the last tracked position
× a pointercancel before any move ends at the drag origin (dx 0), not at 0,0
AssertionError: expected { dx: -100, dy: -100, x: +0, …(3) } to match object { Object (dx, dy, ...) }
Tests  2 failed | 11 passed (13)

After the fix: 13 passed (13). All nine pre-existing hook tests pass unchanged — including a normal pointerup still ends from its own (real) coordinates, which pins that the user-driven path still trusts its event.

Consumer sweep: 115 test files across every usePointerDrag call site (ChatInput, SessionGridLayout, ChatSidebar, useColumnResize, ColumnSplitter, BottomTerminalPanel, DetailPanel, SidePanel, FileExplorerPage, ResizeHandle) — all green. tsc -b clean, eslint clean, theme/phantom/i18n lints clean, full website suite green.

Manual verification

Verified through the jsdom harness above (synthetic pointerCancel with explicit coordinates). No real-device reproduction was performed — provoking a genuine palm-rejection or scroll-takeover cancel deterministically requires hardware; the unit pins encode both engine behaviors (sentinel and conformant) instead.

No user-visible UI change: the fix alters which coordinates an internal tracker trusts on a platform-fired drag end. Layout, styling, and the user-driven drag path are untouched, so there is nothing to screenshot.

Related Issues

Follow-up to #8958 (capture-loss drag ends) and #8904 (capture-loss termination) — closes the pointercancel residual named in #8958's review round. Non-closing reference: no tracker issue exists for this residual.

Pattern harvest

The deny-list trusted an event type nobody had audited: the guard was written for the lostpointercapture incident and excluded exactly that type, leaving every other platform-fired end trusted by default. The durable shape is to key coordinate trust to the one user-driven end type and exclude platform-fired ends as a class.

Rule candidate: when guarding against unreliable event payloads, allow-list the event types whose payload the spec defines as user-driven; a deny-list built from the incident that prompted it silently trusts every type not yet witnessed.

Adjacent same-seam residual, deliberately not in this PR (one topic): the hook swallows setPointerCapture failure, so a drag whose capture never engaged can still miss its end if the pointer is released off-element. Registered follow-up; distinct mechanism (capture acquisition, not end-coordinate trust).

Checklist

• Tests added/updated and passing locally (13/13 targeted; 115 consumer files; full suite)
• Reproducer fails before and passes after (output pasted above)
• Docs: no owning doc names this hook's coordinate contract; the in-file contract comment is the doc of record and is updated in the same commit
• Conventional commit, one topic, single commit
• No baselined file reformatted

Contribution License Agreement

Per the template placeholder (CLA text pending): offered under the same terms as my prior merged contributions to this repository (#8835).

[github-actions]

UX Review (Fable 5, fork) — ✅ PASS

UX-level review of ee921719d25de15c7d25e221933925abc716274a via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The diff touches only usePointerDrag.ts (an internal coordinate-trust guard) and its tests — no JSX, no user-facing strings, no controls added or changed, so there is nothing for a blind read or screenshot to show. The user-facing effect is purely corrective: a platform-fired pointercancel can no longer snap a resized pane to a collapsed/extreme size and persist it.

UX-Verdict: PASS

Internal drag-hook guard fix with no user-visible surface; it only prevents a phantom pane-collapse on platform-cancelled drags.

[UX-REVIEWED] ee92171

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

Design-level review of ee921719d25de15c7d25e221933925abc716274a via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The patch matches the description end-to-end: the deny-list → allow-list inversion in usePointerDrag.ts:116 is the structural fix for the class of failure #8958 patched per-type, the trade-off on conformant engines is correctly reasoned (tracker already holds the last dispatched coordinates), tests pin both engine behaviors plus the implicit-release idempotence, and the adjacent capture-acquisition residual is properly scoped out. No design-level concerns.

Design-Verdict: PASS

Inverting to an allow-list on the one spec-defined user-driven end fixes the root cause #8958 only symptom-patched; lossless on conformant engines.

[DESIGN-REVIEWED] ee92171

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed ee921719d25de15c7d25e221933925abc716274a via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] ee92171

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed ee921719d25de15c7d25e221933925abc716274a via the fork AI-review pipeline; updated in place on each push.

Review details

I've analyzed the diff against the base code. The change narrows the tracker-refresh allow-list in end() from e.type !== 'lostpointercapture' to e.type === 'pointerup', so pointercancel now derives its onEnd payload from the last tracked position rather than the cancel event's own coordinates.

Verifying the falsification bar myself:

• No regression path exists. The tracker (lastX/lastY) is refreshed unconditionally on every pointermove (lines 82–83, before the threshold check). On a spec-conformant engine pointercancel coordinates equal the last dispatched pointer event — exactly what the tracker holds — so committing the tracked position is lossless. On a non-conformant engine (0,0) it's a strict fix versus the prior dx ≈ -startX.
• Idempotence preserved. end flips s.active = false before releasePointerCapture, so a lostpointercapture following a pointercancel is a no-op re-entry — no double onEnd.
• pointerup still refreshes, preserving the case where the terminal up position legitimately differs from the last move.
• No AUTOSDE frontend rule (icons, strings, colors, dangerouslySetInnerHTML, dates) is implicated; the diff is hook logic plus added tests.

The discovery pass's "no candidates" holds under independent re-derivation, and I found no groundable Step-2 finding: every (a)/(b)/(c) chain I could construct resolves to "lossless or improved," not an observable wrong outcome.

No findings.

[OPUS-REVIEWED] ee92171

[github-actions]

First Principles Review (Fable 5, fork) — ✅ PASS

Premise-level review of ee921719d25de15c7d25e221933925abc716274a via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All evidence checked. The cancel path in MarkdownRenderer.endSwipe hits abortSwipe() before the e.clientY read, so it never trusts cancel coordinates — sibling count outside the hook is zero. The fix lands in the one shared mechanism (usePointerDrag) that all 10 resizer consumers route through, the described failing-before/passing-after test claims match what the base code would actually do (base guard e.type !== 'lostpointercapture' demonstrably lets pointercancel refresh the tracker to 0,0), and no new public surface, config key, or flag ships. Final review:

First-Principles-Verdict: PASS

A real spec-divergence defect fixed at its cause — the trust classification itself — in the one shared hook, with zero new surface.

What this change ships

Intent: stop a platform-fired pointercancel carrying bogus 0,0 coordinates from making resizers persist a corrupted pane size. This is a FIX.

1. A mid-drag pointercancel now ends the drag at the last tracked position, not the event's coordinates — justified
2. Tracker-refresh guard inverted from deny-list to allow-list (pointerup only) — justified; cause-level, declared
3. Contract comment and DragInternal docstring rewritten to match — rides along legitimately (same-commit doc sync is a documented invariant; the in-file comment is the doc of record)
4. Four tests pinning sentinel, pre-move, conformant, and double-fire cases — justified

Every item is derived: the harm is a persisted wrong layout (persist(apply(sign * dx)) in onEnd, written to localStorage), the cause is a platform rule (Pointer Events L3's cancel-coordinate clarification exists precisely because engines diverged), and it is the same defect class #8958 already established as real. The fix reuses the existing tracker mechanism from #8958 rather than adding a second one. Only three event types reach end (up/cancel/lostcapture), so the allow-list is the smallest expression — smaller than a two-entry deny-list. Sibling count for "commit end state from cancel coordinates": 0 (grepped pointercancel across website/src, 17 files; every other handler discards state or aborts before reading coordinates — useLongPressReorder.ts:115, EmbedTabStrip.tsx:280, DiagramLightbox.tsx:169, MarkdownRenderer.tsx:4141). Consumer sweep verified: exactly the 10 named call sites. No config key, flag, or exported symbol added.

[FIRST-PRINCIPLES-REVIEWED] ee92171

[iamwhatever]

Tier 1 auto-approve: fix (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: single clear root cause -- the drag hook refreshed its position tracker on every terminal pointer event, so a platform-fired pointercancel carrying sentinel 0,0 coordinates produced an end payload with dx of roughly -startX that resizers persist to localStorage; the fix narrows the tracker refresh to an allow-list of pointerup only, so the end payload derives from the last user-driven position. Touches one source hook plus its own test file, no behaviour change on spec-conformant engines. CodeQL is not applicable on this fork PR (default-setup emits no check-run); SAST coverage is Semgrep only, latest run success with 0 annotations.