[issues/559] Fix paste pipeline regression — restore multi-command fallback for AI assistant webview delivery#566
Merged
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
WalkthroughThis PR refactors the AI assistant clipboard-paste flow by introducing multi-command iteration with pre and post delays. Constants are reorganized into a dedicated AI assistant paste module, ChangesAI Assistant Clipboard Paste Flow
Sequence DiagramsequenceDiagram
participant Test as Integration Test
participant Adapter as VscodeAdapter
participant VSCode as VS Code Commands
participant Webview as AI Assistant Webview
Test->>Adapter: pasteClipboardToAiAssistant()
Adapter->>Adapter: wait FOCUS_TO_PASTE_DELAY_MS
loop For each command in AI_ASSISTANT_PASTE_COMMANDS
Adapter->>VSCode: dispatch editor.action.clipboardPasteAction
alt Command succeeds
VSCode-->>Adapter: resolved
Adapter->>Webview: clipboard content pasted
Adapter->>Adapter: wait CLIPBOARD_POST_PASTE_DELAY_MS
Adapter-->>Test: return true
else Command fails
VSCode-->>Adapter: rejected
Adapter->>Adapter: try next command in sequence
end
end
alt All commands failed
Adapter->>Adapter: log warning
Adapter-->>Test: return false
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly Related Issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
The PR introduces changes to the behavior of the Claude Code Chat integration, specifically regarding clipboard pasting and the logging of paste commands. There are also updates to test cases related to these features. Suggested test cases:
Generated by QA Gap Check (GPT-4o-mini via GitHub Models) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/rangelink-vscode-extension/src/__tests__/ide/vscode/VscodeAdapter.test.ts (1)
780-912: ⚡ Quick winReplace repeated semantic delay literals with named constants.
The new tests hardcode
200in multiple places; use SCREAMING_SNAKE_CASE constants (or import paste-delay constants) to keep assertions resilient and guideline-compliant.Proposed cleanup
describe('pasteClipboardToAiAssistant', () => { + const PRE_PASTE_DELAY_MS = 200; + const DEFAULT_POST_PASTE_DELAY_MS = 200; + it('should enforce 200 pre-paste delay and dispatch the first command on success', async () => { @@ expect(callOrder).toStrictEqual([ - 'delay-200', + `delay-${PRE_PASTE_DELAY_MS}`, 'cmd-editor.action.clipboardPasteAction', - 'delay-200', + `delay-${DEFAULT_POST_PASTE_DELAY_MS}`, ]); @@ - delay: 200, - prePasteDelay: 200, + delay: PRE_PASTE_DELAY_MS, + prePasteDelay: PRE_PASTE_DELAY_MS, @@ - delay: 200, - prePasteDelay: 200, + delay: DEFAULT_POST_PASTE_DELAY_MS, + prePasteDelay: PRE_PASTE_DELAY_MS,As per coding guidelines, "Define named constants for all numeric literals with semantic meaning using SCREAMING_SNAKE_CASE".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rangelink-vscode-extension/src/__tests__/ide/vscode/VscodeAdapter.test.ts` around lines 780 - 912, Replace the hardcoded 200 ms literals in these tests with a named constant (e.g., PRE_PASTE_DELAY_MS or DEFAULT_PRE_PASTE_DELAY_MS) or import the existing paste-delay constants and use them in expectations and spy calls related to adapter.pasteClipboardToAiAssistant; update references in assertions that check delay calls, callOrder entries that expect 'delay-200', and logger expects (where delay: 200 or prePasteDelay: 200) so they assert against the constant instead of the numeric literal while keeping identifiers like pasteClipboardToAiAssistant, adapter.delay, delaySpy, and mockLogger unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@packages/rangelink-vscode-extension/src/__tests__/ide/vscode/VscodeAdapter.test.ts`:
- Around line 780-912: Replace the hardcoded 200 ms literals in these tests with
a named constant (e.g., PRE_PASTE_DELAY_MS or DEFAULT_PRE_PASTE_DELAY_MS) or
import the existing paste-delay constants and use them in expectations and spy
calls related to adapter.pasteClipboardToAiAssistant; update references in
assertions that check delay calls, callOrder entries that expect 'delay-200',
and logger expects (where delay: 200 or prePasteDelay: 200) so they assert
against the constant instead of the numeric literal while keeping identifiers
like pasteClipboardToAiAssistant, adapter.delay, delaySpy, and mockLogger
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8dd22e80-f348-451e-a6c1-4fe4049e28d3
📒 Files selected for processing (12)
packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0.yamlpackages/rangelink-vscode-extension/src/__integration-tests__/helpers/index.tspackages/rangelink-vscode-extension/src/__integration-tests__/helpers/logBasedUiAssertions.tspackages/rangelink-vscode-extension/src/__integration-tests__/suite/builtInAiAssistants.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/ComposablePasteDestination.integration.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/insertFactories/aiAssistantInsertFactory.test.tspackages/rangelink-vscode-extension/src/__tests__/ide/vscode/VscodeAdapter.test.tspackages/rangelink-vscode-extension/src/constants/aiAssistantPasteConstants.tspackages/rangelink-vscode-extension/src/constants/index.tspackages/rangelink-vscode-extension/src/constants/pasteTimingConstants.tspackages/rangelink-vscode-extension/src/destinations/capabilities/insertFactories/aiAssistantInsertFactory.tspackages/rangelink-vscode-extension/src/ide/vscode/VscodeAdapter.ts
💤 Files with no reviewable changes (1)
- packages/rangelink-vscode-extension/src/constants/pasteTimingConstants.ts
…llback for AI assistant webview delivery ## Summary The "single clipboard write per R-* operation" refactor (PR #556, `issues/547`) inadvertently reduced a three-command paste fallback chain to a single `editor.action.clipboardPasteAction`. That command only reaches VS Code text editors — for webview-hosted chat inputs (Claude Code, Cursor AI, GitHub Copilot Chat, custom AI Tier 1/2 webview targets) the system-level fallbacks `execPaste` / `paste` were what actually delivered content. Without them the dispatch returned success, but no content arrived in the chat input. This PR restores the fallback chain, logs the successful command for diagnostics, and converts the silent-pass `claude-code-004/005` tests to verdict-based assertions so the same class of regression cannot land green in the future. ## Changes - Renamed `src/constants/pasteTimingConstants.ts` → `aiAssistantPasteConstants.ts` and added `AI_ASSISTANT_PASTE_COMMANDS = ['editor.action.clipboardPasteAction', 'execPaste', 'paste']` alongside the existing delay constants. The file now correctly reflects its scope (was a name-vs-content drift from #556 that removed the commands). - `VscodeAdapter.pasteTextFromClipboard()` now iterates `AI_ASSISTANT_PASTE_COMMANDS` and stops on the first success. The `command` field is included in the `Clipboard paste succeeded` log so tests can assert which command delivered the paste. Per-command failures log at `debug` with `Paste command failed, trying next`; an all-failed path logs at `warn` with `allCommandsFailed: true`. - Inline JSDoc on `pasteTextFromClipboard` explains WHY the fallback chain exists — non-obvious enough that the next refactor would otherwise repeat the same mistake. - Added `assertPasteCommandLogged(lines, { command })` helper to `logBasedUiAssertions.ts` for integration-test assertions on which paste command succeeded. - Updated `VscodeAdapter.test.ts` `pasteTextFromClipboard` describe block: 5 tests now cover the fallback chain (first-success, fall-through-to-second, all-fail, default post-paste delay, custom post-paste delay). All assertions include the `command` field per the test contract. - Converted `claude-code-004` and `claude-code-005` from `waitForHuman` to `waitForHumanVerdict`. The silent-pass trap is closed for these specific TCs (broader audit tracked in #560). Setup is now programmatic (file open + selection via `vscode.workspace.openTextDocument` + `editor.selection`) and the send fires via `executeCommand(CMD_COPY_LINK_RELATIVE)` rather than asking the human to press the chord. - Added the wave-1 Claude Code TCs from #483: `claude-code-001` (automated picker-inspection in `standardSuite`, uses `openAndDismiss` from #561), `claude-code-002` (assisted send-delivery with `waitForHumanVerdict`), and the deletion of `claude-code-003` (superseded by `claude-code-005`'s richer warm-path scenario). YAML flips for 001 → `automated: true` and 002 → `automated: assisted`. ## Key Discoveries - The 547 refactor's "Test Plan" claimed `pnpm test:release passes` — technically true, but the with-extensions tests were not in that baseline AND `claude-code-004/005` used `waitForHuman` (Cancel-to-resolve) rather than `waitForHumanVerdict` (PASS/FAIL-to-resolve). Code-side log assertions stayed green even though the webview received nothing. The regression therefore landed undetected. The verdict-helper conversion in this PR closes that specific silent-pass trap; the broader audit lives in #560. - The `dummy-ai-extension` fixture is already webview-based and exposes `dummyAi.getText` for reading what landed in the webview textareas — the missing piece for a fully automated webview paste-delivery test. Adding one is blocked on a new programmatic bind command for custom AI assistants (the current bind path requires a human picker selection); that command is out of scope here and will be filed as a follow-up issue. - The 547 paste pipeline was audited beyond the bug-fix change: padding is correctly pre-applied at all 4 call sites (`LinkGenerator`, `TextSelectionPaster`, `FilePathPaster`, `TerminalSelectionService`); the two-delay model is used consistently in the single `pasteTextFromClipboard` chokepoint; the `focusCommands` fallback iteration in `AIAssistantFocusCapability` is intact. No additional regressions found. ## Test Plan - [x] All existing unit tests pass — 107 suites, 228 tests in `VscodeAdapter.test.ts` (including 5 new fallback tests), 98.42% statement coverage. - [x] `pnpm test:release:automated` — 165 of 175 passing. The 10 failures are pre-existing flakes (`langswitch-binding-*`, `smart-padding-*`, `stale-viewcolumn-001`) on non-AI-assistant code paths, same as noted in PR #561's description. My changes don't touch terminal/editor paste paths. - [x] QA coverage validator passes — 121 automated / 115 assisted / 12 false markers all align with integration tests. - [ ] **Assisted verification (definitive bug-fix proof):** `pnpm test:release:with-extensions --grep "claude-code-00[245]"` runs `claude-code-002` (cleanest reproduction), `claude-code-004` (cold panel paste), and `claude-code-005` (cold→warm sequence). Each prompts PASS/FAIL on whether the RangeLink actually arrived in Claude Code's chat input. All three must produce PASS verdicts. - [ ] CI `test-with-extensions` job (from #564) runs green on the PR. ## Related - Closes #559 - Resolves the regression introduced by #556 - Companion to #560 (silent-pass-trap audit) — this PR fixes the 547-introduced subset (`claude-code-004/005`); the broader audit remains open. - Folds in the Wave-1 Claude Code work from #483 (`claude-code-001` / `claude-code-002` integration tests + the `claude-code-003` deletion). - Complements #563 — this PR fixes the bug; #563 backfills coverage that would have caught it. They land independently; whichever merges first, the other rebases. - Follow-up issue to be filed: a programmatic bind command for custom AI assistants (currently the bind path requires a human picker selection), which would unblock a fully automated webview paste-delivery test using the dummy-ai-extension's `dummyAi.getText` reader.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The "single clipboard write per R-* operation" refactor (PR #556,
issues/547) inadvertently reduced a three-command paste fallback chain to a singleeditor.action.clipboardPasteAction. That command only reaches VS Code text editors — for webview-hosted chat inputs (Claude Code, Cursor AI, GitHub Copilot Chat, custom AI Tier 1/2 webview targets) the system-level fallbacksexecPaste/pastewere what actually delivered content. Without them the dispatch returned success, but no content arrived in the chat input. This PR restores the fallback chain, logs the successful command for diagnostics, and converts the silent-passclaude-code-004/005tests to verdict-based assertions so the same class of regression cannot land green in the future.Changes
src/constants/pasteTimingConstants.ts→aiAssistantPasteConstants.tsand addedAI_ASSISTANT_PASTE_COMMANDS = ['editor.action.clipboardPasteAction', 'execPaste', 'paste']alongside the existing delay constants. The file now correctly reflects its scope (was a name-vs-content drift from [issues/547] Single clipboard write per R-* operation #556 that removed the commands).VscodeAdapter.pasteTextFromClipboard()now iteratesAI_ASSISTANT_PASTE_COMMANDSand stops on the first success. Thecommandfield is included in theClipboard paste succeededlog so tests can assert which command delivered the paste. Per-command failures log atdebugwithPaste command failed, trying next; an all-failed path logs atwarnwithallCommandsFailed: true.pasteTextFromClipboardexplains WHY the fallback chain exists — non-obvious enough that the next refactor would otherwise repeat the same mistake.assertPasteCommandLogged(lines, { command })helper tologBasedUiAssertions.tsfor integration-test assertions on which paste command succeeded.VscodeAdapter.test.tspasteTextFromClipboarddescribe block: 5 tests now cover the fallback chain (first-success, fall-through-to-second, all-fail, default post-paste delay, custom post-paste delay). All assertions include thecommandfield per the test contract.claude-code-004andclaude-code-005fromwaitForHumantowaitForHumanVerdict. The silent-pass trap is closed for these specific TCs (broader audit tracked in Silent-pass trap audit —waitForHumanused for visual verification #560). Setup is now programmatic (file open + selection viavscode.workspace.openTextDocument+editor.selection) and the send fires viaexecuteCommand(CMD_COPY_LINK_RELATIVE)rather than asking the human to press the chord.claude-code-001(automated picker-inspection instandardSuite, usesopenAndDismissfrom [issues/557] Automate 33 assisted integration tests viacloseQuickOpendismissal #561),claude-code-002(assisted send-delivery withwaitForHumanVerdict), and the deletion ofclaude-code-003(superseded byclaude-code-005's richer warm-path scenario). YAML flips for 001 →automated: trueand 002 →automated: assisted.Key Discoveries
pnpm test:release passes— technically true, but the with-extensions tests were not in that baseline ANDclaude-code-004/005usedwaitForHuman(Cancel-to-resolve) rather thanwaitForHumanVerdict(PASS/FAIL-to-resolve). Code-side log assertions stayed green even though the webview received nothing. The regression therefore landed undetected. The verdict-helper conversion in this PR closes that specific silent-pass trap; the broader audit lives in Silent-pass trap audit —waitForHumanused for visual verification #560.dummy-ai-extensionfixture is already webview-based and exposesdummyAi.getTextfor reading what landed in the webview textareas — the missing piece for a fully automated webview paste-delivery test. Adding one is blocked on a new programmatic bind command for custom AI assistants (the current bind path requires a human picker selection); that command is out of scope here and will be filed as a follow-up issue.LinkGenerator,TextSelectionPaster,FilePathPaster,TerminalSelectionService); the two-delay model is used consistently in the singlepasteTextFromClipboardchokepoint; thefocusCommandsfallback iteration inAIAssistantFocusCapabilityis intact. No additional regressions found.Test Plan
VscodeAdapter.test.ts(including 5 new fallback tests), 98.42% statement coverage.pnpm test:release:automated— 165 of 175 passing. The 10 failures are pre-existing flakes (langswitch-binding-*,smart-padding-*,stale-viewcolumn-001) on non-AI-assistant code paths, same as noted in PR [issues/557] Automate 33 assisted integration tests viacloseQuickOpendismissal #561's description. My changes don't touch terminal/editor paste paths.pnpm test:release:with-extensions --grep "claude-code-00[245]"runsclaude-code-002(cleanest reproduction),claude-code-004(cold panel paste), andclaude-code-005(cold→warm sequence). Each prompts PASS/FAIL on whether the RangeLink actually arrived in Claude Code's chat input. All three must produce PASS verdicts.test-with-extensionsjob (from [issues/562] Add CI job for integration tests with marketplace extensions #564) runs green on the PR.Related
waitForHumanused for visual verification #560 (silent-pass-trap audit) — this PR fixes the 547-introduced subset (claude-code-004/005); the broader audit remains open.claude-code-001/claude-code-002integration tests + theclaude-code-003deletion).dummyAi.getTextreader.Summary by CodeRabbit
Bug Fixes
Tests
Refactor