Skip to content

Refresh rich Markdown links when document metadata changes - #26595

Merged
OrcaWin merged 2 commits into
mainfrom
nwparker/remote-markdown-link-refresh
Oct 9, 2026
Merged

OrcaWin merged 2 commits into
mainfrom
nwparker/remote-markdown-link-refresh

Conversation

@nwparker

@nwparker nwparker commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​179 $\color{#cf222e}{\Huge{\mathbf{−}}}$​5 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​174
Prod 5 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​67 $\color{#cf222e}{\Huge{\mathbf{−}}}$​37 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​30

ELI5

A link in the rich Markdown editor could look broken even after Orca learned that its remote document exists. Refresh the existing link when the document list changes, so its appearance matches where it can take you.

What Changed

The existing document-link decoration plugin now publishes resolution for rendered link nodes as well as links being typed. Extension storage is typed, with its existing index cache moved to a focused module to keep the extension within the file-size limit. When metadata changes, the changed decoration asks ProseMirror to update the existing link view. Text, selection and the document itself stay intact.

The real Docker conversion regression runs in the existing template-building CI lane. The shared Docker-lane contract is extracted into a separate test file without dropping existing checks.

Why

The link view previously assumed every transaction would call its update method. ProseMirror reuses an unchanged node when its decorations are also unchanged, so a metadata-only transaction left the old missing-link class in place. A decoration carries the changed resolution through the existing rendering mechanism without reparsing the document, moving the cursor or adding a separate listener lifecycle.

Linked Issue

Found while exercising SSH consolidation in #26483. Independent of the other consolidation fixes.

Visual Proof

markdown-link-refresh-before.png
markdown-link-refresh-after.png
markdown-link-refresh-runtime-red.log
markdown-link-refresh-runtime-green.log

The before and after show the same rich link as missing versus resolved. Runtime logs include positive host metadata and the corrected modifier-click opening a sibling document that exists only on that host.

Testing

  • Manually exercised hidden macOS Electron renderers against an owned Linux Docker SSH host with the installed 1.4.223 adhoc server template.
  • Two actual Tiptap NodeView regressions fail before the fix and pass afterward: late document discovery and document removal. They verify unchanged document text, selection and link DOM identity where applicable.
  • 109 tests / 12 files pass; one existing test is skipped. Includes document-link parsing, keyboard handling, search, metadata synchronization and CI routing contracts.
  • Original real conversion keeps the rich link missing for ten seconds even though the host lists its target. Corrected real conversion resolves it and modifier-click opens the host-only sibling (final follow-up run: 9.7 s).
  • Full pnpm tc, pnpm lint, changed-code quality, affected-file lint and fresh E2E build pass.

Review

Existing CSS classes and design tokens are unchanged. Resolution is decoration state, not saved document content. The existing metadata transaction triggers the refresh; no added network requests, polling or effects.

Agent skill upstream boundary

  • Not applicable; no upstream skill-installer content copied.

Notes

No RPC or persisted schema change. The same rendering path handles local, remote and folder-workspace document lists. Modifier-click validation uses the platform’s shortcut modifier. Runtime checks cover macOS client/Linux SSH host; no Windows runtime claim.

The independent baseline also logs the wrong-host Markdown metadata call fixed by #26590 and contains the duplicate file records fixed by #26593. This PR fixes link resolution refresh; it does not claim either of those fixes or suppress unrelated reconnect logs.

Checklist

  • Focused change with the mechanism and rationale explained
  • Before/after screenshots and runtime logs attached
  • Self-reviewed for correctness, security and performance
  • Cross-platform, SSH, folder workspaces and older peers considered
  • Required local checks and runtime regression pass

@OrcaWin
OrcaWin force-pushed the nwparker/remote-markdown-link-refresh branch from 101c15b to 9dcd07e Compare October 9, 2026 02:22
@OrcaWin
OrcaWin changed the base branch from OrcaWin/phase35 to nwparker/remote-markdown-conversion-testing October 9, 2026 02:22
@OrcaWin
OrcaWin force-pushed the nwparker/remote-markdown-conversion-testing branch from 2bf93f3 to ffb4496 Compare October 9, 2026 05:04
@OrcaWin
OrcaWin force-pushed the nwparker/remote-markdown-link-refresh branch from 9dcd07e to e5123f3 Compare October 9, 2026 05:06
@OrcaWin
OrcaWin force-pushed the nwparker/remote-markdown-conversion-testing branch from ffb4496 to 2e95086 Compare October 9, 2026 05:28
@OrcaWin
OrcaWin force-pushed the nwparker/remote-markdown-link-refresh branch from e5123f3 to 8636c57 Compare October 9, 2026 05:30
@OrcaWin
OrcaWin changed the base branch from nwparker/remote-markdown-conversion-testing to main October 9, 2026 05:45
@OrcaWin
OrcaWin force-pushed the nwparker/remote-markdown-link-refresh branch from 8636c57 to b99c90a Compare October 9, 2026 05:45
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

📝 Walkthrough

Walkthrough

The rich Markdown link extension now caches a document index and adds resolution decorations to links. Unit tests cover changes to link resolution while preserving document content and link labels. A Docker-gated E2E test covers retained links in a restored remote worktree and navigation to a sibling document. CI source routing, job selection, and workflow configuration direct this test to a dedicated Docker lane.

Priority: ⬇️ Low

Merge Risk: 🔵 Low · up to b99c9

The current change is mergeable with a bounded CI coverage gap: future changes limited to the link cache will not automatically run its dedicated E2E test. Add the storage module to the route and routing test.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 7 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: refreshing rich Markdown links when document metadata changes.
Description check Passed The description is complete and focused. It explains the user impact, implementation, rationale, linked issue reference, visual proof, testing, platform coverage, review notes, and checklist status. T…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 7 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d4c0dfa3-69de-4ad7-9b11-f7f0d5555a68
📥 Commits

Reviewing files that changed from the base of the PR and between 45a969f and b99c90a.

📒 Files selected for processing (8)
  • .github/workflows/e2e.yml
  • config/scripts/ci-e2e-job-selection.mjs
  • config/scripts/ci-e2e-job-selection.test.mjs
  • config/scripts/pr-e2e-source-routing.mjs
  • src/renderer/src/components/editor/rich-markdown-doc-link-resolution.test.ts
  • src/renderer/src/components/editor/rich-markdown-doc-link-storage.ts
  • src/renderer/src/components/editor/rich-markdown-doc-link.ts
  • tests/e2e/ssh-orcad-markdown-link-refresh.spec.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

specs: ['tests/e2e/ssh-orcad-markdown-link-refresh.spec.ts'],
matches: (file) =>
isProductSource(file) &&
/^src\/renderer\/src\/components\/editor\/(?:rich-markdown-doc-link|useRichMarkdownProgrammaticSync)\.ts$/.test(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Route storage changes to the link-refresh spec.

The new rich-markdown-doc-link-storage.ts module owns the document-index cache, but this pattern does not match it. If a later PR changes only that module, this route will not select the dedicated link-refresh E2E spec. Add the storage module to the pattern and its routing test.

@OrcaWin
OrcaWin merged commit d2d2d39 into main Oct 9, 2026
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants