Skip to content

Image viewer: a copy that cannot be drawn says so, on the image it copied - #18431

Merged
teamleaderleo merged 32 commits into
feat-cmux-nextfrom
feat-cmux-next-image-copy-failure
Oct 7, 2026
Merged

teamleaderleo merged 32 commits into
feat-cmux-nextfrom
feat-cmux-next-image-copy-failure

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

The image viewer's Copy button now always gives an answer, and that answer only shows on the image it copied. This follows up a review of #18200.

  • No silent hang: Copy draws the image to a canvas, then hands the PNG to the clipboard as a promise. If drawing threw, that promise never settled, so the button never said anything. That happens on a tainted canvas, for example an SVG with HTML inside. Now a draw that throws, a canvas with no 2D context, or an image with no size fails the copy, and the button says "Couldn't copy image".
  • No blank copy: an image with no size used to be copied as a blank 1x1 PNG, reported as "Copied". It now fails instead.
  • Right image: if you copy an image, then step to the next one before the copy finishes, the result no longer shows on the new image.

Follows #18200 (the image viewer), which landed as f2eb10f.

Testing

  • Red commit 4672c0f. Tests run in webviews/ on the local machine.
    • bun test src/agent-session/acpmux/conversation/clipboard.test.ts: 1 pass, 1 fail at the red commit (the draw error escaped instead of failing the copy); 2 pass after the fix.
    • bun test src/agent-session/acpmux/conversation/imageViewer.test.tsx: 11 pass, 1 fail at the red commit (the late result showed "Copied" on the next image); 12 pass after the fix.
  • bun test src/agent-session/acpmux/conversation test/ui-rules.test.ts test/gallery-coverage.test.ts test/pane-english.test.ts: 194 pass.
  • vp check passes on the changed files, and the typecheck passes on all 1023 files.

Changelog

none (the image viewer has not shipped yet; #18200 carries its changelog line)

Proof

No visual change: the button's labels and icons are the ones #18200 added. The tests above cover the behavior.

Checklist

  • Behavior changes have added or updated tests, or Testing says why not
  • UI, settings, menu, schema, help-text or user-facing docs change: localization audited. No new strings; the change reuses image.copyFailed and image.copy.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

The image viewer's Copy button now always answers, and its answer stays on the image it copied.

  • A copy that cannot be drawn or encoded (tainted canvas, no 2D context, zero size) fails and shows "Couldn't copy image" instead of hanging; a zero-size image used to be copied as a blank 1x1 PNG and reported "Copied".
  • If you step to another image before a copy finishes, the result no longer appears on the new image.

Written for commit 84e78d7. Summary will update on new commits.

View guided diff Turn on auto-fix


Note

Low Risk
Localized clipboard and UI feedback in the unreleased image viewer; no auth or data-path changes.

Overview
Image viewer copy now always resolves and ties success/failure feedback to the image that was copied.

copyImage rejects when the source cannot be turned into a PNG (zero dimensions, missing 2D context, tainted canvas / draw or encode errors) instead of leaving the clipboard promise pending or copying a blank 1×1 PNG as “Copied”. The Copy handler records the clicked src and only updates the button label when that same image is still on screen when the async copy finishes, so stepping to another image before copy completes no longer shows “Copied” on the wrong slide.

Adds unit tests for copyImage failure paths and a viewer test for the late-completion case.

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

teamleaderleo and others added 30 commits October 6, 2026 20:03
A reply's inline image opens a viewer over the pane: zoom about the
pointer (pinch, Command-scroll, double click, + - 0), pan a zoomed image,
arrows and the arrow keys through every image of the chat, copy as PNG,
Escape or a click beside the image to close. The mock light-theme chat
shows three screens to open.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
agent-pane.image-viewer: a reply with three screenshots, and images in
two replies the viewer's arrows go through. The opened viewer needs a
click, so its states wait for play steps.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Red: the viewer's image list keeps an image on a fence's later line,
loses images after a lone backtick, with parentheses in the target or
inside bold; Tab leaves the viewer; an image the list misses opens
without its name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The viewer's image list now comes from Markdown.tsx's own block parser
and inline pattern (inlineImages), so it holds exactly the images a
reply draws: none from code, and those after a lone backtick, with
parentheses in the target or inside bold. An image the list still
misses opens under its own name. WebKit's pinch (gesture events) zooms
about the pointer; the stage's padding is symmetric so zoom stays about
the pointer; Tab wraps inside the viewer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e data URLs

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Notes draw under the reply by first reference, so the viewer lists their
images there too; a reply whose only image has an upper-case data URL
scheme is read, as Markdown.tsx draws it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The mock chat and the gallery entry show a chart, a gradient and a
diagram instead of drawn app screens: previews are never mockups of cmux.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The viewer is a ui/Dialog (Base UI): it traps focus, makes the pane
behind inert, closes on Escape and returns focus to the image, replacing
the hand-rolled role, tab index and Tab handling the UI rules forbid.
Only its own image keys (arrows, + - 0) stay a key handler, marked
ui-allow.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ext-image-viewer

# Conflicts:
#	webviews/src/agent-session/acpmux/conversation/Markdown.tsx
…e reply budget

Red: since base moved data URLs over 4 KB behind short refs and capped a
reply at 8 MB, chatImages lists none of a reply's long images.

Focused: bun test src/agent-session/acpmux/conversation/imageViewer.test.tsx -t budget
(fail: 0 listed, 6 drawn)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…he reply budget

inlineImages mirrors renderInline again: it takes the URLs back from
capDataUrls' short refs (passed down into bold, italic and link text),
and chatImages drops the reply's images past dataUrlsOverBudget, which
draw as their name.

Focused: bun test src/agent-session/acpmux/conversation/imageViewer.test.tsx -t budget
(pass)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Red since the move to ui/Dialog: a press inside the viewer focuses the
dialog popup, above the body that handled the image keys, and the
wheel and gesture listeners ran before the portal mounted the stage.

Focused: bun test src/agent-session/acpmux/conversation/imageViewer.test.tsx
(fail: keys stay at scale(1); Control-scroll not prevented)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ui/Dialog passes onKeyDown to its popup, which takes focus after a press
inside, so the viewer's arrows and + - 0 keys go there. The stage is a
callback ref, so the pinch and wheel listeners attach once the portal
mounts it.

Focused: bun test src/agent-session/acpmux/conversation/imageViewer.test.tsx
(11 pass)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ext-image-viewer

# Conflicts:
#	webviews/src/agent-session/acpmux/App.tsx
The opened viewer as play steps on the reply with three images, and
anchors that prove nothing under the viewer moves. Regenerated agent
pane bundle for the base merge.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ext-image-viewer

# Conflicts:
#	webviews/src/agent-session/acpmux/Localizable.xcstrings
#	webviews/src/agent-session/acpmux/conversation/Markdown.tsx
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ext-image-viewer

# Conflicts:
#	webviews/src/agent-session/acpmux/conversation/Markdown.tsx
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ext-image-viewer

# Conflicts:
#	webviews/src/agent-session/acpmux/App.tsx
Base no longer commits the web bundles: the agent-pane bundle files are
dropped; only GENERATED.md stays.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Picks up the bundle-removal CI fixes. The string catalog keeps base's text
and adds only this branch's keys.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The merge kept this branch's generated strings and dropped base's
composer.connectSSH and composer.connectCloud.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ys on its image

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…mage

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

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: manaflow-ai/cmux/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 591e4b8e-fb28-4d67-a92e-913681b447d6

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
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@blacksmith-sh

blacksmith-sh Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Blacksmith runners detected OOM events on the following jobs:

Job Details
web / react-apps-check View Job

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

CI fast guards failed on 0d7bb37c83 (https://github.com/manaflow-ai/cmux/actions/runs/37684327137). It does not block the merge; a red guard merged into main breaks it for every open PR.

Check that push and pull_request path filters agree

Test: python3 tests/test_ci_workflow_path_filter_parity.py

Traceback (most recent call last):
  File "/home/runner/_work/cmux/cmux/tests/test_ci_workflow_path_filter_parity.py", line 247, in <module>
    test_every_push_only_pattern_is_still_needed()
  File "/home/runner/_work/cmux/cmux/tests/test_ci_workflow_path_filter_parity.py", line 183, in test_every_push_only_pattern_is_still_needed
    assert not stale, (
           ^^^^^^^^^
AssertionError: cmux-next.yml: Packages/Shared/**, Packages/iOS/** is no longer push-only. Remove it from PUSH_ONLY_PATTERNS.

Fix: the assertion above names what the guard expects; change the tree to match it.
Reproduce in seconds, no build: scripts/ci/guards-local.sh --step 'Check that push and pull_request path filters agree'

Validate package test selection

Test: python3 tests/test_ci_select_package_tests.py

    result, count = run_package_step("CmuxUpdater", [(startup, 1), (passed, 0)])
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/_work/cmux/cmux/tests/test_ci_select_package_tests.py", line 102, in run_package_step
    count = int((root / "calls").read_text())
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/pathlib.py", line 1029, in read_text
    with self.open(mode='r', encoding=encoding, errors=errors) as f:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/pathlib.py", line 1015, in open
    return io.open(self, mode, buffering, encoding, errors, newline)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/tmpifjmcxq1/calls'

Fix: the assertion above names what the guard expects; change the tree to match it.
Reproduce in seconds, no build: scripts/ci/guards-local.sh --step 'Validate package test selection'

Validate local hook trust boundary

Test: python3 tests/test_preflight_trust.py

  • PreflightTrustTests.test_hook_installation_has_no_candidate_code_runner
AssertionError: True is not false

Fix: the assertion above names what the guard expects; change the tree to match it.
Reproduce in seconds, no build: scripts/ci/guards-local.sh --step 'Validate local hook trust boundary'

Validate the package test lane's single-suite phase

suite run failed (1): ::error title=No macOS SDK::package-test-lane.sh: xcrun found no macOS SDK in /Applications/Xcode_26.6.app/Contents/Developer (got '')
no build of the package: ''
no filtered test run: ''
string catalogs were not compiled in the package: ''

... and 11 more failing tests in the run log.
Fix: the assertion above names what the guard expects; change the tree to match it.
Reproduce in seconds, no build: scripts/ci/guards-local.sh --step 'Validate the package test lane'"'"'s single-suite phase'

Validate notification semantics gates

Test: python3 tests/test_ci_notification_semantics.py

  • NotificationSemanticsTests.test_a_failing_package_does_not_hide_later_packages
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/tmpekvbpul3/calls.jsonl'
  • NotificationSemanticsTests.test_packages_prebuild_in_parallel_then_test_serially
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/tmpofg64oya/calls.jsonl'

Fix: the assertion above names what the guard expects; change the tree to match it.
Reproduce in seconds, no build: scripts/ci/guards-local.sh --step 'Validate notification semantics gates'

Validate fork runner routing

Test: python3 tests/test_ci_fork_runner_routing.py

  • ForkRunnerRoutingTests.test_fork_pull_requests_into_manaflow_ai_never_reach_an_owned_runner_label
AssertionError: Lists differ: ['cmux-next.yml:956 has no top-level fork [6389 chars] }}"] != []

First list contains 11 additional elements.
First extra element 0:
'cmux-next.yml:956 has no top-level fork pull-request branch: runs-on: ${{ needs.macos-placement.outputs.runner || github.repository_owner != \'manaflow-ai\' && \'macos-26\' || (github.event_name == \'pull_request\' && github.event.pull_request.head.repo.full_name != github.repository && \'blacksmith-6vcpu-macos-26\' || vars.CI_PR_POOL_OWNED == \'1\' && contains(fromJSON(\'["pull_request","push","workflow_dispatch"]\'), github.event_name) && (github.run_attempt <= 2 || github.triggering_actor != \'github-actions[bot]\') && vars.CI_SIDE_LANE_RUNNER || vars.MACOS_RUNNER_26 || \'blacksmith-6vcpu-macos-26\') }}'

Diff is 6836 characters long. Set self.maxDiff to None to see it.
  • ForkRunnerRoutingTests.test_no_workflow_falls_back_to_blacksmith_outside_manaflow_ai
AssertionError: Lists differ: ["cmux-next.yml:956: blacksmith-6vcpu-maco[2411 chars] }}"] != []

First list contains 10 additional elements.
First extra element 0:
"cmux-next.yml:956: blacksmith-6vcpu-macos-26 is selectable outside manaflow-ai, where no Blacksmith runner exists; start the expression with the owner fork branch, e.g. ${{ github.repository_owner != 'manaflow-ai' && 'ubuntu-24.04' || ... }}"

Diff is 2673 characters long. Set self.maxDiff to None to see it. : 
cmux-next.yml:956: blacksmith-6vcpu-macos-26 is selectable outside manaflow-ai, where no Blacksmith runner exists; start the expression with the owner fork branch, e.g. ${{ github.repository_owner != 'manaflow-ai' && 'ubuntu-24.04' || ... }}
cmux-next.yml:964: blacksmith-6vcpu-macos-26 is selectable outside manaflow-ai, where no Blacksmith runner exists; start the expression with the owner fork branch, e.g. ${{ github.repository_owner != 'manaflow-ai' && 'ubuntu-24.04' || ... }}
cmux-next.yml:1177: blacksmith-6vcpu-macos-26 is selectable outside manaflow-ai, where no Blacksmith runner exists; start the expression with the owner fork branch, e.g. ${{ github.repository_owner != 'manaflow-ai' && 'ubuntu-24.04' || ... }}
cmux-next.yml:1186: blacksmith-6vcpu-macos-26 is selectable outside manaflow-ai, where no Blacksmith runner exists; start the expression with the owner fork branch, e.g. ${{ github.repository_owner != 'manaflow-ai' && 'ubuntu-24.04' || ... }}
cmux-next.yml:1347: blacksmith-6vcpu-macos-26 is sel ...

Fix: the assertion above names what the guard expects; change the tree to match it.
Reproduce in seconds, no build: scripts/ci/guards-local.sh --step 'Validate fork runner routing'

Validate reusable guard workflow structure

  • Wiring.test_main_full_suite_admission_compiles_on_the_seed_pool_and_xcode
AssertionError: 'blacksmith-6vcpu-macos-15' != 'pool-pr'
- blacksmith-6vcpu-macos-15
+ pool-pr
  • Wiring.test_no_retry_attempt_of_any_workflow_takes_an_owned_label (workflow='cmux-next-batch.yml', job='regen', attempt='3')
AssertionError: True is not false : ['glaeda-side-std-xcode-26.6']
  • Wiring.test_no_retry_attempt_of_any_workflow_takes_an_owned_label (workflow='cmux-next-batch.yml', job='build', attempt='3')
AssertionError: True is not false : ['glaeda-side-std-xcode-26.6']

Fix: the assertion above names what the guard expects; change the tree to match it.
Reproduce in seconds, no build: scripts/ci/guards-local.sh --step 'Validate reusable guard workflow structure'

Run canonical CMUX CI guard profile

test_cmux_next_does_not_run_full_suite_for_unrelated_packages

Fix: the assertion above names what the guard expects; change the tree to match it.
Reproduce in seconds, no build: scripts/ci/guards-local.sh --step 'Run canonical CMUX CI guard profile'

Agents: python3 scripts/ci/guard_attribution.py fix applies the mechanical fixes locally. This comment is updated in place on each push.

Base automatically changed from feat-cmux-next-image-viewer to feat-cmux-next October 7, 2026 20:53
@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Unclear: web / react-apps-check failed: no known signature; failed step: Test webviews.

CI failed on 84e78d77d1 (run 37685621898 attempt 1): 1 unknown.

Job Verdict Why
web / react-apps-check unknown no known signature; failed step: Test webviews

Not re-run automatically: web / react-apps-check is not a machine failure.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's; yours means the failing file is one this PR changes, also red on main that main's latest full suite fails the same way, seen on other PRs that it failed on another pull request's run lately.

# Conflicts:
#	webviews/src/agent-session/acpmux/conversation/ImageViewer.tsx
#	webviews/src/agent-session/acpmux/conversation/clipboard.ts
#	webviews/src/agent-session/acpmux/conversation/imageViewer.test.tsx
@teamleaderleo
teamleaderleo merged commit 6314b98 into feat-cmux-next Oct 7, 2026
64 of 67 checks passed
@teamleaderleo
teamleaderleo deleted the feat-cmux-next-image-copy-failure branch October 7, 2026 21:25
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

The feat-cmux-next push run https://github.com/manaflow-ai/cmux/actions/runs/37689519735 at 3e0a1ad failed: cmux-next generated files.
Those jobs last passed at a14deb7. This pull request is one of 4 merged since: #18399, #18200, #18401, #18431.
If the failure is in your change, fix forward on feat-cmux-next. Pull requests run only the tiers their paths reach (docs/ci/cmux-next-tiers.md); label a batch PR full-ci to run them all before merging.

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.

1 participant