Skip to content

PR #327's ProseMirror widget keys omit cursor colour and name, so recycled cursor DOM goes stale #331

Closed

Description

@dezren39

Finding

Open draft PR #327 (⚡ Bolt: Optimize ProseMirror decoration recycling, head 59decd10f6) removes the decorationGeneration counter in pkgs/id/web/src/cursors.ts and gives the collaborative-cursor widgets stable, state-derived keys — but two of the new keys omit properties that are rendered into the widget DOM, so ProseMirror will keep showing the old DOM after those properties change.

ProseMirror compares a widget decoration's key instead of its DOM node to decide whether it needs to be redrawn (ProseMirror reference: widget spec key — "When comparing decorations of this type (in order to decide whether it needs to be redrawn), ProseMirror will by default compare the widget DOM node by identity. If you pass a key, that key will be compared instead"). A property that changes without changing the key therefore never reaches the screen.

Evidence (PR #327 head, cross-checked via gh api contents?ref=59decd10f6)

  • Standalone remote cursor, cursors.ts:738 on the PR head:
    key: cursor-${cursor.clientID}-o${cursor.opacity}-c${connectionState}
    — includes opacity and connection state, but not cursor.color and not cursor.name. The widget it keys is built by createStandaloneCursor / createCursorLine (same file): caret borderColor = cursor.color, label backgroundColor = cursor.color, label text colour derived from cursor.color, and label textContent = cursor.name.
  • Cluster/merged cursor lines, cursors.ts:715 on the PR head:
    key: cursor-cluster-${group.position}-${clusterKey}-c${group.mostRecentColor}
    — the merged bar appended to the leftmost line renders one segment per cursor with that cursor's color (background), name (text), opacity and connection-state-driven strobe state (createMergedBar), and non-leftmost group lines render their first cursor's opacity/strobe state (createCursorLine); none of those per-cursor values are in the key (clusterKey encodes only clientID@head per cursor, and mostRecentColor is only the owning group's first cursor's colour).
  • Contrast inside the same diff: the own-cursor key (cursors.ts:651) correctly includes its only rendered variable property, ownCursor.color. Contrast across the sibling drafts tracked by Triage 11 overlapping Jules Bolt and Palette draft PRs #310: PR ⚡ Bolt: optimize ProseMirror cursor DOM recycling #217's standalone key includes -color-${cursor.color}, its cluster key includes connection state plus an opacity hash, and its selection key includes colour — ⚡ Bolt: Optimize ProseMirror decoration recycling #327 drops colour from the standalone and selection keys.
  • Colour/name are not session constants in this code: the plugin's apply() "update" case replaces color on every cursor update and takes name from the update when present (name = cursorUpdate.name ?? existing?.name), and createCursorDecorations falls back to getColorForClient(clientID) only when an update carries no colour — so a peer's colour source/value and display name can both change mid-session. While a peer is active their opacity stays at 1.0 (see getOpacityForAge in cursor-utils.ts: 1.0 for the first 30s after each update, and every update refreshes lastUpdate), so no other key component forces a redraw either.

Why it matters

If #327 (or a variant with the same keys) merges as-is, a collaborator who changes colour or display name — or whose first updates arrive colourless and later carry an explicit colour — keeps rendering with the stale caret colour and stale name label for everyone else, indefinitely while they stay active. That is a correctness regression introduced specifically by removing the generation counter, in the one file the PR touches. The PR's own added learning note (.jules/bolt.md) states the rule it breaks: properties that dynamically dictate the widget's appearance (it names opacity, connectionState and colour) must be included in the key.

Acceptance criteria

  • Whichever ProseMirror-recycling PR survives the Triage 11 overlapping Jules Bolt and Palette draft PRs #310 triage, every widget key in cursors.ts covers all properties rendered into that widget's DOM: standalone cursor keys include colour and name; cluster keys include the per-cursor segment values (colour, name, opacity) and connection state, not just the group's most-recent colour.
  • The selection decoration's colour handling is checked the same way (PR ⚡ Bolt: Optimize ProseMirror decoration recycling #327's selection-…-o${opacity} key omits cursor.color, which is half of its rendered background-color: ${cursor.color}${selectionOpacity} style).
  • A test or equivalent check pins the key construction: changing only a cursor's colour (or only its name) produces a different standalone key. There is currently no cursors.test.ts (only cursor-utils.test.ts), so the key derivation may need extracting to a pure helper to be testable.

Found by the pr-diff-review job reviewing PR #327 (open) in developing-today/code. Cross-repo check: createCursorDecorations / collab-cursor exist only in developing-today/code (GitHub code search), so this is a single-repo issue. Related triage: #310.

Activity

  1. dezren39 commented on Oct 24, 2026

    @dezren39
    MemberAuthor

    id train triage (2026-10-24): needs-decision — Muse: assign to Session A (phases 0-10 review); ProseMirror cursor widget keys omit colour/name (PR #327 draft).

  2. dezren39 commented on Oct 26, 2026

    @dezren39
    MemberAuthor

    Intake review

    🤖 Agent note · filed

    • Agent: fleet monitor
    • Session / ID: repo-fleet-monitor run 2026-10-26 17:27 CDT
    • Found via: monitor reconciliation
    • Task: Intake enrichment (§5) for this issue
    • Expectation: s — done = labels normalized, acceptance criteria confirmed present, still-live check recorded

    Still-live check (verified this run): PR #327 is now CLOSED (unmerged draft), and every Jules draft in the #310 triage table is closed — so no live PR currently carries these widget keys. That does not resolve the finding: no merged PR fixed the key construction, and the body's acceptance criteria are written for "whichever ProseMirror-recycling PR survives the #310 triage". With no survivor, this issue is the standing pin for any future recycling attempt. Not closing.

    Related: #310 (triage that owned PR #327's fate), PR #217 (closed sibling whose keys included colour — the contrast case in the body).

    Notes / risks: acceptance criteria are complete — no body edit needed. Risk if a recycling PR is revived without this check: stale caret colour/name for active collaborators, indefinitely, per the body's mechanism.

    Suggested approach: if cursor-decoration recycling is re-attempted, extract the key derivation to a pure helper and pin it with the colour-only/name-only key-change test the body asks for, before the recycling change itself lands.

  3. added
    kind/bugSomething isn't working (prefixed form of the classic bug label)
    priority/normalOrdinary priority (the default)
    effort/sSmall: expected under 2 hours of work
    app/idPackage/app: id (pkgs/ or apps/ in this repo)
    meta/enrichedIntake review done: acceptance criteria, links, and normalized labels added
    work/assignedQueue worker lifecycle state
    queue/claimedQueue worker lifecycle state
    and removed on Oct 26, 2026
  4. dezren39 commented on Nov 1, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · picked up

  5. dezren39 commented on Nov 1, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · done (triage)

    • Agent: queue worker (repo-worker-code), Lane A shift
    • Session / ID: queue task 2087
    • Found via: queue claim
    • Task: Triage verdict — the flawed keys existed only in a draft PR that closed unmerged; current main's keys carry every rendered property.
    • Expectation: triage complete — closing as no longer reproducible.

    Verdict: NO LONGER REPRODUCIBLE — never landed, and main is correct.

    So the stale-recycled-DOM failure #331 describes cannot occur on current main: a cursor colour or name change always changes the key, and ProseMirror redraws. Closing as no longer reproducible (the defect's only home was the abandoned draft). No follow-up fix packet — there is nothing left to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

app/idPackage/app: id (pkgs/ or apps/ in this repo)effort/sSmall: expected under 2 hours of workkind/bugSomething isn't working (prefixed form of the classic bug label)meta/enrichedIntake review done: acceptance criteria, links, and normalized labels addedpriority/normalOrdinary priority (the default)source/pr-diff-reviewFiled by the pr-diff-review job

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions