Skip to content

🤖 feat: add copy-file button to immersive review file view - #3919

Open
ibetitsmike wants to merge 19 commits into
mainfrom
mike/fix-2842-copy-file-button
Open

🤖 feat: add copy-file button to immersive review file view#3919
ibetitsmike wants to merge 19 commits into
mainfrom
mike/fix-2842-copy-file-button

Conversation

@ibetitsmike

Copy link
Copy Markdown
Contributor

Summary

Adds a "copy file" button (and a y keyboard shortcut) to the immersive review file view that copies the entire viewed file's text to the clipboard.

Fixes #2842

Background

The file view offers no way to copy the whole file: Ctrl+A selects content beyond the viewed file. The issue predates the removal of the Explorer file viewer (#3208); the immersive review view is the current single-file surface, so the affordance lands in its header.

Implementation

  • REVIEW_COPY_FILE (y) keybind, wired into the existing immersive keydown handler so editable-element, dialog, notes-panel, and touch gating apply for free. Surfaced in Settings → Keybinds under Immersive Review.
  • Header button next to the file navigation chevrons: lucide Copy icon flipping to Check with the shared tooltip (Copy file (Y), shortcut hint hidden on mobile). Hidden when the review is complete.
  • The copy fetches the file fresh via buildReadFileScript without display budgets, so it yields the entire on-disk file even when the overlay fell back to compact hunks (>1500 lines / 256KB); non-text (binary/image) results are rejected.

Validation

  • make static-check, targeted unit tests (ImmersiveReviewView.test.tsx: full-file copy under a line-budgeted display read, keybind + editable-element guard, hidden-on-complete), and tests/ui/review.
  • Remote dogfood UAT passed 10/10 scenarios on this exact commit, including full-file copy of a 2000-line file with only one hunk in the diff, keybind guards, review-complete hiding, 375px-width layout, and immersive navigation regression checks.

Generated with mux • Model: anthropic:claude-fable-5 • Thinking: xhigh

Fixes #2842: the file view had no way to copy the whole file, and
Ctrl+A selects beyond the viewed file. Adds a header button plus the
'y' keybind that read the entire on-disk file (bypassing the overlay's
1500-line/256KB display budget) and copy it to the clipboard.
@ibetitsmike

Copy link
Copy Markdown
Contributor 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: 5d6681f401

ℹ️ 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 src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx Outdated
@chatgpt-codex-connector

This comment has been minimized.

… reads

Codex review: discard copy completions after navigating files, hide the
button for deleted files, and reject IPC-truncated reads instead of
silently copying a partial file.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@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: 50c6f938c0

ℹ️ 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 src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx Outdated
Comment thread src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx Outdated
Comment thread src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx Outdated
Derive deleted-file state from unfiltered hunks so filtered-out files
stay gated, copy SVG markup as text instead of rejecting it as an
image, and drop the keybind value from the handler comment.
@ibetitsmike

Copy link
Copy Markdown
Contributor 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: 5c95042f20

ℹ️ 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 src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx Outdated
Comment thread src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx Outdated

@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 Security Review

Here are some automated security review suggestions for this pull request.

Reviewed commit: 5c95042f20

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

Comment thread src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx Outdated
Derive deletion from file-tree numstat so hunk-less deletions stay
gated, anchor relative read paths with ./ so option-like names (-n)
read correctly, and surface copy failures on the button instead of
only logging to the console.
@ibetitsmike

Copy link
Copy Markdown
Contributor 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: cc2c698ce2

ℹ️ 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 src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx Outdated
Comment thread src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx Outdated
Codex security finding: a changed symlink pointing outside the
workspace let the copy action (and the shared overlay read path)
read arbitrary external files. buildReadFileScript now fails closed
with exit 44 when the physically resolved path escapes the execution
root; in-workspace symlinks still read normally.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

Drop the manual useMemo (React Compiler convention) and invalidate
in-flight copies on unmount so a late read cannot overwrite the
clipboard after leaving immersive review.
@chatgpt-codex-connector

This comment has been minimized.

@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

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

ℹ️ 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 src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx Outdated
Replace the new useCallbacks with plain functions (React Compiler
convention), route the keyboard path through a ref like the file's
other handlers, and announce copy outcomes via an sr-only live region
so the keyboard shortcut result reaches assistive technology.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

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

ℹ️ 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 src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx Outdated
Run copy reads from the repo root for single-project workspaces
(multi-project hunk paths stay container-root-relative and keep
default mode), and reject unsuccessful script exits so partial
output like a lone stat size can never be copied as empty text.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@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: 2d488f2d75

ℹ️ 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 src/browser/utils/fileRead.ts Outdated
Comment thread src/browser/utils/fileRead.ts Outdated
Comment thread src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx Outdated

@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 Security Review

Here are some automated security review suggestions for this pull request.

Reviewed commit: fa11fa1620

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

Comment thread src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx Outdated
Codex ABA finding: navigating A -> B -> A while a copy of A was in
flight let the stale read pass the path check and reach the clipboard,
while the pending slot blocked a replacement copy. Navigation now bumps
the request generation and frees the slot.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

1 similar comment
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

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

ℹ️ 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 src/browser/utils/fileRead.ts
@ibetitsmike

Copy link
Copy Markdown
Contributor 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: b7b73da09e

ℹ️ 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 src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx Outdated

@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 Security Review

Here are some automated security review suggestions for this pull request.

Reviewed commit: b7b73da09e

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

Comment thread src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx Outdated
Codex finding: an agent edit or diff refresh changes the same path's
content version while a copy read is pending, so the stale result
could reach the clipboard and report success against the new diff.
The invalidation effect and feedback identity now include the file's
content version.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

Codex finding: parseNumstat trimmed the whole output, so the LAST
line's path lost trailing whitespace (a file literally named '.env '
became '.env') and downstream reads targeted the wrong file. Split
lines before filtering instead.
@ibetitsmike

Copy link
Copy Markdown
Contributor 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: 7748b85bd6

ℹ️ 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 src/browser/utils/fileRead.test.ts Outdated
Comment thread src/common/utils/git/numstatParser.ts
Codex finding: the size-check test asserted the exact realpath command
string; a plain end-to-end bash read validates the same script without
pinning implementation literals.
@ibetitsmike

Copy link
Copy Markdown
Contributor 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: 9fd96cc684

ℹ️ 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 src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx Outdated
Codex finding: base64 expansion pushed files over ~780KB past the
1MiB IPC output cap, so copies of ordinary sub-10MB files always
failed with a generic message. Copies now use a deterministic 750KB
budget and report the size limit when exceeded.
@ibetitsmike

Copy link
Copy Markdown
Contributor 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: 4ed0da1da1

ℹ️ 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 src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.test.tsx Outdated
Replace the generated-shell literal assertion with a real-bash
red-green pair (oversized file exits too-large with no payload;
in-budget file reads fully) in fileRead.test.ts.
@ibetitsmike

Copy link
Copy Markdown
Contributor 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: 5d7c9510e4

ℹ️ 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 src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx Outdated

@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 Security Review

Here are some automated security review suggestions for this pull request.

Reviewed commit: 5d7c9510e4

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

Stop trimming whole name-status output so trailing-space paths keep
their change status (matching the numstat fix), and show a visible
failure when copy is invoked while the backend API is unavailable.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

Codex finding: reprojectRepoRootFilePath trimmed file paths, undoing
the parser fixes downstream (a file named '.env ' again read '.env').
File-path normalization now keeps edge whitespace; trimming remains
only for config-controlled project paths.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

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

ℹ️ 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".

// swapping the symlink between validation and read cannot redirect the read.
return `${anchorScript}
[ -n "$anchor" ] || exit ${EXIT_CODE_OUTSIDE_WORKSPACE}
resolved=$(realpath ${file} 2>/dev/null || readlink -f ${file} 2>/dev/null)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Resolve file paths without GNU-only commands

On older macOS or remote BSD environments that provide neither realpath nor GNU readlink -f, both commands fail and resolved is empty, so every full-file overlay read and Copy action exits as “outside workspace” even for ordinary files. The repository already models the relevant BSD/macOS behavior in src/node/services/tools/mux_agents.test.ts:248-257; use a portable resolver or perform containment and reading in the Node backend.

Useful? React with 👍 / 👎.

try {
const result = await api.workspace.executeBash({
workspaceId: props.workspaceId,
script: buildReadFileScript(filePath, {

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 Isolate the file payload from tool_env output

In a trusted workspace whose .mux/tool_env prints anything during activation, this read no longer starts with the expected size line: src/node/services/tools/bash.ts:828-835 prepends source ... 2>&1, so even a harmless echo ready makes processFileContents reject every Copy attempt (and can also break full-file hydration). Frame the payload with unambiguous sentinels, suppress prelude output for this IPC, or use a dedicated file-read API.

AGENTS.md reference: AGENTS.md:L52-L52

Useful? React with 👍 / 👎.

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.

Add copy file button

1 participant