-
Notifications
You must be signed in to change notification settings - Fork 0
[issues/552] Claude Code cold-start re-focus loop #567
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
21fa540
2141ae0
e0247f7
32ea178
c72339b
c021500
dab7873
382e3c2
191d501
f0d5077
fa7a16c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,15 +7,12 @@ import { | |
| CMD_BIND_TO_CLAUDE_CODE, | ||
| CMD_BIND_TO_DESTINATION, | ||
| CMD_COPY_LINK_RELATIVE, | ||
| CMD_UNBIND_DESTINATION, | ||
| CMD_JUMP_TO_DESTINATION, | ||
| } from '../../constants/commandIds'; | ||
| import { CLAUDE_CODE_EXTENSION_ID } from '../../utils/aiAssistants/isClaudeCodeAvailable'; | ||
| import { | ||
| activateExtension, | ||
| assertStatusBarMsgLogged, | ||
| cleanupFiles, | ||
| closeAllEditors, | ||
| createLogger, | ||
| createWorkspaceFile, | ||
| extractQuickPickItemsLogged, | ||
| getLogCapture, | ||
|
|
@@ -30,17 +27,10 @@ const AI_ASSISTANTS_GROUP_LABEL = 'AI Assistants'; | |
| const CLAUDE_CODE_DISPLAY_NAME = 'Claude Code Chat'; | ||
| const CLAUDE_CODE_BIND_STATUS_BAR_MESSAGE = '✓ RangeLink bound to Claude Code Chat'; | ||
|
|
||
| suite('Built-in AI Assistants', () => { | ||
| const log = createLogger('builtInAiAssistants'); | ||
| standardSuite('Built-in AI Assistants', (log) => { | ||
| const tmpFileUris: vscode.Uri[] = []; | ||
|
|
||
| suiteSetup(async () => { | ||
| await activateExtension(); | ||
| }); | ||
|
|
||
| teardown(async () => { | ||
| await vscode.commands.executeCommand(CMD_UNBIND_DESTINATION); | ||
| await closeAllEditors(); | ||
| cleanupFiles(tmpFileUris); | ||
| tmpFileUris.length = 0; | ||
| await settle(); | ||
|
|
@@ -194,7 +184,10 @@ suite('Built-in AI Assistants', () => { | |
| 'Human reported the cold-send RangeLink did not appear in Claude Code chat', | ||
| ); | ||
|
|
||
| // Select lines 3-4 for warm send | ||
| // Select lines 3-4 for warm send. Re-focus the editor first — the cold | ||
| // verdict dialog stole focus and the editor must be active for the send | ||
| // command to see the selection. | ||
| await vscode.window.showTextDocument(doc); | ||
| editor.selection = new vscode.Selection(2, 0, 3, 6); | ||
| await settle(); | ||
|
|
||
|
|
@@ -309,4 +302,65 @@ standardSuite('Built-in AI Assistants — Destination Picker', (log) => { | |
|
|
||
| log('✓ github-copilot-chat-001 — log confirms "GitHub Copilot Chat" appears in R-D picker'); | ||
| }); | ||
|
|
||
| test('claude-code-006: Cold-start default settings produce correct ColdRefocusConfig', async function (this: MochaContext) { | ||
| const config = vscode.workspace.getConfiguration('rangelink.destinations.claudeCode'); | ||
| const totalMs = config.get<number>('coldStartDelayMs', 1500); | ||
| const intervalMs = config.get<number>('coldRefocusIntervalMs', 300); | ||
|
|
||
| assert.strictEqual(totalMs, 1500, 'Expected default coldStartDelayMs to be 1500'); | ||
| assert.strictEqual(intervalMs, 300, 'Expected default coldRefocusIntervalMs to be 300'); | ||
| assert.ok( | ||
|
Comment on lines
+308
to
+313
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Default-value assertions look stale and should come from shared defaults constants. This test currently hardcodes Proposed fix+import {
+ CLAUDE_CODE_COLD_START_DELAY_MS,
+ CLAUDE_CODE_COLD_REFOCUS_INTERVAL_MS,
+} from '../../constants/settingDefaults';
...
- const totalMs = config.get<number>('coldStartDelayMs', 1500);
- const intervalMs = config.get<number>('coldRefocusIntervalMs', 300);
+ const totalMs = config.get<number>('coldStartDelayMs', CLAUDE_CODE_COLD_START_DELAY_MS);
+ const intervalMs = config.get<number>(
+ 'coldRefocusIntervalMs',
+ CLAUDE_CODE_COLD_REFOCUS_INTERVAL_MS,
+ );
- assert.strictEqual(totalMs, 1500, 'Expected default coldStartDelayMs to be 1500');
- assert.strictEqual(intervalMs, 300, 'Expected default coldRefocusIntervalMs to be 300');
+ assert.strictEqual(
+ totalMs,
+ CLAUDE_CODE_COLD_START_DELAY_MS,
+ `Expected default coldStartDelayMs to be ${CLAUDE_CODE_COLD_START_DELAY_MS}`,
+ );
+ assert.strictEqual(
+ intervalMs,
+ CLAUDE_CODE_COLD_REFOCUS_INTERVAL_MS,
+ `Expected default coldRefocusIntervalMs to be ${CLAUDE_CODE_COLD_REFOCUS_INTERVAL_MS}`,
+ );As per coding guidelines, "Define named constants for all numeric literals with semantic meaning using SCREAMING_SNAKE_CASE". 🤖 Prompt for AI Agents |
||
| totalMs > intervalMs, | ||
| `Expected coldStartDelayMs (${totalMs}) > coldRefocusIntervalMs (${intervalMs})`, | ||
| ); | ||
|
|
||
| log('✓ Default cold-start config produces valid ColdRefocusConfig'); | ||
| }); | ||
|
|
||
| test('claude-code-007: Cold-start validation rejects invalid config and falls back to defaults', async function (this: MochaContext) { | ||
| if (!vscode.extensions.getExtension(CLAUDE_CODE_EXTENSION_ID)) { | ||
| log('Skipping claude-code-007 — Claude Code extension not installed in this test config'); | ||
| this.skip(); | ||
| } | ||
|
|
||
| const config = vscode.workspace.getConfiguration('rangelink.destinations.claudeCode'); | ||
|
|
||
| // Set delay <= interval so the validation rejects it (by default, delay > | ||
| // interval so the config is valid; flipping that relationship makes it invalid). | ||
| const INVALID_DELAY_MS = 100; | ||
| const INVALID_INTERVAL_MS = 400; | ||
| await config.update('coldStartDelayMs', INVALID_DELAY_MS, vscode.ConfigurationTarget.Workspace); | ||
| await config.update( | ||
| 'coldRefocusIntervalMs', | ||
| INVALID_INTERVAL_MS, | ||
| vscode.ConfigurationTarget.Workspace, | ||
| ); | ||
| await settle(); | ||
|
|
||
| const logCapture = getLogCapture(); | ||
| logCapture.mark('before-cc-007'); | ||
|
|
||
| // Validation lives inside getColdRefocus, which is a thunk only invoked | ||
| // during focus() — not at bind() time. CMD_JUMP_TO_DESTINATION triggers | ||
| // focusBoundDestination() → focus() → getColdRefocus() → validation warning. | ||
| await vscode.commands.executeCommand(CMD_BIND_TO_CLAUDE_CODE); | ||
| await settle(); | ||
|
|
||
| await vscode.commands.executeCommand(CMD_JUMP_TO_DESTINATION); | ||
| await settle(); | ||
|
|
||
| const lines = logCapture.getLinesSince('before-cc-007'); | ||
| const warningLog = lines.find( | ||
| (line) => | ||
| line.includes('coldStartDelayMs must be greater than coldRefocusIntervalMs') && | ||
| line.includes('using defaults'), | ||
| ); | ||
| assert.ok( | ||
| warningLog, | ||
| 'Expected validation warning log with "using defaults" when coldStartDelayMs <= coldRefocusIntervalMs', | ||
| ); | ||
|
|
||
| log('✓ claude-code-007 — invalid config triggers fallback to defaults'); | ||
| }); | ||
|
Comment on lines
+321
to
+365
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verify the actual fallback values, not just the warning log. The test asserts that a validation warning is logged but doesn't verify that the fallback values are actually used. The test name claims the config "falls back to defaults," but the test only checks for the warning. Additional concerns
💚 Proposed enhancementAfter the bind command, retrieve the actual configuration values that were applied and assert they match the expected defaults: await vscode.commands.executeCommand(CMD_BIND_TO_CLAUDE_CODE);
await settle();
+// Verify fallback values were applied
+const configAfterBind = vscode.workspace.getConfiguration('rangelink.destinations.claudeCode');
+const actualTotalMs = configAfterBind.get<number>('coldStartDelayMs', 1500);
+const actualIntervalMs = configAfterBind.get<number>('coldRefocusIntervalMs', 300);
+assert.strictEqual(actualTotalMs, 1500, 'Expected fallback to default coldStartDelayMs');
+assert.strictEqual(actualIntervalMs, 300, 'Expected fallback to default coldRefocusIntervalMs');
+
-await vscode.commands.executeCommand(CMD_JUMP_TO_DESTINATION);
-await settle();
-
const lines = logCapture.getLinesSince('before-cc-007');
const warningLog = lines.find((line) =>
line.includes('coldStartDelayMs must be greater than coldRefocusIntervalMs'),
);(Note: Replace As per coding guidelines (qa-test-cases-v1.1.0.yaml), claude-code-007 should "bind to Claude Code, assert a warning log, and assert fallback to defaults." 🤖 Prompt for AI Agents |
||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect default value documented for coldStartDelayMs.
The table shows
1500as the default forrangelink.destinations.claudeCode.coldStartDelayMs, but the PR objectives specify the default should be2500. This discrepancy will mislead users about the actual default behavior.Per PR objectives: "rangelink.destinations.claudeCode.coldStartDelayMs — default 2500 (range 500–15000)"
🔧 Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents