Skip to content

Render Command-clicked HTML files in browser panes - #9096

Merged
austinywang merged 8 commits into
mainfrom
issue-8627-html-click-render
Jul 29, 2026
Merged

austinywang merged 8 commits into
mainfrom
issue-8627-html-click-render

Conversation

@austinywang

@austinywang austinywang commented Jul 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Route terminal-linked .html and .htm files to the existing cmux Browser pane instead of FilePreviewPanel, across visible-path and Ghostty OPEN_URL routing in Workspace and Dock containers.
  • Preserve FilePreview/external fallbacks when browser routing is disabled, unavailable, or remote.
  • Keep local navigation on the existing BrowserPanel file-loading path, which uses loadFileURL(_:allowingReadAccessTo:) with the containing directory so sibling page assets work.
  • Distinguish custom, WebKit-default, and non-web user-agent policy outcomes so file:// navigation clears stale custom identity without entering a cancel/replay loop.

Why: command-clicked agent-generated HTML was shown as raw source, and the first runtime build exposed a separate WebKit user-agent replay loop that left the Browser pane blank.

Regression coverage

  • 147d96f417: failing behavior tests for .html/.htm visible-path routing and Dock terminal-link routing.
  • fb56973857 and 1c848d8311: failing WebKit/package policy tests proving non-web destinations must be distinct from WebKit-default HTTP(S) identity.
  • 74b1dd9faf: explicit policy resolution fix; local files clear custom identity without requesting navigation restart.

Testing

  • swift test --package-path Packages/macOS/CmuxBrowser — 251 tests passed.
  • xcrun swiftc -frontend -parse on every changed Swift file — passed.
  • ./scripts/lint-pbxproj-test-wiring.sh — passed (625 test files checked).
  • git diff --check — passed.
  • Canonical cmux Aziz policy check — clean.
  • Full CI run 30432310065 passed, including all app-host shards, package tests, warning-budget/build-and-lag coverage, and the universal Release build.
  • Tagged Blacksmith cloud build 30435672942 passed at 0c2c39703a.

Final tagged-app verification completed at 0c2c39703a using the production Command-click harness through the tag-bound socket. /tmp/test.html opened as file:///tmp/test.html in a Browser panel (no FilePreview or Markdown panel), rendered hello and rendered, and computed the paragraph as rgb(0, 128, 0). A second local page loaded sibling CSS and JavaScript from its containing directory, and the debug log showed one file: policy decision followed by allow with no cancel/replay loop.

Demo Video

  • No video attached. Final isolated tagged-app verification completed successfully; screenshots were captured for both the exact rendered repro and the sibling-asset proof.

Review Trigger (Copy/Paste as PR comment)

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

Checklist

  • I tested the change locally (package tests, parser, wiring, and the earlier tagged routing repro)
  • I added or updated tests for behavior changes
  • I updated docs/changelog if needed (not applicable: no docs or user-facing copy changed)
  • I requested bot reviews after my latest commit (trigger comment below)
  • All code review bot comments are resolved or have a verified reasoned disposition
  • All human review comments are resolved (none submitted)

Closes #8627


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Open command‑clicked and terminal‑linked .html/.htm files in the in‑app browser pane across Workspace and Dock, with deferral and safe fallbacks. Also formalizes non‑web user‑agent policy so local files clear custom identity without restarting, and fixes an RPC session capture bug, fulfilling #8627.

  • New Features

    • Route .html/.htm to the Browser from command‑click and terminal links via TerminalHTMLFileBrowserAction, with injectable UserDefaults, deferral when containers aren’t ready, and fallback to file preview or external; keep BrowserPanel’s local asset loading path.
    • Add BrowserUserAgentPolicyResolution in CmuxBrowser: non‑web destinations are not applicable and clear the custom identity without a restart; Google Sheets uses WebKit’s default identity. Tests cover visible‑path and Dock routing and these policy outcomes.
  • Bug Fixes

    • In CmuxMobileRPC, capture the MobileCoreRPCSession actor immutably during queued write recovery to prevent dropped work.

Written for commit 0c2c397. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Supported .html/.htm terminal links now open in a Browser panel (including docked layouts) when enabled in settings.
    • HTML link handling can be deferred and retried until the target panel/container is available.
  • Bug Fixes
    • Improved local HTML routing reliability with more consistent fallbacks to the external browser when browser handling isn’t possible.
    • Browser user-agent policy now clears any custom identity for non-web destinations without requiring a restart.
  • Tests
    • Added/updated coverage for HTML routing in visible/docked panels and destination-based user-agent policy resolution behavior.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8f402a65-5f6b-43f2-9704-d77a347ff433

📥 Commits

Reviewing files that changed from the base of the PR and between ae52684 and 0c2c397.

📒 Files selected for processing (11)
  • Packages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileCoreRPCSession.swift
  • Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/BrowserUserAgentPolicy.swift
  • Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/BrowserUserAgentPolicyResolution.swift
  • Packages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/BrowserUserAgentPolicyTests.swift
  • Sources/CommandClickFileOpenRouter.swift
  • Sources/Panels/WKWebView+BrowserUserAgentPolicy.swift
  • Sources/TerminalHTMLFileBrowserAction.swift
  • Sources/TerminalLinkOpenCoordinator.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/BrowserUserAgentPolicyWebKitTests.swift
  • cmuxTests/TerminalLinkOpenCoordinatorTests.swift

📝 Walkthrough

Walkthrough

HTML and HTM local files now route through the cmux browser with deferred container handling, configurable defaults, and fallback behavior. Browser user-agent handling now exposes explicit destination states. Mobile demand recovery retains its session during grace-period processing.

Changes

HTML routing

Layer / File(s) Summary
HTML browser action
Sources/TerminalHTMLFileBrowserAction.swift, cmux.xcodeproj/project.pbxproj
Adds HTML/HTM validation, browser availability gating, normalized file URL resolution, and project compilation wiring.
Coordinator routing
Sources/TerminalLinkOpenCoordinator.swift
Centralizes local-file routing, deferred HTML browser opening, container re-resolution, and terminal-file or external fallbacks.
Command-click integration and validation
Sources/CommandClickFileOpenRouter.swift, cmuxTests/TerminalLinkOpenCoordinatorTests.swift
Threads configurable defaults through routing, attempts browser opening before file preview, and tests visible and docked HTML flows.

Browser user-agent policy

Layer / File(s) Summary
Destination policy resolution
Packages/macOS/CmuxBrowser/Sources/CmuxBrowser/BrowserUserAgentPolicy*.swift, Packages/macOS/CmuxBrowser/Tests/CmuxBrowserTests/BrowserUserAgentPolicyTests.swift
Replaces string-or-nil user-agent selection with explicit custom, WebKit-default, and not-applicable outcomes.
WebKit policy application
Sources/Panels/WKWebView+BrowserUserAgentPolicy.swift, cmuxTests/BrowserUserAgentPolicyWebKitTests.swift
Applies resolved user-agent outcomes and clears custom identity for non-web destinations.

Mobile demand recovery

Layer / File(s) Summary
Grace-period recovery execution
Packages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileCoreRPCSession.swift
Uses a strong session capture so queued demand recovery and cancelled-write recycling execute during the grace period.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant TerminalLinkOpenCoordinator
  participant TerminalHTMLFileBrowserAction
  participant TerminalLinkOpenContainer
  participant BrowserPanel
  TerminalLinkOpenCoordinator->>TerminalHTMLFileBrowserAction: validate HTML file URL
  TerminalLinkOpenCoordinator->>TerminalLinkOpenContainer: defer browser opening
  TerminalLinkOpenContainer->>TerminalHTMLFileBrowserAction: open normalized browser URL
  TerminalHTMLFileBrowserAction->>BrowserPanel: open terminal browser link
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning BrowserUserAgentPolicy and CmuxMobileRPC changes are unrelated to #8627's HTML-routing requirement and look like separate fixes. Split the user-agent policy and MobileCoreRPCSession fixes into separate PRs, or document why they are required for #8627.
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: rendering command-clicked HTML files in browser panes.
Description check ✅ Passed The description follows the template with Summary, Testing, Demo Video, Review Trigger, and Checklist sections.
Linked Issues check ✅ Passed The PR routes local .html/.htm command-clicks to the browser pane, matching #8627's expected rendered-browser behavior.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Cmux Swift Actor Isolation ✅ Passed Changed UI coordinators stay @MainActor; the new settings helpers are stateless/Sendable and no background access to UI-bound stores was introduced.
Cmux Swift Blocking Runtime ✅ Passed PR diff adds no new semaphores, sync waits, sleeps, or locks; only a strong self capture and existing async deferrals remain.
Cmux Browser Automation Off-Main ✅ Passed The PR diff only adds HTML-routing and user-agent policy code/tests; it does not modify TerminalController or ControlCommandExecutionPolicy, so no browser socket automation path was moved off-main.
Cmux Expensive Synchronous Load ✅ Passed Touched code only adds HTML/browser routing, UA policy, and an RPC task-capture fix; no agent-history load/JSONL parsing was added on MainActor or interactive paths.
Cmux Cache Substitution Correctness ✅ Passed No persistence/history/undo/snapshot path swaps in a stale cache; the edits only thread UserDefaults and transient browser UA policy/state.
Cmux No Hacky Sleeps ✅ Passed Diff touches only Swift, tests, and pbxproj; no non-Swift runtime files were changed and no fixed-delay hacks were introduced.
Cmux Algorithmic Complexity ✅ Passed Touched runtime paths add only constant-time routing checks and a bounded path-component test; no nested scans or per-target rescans over scalable collections were introduced.
Cmux Swift Concurrency ✅ Passed PASS: the only new async change is a stronger capture in an existing recovery Task; no new bg queues, Combine, or completion-handler patterns were introduced.
Cmux Swift @Concurrent ✅ Passed Changed Swift code is sync, @MainActor, or actor-isolated; no @concurrent misuse or missing @concurrent on nonisolated async work was introduced.
Cmux Swift Package Boundaries ✅ Passed The new app-target code is mostly AppKit/Ghostty routing glue around package types and container protocols, not reusable domain logic that should be extracted.
Cmux Swiftpm Lockfiles ✅ Passed HEAD only changes MobileCoreRPCSession.swift; no Package.resolved, Xcode project, or .gitignore dependency files were modified.
Cmux Swift Logging ✅ Passed Only new logging is DEBUG-gated cmuxDebugLog in TerminalLinkOpenCoordinator; no print/NSLog/Logger additions in changed runtime files.
Cmux User-Facing Error Privacy ✅ Passed The only production diff is an RPC actor-capture fix; it adds no user-facing errors, alerts, or sensitive copy.
Cmux Full Internationalization ✅ Passed PASS: no user-facing Swift text or locale assets were added; only internal routing reasons/logs changed, and no Resources/*.xcstrings, Info.plist, or web locale files were touched.
Cmux Swiftui State Layout ✅ Passed Diff touches routing/actor/browser code and tests only; no new ObservableObject/@published, GeometryReader, lazy-list store refs, or render-time SwiftUI state writes.
Cmux Architecture Rethink ✅ Passed PASS: HTML routing is centralized in TerminalHTMLFileBrowserAction, and the only deferred open is a documented Ghostty bridge; no new split ownership or polling/locks.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed Diff only changes MobileCoreRPCSession actor capture; no NSWindow/NSPanel/WindowGroup code or cmuxAuxiliaryWindowIdentifiers changes.
Cmux Source Artifacts ✅ Passed Only a Swift source file changed; no logs, caches, build outputs, scratch dirs, or other artifact paths were added.
Cmux No Test Or Debug Seam In Production Source ✅ Passed HEAD only changes MobileCoreRPCSession task capture; no new debug/test seam or visibility-widening test hook was introduced.
Cmux No Ambient Global State ✅ Passed No new file-scope funcs/mutable globals or new singleton state were introduced; new behavior lives on injectable types, and existing singletons were only reused.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-8627-html-click-render

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.

@austinywang

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 28, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@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: 1

🤖 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 `@cmuxTests/TerminalLinkOpenCoordinatorTests.swift`:
- Around line 185-201: Thread an injectable defaults parameter through
CommandClickFileOpenRouter.shouldRouteInCmux and openInCmux, replacing their
hardcoded FileRouteSettingsStore(defaults: .standard) and
TerminalHTMLFileBrowserAction() dependencies. Update
withStandardHTMLRoutingEnabled and dockHTMLPathOpensInBrowser to use the
isolated makeDefaults() suite, while preserving the existing routing behavior.
🪄 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 Plus

Run ID: 7002d762-7889-41e1-814f-1035a7e9776d

📥 Commits

Reviewing files that changed from the base of the PR and between 72a457c and 9f0472d.

📒 Files selected for processing (5)
  • Sources/CommandClickFileOpenRouter.swift
  • Sources/TerminalHTMLFileBrowserAction.swift
  • Sources/TerminalLinkOpenCoordinator.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/TerminalLinkOpenCoordinatorTests.swift

Comment thread cmuxTests/TerminalLinkOpenCoordinatorTests.swift Outdated
@cursor

cursor Bot commented Jul 29, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@austinywang

Copy link
Copy Markdown
Contributor Author

CodeRabbit localization pre-merge finding disposition: no fix is needed. This PR does not modify Resources/Localizable.xcstrings (or any resource/catalog file), and the changed production Swift adds no user-facing string. gh pr diff 9096 --name-only contains only the browser-policy/routing Swift sources, tests, and their pbxproj wiring. The localization finding appears to have been computed from unrelated/stale diff context.

@austinywang

Copy link
Copy Markdown
Contributor Author

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@cubic-dev-ai

cubic-dev-ai Bot commented Jul 29, 2026

Copy link
Copy Markdown

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@austinywang I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 241,631 of the 240,000 allowed lines of code this month. Reviews resume on 1 August 2026 (in 3 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews.

To help optimise your usage, you can tune cubic to get the most out of your usage limits:

Learn more →

@coderabbitai

coderabbitai Bot commented Jul 29, 2026 •

Copy link
Copy Markdown

@austinywang: I’ll review the latest changes in #9096.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

1 similar comment
@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@austinywang

Copy link
Copy Markdown
Contributor Author

Latest HEAD includes isolated CI-unblock commit 0c2c39703a: current main introduced a strict-concurrency compile failure in MobileCoreRPCSession.swift; the correction captures the session actor immutably. swift test --package-path Packages/iOS/CmuxMobileRPC compiled cleanly at the affected site and all 141 tests passed. The #8627 runtime behavior is unchanged by this commit.

Please review the full latest-commit diff:
@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@cubic-dev-ai

cubic-dev-ai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Latest HEAD includes isolated CI-unblock commit 0c2c39703a: current main introduced a strict-concurrency compile failure in MobileCoreRPCSession.swift; the correction captures the session actor immutably. swift test --package-path Packages/iOS/CmuxMobileRPC compiled cleanly at the affected site and all 141 tests passed. The #8627 runtime behavior is unchanged by this commit.

Please review the full latest-commit diff:
@codex review
@coderabbitai review
...

@austinywang I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 241,631 of the 240,000 allowed lines of code this month. Reviews resume on 1 August 2026 (in 3 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews.

To help optimise your usage, you can tune cubic to get the most out of your usage limits:

Learn more →

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026 •

Copy link
Copy Markdown

@austinywang: I’ll perform a full review of the latest PR diff, including commit 0c2c39703a and its MobileCoreRPCSession.swift concurrency fix.

✅ Action performed

Full review finished.

@austinywang

Copy link
Copy Markdown
Contributor Author

CodeRabbit out-of-scope warning disposition: both auxiliary changes are required to complete and validate #8627, and they are isolated in separate commits. The browser user-agent policy change is part of the runtime root cause: after HTML routing correctly opened file:///tmp/test.html, WKWebView stayed blank because file: and Google Sheets shared the same optional-nil outcome, causing local navigation to enter a cancel/replay loop. The explicit .custom / .webKitDefault / .notApplicable states eliminate that bug class and are covered by red/green tests. The three-line MobileCoreRPCSession actor-capture commit changes no #8627 behavior; current main failed all four app-host shards at that line, so it is an auditable CI-unblock commit. The affected package passed all 141 tests, the complete app-host matrix passed, and full CI run https://github.com/manaflow-ai/cmux/actions/runs/30432310065 is green.

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.

Cmd/ctrl-click on a .html file opens raw source viewer instead of rendering in the browser pane

1 participant