Merge branch 'main' into nick/parts-system-ui-mutations

Author: harbournick

packages/sdk/langs/python/superdoc/transport.py

@@ -435,24 +435,33 @@ async def connect(self) -> None:
         """Ensure the host process is running and handshake is complete."""
         await self._ensure_connected()
 
+    async def _await_in_flight_cleanup(self) -> None:
+        """Wait for an already-running cleanup task, if any."""
+        existing = self._cleanup_task
+        if existing and not existing.done():
+            try:
+                await asyncio.shield(existing)
+            except asyncio.CancelledError:
+                raise
+            except Exception:
+                pass
+
     async def dispose(self) -> None:
         """Gracefully shut down the host process."""
         if self._state == _State.DISCONNECTED:
+            # Reader-triggered cleanup flips state to DISCONNECTED before the
+            # subprocess is fully reaped. If that cleanup is still in flight,
+            # wait for it so dispose() doesn't return while the host process
+            # is still being torn down.
+            await self._await_in_flight_cleanup()
             return
         if self._state == _State.DISPOSING:
             # A reader-triggered cleanup is in flight (or an earlier teardown
             # left state in DISPOSING briefly). Wait for it so the caller
             # observes "host fully torn down" by the time dispose() returns.
             # shield() so a cancelled dispose() doesn't interrupt _cleanup
             # mid-flight and leak the host process.
-            existing = self._cleanup_task
-            if existing and not existing.done():
-                try:
-                    await asyncio.shield(existing)
-                except asyncio.CancelledError:
-                    raise
-                except Exception:
-                    pass
+            await self._await_in_flight_cleanup()
             return
 
         self._stopping = True

packages/sdk/langs/python/tests/test_transport.py

@@ -764,6 +764,54 @@ async def slow_cleanup(error):
         finally:
             _cleanup_wrapper(cli)
 
+    @pytest.mark.asyncio
+    async def test_dispose_waits_for_cleanup_after_state_flips_disconnected(self):
+        # `_cleanup()` flips state to DISCONNECTED before awaiting
+        # `process.wait()`. dispose() must still wait for that cleanup task
+        # instead of short-circuiting and returning while teardown is in
+        # flight.
+        cli = _mock_cli_bin({'handshake': 'ok'})
+        try:
+            transport = AsyncHostTransport(cli, startup_timeout_ms=5_000)
+            await transport.connect()
+            process = transport._process
+            assert process is not None
+
+            wait_started = asyncio.Event()
+            release = asyncio.Event()
+            real_wait = process.wait
+
+            async def slow_wait():
+                wait_started.set()
+                await release.wait()
+                return await real_wait()
+
+            process.wait = slow_wait  # type: ignore[assignment]
+
+            transport._schedule_cleanup(
+                SuperDocError('reader-overflow', code=HOST_PROTOCOL_ERROR),
+            )
+            cleanup_task = transport._cleanup_task
+            assert cleanup_task is not None
+
+            await asyncio.wait_for(wait_started.wait(), timeout=2.0)
+            assert transport.state == 'DISCONNECTED'
+            assert transport._process is None
+            assert not cleanup_task.done()
+
+            dispose_task = asyncio.create_task(transport.dispose())
+            await asyncio.sleep(0.05)
+            assert not dispose_task.done()
+
+            release.set()
+            await dispose_task
+            assert transport.state == 'DISCONNECTED'
+            assert transport._cleanup_task is None
+            await process.wait()
+            assert process.returncode is not None
+        finally:
+            _cleanup_wrapper(cli)
+
     @pytest.mark.asyncio
     async def test_ensure_connected_drains_in_flight_cleanup_before_spawn(self):
         # Round-3 regression: without this drain, `_start_host` reassigns

packages/super-editor/src/editors/v1/core/commands/insertHeadingAt.js

@@ -1,3 +1,5 @@
+import { buildTextWithTabs } from '../../document-api-adapters/helpers/text-with-tabs.js';
+
 /**
  * Insert a heading node at an absolute document position.
  *
@@ -25,13 +27,13 @@ export const insertHeadingAt =
       },
     };
     const normalizedText = typeof text === 'string' ? text : '';
-    const textNode = normalizedText.length > 0 ? state.schema.text(normalizedText) : null;
+    // buildTextWithTabs splits '\t' into real tab nodes so exports emit <w:tab/>.
+    const content = normalizedText.length > 0 ? buildTextWithTabs(state.schema, normalizedText, undefined) : null;
 
     let paragraphNode;
     try {
       paragraphNode =
-        paragraphType.createAndFill(attrs, textNode ?? undefined) ??
-        paragraphType.create(attrs, textNode ? [textNode] : undefined);
+        paragraphType.createAndFill(attrs, content ?? undefined) ?? paragraphType.create(attrs, content ?? undefined);
     } catch {
       return false;
     }

packages/super-editor/src/editors/v1/core/commands/insertHeadingAt.test.js

@@ -145,7 +145,7 @@ describe('insertHeadingAt', () => {
 
     insertHeadingAt({ pos: 0, level: 1, text: 'Hello' })({ state, dispatch });
 
-    expect(state.schema.text).toHaveBeenCalledWith('Hello');
+    expect(state.schema.text).toHaveBeenCalledWith('Hello', undefined);
   });
 
   // --- tracked mode ---

packages/super-editor/src/editors/v1/core/commands/insertListItemAt.js

@@ -1,4 +1,5 @@
 import { getResolvedParagraphProperties } from '@extensions/paragraph/resolvedPropertiesCache.js';
+import { buildTextWithTabs } from '../../document-api-adapters/helpers/text-with-tabs.js';
 
 /**
  * Insert a list-item paragraph before/after a target list paragraph position.
@@ -42,12 +43,12 @@ export const insertListItemAt =
     };
 
     const normalizedText = typeof text === 'string' ? text : '';
-    const textNode = normalizedText.length > 0 ? state.schema.text(normalizedText) : undefined;
+    // buildTextWithTabs splits '\t' into real tab nodes so exports emit <w:tab/>.
+    const content = normalizedText.length > 0 ? buildTextWithTabs(state.schema, normalizedText, undefined) : undefined;
 
     let paragraphNode;
     try {
-      paragraphNode =
-        paragraphType.createAndFill(attrs, textNode) ?? paragraphType.create(attrs, textNode ? [textNode] : undefined);
+      paragraphNode = paragraphType.createAndFill(attrs, content) ?? paragraphType.create(attrs, content ?? undefined);
     } catch {
       return false;
     }

packages/super-editor/src/editors/v1/core/commands/insertListItemAt.test.js

@@ -175,7 +175,7 @@ describe('insertListItemAt', () => {
       dispatch,
     });
 
-    expect(state.schema.text).toHaveBeenCalledWith('New item');
+    expect(state.schema.text).toHaveBeenCalledWith('New item', undefined);
   });
 
   it('does not inherit sdBlockId from the target node when omitted', () => {

packages/super-editor/src/editors/v1/core/commands/insertParagraphAt.js

@@ -1,3 +1,5 @@
+import { buildTextWithTabs } from '../../document-api-adapters/helpers/text-with-tabs.js';
+
 /**
  * Insert a paragraph node at an absolute document position.
  *
@@ -16,13 +18,14 @@ export const insertParagraphAt =
       ...(paraId ? { paraId } : undefined),
     };
     const normalizedText = typeof text === 'string' ? text : '';
-    const textNode = normalizedText.length > 0 ? state.schema.text(normalizedText) : null;
+    // buildTextWithTabs splits '\t' into real tab nodes so exports emit <w:tab/>
+    // instead of a raw tab character inside <w:t>.
+    const content = normalizedText.length > 0 ? buildTextWithTabs(state.schema, normalizedText, undefined) : null;
 
     let paragraphNode;
     try {
       paragraphNode =
-        paragraphType.createAndFill(attrs, textNode ?? undefined) ??
-        paragraphType.create(attrs, textNode ? [textNode] : undefined);
+        paragraphType.createAndFill(attrs, content ?? undefined) ?? paragraphType.create(attrs, content ?? undefined);
     } catch {
       return false;
     }

packages/super-editor/src/editors/v1/core/commands/insertParagraphAt.test.js

@@ -100,7 +100,7 @@ describe('insertParagraphAt', () => {
 
     insertParagraphAt({ pos: 0, text: 'Hello' })({ state, dispatch });
 
-    expect(state.schema.text).toHaveBeenCalledWith('Hello');
+    expect(state.schema.text).toHaveBeenCalledWith('Hello', undefined);
   });
 
   it('sets forceTrackChanges meta when tracked is true', () => {

packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/EditorInputManager.ts

@@ -137,6 +137,11 @@ function isDirectSingleCommentHighlightHit(target: EventTarget | null): boolean
   return getCommentHighlightThreadIds(target).length === 1;
 }
 
+function isDirectTrackedChangeHit(target: EventTarget | null): boolean {
+  if (!(target instanceof Element)) return false;
+  return target.closest(TRACK_CHANGE_SELECTOR) != null;
+}
+
 function resolveTrackChangeThreadId(target: EventTarget | null): string | null {
   if (!(target instanceof Element)) {
     return null;
@@ -258,11 +263,11 @@ function shouldIgnoreRepeatClickOnActiveComment(
     return false;
   }
 
-  // Direct clicks on commented text should place a caret at the clicked
-  // position and let the comments plugin infer the active thread from the
-  // resulting selection. Only preserve the pointerdown short-circuit for
-  // nearby non-text surfaces, such as split-run gaps.
-  if (isDirectSingleCommentHighlightHit(target)) {
+  // Direct clicks on single-thread comment text or tracked-change text should
+  // place a caret at the clicked position and let comment/thread activation be
+  // inferred from the resulting selection. Only preserve the pointerdown
+  // short-circuit for nearby non-text surfaces, such as split-run gaps.
+  if (isDirectSingleCommentHighlightHit(target) || isDirectTrackedChangeHit(target)) {
     return false;
   }
 
@@ -2604,7 +2609,9 @@ export class EditorInputManager {
   }
 
   #handleSingleCommentHighlightClick(event: PointerEvent, target: HTMLElement | null, editor: Editor): boolean {
-    if (isDirectSingleCommentHighlightHit(target)) {
+    // Direct hits on inline annotated text should not be intercepted here.
+    // Let generic click-to-position place the caret at the clicked pixel.
+    if (isDirectSingleCommentHighlightHit(target) || isDirectTrackedChangeHit(target)) {
       return false;
     }
 

packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.activeCommentClick.test.ts

@@ -238,7 +238,7 @@ describe('EditorInputManager - single-thread comment highlight clicks', () => {
     expect(viewportHost.setPointerCapture).toHaveBeenCalled();
   });
 
-  it('activates a tracked-change decoration when it owns the clicked visual surface', () => {
+  it('lets direct clicks on a tracked-change decoration fall through to generic caret placement', () => {
     mockEditor.state.comments$.activeThreadId = 'comment-2';
 
     const trackedChange = document.createElement('span');
@@ -248,12 +248,32 @@ describe('EditorInputManager - single-thread comment highlight clicks', () => {
 
     dispatchPointerDown(trackedChange);
 
-    expect(mockEditor.commands.setCursorById).toHaveBeenCalledWith('change-1', {
-      activeCommentId: 'change-1',
-    });
-    expect(resolvePointerPositionHit).not.toHaveBeenCalled();
-    expect(mockEditor.state.tr.setSelection).not.toHaveBeenCalled();
-    expect(viewportHost.setPointerCapture).not.toHaveBeenCalled();
+    expect(mockEditor.commands.setCursorById).not.toHaveBeenCalled();
+    expect(resolvePointerPositionHit).toHaveBeenCalled();
+    expect(TextSelection.create as unknown as Mock).toHaveBeenCalled();
+    expect(mockEditor.state.tr.setSelection).toHaveBeenCalled();
+    expect(viewportHost.setPointerCapture).toHaveBeenCalled();
+  });
+
+  it('lets repeat direct clicks on the active tracked-change decoration reposition caret', () => {
+    mockEditor.state.comments$.activeThreadId = 'change-1';
+
+    const trackedChange = document.createElement('span');
+    trackedChange.className = 'track-delete-dec highlighted';
+    trackedChange.setAttribute('data-track-change-id', 'change-1');
+    viewportHost.appendChild(trackedChange);
+
+    dispatchPointerDown(trackedChange);
+
+    expect(mockEditor.emit).not.toHaveBeenCalledWith(
+      'commentsUpdate',
+      expect.objectContaining({ activeCommentId: 'change-1' }),
+    );
+    expect(mockEditor.commands.setCursorById).not.toHaveBeenCalled();
+    expect(resolvePointerPositionHit).toHaveBeenCalled();
+    expect(TextSelection.create as unknown as Mock).toHaveBeenCalled();
+    expect(mockEditor.state.tr.setSelection).toHaveBeenCalled();
+    expect(viewportHost.setPointerCapture).toHaveBeenCalled();
   });
 
   it('activates a nearby single-thread highlight when a split-run gap receives the pointer event', () => {