Skip to content

[codex] Structure mobile external link failures#3363

Merged
juliusmarminge merged 3 commits into
mainfrom
codex/mobile-external-link-errors
Jun 20, 2026
Merged

[codex] Structure mobile external link failures#3363
juliusmarminge merged 3 commits into
mainfrom
codex/mobile-external-link-errors

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Jun 20, 2026

Copy link
Copy Markdown
Member

Summary

  • route file previews, markdown links, and pull-request links through one best-effort mobile external-link boundary
  • log a schema-tagged error with semantic target, URL scheme/host, and exact cause without retaining path/query/fragment data
  • replace cause-derived PR alerts with a stable user-facing message while preventing unhandled Linking.openURL rejections

Validation

  • vp test apps/mobile/src/lib/openExternalUrl.test.ts (2 tests)
  • vp check (passes with 20 pre-existing warnings)
  • vp run typecheck
  • vp run lint:mobile

Overlap

  • Exact current and prior filename scan found no open PR overlap.

Note

Low Risk
Localized UX and logging refactor around link opening; no auth or data-model changes, with explicit tests for log redaction.

Overview
Adds tryOpenExternalUrl as the shared mobile boundary for opening links: it wraps Linking.openURL, returns success/failure, and on failure logs a schema-tagged ExternalUrlOpenError with semantic target (file-preview, markdown-link, pull-request), scheme, and host only—no full URL or raw cause in console output.

File previews, markdown (including native selectable links via onLinkPress), Safari preview, git PR controls, and the git action overlay now call this helper instead of Linking.openURL directly. Open PR flows in git sheets use a fixed "Unable to open PR" alert when the helper returns false, replacing error-message-derived alerts. Unit tests cover success and sanitized failure logging.

Reviewed by Cursor Bugbot for commit e4a3d13. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Structure mobile external link failures with a centralized tryOpenExternalUrl helper

  • Introduces tryOpenExternalUrl in openExternalUrl.ts, a boolean-returning wrapper around Linking.openURL that logs a privacy-preserving diagnostic (scheme, host, target — no URL or error message) on failure.
  • Replaces direct Linking.openURL calls across markdown preview, file preview, and PR link handlers with tryOpenExternalUrl, each passing a typed ExternalUrlTarget string for structured logging.
  • PR open flows in ThreadGitControls and GitOverviewSheet now show a generic "Unable to open PR" alert on failure instead of surfacing raw error messages.
  • Adds unit tests verifying success behavior and that failure logs contain no sensitive URL or error content.

Macroscope summarized e4a3d13.

Co-authored-by: codex <codex@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0054e90d-e645-473f-856b-3ca99d897796

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/mobile-external-link-errors

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

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Jun 20, 2026

@cursor cursor 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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: iOS file markdown bypasses boundary
    • Passed onLinkPress callback (calling tryOpenExternalUrl) to SelectableMarkdownText in FileMarkdownPreview, so iOS link taps now route through the shared boundary instead of falling through to raw Linking.openURL.

Create PR

Or push these changes by commenting:

@cursor push 3a0155209b
Preview (3a0155209b)
diff --git a/apps/mobile/src/features/files/FileMarkdownPreview.tsx b/apps/mobile/src/features/files/FileMarkdownPreview.tsx
--- a/apps/mobile/src/features/files/FileMarkdownPreview.tsx
+++ b/apps/mobile/src/features/files/FileMarkdownPreview.tsx
@@ -1,4 +1,4 @@
-import { useMemo } from "react";
+import { useCallback, useMemo } from "react";
 import {
   Markdown,
   type CustomRenderers,
@@ -144,12 +144,19 @@
 
 export function FileMarkdownPreview(props: { readonly markdown: string }) {
   const styles = useMarkdownPreviewStyles();
+  const onLinkPress = useCallback((href: string) => {
+    void tryOpenExternalUrl(href, "markdown-link");
+  }, []);
 
   return (
     <ScrollView className="flex-1 bg-card" contentContainerStyle={{ padding: 18 }}>
       <View className="mx-auto w-full max-w-[760px]">
         {hasNativeSelectableMarkdownText() ? (
-          <SelectableMarkdownText markdown={props.markdown} textStyle={styles.nativeTextStyle} />
+          <SelectableMarkdownText
+            markdown={props.markdown}
+            textStyle={styles.nativeTextStyle}
+            onLinkPress={onLinkPress}
+          />
         ) : (
           <Markdown
             options={{ gfm: true }}

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit aa8f9dc. Configure here.

Comment thread apps/mobile/src/features/files/FileMarkdownPreview.tsx
@macroscopeapp

macroscopeapp Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved

This PR refactors external URL opening into a centralized utility with structured error logging, replacing scattered direct Linking.openURL calls. The changes are mechanical replacements with improved error handling and include tests. No new features or significant behavior changes are introduced.

You can customize Macroscope's approvability policy. Learn more.

Co-authored-by: codex <codex@users.noreply.github.com>
@github-actions github-actions Bot added size:L 100-499 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Jun 20, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jun 20, 2026
Co-authored-by: codex <codex@users.noreply.github.com>
@macroscopeapp macroscopeapp Bot dismissed their stale review June 20, 2026 17:11

Dismissing prior approval to re-evaluate e4a3d13

@juliusmarminge juliusmarminge merged commit d60f5c6 into main Jun 20, 2026
16 checks passed
@juliusmarminge juliusmarminge deleted the codex/mobile-external-link-errors branch June 20, 2026 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant