Skip to content

fix(clients): copy filesystem paths with native separators - #19201

Open
tonyjosephsebastians wants to merge 4 commits into
pingdotgg:mainfrom
tonyjosephsebastians:fix/windows-file-link-copy
Open

tonyjosephsebastians wants to merge 4 commits into
pingdotgg:mainfrom
tonyjosephsebastians:fix/windows-file-link-copy

Conversation

@tonyjosephsebastians

@tonyjosephsebastians tonyjosephsebastians commented Oct 14, 2026 •

Copy link
Copy Markdown

Summary

Fixes #19142.

Filesystem-path copy actions can put forward-slash Windows paths on the clipboard, such as C:/Users/me/project/reports/Device Users.csv, instead of C:\Users\me\project\reports\Device Users.csv.

Use the shared formatFilePathForClipboard helper to select separators from the target environment. This covers web/desktop and mobile chat file menus, image/video media menus, workspace-header copies, and file-preview copies. POSIX paths retain their style when accessed from a Windows client. Copy feedback shows the clipboard value.

Formatting happens at clipboard writes. File opening, sharing, downloading, and filesystem permissions use their existing paths and behavior. URL, content, and composer-reference copy actions retain their intended representations. Mobile file-preview menu dependencies include the workspace directory so retained actions update when it changes.

Scope and approval

This addresses one underlying separator defect across existing filesystem-path copy actions. The shared formatter is small and reuses the existing Windows-path detector; client changes apply it at their clipboard boundaries.

The change falls under the focused obvious-bug exception in CONTRIBUTING.md. No maintainer approval is claimed.

Tests done

Verified locally on Windows with Node 24, Vite+, and the updated pinned typechecker:

  • The original file-link handler reproduced four clipboard failures. The original media-menu handlers reproduced eight additional failures across web and mobile for Windows drive and UNC image/video paths.
  • 482 tests passed across 13 focused suites:
    vp test run apps/web/src/components/ChatMarkdown.test.tsx apps/web/src/markdown-links.test.ts apps/web/src/filePathDisplay.test.ts packages/shared/src/fileLinks.test.ts apps/mobile/src/features/threads/fileChipMenu.test.ts packages/client-runtime/src/mediaActions.test.ts apps/web/src/components/BranchToolbar.panel.test.tsx apps/web/src/components/BranchToolbar.logic.test.ts apps/web/src/components/ChatView.logic.test.ts apps/web/src/components/media/MediaActions.test.tsx apps/mobile/src/lib/mediaActions.test.ts packages/client-runtime/src/mediaReference.test.ts packages/client-runtime/src/markdownImages.test.ts --pool=threads --maxWorkers=1
  • Tests cover full/relative file and media paths, spaces, drive paths, UNC shares, POSIX targets, unknown workspace roots, line/column suffixes, Windows/macOS browser platform values, and workspace context-menu clipboard output.
  • Media tests preserve URL copies and check pending/denied/revoked host grants, image copying, downloads, and sharing behavior.
  • Typechecks passed: vp run --filter @t3tools/web typecheck and vp run --filter @t3tools/client-runtime typecheck. Shared-client checking reports existing informational Effect suggestions.
  • Before the latest commit, the full PR diff and filesystem-path copy call sites were reviewed. Structured scoped lint reported zero errors and no warnings on changed lines; existing warnings outside the changes remain. This review caught and corrected a missing workspace-directory dependency in the mobile file-preview menu.
  • Formatting, staged formatting hooks, and diff checks passed.

Remaining verification

  • Native Windows Open-dialog verification remains pending; automated tests verify clipboard payloads.
  • Native iOS/Android device verification and the full mobile package typecheck were not run. Mobile coverage here uses the file-menu and media-action logic tests.
  • Before/after UI screenshots remain pending. The built-in Browser reported unavailable from both preview_status and preview_open, so the required UI evidence could not be captured.

Copy Markdown file-link paths with native separators for Windows drive and
UNC targets, including relative path copies. Use the target path to choose
separators so remote POSIX workspaces keep their original clipboard paths.

Fixes pingdotgg#19142

Verified with 156 focused tests, the web typecheck, formatting, and scoped
lint. Lint reports only pre-existing warnings outside the changed code.
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 14, 2026
AdEx-Partners-DE added a commit to AdEx-Partners-DE/t3code that referenced this pull request Oct 14, 2026
…ream pingdotgg#19201)

Own diff of pingdotgg#19201 at 1173de5 against its merge base.
ChatMarkdown.test.tsx: 82 passed before, 98 after; tsc clean in apps/web.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tonyjosephsebastians
tonyjosephsebastians marked this pull request as ready for review October 14, 2026 01:41
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-14T02:34:05.080287Z f3abf50 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@tonyjosephsebastians

Copy link
Copy Markdown
Author

@juliusmarminge, this PR fixes the Windows file-path clipboard issue in #19142. I found your recent work on Markdown file links and previews, so I thought you would be a suitable reviewer.

The original handler reproduced four failing clipboard cases. The fix passes 156 focused tests covering Windows drive paths, UNC shares, spaces, relative paths, and POSIX remote workspaces, plus the web typecheck. Scoped lint reports only existing warnings.

Native Windows Open-dialog verification and before/after UI screenshots remain pending: the built-in Browser was unavailable for verification. These limitations are recorded in the PR description.

Could you review it or direct it to the appropriate reviewer when you have capacity?

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1173de5039

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/web/src/components/ChatMarkdown.tsx Outdated
@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b78df4a9-fa1c-40a9-b28d-1968341b6c6a
📥 Commits

Reviewing files that changed from the base of the PR and between f3abf50 and 9e31c7d.

📒 Files selected for processing: 8
  • apps/mobile/src/features/files/ThreadFilesRouteScreen.tsx
  • apps/mobile/src/lib/mediaActions.test.ts
  • apps/mobile/src/state/mediaActions.ts
  • apps/web/src/components/BranchToolbar.panel.test.tsx
  • apps/web/src/components/BranchToolbar.tsx
  • apps/web/src/components/ChatView.tsx
  • apps/web/src/components/media/MediaActions.test.tsx
  • apps/web/src/components/media/MediaActions.tsx

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

📝 Walkthrough

Web and mobile clipboard actions now format file paths for Windows hosts by converting forward slashes to backslashes. Tests cover Windows, UNC, POSIX, relative, and unknown-host paths. URL copy actions retain their original values.

Changes: Clipboard path formatting

Layer / File(s) Summary
Add shared clipboard path formatting
packages/client-runtime/src/mediaActions.ts, packages/client-runtime/src/mediaActions.test.ts
The formatter converts forward slashes to backslashes when the host path is a Windows absolute path. Tests cover Windows, UNC, mixed-separator, and unknown-host paths.
Format web file and workspace path copies
apps/web/src/components/ChatMarkdown.tsx, apps/web/src/components/ChatMarkdown.test.tsx, apps/web/src/components/BranchToolbar.tsx, apps/web/src/components/BranchToolbar.panel.test.tsx, apps/web/src/components/ChatView.tsx, apps/web/src/components/media/MediaActions.tsx, apps/web/src/components/media/MediaActions.test.tsx
Web file-link, workspace, right-panel, and media copy actions format file paths before copying. Tests cover Windows, UNC, POSIX, relative, and line-and-column cases. URL copies remain unchanged.
Format mobile file and media path copies
apps/mobile/src/features/threads/fileChipMenu.ts, apps/mobile/src/features/threads/ThreadFeed.tsx, apps/mobile/src/features/threads/fileChipMenu.test.ts, apps/mobile/src/features/files/ThreadFilesRouteScreen.tsx, apps/mobile/src/state/mediaActions.ts, apps/mobile/src/lib/mediaActions.test.ts
Mobile file-chip, thread-file, and media copy actions format file paths before copying. Relative media copies are limited to file references. Tests cover Windows, UNC, POSIX, unknown-workspace, and URL cases.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 9e31c

The change makes copied Windows file paths use native backslashes, and URL copies are unchanged. No concrete merge-blocking risk was found. Native Windows and mobile verification remain unconfirmed.

Security architecture risk: ⚪ Minimal · up to 02abc

The change adjusts copied path separators without expanding file access or action permissions. Existing target selection and failure handling are preserved, and no introduced security exposure was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — Markdown-derived paths can influence text copied through the existing context-menu actions. The changed flow remains bounded to the initiating client's clipboard and associated feedback; separator formatting does not itself read files, contact a UNC host, or execute a command.

Trust Boundaries and Controls

  • observed — Copy formatting does not replace the resolved target or alter privileged action dispatch. Web editor, browser, and reveal callbacks retain their existing gates. Mobile continues resolving the file-chip target before selecting the requested available path; open and save actions remain separate branches.

Resilience and Maintainability Implications

  • observed — The formatter adds no mutable state or multi-step transaction. Mobile clipboard writes remain fire-and-forget, with clipboard and haptic failures caught independently. Haptic execution is not ordered after successful copying, and concurrent writes are not serialized, but both behaviors predate this PR.
Pre-merge checks: Passed 4
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Issue #19142 requires native Windows separators for copied full and relative Markdown file-link paths. ChatMarkdown.tsx now formats both clipboard values with formatFilePathForClipboard. The forma…
Out of Scope Changes check Passed The production changes apply the same clipboard-boundary formatting to related filesystem-path copy actions in web, mobile, media menus, workspace actions, and file previews. Tests support these chang…
Title check Passed The title clearly and concisely describes the main change: copying filesystem paths with native separators.
Description check Passed The description explains the problem, implementation, scope exception, focused verification, and remaining verification limits. It uses alternate headings such as Summary and Tests done, but it provid…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Move clipboard path formatting into shared client logic and use it for web, desktop, and mobile chat file menus. Add mobile-menu and shared formatter regression coverage for Windows drive and UNC paths, POSIX hosts, and unknown workspace roots.
@github-actions github-actions Bot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Oct 14, 2026
@tonyjosephsebastians

Copy link
Copy Markdown
Author

"@codex review".

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f3abf50b8f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


/** Format clipboard text using the file's host path, which can differ from the client's OS. */
export function formatFilePathForClipboard(path: string, hostPath = path): string {
return isWindowsAbsolutePath(hostPath) ? path.replaceAll("/", "\\") : path;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Apply native formatting to media path actions

On a Windows-hosted Markdown image or video (for example an authored C:/repo/shot.png), the right-click/long-press media menus still bypass this formatter: apps/web/src/components/media/MediaActions.tsx:165-174 and apps/mobile/src/state/mediaActions.ts:109-120 send reference.path/relativePath directly to the clipboard, while classifyMarkdownImageSource preserves that forward-slash drive path. Consequently the file-chip menu is fixed but the same Copy full/relative path actions on chat media still copy non-native separators on both clients; route those payloads through this helper as well.

AGENTS.md reference: AGENTS.md:L67-L70

Useful? React with 👍 / 👎.

Use shared host-path formatting for web and mobile media menus, workspace-header copies, and file-preview copies. Preserve URL payloads and refresh memoized mobile menu actions when the workspace directory changes. Add image, video, drive-path, UNC, and workspace-menu regression coverage.
@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 Oct 14, 2026
@tonyjosephsebastians tonyjosephsebastians changed the title fix(web): copy Windows file links with native separators fix(clients): copy filesystem paths with native separators Oct 14, 2026

This branch has not been deployed

No deployments
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:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: "Copy full path" on a file link copies a forward-slash path on Windows

1 participant