fix(lint): render-sync freshness-critical latest-value refs (#1063 follow-up) - #1069
Merged
Merged
Conversation
…#1063) Second cross-model audit round caught a real regression the first pass missed: the six latest-value refs were moved to a passive useEffect, but the deferred init parse (setTimeout) reads contentRef/preserveLineBreaksRef to recover from content drift, and the unmount-flush path (#755) also needs pre-effect freshness. A passive effect has nondeterministic ordering versus setTimeout(0), risking a stale read that initializes the editor with old content. Revert to a render-phase sync (block-scoped react-hooks/refs disable) — the original, correct behavior. Also corrects the useGenieInvocation disable reason: the MCP bridge handler is a synchronous CustomEvent handler (not async), so the render-phase write is needed for the synchronous handshake to see the latest invokeGenie.
…#1063) Round 2 of the cross-model audit found two more refs with the same hazard as TiptapEditor — moved to a passive effect but read from paths that can run in the commit-to-passive-flush gap: - SourceEditor: hiddenRef/cursorInfoRef (+ setter refs) are read by CodeMirror's update listener, a delayed focus/restore setTimeout, and an interval poll. A stale hiddenRef could let a now-hidden editor write/sync/focus; a stale cursorInfoRef could restore the previous render's cursor. - useTabDragOut: the drag callback/bar refs are read by synchronous document pointer listeners during an active drag. Both reverted to render-phase sync (block-scoped react-hooks/refs disable). The other passive-synced refs (SourcePane, useContentServer, useTerminalSessions) were audited and confirmed safe — their readers are async and the mirrored callbacks are stable, so the commit-to-passive window is harmless there.
xiaolai
added a commit
that referenced
this pull request
Aug 12, 2026
…llow-up) (#1069) * fix(lint): revert TiptapEditor latest-value refs to render-phase sync (#1063) Second cross-model audit round caught a real regression the first pass missed: the six latest-value refs were moved to a passive useEffect, but the deferred init parse (setTimeout) reads contentRef/preserveLineBreaksRef to recover from content drift, and the unmount-flush path (#755) also needs pre-effect freshness. A passive effect has nondeterministic ordering versus setTimeout(0), risking a stale read that initializes the editor with old content. Revert to a render-phase sync (block-scoped react-hooks/refs disable) — the original, correct behavior. Also corrects the useGenieInvocation disable reason: the MCP bridge handler is a synchronous CustomEvent handler (not async), so the render-phase write is needed for the synchronous handshake to see the latest invokeGenie. * fix(lint): render-sync SourceEditor + useTabDragOut latest-value refs (#1063) Round 2 of the cross-model audit found two more refs with the same hazard as TiptapEditor — moved to a passive effect but read from paths that can run in the commit-to-passive-flush gap: - SourceEditor: hiddenRef/cursorInfoRef (+ setter refs) are read by CodeMirror's update listener, a delayed focus/restore setTimeout, and an interval poll. A stale hiddenRef could let a now-hidden editor write/sync/focus; a stale cursorInfoRef could restore the previous render's cursor. - useTabDragOut: the drag callback/bar refs are read by synchronous document pointer listeners during an active drag. Both reverted to render-phase sync (block-scoped react-hooks/refs disable). The other passive-synced refs (SourcePane, useContentServer, useTerminalSessions) were audited and confirmed safe — their readers are async and the mirrored callbacks are stable, so the commit-to-passive window is harmless there.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1068. A two-round cross-model audit (Codex) of the merged react-hooks-7 adoption found that several "latest-value" refs were moved from a render-phase write into a post-commit
useEffect— which introduces a commit-to-passive-flush staleness window. For refs read by synchronous handlers or timers, that window is a real bug.Findings fixed (4, across 2 rounds)
TiptapEditor.tsxsetTimeout+ the #755 unmount-flush — could read stale content and initialize the editor with old textuseGenieInvocation.tsSourceEditor.tsxhiddenRef/cursorInfoRefread by CodeMirror's update listener, a delayed focus/restoresetTimeout, and an interval poll — stalehiddenRefcould let a hidden editor write/focus; stalecursorInfoRefrestores the wrong cursoruseTabDragOut.tsAll four reverted to render-phase sync with a block-scoped
react-hooks/refsdisable — the original, correct behavior.Deliberately left as passive effects
SourcePane(onDiagnosticsRef),useContentServer(startServerRef),useTerminalSessions(callbacksRef) were audited and confirmed safe: their readers are async and the mirrored callbacks are stable, so the commit-to-passive window is harmless. This is the correct per-case split — freshness-critical refs are render-synced; stable async-only refs stay in effects (more concurrent-safe).Verification
Each fix independently re-verified FIXED by a fresh Codex pass. Full
eslint srcclean, tsc clean, file-size at baseline, 275 affected tests pass.Refs #1063