[lancy69]

Issue for this PR

Closes #58878

Design approval requested on the issue; still pending. Implementation prepared with OpenAI Codex assistance.

Type of change

• Bug fix
• New feature
• Refactor / code improvement
• Documentation

What does this PR do?

Copy complete file text from the review header rather than diff hunks. Provide success/error feedback and leave the clipboard unchanged for unavailable or binary content. Available reviewed snapshots take precedence over current disk contents, including empty snapshots. When no snapshot is supplied, non-deleted files fall back to a fresh disk read. Deleted files use complete old text when available.

Merged upstream dev, kept the release-managed changelog unchanged, and adopted the shared clipboard helper so insecure-origin fallback matches upstream behavior. Addressed the remaining review feedback in 3fcf27383a: prefer diff.after snapshots and simplify the first E2E test without replacing navigator.clipboard.

How did you verify your code works?

Latest review-fix verification (3fcf27383a):

• bun run check: lint and all 35 typecheck tasks passed (lint warnings remain).
• bun run typecheck:tsc in packages/session-ui: passed.
• Both new component regressions failed before the fix (later disk text copied; empty snapshot button disabled), then passed 10 repetitions each with 2 workers and no retries (20/20).
• Cold full session-ui component suite: 179/179 passed with 2 workers and no retries; no dependency re-optimization reloads appeared.
• Production-built app clipboard regressions: 3/3 passed with full parallelism enabled.
• Direct Chromium smoke exercised the real preview: keyboard copy preserved the reviewed snapshot despite different disk text; an empty snapshot copied an empty string without a reader.

Bun 1.4.4: pre-push bun run check passed (lint and 35 typecheck tasks). Changed-file lint reported no problems; existing unrelated repository lint warnings remain.

Built review suite: 29/32 passed, including all three clipboard regressions. The three failures (open-file launcher selection, Markdown/DOCX tab switching, and session-tab close shortcut) also failed at the same assertions on unmodified upstream f29e2813 with a fresh production build; no unrelated behavior was changed.

Actual Chromium on the app's in-memory E2E API harness verified all 849 file characters, unchanged lines beyond the diff, keyboard/pointer copy, fresh reads after edits, clipboard-API fallback, deleted old content without reading the missing path, and binary/unavailable clipboard retention. English LTR and forced RTL, mixed Arabic text, and copy-button hit geometry were exercised.

Upstream full CI currently requires maintainer approval before jobs can run; green metadata checks do not establish full CI success.

Screenshots / recordings

Before: upstream preview component from f29e2813 mounted in the merged app through an isolated Vite override, not a complete historical app build. Both before and after use the in-memory E2E API harness.

Checklist

• I have tested my changes locally
• I have not included unrelated changes in this PR

Visual

<svg xmlns="http://www.w3.org/2000/svg" width="240" height="72" viewBox="0 0 240 72">
  <g fill="none" stroke="currentColor" stroke-width="1.5">
    <rect x="8" y="8" width="224" height="56" rx="6" opacity=".3"/>
    <path d="M20 25h135M20 38h112M20 51h145" opacity=".5"/>
    <rect x="197" y="22" width="14" height="18" rx="2"/>
    <path d="M202 18h14v18" opacity=".5"/>
  </g>
  <path d="m196 51 5 5 10-11" fill="none" stroke="currentColor" stroke-width="2">
    <animate attributeName="opacity" values="1;.25;1" dur="2s" repeatCount="indefinite"/>
  </path>
</svg>

[lancy69]

Merged current dev and resolved conflicts in d3db361; adopted the shared clipboard helper. Local check and all clipboard regressions pass; before/after evidence is in the description. Three other review-suite failures reproduced at the same assertions on unmodified upstream f29e281, so they are documented rather than changed here. Could a maintainer approve the full CI workflow runs? Design approval requested in #58878 remains pending.

[opencodejr]

This works as described. On the base branch the review file header has no copy action. With this PR, the new button copied the whole current file (not just the hunk) and showed the success toast. The three new e2e tests pass, and session-ui and gui-plugins type-check. The image and audio previews that share readFile still get binary content, so the switch to returning text for text-like files doesn't affect them.

Public API: unchanged (UI-only, plus four new English i18n keys).

Points worth addressing, details inline:

• Please drop the CHANGELOG.md entry. Only the release automation writes that file.
• The button always copies the file as it is on disk now, even in the "turn" and "branch" review modes, where that can differ from the version the panel shows.
• The first e2e test is harder to follow than it needs to be.

[opencodejr]

The CHANGELOG.md entry is gone, so that point is addressed. Otherwise the push only merges dev: the feature code and tests are unchanged since the last review. On the new head, the three new e2e tests still pass and session-ui and gui-plugins type-check.

Public API: unchanged.

Two earlier points are still open in their threads:

• The button copies the file as it is on disk now in the "turn" and "branch" modes.
• The first e2e test still swaps navigator.clipboard out and back in.

[opencodejr]

This commit fixes both earlier points. The copy button now uses the version of the file being reviewed when the diff includes it, even when that file is empty, and only reads from disk otherwise. The first e2e test no longer swaps navigator.clipboard in and out. On this head, the new component tests (2) and the review copy e2e tests (3) pass, and session-ui type-checks.

Public API: unchanged.

One new point: the paragraph added to packages/app/README.md isn't needed (see inline).

[opencodejr]

This README covers building and testing the app, not individual UI features, and nothing else documents single buttons here. The behaviour is already described in the code and the tests, so this section can be dropped to keep the change focused.