Skip to content

Fix VS Code Define Step never recovering after a post-open build - #26

Merged
clrudolphi merged 2 commits into
masterfrom
fix/vscode-define-step-applyedit
Jul 3, 2026
Merged

Fix VS Code Define Step never recovering after a post-open build#26
clrudolphi merged 2 commits into
masterfrom
fix/vscode-define-step-applyedit

Conversation

@clrudolphi

@clrudolphi clrudolphi commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Closes VS Code: "Define Step" code action has no visible effect #2. The issue's own hypothesis — a bad WorkspaceEdit shape (flat changes map instead of documentChanges with CreateFile) — was tested and disproven: FeatureCodeActionHandler already sends the correct documentChanges/CreateFile/TextDocumentEdit shape, and it round-trips correctly through vscode-languageclient's real type guards and apply logic (verified with a raw wire capture, a standalone Node driver using the actual vscode-jsonrpc/vscode-languageserver-types packages against the real server, and a full VS Code Extension Host run that created the file with correct content end-to-end).
  • The actual root cause: server-side binding discovery (ConnectorBindingRegistryProvider) reflects over a project's OutputAssemblyPath DLL. If that DLL doesn't exist yet when the VS Code extension's ProjectManager sends its first reqnroll/projectLoaded (e.g. a freshly cloned repo opened before ever running dotnet build), discovery fails once ("Output assembly not found") and is never retried. Visual Studio doesn't hit this because VsProjectEventMonitor hooks DTE's BuildEvents.OnBuildDone and re-sends every project after each build; the VS Code extension had no equivalent, so a user who opens the project, builds it, and then edits a feature file ends up with a permanently empty match cache — diagnostics, semantic tokens, and code actions for undefined steps all silently absent.

Revised fix (was: full reqnroll/projectLoaded resend)

The server already declares a standard workspace/didChangeWatchedFiles registration for **/bin/**/*.dll (WatchedFilesHandler.cs) specifically to retry discovery once the output assembly appears, resolving the owning project from the OutputAssemblyPath it was given at initial registration (that path is computed from MSBuild properties and is correct even before the file exists). Whether each IDE's LSP client actually delivers those dynamically registered watched-file events reliably is an open design question (Q9 in docs/LSP-IDE-Support-Open-Questions.md) — VS Code's files.watcherExclude commonly excludes bin//obj/ from the file watching a dynamically-registered FileSystemWatcherFeature relies on, which is the most likely reason this wasn't firing.

Rather than working around that by re-running dotnet msbuild and resending the full reqnroll/projectLoaded + reqnroll/projectFiles baseline on every rebuild (the original approach in this PR), ProjectManager's fallback watcher now sends the same standard workspace/didChangeWatchedFiles notification directly — landing on the server's existing handler with no new server code and no extra MSBuild invocation per rebuild. This is lighter weight and reuses the server's existing debounce/cancel-and-restart discovery logic instead of duplicating it client-side.

A follow-up issue tracks investigating and closing Q9 properly (confirming the exclusion-pattern hypothesis and deciding whether the dynamic-registration path can be made reliable enough to drop this client-side fallback entirely).

Test plan

  • Added a unit test asserting bin/**/*.dll paths resolve to their owning project via the existing findOwningProjectFile helper.
  • Manually verified end-to-end in a real VS Code Extension Host: with the fix reverted, a code action requested before a build stayed absent forever even after building; with the fix applied, "Define missing step" appears within ~20s of the build completing, applies correctly, and creates the populated step-definition file — all without restarting VS Code.
  • Full LSP spec suite: 131 passed, 18 pre-existing skips, 0 failures (dotnet test tests/LSP/Reqnroll.IdeSupport.LSP.Server.Specs).
  • npm run compile / npm run lint / npm test (VS Code extension unit suite): all clean, 17 passing.

🤖 Generated with Claude Code

  • Not yet re-verified: the manual Extension Host build-then-code-action scenario was only run against the original (heavier) fix; it has not been re-run against this lighter-weight didChangeWatchedFiles revision. The automated unit suite passes and the logic change is small/mechanical (same trigger, same server-side handler, different message), but a manual re-run is recommended before merge.

clrudolphi and others added 2 commits July 2, 2026 19:10
Root cause: extensive investigation (raw wire captures, a real dotnet-hosted
LSP client, and a full VS Code Extension Host run) proved the reported
hypothesis wrong — FeatureCodeActionHandler's WorkspaceEdit already uses
DocumentChanges with CreateFile + TextDocumentEdit, and vscode-languageclient
applies that shape correctly (verified against the library's own
CreateFile.is()/TextDocumentEdit.is() type guards and a live apply that
produced the file with correct content).

The real gap: server-side binding discovery (ConnectorBindingRegistryProvider)
reflects over a project's OutputAssemblyPath DLL. If that DLL doesn't exist
when the VS Code extension's ProjectManager sends its first
reqnroll/projectLoaded (e.g. a freshly cloned repo opened before `dotnet
build` has ever run), discovery fails once ("Output assembly not found") and
is never retried. Visual Studio doesn't have this problem because
VsProjectEventMonitor hooks DTE's BuildEvents.OnBuildDone and re-sends every
project after each build; the VS Code extension had no equivalent signal, so
a user who opens, builds, then edits a feature file gets a permanently empty
match cache — matching diagnostics, semantic tokens, and code actions all
silently absent (reproduced end-to-end in a real Extension Host and confirmed
fixed by rebuilding the same scenario against the fix below).

Fix: ProjectManager now watches **/bin/**/*.dll and re-sends
reqnroll/projectLoaded (refreshing outputAssemblyPath) plus the
reqnroll/projectFiles baseline whenever an output assembly is created or
changed, debounced through the existing scheduleResend path. This covers
`dotnet build` from any terminal, a VS Code build task, or C# Dev Kit.

Test plan:
- Added a unit test asserting bin/**/*.dll paths resolve to their owning
  project via the existing findOwningProjectFile helper.
- Manually verified end-to-end in a real VS Code Extension Host: with the fix
  reverted, a code action requested before a build stayed absent forever
  after building; with the fix applied, "Define missing step" appears within
  ~20s of the build completing, applies correctly, and creates the populated
  step-definition file — all without restarting VS Code.
- Full LSP spec suite: 131 passed, 18 pre-existing skips, 0 failures
  (dotnet test tests/LSP/Reqnroll.IdeSupport.LSP.Server.Specs).
- npm run compile / npm run lint / npm test (VS Code extension unit suite):
  all clean, 18 passing.

Closes #2

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The original fix for #2 re-ran MSBuild evaluation and resent the full
reqnroll/projectLoaded + reqnroll/projectFiles baseline whenever a
project's output DLL was (re)built. That's heavier than the bug needs:
the server's WatchedFilesHandler already declares a standard
workspace/didChangeWatchedFiles registration for **/bin/**/*.dll and
resolves the owning project from the OutputAssemblyPath it received at
initial registration (computed from MSBuild properties, correct
whether or not the file exists yet) — it just needs to know the file
changed, not a full re-evaluation.

The client's fallback watcher now sends that same standard LSP
notification directly instead of a heavier resend, landing on the
server's existing handler with no new server code and no extra
dotnet msbuild invocation per rebuild.
@clrudolphi
clrudolphi merged commit 4fe7ebd into master Jul 3, 2026
6 checks passed
@clrudolphi
clrudolphi deleted the fix/vscode-define-step-applyedit branch July 3, 2026 15:24
Sign up for free to 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.

VS Code: "Define Step" code action has no visible effect

1 participant