Skip to content

Improve markdown viewer rendering and selection - #3664

Merged
lawrencecchen merged 7 commits into
manaflow-ai:mainfrom
tobi:markdown-webview-renderer
May 13, 2026
Merged

lawrencecchen merged 7 commits into
manaflow-ai:mainfrom
tobi:markdown-webview-renderer

Conversation

@tobi

@tobi tobi commented May 6, 2026 •

Copy link
Copy Markdown
Contributor

I use cmux's markdown viewer quite a bit, and the main annoyance was that the previous SwiftUI/MarkdownUI renderer could never select more than one paragraph at a time. This moves markdown rendering to a WebKit-backed viewer so selecting and copying real document content works naturally across headings, paragraphs, tables, code blocks, diagrams, and charts.

What changed

  • Replaced the old SwiftUI markdown rendering path with a WKWebView renderer so text selection works across the full document.
  • Bundled GitHub-style markdown rendering assets locally:
    • marked.js
    • github-markdown-css
    • highlight.js with GitHub light/dark themes
  • Added toolbar actions for copying the source markdown and the currently rendered HTML.
  • Added lazy-loaded local support for fenced Mermaid, Vega, and Vega-Lite blocks without CDN/runtime network dependencies.
  • Routed normal HTTP/HTTPS links into new cmux browser tabs instead of opening the system browser by default.
  • Added local markdown file resolution for relative markdown links and path-like inline code spans, opening resolved files as cmux markdown tabs.
  • Kept same-document fragment links inside the WebView so heading anchors scroll in-place.
  • Sanitized rendered HTML before insertion so raw markdown HTML cannot run active content inside the viewer bridge.
  • Preserved first-click pane focus with a small MarkdownWebView subclass instead of a full-panel pointer overlay that interfered with WebKit interactions.
  • Strengthened markdown live reload by keeping a directory watcher attached when a file is missing and reattaching when it reappears.

Validation

  • Ran ./scripts/reload.sh --tag markdown-webview-review successfully.
  • Ran ./scripts/reload.sh --tag markdown-webview-review --launch successfully.
  • Manually opened markdown in the tagged app and verified rendered selection/viewing behavior, Mermaid, Vega-Lite, tables, task lists, and code blocks.

Screenshot

cmux-markdown

Notes

  • Local unit tests were not run, following the repository policy to avoid local test runs.
  • The screenshot is intentionally not committed to the repo; attach ~/Desktop/cmux-markdown.png to this PR in GitHub if a rendered preview should be visible inline.

Summary by CodeRabbit

  • New Features

    • WebKit-based Markdown viewer with GitHub-style light/dark themes, improved syntax highlighting, Mermaid and Vega-Lite support (lazy-loaded), enhanced frontmatter display, and toolbar actions: "Copy as Markdown" and "Copy as HTML" with temporary confirmation.
  • Bug Fixes

    • More reliable live file-change detection and watcher recovery to keep content in sync.
  • Localization

    • Added English and Japanese strings for copy actions.
  • Tests & Docs

    • Updated web-view tests and added bundled web-asset license documentation.

Review Change Stack

@vercel

vercel Bot commented May 6, 2026

Copy link
Copy Markdown

@tobi is attempting to deploy a commit to the Manaflow Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented May 6, 2026 •

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Replaces the SwiftUI Markdown renderer with a WKWebView-based viewer using bundled marked.js/highlight.js and GitHub CSS; adds shell HTML/sanitizer, asset loader, link resolver, dual file/directory watching, Xcode wiring, localizations, tests, and license documentation.

Changes

Markdown Web Viewer

Layer / File(s) Summary
Xcode project wiring and resource registration
GhosttyTabs.xcodeproj/project.pbxproj
Project updated to include Resources/markdown-viewer and compile the new Markdown viewer Swift sources into GhosttyTabs.
Localizations and licenses
Resources/Localizable.xcstrings, THIRD_PARTY_LICENSES.md
Adds four copy-related localized strings (EN/JA) and documents bundled markdown viewer third-party licenses.
Markdown CSS and highlight themes
Resources/markdown-viewer/github-markdown.css, highlight-github.css, highlight-github-dark.css
Adds GitHub-styled markdown CSS and light/dark highlight.js theme files covering typography, layout, and Prism-style token mappings.
Markdown viewer shell (HTML/JS)
Resources/markdown-viewer/shell.html
Implements the viewer shell with marked.js/highlight.js rendering, DOM sanitization, frontmatter handling, special-block (mermaid/vega) lazy rendering, and JS<->Swift bridge APIs for lib loading and file resolution.
Assets loader
Sources/Panels/MarkdownViewerAssets.swift
Adds an @MainActor singleton that loads and caches bundled web assets and provides memoized lazy asset accessors.
Web renderer & bridge
Sources/Panels/MarkdownWebRenderer.swift
Implements WKWebView wrapper and Coordinator to load shell HTML, push markdown, handle JS messages (lazy libs, file resolution), intercept navigation, and expose rendered HTML/text accessors.
Panel UI & copy toolbar
Sources/Panels/MarkdownPanelView.swift
Replaces MarkdownUI rendering with MarkdownWebRenderer, adds copy-as-Markdown/HTML toolbar actions and confirmation badge, and forwards pointer-down via a MarkdownWebView subclass.
File/directory watching
Sources/Panels/MarkdownPanel.swift
Refactors file-watching to maintain both a file DispatchSource and a directory fallback watcher, simplifying lifecycle and reattach behavior.
Link resolver & tests
Sources/Panels/MarkdownPanelFileLinkResolver.swift, cmuxTests/*
Adds MarkdownPanelFileLinkResolver for markdown-path heuristics and resolution; updates tests to validate resolver behavior and first-click focus, removes legacy pointer-observer tests.

Sequence Diagram

sequenceDiagram
    participant User
    participant Panel as MarkdownPanel
    participant Watcher as File/Dir Watcher
    participant Renderer as MarkdownWebRenderer
    participant WebView as WKWebView_JS
    participant Assets as MarkdownViewerAssets

    User->>Panel: Open markdown file
    Panel->>Watcher: startFileWatcher()
    Watcher-->>Panel: file descriptor opened or directory fallback

    Panel->>Renderer: update(markdown,isDark,filePath)
    Renderer->>Assets: shellHTML() / lazyAsset(...)
    Assets-->>Renderer: return cached assets

    Renderer->>WebView: inject shell HTML + assets and push markdown
    WebView->>WebView: execute marked.js render and highlight.js
    WebView-->>Renderer: rendered HTML/text ready
    Renderer-->>Panel: provide rendered HTML/text for copy
    Panel->>User: show confirmation badge

    Watcher->>Panel: detect file change
    Panel->>Renderer: update content -> re-render
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

🐰 I hop through CSS and shell so bright,

marked and highlight render each byte,
a watcher listens while assets take flight,
links resolve and JS hums at night,
copy as HTML or text — delight!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.81% which is insufficient. The required threshold is 80.00%. 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 accurately describes the main change: replacing the markdown viewer to improve rendering and enable text selection across document elements.
Description check ✅ Passed The PR description is comprehensive and well-structured, covering what changed, why, validation steps, and includes a screenshot demonstrating the improvements.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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 and usage tips.

@tobi
tobi force-pushed the markdown-webview-renderer branch from 1dd2f3f to 8a9d8d6 Compare May 6, 2026 21:19
@tobi
tobi marked this pull request as ready for review May 6, 2026 21:21
@greptile-apps

greptile-apps Bot commented May 6, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR replaces the previous SwiftUI/MarkdownUI renderer with a WKWebView-backed viewer so that text selection works continuously across the full document. The old single-responsibility bloat in MarkdownPanelView.swift has been cleanly split into MarkdownWebRenderer.swift, MarkdownViewerAssets.swift, and MarkdownPanelFileLinkResolver.swift.

  • Bundled GitHub-style CSS, marked.js, highlight.js, and lazy-loaded Mermaid/Vega-Lite assets replace the old MarkdownUI path; link routing, same-document anchor navigation, and local markdown file links are handled via a WKScriptMessageHandler bridge.
  • HTML sanitization (sanitizeRenderedHTML) blocks <script>, <iframe>, on* handlers, and unsafe URL schemes before content is inserted into the DOM; Mermaid uses securityLevel: 'strict' and Vega specs are scanned for external URL references before rendering.
  • TextEdit mode is added to MarkdownPanel with live file-watching improvements (directory-watcher fallback + reattachment), a displayMode toggle, and isDirty/isSaving propagation to the tab bar.

Confidence Score: 4/5

Safe to merge with the asyncAfter animation issue addressed; the WebKit bridge, sanitizer, and file-watcher logic are well-constructed.

The triggerFocusFlashAnimation function in MarkdownPanelView.swift still uses DispatchQueue.main.asyncAfter to sequence animation segments, flagged in an earlier review round and not yet addressed. Every other previously-flagged item — the WKUserContentController retain leak, the completion-handler copy-as-HTML path, the NSLock on MarkdownViewerAssets, and the loadShell double-render race — has been resolved cleanly. The new file-watching, HTML sanitization, and WebKit bridge code are all solid.

Sources/Panels/MarkdownPanelView.swift — triggerFocusFlashAnimation still schedules animation segments with DispatchQueue.main.asyncAfter rather than Task/Task.sleep.

Important Files Changed

Filename Overview
Sources/Panels/MarkdownWebRenderer.swift New 592-line file containing the WKWebView NSViewRepresentable bridge, Coordinator, navigation delegate, script-message handler, link routing, and NSColor theme helpers. dismantleNSView correctly cleans up the WKUserContentController script-message handler.
Sources/Panels/MarkdownPanelView.swift Refactored from ~1570 lines to 334 lines; responsibilities split into separate files. flashCopyConfirmation is correctly migrated to Task/Task.sleep. triggerFocusFlashAnimation still uses DispatchQueue.main.asyncAfter (flagged in an earlier review round, still unaddressed).
Sources/Panels/MarkdownPanel.swift Model expanded with TextEdit mode, isDirty/isSaving tracking, and improved file-watcher logic including directory-watcher fallback and re-attachment on file reappearance.
Sources/Panels/MarkdownViewerAssets.swift New @mainactor singleton loading bundled markdown viewer assets with zlib decompression support. shellHTML(isDark:) accepts but silences its isDark parameter; the actual theme switching is handled client-side by CSS media queries.
Resources/markdown-viewer/shell.html 999-line self-contained HTML shell. sanitizeRenderedHTML correctly strips script, iframe, on* handlers, style attributes, and unsafe src/href URLs. Mermaid uses securityLevel:'strict'. Vega external-URL validation covers the main data.url loading paths.
Sources/Workspace.swift Adds splitPaneWithMarkdown, openDroppedFileSurfaces, and splitPaneWithDroppedFile helpers so markdown files dropped onto panes open as markdown tabs. installMarkdownPanelSubscription now combines displayTitle and isDirty to propagate dirty state to the tab bar.
Sources/Panels/PanelContentView.swift Adds reusable PanelFilePathHeader and PanelHeaderIconButton shared components, eliminating duplicated header layout between MarkdownPanelView and FilePreviewPanelView.
Sources/Panels/MarkdownPanelFileLinkResolver.swift Clean 58-line resolver: markdown extension detection and relative-path resolution with PWD fallback. Correctly strips fragment and query components before resolution.

Sequence Diagram

sequenceDiagram
    participant SwiftUI as MarkdownPanelView
    participant Renderer as MarkdownWebRenderer
    participant Coord as Coordinator
    participant WV as MarkdownWebView
    participant JS as shell.html JS

    SwiftUI->>Renderer: makeNSView()
    Renderer->>Coord: loadShell(theme:, initialMarkdown:)
    Coord->>WV: loadHTMLString(shellHTML, baseURL: filePath)
    WV-->>Coord: didFinish (WKNavigationDelegate)
    Coord->>WV: evaluateJavaScript(renderMarkdownScript)
    WV->>JS: __cmuxRenderMarkdown(md)
    JS->>JS: sanitizeRenderedHTML(marked.parse(body))
    JS->>JS: "contentEl.innerHTML = sanitizedHTML"

    alt Mermaid/Vega blocks present
        JS-->>Coord: postMessage lib mermaid
        Coord->>WV: evaluateJavaScript(mermaidSource)
        WV->>JS: __cmuxLibLoaded mermaid
        JS->>JS: "mermaid.render then el.innerHTML = svg"
    end

    SwiftUI->>Renderer: updateNSView content changed
    Renderer->>Coord: update(markdown:, theme:)
    Coord->>WV: evaluateJavaScript(renderMarkdownScript)

    alt User clicks local .md link
        JS-->>Coord: postMessage openMarkdownFile
        Coord->>Coord: openMarkdownFile(resolved)
        Coord->>SwiftUI: workspace.newMarkdownSurface
    end

    SwiftUI->>WV: dismantleNSView
    WV->>Coord: removeScriptMessageHandler cmuxLib
Loading

Reviews (6): Last reviewed commit: "merge: sync markdown viewer with main" | Re-trigger Greptile

Comment thread Sources/Panels/MarkdownPanelView.swift
Comment thread Sources/Panels/MarkdownPanelView.swift
Comment thread Sources/Panels/MarkdownPanelView.swift Outdated
Comment thread Sources/Panels/MarkdownPanelView.swift Outdated
Comment thread Sources/Panels/MarkdownPanelView.swift
coderabbitai[bot]
coderabbitai Bot previously requested changes May 6, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Sources/Panels/MarkdownPanel.swift`:
- Around line 100-103: When open(filePath) or open(directoryPath) fails and fd <
0, don't give up: implement a fallback that walks up the path to the nearest
existing ancestor and tries to open and watch that directory (use the same
open(...) codepath but on parent paths) and if no ancestor exists schedule a
retry/backoff to attempt reopening the original file/directory; update the guard
branch around fd (and the similar logic at lines 146-150) to call that
ancestor-fallback-and-retry routine instead of only calling
startDirectoryWatcher(), ensuring startDirectoryWatcher() is still used when
appropriate and preserving the existing watcher setup once a valid ancestor fd
is obtained.

In `@Sources/Panels/MarkdownPanelView.swift`:
- Around line 159-168: In copyAsHTML(), stop overwriting the pasteboard with an
empty or raw-HTML string when the renderer hasn't produced HTML; instead bail
out early if HTML is nil and do not clear the clipboard, and when HTML is
present write the HTML to .html and a plain-text rendering to .string (either by
calling a plain-text helper such as renderer.requestRenderedText or by deriving
plain text from the HTML) so plain-text targets receive readable content; only
call flashCopyConfirmation(.html) after successfully setting both pasteboard
types.
- Around line 1396-1398: The Vega charts are being rendered with renderer:
'canvas', which loses content when window.__cmuxRenderedHTML() serializes
innerHTML; update the vegaEmbed invocation (the opts object passed to vegaEmbed)
so that when producing HTML for copy/export you use renderer: 'svg' (e.g., set
opts.renderer = 'svg' when exporting) or, if you must keep canvas for
interactive display, add a conversion step after vegaEmbed resolves that finds
any canvas elements produced by vegaEmbed and replaces them with <img> elements
whose src is the canvas.toDataURL() before window.__cmuxRenderedHTML() is
called; reference the opts object and vegaEmbed call to implement the change.
- Around line 1393-1398: The code currently passes the parsed Vega/Vega-Lite
spec (spec from JSON.parse(raw)) straight into vegaEmbed(el, spec, opts), which
can trigger network fetches from data.url, top-level datasets, or image mark
URLs; before calling vegaEmbed, validate and sanitize the spec by recursively
inspecting known fields (data entries, top-level datasets, mark/image url
properties, and any nested objects) and either remove/reject any external URL
references or reject the spec entirely if external resources are present; ensure
only inline datasets with "values" are allowed, log or surface a clear error
when a URL is found, and only call vegaEmbed(el, spec, opts) when the sanitized
spec contains no external URL fields.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8c70c901-c155-41ce-b934-de262efab841

📥 Commits

Reviewing files that changed from the base of the PR and between 0804896 and 8a9d8d6.

⛔ Files ignored due to path filters (6)
  • Resources/markdown-viewer/highlight.min.js is excluded by !**/*.min.js
  • Resources/markdown-viewer/marked.min.js is excluded by !**/*.min.js
  • Resources/markdown-viewer/mermaid.min.js is excluded by !**/*.min.js
  • Resources/markdown-viewer/vega-embed.min.js is excluded by !**/*.min.js
  • Resources/markdown-viewer/vega-lite.min.js is excluded by !**/*.min.js
  • Resources/markdown-viewer/vega.min.js is excluded by !**/*.min.js
📒 Files selected for processing (10)
  • GhosttyTabs.xcodeproj/project.pbxproj
  • Resources/Localizable.xcstrings
  • Resources/markdown-viewer/github-markdown.css
  • Resources/markdown-viewer/highlight-github-dark.css
  • Resources/markdown-viewer/highlight-github.css
  • Sources/Panels/MarkdownPanel.swift
  • Sources/Panels/MarkdownPanelView.swift
  • cmuxTests/InactivePaneFirstClickFocusTests.swift
  • cmuxTests/SessionPersistenceTests.swift
  • cmuxTests/WindowAndDragTests.swift
💤 Files with no reviewable changes (1)
  • cmuxTests/WindowAndDragTests.swift

Comment thread Sources/Panels/MarkdownPanel.swift
Comment thread Sources/Panels/MarkdownPanelView.swift
Comment thread Sources/Panels/MarkdownPanelView.swift Outdated
Comment thread Sources/Panels/MarkdownPanelView.swift Outdated
coderabbitai[bot]
coderabbitai Bot previously requested changes May 7, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@GhosttyTabs.xcodeproj/project.pbxproj`:
- Line 330: The new markdown-viewer resource bundle (referenced as
markdown-viewer / PBXBuildFile entry A9D9000000000000000F0017) includes vendored
libraries (highlight.js, marked.js, mermaid.js, vega, vega-lite and their CSS)
that are not yet listed in THIRD_PARTY_LICENSES.md; before committing the
resource to the project, add entries for each library to THIRD_PARTY_LICENSES.md
with the library name, version, license type, copyright statement and a link to
the original source, and ensure the build/bundling step that includes
markdown-viewer only proceeds after that file is updated.

In `@Resources/markdown-viewer/shell.html`:
- Around line 597-608: applyResolvedMarkdownFile currently injects absolute
filesystem paths into the DOM (el.setAttribute('title', 'Open markdown file: ' +
result.path)) and marks elements with data-cmux-* attributes which are later
exported by __cmuxRenderedHTML; remove the title assignment in
applyResolvedMarkdownFile (stop writing result.path into the element title) and
update __cmuxRenderedHTML to clone the rendered DOM and strip any attributes
matching /^data-cmux-/ and any bridge-only attributes (and remove title if it
was set) before returning the HTML so exported/copied markup contains no local
paths or cmux metadata.

In `@Sources/Panels/MarkdownPanel.swift`:
- Around line 228-237: The current scheduleWatcherRetry() uses
DispatchQueue.main.asyncAfter to retry after 1 second; replace this time-based
retry with an event-driven fallback: remove the asyncAfter timer and instead
register a cancellation-aware watcher/observer that triggers startFileWatcher()
when a real filesystem/owner event or notification occurs (e.g. a DispatchSource
monitoring the parent directory/file descriptor, a FileWatcher delegate
callback, or NotificationCenter file-change event). Keep the same guard logic
around watcherRetryWorkItem, isClosed and clearing watcherRetryWorkItem, but
change watcherRetryWorkItem to represent a subscription/observer token (and
cancel it appropriately) and invoke startFileWatcher() only when the
filesystem/event callback fires. Ensure the new subscription is cleaned up on
close and is cancellation-aware.

In `@Sources/Panels/MarkdownPanelView.swift`:
- Around line 432-448: When themeChanged but content hasn't changed, also
trigger re-rendering of Mermaid/Vega blocks: after calling
window.__cmuxApplyTheme in the themeChanged branch (where mermaidInitialized is
reset), execute the page-level reprocessing that postProcessSpecialBlocks
performs (or clear each block's data-rendered attribute) so Mermaid/Vega are
re-rendered with the new palette; i.e., in the same place where you call
webView.evaluateJavaScript(js...), extend the JS to either call the page
function that runs postProcessSpecialBlocks() or remove data-rendered="1" on
mermaid/vega nodes so postProcessSpecialBlocks() will re-run rendering with the
updated theme.

In `@Sources/Panels/MarkdownViewerAssets.swift`:
- Around line 49-63: loadAsset currently returns an empty string when an asset
is missing which hides failures; instead fail fast with a clear error. Replace
the final return "" with a fatalError (or preconditionFailure) that includes the
asset name and extension (use the existing loadAsset(name:ext:) function and the
name/ext parameters) so missing core assets like shell.html or marked.min.js
cause an explicit crash with a descriptive message; if you prefer a throwable
API, change loadAsset's signature to throw and propagate that error through
callers (update call sites that use loadAsset accordingly).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1746181d-29b2-4cdf-b6db-b924966aa69c

📥 Commits

Reviewing files that changed from the base of the PR and between 8a9d8d6 and 354fff6.

📒 Files selected for processing (6)
  • GhosttyTabs.xcodeproj/project.pbxproj
  • Resources/markdown-viewer/shell.html
  • Sources/Panels/MarkdownPanel.swift
  • Sources/Panels/MarkdownPanelFileLinkResolver.swift
  • Sources/Panels/MarkdownPanelView.swift
  • Sources/Panels/MarkdownViewerAssets.swift

Comment thread GhosttyTabs.xcodeproj/project.pbxproj
Comment thread Resources/markdown-viewer/shell.html
Comment thread Sources/Panels/MarkdownPanel.swift Outdated
Comment thread Sources/Panels/MarkdownPanelView.swift Outdated
Comment thread Sources/Panels/MarkdownViewerAssets.swift Outdated
coderabbitai[bot]
coderabbitai Bot previously requested changes May 7, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Resources/markdown-viewer/shell.html`:
- Around line 426-432: The heading() renderer currently builds a slug (slug) but
reuses the same id for duplicate headings; modify heading(text, level, raw) to
track slugs per-render (e.g., a slugCounts map or slugger object created/reset
at the start of each render) and, when a generated slug already exists, append a
numeric suffix like -1, -2, ... to make each id unique; ensure the counter is
incremented for each duplicate and that the slugCounts are reset for every new
render so IDs match GitHub-style anchor behavior.
- Around line 504-535: The function firstVegaExternalReference currently flags
any descendant key named "url"/"href"/"src" as external; change it to only treat
those keys as external when they occur in schema locations that actually load
external resources (e.g., under a data spec). Update the check inside
firstVegaExternalReference (the block that looks at lower === 'url' || 'href' ||
'src') to verify the full childPath against an allowlist/regex of valid
external-resource locations (for example require childPath to start with or
contain a top-level "data" segment or other known loader segments) instead of
flagging every occurrence; implement this by computing and testing childPath (or
splitting path into segments) and only returning childPath when it matches the
allowed patterns. Ensure you keep existing behavior for arrays and recursion
(firstVegaExternalReference) unchanged.

In `@Sources/Panels/MarkdownPanelView.swift`:
- Around line 173-179: The flashCopyConfirmation(_:) timer can let an older
asyncAfter clear a newer confirmation; update flashCopyConfirmation to increment
and store a generation/token (e.g., focusFlashAnimationGeneration) each time you
set copyConfirmation = kind, capture that token in the
DispatchQueue.main.asyncAfter closure, and only set copyConfirmation = nil if
the captured token still matches the current token—use the existing function
name flashCopyConfirmation(_:) and variables copyConfirmation and the new
focusFlashAnimationGeneration token to locate and implement the change.

In `@THIRD_PARTY_LICENSES.md`:
- Around line 209-217: THIRD_PARTY_LICENSES.md lists "github-markdown-css" and
"mermaid.min.js" as “bundled” without concrete versions; update the Mermaid and
github-markdown-css entries to pin exact versions or immutable commit SHAs
(e.g., npm package versions or GitHub commit SHA) and include the precise source
URL for that version, keeping the existing license and copyright text;
specifically edit the Mermaid block and the github-markdown-css block in
THIRD_PARTY_LICENSES.md to replace "bundled" with the exact version/sha and
source link so auditing and vulnerability tracking can reference an immutable
artifact.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: d0c36424-c8cc-4f00-8071-d5d234ff6fdc

📥 Commits

Reviewing files that changed from the base of the PR and between 354fff6 and 337f3d0.

📒 Files selected for processing (7)
  • GhosttyTabs.xcodeproj/project.pbxproj
  • Resources/markdown-viewer/shell.html
  • Sources/Panels/MarkdownPanel.swift
  • Sources/Panels/MarkdownPanelView.swift
  • Sources/Panels/MarkdownViewerAssets.swift
  • Sources/Panels/MarkdownWebRenderer.swift
  • THIRD_PARTY_LICENSES.md

Comment thread Resources/markdown-viewer/shell.html
Comment thread Resources/markdown-viewer/shell.html
Comment thread Sources/Panels/MarkdownPanelView.swift
Comment thread THIRD_PARTY_LICENSES.md Outdated
@tobi

tobi commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

All review threads are resolved. Please re-review the latest head (4674dc2).

@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown

@tobi: Understood — all threads are resolved. I'll kick off a fresh review of the latest head (4674dc29) now.

✅ Actions performed

Full review triggered.

coderabbitai[bot]
coderabbitai Bot previously requested changes May 12, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Sources/Panels/MarkdownPanel.swift`:
- Around line 149-150: The early-return in the directory watcher setup that
checks "if directoryWatchPath == directoryPath, directoryWatchSource != nil {
return }" can leave a stale inode watched after a delete/rename; change the
logic so you only short-circuit when the existing watcher is still valid by
inspecting its event flags, i.e., query the current directoryWatchSource (or the
source passed into the directory watcher handler) for data flags and if it
contains .delete or .rename (inode replacement) then do not return but cancel
and recreate the watcher; otherwise you may keep returning early. Update the
setup/handler around directoryWatchPath, directoryPath and directoryWatchSource
to explicitly cancel and reattach the dispatch_source if .delete or .rename is
observed, and only return early when no such flags are present.

In `@Sources/Panels/MarkdownWebRenderer.swift`:
- Around line 268-313: The comments claim per-source script tags are appended
and state resets on theme switch, but the code in handleLibRequest instead
concatenates sources and runs a single webView.evaluateJavaScript call and
loadShell is not invoked on theme changes; fix by implementing the documented
behavior: in handleLibRequest build a JS snippet that iterates over the sources
array and for each creates a script element (setting either src or textContent
depending on whether assets.lazyAsset returns a URL or code), assigns onerror to
remove the lib from requestedLibs via a callback (so the Swift side can retry)
or at least logs failure, appends each script to document.head, and then calls
window.__cmuxLibLoaded(lib) after all scripts are appended; update the comment
above handleLibRequest to accurately describe this injection behavior and remove
the incorrect claim about theme-based reset (requestedLibs.reset happens only in
loadShell).

In `@THIRD_PARTY_LICENSES.md`:
- Line 198: The listed third-party entries for marked, highlight.js, Vega,
Vega-Lite, and Vega-Embed use generic repository URLs; update each Source URL to
an immutable, version-specific link (e.g., a release tarball or GitHub
tree/tag/commit URL) that matches the exact bundled version used in the project
so auditability is preserved—locate the entries for "marked", "highlight.js",
"Vega", "Vega-Lite", and "Vega-Embed" in the THIRD_PARTY_LICENSES.md and replace
their generic https://github.com/... links with the corresponding release/tag
URLs (or commit hashes) for the exact versions shipped.
- Around line 193-240: Update the bundled library entries in
THIRD_PARTY_LICENSES.md for marked, mermaid, vega, vega-lite, and vega-embed to
their current latest released versions and corresponding
license/copyright/source URLs; specifically locate the sections titled "marked",
"Mermaid", "Vega", "Vega-Lite", and "Vega-Embed" and replace the Version,
Copyright, and Source fields with the upstream project’s latest metadata, then
run your dependency manager (package.json / yarn.lock / pnpm-lock.yaml) to bump
the packages to those versions and run tests/build to catch breaking changes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b49f1fb7-1f3d-48fe-bd12-7c26bf3262a3

📥 Commits

Reviewing files that changed from the base of the PR and between 0804896 and 4674dc2.

⛔ Files ignored due to path filters (6)
  • Resources/markdown-viewer/highlight.min.js is excluded by !**/*.min.js
  • Resources/markdown-viewer/marked.min.js is excluded by !**/*.min.js
  • Resources/markdown-viewer/mermaid.min.js is excluded by !**/*.min.js
  • Resources/markdown-viewer/vega-embed.min.js is excluded by !**/*.min.js
  • Resources/markdown-viewer/vega-lite.min.js is excluded by !**/*.min.js
  • Resources/markdown-viewer/vega.min.js is excluded by !**/*.min.js
📒 Files selected for processing (15)
  • GhosttyTabs.xcodeproj/project.pbxproj
  • Resources/Localizable.xcstrings
  • Resources/markdown-viewer/github-markdown.css
  • Resources/markdown-viewer/highlight-github-dark.css
  • Resources/markdown-viewer/highlight-github.css
  • Resources/markdown-viewer/shell.html
  • Sources/Panels/MarkdownPanel.swift
  • Sources/Panels/MarkdownPanelFileLinkResolver.swift
  • Sources/Panels/MarkdownPanelView.swift
  • Sources/Panels/MarkdownViewerAssets.swift
  • Sources/Panels/MarkdownWebRenderer.swift
  • THIRD_PARTY_LICENSES.md
  • cmuxTests/InactivePaneFirstClickFocusTests.swift
  • cmuxTests/SessionPersistenceTests.swift
  • cmuxTests/WindowAndDragTests.swift
💤 Files with no reviewable changes (1)
  • cmuxTests/WindowAndDragTests.swift

Comment thread Sources/Panels/MarkdownPanel.swift
Comment thread Sources/Panels/MarkdownWebRenderer.swift
Comment thread THIRD_PARTY_LICENSES.md Outdated
Comment on lines +193 to +240
### marked

- **Version:** 13.0.3
- **License:** MIT License
- **Copyright:** Copyright (c) 2011-2024, Christopher Jeffrey
- **Source:** https://github.com/markedjs/marked

### highlight.js

- **Version:** 11.10.0
- **License:** BSD 3-Clause License
- **Copyright:** Copyright (c) 2006-2024 Josh Goebel and other contributors
- **Source:** https://github.com/highlightjs/highlight.js

### github-markdown-css

- **Version:** 5.6.1
- **License:** MIT License
- **Copyright:** Copyright (c) Sindre Sorhus
- **Source:** https://github.com/sindresorhus/github-markdown-css/tree/v5.6.1

### Mermaid

- **Version:** 11.4.1
- **License:** MIT License
- **Copyright:** Copyright (c) 2014-2024 Knut Sveidqvist and Mermaid contributors
- **Source:** https://github.com/mermaid-js/mermaid/releases/tag/mermaid%4011.4.1

### Vega

- **Version:** 5.30.0
- **License:** BSD 3-Clause License
- **Copyright:** Copyright (c) 2015-2024 University of Washington Interactive Data Lab and contributors
- **Source:** https://github.com/vega/vega

### Vega-Lite

- **Version:** 5.21.0
- **License:** BSD 3-Clause License
- **Copyright:** Copyright (c) 2015-2024 University of Washington Interactive Data Lab and contributors
- **Source:** https://github.com/vega/vega-lite

### Vega-Embed

- **Version:** 6.26.0
- **License:** BSD 3-Clause License
- **Copyright:** Copyright (c) 2015-2024 University of Washington Interactive Data Lab and contributors
- **Source:** https://github.com/vega/vega-embed

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check for security advisories on bundled markdown viewer dependencies.

echo "Checking npm packages for security advisories..."
echo ""

# Check each package
for pkg in "marked@13.0.3" "highlight.js@11.10.0" "github-markdown-css@5.6.1" "mermaid@11.4.1" "vega@5.30.0" "vega-lite@5.21.0" "vega-embed@6.26.0"; do
  pkg_name="${pkg%@*}"
  pkg_version="${pkg#*@}"
  echo "=== $pkg_name ==="
  
  # Query npm registry for latest version
  latest=$(curl -s "https://registry.npmjs.org/$pkg_name/latest" | jq -r '.version')
  echo "Bundled: $pkg_version | Latest: $latest"
  
  # Check GitHub security advisories
  gh api graphql -f query="
  {
    securityVulnerabilities(first: 10, ecosystem: NPM, package: \"$pkg_name\") {
      nodes {
        advisory {
          summary
          severity
          publishedAt
        }
        vulnerableVersionRange
        firstPatchedVersion {
          identifier
        }
      }
    }
  }" --jq ".data.securityVulnerabilities.nodes[] | select(.vulnerableVersionRange | test(\"$pkg_version\"; \"i\")) | {severity: .advisory.severity, summary: .advisory.summary, patched: .firstPatchedVersion.identifier}"
  
  echo ""
done

Repository: manaflow-ai/cmux

Length of output: 476


Consider updating bundled library versions to latest releases.

The specified versions are free from known security vulnerabilities. However, several libraries are outdated:

  • marked is 5 minor versions behind (13.0.3 → 18.0.3)
  • mermaid is 11 minor versions behind (11.4.1 → 11.15.0)
  • vega, vega-lite, and vega-embed are all at least one major version behind

Updating to latest versions would improve compatibility, performance, and access to bug fixes.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@THIRD_PARTY_LICENSES.md` around lines 193 - 240, Update the bundled library
entries in THIRD_PARTY_LICENSES.md for marked, mermaid, vega, vega-lite, and
vega-embed to their current latest released versions and corresponding
license/copyright/source URLs; specifically locate the sections titled "marked",
"Mermaid", "Vega", "Vega-Lite", and "Vega-Embed" and replace the Version,
Copyright, and Source fields with the upstream project’s latest metadata, then
run your dependency manager (package.json / yarn.lock / pnpm-lock.yaml) to bump
the packages to those versions and run tests/build to catch breaking changes.

Comment thread THIRD_PARTY_LICENSES.md Outdated
@lawrencecchen
lawrencecchen dismissed stale reviews from coderabbitai[bot], coderabbitai[bot], coderabbitai[bot], and coderabbitai[bot] May 13, 2026 04:58

All actionable inline threads are resolved; CodeRabbit passed on latest head f3dfd15.

@lawrencecchen
lawrencecchen merged commit 9c38779 into manaflow-ai:main May 13, 2026
7 of 9 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