[issues/500] Extend AI assistants via custom settings#502
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 33 minutes and 19 seconds. ⌛ 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 (6)
WalkthroughAdds configurable "custom AI assistants" and a three‑tier command model: config parsing, new custom-ai destination kinds/types/guards, destination builder/registration changes (including built-in overrides), focus/insert capability implementations, clipboard-preservation predicate plumbing, i18n entries, tests, test fixtures, scripts and docs. No breaking public API removals. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RangeLink as "RangeLink Extension"
participant Config as "Config Parser"
participant Registry
participant CustomExt as "Custom AI Extension"
participant Clipboard
User->>RangeLink: open destination picker
RangeLink->>Config: read rangelink.customAiAssistants
Config-->>RangeLink: parsed custom-ai:<id> configs
RangeLink->>Registry: register builders for custom-ai:<id>
Registry->>CustomExt: probe extension active / getCommands
alt extension active
CustomExt-->>Registry: active = true
else fallback to commands
CustomExt-->>Registry: available commands list
end
Registry-->>RangeLink: include custom AI in picker
User->>RangeLink: select custom AI
RangeLink->>Clipboard: copy content
RangeLink->>CustomExt: invoke focus/insert commands or write clipboard
CustomExt-->>User: focused/ready (user pastes if manual)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 user-visible changes in the README regarding the support for custom AI tools and clarifies the destinations feature. Additionally, a new ADR document was added that outlines changes to the paste functionality for custom AI assistants. 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.
Actionable comments posted: 6
🧹 Nitpick comments (4)
packages/rangelink-vscode-extension/src/__tests__/destinations/DestinationAvailabilityService.test.ts (1)
1139-1170: Consider adding logger assertions for custom AI processing.The test verifies the functional behavior but doesn't assert logging. Per coding guidelines, logging verification should be consolidated with behavior tests.
📝 Suggested addition
expect(result[customKind]).toStrictEqual([ { label: 'Spark AI', displayName: 'Spark AI', bindOptions: { kind: customKind }, itemKind: 'bindable', }, ]); + expect(mockLogger.debug).toHaveBeenCalledWith( + expect.objectContaining({ + fn: 'DestinationAvailabilityService.getGroupedDestinationItems', + }), + expect.any(String), + ); });As per coding guidelines: "Include logger assertions in tests that verify method behavior."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/__tests__/destinations/DestinationAvailabilityService.test.ts` around lines 1139 - 1170, Add assertions that the mockLogger was called appropriately when custom AI processing runs in the DestinationAvailabilityService.getGroupedDestinationItems test: after calling getGroupedDestinationItems on the customService instance, verify mockLogger methods (e.g., mockLogger.info or mockLogger.debug/error used elsewhere in the service) were invoked with the expected message or context indicating the custom kind (customKind / 'custom-ai:acme.spark-ai') or displayName ('Spark AI'), ensuring the logging call(s) correspond to the same execution path that produced the menu item; update the test to expect those logger calls in addition to the existing result assertions.packages/rangelink-vscode-extension/scripts/setup-integration-test-settings.js (1)
31-35: Consider logging non-ENOENT errors for debuggability.The catch block silently swallows all errors, including permission issues or JSON parse failures. While starting fresh is reasonable for missing files, other errors could indicate real problems.
📝 Proposed improvement
let existing = {}; try { existing = JSON.parse(fs.readFileSync(SETTINGS_FILE, 'utf8')); -} catch { - // File doesn't exist yet — start fresh +} catch (err) { + if (err.code !== 'ENOENT') { + console.warn(`[setup-integration-test-settings] Warning: ${err.message}, starting fresh`); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/scripts/setup-integration-test-settings.js` around lines 31 - 35, The catch currently swallows all errors when reading/parsing SETTINGS_FILE (JSON.parse(fs.readFileSync(SETTINGS_FILE, 'utf8'))), which hides permission or parse failures; update the catch to inspect the thrown error (e.g. check err.code === 'ENOENT') and only treat ENOENT as “start fresh”, while for other errors log the error (including the error.message and SETTINGS_FILE) or rethrow it so failures like permission denied or malformed JSON are visible; refer to the existing variable existing, SETTINGS_FILE, JSON.parse and fs.readFileSync to locate and update the block.packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts (1)
382-415: Add logger assertions to the new availability tests.These cases cover the boolean result, but not the new debug branches you added for extension-based vs command-based availability. Lock those log payloads/messages in here so the diagnostics do not drift silently. As per coding guidelines: "Include logger assertions in tests that verify method behavior, consolidating logging verification with behavior tests rather than creating separate logging tests".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts` around lines 382 - 415, The three availability tests for createCustomAiAssistantBuilder currently assert only boolean outcomes; update each test to also assert the expected logger/debug calls so the new debug branches are locked in: in the "extension is active" test assert that the mock context's logger (from createMockContext) received a debug/info call indicating extension-based availability (e.g., mentions isActive or extension found), in the "falls back to command check" test assert the logger was called with a message indicating command-based fallback (e.g., mentions checking commands or found sparkAi.focus), and in the "neither extension nor commands available" test assert the logger was called with the not-available diagnostic; use the same mock context.logger/spy used elsewhere and check calls after awaiting destination.isAvailable() to ensure the exact logging branch is tested.packages/rangelink-vscode-extension/src/types/DestinationKind.ts (1)
54-58: Consider makingisAnyAiAssistantKinda type predicate for consistency and proper narrowing.The function currently returns
boolean, which prevents TypeScript from narrowing the type in control flow. SinceisCustomAiAssistantKinduses a type predicate return type (kind is CustomAiAssistantKind), this companion function should do the same:Suggested adjustment
-export const isAnyAiAssistantKind = (kind: DestinationKind): boolean => - isCustomAiAssistantKind(kind) || (AI_ASSISTANT_KINDS as readonly string[]).includes(kind); +export const isAnyAiAssistantKind = ( + kind: string, +): kind is AIAssistantDestinationKind | CustomAiAssistantKind => + isCustomAiAssistantKind(kind) || (AI_ASSISTANT_KINDS as readonly string[]).includes(kind);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/types/DestinationKind.ts` around lines 54 - 58, The function isAnyAiAssistantKind currently returns boolean which prevents TS narrowing; change its signature to be a type predicate (e.g. (kind: DestinationKind): kind is /*appropriate AI assistant union type*/ ) so callers can narrow the type. Update the export of isAnyAiAssistantKind to return a predicate that combines isCustomAiAssistantKind and the built-in assistant kinds (use AI_ASSISTANT_KINDS in the check) and reference the same union type used for built-in assistants (e.g. AiAssistantKind) together with CustomAiAssistantKind so the final type predicate reads like "kind is AiAssistantKind | CustomAiAssistantKind".
🤖 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/src/__integration-tests__/suite/customAiAssistants.test.ts`:
- Around line 60-86: The integration tests are too weak: instead of only
asserting absence of a "commandAvailable:true" line, make the test explicitly
verify the assistant availability is evaluated and logged as unavailable (look
for a log like 'customAiAssistant.isAvailable' with '"commandAvailable":false'
or an explicit "not available" message emitted by
parseCustomAiAssistants/customAiAssistant.isAvailable), or call the availability
check directly in the test to force evaluation and assert its false result; and
replace the placeholder test custom-ai-assistant-006 with an end-to-end
malformed-settings scenario that injects invalid settings, runs
parseCustomAiAssistants, and asserts the invalid entries are skipped (via
returned parsed list or explicit skip logs) so the test verifies concrete
behavior rather than being a no-op.
In
`@packages/rangelink-vscode-extension/src/destinations/DestinationAvailabilityService.ts`:
- Line 13: Remove the now-unused import DESTINATION_KINDS from
DestinationAvailabilityService; locate the import list that currently includes
DESTINATION_KINDS and delete that identifier so ESLint no longer flags
`@typescript-eslint/no-unused-vars`, since the code uses
registry.getSupportedKinds() instead of DESTINATION_KINDS elsewhere in the
module.
- Around line 252-259: The picker falls back to raw kind because
this.registry.getDisplayNames() only covers built-ins; update
DestinationAvailabilityService so when displayNames[kind] is undefined and kind
startsWith 'custom-ai:' you extract the extensionId from kind and query the
registry for that custom assistant's configured name (e.g., lookup a custom
assistant entry or method on this.registry using the extensionId) and use that
as displayName; keep the existing fallback to kind only if no custom assistant
display name is found.
In `@packages/rangelink-vscode-extension/src/destinations/destinationBuilders.ts`:
- Around line 270-299: The isAvailable function short-circuits when an
installed-but-inactive extension exists by returning extension.isActive
immediately; instead compute extensionActive =
(context.ideAdapter.getExtension(extensionId)?.isActive === true), fetch
commands via context.ideAdapter.getCommands() and compute commandAvailable =
focusCommands.some(...), then set available = extensionActive ||
commandAvailable and return that; update the context.logger.debug call to log
extensionFound (extension !== undefined), extensionActive, commandAvailable,
checkedCommands and a single availability message using available to reflect
either source enabling the assistant (referencing isAvailable, getExtension,
getCommands, focusCommands, extensionId, extensionName, context.logger.debug).
In
`@packages/rangelink-vscode-extension/src/destinations/utils/buildDestinationQuickPickItems.ts`:
- Around line 20-21: buildDestinationQuickPickItems currently only iterates
DESTINATION_PICKER_SEQUENCE so custom AI kinds added to the grouped map by
DestinationAvailabilityService.getGroupedDestinationItems() are ignored; update
buildDestinationQuickPickItems to include custom kinds by either appending keys
that satisfy isDestinationKind but are not in DESTINATION_PICKER_SEQUENCE to the
iteration sequence or by iterating remaining grouped keys after the main loop
and processing them the same way as built-ins, ensuring you reference
DESTINATION_PICKER_SEQUENCE, grouped, isDestinationKind, DESTINATION_KINDS and
the buildDestinationQuickPickItems function when making the change.
In `@README.md`:
- Around line 61-67: Update the README table cell under "Destinations" that
currently reads "Auto-send links to Claude Code, Cursor AI, Copilot Chat,
Terminal, Text Editor, or any custom AI assistant" to append a visible
<sup>Unreleased</sup> marker after "custom AI assistant"; also search for any
other occurrences of the phrase "custom AI assistant" or "Custom AI assistants"
(including the R-Keybinding Family mentions) and add the same
<sup>Unreleased</sup> marker so all unreleased custom assistant references are
consistently annotated.
---
Nitpick comments:
In
`@packages/rangelink-vscode-extension/scripts/setup-integration-test-settings.js`:
- Around line 31-35: The catch currently swallows all errors when
reading/parsing SETTINGS_FILE (JSON.parse(fs.readFileSync(SETTINGS_FILE,
'utf8'))), which hides permission or parse failures; update the catch to inspect
the thrown error (e.g. check err.code === 'ENOENT') and only treat ENOENT as
“start fresh”, while for other errors log the error (including the error.message
and SETTINGS_FILE) or rethrow it so failures like permission denied or malformed
JSON are visible; refer to the existing variable existing, SETTINGS_FILE,
JSON.parse and fs.readFileSync to locate and update the block.
In
`@packages/rangelink-vscode-extension/src/__tests__/destinations/DestinationAvailabilityService.test.ts`:
- Around line 1139-1170: Add assertions that the mockLogger was called
appropriately when custom AI processing runs in the
DestinationAvailabilityService.getGroupedDestinationItems test: after calling
getGroupedDestinationItems on the customService instance, verify mockLogger
methods (e.g., mockLogger.info or mockLogger.debug/error used elsewhere in the
service) were invoked with the expected message or context indicating the custom
kind (customKind / 'custom-ai:acme.spark-ai') or displayName ('Spark AI'),
ensuring the logging call(s) correspond to the same execution path that produced
the menu item; update the test to expect those logger calls in addition to the
existing result assertions.
In
`@packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts`:
- Around line 382-415: The three availability tests for
createCustomAiAssistantBuilder currently assert only boolean outcomes; update
each test to also assert the expected logger/debug calls so the new debug
branches are locked in: in the "extension is active" test assert that the mock
context's logger (from createMockContext) received a debug/info call indicating
extension-based availability (e.g., mentions isActive or extension found), in
the "falls back to command check" test assert the logger was called with a
message indicating command-based fallback (e.g., mentions checking commands or
found sparkAi.focus), and in the "neither extension nor commands available" test
assert the logger was called with the not-available diagnostic; use the same
mock context.logger/spy used elsewhere and check calls after awaiting
destination.isAvailable() to ensure the exact logging branch is tested.
In `@packages/rangelink-vscode-extension/src/types/DestinationKind.ts`:
- Around line 54-58: The function isAnyAiAssistantKind currently returns boolean
which prevents TS narrowing; change its signature to be a type predicate (e.g.
(kind: DestinationKind): kind is /*appropriate AI assistant union type*/ ) so
callers can narrow the type. Update the export of isAnyAiAssistantKind to return
a predicate that combines isCustomAiAssistantKind and the built-in assistant
kinds (use AI_ASSISTANT_KINDS in the check) and reference the same union type
used for built-in assistants (e.g. AiAssistantKind) together with
CustomAiAssistantKind so the final type predicate reads like "kind is
AiAssistantKind | CustomAiAssistantKind".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 46bce8bf-436d-4732-bb66-58e529a42e6e
📒 Files selected for processing (24)
README.mdpackages/rangelink-vscode-extension/CHANGELOG.mdpackages/rangelink-vscode-extension/README.mdpackages/rangelink-vscode-extension/package.jsonpackages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yamlpackages/rangelink-vscode-extension/scripts/setup-integration-test-settings.jspackages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.tspackages/rangelink-vscode-extension/src/__tests__/constants/packageJsonContracts.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/DestinationAvailabilityService.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.tspackages/rangelink-vscode-extension/src/__tests__/helpers/createMockDestinationRegistry.tspackages/rangelink-vscode-extension/src/config/__tests__/parseCustomAiAssistants.test.tspackages/rangelink-vscode-extension/src/config/parseCustomAiAssistants.tspackages/rangelink-vscode-extension/src/createWiringServices.tspackages/rangelink-vscode-extension/src/destinations/ComposablePasteDestination.tspackages/rangelink-vscode-extension/src/destinations/DestinationAvailabilityService.tspackages/rangelink-vscode-extension/src/destinations/PasteDestinationManager.tspackages/rangelink-vscode-extension/src/destinations/destinationBuilders.tspackages/rangelink-vscode-extension/src/destinations/utils/buildDestinationQuickPickItems.tspackages/rangelink-vscode-extension/src/i18n/messages.en.tspackages/rangelink-vscode-extension/src/types/BindOptions.tspackages/rangelink-vscode-extension/src/types/DestinationKind.tspackages/rangelink-vscode-extension/src/types/MessageCode.tspackages/rangelink-vscode-extension/src/types/index.ts
👮 Files not reviewed due to content moderation or server errors (8)
- packages/rangelink-vscode-extension/src/tests/constants/packageJsonContracts.test.ts
- packages/rangelink-vscode-extension/src/types/MessageCode.ts
- packages/rangelink-vscode-extension/package.json
- packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yaml
- packages/rangelink-vscode-extension/src/config/tests/parseCustomAiAssistants.test.ts
- packages/rangelink-vscode-extension/src/i18n/messages.en.ts
- packages/rangelink-vscode-extension/README.md
- packages/rangelink-vscode-extension/CHANGELOG.md
couimet
pushed a commit
that referenced
this pull request
Mar 26, 2026
…allback, type predicate Four critical fixes from CodeRabbit review: 1. **Availability no longer short-circuits on inactive extension** — `isAvailable` now computes both `extensionActive` and `commandAvailable` and returns the OR. An extension could be inactive but its commands registered (e.g., during activation). 2. **Custom AI kinds now appear in the destination picker** — `buildDestinationQuickPickItems` only iterated `DESTINATION_PICKER_SEQUENCE` (built-in kinds), silently dropping custom AI entries. Now appends custom kinds from the grouped map after the built-in AI assistant sequence, in the 'ai' group. 3. **Display name reads from destination, not registry map** — `DestinationAvailabilityService` previously looked up `displayNames[kind]` which returned `undefined` for custom kinds (only built-in names in the map). Now creates the destination and reads its `displayName` property directly. 4. **`isAnyAiAssistantKind` is now a type predicate** — enables TypeScript narrowing, consistent with `isCustomAiAssistantKind`. Also removed unused `DESTINATION_KINDS` import from `DestinationAvailabilityService`. Ref: #502 (review)
couimet
pushed a commit
that referenced
this pull request
Mar 26, 2026
…x docs markers Addressed remaining CodeRabbit review items: - **destinationBuilders.test.ts**: added logger assertions to all 3 custom AI availability tests — locks in the debug log format with extensionFound, extensionActive, commandAvailable, and available fields - **DestinationAvailabilityService.test.ts**: added logger assertion to custom AI test verifying the "Using all registered destination kinds" debug log - **customAiAssistants.test.ts**: TC 005 now explicitly checks for `"available":false` and `"extensionFound":false` + `"commandAvailable":false` in logs; TC 006 replaced no-op with real assertion verifying exactly 1 custom AI registration exists - **setup-integration-test-settings.js**: catch block now logs non-ENOENT errors for debuggability - **README.md**: added `<sup>Unreleased</sup>` markers to Destinations table and Perfect For section Ref: #502 (review)
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
1-15:⚠️ Potential issue | 🟡 MinorAdd the required
> [!IMPORTANT]banner for unreleased features.The README documents unreleased custom AI assistant features (lines 63, 75) but is missing the required banner at the top. As per coding guidelines, when documenting unreleased features in README, a
> [!IMPORTANT]banner must be added if one doesn't exist.📢 Proposed fix to add the banner
 +> [!IMPORTANT] +> This README documents unreleased features. See release notes for availability. + > **"Claude Code today. Cursor AI tomorrow. Different shortcuts, different muscle memory."**<br /> > **RangeLink ends it.** One keybinding. Any AI, any tool. Character-level precision. `recipes/baking/chickenpie.ts#L3C14-L314C16`As per coding guidelines: "Add a
> [!IMPORTANT]banner at the top if one doesn't exist" when documenting unreleased features in README.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 1 - 15, The README documents unreleased custom AI assistant features but is missing the required banner; add a top-level `> [!IMPORTANT]` banner (the literal token) above the unreleased-features documentation so the sections referencing unreleased features (e.g., the entries near the README content that mention recipes/baking/chickenpie.ts#L3C14-L314C16 and the sections around lines 63 and 75) are clearly marked as important/unreleased; ensure the banner text precedes those sections and follows the exact `> [!IMPORTANT]` format required by the guidelines.
🧹 Nitpick comments (1)
README.md (1)
119-119: Consider mentioning custom AI assistants for consistency.Lines 63 and 75 reference custom AI assistants/tools as part of RangeLink's destination capabilities, but line 119 in the History section lists destinations without mentioning them. While the History section may intentionally focus on shipped features, adding a brief mention with an unreleased marker would maintain consistency across the README.
✨ Optional consistency improvement
-Today, with **destinations**, RangeLink sends links directly where you need them — Claude Code, Cursor AI, GitHub Copilot Chat, terminals, or text editors. No more copy-paste friction. It helps developers share code with precision across any AI assistant, VSCode, Cursor, GitHub, Slack, and more. One format, zero friction. +Today, with **destinations**, RangeLink sends links directly where you need them — Claude Code, Cursor AI, GitHub Copilot Chat, terminals, text editors, or custom AI tools <sup>Unreleased</sup>. No more copy-paste friction. It helps developers share code with precision across any AI assistant, VSCode, Cursor, GitHub, Slack, and more. One format, zero friction.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 119, The History section's sentence beginning "Today, with **destinations**, RangeLink sends links directly where you need them..." omits mention of custom AI assistants; update that sentence to include a brief parenthetical or trailing clause such as "including custom AI assistants (unreleased)" to match earlier mentions of custom assistants/tools referenced elsewhere, keeping the tone concise and clearly marking the feature as unreleased.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@README.md`:
- Around line 1-15: The README documents unreleased custom AI assistant features
but is missing the required banner; add a top-level `> [!IMPORTANT]` banner (the
literal token) above the unreleased-features documentation so the sections
referencing unreleased features (e.g., the entries near the README content that
mention recipes/baking/chickenpie.ts#L3C14-L314C16 and the sections around lines
63 and 75) are clearly marked as important/unreleased; ensure the banner text
precedes those sections and follows the exact `> [!IMPORTANT]` format required
by the guidelines.
---
Nitpick comments:
In `@README.md`:
- Line 119: The History section's sentence beginning "Today, with
**destinations**, RangeLink sends links directly where you need them..." omits
mention of custom AI assistants; update that sentence to include a brief
parenthetical or trailing clause such as "including custom AI assistants
(unreleased)" to match earlier mentions of custom assistants/tools referenced
elsewhere, keeping the tone concise and clearly marking the feature as
unreleased.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4ef41944-a2a5-472a-9eb3-6e1e5840c9ca
📒 Files selected for processing (9)
README.mdpackages/rangelink-vscode-extension/scripts/setup-integration-test-settings.jspackages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/DestinationAvailabilityService.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.tspackages/rangelink-vscode-extension/src/destinations/DestinationAvailabilityService.tspackages/rangelink-vscode-extension/src/destinations/destinationBuilders.tspackages/rangelink-vscode-extension/src/destinations/utils/buildDestinationQuickPickItems.tspackages/rangelink-vscode-extension/src/types/DestinationKind.ts
✅ Files skipped from review due to trivial changes (2)
- packages/rangelink-vscode-extension/scripts/setup-integration-test-settings.js
- packages/rangelink-vscode-extension/src/integration-tests/suite/customAiAssistants.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/rangelink-vscode-extension/src/tests/destinations/DestinationAvailabilityService.test.ts
- packages/rangelink-vscode-extension/src/destinations/destinationBuilders.ts
- packages/rangelink-vscode-extension/src/tests/destinations/destinationBuilders.test.ts
- packages/rangelink-vscode-extension/src/types/DestinationKind.ts
- packages/rangelink-vscode-extension/src/destinations/utils/buildDestinationQuickPickItems.ts
couimet
pushed a commit
that referenced
this pull request
Mar 26, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 14: A blank line exists inside the callout blockquote in README.md (the
<details> / <summary> callout), causing markdownlint MD028; remove the empty
line so the blockquote content is contiguous (no extra blank line between the
blockquote marker and the callout text) to satisfy MD028 and keep the <details>
/ <summary> block properly formatted.
🪄 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: ca1a7b47-8310-4058-8514-635b598fb603
📒 Files selected for processing (2)
README.mdpackages/rangelink-vscode-extension/README.md
✅ Files skipped from review due to trivial changes (1)
- packages/rangelink-vscode-extension/README.md
added 21 commits
April 15, 2026 09:19
…TCs) Adds 9 assisted integration tests for the R-D Bind to Destination flow — the multi-step pattern where the human opens the picker, selects a destination, and the test asserts toast/status bar messages via log capture. Also removes 3 duplicate TCs (001-003) that were fully covered by existing terminal/file picker tests. - New `bindToDestination.test.ts` with 9 `[assisted]` tests covering bind success (terminal, file, AI assistant), smart-bind confirmation dialog (items, Yes/replace, No/keep), escape handling, and AI assistant re-bind/switch scenarios - Deleted TCs 001-003 from QA YAML — "picker opens via trigger X" was already tested by every terminal/file picker assisted test (no renumbering per QA001) - Updated TCs 004-012 from `automated: false` to `automated: assisted` - AI assistant TCs (006, 011, 012) are included as assisted — they require running in an IDE with the relevant extensions installed - TC 012 (switch between AI assistants) includes a setup `waitForHuman` prompting to install a second AI extension; will be fully testable once #500 (custom AI assistants) lands - Fixed smart-bind confirmation tests (007, 009) — removed incorrect "Escape again" step; the confirmation dialog is invoked after `DestinationPicker.pick()` returns (in `commitBind`), so no parent picker reopens - [x] All 1737 unit tests pass (99 suites) - [x] QA coverage validator passes (38 assisted entries, 59 automated, 98 false) - [x] 9/10 assisted tests pass with human interaction (`pnpm test:release:grep "bind-to-destination"`) - [ ] TC 012 deferred — requires two AI assistant extensions in the test host (blocked on #500) - CHANGELOG: not needed — test infrastructure, not user-facing - README: not needed - Closes #506 - Parent: #504 - Blocked: TC 012 depends on #500 for testability
The type system now accepts `custom-ai:${string}` as a valid `DestinationKind` alongside the 5 built-in kinds. This enables user-defined AI assistants to be registered, bound, and displayed without modifying the hardcoded kind union for each new tool.
Added `isCustomAiAssistantKind()` and `isAnyAiAssistantKind()` type guards. Updated switch statements in `PasteDestinationManager` (bind routing, failure messages) and `DestinationAvailabilityService` (menu population) to handle custom kinds via default cases that check the prefix — preserving compile-time exhaustiveness for built-in kinds while remaining open to runtime-registered customs.
Users can now define custom AI assistants in `settings.json` via `rangelink.customAiAssistants`. Each entry requires `extensionId` (for availability detection), `extensionName` (display name in the R-D menu), and `focusCommands` (VS Code commands to focus the chat panel).
The parser validates every entry at activation — missing fields, empty strings, empty command arrays, and duplicate IDs are logged as warnings and skipped. Valid entries are transformed into `CustomAiAssistantConfig` objects with a `custom-ai:{extensionId}` kind for the destination registry.
The package.json schema includes JSON validation (`required`, `minItems`, `additionalProperties: false`) so VS Code highlights malformed entries in the settings UI.
… and registration `createCustomAiAssistantBuilder` creates a `DestinationBuilder` from a `CustomAiAssistantConfig`. The builder produces a `ComposablePasteDestination` with clipboard-based paste and dual-path availability detection: checks `getExtension(extensionId)` first (is it installed + active?), then falls back to checking if any `focusCommand` is a registered VS Code command. Custom assistants are parsed from settings at activation via `parseCustomAiAssistants()`, then registered alongside the 5 built-in kinds in `registerAllDestinationBuilders()`. The `DestinationAvailabilityService` now iterates all registered kinds (via `registry.getSupportedKinds()`) instead of the hardcoded `DESTINATION_KINDS` array — so custom assistants appear in the R-D menu automatically. Also widened `AiAssistantDestinationParams.id` to accept `CustomAiAssistantKind` and updated the mock registry to return built-in kinds by default.
Added 2 unit tests for `DestinationAvailabilityService.getGroupedDestinationItems` covering custom AI assistant kinds: available custom assistant appears with correct displayName and bindOptions, unavailable custom assistant is excluded from results. The menu population logic was already implemented in S003 (switched from `DESTINATION_KINDS` to `registry.getSupportedKinds()`), so these tests verify the existing behavior rather than adding new code.
…stom AI assistants CHANGELOG entry under [Unreleased] describes the new `rangelink.customAiAssistants` setting with extensionId/extensionName/focusCommands schema. README updated to mention custom AI tool support in "Features at a Glance" and "Perfect For" sections. 7 QA test cases added under `custom-ai-assistant` slug (5 automated, 2 manual). Integration tests use a pre-created settings file via `setup-integration-test-settings.js` to configure a fake custom AI assistant (`fake-tool.test`) before extension activation. Tests verify config parsing, registry registration, command detection, and unavailability at startup — all via log assertions without triggering blocking QuickPick dialogs.
…allback, type predicate Four critical fixes from CodeRabbit review: 1. **Availability no longer short-circuits on inactive extension** — `isAvailable` now computes both `extensionActive` and `commandAvailable` and returns the OR. An extension could be inactive but its commands registered (e.g., during activation). 2. **Custom AI kinds now appear in the destination picker** — `buildDestinationQuickPickItems` only iterated `DESTINATION_PICKER_SEQUENCE` (built-in kinds), silently dropping custom AI entries. Now appends custom kinds from the grouped map after the built-in AI assistant sequence, in the 'ai' group. 3. **Display name reads from destination, not registry map** — `DestinationAvailabilityService` previously looked up `displayNames[kind]` which returned `undefined` for custom kinds (only built-in names in the map). Now creates the destination and reads its `displayName` property directly. 4. **`isAnyAiAssistantKind` is now a type predicate** — enables TypeScript narrowing, consistent with `isCustomAiAssistantKind`. Also removed unused `DESTINATION_KINDS` import from `DestinationAvailabilityService`. Ref: #502 (review)
…x docs markers Addressed remaining CodeRabbit review items: - **destinationBuilders.test.ts**: added logger assertions to all 3 custom AI availability tests — locks in the debug log format with extensionFound, extensionActive, commandAvailable, and available fields - **DestinationAvailabilityService.test.ts**: added logger assertion to custom AI test verifying the "Using all registered destination kinds" debug log - **customAiAssistants.test.ts**: TC 005 now explicitly checks for `"available":false` and `"extensionFound":false` + `"commandAvailable":false` in logs; TC 006 replaced no-op with real assertion verifying exactly 1 custom AI registration exists - **setup-integration-test-settings.js**: catch block now logs non-ENOENT errors for debuggability - **README.md**: added `<sup>Unreleased</sup>` markers to Destinations table and Perfect For section Ref: #502 (review)
The previous order led with delimiters (rarely changed) and buried clipboard preservation and custom AI assistants at the bottom. The new sequence follows "how quickly will a user want to change this": 1. Custom AI Assistants — the extensibility unlock 2. Clipboard — affects every operation 3. Smart Padding — noticed on first paste 4. Feedback & Warnings — merged dirty-buffer + navigation toasts 5. Terminal Picker — situational 6. Delimiters — defaults match GitHub, rarely touched 7. Feature Flags — opt-in discovery Within delimiters, reordered to match left-to-right reading of a link: hash (#), line (L), position (C), range (-). README "Warning Settings" and "Navigation Settings" merged into a single "Feedback & Warning Settings" section — all three settings answer the same question: "should RangeLink show me a notification?"
Webview-based assistants (e.g., Pi) can't receive VS Code paste commands — the clipboard-paste flow silently fails because the paste targets the editor layer, not the webview's internal textarea. Rather than a single focusCommands array, the schema now expresses three distinct delivery tiers so RangeLink can match each assistant's actual capability. Tier priority: insertCommands (direct text via command arg) → focusAndPasteCommands (focus + auto-paste via clipboard) → focusCommands (focus only, user pastes manually via toast). Benefits: - Webview-based assistants work immediately via focusCommands + toast - Version-resilient: when an assistant ships a direct-insert command, users add one line — no other config changes - Built-in assistants are unaffected — they keep their existing code path
…olation
Tier 1 insert commands need a way to pass link text to extensions with varying argument formats. This utility deep-walks a JSON template, replacing ${content} placeholders with the actual link text — supporting strings, arrays, and nested objects.
Keeps the simple case simple: plain string commands pass text as the first positional arg (no template needed). The template syntax handles arbitrary shapes for extensions with non-standard command signatures.
…(Tier 3)
Two new insert factories for the three-tier custom AI assistant architecture:
DirectInsertFactory delivers link text via executeCommand args — clipboard is never touched. Supports plain positional args and ${content} template interpolation for extensions with non-standard command signatures.
ManualPasteInsertFactory writes to clipboard without preservation and returns success — the user pastes manually after the toast. Clipboard is intentionally NOT restored so the link remains available for Cmd+V.
The existing AIAssistantInsertFactory (Tier 2, clipboard + auto-paste) is unchanged.
TieredFocusCapability tries command tiers in priority order (insertCommands → focusAndPasteCommands → focusCommands), falling through when all commands in a tier fail. Each tier carries its own InsertFactory, so the winning tier determines both the focus behavior and the text delivery mechanism. A lastTierLabel property exposes which tier won — needed downstream for toast vs silent success feedback.
The parser now validates three optional command arrays with an "at least one required" constraint. insertCommands entries are normalized from plain strings to { command, args? } objects. Invalid entries within insertCommands are individually skipped with warnings rather than rejecting the entire assistant.
FocusCapabilityFactory gains createCustomAIAssistantCapability() which builds a TieredFocusCapability from the parsed config — populating only the tiers that have commands. The existing createAIAssistantCapability stays unchanged for built-in assistants. The custom AI destination builder now uses the tiered capability and reads lastTierLabel after focus to decide toast behavior: when Tier 3 (focusCommands) wins, getUserInstruction returns the "paste now" toast even on success — because the user still needs to Cmd+V manually.
…ommand schema The integration test settings, QA YAML test cases, and integration tests all assumed the old single-focusCommands config shape. Updated to exercise the three-tier schema (insertCommands + focusCommands) so the test infrastructure validates the new parser, tiered capability registration, and command detection across tiers. Also fixed the user-data-dir mismatch: .vscode-test configs used mkdtempSync (random path) while the settings setup script wrote to a fixed path — the settings were never found. Both configs now use the same fixed /tmp/rl-vscode-test path, and setup-integration-test-settings.js is wired into test:release:prepare so all three test commands (test:release, test:release:automated, test:release:grep) get the pre-populated settings.
added 9 commits
April 15, 2026 09:19
Collapsed 7 numbered sub-decisions into flowing prose — the decisions are tightly coupled and read better as narrative than spec sections. Stripped implementation details (getCommands vs executeCommand mechanics, interpolation recursion, clipboard backup/restore flow) — the code captures those. Added confidence levels per decision area. Reformatted alternatives as bullet list. Unified examples to acme.spark-ai theme for consistency with README and tests. Ref: https://martinfowler.com/bliki/ArchitectureDecisionRecord.html
The bind-time resolution refactoring introduced three new files (LazyResolvedFocusCapability, ResolvedFocusCapability, resolveFocusTier) that lacked unit test coverage, pushing all four coverage thresholds below their minimums. Added 12 tests across 2 new test files covering: lazy resolution with getCommands caching, fallback tier detection via fallbackTierIndex, resolution failure caching, tier walkthrough with registered/unregistered commands, and probeMode delegation. Renamed TieredFocusCapability.test.ts to ResolvedFocusCapability.test.ts to match the class it actually tests.
The old Custom AI Assistant section documented only focusCommands (single tier). Rewritten to cover the full three-tier design: insertCommands (direct text injection), focusAndPasteCommands (focus + auto-paste), focusCommands (focus + manual paste toast). Added examples for each scenario, ${content} template interpolation, built-in override mechanism, and bind-time resolution explanation.
CHANGELOG [Unreleased] updated to match: three-tier schema, bind-time resolution, built-in override with fallback, ${content} interpolation.
… template testing
The dummy AI test extension now has a dummyAi.insertWithArgs command that accepts an object argument with a text field — exercising the ${content} template interpolation path end-to-end. A fourth settings entry uses the template format: { command: 'dummyAi.insertWithArgs', args: [{ text: '${content}', source: 'rangelink' }] }.
This raises ADR-0002 confidence for template interpolation from LOW to MEDIUM — the template is parsed at activation and the command is registered in the VS Code host.
…flow
Five assisted integration tests written for the paste flow — require human interaction via test:release:grep to verify. Not yet run in the VS Code host.
Tests cover: Tier 1 direct insert to dummy textarea, Tier 1 clipboard isolation (sentinel preserved), Tier 3 manual paste toast, Tier 2→3 fallback (nonexistent command skipped), and ${content} template interpolation via insertWithArgs.
Also adds a fallback settings entry and QA YAML TCs (custom-ai-assistant-010 through -014).
Built-in AI assistant builders (Cursor AI, Claude Code, GitHub Copilot Chat) were three near-identical functions differing only in command lists and i18n keys. Consolidate them into a BUILTIN_AI_ASSISTANTS lookup table with a single generic builder, eliminating ~120 lines of duplication.
Replace TieredFocusCapability with LazyResolvedFocusCapability — tier resolution now happens once on first focus() call instead of eagerly at construction. Add FocusTierProbeMode ('execute' | 'none') so Tier 1 insertCommands can skip executeCommand during resolution (avoids side effects for direct-insert commands).
Split FocusCapabilityFactory.createCustomAIAssistantCapability into composable steps: buildCustomAIAssistantTiers(), buildBuiltinFallbackTier(), and createLazyResolvedCapability(). This enables the new built-in override path — when a custom assistant's extensionId matches a built-in, createOverriddenBuiltinBuilder merges user-defined tiers with the built-in's hardcoded focus commands as a fallback tier.
Unit tests rewritten for data-driven built-in registration and override builder paths.
ClipboardPreserver.preserve() unconditionally restored the clipboard in its finally block, which wiped the RangeLink that ManualPasteInsertFactory had placed there for the user to Cmd+V. The link was written correctly but immediately overwritten by the sentinel restore. The fix adds a shouldRestore callback evaluated AFTER the paste operation (when the tier is resolved) but BEFORE the clipboard restore. Tier 3 destinations signal via shouldPreserveClipboard() that the link must stay on the clipboard, so the restore is skipped. Also introduces FocusTierLabel union type so tier-dependent comparisons are type-checked — renaming a label in the factory now produces a compile error at every consumption site. Benefits: - Tier 3 (focusCommands) manual paste now works end-to-end — link stays on clipboard - Tier 1 and Tier 2 destinations are unaffected (shouldRestore defaults to true) - First-call timing issue solved — callback runs post-resolution, not pre-resolution - Tier label comparisons are type-safe (FocusTierLabel union, not raw strings)
…ride path The override mechanism (custom config with a built-in extensionId) was designed and unit-tested but lacked integration test coverage in a real VS Code host. Without host validation, a registration ordering bug could silently cause the override to register as a separate custom-ai: kind instead of replacing the built-in. Adds a settings entry with extensionId github.copilot-chat pointing to the dummy extension's insertCommands. Two automated tests verify that the override registers under the built-in kind and does not create a separate custom-ai: registration. ADR-0002 confidence for built-in override updated from MEDIUM to HIGH.
…nd assisted test robustness
Several issues discovered when running integration tests in the VS Code host for the first time with the dummy AI extension.
`createAndOpenFile` returns `Promise<Uri>` but all assisted tests destructured it as `{ uri }` — compiled under main tsconfig but failed under tsconfig.integration.json. The `custom-ai-assistant-005` test relied on availability logs from other (assisted) tests triggering the picker, which never happens in automated mode — rewrote to check command registration instead.
The dummy extension's webview panel was never initialized during tests because `WebviewViewProvider.resolveWebviewView` is only called when the sidebar view is shown. Added `ensureView()` with a readiness handshake: the webview sends a `ready` message after its script loads, and `ensureView()` polls for the promise then awaits it. Insert commands now auto-initialize the panel on cold start. Added `dummyAi.chatView.focus` fallback for `focusPanel`/`focusForPaste` when `_view` is undefined.
New cold-start test (custom-ai-assistant-017) verifies Tier 1 direct insert works when the panel has never been opened. Both focusCommands paths now include manual paste verification — Tier 3 direct (custom-ai-assistant-012) and Tier 2→3 fallback (custom-ai-assistant-013) prompt the user to Cmd+V into the tier2 textarea, proving the link is actually on the clipboard and not restored to the sentinel.
added 2 commits
April 15, 2026 09:30
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (9)
packages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/insertFactories/manualPasteInsertFactory.test.ts (2)
34-45: Add logger assertion in this behavior test.This test validates method behavior but currently skips log verification, which leaves observability behavior unguarded.
As per coding guidelines "Include logger assertions in tests that verify method behavior - logging provides critical visibility for debugging and should not be tested separately".♻️ Proposed assertion addition
it('does not execute any paste commands', async () => { @@ await insertFn(LINK_TEXT); expect(executeCommandSpy).not.toHaveBeenCalled(); + expect(mockLogger.info).toHaveBeenCalledWith( + { fn: 'ManualPasteInsertFactory.insert', textLength: LINK_TEXT.length }, + 'Link copied to clipboard for manual paste', + ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/insertFactories/manualPasteInsertFactory.test.ts` around lines 34 - 45, The test should also assert that the operation logs expected information: before calling insertFn (from ManualPasteInsertFactory.forTarget) spy on the logger method used (e.g., jest.spyOn(mockLogger, 'info') or the appropriate level used by ManualPasteInsertFactory), then after await insertFn(LINK_TEXT) add an expectation that mockLogger.<level> was called (and optionally with the expected message or partial match) to ensure logging occurred alongside the clipboard/write behavior; keep the existing executeCommandSpy assertion.
29-30: Replace semantic magic number with a named constant.
18is tied toLINK_TEXTlength and becomes brittle if test input changes.As per coding guidelines "Define named constants for all numeric literals with semantic meaning using SCREAMING_SNAKE_CASE for constant names".♻️ Proposed cleanup
const LINK_TEXT = 'src/app.ts#L10-L20'; +const LINK_TEXT_LENGTH = LINK_TEXT.length; @@ expect(mockLogger.info).toHaveBeenCalledWith( - { fn: 'ManualPasteInsertFactory.insert', textLength: 18 }, + { fn: 'ManualPasteInsertFactory.insert', textLength: LINK_TEXT_LENGTH }, 'Link copied to clipboard for manual paste', );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/insertFactories/manualPasteInsertFactory.test.ts` around lines 29 - 30, Replace the magic number 18 used in the test expectation with a named constant: declare a SCREAMING_SNAKE_CASE constant (e.g., LINK_TEXT_LENGTH) and set it to LINK_TEXT.length (or the appropriate source string length) and use that constant in the test tuple entry for ManualPasteInsertFactory.insert so the expected textLength stays correct if LINK_TEXT changes.packages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/resolveFocusTier.test.ts (1)
20-169: Add logger assertions to the remaining behavior tests.Several tests validate resolver behavior but don’t verify corresponding debug logs, which leaves observability contracts partially untested.
As per coding guidelines "Include logger assertions in tests that verify method behavior - logging provides critical visibility for debugging and should not be tested separately".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/resolveFocusTier.test.ts` around lines 20 - 169, Add logger assertions to the tests that currently lack them: in the "resolves to first tier when its command is registered" test assert mockLogger.debug was called (or at least mockLogger.debug.mock.calls length > 0) with { fn: 'resolveFocusTier', tier: 'insertCommands', checkedCommands: ['sparkAi.insertText'], logPrefix: LOG_PREFIX } and the matching message; in "skips tiers with empty command arrays" assert mockLogger.debug was called with { fn: 'resolveFocusTier', tier: 'insertCommands', checkedCommands: [], logPrefix: LOG_PREFIX } and the message about no registered commands for insertCommands (same shape as the existing assertion in the other test); and in any success-path tests that currently lack logging checks (e.g., first test) ensure you also assert mockLogger.warn was not called (mockLogger.warn.toHaveBeenCalledTimes(0)) where appropriate; use resolveFocusTier, mockLogger, LOG_PREFIX, and the tier labels from the test to locate and implement these assertions.packages/rangelink-vscode-extension/src/clipboard/ClipboardPreserver.ts (1)
2-13: Minor JSDoc wording drift: “only the operation” is no longer fully accurate.Please update the earlier note so docs consistently reflect the new optional
shouldRestoreargument.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/clipboard/ClipboardPreserver.ts` around lines 2 - 13, Update the JSDoc sentence that currently reads "Call sites pass only the operation to wrap." to indicate callers may also pass the optional shouldRestore callback; specifically, in ClipboardPreserver.ts (the JSDoc for the wrapper that takes fn and shouldRestore) change that note to something like "Call sites pass the operation to wrap and may optionally pass a shouldRestore callback to decide post-operation clipboard restoration." Keep the rest of the param descriptions for fn and shouldRestore intact.packages/rangelink-vscode-extension/src/__tests__/services/ClipboardRouter.test.ts (1)
137-139: Avoid indexingmock.calls; capture/assert callback via matcher + implementation.Direct
.mock.calls[0]access is brittle and against the test-style rule for mock parameter verification.Proposed refactor
+ let capturedShouldRestore: (() => boolean) | undefined; + mockPreserver.preserve.mockImplementation(async (fn, shouldRestore) => { + capturedShouldRestore = shouldRestore; + return fn(); + }); + await router.copyAndSendToDestination(options); - const shouldRestoreCallback = mockPreserver.preserve.mock.calls[0][1] as () => boolean; - expect(shouldRestoreCallback()).toBe(false); + expect(mockPreserver.preserve).toHaveBeenCalledWith( + expect.any(Function), + expect.any(Function), + ); + expect(capturedShouldRestore?.()).toBe(false); expect(mockDestinationManager.isClipboardRestorationApplicable).toHaveBeenCalledTimes(1);As per coding guidelines: “
**/*.test.ts: Use.toHaveBeenCalledWith(param1, param2, ...)… instead of accessing.mock.calls[0]”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/__tests__/services/ClipboardRouter.test.ts` around lines 137 - 139, Replace the brittle direct access to mockPreserver.preserve.mock.calls[0][1] by capturing the callback via a mockImplementation and asserting the call using matchers: set mockPreserver.preserve.mockImplementation((arg, shouldRestore) => { capturedShouldRestore = shouldRestore; return /* appropriate value */; }); then assert expect(mockPreserver.preserve).toHaveBeenCalledWith(expect.any(Function), expect.any(Function)) (or other specific matchers), call expect(capturedShouldRestore()).toBe(false), and keep expect(mockDestinationManager.isClipboardRestorationApplicable).toHaveBeenCalledTimes(1); reference the mockPreserver.preserve method and the capturedShouldRestore callback instead of indexing mock.calls.packages/rangelink-vscode-extension/.vscode-test.mjs (1)
7-7: Consider adding explicit cleanup for the user data directory to improve test isolation.The fixed user data directory (
/tmp/rl-vscode-test) combined with the merge-based settings setup enables settings pre-population but allows stale VS Code state to persist between test runs. While the setup script overwrites therangelink.customAiAssistantssetting each timepnpm test:release:prepareruns, other cached extension state and VS Code metadata may remain.Adding
rm -rf /tmp/rl-vscode-testbefore thesetup-integration-test-settings.jsinvocation would ensure a clean slate for each test run.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/.vscode-test.mjs` at line 7, The test setup uses a fixed USER_DATA_DIR (USER_DATA_DIR) which can preserve stale VS Code state across runs; before invoking the setup script (setup-integration-test-settings.js) ensure the directory is removed so each run starts clean: add a pre-step that deletes USER_DATA_DIR (e.g., rm -rf or fs.rmSync(pathToUserDataDir, { recursive: true, force: true })) and then invoke the settings setup so only the intended settings are populated.packages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/ResolvedFocusCapability.test.ts (1)
34-37: Use the repo’sResultmatchers instead of unwrappingresult.success.These assertions should use
toBeOk()/toBeErr()so the suite stays consistent with the rest of the test helpers and fails with better diagnostics.Example
- expect(result.success).toBe(true); + expect(result).toBeOk(); @@ - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error.reason).toBe('COMMAND_FOCUS_FAILED'); - } + expect(result).toBeErr(); + expect(result.error.reason).toBe('COMMAND_FOCUS_FAILED');Based on learnings: Applies to **/*.test.ts : Use custom matchers for Result and error assertions:
toBeRangeLinkExtensionErrorErr(),toBeOkWith(),toBeOk(),toBeErr()instead of manually unwrapping Result types withresult.successchecks.Also applies to: 59-63, 82-84, 102-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/ResolvedFocusCapability.test.ts` around lines 34 - 37, Replace the manual unwrap of the Result object in the test (the `result` from `capability.focus(CONTEXT)`) with the repo’s custom Result matchers: use `expect(result).toBeOk()` (or `toBeOkWith(...)` if you need to assert the payload) instead of `expect(result.success).toBe(true)`, and keep the existing spy assertion `expect(factory.forTarget).toHaveBeenCalled()`; also apply the same change to the other Result assertions called out (lines referenced in the comment) by switching any `result.success` checks to `toBeOk()`/`toBeErr()`/`toBeRangeLinkExtensionErrorErr()` as appropriate.packages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/LazyResolvedFocusCapability.test.ts (1)
39-46: Switch these assertions to the sharedResultmatchers.The new suite is manually checking
result.success; repo tests standardize ontoBeOk()/toBeErr()for this.Example
- expect(result1.success).toBe(true); + expect(result1).toBeOk(); @@ - expect(result1.success).toBe(false); + expect(result1).toBeErr(); @@ - expect(result.success).toBe(true); + expect(result).toBeOk();Based on learnings: Applies to **/*.test.ts : Use custom matchers for Result and error assertions:
toBeRangeLinkExtensionErrorErr(),toBeOkWith(),toBeOk(),toBeErr()instead of manually unwrapping Result types withresult.successchecks.Also applies to: 62-66, 150-152
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/LazyResolvedFocusCapability.test.ts` around lines 39 - 46, Replace the manual Result assertions on the focus calls with the repo's shared Result matchers: for both await capability.focus(CONTEXT) results (result1 and result2) use toBeOk() (or toBeOkWith(...) if you want to assert payload) instead of checking result.success, keep the existing check for capability.resolvedTierLabel === 'insertCommands' and the spy assertion expect(getCommandsSpy).toHaveBeenCalledTimes(1); also update the other similar checks in this test (the other occurrences around the same test for focus) to use toBeOk()/toBeErr()/toBeOkWith() as appropriate.packages/rangelink-vscode-extension/src/__tests__/helpers/createMockFocusCapabilityFactory.ts (1)
9-14: Make the lazy-capability mock stateful.The real
LazyResolvedFocusCapabilityonly exposesresolvedTierLabel/isFallbackResolutionafter resolution. This fixed object never changes, so tests using this helper can't exercise the pre/post-focus behavior that now controls clipboard restoration and user instructions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/__tests__/helpers/createMockFocusCapabilityFactory.ts` around lines 9 - 14, The mock createMockLazyResolvedFocusCapability currently returns a fixed object so resolvedTierLabel/isFallbackResolution never change; update createMockLazyResolvedFocusCapability to return a stateful jest.Mocked<LazyResolvedFocusCapability> where resolvedTierLabel and isFallbackResolution start as undefined/false and the focus method (jest.fn()) updates those fields (and any resolved payload) when invoked so tests can observe pre-focus and post-focus states; keep using the same function name createMockLazyResolvedFocusCapability and the focus jest.fn to implement the state transition and any return value expected by callers.
🤖 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/.vscode-test.automated.mjs`:
- Line 8: The USER_DATA_DIR constant currently points to a fixed temp path which
can cause state leakage across test runs; change USER_DATA_DIR to create a
unique per-run directory (e.g., via fs.mkdtempSync or by appending a run-unique
suffix such as Date.now(), process.pid or a UUID) so each test invocation uses
an isolated user-data dir; update any teardown/cleanup logic that expects the
old fixed path to remove the created temp directory after the run.
In `@packages/rangelink-vscode-extension/package.json`:
- Around line 347-406: The schema currently allows an assistant entry with none
of the tier arrays populated, so add validation to the object defined by "items"
to require at least one tier and to forbid empty tier arrays: (1) add an "anyOf"
at the same level as "properties" for the assistant object that enforces at
least one of ["insertCommands","focusAndPasteCommands","focusCommands"] is
present; (2) add "minItems": 1 to each of the three array schemas
("insertCommands", "focusAndPasteCommands", "focusCommands") so empty arrays are
invalid; this aligns the JSON schema with the runtime check in
parseCustomAiAssistants().
- Around line 342-406: Add an automated test that verifies Tier 2
("focusAndPasteCommands") preserves clipboard after RangeLink execution: update
or add a new integration spec (e.g., custom-ai-assistant-018) that registers a
fake assistant with focusAndPasteCommands present, simulates selecting it,
asserts the paste flow runs, and then checks the clipboard content was restored;
reference the existing parseCustomAiAssistants.test.ts and the
focusAndPasteCommands/insertCommands handling logic to mirror other cases
(010–017) and ensure the clipboard-preservation assertion is included.
In `@packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yaml`:
- Around line 2556-2558: Update the assisted test step that still uses the old
command label 'Copy Link with Relative Path' to the new command name 'Send
RangeLink' so testers will match current UI; specifically replace the YAML list
entry string "Execute R-L (Copy Link with Relative Path)" with "Execute R-L
(Send RangeLink)" in qa-test-cases-v1.1.0-003.yaml (the step near the entries
mentioning 'Bind to Dummy AI (Tier 1) via R-D picker' and 'Check dummy extension
textarea via dummyAi.getText') so the test refers to the renamed flow.
In
`@packages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.ts`:
- Around line 39-40: Replace the magic numeric literals in the test assertions
with SCREAMING_SNAKE_CASE named constants to express intent: define constants
such as EXPECTED_CUSTOM_ASSISTANTS_COUNT = 6, EXPECTED_ACTIVE_ASSISTANTS_COUNT =
5, EXPECTED_DISABLED_ASSISTANTS_COUNT = 1, EXPECTED_ZERO_ASSISTANTS = 0 (or
names that match the semantic meaning used in these tests) at the top of the
test file, then update the parseLogLine.includes(...) assertions (and the other
occurrences mentioned around the same assertions) to use those constants instead
of raw numbers so the intent of parseLogLine.includes('"count":6') and the
similar checks is explicit and maintainable.
- Around line 167-177: The test custom-ai-assistant-016 currently only asserts
that a registration log for "github-copilot-chat" exists; update it to also
assert the overridden flag is present and true in the logging details by
inspecting the same captured log line(s) from allLines/logCapture: locate the
registration entry (overrideRegistration) and either check that the text
contains the explicit marker like '"overridden":true' or parse the structured
JSON payload from that log line and assert payload.overridden === true so the
test will fail if the overridden flag is missing or false.
- Around line 21-541: Add QA YAML entries to qa-test-cases-v1.1.0.yaml for every
test ID referenced in
src/__integration-tests__/suite/customAiAssistants.test.ts: create entries for
custom-ai-assistant-001 through custom-ai-assistant-009 with automated: false,
and for custom-ai-assistant-010 through custom-ai-assistant-017 with automated:
assisted; ensure each entry uses the exact id string (e.g.,
"custom-ai-assistant-001") and includes a short title matching the test purpose
(tier config, direct insert, clipboard handling, template interpolation,
fallback logic) so the QA file mirrors the integration tests.
In
`@packages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/insertFactories/directInsertFactory.test.ts`:
- Around line 36-55: The test for DirectInsertFactory.forTarget() only uses a
single arg so it doesn't exercise spreading of multiple interpolated args;
update the entries passed to DirectInsertFactory to include at least two
template args (e.g., two objects with '${content}' templates) so the code path
that builds and spreads ...argsArray is executed and validated, then assert
executeCommand was called with both arguments in order; also add an assertion
that mockLogger (mockLogger) received the expected success log entry after
insert to cover the logging behavior.
In `@packages/rangelink-vscode-extension/src/config/parseCustomAiAssistants.ts`:
- Around line 87-92: The parser currently drops malformed optional string-array
fields silently; update parseOptionalStringArray to log a warning when an input
exists but fails validation: inside parseOptionalStringArray (which calls
isNonEmptyStringArray), check if raw is not undefined/null and
isNonEmptyStringArray(raw) is false, then call the project's warning/logger
(e.g., processLogger.warn or the parser's existing warning function) with a
message naming the field (pass a fieldName parameter or include context) and the
raw value so the user can diagnose the misconfiguration; apply the same change
to the other optional-array parsing use-site(s) referenced around lines 143-145
(the other helper or call sites for focusAndPasteCommands / focusCommands) so
malformed arrays emit warnings instead of being silently dropped.
- Around line 94-103: Add QA coverage YAML cases for the new user-facing setting
parsing in parseCustomAiAssistants: create tests that (1) register a valid Tier
1 insertCommands assistant and assert it appears and binds, (2) register a valid
Tier 2 focusAndPasteCommands assistant and assert clipboard/paste behavior, (3)
register a valid Tier 3 focusCommands assistant and assert focus-only binding,
(4) include an invalid entry and assert it is skipped with a warning log, (5)
include duplicate assistants with the same extensionId and assert the first is
kept and a warning is emitted, and (6) verify built-in assistant override/merge
semantics when rangelink.customAiAssistants is present; reference
parseCustomAiAssistants and the rangelink.customAiAssistants setting and ensure
QA YAML asserts on emitted warnings and final assistant registry state.
- Around line 127-168: After validating extensionId and extensionName in
parseCustomAiAssistants, trim them and use the trimmed values everywhere:
replace uses of extensionId/extensionName when building kind, checking/inserting
into seenIds, logging, and storing in the resulting CustomAiAssistantConfig with
trimmedExtensionId/trimmedExtensionName; also ensure command arrays produced by
parseInsertCommands and parseOptionalStringArray are normalized to trimmed
non-empty strings (either by updating
parseInsertCommands/parseOptionalStringArray to trim entries or by
mapping/triming the returned arrays before the `if (!insertCommands && ...)`
check and before assigning to config) so IDs and commands don't contain
surrounding whitespace that breaks duplicate checks and lookups.
In
`@packages/rangelink-vscode-extension/src/destinations/capabilities/LazyResolvedFocusCapability.ts`:
- Around line 51-105: Concurrent calls to focus() can race because multiple
callers may pass the if (!this.resolved) check and duplicate resolution; add an
in-flight resolution promise (e.g. this.resolutionPromise) that is assigned
before awaiting ideAdapter.getCommands() so subsequent callers await the same
promise, ensure the promise resolves by setting this.resolutionResult,
this.resolved (new ResolvedFocusCapability(...)) or this.resolutionFailed on
error, clear the promise after resolution/failure, and change the end of focus()
to await the in-flight promise when present and then return
this.resolved.focus(context).
In
`@packages/rangelink-vscode-extension/src/destinations/DestinationAvailabilityService.ts`:
- Around line 248-253: The custom-AI branch bypasses the central logging by
calling destination.isAvailable() directly; change it to invoke
DestinationAvailabilityService.isAIAssistantAvailable(kind, destination?)
instead (or route through the existing isAIAssistantAvailable method) so the
availability check and the debug log are emitted consistently for custom
assistants; locate the branch using isCustomAiAssistantKind(kind), replace the
direct registry.create(...)/destination.isAvailable() call with a call to the
service's isAIAssistantAvailable (passing the same kind and/or the created
destination as needed) and use its returned boolean and displayName for the
subsequent logic.
In `@packages/rangelink-vscode-extension/src/destinations/destinationBuilders.ts`:
- Around line 294-317: createOverriddenBuiltinBuilder currently returns
isAvailable() that only calls builtin.isAvailable(context); change it to consult
the user tiers first: use the same checks the focusCapability factory uses for
availability on the built user tiers produced by buildCustomAIAssistantTiers
(and/or the lazyCapability created by createLazyResolvedCapability) and if any
user tier is available return true, otherwise fall back to calling
builtin.isAvailable(context). Reference symbols: createOverriddenBuiltinBuilder,
userTiers, fallbackTier, lazyCapability, buildCustomAIAssistantTiers,
buildBuiltinFallbackTier, createLazyResolvedCapability, and builtin.isAvailable.
In `@packages/rangelink-vscode-extension/src/utils/interpolateArgs.ts`:
- Around line 16-18: The current interpolation only replaces the first
occurrence because template.replace(CONTENT_PLACEHOLDER, content) is used;
update the logic in interpolateArgs (the return expression using template,
CONTENT_PLACEHOLDER and content) to replace all occurrences of
CONTENT_PLACEHOLDER (e.g., use template.replaceAll(CONTENT_PLACEHOLDER, content)
or template.replace(new RegExp(escape(CONTENT_PLACEHOLDER), 'g'), content) as a
fallback) so every `${content}` token is substituted rather than just the first
one.
In
`@packages/rangelink-vscode-extension/test-fixtures/dummy-ai-extension/extension.js`:
- Around line 149-158: The requestText() timeout currently resolves with {
tier1: '', tier2: '' }, masking transport failures; change requestText to create
a Promise((resolve, reject)) and store a reject handler (e.g.
this._pendingTextReject) alongside this._pendingTextResolve, keep postMessage({
type: 'getText' }) as-is, and on timeout call the reject handler with a clear
Error (e.g. new Error('webview requestText timeout')) and clear pending
handlers; ensure the message response path (the code that calls
this._pendingTextResolve) still calls resolve and clears both pending handlers.
- Around line 35-42: The focusForPaste message is being posted immediately after
calling dummyAi.chatView.focus() / provider.reveal(), which can occur before the
webview completes its handshake (the script's 'ready' message from
resolveWebviewView()), so change the flow to await the webview handshake before
calling provider.postMessage({ type: 'focusForPaste' }); specifically, after
calling dummyAi.chatView.focus() or provider.reveal() use the
provider._view.webview message event (or a provider-ready promise you add) to
wait for a message of type 'ready' (the same handshake emitted from the webview
script in resolveWebviewView), and only then invoke provider.postMessage({ type:
'focusForPaste' }) so the Tier 2 focus signal is reliably delivered.
---
Nitpick comments:
In `@packages/rangelink-vscode-extension/.vscode-test.mjs`:
- Line 7: The test setup uses a fixed USER_DATA_DIR (USER_DATA_DIR) which can
preserve stale VS Code state across runs; before invoking the setup script
(setup-integration-test-settings.js) ensure the directory is removed so each run
starts clean: add a pre-step that deletes USER_DATA_DIR (e.g., rm -rf or
fs.rmSync(pathToUserDataDir, { recursive: true, force: true })) and then invoke
the settings setup so only the intended settings are populated.
In
`@packages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/insertFactories/manualPasteInsertFactory.test.ts`:
- Around line 34-45: The test should also assert that the operation logs
expected information: before calling insertFn (from
ManualPasteInsertFactory.forTarget) spy on the logger method used (e.g.,
jest.spyOn(mockLogger, 'info') or the appropriate level used by
ManualPasteInsertFactory), then after await insertFn(LINK_TEXT) add an
expectation that mockLogger.<level> was called (and optionally with the expected
message or partial match) to ensure logging occurred alongside the
clipboard/write behavior; keep the existing executeCommandSpy assertion.
- Around line 29-30: Replace the magic number 18 used in the test expectation
with a named constant: declare a SCREAMING_SNAKE_CASE constant (e.g.,
LINK_TEXT_LENGTH) and set it to LINK_TEXT.length (or the appropriate source
string length) and use that constant in the test tuple entry for
ManualPasteInsertFactory.insert so the expected textLength stays correct if
LINK_TEXT changes.
In
`@packages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/LazyResolvedFocusCapability.test.ts`:
- Around line 39-46: Replace the manual Result assertions on the focus calls
with the repo's shared Result matchers: for both await capability.focus(CONTEXT)
results (result1 and result2) use toBeOk() (or toBeOkWith(...) if you want to
assert payload) instead of checking result.success, keep the existing check for
capability.resolvedTierLabel === 'insertCommands' and the spy assertion
expect(getCommandsSpy).toHaveBeenCalledTimes(1); also update the other similar
checks in this test (the other occurrences around the same test for focus) to
use toBeOk()/toBeErr()/toBeOkWith() as appropriate.
In
`@packages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/ResolvedFocusCapability.test.ts`:
- Around line 34-37: Replace the manual unwrap of the Result object in the test
(the `result` from `capability.focus(CONTEXT)`) with the repo’s custom Result
matchers: use `expect(result).toBeOk()` (or `toBeOkWith(...)` if you need to
assert the payload) instead of `expect(result.success).toBe(true)`, and keep the
existing spy assertion `expect(factory.forTarget).toHaveBeenCalled()`; also
apply the same change to the other Result assertions called out (lines
referenced in the comment) by switching any `result.success` checks to
`toBeOk()`/`toBeErr()`/`toBeRangeLinkExtensionErrorErr()` as appropriate.
In
`@packages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/resolveFocusTier.test.ts`:
- Around line 20-169: Add logger assertions to the tests that currently lack
them: in the "resolves to first tier when its command is registered" test assert
mockLogger.debug was called (or at least mockLogger.debug.mock.calls length > 0)
with { fn: 'resolveFocusTier', tier: 'insertCommands', checkedCommands:
['sparkAi.insertText'], logPrefix: LOG_PREFIX } and the matching message; in
"skips tiers with empty command arrays" assert mockLogger.debug was called with
{ fn: 'resolveFocusTier', tier: 'insertCommands', checkedCommands: [],
logPrefix: LOG_PREFIX } and the message about no registered commands for
insertCommands (same shape as the existing assertion in the other test); and in
any success-path tests that currently lack logging checks (e.g., first test)
ensure you also assert mockLogger.warn was not called
(mockLogger.warn.toHaveBeenCalledTimes(0)) where appropriate; use
resolveFocusTier, mockLogger, LOG_PREFIX, and the tier labels from the test to
locate and implement these assertions.
In
`@packages/rangelink-vscode-extension/src/__tests__/helpers/createMockFocusCapabilityFactory.ts`:
- Around line 9-14: The mock createMockLazyResolvedFocusCapability currently
returns a fixed object so resolvedTierLabel/isFallbackResolution never change;
update createMockLazyResolvedFocusCapability to return a stateful
jest.Mocked<LazyResolvedFocusCapability> where resolvedTierLabel and
isFallbackResolution start as undefined/false and the focus method (jest.fn())
updates those fields (and any resolved payload) when invoked so tests can
observe pre-focus and post-focus states; keep using the same function name
createMockLazyResolvedFocusCapability and the focus jest.fn to implement the
state transition and any return value expected by callers.
In
`@packages/rangelink-vscode-extension/src/__tests__/services/ClipboardRouter.test.ts`:
- Around line 137-139: Replace the brittle direct access to
mockPreserver.preserve.mock.calls[0][1] by capturing the callback via a
mockImplementation and asserting the call using matchers: set
mockPreserver.preserve.mockImplementation((arg, shouldRestore) => {
capturedShouldRestore = shouldRestore; return /* appropriate value */; }); then
assert expect(mockPreserver.preserve).toHaveBeenCalledWith(expect.any(Function),
expect.any(Function)) (or other specific matchers), call
expect(capturedShouldRestore()).toBe(false), and keep
expect(mockDestinationManager.isClipboardRestorationApplicable).toHaveBeenCalledTimes(1);
reference the mockPreserver.preserve method and the capturedShouldRestore
callback instead of indexing mock.calls.
In `@packages/rangelink-vscode-extension/src/clipboard/ClipboardPreserver.ts`:
- Around line 2-13: Update the JSDoc sentence that currently reads "Call sites
pass only the operation to wrap." to indicate callers may also pass the optional
shouldRestore callback; specifically, in ClipboardPreserver.ts (the JSDoc for
the wrapper that takes fn and shouldRestore) change that note to something like
"Call sites pass the operation to wrap and may optionally pass a shouldRestore
callback to decide post-operation clipboard restoration." Keep the rest of the
param descriptions for fn and shouldRestore intact.
🪄 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: 1bd39f89-2c5b-46ec-b71e-2a0d9678f8c6
⛔ Files ignored due to path filters (1)
packages/rangelink-vscode-extension/test-fixtures/dummy-ai-extension/media/icon.svgis excluded by!**/*.svg
📒 Files selected for processing (62)
README.mddocs/ADR/0002-three-tier-custom-ai-assistant-commands.mddocs/ADR/README.mdpackages/rangelink-vscode-extension/.vscode-test.automated.mjspackages/rangelink-vscode-extension/.vscode-test.mjspackages/rangelink-vscode-extension/CHANGELOG.mdpackages/rangelink-vscode-extension/README.mdpackages/rangelink-vscode-extension/package.jsonpackages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yamlpackages/rangelink-vscode-extension/scripts/setup-integration-test-settings.jspackages/rangelink-vscode-extension/src/__integration-tests__/suite/bindToDestination.test.tspackages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.tspackages/rangelink-vscode-extension/src/__tests__/constants/packageJsonContracts.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/ComposablePasteDestination.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/DestinationAvailabilityService.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/PasteDestinationManager.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/FocusCapabilityFactory.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/LazyResolvedFocusCapability.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/ResolvedFocusCapability.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/insertFactories/directInsertFactory.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/insertFactories/manualPasteInsertFactory.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/resolveFocusTier.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.tspackages/rangelink-vscode-extension/src/__tests__/helpers/createBaseMockPasteDestination.tspackages/rangelink-vscode-extension/src/__tests__/helpers/createMockClipboardPreserver.tspackages/rangelink-vscode-extension/src/__tests__/helpers/createMockDestinationManager.tspackages/rangelink-vscode-extension/src/__tests__/helpers/createMockDestinationRegistry.tspackages/rangelink-vscode-extension/src/__tests__/helpers/createMockFocusCapabilityFactory.tspackages/rangelink-vscode-extension/src/__tests__/services/ClipboardRouter.test.tspackages/rangelink-vscode-extension/src/__tests__/utils/clipboardPreservation.test.tspackages/rangelink-vscode-extension/src/clipboard/ClipboardPreserver.tspackages/rangelink-vscode-extension/src/clipboard/DefaultClipboardPreserver.tspackages/rangelink-vscode-extension/src/config/__tests__/parseCustomAiAssistants.test.tspackages/rangelink-vscode-extension/src/config/parseCustomAiAssistants.tspackages/rangelink-vscode-extension/src/createWiringServices.tspackages/rangelink-vscode-extension/src/destinations/ComposablePasteDestination.tspackages/rangelink-vscode-extension/src/destinations/DestinationAvailabilityService.tspackages/rangelink-vscode-extension/src/destinations/PasteDestination.tspackages/rangelink-vscode-extension/src/destinations/PasteDestinationManager.tspackages/rangelink-vscode-extension/src/destinations/capabilities/FocusCapabilityFactory.tspackages/rangelink-vscode-extension/src/destinations/capabilities/LazyResolvedFocusCapability.tspackages/rangelink-vscode-extension/src/destinations/capabilities/ResolvedFocusCapability.tspackages/rangelink-vscode-extension/src/destinations/capabilities/insertFactories/directInsertFactory.tspackages/rangelink-vscode-extension/src/destinations/capabilities/insertFactories/index.tspackages/rangelink-vscode-extension/src/destinations/capabilities/insertFactories/manualPasteInsertFactory.tspackages/rangelink-vscode-extension/src/destinations/capabilities/resolveFocusTier.tspackages/rangelink-vscode-extension/src/destinations/destinationBuilders.tspackages/rangelink-vscode-extension/src/destinations/types/FocusTier.tspackages/rangelink-vscode-extension/src/destinations/types/index.tspackages/rangelink-vscode-extension/src/destinations/utils/buildDestinationQuickPickItems.tspackages/rangelink-vscode-extension/src/i18n/messages.en.tspackages/rangelink-vscode-extension/src/services/ClipboardRouter.tspackages/rangelink-vscode-extension/src/types/BindOptions.tspackages/rangelink-vscode-extension/src/types/DestinationKind.tspackages/rangelink-vscode-extension/src/types/MessageCode.tspackages/rangelink-vscode-extension/src/types/index.tspackages/rangelink-vscode-extension/src/utils/__tests__/interpolateArgs.test.tspackages/rangelink-vscode-extension/src/utils/clipboardPreservation.tspackages/rangelink-vscode-extension/src/utils/index.tspackages/rangelink-vscode-extension/src/utils/interpolateArgs.tspackages/rangelink-vscode-extension/test-fixtures/dummy-ai-extension/extension.jspackages/rangelink-vscode-extension/test-fixtures/dummy-ai-extension/package.json
✅ Files skipped from review due to trivial changes (12)
- docs/ADR/README.md
- packages/rangelink-vscode-extension/src/utils/index.ts
- packages/rangelink-vscode-extension/src/destinations/types/index.ts
- packages/rangelink-vscode-extension/src/destinations/capabilities/insertFactories/index.ts
- packages/rangelink-vscode-extension/src/types/MessageCode.ts
- packages/rangelink-vscode-extension/scripts/setup-integration-test-settings.js
- README.md
- packages/rangelink-vscode-extension/src/i18n/messages.en.ts
- packages/rangelink-vscode-extension/src/destinations/types/FocusTier.ts
- packages/rangelink-vscode-extension/src/createWiringServices.ts
- packages/rangelink-vscode-extension/src/integration-tests/suite/bindToDestination.test.ts
- packages/rangelink-vscode-extension/test-fixtures/dummy-ai-extension/package.json
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/rangelink-vscode-extension/src/tests/helpers/createMockDestinationRegistry.ts
- packages/rangelink-vscode-extension/src/destinations/utils/buildDestinationQuickPickItems.ts
- packages/rangelink-vscode-extension/src/tests/destinations/DestinationAvailabilityService.test.ts
- packages/rangelink-vscode-extension/src/types/BindOptions.ts
- packages/rangelink-vscode-extension/src/tests/constants/packageJsonContracts.test.ts
- packages/rangelink-vscode-extension/src/types/DestinationKind.ts
- packages/rangelink-vscode-extension/src/config/tests/parseCustomAiAssistants.test.ts
- packages/rangelink-vscode-extension/src/destinations/ComposablePasteDestination.ts
- packages/rangelink-vscode-extension/src/tests/destinations/destinationBuilders.test.ts
| const MOCHA_TIMEOUT_MS = 20_000; | ||
| const ASSISTED_TEST_GREP = '\\[assisted\\]'; | ||
| const USER_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), 'rl-vscode-test-')); | ||
| const USER_DATA_DIR = path.join(os.tmpdir(), 'rl-vscode-test'); |
Contributor
There was a problem hiding this comment.
Use a unique temp user-data dir per run to avoid state leakage.
A fixed USER_DATA_DIR can preserve stale extension state between runs/retries and introduce flaky integration behavior.
Proposed fix
+import * as fs from 'node:fs';
import * as os from 'node:os';
import * as path from 'node:path';
@@
-const USER_DATA_DIR = path.join(os.tmpdir(), 'rl-vscode-test');
+const USER_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), 'rl-vscode-test-'));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const USER_DATA_DIR = path.join(os.tmpdir(), 'rl-vscode-test'); | |
| import * as fs from 'node:fs'; | |
| import * as os from 'node:os'; | |
| import * as path from 'node:path'; | |
| const USER_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), 'rl-vscode-test-')); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/rangelink-vscode-extension/.vscode-test.automated.mjs` at line 8,
The USER_DATA_DIR constant currently points to a fixed temp path which can cause
state leakage across test runs; change USER_DATA_DIR to create a unique per-run
directory (e.g., via fs.mkdtempSync or by appending a run-unique suffix such as
Date.now(), process.pid or a UUID) so each test invocation uses an isolated
user-data dir; update any teardown/cleanup logic that expects the old fixed path
to remove the created temp directory after the run.
Comment on lines
+342
to
+406
| "rangelink.customAiAssistants": { | ||
| "type": "array", | ||
| "default": [], | ||
| "description": "Custom AI assistants for RangeLink destination picker. At least one command tier required per entry. Run \"Developer: Reload Window\" after changes. See README for details.", | ||
| "markdownDescription": "Custom AI assistants for the RangeLink destination picker. At least one command tier required per entry. Run **Developer: Reload Window** after changes. See the README for configuration guide and examples.", | ||
| "items": { | ||
| "type": "object", | ||
| "required": [ | ||
| "extensionId", | ||
| "extensionName" | ||
| ], | ||
| "properties": { | ||
| "extensionId": { | ||
| "type": "string", | ||
| "description": "VS Code extension identifier (publisher.name) used to detect availability" | ||
| }, | ||
| "extensionName": { | ||
| "type": "string", | ||
| "description": "Display name shown in the destination picker menu" | ||
| }, | ||
| "insertCommands": { | ||
| "type": "array", | ||
| "items": { | ||
| "oneOf": [ | ||
| { | ||
| "type": "string", | ||
| "description": "Command ID — link text is passed as the first argument" | ||
| }, | ||
| { | ||
| "type": "object", | ||
| "required": [ | ||
| "command" | ||
| ], | ||
| "properties": { | ||
| "command": { | ||
| "type": "string", | ||
| "description": "VS Code command ID" | ||
| }, | ||
| "args": { | ||
| "description": "Argument template — use ${content} as placeholder for the link text" | ||
| } | ||
| }, | ||
| "additionalProperties": false | ||
| } | ||
| ] | ||
| }, | ||
| "description": "Tier 1: Commands that accept text directly as an argument (clipboard is never touched). Plain strings pass text as the first argument. Objects use ${content} interpolation in the args template." | ||
| }, | ||
| "focusAndPasteCommands": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| }, | ||
| "description": "Tier 2: Commands that focus the AI chat input where VS Code paste commands work. RangeLink auto-pastes via clipboard (clipboard content is preserved)." | ||
| }, | ||
| "focusCommands": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| }, | ||
| "description": "Tier 3: Commands that focus/reveal the AI panel but cannot receive paste commands (e.g., webview-based assistants). RangeLink copies the link to clipboard and shows a \"paste now\" toast. Clipboard content is NOT restored." | ||
| } | ||
| }, | ||
| "additionalProperties": false | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find all QA YAML files in the VSCode extension directory
find packages/rangelink-vscode-extension/qa -name "*.yaml" -type f 2>/dev/null | head -20Repository: couimet/rangeLink
Length of output: 332
🏁 Script executed:
# Also search for any test cases related to customAiAssistants or assistant in integration tests
fd -t f "\.test\.ts$" packages/rangelink-vscode-extension/src/__integration-tests__ 2>/dev/null | head -20Repository: couimet/rangeLink
Length of output: 1922
🏁 Script executed:
# Check if there are any recent QA YAML files to understand the versioning pattern
git ls-files packages/rangelink-vscode-extension/qa/*.yaml 2>/dev/null | sortRepository: couimet/rangeLink
Length of output: 332
🏁 Script executed:
# Check the size and content of the latest QA YAML file
wc -l packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0.yamlRepository: couimet/rangeLink
Length of output: 130
🏁 Script executed:
# Check if customAiAssistants is mentioned in the QA YAML
rg -i "customaiassistant|custom.*assistant" packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0.yamlRepository: couimet/rangeLink
Length of output: 43
🏁 Script executed:
# Check the integration test file for customAiAssistants
wc -l packages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.tsRepository: couimet/rangeLink
Length of output: 159
🏁 Script executed:
# Read the customAiAssistants integration test file
cat -n packages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.tsRepository: couimet/rangeLink
Length of output: 23844
🏁 Script executed:
# Read the QA YAML file to check what custom assistant test cases are documented
# Use sed to extract relevant sections
rg -A 10 -B 2 "custom.*assistant|customai" packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0.yaml -iRepository: couimet/rangeLink
Length of output: 43
🏁 Script executed:
# Also check the numbered QA YAML files to see if they contain more custom assistant cases
for f in packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-*.yaml; do echo "=== $f ==="; rg -i "custom.*assistant|customai" "$f" || echo "No matches"; doneRepository: couimet/rangeLink
Length of output: 3133
🏁 Script executed:
# Read the full QA YAML file section for custom AI assistants to see all details
sed -n '/# Section — Custom AI Assistants/,/# Section/p' packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yaml | head -150Repository: couimet/rangeLink
Length of output: 6821
🏁 Script executed:
# Also check if there's a test for invalid entries or Tier 2 clipboard in integration tests
rg -i "invalid|tier.*2|tier2|clipboard.*preserve" packages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.ts -A 3Repository: couimet/rangeLink
Length of output: 3587
🏁 Script executed:
# Check the full QA YAML section for Tier 2 and Tier 3 to see if both clipboard behaviors are documented
sed -n '/custom-ai-assistant-010/,/custom-ai-assistant-020/p' packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yamlRepository: couimet/rangeLink
Length of output: 4844
🏁 Script executed:
# Search for Tier 2 clipboard preservation details in QA YAML
sed -n '/custom-ai-assistant-011/,/custom-ai-assistant-015/p' packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yamlRepository: couimet/rangeLink
Length of output: 2473
🏁 Script executed:
# Search for any test that covers Tier 2 with clipboard preservation (not fallback)
rg -i "tier.?2.*clipboard|clipboard.*tier.?2|focusandpaste" packages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.ts -B 5 -A 5Repository: couimet/rangeLink
Length of output: 2194
🏁 Script executed:
# Check if there's a separate test file or scenario for clipboard preservation with Tier 2
find packages/rangelink-vscode-extension/src/__integration-tests__ -name "*.test.ts" -exec grep -l "tier.?2.*clipboard\|focusAndPaste.*clipboard\|clipboard.*restore" {} \;Repository: couimet/rangeLink
Length of output: 253
🏁 Script executed:
# Check the clipboardPreservation.test.ts file for Tier 2 or custom AI assistant tests
rg -i "tier.*2|custom.*assistant|focusandpaste" packages/rangelink-vscode-extension/src/__integration-tests__/suite/clipboardPreservation.test.ts -B 3 -A 3Repository: couimet/rangeLink
Length of output: 43
🏁 Script executed:
# Also check for any reference to invalid entries in QA YAML or integration tests
rg -i "invalid|malformed|skip" packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yamlRepository: couimet/rangeLink
Length of output: 538
🏁 Script executed:
# Check if there's a test for invalid entries in integration tests
rg -i "invalid|malformed|skip" packages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.tsRepository: couimet/rangeLink
Length of output: 554
🏁 Script executed:
# Let me get full context on what tests exist for Tier 2 by checking QA YAML more broadly
rg "custom-ai-assistant-01[0-9]" packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yaml | cut -d: -f2 | sort -uRepository: couimet/rangeLink
Length of output: 260
🏁 Script executed:
# Check if there's validation or error handling for invalid entries in the source code
find packages/rangelink-vscode-extension/src -name "*.ts" -not -path "*__integration-tests__*" | xargs grep -l "parseCustomAiAssistants\|validateCustomAi\|invalid.*assistant" | head -5Repository: couimet/rangeLink
Length of output: 486
🏁 Script executed:
# Check the unit tests for parseCustomAiAssistants to see if invalid entries are tested
cat packages/rangelink-vscode-extension/src/config/__tests__/parseCustomAiAssistants.test.tsRepository: couimet/rangeLink
Length of output: 10999
🏁 Script executed:
# Also check the actual implementation to understand how invalid entries are handled
wc -l packages/rangelink-vscode-extension/src/config/parseCustomAiAssistants.tsRepository: couimet/rangeLink
Length of output: 138
Most scenarios are covered; verify Tier 2 clipboard restoration test exists.
Invalid entries are handled and tested in unit tests (parseCustomAiAssistants.test.ts), which appropriately validates skipping of missing tiers, missing/duplicate IDs, and malformed entries. Integration tests and QA YAML cover Tier 1 direct insert (010, 017), Tier 1 clipboard isolation (011), Tier 3 manual paste (012), Tier 2→3 fallback (013), ${content} template (014), and built-in override (015, 016). However, there is no explicit test for a successful Tier 2 flow where focusAndPasteCommands are available and clipboard is preserved after R-L execution. Test 013 only covers the fallback case (when Tier 2 commands don't exist). Consider adding a test case (e.g., custom-ai-assistant-018) for Tier 2 clipboard preservation to complete coverage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/rangelink-vscode-extension/package.json` around lines 342 - 406, Add
an automated test that verifies Tier 2 ("focusAndPasteCommands") preserves
clipboard after RangeLink execution: update or add a new integration spec (e.g.,
custom-ai-assistant-018) that registers a fake assistant with
focusAndPasteCommands present, simulates selecting it, asserts the paste flow
runs, and then checks the clipboard content was restored; reference the existing
parseCustomAiAssistants.test.ts and the focusAndPasteCommands/insertCommands
handling logic to mirror other cases (010–017) and ensure the
clipboard-preservation assertion is included.
Comment on lines
+347
to
+406
| "items": { | ||
| "type": "object", | ||
| "required": [ | ||
| "extensionId", | ||
| "extensionName" | ||
| ], | ||
| "properties": { | ||
| "extensionId": { | ||
| "type": "string", | ||
| "description": "VS Code extension identifier (publisher.name) used to detect availability" | ||
| }, | ||
| "extensionName": { | ||
| "type": "string", | ||
| "description": "Display name shown in the destination picker menu" | ||
| }, | ||
| "insertCommands": { | ||
| "type": "array", | ||
| "items": { | ||
| "oneOf": [ | ||
| { | ||
| "type": "string", | ||
| "description": "Command ID — link text is passed as the first argument" | ||
| }, | ||
| { | ||
| "type": "object", | ||
| "required": [ | ||
| "command" | ||
| ], | ||
| "properties": { | ||
| "command": { | ||
| "type": "string", | ||
| "description": "VS Code command ID" | ||
| }, | ||
| "args": { | ||
| "description": "Argument template — use ${content} as placeholder for the link text" | ||
| } | ||
| }, | ||
| "additionalProperties": false | ||
| } | ||
| ] | ||
| }, | ||
| "description": "Tier 1: Commands that accept text directly as an argument (clipboard is never touched). Plain strings pass text as the first argument. Objects use ${content} interpolation in the args template." | ||
| }, | ||
| "focusAndPasteCommands": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| }, | ||
| "description": "Tier 2: Commands that focus the AI chat input where VS Code paste commands work. RangeLink auto-pastes via clipboard (clipboard content is preserved)." | ||
| }, | ||
| "focusCommands": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| }, | ||
| "description": "Tier 3: Commands that focus/reveal the AI panel but cannot receive paste commands (e.g., webview-based assistants). RangeLink copies the link to clipboard and shows a \"paste now\" toast. Clipboard content is NOT restored." | ||
| } | ||
| }, | ||
| "additionalProperties": false | ||
| } |
Contributor
There was a problem hiding this comment.
The schema doesn't enforce the tier requirement it documents.
An entry with only extensionId / extensionName, or with insertCommands: [], still validates here and only gets rejected later by parseCustomAiAssistants(). That means users won't get JSON-schema feedback for the exact misconfigurations this setting is supposed to catch. Add minItems: 1 to each tier array and an item-level anyOf/oneOf that requires at least one of insertCommands, focusAndPasteCommands, or focusCommands.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/rangelink-vscode-extension/package.json` around lines 347 - 406, The
schema currently allows an assistant entry with none of the tier arrays
populated, so add validation to the object defined by "items" to require at
least one tier and to forbid empty tier arrays: (1) add an "anyOf" at the same
level as "properties" for the assistant object that enforces at least one of
["insertCommands","focusAndPasteCommands","focusCommands"] is present; (2) add
"minItems": 1 to each of the three array schemas ("insertCommands",
"focusAndPasteCommands", "focusCommands") so empty arrays are invalid; this
aligns the JSON schema with the runtime check in parseCustomAiAssistants().
Comment on lines
+21
to
+541
| suite('Custom AI Assistants', () => { | ||
| suiteSetup(async () => { | ||
| await activateExtension(); | ||
| }); | ||
|
|
||
| test('custom-ai-assistant-001: three-tier config is parsed and logged at activation', () => { | ||
| const logCapture = getLogCapture(); | ||
| const allLines = logCapture.getAllLines(); | ||
|
|
||
| const parseLogLine = allLines.find( | ||
| (line) => line.includes('parseCustomAiAssistants') && line.includes('Loaded'), | ||
| ); | ||
|
|
||
| assert.ok( | ||
| parseLogLine, | ||
| 'Expected parseCustomAiAssistants INFO log showing loaded custom AI assistants — if missing, the rangelink.customAiAssistants setting may not be configured in the test workspace', | ||
| ); | ||
| assert.ok( | ||
| parseLogLine.includes('"count":6'), | ||
| `Expected 6 custom AI assistants loaded but got: ${parseLogLine}`, | ||
| ); | ||
| assert.ok( | ||
| parseLogLine.includes('rangelink.dummy-ai-extension'), | ||
| `Expected log to mention 'rangelink.dummy-ai-extension' extensionId but got: ${parseLogLine}`, | ||
| ); | ||
| }); | ||
|
|
||
| test('custom-ai-assistant-002: all three tier entries registered as destination kinds', () => { | ||
| const logCapture = getLogCapture(); | ||
| const allLines = logCapture.getAllLines(); | ||
|
|
||
| const registrationLogs = allLines.filter( | ||
| (line) => line.includes('Registering builder for destination') && line.includes('custom-ai:'), | ||
| ); | ||
|
|
||
| assert.strictEqual( | ||
| registrationLogs.length, | ||
| 5, | ||
| `Expected 5 custom AI registrations but found ${registrationLogs.length}: ${registrationLogs.join('\n')}`, | ||
| ); | ||
| assert.ok( | ||
| registrationLogs.some((l) => l.includes('custom-ai:rangelink.dummy-ai-extension"')), | ||
| 'Expected registration for Tier 1 (rangelink.dummy-ai-extension)', | ||
| ); | ||
| assert.ok( | ||
| registrationLogs.some((l) => l.includes('custom-ai:rangelink.dummy-ai-extension-tier2')), | ||
| 'Expected registration for Tier 2 (rangelink.dummy-ai-extension-tier2)', | ||
| ); | ||
| assert.ok( | ||
| registrationLogs.some((l) => l.includes('custom-ai:rangelink.dummy-ai-extension-tier3')), | ||
| 'Expected registration for Tier 3 (rangelink.dummy-ai-extension-tier3)', | ||
| ); | ||
| assert.ok( | ||
| registrationLogs.some((l) => l.includes('custom-ai:rangelink.dummy-ai-extension-template')), | ||
| 'Expected registration for Template (rangelink.dummy-ai-extension-template)', | ||
| ); | ||
| assert.ok( | ||
| registrationLogs.some((l) => l.includes('custom-ai:rangelink.dummy-ai-extension-fallback')), | ||
| 'Expected registration for Fallback (rangelink.dummy-ai-extension-fallback)', | ||
| ); | ||
| }); | ||
|
|
||
| test('custom-ai-assistant-004: dummy extension insertText command is detectable', async () => { | ||
| const commands = await vscode.commands.getCommands(true); | ||
| assert.ok( | ||
| commands.includes('dummyAi.insertText'), | ||
| 'Expected dummyAi.insertText to be registered — dummy-ai-extension should be loaded', | ||
| ); | ||
| }); | ||
|
|
||
| test('custom-ai-assistant-005: tier 1 entry has its insertCommand registered', async () => { | ||
| const commands = await vscode.commands.getCommands(true); | ||
| assert.ok( | ||
| commands.includes('dummyAi.insertText'), | ||
| 'Expected dummyAi.insertText to be registered — Tier 1 entry should be available when this command exists', | ||
| ); | ||
|
|
||
| const logCapture = getLogCapture(); | ||
| const allLines = logCapture.getAllLines(); | ||
| const registrationLog = allLines.find( | ||
| (line) => | ||
| line.includes('Registering builder for destination') && | ||
| line.includes('custom-ai:rangelink.dummy-ai-extension"'), | ||
| ); | ||
| assert.ok( | ||
| registrationLog, | ||
| 'Expected Tier 1 entry (rangelink.dummy-ai-extension) to be registered as a destination kind', | ||
| ); | ||
| }); | ||
|
|
||
| test('custom-ai-assistant-006: dummy extension commands are all registered', async () => { | ||
| const commands = await vscode.commands.getCommands(true); | ||
|
|
||
| assert.ok(commands.includes('dummyAi.insertText'), 'Expected dummyAi.insertText command'); | ||
| assert.ok( | ||
| commands.includes('dummyAi.insertWithArgs'), | ||
| 'Expected dummyAi.insertWithArgs command', | ||
| ); | ||
| assert.ok(commands.includes('dummyAi.focusForPaste'), 'Expected dummyAi.focusForPaste command'); | ||
| assert.ok(commands.includes('dummyAi.focusPanel'), 'Expected dummyAi.focusPanel command'); | ||
| assert.ok(commands.includes('dummyAi.getText'), 'Expected dummyAi.getText command'); | ||
| assert.ok(commands.includes('dummyAi.clearAll'), 'Expected dummyAi.clearAll command'); | ||
| }); | ||
|
|
||
| test('custom-ai-assistant-009: template entry with ${content} interpolation is parsed and registered', () => { | ||
| const logCapture = getLogCapture(); | ||
| const allLines = logCapture.getAllLines(); | ||
|
|
||
| const registrationLog = allLines.find( | ||
| (line) => | ||
| line.includes('Registering builder for destination') && | ||
| line.includes('custom-ai:rangelink.dummy-ai-extension-template'), | ||
| ); | ||
|
|
||
| assert.ok( | ||
| registrationLog, | ||
| 'Expected registration for template entry (rangelink.dummy-ai-extension-template)', | ||
| ); | ||
| }); | ||
|
|
||
| test('custom-ai-assistant-015: built-in override registers under built-in kind', () => { | ||
| const logCapture = getLogCapture(); | ||
| const allLines = logCapture.getAllLines(); | ||
|
|
||
| const overrideRegistration = allLines.find( | ||
| (line) => | ||
| line.includes('Registering builder for destination') && | ||
| line.includes('"kind":"github-copilot-chat"'), | ||
| ); | ||
| assert.ok( | ||
| overrideRegistration, | ||
| 'Expected override entry (github.copilot-chat extensionId) to register under github-copilot-chat kind', | ||
| ); | ||
|
|
||
| const customAiRegistrations = allLines.filter( | ||
| (line) => | ||
| line.includes('Registering builder for destination') && | ||
| line.includes('"kind":"github-copilot-chat"'), | ||
| ); | ||
| assert.strictEqual( | ||
| customAiRegistrations.length, | ||
| 1, | ||
| `Expected exactly 1 registration for github-copilot-chat (override replaces built-in) but found ${customAiRegistrations.length}`, | ||
| ); | ||
| }); | ||
|
|
||
| test('custom-ai-assistant-016: built-in override carries overridden flag in logging details', async () => { | ||
| const logCapture = getLogCapture(); | ||
| const allLines = logCapture.getAllLines(); | ||
|
|
||
| const overrideRegistration = allLines.find( | ||
| (line) => | ||
| line.includes('Registering builder for destination') && | ||
| line.includes('"kind":"github-copilot-chat"'), | ||
| ); | ||
| assert.ok(overrideRegistration, 'Expected github-copilot-chat registration log'); | ||
|
|
||
| const noSeparateCustomRegistration = allLines.filter( | ||
| (line) => | ||
| line.includes('Registering builder for destination') && | ||
| line.includes('custom-ai:github.copilot-chat'), | ||
| ); | ||
| assert.strictEqual( | ||
| noSeparateCustomRegistration.length, | ||
| 0, | ||
| 'Override should NOT register as custom-ai:github.copilot-chat — it takes over the built-in kind', | ||
| ); | ||
| }); | ||
|
|
||
| test('custom-ai-assistant-008: parser normalizes plain string insertCommands', () => { | ||
| const logCapture = getLogCapture(); | ||
| const allLines = logCapture.getAllLines(); | ||
|
|
||
| const parseLog = allLines.find( | ||
| (line) => line.includes('parseCustomAiAssistants') && line.includes('Loaded'), | ||
| ); | ||
|
|
||
| assert.ok( | ||
| parseLog, | ||
| 'Expected parseCustomAiAssistants to load assistants with insertCommands configured as plain strings', | ||
| ); | ||
| assert.ok( | ||
| parseLog.includes('rangelink.dummy-ai-extension'), | ||
| `Expected loaded assistant to include rangelink.dummy-ai-extension but got: ${parseLog}`, | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| suite('Custom AI Assistants — Cold Start', () => { | ||
| const log = createLogger('customAiColdStart'); | ||
| const tmpFileUris: vscode.Uri[] = []; | ||
|
|
||
| suiteSetup(async () => { | ||
| await activateExtension(); | ||
| printAssistedBanner(); | ||
| }); | ||
|
|
||
| teardown(async () => { | ||
| await vscode.commands.executeCommand('rangelink.unbindDestination'); | ||
| await vscode.commands.executeCommand('dummyAi.clearAll'); | ||
| await closeAllEditors(); | ||
| cleanupFiles(tmpFileUris); | ||
| tmpFileUris.length = 0; | ||
| await settle(); | ||
| }); | ||
|
|
||
| test('[assisted] custom-ai-assistant-017: Tier 1 direct insert works when panel is not yet visible', async () => { | ||
| const uri = await createAndOpenFile('__rl-test-cold-start', 'cold start test'); | ||
| tmpFileUris.push(uri); | ||
| await settle(); | ||
|
|
||
| await waitForHuman( | ||
| 'custom-ai-assistant-017', | ||
| "Cmd+R Cmd+D → select 'Dummy AI (Tier 1)' (panel should NOT be open yet)", | ||
| ['Press Cmd+R Cmd+D and select "Dummy AI (Tier 1)" from the picker.'], | ||
| ); | ||
|
|
||
| const logCapture = getLogCapture(); | ||
| logCapture.mark('before-cold-start'); | ||
|
|
||
| await vscode.commands.executeCommand('editor.action.selectAll'); | ||
| await vscode.commands.executeCommand('rangelink.copyLinkWithRelativePath'); | ||
| await settle(); | ||
|
|
||
| const lines = logCapture.getLinesSince('before-cold-start'); | ||
|
|
||
| const directInsertLog = lines.find( | ||
| (line) => | ||
| line.includes('DirectInsertFactory.insert') && line.includes('Direct insert succeeded'), | ||
| ); | ||
| assert.ok(directInsertLog, 'Expected DirectInsertFactory.insert success log'); | ||
|
|
||
| const textResult = (await vscode.commands.executeCommand('dummyAi.getText')) as | ||
| | { tier1: string; tier2: string } | ||
| | undefined; | ||
| assert.ok(textResult, 'Expected dummyAi.getText to return a result'); | ||
| assert.ok( | ||
| textResult!.tier1.length > 0, | ||
| 'Expected tier1 textarea to contain the link even though panel was not pre-opened', | ||
| ); | ||
| assert.strictEqual( | ||
| textResult!.tier2, | ||
| '', | ||
| 'Expected tier2 textarea to be empty (no cross-contamination)', | ||
| ); | ||
|
|
||
| log('✓ Tier 1 cold start — panel auto-initialized, text delivered'); | ||
| }); | ||
| }); | ||
|
|
||
| suite('Custom AI Assistants — Paste Flow', () => { | ||
| const log = createLogger('customAiPasteFlow'); | ||
| const tmpFileUris: vscode.Uri[] = []; | ||
|
|
||
| suiteSetup(async () => { | ||
| await activateExtension(); | ||
| await vscode.commands.executeCommand('dummyAi.focusPanel'); | ||
| await settle(); | ||
| await vscode.commands.executeCommand('dummyAi.getText'); | ||
| printAssistedBanner(); | ||
| }); | ||
|
|
||
| teardown(async () => { | ||
| await vscode.commands.executeCommand('rangelink.unbindDestination'); | ||
| await vscode.commands.executeCommand('dummyAi.clearAll'); | ||
| await closeAllEditors(); | ||
| cleanupFiles(tmpFileUris); | ||
| tmpFileUris.length = 0; | ||
| await settle(); | ||
| }); | ||
|
|
||
| test('[assisted] custom-ai-assistant-010: Tier 1 direct insert delivers text to dummy textarea', async () => { | ||
| const uri = await createAndOpenFile('__rl-test-tier1', 'hello world\nline two\nline three'); | ||
| tmpFileUris.push(uri); | ||
| await settle(); | ||
|
|
||
| await waitForHuman('custom-ai-assistant-010', "Cmd+R Cmd+D → select 'Dummy AI (Tier 1)'", [ | ||
| 'Press Cmd+R Cmd+D and select "Dummy AI (Tier 1)" from the picker.', | ||
| ]); | ||
|
|
||
| const logCapture = getLogCapture(); | ||
| logCapture.mark('before-tier1-paste'); | ||
|
|
||
| await vscode.commands.executeCommand('editor.action.selectAll'); | ||
| await vscode.commands.executeCommand('rangelink.copyLinkWithRelativePath'); | ||
| await settle(); | ||
|
|
||
| const lines = logCapture.getLinesSince('before-tier1-paste'); | ||
|
|
||
| const directInsertLog = lines.find( | ||
| (line) => | ||
| line.includes('DirectInsertFactory.insert') && line.includes('Direct insert succeeded'), | ||
| ); | ||
| assert.ok(directInsertLog, 'Expected DirectInsertFactory.insert success log'); | ||
|
|
||
| const textResult = (await vscode.commands.executeCommand('dummyAi.getText')) as | ||
| | { tier1: string; tier2: string } | ||
| | undefined; | ||
| assert.ok(textResult, 'Expected dummyAi.getText to return a result'); | ||
| assert.ok( | ||
| textResult!.tier1.length > 0, | ||
| `Expected tier1 textarea to contain the link but got empty string`, | ||
| ); | ||
| assert.strictEqual( | ||
| textResult!.tier2, | ||
| '', | ||
| 'Expected tier2 textarea to be empty (no cross-contamination)', | ||
| ); | ||
|
|
||
| log('✓ Tier 1 direct insert delivered text to dummy textarea'); | ||
| }); | ||
|
|
||
| test('[assisted] custom-ai-assistant-011: Tier 1 clipboard isolation — sentinel preserved', async () => { | ||
| const uri = await createAndOpenFile('__rl-test-tier1-clip', 'clipboard test'); | ||
| tmpFileUris.push(uri); | ||
| await settle(); | ||
|
|
||
| await waitForHuman('custom-ai-assistant-011', "Cmd+R Cmd+D → select 'Dummy AI (Tier 1)'", [ | ||
| 'Press Cmd+R Cmd+D and select "Dummy AI (Tier 1)" from the picker.', | ||
| ]); | ||
|
|
||
| await writeClipboardSentinel(); | ||
| const logCapture = getLogCapture(); | ||
| logCapture.mark('before-tier1-clip'); | ||
|
|
||
| await vscode.commands.executeCommand('editor.action.selectAll'); | ||
| await vscode.commands.executeCommand('rangelink.copyLinkWithRelativePath'); | ||
| await settle(); | ||
|
|
||
| await assertClipboardRestored( | ||
| 'Tier 1 should not disturb clipboard — outer preserve restores sentinel', | ||
| ); | ||
|
|
||
| log('✓ Tier 1 clipboard isolation — sentinel preserved after R-L'); | ||
| }); | ||
|
|
||
| test('[assisted] custom-ai-assistant-012: Tier 3 shows manual-paste toast and clipboard not restored', async () => { | ||
| const uri = await createAndOpenFile('__rl-test-tier3', 'tier three test'); | ||
| tmpFileUris.push(uri); | ||
| await settle(); | ||
|
|
||
| await waitForHuman('custom-ai-assistant-012', "Cmd+R Cmd+D → select 'Dummy AI (Tier 3)'", [ | ||
| 'Press Cmd+R Cmd+D and select "Dummy AI (Tier 3)" from the picker.', | ||
| ]); | ||
|
|
||
| await writeClipboardSentinel(); | ||
| const logCapture = getLogCapture(); | ||
| logCapture.mark('before-tier3-paste'); | ||
|
|
||
| await vscode.commands.executeCommand('editor.action.selectAll'); | ||
| await vscode.commands.executeCommand('rangelink.copyLinkWithRelativePath'); | ||
| await settle(); | ||
|
|
||
| const lines = logCapture.getLinesSince('before-tier3-paste'); | ||
|
|
||
| assertToastLogged(lines, { | ||
| type: 'info', | ||
| message: 'Paste (Cmd/Ctrl+V) in Dummy AI (Tier 3) to use.', | ||
| }); | ||
|
|
||
| const manualPasteLog = lines.find( | ||
| (line) => | ||
| line.includes('ManualPasteInsertFactory.insert') && | ||
| line.includes('Link copied to clipboard for manual paste'), | ||
| ); | ||
| assert.ok(manualPasteLog, 'Expected ManualPasteInsertFactory success log'); | ||
|
|
||
| const clipboardContent = await assertClipboardChanged( | ||
| 'Tier 3 clipboard should NOT be restored — link must stay for manual paste', | ||
| ); | ||
| assert.ok(clipboardContent.length > 0, 'Clipboard should contain the RangeLink'); | ||
|
|
||
| const skipRestoreLog = lines.find( | ||
| (line) => | ||
| line.includes('withClipboardPreservation') && | ||
| line.includes('Clipboard restoration skipped'), | ||
| ); | ||
| assert.ok(skipRestoreLog, 'Expected clipboard restoration skip log'); | ||
|
|
||
| await waitForHuman( | ||
| 'custom-ai-assistant-012-paste', | ||
| 'Cmd+V in the Dummy AI tier2 textarea to verify clipboard has the link', | ||
| [ | ||
| 'Click on the Dummy AI sidebar panel (tier2 textarea).', | ||
| 'Press Cmd+V to paste — the RangeLink should appear.', | ||
| ], | ||
| ); | ||
|
|
||
| const textResult = (await vscode.commands.executeCommand('dummyAi.getText')) as | ||
| | { tier1: string; tier2: string } | ||
| | undefined; | ||
| assert.ok(textResult, 'Expected dummyAi.getText to return a result'); | ||
| assert.ok( | ||
| textResult!.tier2.length > 0, | ||
| 'Expected tier2 textarea to contain the pasted link after manual Cmd+V', | ||
| ); | ||
| assert.strictEqual( | ||
| textResult!.tier1, | ||
| '', | ||
| 'Expected tier1 textarea to be empty (Tier 3 uses manual paste, not direct insert)', | ||
| ); | ||
|
|
||
| log( | ||
| '✓ Tier 3 shows manual-paste toast, clipboard not restored (link stays), manual paste verified', | ||
| ); | ||
| }); | ||
|
|
||
| test('[assisted] custom-ai-assistant-013: Tier 2→3 fallback — clipboard not restored and manual paste works', async () => { | ||
| const uri = await createAndOpenFile('__rl-test-fallback', 'fallback test'); | ||
| tmpFileUris.push(uri); | ||
| await settle(); | ||
|
|
||
| await waitForHuman('custom-ai-assistant-013', "Cmd+R Cmd+D → select 'Dummy AI (Fallback)'", [ | ||
| 'Press Cmd+R Cmd+D and select "Dummy AI (Fallback)" from the picker.', | ||
| ]); | ||
|
|
||
| await writeClipboardSentinel(); | ||
| const logCapture = getLogCapture(); | ||
| logCapture.mark('before-fallback-paste'); | ||
|
|
||
| await vscode.commands.executeCommand('editor.action.selectAll'); | ||
| await vscode.commands.executeCommand('rangelink.copyLinkWithRelativePath'); | ||
| await settle(); | ||
|
|
||
| const lines = logCapture.getLinesSince('before-fallback-paste'); | ||
|
|
||
| const tier2SkipLog = lines.find( | ||
| (line) => | ||
| line.includes('resolveFocusTier') && | ||
| line.includes('focusAndPasteCommands') && | ||
| line.includes('no registered commands'), | ||
| ); | ||
| assert.ok(tier2SkipLog, 'Expected Tier 2 skip log — nonexistent.paste not registered'); | ||
|
|
||
| const tier3ResolveLog = lines.find( | ||
| (line) => | ||
| line.includes('resolveFocusTier') && | ||
| line.includes('focusCommands') && | ||
| line.includes('dummyAi.focusPanel'), | ||
| ); | ||
| assert.ok(tier3ResolveLog, 'Expected resolution to focusCommands tier via dummyAi.focusPanel'); | ||
|
|
||
| assertToastLogged(lines, { | ||
| type: 'info', | ||
| message: 'Paste (Cmd/Ctrl+V) in Dummy AI (Fallback) to use.', | ||
| }); | ||
|
|
||
| const clipboardContent = await assertClipboardChanged( | ||
| 'Fallback→Tier 3 clipboard should NOT be restored — link must stay for manual paste', | ||
| ); | ||
| assert.ok(clipboardContent.length > 0, 'Clipboard should contain the RangeLink'); | ||
|
|
||
| const skipRestoreLog = lines.find( | ||
| (line) => | ||
| line.includes('withClipboardPreservation') && | ||
| line.includes('Clipboard restoration skipped'), | ||
| ); | ||
| assert.ok(skipRestoreLog, 'Expected clipboard restoration skip log for fallback→focusCommands'); | ||
|
|
||
| await waitForHuman( | ||
| 'custom-ai-assistant-013-paste', | ||
| 'Cmd+V in the Dummy AI tier2 textarea to verify clipboard has the link', | ||
| [ | ||
| 'Click on the Dummy AI sidebar panel (tier2 textarea).', | ||
| 'Press Cmd+V to paste — the RangeLink should appear.', | ||
| ], | ||
| ); | ||
|
|
||
| const textResult = (await vscode.commands.executeCommand('dummyAi.getText')) as | ||
| | { tier1: string; tier2: string } | ||
| | undefined; | ||
| assert.ok(textResult, 'Expected dummyAi.getText to return a result'); | ||
| assert.ok( | ||
| textResult!.tier2.length > 0, | ||
| 'Expected tier2 textarea to contain the pasted link after manual Cmd+V', | ||
| ); | ||
| assert.strictEqual( | ||
| textResult!.tier1, | ||
| '', | ||
| 'Expected tier1 textarea to be empty (fallback resolved to focusCommands, not direct insert)', | ||
| ); | ||
|
|
||
| log('✓ Tier 2→3 fallback: clipboard not restored, manual paste verified'); | ||
| }); | ||
|
|
||
| test('[assisted] custom-ai-assistant-014: ${content} template delivers text via insertWithArgs', async () => { | ||
| const uri = await createAndOpenFile('__rl-test-template', 'template test content'); | ||
| tmpFileUris.push(uri); | ||
| await settle(); | ||
|
|
||
| await waitForHuman('custom-ai-assistant-014', "Cmd+R Cmd+D → select 'Dummy AI (Template)'", [ | ||
| 'Press Cmd+R Cmd+D and select "Dummy AI (Template)" from the picker.', | ||
| ]); | ||
|
|
||
| const logCapture = getLogCapture(); | ||
| logCapture.mark('before-template-paste'); | ||
|
|
||
| await vscode.commands.executeCommand('editor.action.selectAll'); | ||
| await vscode.commands.executeCommand('rangelink.copyLinkWithRelativePath'); | ||
| await settle(); | ||
|
|
||
| const lines = logCapture.getLinesSince('before-template-paste'); | ||
|
|
||
| const insertLog = lines.find( | ||
| (line) => | ||
| line.includes('DirectInsertFactory.insert') && line.includes('dummyAi.insertWithArgs'), | ||
| ); | ||
| assert.ok(insertLog, 'Expected DirectInsertFactory success log for dummyAi.insertWithArgs'); | ||
|
|
||
| const textResult = (await vscode.commands.executeCommand('dummyAi.getText')) as | ||
| | { tier1: string; tier2: string } | ||
| | undefined; | ||
| assert.ok(textResult, 'Expected dummyAi.getText to return a result'); | ||
| assert.ok( | ||
| textResult!.tier1.length > 0, | ||
| 'Expected tier1 textarea to contain the link via template interpolation', | ||
| ); | ||
|
|
||
| log('✓ ${content} template interpolation delivered text to dummy textarea via insertWithArgs'); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
LATEST_QA_FILE="$(fd -p 'qa-test-cases-*.yaml' packages/rangelink-vscode-extension/qa | sort | tail -n1)"
echo "Using QA file: ${LATEST_QA_FILE}"
# Verify that Custom AI Assistant scenario IDs from this suite exist in QA YAML
rg -n 'custom-ai-assistant-(001|002|004|005|006|008|009|010|011|012|013|014|015|016|017)' "${LATEST_QA_FILE}"Repository: couimet/rangeLink
Length of output: 134
🏁 Script executed:
#!/bin/bash
# Find the QA YAML file with correct path
find packages/rangelink-vscode-extension/qa -type f -name "*.yaml" 2>/dev/null | head -20Repository: couimet/rangeLink
Length of output: 332
🏁 Script executed:
#!/bin/bash
# Search for custom-ai-assistant test IDs in the main QA file (latest version)
echo "=== Searching in qa-test-cases-v1.1.0.yaml ==="
rg -n 'custom-ai-assistant-(001|002|004|005|006|008|009|010|011|012|013|014|015|016|017)' \
packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0.yaml || echo "No matches found"
echo ""
echo "=== File size/line count ==="
wc -l packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0.yamlRepository: couimet/rangeLink
Length of output: 224
🏁 Script executed:
#!/bin/bash
# Search for any custom-ai references in the QA YAML
echo "=== Searching for any 'custom-ai' or 'Custom AI' references ==="
rg -i 'custom.ai|custom AI' packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0.yaml | head -20
echo ""
echo "=== Checking test case ID patterns in QA YAML ==="
rg -o 'id:\s*[a-z0-9\-]+' packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0.yaml | head -30Repository: couimet/rangeLink
Length of output: 935
QA YAML coverage missing for custom AI assistant test cases.
The test suite introduces user-visible custom AI assistant behavior (tier configuration, direct insert, clipboard handling, template interpolation, fallback logic) but no corresponding test case entries exist in qa-test-cases-v1.1.0.yaml.
Add QA YAML entries for these test case IDs with appropriate automated flags:
custom-ai-assistant-001throughcustom-ai-assistant-009: markautomated: false(log-based, require extension configuration)custom-ai-assistant-010throughcustom-ai-assistant-017: markautomated: assisted(require[assisted]human interaction per test markers)
Reference the integration test suite for scenario details, and ensure id values align exactly with test case names in both qa/*.yaml and src/__integration-tests__/suite/customAiAssistants.test.ts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.ts`
around lines 21 - 541, Add QA YAML entries to qa-test-cases-v1.1.0.yaml for
every test ID referenced in
src/__integration-tests__/suite/customAiAssistants.test.ts: create entries for
custom-ai-assistant-001 through custom-ai-assistant-009 with automated: false,
and for custom-ai-assistant-010 through custom-ai-assistant-017 with automated:
assisted; ensure each entry uses the exact id string (e.g.,
"custom-ai-assistant-001") and includes a short title matching the test purpose
(tier config, direct insert, clipboard handling, template interpolation,
fallback logic) so the QA file mirrors the integration tests.
Comment on lines
+248
to
+253
| if (isCustomAiAssistantKind(kind)) { | ||
| const destination = this.registry.create({ kind } as BindOptions); | ||
| const available = await destination.isAvailable(); | ||
| if (!available) break; | ||
|
|
||
| const displayName = destination.displayName; |
Contributor
There was a problem hiding this comment.
Custom AI availability is bypassing the central logging path.
Built-in assistants go through isAIAssistantAvailable(), but this branch calls destination.isAvailable() directly. That means custom assistants skip the DestinationAvailabilityService.isAIAssistantAvailable debug log, so diagnostics are inconsistent and the new QA case expecting an availability-check log for custom assistants won’t be satisfiable from this code path.
Possible fix
if (isCustomAiAssistantKind(kind)) {
const destination = this.registry.create({ kind } as BindOptions);
- const available = await destination.isAvailable();
+ const available = await destination.isAvailable();
+ this.logger.debug(
+ { fn: 'DestinationAvailabilityService.isAIAssistantAvailable', kind, available },
+ `AI assistant ${kind} availability: ${available}`,
+ );
if (!available) break;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/rangelink-vscode-extension/src/destinations/DestinationAvailabilityService.ts`
around lines 248 - 253, The custom-AI branch bypasses the central logging by
calling destination.isAvailable() directly; change it to invoke
DestinationAvailabilityService.isAIAssistantAvailable(kind, destination?)
instead (or route through the existing isAIAssistantAvailable method) so the
availability check and the debug log are emitted consistently for custom
assistants; locate the branch using isCustomAiAssistantKind(kind), replace the
direct registry.create(...)/destination.isAvailable() call with a call to the
service's isAIAssistantAvailable (passing the same kind and/or the created
destination as needed) and use its returned boolean and displayName for the
subsequent logic.
Comment on lines
+294
to
+317
| const createOverriddenBuiltinBuilder = | ||
| (config: CustomAiAssistantConfig, builtin: BuiltinAiAssistantDef): DestinationBuilder => | ||
| (_options, context) => { | ||
| const userTiers = context.factories.focusCapability.buildCustomAIAssistantTiers(config); | ||
| const fallbackTier = context.factories.focusCapability.buildBuiltinFallbackTier( | ||
| builtin.focusAndPasteCommands, | ||
| ); | ||
|
|
||
| const allTiers = [...userTiers, fallbackTier]; | ||
| const fallbackTierIndex = userTiers.length; | ||
|
|
||
| const lazyCapability = context.factories.focusCapability.createLazyResolvedCapability( | ||
| allTiers, | ||
| builtin.displayName, | ||
| fallbackTierIndex, | ||
| ); | ||
|
|
||
| return ComposablePasteDestination.createAiAssistant({ | ||
| id: builtin.kind, | ||
| displayName: builtin.displayName, | ||
| focusCapability: lazyCapability, | ||
| shouldPreserveClipboard: () => lazyCapability.resolvedTierLabel !== 'focusCommands', | ||
| isAvailable: async () => builtin.isAvailable(context), | ||
| jumpSuccessMessage: formatMessage(builtin.jumpMessageCode), |
Contributor
There was a problem hiding this comment.
Overridden built-ins ignore the custom commands during availability checks.
Once a config matches a built-in extensionId, isAvailable() delegates only to builtin.isAvailable(context). That hides valid overrides like the github.copilot-chat fixture in packages/rangelink-vscode-extension/scripts/setup-integration-test-settings.js, where the user tier is dummyAi.insertText: if the real built-in assistant isn't installed/active, the override never appears even though the configured commands work. Availability here needs to consider the user tiers first, then the built-in fallback.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/rangelink-vscode-extension/src/destinations/destinationBuilders.ts`
around lines 294 - 317, createOverriddenBuiltinBuilder currently returns
isAvailable() that only calls builtin.isAvailable(context); change it to consult
the user tiers first: use the same checks the focusCapability factory uses for
availability on the built user tiers produced by buildCustomAIAssistantTiers
(and/or the lazyCapability created by createLazyResolvedCapability) and if any
user tier is available return true, otherwise fall back to calling
builtin.isAvailable(context). Reference symbols: createOverriddenBuiltinBuilder,
userTiers, fallbackTier, lazyCapability, buildCustomAIAssistantTiers,
buildBuiltinFallbackTier, createLazyResolvedCapability, and builtin.isAvailable.
Comment on lines
+35
to
+42
| vscode.commands.registerCommand('dummyAi.focusForPaste', async () => { | ||
| if (!provider?._view) { | ||
| await vscode.commands.executeCommand('dummyAi.chatView.focus'); | ||
| } else { | ||
| provider.reveal(); | ||
| } | ||
| provider?.postMessage({ type: 'focusForPaste' }); | ||
| }), |
Contributor
There was a problem hiding this comment.
Wait for the webview handshake before sending focusForPaste.
On first open, dummyAi.chatView.focus only starts resolveWebviewView(). The immediate postMessage({ type: 'focusForPaste' }) can run before the webview script posts ready, so the Tier 2 focus signal gets lost and the integration flow becomes flaky.
Suggested fix
vscode.commands.registerCommand('dummyAi.focusForPaste', async () => {
- if (!provider?._view) {
- await vscode.commands.executeCommand('dummyAi.chatView.focus');
- } else {
- provider.reveal();
- }
+ await provider?.ensureView();
+ provider?.reveal();
provider?.postMessage({ type: 'focusForPaste' });
}),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/rangelink-vscode-extension/test-fixtures/dummy-ai-extension/extension.js`
around lines 35 - 42, The focusForPaste message is being posted immediately
after calling dummyAi.chatView.focus() / provider.reveal(), which can occur
before the webview completes its handshake (the script's 'ready' message from
resolveWebviewView()), so change the flow to await the webview handshake before
calling provider.postMessage({ type: 'focusForPaste' }); specifically, after
calling dummyAi.chatView.focus() or provider.reveal() use the
provider._view.webview message event (or a provider-ready promise you add) to
wait for a message of type 'ready' (the same handshake emitted from the webview
script in resolveWebviewView), and only then invoke provider.postMessage({ type:
'focusForPaste' }) so the Tier 2 focus signal is reliably delivered.
Comment on lines
+149
to
+158
| requestText() { | ||
| return new Promise((resolve) => { | ||
| this._pendingTextResolve = resolve; | ||
| this.postMessage({ type: 'getText' }); | ||
| setTimeout(() => { | ||
| if (this._pendingTextResolve) { | ||
| this._pendingTextResolve({ tier1: '', tier2: '' }); | ||
| this._pendingTextResolve = undefined; | ||
| } | ||
| }, 2000); |
Contributor
There was a problem hiding this comment.
Don't mask a webview timeout as a valid empty response.
Resolving { tier1: '', tier2: '' } on timeout makes a transport failure indistinguishable from a real empty state. That turns fixture races into misleading assertion failures instead of surfacing the actual problem.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/rangelink-vscode-extension/test-fixtures/dummy-ai-extension/extension.js`
around lines 149 - 158, The requestText() timeout currently resolves with {
tier1: '', tier2: '' }, masking transport failures; change requestText to create
a Promise((resolve, reject)) and store a reject handler (e.g.
this._pendingTextReject) alongside this._pendingTextResolve, keep postMessage({
type: 'getText' }) as-is, and on timeout call the reject handler with a clear
Error (e.g. new Error('webview requestText timeout')) and clear pending
handlers; ensure the message response path (the code that calls
this._pendingTextResolve) still calls resolve and clears both pending handlers.
… style, parser hardening
CI was failing on coverage thresholds because the new destinationBuilders code paths (createCustomAiAssistantBuilder, createOverriddenBuiltinBuilder) had no unit tests. Added 15 tests covering availability checks, shouldPreserveClipboard, getUserInstruction branches, and the full override builder flow. Coverage for destinationBuilders.ts went from 82% to 100% statements.
Fixed a real concurrency bug in LazyResolvedFocusCapability — two rapid focus() calls could both pass the `if (!this.resolved)` check and trigger duplicate getCommands() resolution. Added an in-flight promise guard so the second caller awaits the first resolution.
interpolateArgs used template.replace() which only replaced the first ${content} occurrence. Changed to split/join for ES2017 target compatibility.
Parser now trims extensionId/extensionName before duplicate checks and config storage, preventing whitespace from breaking override lookups. Malformed optional arrays (focusAndPasteCommands, focusCommands) now log a warning instead of being silently dropped.
Test style aligned with CLAUDE.md: Result matchers (toBeOk/toBeErrWith with toStrictEqual) instead of result.success checks, mockImplementation capture instead of mock.calls[0] access, exact error references instead of expect.any(Error), exact values instead of expect.any(Array)/expect.any(Number), logger assertions added to resolveFocusTier and manualPasteInsertFactory tests, magic number replaced with LINK_TEXT_LENGTH, named constants in integration tests. Harmonized tests to use `sparkAi`.
Ignored Feedback:
- Stateful mock for LazyResolvedFocusCapability helper: adds complexity without catching real bugs — the real class has its own test file
- Package.json schema anyOf for "at least one command array": JSON Schema can't express this cleanly; parser enforces at runtime
- DestinationAvailabilityService custom AI logging bypass: custom path has its own logging inside the builder
- Override builder availability should check user tiers: built-in check is the correct abstraction for picker visibility
- Dummy extension requestText timeout should reject: test fixture convention — resolving with empty lets tests assert on empty state
- User data directory cleanup between runs: merge-based approach is intentional for faster cold starts
Ref: #502 (review)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.ts (1)
144-168: Consider asserting theoverriddenflag in the registration log.The test
custom-ai-assistant-015verifies that the override registers under the built-in kind, but doesn't explicitly assert that theoverridden: trueflag is present in the log. Adding this assertion would catch regressions where the override logic runs but doesn't properly mark the destination as overridden.♻️ Suggested enhancement
const overrideRegistration = allLines.find( (line) => line.includes('Registering builder for destination') && line.includes('"kind":"github-copilot-chat"'), ); assert.ok( overrideRegistration, 'Expected override entry (github.copilot-chat extensionId) to register under github-copilot-chat kind', ); + assert.ok( + overrideRegistration.includes('"overridden":true'), + 'Expected override registration to include overridden:true flag', + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.ts` around lines 144 - 168, The test custom-ai-assistant-015 should also assert that the registration log marks the override by checking for the overridden flag; after obtaining log lines via getLogCapture().getAllLines() and locating the overrideRegistration/customAiRegistrations entries, add an assertion that the relevant registration line includes the overridden flag (e.g., contains '"overridden":true' or '"overridden": true') so the test verifies the override is actually marked as overridden in addition to registering under "github-copilot-chat".
🤖 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/src/__tests__/destinations/destinationBuilders.test.ts`:
- Around line 484-517: The tests for createCustomAiAssistantBuilder are fragile
because they stub FocusCapabilityFactory to return a single reused
LazyResolvedFocusCapability instance and then mutate its resolvedTierLabel,
which bypasses the real buildCustomAIAssistantTiers logic and relies on
mockReturnValue reuse; instead update the tests to exercise the actual tier
resolution: stop stubbing buildCustomAIAssistantTiers to return [] (or replace
mockReturnValue with mockImplementation that returns a fresh
LazyResolvedFocusCapability per call), or mock only external dependencies used
inside LazyResolvedFocusCapability (e.g., ideAdapter.getCommands()) so the real
LazyResolvedFocusCapability runs and produces resolvedTierLabel, then assert
shouldPreserveClipboard and getUserInstruction based on the real resolution flow
in createCustomAiAssistantBuilder / FocusCapabilityFactory /
LazyResolvedFocusCapability.
In
`@packages/rangelink-vscode-extension/src/destinations/capabilities/LazyResolvedFocusCapability.ts`:
- Around line 57-64: The current focus() flow assigns this.resolutionInFlight =
this.resolve(context) but doesn’t handle exceptions from resolve(), so a
rejected resolutionInFlight remains set and subsequent focus() calls keep
awaiting the same rejected promise; update focus() to wrap the await in
try/catch/finally: await this.resolutionInFlight inside a try block, set
this.resolutionFailed = true in the catch when resolve() throws, and always
clear this.resolutionInFlight in a finally block so the in-flight promise is
cleared on both success and failure; ensure resolve() errors propagate into the
catch so resolutionFailed is set and downstream checks for this.resolutionFailed
behave as intended.
---
Nitpick comments:
In
`@packages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.ts`:
- Around line 144-168: The test custom-ai-assistant-015 should also assert that
the registration log marks the override by checking for the overridden flag;
after obtaining log lines via getLogCapture().getAllLines() and locating the
overrideRegistration/customAiRegistrations entries, add an assertion that the
relevant registration line includes the overridden flag (e.g., contains
'"overridden":true' or '"overridden": true') so the test verifies the override
is actually marked as overridden in addition to registering under
"github-copilot-chat".
🪄 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: ac967f52-e56d-41a9-9ce7-dc11ac02145c
📒 Files selected for processing (17)
packages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0-003.yamlpackages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/FocusCapabilityFactory.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/LazyResolvedFocusCapability.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/ResolvedFocusCapability.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/insertFactories/directInsertFactory.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/insertFactories/manualPasteInsertFactory.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/capabilities/resolveFocusTier.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.tspackages/rangelink-vscode-extension/src/__tests__/services/ClipboardRouter.test.tspackages/rangelink-vscode-extension/src/clipboard/ClipboardPreserver.tspackages/rangelink-vscode-extension/src/config/__tests__/parseCustomAiAssistants.test.tspackages/rangelink-vscode-extension/src/config/parseCustomAiAssistants.tspackages/rangelink-vscode-extension/src/destinations/capabilities/LazyResolvedFocusCapability.tspackages/rangelink-vscode-extension/src/utils/__tests__/interpolateArgs.test.tspackages/rangelink-vscode-extension/src/utils/interpolateArgs.tspackages/rangelink-vscode-extension/test-fixtures/dummy-ai-extension/extension.js
✅ Files skipped from review due to trivial changes (2)
- packages/rangelink-vscode-extension/src/utils/tests/interpolateArgs.test.ts
- packages/rangelink-vscode-extension/src/tests/destinations/capabilities/ResolvedFocusCapability.test.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/rangelink-vscode-extension/src/tests/services/ClipboardRouter.test.ts
- packages/rangelink-vscode-extension/src/tests/destinations/capabilities/resolveFocusTier.test.ts
- packages/rangelink-vscode-extension/src/clipboard/ClipboardPreserver.ts
- packages/rangelink-vscode-extension/src/tests/destinations/capabilities/insertFactories/manualPasteInsertFactory.test.ts
- packages/rangelink-vscode-extension/src/tests/destinations/capabilities/FocusCapabilityFactory.test.ts
- packages/rangelink-vscode-extension/src/utils/interpolateArgs.ts
- packages/rangelink-vscode-extension/src/tests/destinations/capabilities/insertFactories/directInsertFactory.test.ts
- packages/rangelink-vscode-extension/src/config/tests/parseCustomAiAssistants.test.ts
- packages/rangelink-vscode-extension/src/tests/destinations/capabilities/LazyResolvedFocusCapability.test.ts
- packages/rangelink-vscode-extension/src/config/parseCustomAiAssistants.ts
Comment on lines
+484
to
+517
| it('shouldPreserveClipboard returns true when resolvedTierLabel is not focusCommands', () => { | ||
| const context = createMockContext(); | ||
| const mockCapability = context.factories.focusCapability.createLazyResolvedCapability([], ''); | ||
| (mockCapability as any).resolvedTierLabel = 'insertCommands'; | ||
|
|
||
| const builder = createCustomAiAssistantBuilder(customConfig); | ||
| const destination = builder({ kind: customConfig.kind }, context); | ||
|
|
||
| expect(destination.shouldPreserveClipboard()).toBe(true); | ||
| }); | ||
|
|
||
| it('shouldPreserveClipboard returns false when resolvedTierLabel is focusCommands', () => { | ||
| const context = createMockContext(); | ||
| const mockCapability = context.factories.focusCapability.createLazyResolvedCapability([], ''); | ||
| (mockCapability as any).resolvedTierLabel = 'focusCommands'; | ||
|
|
||
| const builder = createCustomAiAssistantBuilder(customConfig); | ||
| const destination = builder({ kind: customConfig.kind }, context); | ||
|
|
||
| expect(destination.shouldPreserveClipboard()).toBe(false); | ||
| }); | ||
|
|
||
| it('getUserInstruction returns manual-paste message on success when resolvedTierLabel is focusCommands', () => { | ||
| const context = createMockContext(); | ||
| const mockCapability = context.factories.focusCapability.createLazyResolvedCapability([], ''); | ||
| (mockCapability as any).resolvedTierLabel = 'focusCommands'; | ||
|
|
||
| const builder = createCustomAiAssistantBuilder(customConfig); | ||
| const destination = builder({ kind: customConfig.kind }, context); | ||
|
|
||
| expect(destination.getUserInstruction(AutoPasteResult.Success)).toBe( | ||
| 'Paste (Cmd/Ctrl+V) in Spark AI to use.', | ||
| ); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how the mock factory is configured
rg -n -A 10 'createMockFocusCapabilityFactory' packages/rangelink-vscode-extension/src/__tests__/helpers/Repository: couimet/rangeLink
Length of output: 2896
🏁 Script executed:
#!/bin/bash
# Find and examine the test file to understand the context setup
cat -n packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts | sed -n '480,520p'Repository: couimet/rangeLink
Length of output: 2185
🏁 Script executed:
#!/bin/bash
# Find createMockContext definition
fd -t f 'createMockContext' packages/rangelink-vscode-extension/src/__tests__/Repository: couimet/rangeLink
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Find createCustomAiAssistantBuilder implementation
fd -t f 'destinationBuilders' packages/rangelink-vscode-extension/src/ | grep -v testRepository: couimet/rangeLink
Length of output: 136
🏁 Script executed:
#!/bin/bash
# Search for createMockContext definition more broadly
rg -l 'export.*createMockContext' packages/rangelink-vscode-extension/src/__tests__/Repository: couimet/rangeLink
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Read the destination builders implementation to see how builder uses context
cat -n packages/rangelink-vscode-extension/src/destinations/destinationBuilders.ts | head -100Repository: couimet/rangeLink
Length of output: 4882
🏁 Script executed:
#!/bin/bash
# Search for createCustomAiAssistantBuilder function definition
rg -n 'export.*createCustomAiAssistantBuilder|function createCustomAiAssistantBuilder' packages/rangelink-vscode-extension/src/destinations/Repository: couimet/rangeLink
Length of output: 188
🏁 Script executed:
#!/bin/bash
# Read the createCustomAiAssistantBuilder function
cat -n packages/rangelink-vscode-extension/src/destinations/destinationBuilders.ts | sed -n '217,280p'Repository: couimet/rangeLink
Length of output: 2898
🏁 Script executed:
#!/bin/bash
# Search for createMockContext in test helpers
rg -l 'createMockContext' packages/rangelink-vscode-extension/src/__tests__/ | head -5Repository: couimet/rangeLink
Length of output: 151
🏁 Script executed:
#!/bin/bash
# Find createMockContext implementation
rg -n 'export.*createMockContext|const createMockContext' packages/rangelink-vscode-extension/src/__tests__/ -A 2Repository: couimet/rangeLink
Length of output: 479
🏁 Script executed:
#!/bin/bash
# Look at the test file to see where createMockContext is imported from
head -50 packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts | grep -E 'import|from'Repository: couimet/rangeLink
Length of output: 326
🏁 Script executed:
#!/bin/bash
# Read the createMockContext implementation
cat -n packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts | sed -n '30,60p'Repository: couimet/rangeLink
Length of output: 1398
🏁 Script executed:
#!/bin/bash
# Read the full createMockFocusCapabilityFactory implementation
cat -n packages/rangelink-vscode-extension/src/__tests__/helpers/createMockFocusCapabilityFactory.tsRepository: couimet/rangeLink
Length of output: 1703
🏁 Script executed:
#!/bin/bash
# Let me trace through the exact flow - check if the mock returns the same instance each time
cat -n packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts | sed -n '484,505p'Repository: couimet/rangeLink
Length of output: 1265
🏁 Script executed:
#!/bin/bash
# Verify the mock factory configuration - does mockReturnValue return same instance?
# Let's look for any additional configuration or multiple test runs
rg -B 5 -A 15 'shouldPreserveClipboard returns true when resolvedTierLabel' packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.tsRepository: couimet/rangeLink
Length of output: 982
🏁 Script executed:
#!/bin/bash
# Verify the mock behavior - does mockReturnValue always return the same instance?
# Let's check by understanding how Jest's mockReturnValue works with the test setup
cat -n packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts | sed -n '484,495p'Repository: couimet/rangeLink
Length of output: 767
🏁 Script executed:
#!/bin/bash
# Let me trace: does each test get a NEW context or is it shared?
rg -B 10 'shouldPreserveClipboard returns true when resolvedTierLabel' packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts | head -20Repository: couimet/rangeLink
Length of output: 442
🏁 Script executed:
#!/bin/bash
# Now let me understand the mock return value behavior - is it the same instance each time?
# Create a simple test to verify Jest mockReturnValue behavior
python3 << 'EOF'
# Understanding Jest mockReturnValue:
# mockReturnValue(obj) sets up the mock to always return that SAME obj instance
# So if we call mockReturnValue(createMockLazyResolvedFocusCapability()),
# it creates ONE mock object and ALL calls to the mocked function return that SAME instance
print("Jest mockReturnValue behavior:")
print("mockReturnValue(obj) -> always returns THE SAME obj instance")
print()
print("In the factory setup:")
print(" mockReturnValue(createMockLazyResolvedFocusCapability())")
print(" This creates ONE mock capability instance at setup time")
print(" Every call to createLazyResolvedCapability returns that SAME instance")
EOFRepository: couimet/rangeLink
Length of output: 364
🏁 Script executed:
#!/bin/bash
# Let me verify the actual behavior by checking if Jest clears mocks between tests
cat -n packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts | sed -n '1,30p'Repository: couimet/rangeLink
Length of output: 1115
🏁 Script executed:
#!/bin/bash
# Verify my understanding - let's check if each test creates a fresh context
# and whether the mock factory setup is at describe or it block level
rg -B 20 'shouldPreserveClipboard returns true' packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts | grep -E 'describe|beforeEach|afterEach'Repository: couimet/rangeLink
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Look at the broader test structure around these specific tests
cat -n packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts | sed -n '440,495p'Repository: couimet/rangeLink
Length of output: 2701
🏁 Script executed:
#!/bin/bash
# Check if there's a beforeEach that might reset or share mocks
cat -n packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts | sed -n '400,440p'Repository: couimet/rangeLink
Length of output: 2087
🏁 Script executed:
#!/bin/bash
# Let's find the describe block and see the full structure
cat -n packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts | sed -n '355,400p'Repository: couimet/rangeLink
Length of output: 2165
🏁 Script executed:
#!/an/bin/bash
# Check how customConfig is defined
rg -n 'const customConfig' packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.tsRepository: couimet/rangeLink
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Get the describe block header and customConfig definition
cat -n packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts | sed -n '325,360p'Repository: couimet/rangeLink
Length of output: 1673
🏁 Script executed:
#!/bin/bash
# Now I need to understand the actual issue. Let me check what buildCustomAIAssistantTiers returns
# and how it's used in the real LazyResolvedFocusCapability
rg -n 'buildCustomAIAssistantTiers' packages/rangelink-vscode-extension/src/destinations/Repository: couimet/rangeLink
Length of output: 575
🏁 Script executed:
#!/bin/bash
# Look at LazyResolvedFocusCapability to see how resolvedTierLabel is determined
fd -t f 'LazyResolvedFocusCapability' packages/rangelink-vscode-extension/src/Repository: couimet/rangeLink
Length of output: 269
🏁 Script executed:
#!/bin/bash
# Look at LazyResolvedFocusCapability implementation
cat -n packages/rangelink-vscode-extension/src/destinations/capabilities/LazyResolvedFocusCapability.ts | head -80Repository: couimet/rangeLink
Length of output: 3407
🏁 Script executed:
#!/bin/bash
# Look at buildCustomAIAssistantTiers to understand what tiers are being built
cat -n packages/rangelink-vscode-extension/src/destinations/capabilities/FocusCapabilityFactory.ts | sed -n '70,120p'Repository: couimet/rangeLink
Length of output: 2127
🏁 Script executed:
#!/bin/bash
# Now let me verify my understanding by checking if the mock factory creates a fresh instance each test
# or if it's shared across tests
cat -n packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts | sed -n '484,520p'Repository: couimet/rangeLink
Length of output: 2081
🏁 Script executed:
#!/bin/bash
# Let me trace through more carefully - each test gets createMockContext() which calls createMockFocusCapabilityFactory()
# Does that create a new factory or return a shared one?
cat -n packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts | sed -n '30,40p'Repository: couimet/rangeLink
Length of output: 512
🏁 Script executed:
#!/bin/bash
# Check if buildCustomAIAssistantTiers actually affects what the tiers are in the test config
cat -n packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts | sed -n '335,345p'Repository: couimet/rangeLink
Length of output: 607
Test setup correctly reuses mock capability but masks actual tier resolution logic.
The mock capability returned by the factory IS reused across calls within a single test (since mockReturnValue returns the same instance), so the manual resolvedTierLabel assignment does propagate to what the builder uses. However, the test is fragile because it:
- Bypasses tier building: The mock
buildCustomAIAssistantTiersreturns[], so the actual tier-building logic fromFocusCapabilityFactory.buildCustomAIAssistantTiers(config)never runs. The test manually setsresolvedTierLabelinstead of letting the realLazyResolvedFocusCapabilityresolve it. - Relies on mock reuse assumption: The test depends on Jest's
mockReturnValuebehavior returning the same instance across multiple calls, which is implementation-specific.
Consider testing the actual tier resolution path: either mock only the dependencies (like ideAdapter.getCommands()) and let the real LazyResolvedFocusCapability resolve tiers, or explicitly test that the builder correctly passes tiers to the capability factory.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts`
around lines 484 - 517, The tests for createCustomAiAssistantBuilder are fragile
because they stub FocusCapabilityFactory to return a single reused
LazyResolvedFocusCapability instance and then mutate its resolvedTierLabel,
which bypasses the real buildCustomAIAssistantTiers logic and relies on
mockReturnValue reuse; instead update the tests to exercise the actual tier
resolution: stop stubbing buildCustomAIAssistantTiers to return [] (or replace
mockReturnValue with mockImplementation that returns a fresh
LazyResolvedFocusCapability per call), or mock only external dependencies used
inside LazyResolvedFocusCapability (e.g., ideAdapter.getCommands()) so the real
LazyResolvedFocusCapability runs and produces resolvedTierLabel, then assert
shouldPreserveClipboard and getUserInstruction based on the real resolution flow
in createCustomAiAssistantBuilder / FocusCapabilityFactory /
LazyResolvedFocusCapability.
Comment on lines
+57
to
+64
| if (!this.resolved) { | ||
| if (this.resolutionInFlight) { | ||
| await this.resolutionInFlight; | ||
| } else { | ||
| this.resolutionInFlight = this.resolve(context); | ||
| await this.resolutionInFlight; | ||
| this.resolutionInFlight = undefined; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== LazyResolvedFocusCapability focus()/resolve() paths =="
rg -n -C4 'resolutionInFlight|this\.resolve\(|await this\.ideAdapter\.getCommands\(' \
packages/rangelink-vscode-extension/src/destinations/capabilities/LazyResolvedFocusCapability.ts
echo
echo "== VscodeAdapter.getCommands implementation =="
fd -a 'VscodeAdapter.ts' packages/rangelink-vscode-extension/src \
| xargs -I{} rg -n -C6 '\bgetCommands\s*\(' {}Repository: couimet/rangeLink
Length of output: 1945
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Full focus() method body =="
sed -n '50,72p' packages/rangelink-vscode-extension/src/destinations/capabilities/LazyResolvedFocusCapability.ts
echo
echo "== Full resolve() method body =="
sed -n '74,115p' packages/rangelink-vscode-extension/src/destinations/capabilities/LazyResolvedFocusCapability.ts
echo
echo "== resolveFocusTier function signature and error handling =="
rg -n -A20 'function resolveFocusTier|export.*resolveFocusTier' packages/rangelink-vscode-extension/src --type ts | head -40Repository: couimet/rangeLink
Length of output: 4433
Add try-catch in resolve() and use finally() to clear in-flight promise on both success and failure.
If getCommands() rejects on line 75, resolve() throws without catching. The promise assigned to resolutionInFlight stays rejected, resolutionInFlight = undefined never executes, and subsequent focus() calls re-await the same rejected promise instead of returning Result.err(...) via the second resolutionFailed check.
Wrap resolve() in try-catch to set resolutionFailed = true on exception, and use .finally() in focus() to guarantee resolutionInFlight cleanup on rejection.
Proposed fix
async focus(context: LoggingContext): Promise<FocusResult> {
if (this.resolutionFailed) {
return Result.err({ reason: FocusErrorReason.COMMAND_FOCUS_FAILED });
}
if (!this.resolved) {
- if (this.resolutionInFlight) {
- await this.resolutionInFlight;
- } else {
- this.resolutionInFlight = this.resolve(context);
- await this.resolutionInFlight;
- this.resolutionInFlight = undefined;
- }
+ this.resolutionInFlight ??= this.resolve(context).finally(() => {
+ this.resolutionInFlight = undefined;
+ });
+ await this.resolutionInFlight;
}
if (this.resolutionFailed) {
return Result.err({ reason: FocusErrorReason.COMMAND_FOCUS_FAILED });
}
return this.resolved!.focus(context);
}
private async resolve(context: LoggingContext): Promise<void> {
- const registeredCommands = await this.ideAdapter.getCommands();
- const result = resolveFocusTier(
- this.tiers,
- registeredCommands,
- this.logger,
- this.logPrefix,
- this.fallbackTierIndex,
- );
+ try {
+ const registeredCommands = await this.ideAdapter.getCommands();
+ const result = resolveFocusTier(
+ this.tiers,
+ registeredCommands,
+ this.logger,
+ this.logPrefix,
+ this.fallbackTierIndex,
+ );
- if (!result) {
- this.resolutionFailed = true;
- this.logger.warn(
- { ...context, logPrefix: this.logPrefix },
- `${this.logPrefix}: tier resolution failed — no commands registered`,
- );
- return;
- }
+ if (!result) {
+ this.resolutionFailed = true;
+ this.logger.warn(
+ { ...context, logPrefix: this.logPrefix },
+ `${this.logPrefix}: tier resolution failed — no commands registered`,
+ );
+ return;
+ }
- this.resolutionResult = result;
+ this.resolutionResult = result;
- if (result.isFallback) {
- this.logger.warn(
- {
- ...context,
- tier: result.resolvedTier.label,
- commands: result.resolvedTier.commands,
- logPrefix: this.logPrefix,
- },
- `${this.logPrefix}: custom commands not registered, falling back to built-in commands`,
- );
- } else {
- this.logger.info(
- {
- ...context,
- tier: result.resolvedTier.label,
- logPrefix: this.logPrefix,
- },
- `${this.logPrefix}: resolved to ${result.resolvedTier.label}`,
- );
- }
+ if (result.isFallback) {
+ this.logger.warn(
+ {
+ ...context,
+ tier: result.resolvedTier.label,
+ commands: result.resolvedTier.commands,
+ logPrefix: this.logPrefix,
+ },
+ `${this.logPrefix}: custom commands not registered, falling back to built-in commands`,
+ );
+ } else {
+ this.logger.info(
+ {
+ ...context,
+ tier: result.resolvedTier.label,
+ logPrefix: this.logPrefix,
+ },
+ `${this.logPrefix}: resolved to ${result.resolvedTier.label}`,
+ );
+ }
- this.resolved = new ResolvedFocusCapability(
- this.ideAdapter,
- result.resolvedTier,
- this.logger,
- );
+ this.resolved = new ResolvedFocusCapability(
+ this.ideAdapter,
+ result.resolvedTier,
+ this.logger,
+ );
+ } catch (error) {
+ this.resolutionFailed = true;
+ this.logger.warn(
+ { ...context, logPrefix: this.logPrefix, error },
+ `${this.logPrefix}: tier resolution failed — getCommands threw`,
+ );
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/rangelink-vscode-extension/src/destinations/capabilities/LazyResolvedFocusCapability.ts`
around lines 57 - 64, The current focus() flow assigns this.resolutionInFlight =
this.resolve(context) but doesn’t handle exceptions from resolve(), so a
rejected resolutionInFlight remains set and subsequent focus() calls keep
awaiting the same rejected promise; update focus() to wrap the await in
try/catch/finally: await this.resolutionInFlight inside a try block, set
this.resolutionFailed = true in the catch when resolve() throws, and always
clear this.resolutionInFlight in a finally block so the in-flight promise is
cleared on both success and failure; ensure resolve() errors propagate into the
catch so resolutionFailed is set and downstream checks for this.resolutionFailed
behave as intended.
added 2 commits
April 15, 2026 22:54
…usCapability If getCommands() threw during resolution, the resolutionInFlight promise stayed set as a rejected promise and was never cleared. Subsequent focus() calls would await the same rejected promise indefinitely instead of retrying or returning the clean COMMAND_FOCUS_FAILED error. Wrapped the await in try/finally so resolutionInFlight is always cleared regardless of success or failure. Ignored Feedback: - Assert overridden flag in integration test registration log: the overridden flag lives in loggingDetails on the destination, not in the DestinationRegistry.register() log — the assertion would always fail. Already verified by unit test via destination.getLoggingDetails(). - Fragile shouldPreserveClipboard tests: intentional testing boundary — destinationBuilders tests mock the factory layer to isolate builder orchestration. Real tier resolution is tested in LazyResolvedFocusCapability.test.ts. Ref: #502 (review)
This was referenced Apr 30, 2026
4 tasks
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
Users can connect RangeLink to any AI tool that has a VS Code extension. Each custom assistant declares up to three command tiers in
settings.json— the highest-priority tier with registered commands wins at bind time:insertCommands— direct text delivery viaexecuteCommandargs (no clipboard)focusAndPasteCommands— focus panel + automatic clipboard pastefocusCommands— focus panel + manual paste by user (link stays on clipboard)Tier resolution is lazy (first
focus()call), cached for all subsequent uses. Built-in assistants (Cursor AI, Claude Code, GitHub Copilot Chat) can be overridden with custom config while retaining their availability detection and i18n messages.Changes
Type system —
DestinationKindacceptscustom-ai:${string}template literal types alongside the 5 built-in kinds.isCustomAiAssistantKind()andisAnyAiAssistantKind()type guards.FocusTierLabelunion type for type-safe tier comparisons.Settings schema —
rangelink.customAiAssistantsarray inpackage.jsonwith JSON validation. Each entry specifiesextensionId,extensionName, and one or more command arrays.${content}template interpolation supported in Tier 1 args.Parser —
parseCustomAiAssistants()validates entries at activation — missing fields, empty strings, empty arrays, and duplicate IDs logged as warnings and skipped.Three-tier focus resolution —
LazyResolvedFocusCapability+resolveFocusTierreplace the earlierTieredFocusCapability.FocusTierProbeMode('execute' | 'none') controls whether resolution triggersexecuteCommand.FocusCapabilityFactoryexposes composable steps:buildCustomAIAssistantTiers(),buildBuiltinFallbackTier(),createLazyResolvedCapability().Data-driven built-in assistants —
BUILTIN_AI_ASSISTANTSlookup table replaces three near-identical builder functions with a single generic builder.Built-in override — When a custom assistant's
extensionIdmatches a built-in,createOverriddenBuiltinBuildermerges user-defined tiers with the built-in's hardcoded focus commands as a fallback tier.Clipboard preservation —
ClipboardPreserver.preserve()accepts ashouldRestorecallback evaluated after the paste operation completes. Tier 3 destinations signal viashouldPreserveClipboard()that the link must stay on the clipboard for manual paste. Tier 1 and Tier 2 destinations are unaffected.Destination picker —
DestinationAvailabilityService.getGroupedDestinationItems()iterates all registered kinds viaregistry.getSupportedKinds()instead of a hardcoded array.i18n —
STATUS_BAR_JUMP_SUCCESS_CUSTOM_AI,INFO_CUSTOM_AI_USER_INSTRUCTIONS,ERROR_CUSTOM_AI_NOT_AVAILABLEwith{extensionName}placeholders.Dummy AI test extension — Webview sidebar panel with two textareas (tier1 for
insertText, tier2 for manual paste). Readiness handshake ensures cold-start reliability. Commands:insertText,insertWithArgs,focusForPaste,focusPanel,getText,clearAll.ADR-0002 — Architecture decision record for the three-tier command schema. Documents alternatives considered, confidence levels (all HIGH except
${content}template at MEDIUM), and the built-in override design.Test Plan
test:release:automated— 125 passing including 9 custom AI TCs and 2 built-in override TCstest:release:grep 'custom-ai-assistant-01[0-7]'— 8 assisted TCs pass with human interaction (Tier 1 direct insert, clipboard isolation, Tier 3 manual paste with Cmd+V verification, Tier 2→3 fallback with manual paste,${content}template, cold-start panel auto-initialization)Documentation
${content}template, built-in overrideRelated
Summary by CodeRabbit
New Features
Documentation