Skip to content

[issues/509] Block 4 — Clipboard Preservation (6 TCs, assisted)#530

Merged
couimet merged 3 commits into
mainfrom
issues/509-block4
Apr 28, 2026
Merged

[issues/509] Block 4 — Clipboard Preservation (6 TCs, assisted)#530
couimet merged 3 commits into
mainfrom
issues/509-block4

Conversation

@couimet
Copy link
Copy Markdown
Owner

@couimet couimet commented Apr 27, 2026

Summary

Converts 6 automated: false clipboard-preservation TCs to automated: assisted by adding a new integration test suite that wraps the human interaction with programmatic setup and assertion. The test writes a sentinel to the clipboard, waits for the human to perform the RangeLink operation, then reads the clipboard back to assert restored vs. changed — no clipboard state is inferred from human observation alone.

Changes

  • New clipboardPreservationAssisted.test.ts with 6 assisted TCs (001, 002, 004, 005, 007, 009): each TC creates the required destination programmatically (bound terminal, bound text file, or no destination) so the human only needs to perform the core RangeLink operation
  • Updated qa-test-cases-v1.1.0-003.yaml: TC 001, 002, 004, 005, 007, 009 flipped from automated: falseautomated: assisted with updated descriptions
  • QA counts after this block: 115 assisted / 69 automated / 43 false (was 109/69/49)

Test Plan

  • All existing unit tests pass (1842/1842)
  • New integration tests validated: 6/6 passing in test:release run (2026-04-27)
  • validate-qa-coverage.sh passes via pnpm test:release

Related

Summary by CodeRabbit

  • Tests
    • Added an assisted clipboard preservation test suite with deterministic, assertable scenarios.
    • Improved validation to assert clipboard restore/change behavior across preserve modes (always/never).
    • Expanded coverage for UI flows (binding, paste, picker dismissal) and added robust setup/teardown for repeatable runs.

## Summary

Converts 6 `automated: false` clipboard-preservation TCs to `automated: assisted` by adding a new integration test suite that wraps the human interaction with programmatic setup and assertion. The test writes a sentinel to the clipboard, waits for the human to perform the RangeLink operation, then reads the clipboard back to assert restored vs. changed — no clipboard state is inferred from human observation alone.

## Changes

- New `clipboardPreservationAssisted.test.ts` with 6 assisted TCs (001, 002, 004, 005, 007, 009): each TC creates the required destination programmatically (bound terminal, bound text file, or no destination) so the human only needs to perform the core RangeLink operation
- Updated `qa-test-cases-v1.1.0-003.yaml`: TC 001, 002, 004, 005, 007, 009 flipped from `automated: false` → `automated: assisted` with updated descriptions
- QA counts after this block: 115 assisted / 69 automated / 43 false (was 109/69/49)

## Test Plan

- [x] All existing unit tests pass (1842/1842)
- [x] New integration tests validated: 6/6 passing in `test:release` run (2026-04-27)
- [x] `validate-qa-coverage.sh` passes via `pnpm test:release`

## Related

- Part of #509
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 27, 2026

Warning

Rate limit exceeded

@couimet has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 50 minutes and 43 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 66e301ea-48b0-4442-bc9f-849d72899357

📥 Commits

Reviewing files that changed from the base of the PR and between 06f1a7a and 2f7df29.

📒 Files selected for processing (1)
  • packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yaml

Walkthrough

Reworks clipboard-preservation QA cases and integration tests to use an auto-initialized clipboard sentinel, deterministic UI flows (explicit binds/destination names), and an "assisted" test harness that uses harness assertions (e.g., assertClipboardRestored / assertClipboardChanged) and deterministic waitForHuman steps.

Changes

Cohort / File(s) Summary
QA Test Case Configuration
packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yaml
Reworked Clipboard Preservation cases (IDs clipboard-preservation-001, -002, -004, -005, -007, -009) to auto-write a clipboard sentinel, use deterministic UI flows and explicit destination binds, replace human-read results with harness assertions (assertClipboardRestored / assertClipboardChanged), and change automation status from false to assisted.
Assisted Integration Tests
packages/rangelink-vscode-extension/src/__integration-tests__/suite/clipboardPreservation.test.ts
Added an "Assisted" test suite that introduces assisted-banner setup, logger and temporary resource tracking, expanded waitForHuman-driven scenarios covering clipboard.preserve modes (always/never) across R-L, R-V, AI assistant paste, fresh terminal binds and picker dismissal; moved/rewrote original non-assisted cases (003, 006), added harness assertions, and implemented comprehensive teardown (reset settings, unbind, clear Dummy AI, dispose terminals, close editors, cleanup files).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

Poem

🐰
I hid a sentinel where clipboards sleep,
Assisted I watch while humans take a peep,
Deterministic hops keep states defined,
Restored or changed — no mystery to mind,
A tiny rabbit, testing—swift and neat.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and specifically describes the main change: converting six clipboard-preservation test cases to assisted automation mode, matching the core objective.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issues/509-block4

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 and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 27, 2026

⚠️ QA Coverage Gap Detected

The PR introduces changes to clipboard preservation test cases, modifying scenarios and expected results, which may require additional coverage.

Suggested test cases:

  • Clipboard Preservation: Verify clipboard content is restored after R-L operation (manual — Involves verifying clipboard content after a command that cannot be automated.)
  • Clipboard Preservation: Verify clipboard content is restored after R-V operation from terminal (manual — Involves verifying clipboard content after a command that cannot be automated.)
  • Clipboard Preservation: Verify clipboard content is restored after AI assistant paste (manual — Involves verifying clipboard content after a command that cannot be automated.)
  • Clipboard Preservation: Verify clipboard content is restored after terminal paste (manual — Involves verifying clipboard content after a command that cannot be automated.)

Generated by QA Gap Check (GPT-4o-mini via GitHub Models)

@couimet
Copy link
Copy Markdown
Owner Author

couimet commented Apr 28, 2026

@coderabbitai review - I was rate-limited in #530 (comment)

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 28, 2026

@couimet Sure! Triggering a fresh review of the PR now.


🧠 Learnings used
Learnt from: CR
Repo: couimet/rangeLink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-19T22:46:31.271Z
Learning: Applies to qa/**/*.yaml : The `automated` field in QA YAML requires a matching integration test on the branch - `true` requires non-`[assisted]` test, `assisted` requires `[assisted]`-tagged test, `false` for non-automatable scenarios

Learnt from: CR
Repo: couimet/rangeLink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-19T22:46:31.271Z
Learning: Applies to (qa/**/* | src/__integration-tests__/**/*) : Any change to files in `packages/rangelink-vscode-extension/qa/` or `src/__integration-tests__/` MUST be validated by running `pnpm test:release`

Learnt from: CR
Repo: couimet/rangeLink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-19T22:46:31.271Z
Learning: Applies to !(pattern) : When renaming a TC ID in QA YAML or integration tests, update ALL occurrences in both `qa/*.yaml` (latest file only) and `src/__integration-tests__/suite/*.test.ts` and run validation script

Learnt from: CR
Repo: couimet/rangeLink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-19T22:46:31.271Z
Learning: Applies to qa/**/*.yaml : TC IDs use domain-based naming (e.g., `link-navigation-001`, `text-editor-destination-001`) - never prefix with `bugfix-`, `feature-`, `changed-`, or origin-based prefixes
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@couimet
Copy link
Copy Markdown
Owner Author

couimet commented Apr 28, 2026

@coderabbitai review - I was rate-limited in #530 (comment)

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 28, 2026

@couimet Sure! Triggering a fresh review of the PR now.


🧠 Learnings used
Learnt from: CR
Repo: couimet/rangeLink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-19T22:46:31.271Z
Learning: Applies to qa/**/*.yaml : The `automated` field in QA YAML requires a matching integration test on the branch - `true` requires non-`[assisted]` test, `assisted` requires `[assisted]`-tagged test, `false` for non-automatable scenarios

Learnt from: CR
Repo: couimet/rangeLink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-19T22:46:31.271Z
Learning: Applies to (qa/**/* | src/__integration-tests__/**/*) : Any change to files in `packages/rangelink-vscode-extension/qa/` or `src/__integration-tests__/` MUST be validated by running `pnpm test:release`

Learnt from: CR
Repo: couimet/rangeLink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-19T22:46:31.271Z
Learning: Applies to !(pattern) : When renaming a TC ID in QA YAML or integration tests, update ALL occurrences in both `qa/*.yaml` (latest file only) and `src/__integration-tests__/suite/*.test.ts` and run validation script

Learnt from: CR
Repo: couimet/rangeLink PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-19T22:46:31.271Z
Learning: Applies to qa/**/*.yaml : TC IDs use domain-based naming (e.g., `link-navigation-001`, `text-editor-destination-001`) - never prefix with `bugfix-`, `feature-`, `changed-`, or origin-based prefixes
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yaml`:
- Around line 599-603: The QA spec for test case clipboard-preservation-004 is
inconsistent: update the assistant dependency to match the assisted test flow by
replacing references to real assistants ("AI assistant (Claude Code, Copilot
Chat, or Cursor AI) available") with "Dummy AI (Tier 1)" or alternatively change
the assisted test flow to target one of the real assistants; also ensure the
YAML's automated/assisted field matches the branch test type (use automated only
if there is a corresponding integration test, otherwise mark as assisted and tag
the test with [assisted]) and verify steps that mention clipboard sentinel and
selection behavior remain aligned with the chosen assistant.
- Around line 557-560: The assisted test steps currently tell the user to run
"Press Cmd+R Cmd+D → bind a terminal destination", which conflicts with the
harness's programmatic pre-binding and makes assertions flaky; remove or replace
that manual rebind instruction in the YAML entries that contain the string
'Press Cmd+R Cmd+D → bind a terminal destination' (occurrences around the blocks
that also include 'Press Cmd+R Cmd+L — link appears in terminal' and 'Press
Cancel — test reads clipboard and asserts it equals the sentinel'), mark the
test as [assisted] if it truly requires human interaction, and if the test is
meant to be automated ensure the automated field matches an existing integration
test on the branch—do not rebind destinations in the assisted steps so the
harness pre-bound destination remains intact.

In
`@packages/rangelink-vscode-extension/src/__integration-tests__/suite/clipboardPreservation.test.ts`:
- Around line 176-206: The assisted test clipboard-preservation-002 only checks
clipboard sentinel so it can pass without verifying the transport; update the
test to also assert the bound destination actually received the deterministic
phrase: instruct the human to type a specific phrase (e.g. "rangelink-test-002")
in waitForHuman and after assertClipboardRestored call open the bound file (use
createWorkspaceFile/fileUri or openEditor) and assert its document text contains
that phrase (use the same helpers used elsewhere like openEditor + reading
document.getText or an existing assertFileContains helper). Apply the same
pattern for clipboard-preservation-004 and -007: add a deterministic phrase in
waitForHuman instructions and a destination-side assertion that the bound file
or AI buffer contains that phrase after the R-V step.
- Around line 69-80: Replace the raw string command IDs passed to
vscode.commands.executeCommand in the test (specifically the calls using
'rangelink.pasteCurrentFileRelativePath' and
'rangelink.copyLinkWithRelativePath') with the exported constants from
src/constants/commandIds.ts: import the matching command ID constants at the top
of the test file and call vscode.commands.executeCommand(<COMMAND_CONSTANT>)
instead of the string literals so renames are tracked and tests stay consistent.
- Line 133: The teardown calls cleanupFiles(tmpFileUris) but does not clear the
tmpFileUris array, causing repeated cleanup across tests; after calling
cleanupFiles(tmpFileUris) in the test teardown, clear the tracked URIs (e.g.,
call tmpFileUris.splice(0) or reassign tmpFileUris = []) just like terminals are
cleared with splice(0) so the array is emptied and subsequent teardowns don't
retry removing the same files; update the teardown block referencing tmpFileUris
and ensure cleanupFiles and the clearing call are adjacent.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0b56254f-1ef6-418c-b97c-2b27c3b27402

📥 Commits

Reviewing files that changed from the base of the PR and between c32c656 and 56bd80f.

📒 Files selected for processing (2)
  • packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yaml
  • packages/rangelink-vscode-extension/src/__integration-tests__/suite/clipboardPreservation.test.ts

Comment thread packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yaml Outdated
Comment thread packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yaml Outdated
TC 002, 004, and 007 only asserted clipboard state but gave no evidence that content reached its destination — a test could pass even if the paste operation silently failed. TC 004 was also referencing real AI assistants in its YAML while the actual test used Dummy AI (Tier 1), and TC 001's YAML told the human to manually bind a terminal that the test already pre-binds programmatically.

Benefits:
- TC 002 and 007 now assert the exact phrase landed in the destination text file
- TC 004 now asserts dummyAi.getText().tier1 equals the exact expected link (strictEqual — safe because dummyAi.clearAll in teardown guarantees tier1 starts empty each test)
- YAML preconditions and steps for TC 001/002/004/007 now match what the test actually does
- Raw command ID strings replaced with imported constants (rule T016)
- tmpFileUris.splice(0) added after cleanupFiles to prevent stale-URI cleanup attempts in subsequent tests

Ref: #530 (review)
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yaml (1)

611-622: ⚠️ Potential issue | 🟠 Major

TC clipboard-preservation-005 YAML flow no longer matches the assisted implementation.

This case now specifies a fresh manual bind step, but the assisted test flow pre-binds cbp-005-dest before the human step. Please align one side (either remove the manual fresh-bind wording in YAML, or change the assisted test to actually perform fresh bind during the human step).

Suggested YAML alignment (if pre-bind is intended)
-    scenario: 'always mode: clipboard content before terminal paste (fresh bind) is restored after'
+    scenario: 'always mode: clipboard content before terminal paste is restored after'
     preconditions:
       - 'rangelink.clipboard.preserve = "always" (loaded automatically by test)'
       - 'Test file with 10 lines open in editor'
+      - 'Terminal "cbp-005-dest" pre-bound by the test'
     steps:
       - 'Test writes sentinel to clipboard automatically'
-      - 'Press Cmd+R Cmd+D → bind a fresh terminal destination'
       - 'Click back into the test file, select 2-3 lines'
       - 'Press Cmd+R Cmd+L — link appears in terminal'
       - 'Press Cancel — test reads clipboard and asserts it equals the sentinel'

Based on learnings: The automated field in QA YAML requires a matching integration test on the branch - assisted requires [assisted]-tagged test.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yaml` around
lines 611 - 622, The YAML scenario "always mode: clipboard content before
terminal paste (fresh bind) is restored after" (TC clipboard-preservation-005)
is out-of-sync with the assisted test which pre-binds cbp-005-dest; either
remove the "fresh manual bind" wording from the scenario or update the assisted
test flow to perform the fresh bind during the human step (the step that
currently reads "Press Cmd+R Cmd+D → bind a fresh terminal destination"); also
ensure the automated field value matches the test harness (use "assisted" only
if the branch contains an [assisted]-tagged integration test) and update the
scenario steps and expected_result to reflect the chosen alignment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yaml`:
- Around line 611-622: The YAML scenario "always mode: clipboard content before
terminal paste (fresh bind) is restored after" (TC clipboard-preservation-005)
is out-of-sync with the assisted test which pre-binds cbp-005-dest; either
remove the "fresh manual bind" wording from the scenario or update the assisted
test flow to perform the fresh bind during the human step (the step that
currently reads "Press Cmd+R Cmd+D → bind a fresh terminal destination"); also
ensure the automated field value matches the test harness (use "assisted" only
if the branch contains an [assisted]-tagged integration test) and update the
scenario steps and expected_result to reflect the chosen alignment.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ef7b1e1f-572d-40f8-aa56-6712615d38fd

📥 Commits

Reviewing files that changed from the base of the PR and between 56bd80f and 06f1a7a.

📒 Files selected for processing (2)
  • packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yaml
  • packages/rangelink-vscode-extension/src/__integration-tests__/suite/clipboardPreservation.test.ts

… test

TC 005's YAML still described a "fresh manual bind" step even though the test pre-binds cbp-005-dest via createAndBindCapturingTerminal — the same mismatch that was fixed for TC 001 and TC 002 in the previous pass.

Ref: #530 (review)
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