Skip to content

fix(session-ui): fall back when copying code blocks on insecure origins - #51035

Closed
myagizmaktav wants to merge 3 commits into
anomalyco:devfrom
myagizmaktav:markdown-copy-fallback
Closed

myagizmaktav wants to merge 3 commits into
anomalyco:devfrom
myagizmaktav:markdown-copy-fallback

Conversation

@myagizmaktav

@myagizmaktav myagizmaktav commented Sep 24, 2026 •

Copy link
Copy Markdown

Issue for this PR

Fixes #50988

Type of change

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

What does this PR do?

The copy button on markdown code blocks read navigator.clipboard and returned if it was missing. On a plain-HTTP, non-localhost origin (e.g. opencode serve --hostname 0.0.0.0 opened from another device), the Clipboard API is always missing, so the button did nothing.

The message and tool copy buttons in session-ui already use writeClipboard, which tries a hidden-textarea execCommand("copy") first and falls back to navigator.clipboard. The code-block button now uses the same helper and only shows "copied" when the copy actually succeeded.

markdown.tsx can't import from message-content.tsx because message-content.tsx imports Markdown. So writeClipboard moves unchanged into components/clipboard.ts, and its two existing callers import it from there. To test the real click path, the code-copy handler and its button-state helpers move out of markdown.tsx into components/markdown-code-copy.ts unchanged. Only the clipboard call inside the handler differs.

How did you verify your code works?

New packages/session-ui/src/components/clipboard.test.ts (happy-dom, navigator.clipboard removed, execCommand stubbed):

  • writeClipboard copies through the execCommand path and returns true
  • clicking a code-block copy button copies the code text and sets data-copied="true"

Commands:

  • cd packages/session-ui && bun test src: 195 pass, 0 fail
  • cd packages/session-ui && bun run typecheck: clean
  • repo pre-push hook (oxlint + bun turbo typecheck): 0 lint errors, 35/35 typecheck tasks
  • With the old navigator.clipboard-only handler, the code-block test fails (1 pass, 1 fail)

Screenshots / recordings

No visual change. The existing copy button now works on insecure origins.

Checklist

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

Review follow-up

  • Merged current dev in 20ec936ad134f7f5c9461bc654282333c7f55ce6, preserving upstream Markdown link handling and message text pacing alongside the clipboard extraction. No history rewrite.
  • d58edc01828819cfcda72f2c6d33f6e0cf8b23b3 guards missing execCommand, cleans up the temporary textarea in finally, falls through after legacy-copy exceptions, and routes ToolErrorCard copy through the same helper without showing copied state on failure.
  • New tests cover missing/throwing/false/success legacy copy against missing/rejected/success modern copy. The browser tests click the actual ToolErrorCard copy button.
  • Independent pre-fix checks: helper tests exit 1 (9 pass, 6 fail); ToolErrorCard browser matrix exit 1 (1 pass, 11 fail).
  • Final independent checks with repository-required Bun 1.4.2: bun test src exits 0 (265 pass); package typecheck exits 0; ToolErrorCard browser matrix exits 0 (12 pass); the standard pre-push bun run check exits 0 (36/36 typecheck tasks; lint has advisory warnings, no errors).
  • Full component suite also passed: 146/146 on an isolated Storybook port. An earlier run hit rendering timeouts and was interrupted; the complete rerun passed without source changes. Browser clipboard APIs are controlled in these tests; no real OS clipboard integration claim.

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actions

Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

@ChristopheAwad

Copy link
Copy Markdown

Checked this against #50988, fix looks correct to me.

The old setupCodeCopy just bailed when navigator.clipboard was missing, which is always the case on plain HTTP. Routing it through writeClipboard makes sense, and moving it to clipboard.ts to avoid the cycle with message-content is the right call. Test covering the click path with clipboard hidden helps.

Two things I noticed while reading:

  • tool-error-card.tsx still calls navigator.clipboard.writeText directly, so that one will still fail on insecure origins.
  • clipboard.ts calls document.execCommand("copy") without checking it exists first. Might be worth a typeof guard + try/finally so the textarea always gets cleaned up and it can still fall through to navigator.clipboard.

But for the reported issue, this fixes it. Thanks for picking it up.

@thdxr
thdxr changed the base branch from v2 to dev October 10, 2026 19:16
@myagizmaktav

Copy link
Copy Markdown
Author

Thanks @ChristopheAwad — both findings were valid and are fixed in d58edc0: ToolErrorCard now uses the shared fallback, and legacy copy is guarded with guaranteed textarea cleanup before falling through. Copied feedback remains conditional on success.

Regression tests fail before the fix (6 helper failures; 11 browser failures) and pass afterward: 265 unit tests, 12 real ToolErrorCard click-path browser cases, package typecheck, and repository lint/typecheck. The full 146-case component suite also passes on an isolated Storybook port. Current dev was merged without rewriting history in 20ec936, preserving both sides. #50988 is already linked; no duplicate issue was needed.

@github-actions

Copy link
Copy Markdown
Contributor

Automated PR Cleanup

Thank you for contributing to opencode.

Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions.

This PR was closed because it matched the following cleanup criteria:

  • The PR was created more than 1 month ago
  • The PR had fewer than 2 positive reactions
  • Positive reactions are counted as thumbs-up, heart, celebration, or rocket reactions on the PR

PRs created within the last month are not affected by this cleanup.

If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate.

Thanks again for taking the time to contribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants