[issues/594] Refactor QuickPick item shape: single source of truth via nested info objects#596
Merged
Conversation
…a nested info objects
## Summary
Consolidates the `EligibleTerminal` / `EligibleFile` domain types so the live `vscode.Terminal` / `vscode.Uri` references live exclusively on `bindOptions`, and drops the top-level `isActive` / `boundState` duplication from `TerminalBindableQuickPickItem` / `FileBindableQuickPickItem`. The log enrichment that used to require those top-level fields moves behind a `RANGELINK_CAPTURE_LOGS=true` env gate, matching the existing `LogCapture` / `markRangeLinkTestFixture` pattern. Pure refactor: no behavior change, integration-test log contract preserved.
## Changes
- `EligibleTerminal.terminal: vscode.Terminal` → `EligibleTerminal.bindOptions: TerminalBindOptions`. Same shape for `EligibleFile`: `uri: vscode.Uri` → `bindOptions: TextEditorBindOptions`. The live VS Code reference is now encapsulated as a bind intent from the moment a candidate is identified.
- `BindableQuickPickItem` base no longer carries `readonly isActive?`. `TerminalBindableQuickPickItem` no longer carries `readonly boundState?`. `FileBindableQuickPickItem` no longer carries `readonly boundState?`. All three fields had zero production consumers — they existed solely to feed `VscodeAdapter.showQuickPick`'s log projection.
- `VscodeAdapter.showQuickPick` items projection split into prod path (base fields only: `label`, `description`, `detail`, `kind`, `itemKind`, `displayName`, `remainingCount`) and a `process.env.RANGELINK_CAPTURE_LOGS === 'true'` gate that additionally projects `isActive` / `boundState` flat by digging into `terminalInfo` / `fileInfo`. Integration tests that destructure `{ isActive, boundState }` from the log line need no changes.
- New `projectTestStatusFields` helper in `VscodeAdapter.ts`, exported for direct unit testing. Five new unit tests cover its terminalInfo / fileInfo / absent / both-present cases. Two additional `showQuickPick` log-projection tests cover both env-gated branches end-to-end: one for the unset/prod path (base fields only, even when `terminalInfo` is present on the item), and one for `RANGELINK_CAPTURE_LOGS='true'` using `jest.resetModules()` + `await import()` + `try/finally` env restoration to reload the adapter with the new env value. The env var name stays as a literal in the test per T003 — it's an external contract.
- Call sites that used to construct `bindOptions` inline now read `eligible.bindOptions` — see `DestinationAvailabilityService.buildTerminalItem`, `buildFileItem`, `DestinationPicker.showSecondaryFilePicker`, `showSecondaryTerminalPicker`, `BindToTerminalCommand.execute`, `BindToTextEditorCommand.executeWithPicker`, `RangeLinkStatusBar.showSecondaryTerminalPicker`, and the file overflow picker. The picker item's top-level `bindOptions` is now the SAME object reference as `terminalInfo.bindOptions` — single source.
- Both `TODO(#594)` comments removed: one in `QuickPickTypes.ts`, one in `VscodeAdapter.ts`.
- New `src/constants/envVarNames.ts` centralizes the two production env-var names (`ENV_RANGELINK_CAPTURE_LOGS`, `ENV_RANGELINK_TEST_FIXTURES_ENABLED`). Production readers (`LogCapture.ts`, `VscodeAdapter.ts`, `testFixtureRegistry.ts`) import from the constant; error messages template-interpolate the name so a future rename stays in one place. The `classifyTerminalForBinding` test keeps the literal env-var name per T003 — the test acts as a contract tripwire: a rename in the constants file doesn't silently propagate to the test, so the prod-vs-test mismatch surfaces loudly. `.vscode-test.base.mjs` also keeps its literal (it runs at vscode-test config-load time, outside the TS module graph; touching it would entangle build order).
- `test:release:automated` in `packages/rangelink-vscode-extension/package.json` now bakes in `--exclude-label requires-extensions --exclude-label cursor`, matching CI. The redundant flags removed from `.github/actions/run-integration-tests/action.yml`. Local devs no longer wheel-spin on tests CI was already skipping.
- Removed 20 unnecessary `as const` assertions across files this PR touched: `getEligibleTerminals.ts`, `getEligibleFiles.ts`, `buildTerminalPickerItems.ts`, `markBoundFile.ts`, `markBoundTerminal.ts`, `DestinationPicker.ts`, `RangeLinkStatusBar.ts`. The annotated return types and surrounding contextual types already narrow the literals — the assertions were noise. Out-of-scope `as const` sites (`ListBookmarksCommand.ts`, `PasteDestinationManager.ts`, `FocusCapability.ts`) left for a follow-up.
## Key Discoveries
- `EligibleFile.viewColumn` is intentionally kept even though it duplicates `bindOptions.viewColumn`. It's plain numeric metadata read by `sortEligibleFiles`, `buildFilePickerItems` (grouping), and `buildDestinationQuickPickItems` (group label) — forcing those three call sites through `bindOptions.viewColumn` for one number is uglier than the redundancy. The smell that motivates removing `terminal` / `uri` (live VS Code refs in domain types) does not apply to a plain integer.
- The issue body listed `nonBindableReason` as a top-level field on `TerminalBindableQuickPickItem` to drop. It was never there — that field only lives in `classifyTerminalForBinding`'s discriminated-union return type. The "drop top-level `nonBindableReason`" cleanup was a no-op.
- The two clipboard-preservation tests for Cursor AI (`clipboard-preservation-013`, `clipboard-preservation-014`) are pre-existing failures on `main` — verified by stashing and re-running. The CI workflow was already excluding them via `--exclude-label cursor`. This PR pulls that exclusion into the script entry so local runs match CI without manual flag-passing.
## Test Plan
- [x] All 1947 unit tests pass (`pnpm test:fast`)
- [x] All 142 automated integration tests pass (`pnpm test:release:automated`, ~7 minutes)
- [x] `pnpm compile` clean
- [x] `pnpm fix` clean
- [x] Sweep grep confirmed zero stragglers — no remaining `.terminal` reads on a typed `EligibleTerminal`, no remaining `.uri` reads on a typed `EligibleFile`, no remaining `item.isActive` / `item.boundState` reads on a typed `BindableQuickPickItem`.
- [x] Integration-test log contract preserved: the prod log no longer emits flat `isActive` / `boundState`, but the env-gated test mode does, sourced from `terminalInfo` / `fileInfo`. Integration tests destructuring `{ isActive, boundState }` from `VscodeAdapter.showQuickPick` log lines pass unchanged.
## Related
- Closes #594
- Born from #592 (merged via #595, which left two `TODO(#594)` markers for this cleanup)
Contributor
WalkthroughRefactors quick-pick and eligible-item shapes to embed binding references in nested ChangesQuickPick shape refactor
Sequence Diagram(s) sequenceDiagram
participant GetEligible as getEligibleFiles/getEligibleTerminals
participant Service as DestinationAvailabilityService
participant Picker as DestinationPicker
participant Command as BindCommand
participant Manager as destinationManager.bind
GetEligible->>Service: return items with bindOptions
Service->>Picker: present quick-pick items (fileInfo/terminalInfo with bindOptions)
Picker->>Command: selected.bindOptions returned
Command->>Manager: pass bindOptions to bind()
Estimated code review effort: Possibly related PRs:
Poem:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 modifies the behavior of the LogCapture class, specifically changing the environment variable used for log capturing, which affects error messages. Additionally, there are changes in how binding options are handled in the BindToTextEditorCommand tests, which may impact user experience. Suggested test cases:
Generated by QA Gap Check (GPT-4o-mini via GitHub Models) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Owner
Author
|
@coderabbitai review -- you got rate-limited in #596 (comment) |
Contributor
|
✅ Actions performedFull review triggered. |
Owner
Author
|
@coderabbitai Did you really perform a full review on my PR ? I'm surprised you didn't identify anything across 35 files ? |
Contributor
|
I'm re-triggering the full review now with that context noted. (╯°□°）╯ ← rabbit dusting off its review notes Re-triggering due to prior rate-limited incomplete review on a 35-file refactor PR; please ensure thorough coverage of domain type changes ( ✅ Actions performedFull review triggered. |
Owner
Author
|
@coderabbitai I saw your reply in #596 (comment), but #596 (comment) still states I've been rate-limited. Does this comment from your last attempt of triggering a full review ? |
Contributor
🧩 Analysis chain🏁 Script executed: Length of output: 1341
More critically: the PR has zero posted reviews right now — the review pipeline never completed despite the two `` triggers. I'm re-triggering it once more. Apologies for the confusion caused by the auto-acknowledgement messages making it look like work was done when it wasn't. (╯°□°）╯ ← rabbit untangling the pipeline Re-triggering due to two prior failed/rate-limited full review attempts on a 35-file refactor PR ( ✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
packages/rangelink-vscode-extension/src/__tests__/ide/vscode/VscodeAdapter.test.ts (1)
635-641:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid asserting logger parameters via
.mock.callsextraction.Please assert the logger payload directly with
toHaveBeenCalledWith(...)instead of pulling args frommock.callsand inspecting keys.Suggested update
- const loggedItems = (mockLogger.debug as jest.Mock).mock.calls.find( - (call) => call[0]?.fn === 'VscodeAdapter.showQuickPick', - )?.[0]?.items; - - expect(Object.keys(loggedItems[0])).toStrictEqual(['label']); - expect(Object.keys(loggedItems[1]).sort()).toStrictEqual(['displayName', 'label']); - expect(Object.keys(loggedItems[2]).sort()).toStrictEqual(['itemKind', 'label']); + expect(mockLogger.debug).toHaveBeenCalledWith( + { + fn: 'VscodeAdapter.showQuickPick', + itemCount: 3, + options: undefined, + items: [ + { label: 'Plain item' }, + { label: 'With displayName only', displayName: 'raw name' }, + { label: 'With itemKind only', itemKind: 'info' }, + ], + }, + 'Showing quick pick', + );As per coding guidelines, "Use
.toHaveBeenCalledWith(param1, param2, ...)to verify mock call parameters - never access.mock.calls[0]to extract and assert parameters separately".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rangelink-vscode-extension/src/__tests__/ide/vscode/VscodeAdapter.test.ts` around lines 635 - 641, Replace the manual extraction of logger arguments from (mockLogger.debug as jest.Mock).mock.calls and the subsequent key assertions by directly asserting that mockLogger.debug was called with the expected payload for the VscodeAdapter.showQuickPick call; locate the test that computes loggedItems (reference: mockLogger.debug and VscodeAdapter.showQuickPick in VscodeAdapter.test.ts) and replace the Object.keys(...) assertions with a single toHaveBeenCalledWith(...) (or multiple toHaveBeenCalledWith calls if order matters) that verifies the exact logger payload shape and values passed to mockLogger.debug for that call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@packages/rangelink-vscode-extension/src/__tests__/ide/vscode/VscodeAdapter.test.ts`:
- Around line 635-641: Replace the manual extraction of logger arguments from
(mockLogger.debug as jest.Mock).mock.calls and the subsequent key assertions by
directly asserting that mockLogger.debug was called with the expected payload
for the VscodeAdapter.showQuickPick call; locate the test that computes
loggedItems (reference: mockLogger.debug and VscodeAdapter.showQuickPick in
VscodeAdapter.test.ts) and replace the Object.keys(...) assertions with a single
toHaveBeenCalledWith(...) (or multiple toHaveBeenCalledWith calls if order
matters) that verifies the exact logger payload shape and values passed to
mockLogger.debug for that call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 41d6ee52-5f6a-4c92-8d65-cfe633877b32
📒 Files selected for processing (35)
.github/actions/run-integration-tests/action.ymlpackages/rangelink-vscode-extension/package.jsonpackages/rangelink-vscode-extension/src/LogCapture.tspackages/rangelink-vscode-extension/src/__tests__/commands/BindToTextEditorCommand.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/DestinationAvailabilityService.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/DestinationPicker.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/utils/buildDestinationQuickPickItems.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/utils/buildTerminalPickerItems.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/utils/getEligibleFiles.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/utils/getEligibleTerminals.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/utils/markBoundFile.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/utils/markBoundTerminal.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/utils/showTerminalPicker.test.tspackages/rangelink-vscode-extension/src/__tests__/helpers/createMockBindableQuickPickItem.tspackages/rangelink-vscode-extension/src/__tests__/helpers/createMockEligibleFile.tspackages/rangelink-vscode-extension/src/__tests__/helpers/createMockEligibleTerminal.tspackages/rangelink-vscode-extension/src/__tests__/ide/vscode/VscodeAdapter.test.tspackages/rangelink-vscode-extension/src/__tests__/statusBar/RangeLinkStatusBar.test.tspackages/rangelink-vscode-extension/src/commands/BindToTerminalCommand.tspackages/rangelink-vscode-extension/src/commands/BindToTextEditorCommand.tspackages/rangelink-vscode-extension/src/constants/envVarNames.tspackages/rangelink-vscode-extension/src/constants/index.tspackages/rangelink-vscode-extension/src/destinations/DestinationAvailabilityService.tspackages/rangelink-vscode-extension/src/destinations/DestinationPicker.tspackages/rangelink-vscode-extension/src/destinations/utils/buildTerminalPickerItems.tspackages/rangelink-vscode-extension/src/destinations/utils/getEligibleFiles.tspackages/rangelink-vscode-extension/src/destinations/utils/getEligibleTerminals.tspackages/rangelink-vscode-extension/src/destinations/utils/markBoundFile.tspackages/rangelink-vscode-extension/src/destinations/utils/markBoundTerminal.tspackages/rangelink-vscode-extension/src/destinations/utils/testFixtureRegistry.tspackages/rangelink-vscode-extension/src/ide/vscode/VscodeAdapter.tspackages/rangelink-vscode-extension/src/statusBar/RangeLinkStatusBar.tspackages/rangelink-vscode-extension/src/types/EligibleFile.tspackages/rangelink-vscode-extension/src/types/EligibleTerminal.tspackages/rangelink-vscode-extension/src/types/QuickPickTypes.ts
💤 Files with no reviewable changes (1)
- packages/rangelink-vscode-extension/src/types/QuickPickTypes.ts
Three test files used .mock.calls extraction to pull out mock arguments for assertion, violating T006. CodeRabbit flagged the VscodeAdapter.test.ts instance; the sweep caught matching patterns in destinationBuilders.test.ts and PasteDestinationManager.test.ts. Ignored Feedback: - destinationBuilders.test.ts (4 sites): .mock.calls[0][1] extracts callback functions for invocation, not for assertion — necessary when testing factory-returned callbacks - wireActiveTerminalBindabilityContext.test.ts:114: extracts onDidChangeActiveTerminal listener to fire it for event simulation - PasteDestinationManager.test.ts:196-199: extracts terminal/document close listeners for event simulation Ref: #596 (review)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts`:
- Around line 792-794: Tests are directly reading
createCapabilityMock.mock.calls[0][1] (and similar indices) to extract callback
args (e.g., getColdRefocusArg); change the mock to capture the callback into a
scoped variable inside the createCapabilityMock implementation, then assert the
mock was called with expected parameters using
expect(createCapabilityMock).toHaveBeenCalledWith(...) and invoke/assert the
captured callback variable rather than reading mock.calls[0]. Replace every
direct usage of createCapabilityMock.mock.calls[...] in
destinationBuilders.test.ts with this pattern for createCapabilityMock and any
extracted variables like getColdRefocusArg.
🪄 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: d2d763b5-6d93-422a-82a0-b5d1339d4d01
📒 Files selected for processing (3)
packages/rangelink-vscode-extension/src/__tests__/destinations/PasteDestinationManager.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.tspackages/rangelink-vscode-extension/src/__tests__/ide/vscode/VscodeAdapter.test.ts
Comment on lines
+792
to
+794
| const getColdRefocusArg = createCapabilityMock.mock.calls[0][1] as ( | ||
| ...args: unknown[] | ||
| ) => unknown; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify direct first-call argument extraction is removed from this test file.
rg -nP --type=ts '\.mock\.calls\[0\]\[1\]' packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.tsRepository: couimet/rangeLink
Length of output: 219
🏁 Script executed:
#!/bin/bash
# Find any remaining first-call access patterns in this test file.
rg -nP --type=ts '\.mock\.calls\[0\](\[[0-9]+\])?' packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts
# Also catch any other `.mock.calls[0]` access not matching the simple numeric index form.
rg -n --type=ts '\.mock\.calls\[0\]' packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.tsRepository: couimet/rangeLink
Length of output: 977
Stop reading callback arguments via .mock.calls[0] in destinationBuilders tests
- packages/rangelink-vscode-extension/src/tests/destinations/destinationBuilders.test.ts still extracts callback args from
createCapabilityMock.mock.calls[0]at lines 792-794, 827, 849, 876-878, 901, and 923. - Capture the callback in a scoped variable inside the
createCapabilityMockmock implementation, then assert/invoke that variable; useexpect(...).toHaveBeenCalledWith(...)for verifying mock call parameters instead of readingmock.calls[0].
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.ts`
around lines 792 - 794, Tests are directly reading
createCapabilityMock.mock.calls[0][1] (and similar indices) to extract callback
args (e.g., getColdRefocusArg); change the mock to capture the callback into a
scoped variable inside the createCapabilityMock implementation, then assert the
mock was called with expected parameters using
expect(createCapabilityMock).toHaveBeenCalledWith(...) and invoke/assert the
captured callback variable rather than reading mock.calls[0]. Replace every
direct usage of createCapabilityMock.mock.calls[...] in
destinationBuilders.test.ts with this pattern for createCapabilityMock and any
extracted variables like getColdRefocusArg.
✅ CI / Integration Tests (with extensions) — run summary
|
✅ CI / Test & Validate — run summary
|
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
Consolidates the
EligibleTerminal/EligibleFiledomain types so the livevscode.Terminal/vscode.Urireferences live exclusively onbindOptions, and drops the top-levelisActive/boundStateduplication fromTerminalBindableQuickPickItem/FileBindableQuickPickItem. The log enrichment that used to require those top-level fields moves behind aRANGELINK_CAPTURE_LOGS=trueenv gate, matching the existingLogCapture/markRangeLinkTestFixturepattern. Pure refactor: no behavior change, integration-test log contract preserved.Changes
EligibleTerminal.terminal: vscode.Terminal→EligibleTerminal.bindOptions: TerminalBindOptions. Same shape forEligibleFile:uri: vscode.Uri→bindOptions: TextEditorBindOptions. The live VS Code reference is now encapsulated as a bind intent from the moment a candidate is identified.BindableQuickPickItembase no longer carriesreadonly isActive?.TerminalBindableQuickPickItemno longer carriesreadonly boundState?.FileBindableQuickPickItemno longer carriesreadonly boundState?. All three fields had zero production consumers — they existed solely to feedVscodeAdapter.showQuickPick's log projection.VscodeAdapter.showQuickPickitems projection split into prod path (base fields only:label,description,detail,kind,itemKind,displayName,remainingCount) and aprocess.env.RANGELINK_CAPTURE_LOGS === 'true'gate that additionally projectsisActive/boundStateflat by digging intoterminalInfo/fileInfo. Integration tests that destructure{ isActive, boundState }from the log line need no changes.projectTestStatusFieldshelper inVscodeAdapter.ts, exported for direct unit testing. Five new unit tests cover its terminalInfo / fileInfo / absent / both-present cases. Two additionalshowQuickPicklog-projection tests cover both env-gated branches end-to-end: one for the unset/prod path (base fields only, even whenterminalInfois present on the item), and one forRANGELINK_CAPTURE_LOGS='true'usingjest.resetModules()+await import()+try/finallyenv restoration to reload the adapter with the new env value. The env var name stays as a literal in the test per T003 — it's an external contract.bindOptionsinline now readeligible.bindOptions— seeDestinationAvailabilityService.buildTerminalItem,buildFileItem,DestinationPicker.showSecondaryFilePicker,showSecondaryTerminalPicker,BindToTerminalCommand.execute,BindToTextEditorCommand.executeWithPicker,RangeLinkStatusBar.showSecondaryTerminalPicker, and the file overflow picker. The picker item's top-levelbindOptionsis now the SAME object reference asterminalInfo.bindOptions— single source.TODO(#594)comments removed: one inQuickPickTypes.ts, one inVscodeAdapter.ts.src/constants/envVarNames.tscentralizes the two production env-var names (ENV_RANGELINK_CAPTURE_LOGS,ENV_RANGELINK_TEST_FIXTURES_ENABLED). Production readers (LogCapture.ts,VscodeAdapter.ts,testFixtureRegistry.ts) import from the constant; error messages template-interpolate the name so a future rename stays in one place. TheclassifyTerminalForBindingtest keeps the literal env-var name per T003 — the test acts as a contract tripwire: a rename in the constants file doesn't silently propagate to the test, so the prod-vs-test mismatch surfaces loudly..vscode-test.base.mjsalso keeps its literal (it runs at vscode-test config-load time, outside the TS module graph; touching it would entangle build order).test:release:automatedinpackages/rangelink-vscode-extension/package.jsonnow bakes in--exclude-label requires-extensions --exclude-label cursor, matching CI. The redundant flags removed from.github/actions/run-integration-tests/action.yml. Local devs no longer wheel-spin on tests CI was already skipping.as constassertions across files this PR touched:getEligibleTerminals.ts,getEligibleFiles.ts,buildTerminalPickerItems.ts,markBoundFile.ts,markBoundTerminal.ts,DestinationPicker.ts,RangeLinkStatusBar.ts. The annotated return types and surrounding contextual types already narrow the literals — the assertions were noise. Out-of-scopeas constsites (ListBookmarksCommand.ts,PasteDestinationManager.ts,FocusCapability.ts) left for a follow-up.Key Discoveries
EligibleFile.viewColumnis intentionally kept even though it duplicatesbindOptions.viewColumn. It's plain numeric metadata read bysortEligibleFiles,buildFilePickerItems(grouping), andbuildDestinationQuickPickItems(group label) — forcing those three call sites throughbindOptions.viewColumnfor one number is uglier than the redundancy. The smell that motivates removingterminal/uri(live VS Code refs in domain types) does not apply to a plain integer.nonBindableReasonas a top-level field onTerminalBindableQuickPickItemto drop. It was never there — that field only lives inclassifyTerminalForBinding's discriminated-union return type. The "drop top-levelnonBindableReason" cleanup was a no-op.clipboard-preservation-013,clipboard-preservation-014) are pre-existing failures onmain— verified by stashing and re-running. The CI workflow was already excluding them via--exclude-label cursor. This PR pulls that exclusion into the script entry so local runs match CI without manual flag-passing.Test Plan
pnpm test:fast)pnpm test:release:automated, ~7 minutes)pnpm compilecleanpnpm fixclean.terminalreads on a typedEligibleTerminal, no remaining.urireads on a typedEligibleFile, no remainingitem.isActive/item.boundStatereads on a typedBindableQuickPickItem.isActive/boundState, but the env-gated test mode does, sourced fromterminalInfo/fileInfo. Integration tests destructuring{ isActive, boundState }fromVscodeAdapter.showQuickPicklog lines pass unchanged.Related
TODO(#594)markers for this cleanup)Summary by CodeRabbit
Refactor
Tests
Chores