Skip to content

Improve VS Code extension diagnostics and path handling - #18

Open
codeprakhar25 wants to merge 2 commits into
mainfrom
feat/issue-13
Open

Improve VS Code extension diagnostics and path handling#18
codeprakhar25 wants to merge 2 commits into
mainfrom
feat/issue-13

Conversation

@codeprakhar25

Copy link
Copy Markdown
Owner

Summary

  • Add structured AgentDiff output-channel diagnostics and an AgentDiff: Open Logs command for VS Code Copilot capture.
  • Resolve capture state per edited document for multi-root workspaces, repo initialization checks, document versions, and WSL path/process bridging.
  • Expand extension tests and README docs around capture limits, debugging, and local test workflow.

Test plan

  • git diff --check
  • node --test scripts/tests/test_extension.js

Refs #13

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-apps

greptile-appsBot commented May 7, 2026

Copy link
Copy Markdown

Greptile Summary

This 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.

  • extension.js: openLogsCmd is now registered in context.subscriptions before the Copilot guard, so the diagnostics command works even when Copilot is missing; the save-handler race condition is fixed by deleting the pending entry before awaiting captureDocument; WSL helpers are added for Windows-hosted WSL workspaces.
  • test_extension.js (new): Adds an offline test suite covering capture flow, multi-root path scoping, diagnostics, and WSL path translation — but the "Copilot absent" test contains a stale assertion that expects zero subscriptions when there is now one (the openLogsCmd disposable).
  • package.json / README.md: Manifest updated with the new command declaration; README clarifies Copilot capture scope, WSL install paths, and how to use the output channel.

Confidence Score: 4/5

Safe 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

FilenameOverview
scripts/vscode-extension/extension.jsAdds structured output-channel diagnostics, agentdiff.openLogs command (correctly registered before the Copilot guard), WSL path/process bridging, and per-document capture-state tracking. Race condition from save handler is addressed by deleting the pending entry before awaiting captureDocument.
scripts/tests/test_extension.jsNew comprehensive test suite. The assertion vscode._subscriptions.length === 0 on the 'Copilot absent' test is now wrong — openLogsCmd is pushed to subscriptions before the early return, making the actual count 1.
scripts/vscode-extension/package.jsonAdds the agentdiff.openLogs command declaration to the manifest. Straightforward and correct.
README.mdMinor documentation updates: clarifies Copilot capture scope, notes WSL/remote install path, and adds a paragraph about the AgentDiff output channel and Open Logs command.

Sequence Diagram

sequenceDiagram
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
Loading

Reviews (2): Last reviewed commit: "fix: register openLogs before Copilot gu..." | Re-trigger Greptile

Comment threadscripts/vscode-extension/extension.js
Comment threadscripts/vscode-extension/extension.js
// 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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2FLUSH_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.

Suggested change
constFLUSH_DELAY_MS=Number(process.env.AGENTDIFF_EXT_FLUSH_DELAY_MS||DEFAULT_FLUSH_DELAY_MS);
const_rawFlushDelay=Number(process.env.AGENTDIFF_EXT_FLUSH_DELAY_MS);
constFLUSH_DELAY_MS=Number.isFinite(_rawFlushDelay)&&_rawFlushDelay>0 ? _rawFlushDelay : DEFAULT_FLUSH_DELAY_MS;

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>
@github-actions

Copy link
Copy Markdown

AgentDiff Report

Summary

AgentLines%
Prakhar Khatri579100%

Review Context

  • Intent: unspecified (579 lines, 4 files)
    • Agent/model: Prakhar Khatri / unknown

Files To Review First

FileLinesDominant AgentIntentContext
scripts/vscode-extension/extension.js318Prakhar Khatriunspecifiedtrace f583a76c
scripts/tests/test_extension.js221Prakhar Khatriunspecifiedtrace f583a76c
README.md36Prakhar Khatriunspecifiedtrace f583a76c
scripts/vscode-extension/package.json4Prakhar Khatriunspecifiedtrace f583a76c
Trace details
TraceAgentIntentFilesLines
f583a76cPrakhar KhatriunspecifiedREADME.md, scripts/tests/test_extension.js, scripts/vscode-extension/extension.js, scripts/vscode-extension/package.json566
b500515fPrakhar Khatriunspecifiedscripts/vscode-extension/extension.js13

Comment on lines +355 to +361
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"')));
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1Stale 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.

Suggested change
test('activate returns early without listeners when Copilot is absent',()=>{
constvscode=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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@codeprakhar25