Improve VS Code extension diagnostics and path handling - #18
Improve VS Code extension diagnostics and path handling#18codeprakhar25 wants to merge 2 commits into
Conversation
Sorry, something went wrong.
Add structured AgentDiff extension logging, multi-root and WSL-aware capture handling, and tests/docs so Copilot capture failures are visible and easier to diagnose. Co-authored-by: Cursor <cursoragent@cursor.com>
Greptile SummaryThis PR wires up a structured AgentDiff VS Code output channel with an agentdiff.openLogs command, fixes the Copilot-absent diagnostic flow by registering that command before the Copilot extension guard, adds WSL path/process bridging, and resolves per-document capture state for multi-root workspaces.
Confidence Score: 4/5Safe to merge after fixing one failing test assertion in the new test suite. The extension logic itself is sound — registering openLogsCmd before the Copilot guard is the intended fix and works correctly. The new test suite ships with a stale assertion: the Copilot-absent test expects zero subscriptions but one (openLogsCmd) is now always added before the early return, making node --test fail on the changed path. scripts/tests/test_extension.js — the subscription-count assertion in the Copilot-absent test needs to be updated from 0 to 1. Important Files Changed
Sequence DiagramsequenceDiagram
participant VS as VS Code
participant EXT as extension.js
participant CH as OutputChannel
participant GIT as git (execFile)
participant PY as capture script (spawn)
VS->>EXT: activate(context)
EXT->>CH: createOutputChannel('AgentDiff')
EXT->>VS: registerCommand('agentdiff.openLogs')
Note over EXT: Copilot guard check
alt Copilot missing
EXT->>CH: logEvent warn: inactive_copilot_missing
EXT-->>VS: return (early exit)
else Copilot present
EXT->>VS: register onDidChangeTextDocument
EXT->>VS: register onDidSaveTextDocument
EXT->>VS: registerCommand('agentdiff.captureNow')
end
VS->>EXT: onDidChangeTextDocument(event)
EXT->>EXT: buffer lines in pendingChanges[filePath]
EXT->>EXT: debounce setTimeout(flushAll, FLUSH_DELAY_MS)
VS->>EXT: onDidSaveTextDocument(doc)
EXT->>EXT: pendingChanges.delete(filePath)
EXT->>EXT: cancel timer if no more pending
EXT->>GIT: rev-parse --show-toplevel / --git-common-dir
GIT-->>EXT: repoRoot, gitDir
EXT->>EXT: pathExistsDir(agentdiffDir) initialized?
alt not initialized
EXT->>CH: logEvent warn: skipped_repo_not_initialized
else initialized
EXT->>PY: spawn capture script (JSON payload via stdin)
end
Reviews (2): Last reviewed commit: "fix: register openLogs before Copilot gu..." | Re-trigger Greptile |
Sorry, something went wrong.
| // completions while filtering out most false positives. | ||
| const MIN_COPILOT_CHANGE_LEN = 50; | ||
| const DEFAULT_FLUSH_DELAY_MS = 2000; | ||
| const FLUSH_DELAY_MS = Number(process.env.AGENTDIFF_EXT_FLUSH_DELAY_MS || DEFAULT_FLUSH_DELAY_MS); |
There was a problem hiding this comment.
FLUSH_DELAY_MS can silently become NaN — if AGENTDIFF_EXT_FLUSH_DELAY_MS is set to a non-numeric string (e.g. "off"), Number("off") is NaN. setTimeout(flushAll, NaN) is treated as setTimeout(flushAll, 0) in both browsers and Node.js, causing the debounce window to collapse to zero and every keystroke to trigger an immediate flush.
| const FLUSH_DELAY_MS = Number(process.env.AGENTDIFF_EXT_FLUSH_DELAY_MS || DEFAULT_FLUSH_DELAY_MS); | |
| const _rawFlushDelay = Number(process.env.AGENTDIFF_EXT_FLUSH_DELAY_MS); | |
| const FLUSH_DELAY_MS = Number.isFinite(_rawFlushDelay) && _rawFlushDelay > 0 ? _rawFlushDelay : DEFAULT_FLUSH_DELAY_MS; |
Sorry, something went wrong.
Two bugs in extension.js: 1. agentdiff.openLogs was registered after the copilot guard so it was silently a no-op when capture was inactive (the most useful time). Moved registration before the guard and pushed to subscriptions early. 2. pendingChanges.delete() happened after await captureDocument(), leaving the flush timer able to fire a duplicate capture during the async gap. Delete and cancel the timer before awaiting. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
AgentDiff ReportSummary
Review Context
Files To Review First
|
Sorry, something went wrong.
| test('activate returns early without listeners when Copilot is absent', () => { | ||
| const vscode = makeVscodeMock({ copilotInstalled: false }); | ||
| activateExt(vscode); | ||
|
|
||
| assert.equal(vscode._subscriptions.length, 0); | ||
| assert.ok(vscode._outputLines.some((line) => line.includes('"event":"extension.inactive_copilot_missing"'))); | ||
| }); |
There was a problem hiding this comment.
Stale subscription-count assertion will fail
The test asserts vscode._subscriptions.length === 0, but with the new code activate() now calls context.subscriptions.push(openLogsCmd) before the Copilot guard check. When Copilot is absent the function returns early, but the openLogsCmd disposable is already in context.subscriptions, so the length is 1, not 0. Running node --test on this file will produce an AssertionError: 1 == 0 here.
| test('activate returns early without listeners when Copilot is absent', () => { | |
| const vscode = makeVscodeMock({ copilotInstalled: false }); | |
| activateExt(vscode); | |
| assert.equal(vscode._subscriptions.length, 0); | |
| assert.ok(vscode._outputLines.some((line) => line.includes('"event":"extension.inactive_copilot_missing"'))); | |
| }); | |
| assert.equal(vscode._subscriptions.length, 1); // openLogsCmd is registered before the Copilot guard |
Sorry, something went wrong.
Summary
Test plan
Refs #13