Repository navigation
fix(transcript): show Copied only when error or diff copy succeeds - #29218
Conversation
GitHub Actions
Inactive
GitHub Actions
Inactive
|
Found 10 test failures on Blacksmith runners: Failures
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Gallery before/after for
States
Pick other states with a Passes: CI passes on CI passes on Written by web and size checks: green Shipped in:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="webviews/src/agent-session/acpmux/conversation/TurnRows.tsx">
<violation number="1" location="webviews/src/agent-session/acpmux/conversation/TurnRows.tsx:677">
P2: Rapid clicks can let an older copy result overwrite a newer one, hiding `Copied` even though the latest copy succeeded. Track the latest request before applying its result, or disable the button until the current copy settles.</violation>
</file>
<file name="webviews/src/agent-session/acpmux/conversation/EditDiff.tsx">
<violation number="1" location="webviews/src/agent-session/acpmux/conversation/EditDiff.tsx:89">
P2: A stale copy attempt can overwrite the latest result because this handler has no request ordering. Track a request generation or disable the button, and update state only for the latest click.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| onClick={() => | ||
| void copyText(message).then( | ||
| () => setCopied(true), | ||
| void copyTextResult(message).then( |
There was a problem hiding this comment.
P2: Rapid clicks can let an older copy result overwrite a newer one, hiding Copied even though the latest copy succeeded. Track the latest request before applying its result, or disable the button until the current copy settles.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At webviews/src/agent-session/acpmux/conversation/TurnRows.tsx, line 677:
<comment>Rapid clicks can let an older copy result overwrite a newer one, hiding `Copied` even though the latest copy succeeded. Track the latest request before applying its result, or disable the button until the current copy settles.</comment>
<file context>
@@ -674,8 +674,8 @@ function TurnFailure({
onClick={() =>
- void copyText(message).then(
- () => setCopied(true),
+ void copyTextResult(message).then(
+ (accepted) => setCopied(accepted),
() => setCopied(false),
</file context>
| void copyText(patches.join("")).then( | ||
| () => setCopied(true), | ||
| void copyTextResult(patches.join("")).then( | ||
| (accepted) => setCopied(accepted), |
There was a problem hiding this comment.
P2: A stale copy attempt can overwrite the latest result because this handler has no request ordering. Track a request generation or disable the button, and update state only for the latest click.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At webviews/src/agent-session/acpmux/conversation/EditDiff.tsx, line 89:
<comment>A stale copy attempt can overwrite the latest result because this handler has no request ordering. Track a request generation or disable the button, and update state only for the latest click.</comment>
<file context>
@@ -85,8 +85,8 @@ export function EditDiff({
- void copyText(patches.join("")).then(
- () => setCopied(true),
+ void copyTextResult(patches.join("")).then(
+ (accepted) => setCopied(accepted),
() => setCopied(false),
)
</file context>
|
Owned by an agent session · session |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-fc739fd2f4e557ae7189
GitHub Actions
Inactive
|
Review (subagent claude-opus-5-5 medium): SHIP at e6d1293 The failed-turn "Copy error" button (TurnRows.tsx) and the edit-diff Copy button (EditDiff.tsx) now call Follow-ups:
|
GitHub Actions
Inactive
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-fc739fd2f4e557ae7189
GitHub Actions
Inactive
|
Review (subagent claude-opus-5-5 medium): SHIP at 17acfe8 Delta review over the prior SHIP at e6d1293. The only change is in webviews/test/agent-pane-copy-refusal.test.tsx: the Follow-ups: none |
GitHub Actions
Inactive
…d a customElements registry The edit-diff case renders EditDiff, whose lazily loaded hunks import @pierre/diffs: that module reads customElements at load, and an edit-less file hands it an empty patch it refuses. Either failed the test whenever the chunk resolved inside it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-fc739fd2f4e557ae7189
GitHub Actions
Inactive
|
Review (subagent claude-opus-5-5 medium): SHIP at c4fdbc1 Delta review over the prior SHIP at e6d1293. Only webviews/test/agent-pane-copy-refusal.test.tsx changed: the Follow-ups: none |
|
Review (claude opus, correctness first): SHIP at |
Why
HQ #4203 found that transcript Copy controls could say Copied when the async clipboard was refused and the legacy
document.execCommand("copy")fallback returned false.The current code has already improved the lower-level helper:
copyTextResult(text)returns a real boolean, preserves the fallback's false/throw result, removes its temporary textarea and restores focus. Fenced code and card corners already consume that result throughCopyButton. Two surviving direct consumers still use the legacycopyText(text): Promise<void>wrapper, which awaits the boolean but discards it:TurnRows.tsx;EditDiff.tsx.Both treat the wrapper's fulfilled void promise as proof the clipboard changed, and present Copied after a false fallback.
Fix
copyTextResulthelper.copiedstate from the actualboolean. Keep their rejection callback defensive. Successful fallback and later retry still work.Validation and tests
webviews/test/agent-pane-copy-refusal.test.tsxwith a controllable async clipboard andexecCommandfallback:copyTextResult(only its TypeScript signature erased) in a JS test harness: refusedfalse, accepted fallbacktrue, thrown fallbackfalse, async successtrue; 3 fallbacks appended and cleaned up 3 temporary nodes, with focus restored. Also inspected the two committed callers for use of the boolean.Ownership/overlap
CopyButtonpath; this PR fixes only the remaining direct consumers.CopyButtonremains unchanged; no open competing PR was identified for these exact controls.Acceptance
A fallback
falseor throw keeps Copy error and Copy diff in their original state; success changes to Copied; a subsequent successful retry recovers without an app restart.Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Fixes transcript Copy controls showing Copied when the clipboard was refused and the legacy
execCommand("copy")fallback returned false or threw.The failed-turn card's Copy error and the edit diff's Copy now use
copyTextResultand update theircopiedstate from the actual boolean, so a refused copy keeps the button unchanged and a successful retry recovers normally.webviews/test/agent-pane-copy-refusal.test.tsxcovering refusal, fallback success, thrown fallback, and retry behavior for both controls; the edit-diff case renders the real lazy-loaded hunks with a valid edit fixture.Written for commit 45b8c99. Summary will update on new commits.