Inkeep OSS Sync

.changeset/external-link-desktop-bridge.md

@@ -0,0 +1,12 @@
+---
+'@inkeep/open-knowledge': patch
+---
+
+External links now reliably open in your OS default browser on desktop.
+Clicking an http(s) link in the visual editor, source mode, or a wiki-link
+chip previously could open another Open Knowledge window rendering the page
+— most visibly in windows created after a server restart. Link clicks now
+route straight through the desktop bridge, and the window-level safety net
+is attached to every editor window (including the server-restart recreate
+path), so external URLs land in your browser everywhere. Web behavior is
+unchanged (links still open in a new tab).

packages/app/src/components/GraphPanel.tsx

@@ -400,6 +400,8 @@ export function GraphPanel({ activeDocName }: { activeDocName: string }) {
                 actionLabel: t`Open link`,
                 secondaryLabel: selectedNode.url,
                 onAction: () => {
+                  // openExternalUrl gates unsafe schemes internally (a node URL can
+                  // carry any authored scheme), then routes to the OS browser / new tab.
                   openExternalUrl(selectedNode.url);
                   setIsExpanded(false);
                 },

packages/app/src/components/GraphView.tsx

@@ -414,6 +414,8 @@ function applyGraphNodeClick({
   const action = resolveGraphNodeClickAction(node, docClickBehavior);
 
   if (action.kind === 'external') {
+    // openExternalUrl gates unsafe schemes internally (a graph node URL can
+    // carry any authored scheme), then routes to the OS browser / new tab.
     openExternalUrl(action.url);
     return;
   }
@@ -1191,6 +1193,8 @@ export function GraphView({
             showPointerCursor={(obj) => Boolean(obj && 'kind' in obj)}
             onNodeClick={(node: NodeObject<GraphNode>) => {
               if (node.kind === 'external') {
+                // openExternalUrl gates unsafe schemes internally (a node URL can
+                // carry any authored scheme), then routes to the OS browser / new tab.
                 openExternalUrl(node.url);
                 return;
               }

packages/app/src/editor/extensions/internal-link.external-open.test.ts

@@ -0,0 +1,175 @@
+/**
+ * RED regression tests for inkeep/open-knowledge#617 — WYSIWYG external-link
+ * chips must reach the OS default browser, not a child OpenKnowledge window.
+ *
+ * Contract under test: activating an external (`http(s)://`) link chip routes
+ * through the desktop bridge (`window.okDesktop.shell.openExternal`) when it is
+ * present, exactly as the graph view does (`openExternalUrl` in
+ * `lib/external-link.ts`), and falls back to `window.open` only on web (no
+ * bridge). Today `internal-link.ts`'s `handlePrimary` `case 'external'` calls
+ * `openHashHrefInNewTab` → `window.open(url, '_blank', …)` UNCONDITIONALLY: on
+ * the Electron desktop that `window.open` becomes a new in-app BrowserWindow
+ * that renders the page (the bug), because it relies on a main-process
+ * `setWindowOpenHandler` net that isn't attached on every window.
+ *
+ * Seam: the REAL `handlePrimary` closure is reached the way the InteractionLayer
+ * reaches it in production — via the layer registration the chip installs
+ * (`getInteractionLayer(editor).getRegistration(id).handlePrimary(...)`). No
+ * mock of the routing decision: a real Editor with the real `InternalLink`
+ * extension classifies a real external link mark and runs the real branch. The
+ * only doubles are the two external boundaries the decision targets —
+ * `window.okDesktop.shell.openExternal` (Electron preload bridge) and
+ * `window.open` (browser new-window API).
+ *
+ * Substrate: jsdom via the shared walk-currency harness (`installDomGlobals`),
+ * the same per-file DOM install the other headless editor-plugin suites use.
+ */
+
+import { afterAll, afterEach, beforeAll, describe, expect, mock, test } from 'bun:test';
+import { Editor } from '@tiptap/core';
+import { TextSelection } from '@tiptap/pm/state';
+import StarterKit from '@tiptap/starter-kit';
+import { getInteractionLayer } from '../interaction-layer-host';
+import { installDomGlobals } from '../walk-currency-test-harness';
+import { InternalLink } from './internal-link';
+import { markIdentityKey } from './mark-identity';
+
+let restoreDomGlobals: (() => void) | null = null;
+
+beforeAll(() => {
+  restoreDomGlobals = installDomGlobals();
+});
+
+afterAll(() => {
+  restoreDomGlobals?.();
+  restoreDomGlobals = null;
+});
+
+type OpenExternalBridge = { shell?: { openExternal?: (url: string) => Promise<void> } };
+
+interface OpenExternalWindow {
+  okDesktop?: OpenExternalBridge;
+  open: (url?: string, target?: string, features?: string) => unknown;
+}
+
+function testWindow(): OpenExternalWindow {
+  return globalThis.window as unknown as OpenExternalWindow;
+}
+
+const liveEditors = new Set<Editor>();
+
+afterEach(() => {
+  for (const editor of liveEditors) editor.destroy();
+  liveEditors.clear();
+  // Drop the injected bridge; restore the jsdom stub `window.open` so a leaked
+  // reference from one case can't satisfy another's assertion.
+  const w = testWindow();
+  delete w.okDesktop;
+});
+
+/**
+ * Mount a real editor whose sole `link` mark is the production `InternalLink`
+ * extension (we do NOT use the `mountLightEditor` rig here: it bundles
+ * `LinkFidelity`, and `InternalLink` extends `LinkFidelity`, so both would
+ * define a `link` mark and collide). Returns an `activate` that invokes the
+ * real chip primary-action closure through the InteractionLayer registration —
+ * the same entry point the layer's click / Enter handler calls.
+ */
+function mountWithExternalLink(url: string): {
+  editor: Editor;
+  activate: (newTab: boolean) => boolean | undefined;
+} {
+  const host = document.createElement('div');
+  document.body.appendChild(host);
+  const editor = new Editor({
+    element: host,
+    content: `<p><a href="${url}">go</a></p>`,
+    extensions: [
+      StarterKit.configure({ link: false }),
+      InternalLink.configure({ docName: 'notes/test' }),
+    ],
+  });
+  liveEditors.add(editor);
+
+  // Force one view update so `markIdentityPlugin`'s view lifecycle fires
+  // `onRegister` → `layer.register(...)`. `state.init` already populated `byId`,
+  // but the registration (which carries `handlePrimary`) lands on the first
+  // `update`. A selection-only transaction (`docChanged === false`) triggers it
+  // without mutating the doc.
+  editor.view.dispatch(editor.state.tr.setSelection(TextSelection.create(editor.state.doc, 1)));
+
+  const idState = markIdentityKey.getState(editor.state);
+  const nodeId = [...(idState?.byId.keys() ?? [])][0];
+  if (nodeId === undefined) {
+    throw new Error('setup: no link mark id — external link was not parsed into a link mark');
+  }
+  const layer = getInteractionLayer(editor);
+  const registration = layer.getRegistration(nodeId);
+  if (!registration?.handlePrimary) {
+    throw new Error('setup: chip did not register a handlePrimary hook with the InteractionLayer');
+  }
+  return {
+    editor,
+    activate: (newTab) => registration.handlePrimary?.({ nodeId, type: 'link', newTab }),
+  };
+}
+
+describe('WYSIWYG external-link activation — desktop (bridge present)', () => {
+  test('bare click routes to the OS browser via okDesktop.shell.openExternal, NOT window.open', () => {
+    const url = 'https://youtube.com/watch?v=abc';
+    const openExternal = mock(async (_url: string) => {});
+    const openWindow = mock(() => null);
+    const w = testWindow();
+    w.okDesktop = { shell: { openExternal } };
+    w.open = openWindow as unknown as OpenExternalWindow['open'];
+
+    const { activate } = mountWithExternalLink(url);
+    const handled = activate(false);
+
+    expect(handled).toBe(true);
+    // RED today: `case 'external'` calls `window.open` unconditionally, so the
+    // desktop bridge is never reached.
+    expect(openExternal).toHaveBeenCalledTimes(1);
+    expect(openExternal).toHaveBeenCalledWith(url);
+    // RED today: the bug IS that a new in-app window opens.
+    expect(openWindow).not.toHaveBeenCalled();
+  });
+
+  test('Cmd/Ctrl+click (new-tab gesture) also reaches the OS browser, NOT a child window', () => {
+    // External URLs must land in the OS browser on EVERY activation — bare or
+    // modifier-click. The modifier path must not fall back to window.open.
+    const url = 'https://example.com/path';
+    const openExternal = mock(async (_url: string) => {});
+    const openWindow = mock(() => null);
+    const w = testWindow();
+    w.okDesktop = { shell: { openExternal } };
+    w.open = openWindow as unknown as OpenExternalWindow['open'];
+
+    const { activate } = mountWithExternalLink(url);
+    activate(true);
+
+    expect(openExternal).toHaveBeenCalledTimes(1);
+    expect(openExternal).toHaveBeenCalledWith(url);
+    expect(openWindow).not.toHaveBeenCalled();
+  });
+});
+
+describe('WYSIWYG external-link activation — web (no bridge)', () => {
+  // Regression pin (green today AND after the fix): on web there is no desktop
+  // bridge, so external links keep the `window.open` new-tab behavior. Guards
+  // the fix from regressing the web path to a no-op.
+  test('bare click falls back to window.open with the new-tab + noopener features', () => {
+    const url = 'https://example.com/web';
+    const openWindow = mock(() => null);
+    const w = testWindow();
+    delete w.okDesktop;
+    w.open = openWindow as unknown as OpenExternalWindow['open'];
+
+    const { activate } = mountWithExternalLink(url);
+    const handled = activate(false);
+
+    expect(handled).toBe(true);
+    expect(openWindow).toHaveBeenCalledTimes(1);
+    expect(openWindow).toHaveBeenCalledWith(url, '_blank', 'noopener,noreferrer');
+  });
+});

packages/app/src/editor/extensions/internal-link.ts

@@ -43,10 +43,10 @@ import {
 } from '@inkeep/open-knowledge-core';
 import { type Editor, mergeAttributes } from '@tiptap/core';
 import { createElement } from 'react';
+import { openExternalUrl } from '@/lib/external-link';
 import { resolveLinkTargetIntent } from '../../components/link-target-intent';
 import {
   activateAssetLink,
-  openHashHrefInNewTab,
   openInternalHashHrefInNewTab,
   toInternalHashHref,
 } from '../internal-link-helpers';
@@ -219,15 +219,18 @@ export const InternalLink = LinkFidelity.extend<InternalLinkOptions>({
           }
           return true;
         case 'external':
-          // Refuse javascript:/data:/etc via scheme allowlist.
-          // Fall through if unsafe so the PropPanel surfaces the URL for
-          // the author to edit. External links ALWAYS open in a new tab —
-          // bare-click on cross-origin content shouldn't navigate away
-          // from the editor (both branches end up calling
-          // `openHashHrefInNewTab`, just routed for symmetry with the
-          // doc/anchor branches above).
+          // External links route through the desktop bridge
+          // (`openExternalUrl`) so a click reaches the OS default browser
+          // instead of Electron's default new-window (which renders the page
+          // in an in-app child BrowserWindow). Symmetric with the graph view;
+          // on web it falls back to `window.open(url, '_blank', …)`.
+          //
+          // `openExternalUrl` refuses unsafe schemes internally, so this gate
+          // is for CONTROL FLOW, not security: an unsafe href returns false so
+          // the chip's primary handler falls through and the PropPanel surfaces
+          // the URL for the author to edit (rather than silently no-opening).
           if (!isSafeNavigationUrl(target.url)) return false;
-          openHashHrefInNewTab(target.url);
+          openExternalUrl(target.url);
           return true;
         default:
           return assertNeverLinkTarget(target);

packages/app/src/editor/extensions/wiki-link.external-open.test.ts

@@ -0,0 +1,128 @@
+/**
+ * Regression pins for inkeep/open-knowledge#617 — the WYSIWYG `[[wikilink]]`
+ * external branch must reach the OS default browser, symmetric with the
+ * markdown-link chip (`internal-link.external-open.test.ts`).
+ *
+ * Contract: activating a `[[https://…]]` chip routes through the desktop bridge
+ * (`window.okDesktop.shell.openExternal`) when present, and falls back to
+ * `window.open` on web (no bridge). The routing code is real — a mounted Editor
+ * with the production `WikiLink` NodeView classifies a real external wiki-link
+ * target and runs the real `handlePrimary` external branch, reached the way the
+ * InteractionLayer reaches it in production
+ * (`getRegistration(nodeId).handlePrimary(...)`). The only doubles are the two
+ * external boundaries the decision targets (`okDesktop.shell.openExternal`,
+ * `window.open`).
+ */
+
+import { afterAll, afterEach, beforeAll, describe, expect, mock, test } from 'bun:test';
+import { Editor } from '@tiptap/core';
+import { TextSelection } from '@tiptap/pm/state';
+import StarterKit from '@tiptap/starter-kit';
+import { getInteractionLayer } from '../interaction-layer-host';
+import { installDomGlobals } from '../walk-currency-test-harness';
+import { WikiLink } from './wiki-link';
+
+let restoreDomGlobals: (() => void) | null = null;
+
+beforeAll(() => {
+  restoreDomGlobals = installDomGlobals();
+});
+
+afterAll(() => {
+  restoreDomGlobals?.();
+  restoreDomGlobals = null;
+});
+
+type OpenExternalBridge = { shell?: { openExternal?: (url: string) => Promise<void> } };
+
+interface OpenExternalWindow {
+  okDesktop?: OpenExternalBridge;
+  open: (url?: string, target?: string, features?: string) => unknown;
+}
+
+function testWindow(): OpenExternalWindow {
+  return globalThis.window as unknown as OpenExternalWindow;
+}
+
+const liveEditors = new Set<Editor>();
+
+afterEach(() => {
+  for (const editor of liveEditors) editor.destroy();
+  liveEditors.clear();
+  delete testWindow().okDesktop;
+});
+
+/**
+ * Mount a real editor with the production `WikiLink` NodeView holding a single
+ * external `[[url]]` chip, and return an `activate` that invokes the real chip
+ * primary-action closure through the InteractionLayer registration.
+ */
+function mountWithExternalWikiLink(url: string): {
+  editor: Editor;
+  activate: (newTab: boolean) => boolean | undefined;
+} {
+  const host = document.createElement('div');
+  document.body.appendChild(host);
+  const editor = new Editor({
+    element: host,
+    content: `<p><span data-wiki-link data-target="${url}"></span></p>`,
+    extensions: [StarterKit, WikiLink.configure({ docName: 'notes/test' })],
+  });
+  liveEditors.add(editor);
+
+  // Force a view update so the NodeView's InteractionLayer registration settles.
+  // Position 1 is inside the paragraph's inline content (before the atom chip).
+  editor.view.dispatch(editor.state.tr.setSelection(TextSelection.create(editor.state.doc, 1)));
+
+  const chip = host.querySelector('[data-node-id]');
+  const nodeId = chip?.getAttribute('data-node-id') ?? undefined;
+  if (nodeId === undefined) {
+    throw new Error(
+      'setup: no wiki-link chip — external target was not parsed into a wikiLink node',
+    );
+  }
+  const registration = getInteractionLayer(editor).getRegistration(nodeId);
+  if (!registration?.handlePrimary) {
+    throw new Error('setup: wiki-link chip did not register a handlePrimary hook');
+  }
+  return {
+    editor,
+    activate: (newTab) => registration.handlePrimary?.({ nodeId, type: 'wikiLink', newTab }),
+  };
+}
+
+describe('WYSIWYG wiki-link external activation — desktop (bridge present)', () => {
+  test('bare click routes to the OS browser via okDesktop.shell.openExternal, NOT window.open', () => {
+    const url = 'https://youtube.com/watch?v=abc';
+    const openExternal = mock(async (_url: string) => {});
+    const openWindow = mock(() => null);
+    const w = testWindow();
+    w.okDesktop = { shell: { openExternal } };
+    w.open = openWindow as unknown as OpenExternalWindow['open'];
+
+    const { activate } = mountWithExternalWikiLink(url);
+    const handled = activate(false);
+
+    expect(handled).toBe(true);
+    expect(openExternal).toHaveBeenCalledTimes(1);
+    expect(openExternal).toHaveBeenCalledWith(url);
+    expect(openWindow).not.toHaveBeenCalled();
+  });
+});
+
+describe('WYSIWYG wiki-link external activation — web (no bridge)', () => {
+  test('bare click falls back to window.open with the new-tab + noopener features', () => {
+    const url = 'https://example.com/web';
+    const openWindow = mock(() => null);
+    const w = testWindow();
+    delete w.okDesktop;
+    w.open = openWindow as unknown as OpenExternalWindow['open'];
+
+    const { activate } = mountWithExternalWikiLink(url);
+    const handled = activate(false);
+
+    expect(handled).toBe(true);
+    expect(openWindow).toHaveBeenCalledTimes(1);
+    expect(openWindow).toHaveBeenCalledWith(url, '_blank', 'noopener,noreferrer');
+  });
+});