RoslynMcpServer / rules

---
description: Known pitfalls and regression lessons from production use
alwaysApply: true
---

# Known pitfalls (do not reintroduce)

- **Newer BuildHost binlogs** — BuildHost-net472 uses Visual Studio MSBuild, which can write a newer format than the registered SDK reader. Replay with `AllowForwardCompatibility=true` and subscribe to `RecoverableReadError`. Any such error rejects the file's provenance; count failed files rather than error records. Format mismatch alone is not a failure. Per-binlog version, project starts and failures are in measurement reports.

1. **Relative paths vs CWD** — Cursor often starts MCP with CWD = user home. Relative `filePath` without workspace resolve → wrong/missing files. Fix: `ResolvePathAgainstWorkspace`.

2. **`search_code` without directory** — scanning from home hangs forever. Default to loaded workspace directory; default extension filter `.cs`; add scan timeout.

3. **`get_code_skeleton` vs decompile** — agents pass `assemblyName`/`typeName` to file skeleton. Keep tools distinct; friendly error if `path` missing.

4. **Fuzzy assembly match** — `Contains(name)` picks wrong DLL. Exact `{name}.dll` + deps.json + NuGet/BCL fallback only.

5. **SDK mismatch silent** — `0 Error(s)` while wrong MSBuild (9.x vs 10.x pin). Detect and emit `MCP_MSBUILD_SDK_MISMATCH`; fix env before app code.

6. **NuGet audit invisible at `-v:minimal`** — escalate restore/normal verbosity; parse embedded `error NU####`.

7. **VSTest “partial” on missing counters** — requiring both `Passed:` and `Failed:` is wrong. Success often omits `Failed:`; `.slnx`/MSBuild VSTest fail output often omits `Passed:`. Infer the missing side (`Failed`/`Skipped` → 0, or `Passed = Total − Failed − Skipped`). Total-only without those lines stays unparsed.

8. **MSBuild bitness** — 64-bit process + x86 SDK = BadImageFormat. Register matching MSBuild early.

9. **CWD = BaseDirectory** — publish folder breaks tools; use `ROSLYN_MCP_WORKSPACE` instead.

10. **Hardcoded paths in docs** — breaks other machines; resolve via IDE workspace / `list_directory_tree`.

11. **`run_specific_test` filter** — never put Roslyn `FullyQualifiedFormat` (method `()`) into `FullyQualifiedName=`; VSTest treats `()` as grouping. Prefer `FullyQualifiedName~` with `Ns.Type.Method` (no parens). Do not prefix a leading `.` when the needle is already dotted (`.Ns.Class` does not appear inside `Ns.Class`).

12. **Fake/cached `run_dotnet_build` success** — MCP has **no** result cache. False greens came from (a) MSBuild incremental/up-to-date skipping recompile → default `--no-incremental`; (b) probe `lastExitCode` overwritten by restore exit 0 after a failed build → effective exit = last **build** step only; (c) omitted `-c` on multi-config solutions. Always check metadata `Configuration` / `NoIncremental`.

13. **Publish locked by zombie MCP** — Cursor may keep `RoslynMcpServer.exe` running after MCP disconnect; publish then fails with MSB3027 file locks. `publish-and-verify.ps1` auto-kills repo/publish holders (opt out: `-SkipKill`), then reload MCP.

14. **`apply_patch` replaceAll hang** — never rescan the inserted `newString`. If `newString` contains `oldString` (`Foo` → `Ns.Foos`), `while (IndexOf)` grows the buffer forever, MCP never returns, logs stay empty. Advance `searchFrom` past each insert (same as `string.Replace`); log start/match/write with `newContainsOld` and elapsed ms.

15. **`run_specific_test` / `run_dotnet_test` without `--no-build`** — a single `dotnet test` on a large `.slnx` dumps MSBuild warnings after (or instead of) the VSTest banner. Parser requires `Passed!` / `Total tests:` / `Test Run Successful`; exit 0 with only `Time Elapsed` / `N Warning(s)` becomes **`Status: partial`** + last 2KB of build footer. Split: `dotnet build` then `dotnet test --no-build --no-restore`; parse only the test process. Do not treat exit 0 as success and do not feed the build log to `VstestOutputParser`.

16. **VSTest duration not only `ms`** — slow tests print `[1 s]` / `[1 m 28 s]`. A Passed/Failed regex that requires `ms` only reports **no matching tests** despite `Total tests: 1` / `Passed: 1`. Accept `ms|s|m|h` (compound durations). Do not treat any `[...]` as duration.

17. **`TargetFrameworks` / missing `Compile`** — `Directory.Build.props` with `TargetFrameworks` (even one TFM, or `netstandard2.0;net10.0`) makes design-time evaluation a CrossTargeting outer build: `Build` exists, `Compile` does not. `load_workspace` must pass `targetFramework` (inner TFM, e.g. `net10.0`). Do not treat this as SDK mismatch; `dotnet build` can still succeed. Dedicated failure lists TFMs from nearest props.

18. **`load_workspace` fake Failure from design-time warnings** — MSBuildWorkspace wraps ordinary warnings as `Msbuild failed when processing the file` with `Kind=Failure` and often **omits** `warning ASPDEPR007` / `warning MSB3270`. Inner text like IncludeOpenAPIAnalyzers deprecation, processor-architecture mismatch, or analyzer project without metadata must not fail load when projects opened. Do not treat wrapper word `failed` or `StartsWith("Failure:")` as fatal; block only on `error NU|MSB|NETSDK` or known-hard (missing Compile, empty TFM, could not be loaded, SDK not found). Log `Diagnostic.Kind`, not always “warning”.

19. **VS 2026 / MSBuild 18 `XMakeElements` is not SDK mismatch** — net472 BuildHost registers VS 18 MSBuild and dies in `Microsoft.Build.Shared.XMakeElements` (`RemoteInvocationException` / `TypeInitializationException`). Agents mislabel this as `MCP_MSBUILD_SDK_MISMATCH`. Fix: `Microsoft.CodeAnalysis.Workspaces.MSBuild` **5.9+** (AppDomain isolation). Dedicated report **Workspace Load Failed (VS 2026 / MSBuild 18 BuildHost)**; workaround is a single SDK-style `.csproj`. Do not change `global.json` for this.

20. **Stale symbols after IDE/git vs unsaved buffers** — `find_symbol_*` uses the in-memory snapshot. v1.1.0+ watches **saved** `.cs` (FileSystemWatcher dirty set, batch `WithDocumentText`). Do not hook editor keystrokes. Do not `OpenSolutionAsync` from the watcher. `.csproj`/`.sln` → graph-stale + skip load cache, not a full reload in the event handler. `obj` generated sources still need `reset_workspace`. Linux inotify limit or a watcher `Access denied` → set a full re-read, then log. `Logger.Log` rethrows when a provider throws (Windows Event Log `ObjectDisposedException` after its handle is disposed) and that callback is a thread-pool thread, so an escaping exception kills the process. Catch it. Do not crash.

21. **VSTest `Build FAILED` / `0 Error(s)` is not a compile failure** — after tests fail, MSBuild still prints that footer with zero compiler errors. Do not flatten `Error Message:` to a 512-char one-liner (FluentAssertions diffs live below that). Do not take the last 2KB of a huge integration log (that is the footer). Do not treat `0 Error(s)` as `LooksLikeSilentFailure`. Strip the outcome footer before head+tail; keep assertion multiline; ignore those lines in tool-log highlights. Theory `Failed FQN(args)` must still match `run_specific_test` filters.

22. **`run_specific_test` needle vs xUnit display names** — console logger may print only the method (`methodDisplay=method`) or `FQN(args)` for Theories, while the filter needle is `Ns.Class.Method`. Requiring a dotted name and `line.Contains(fullNeedle)` reports **no matching tests** despite `Total tests: 1, Passed: 1` / `[3 m 8 s]`. Match a display suffix with a `.` boundary (`OtherMethod` ≠ `Method`). Do not emit the no-match agent diagnostic when the outcome line is that display name.

23. **Failed-test Standard Output is not the assertion** — VSTest `Standard Output Messages:` (ConversationId, `TestContext`, `ITestOutputHelper`) must not be used only as an error-body terminator. Keep Error/Stack budgets unchanged; render StdOut/StdErr as separate head+tail excerpts (default 2500/1000). Do not fold stdout into the 3000-char assertion window (that drops ConversationId). Do not dump raw CombinedOutput when failures already parsed (pitfall 21). Agents needing more use `includeFullOutput` / `maxOutputChars`, not shell `dotnet test`.

24. **`TryApplyChanges` + `AddDocument` writes the backing `.csproj`** — `MSBuildWorkspace` implements `ApplyChangesKind.AddDocument` / `RemoveDocument` by editing the project file (explicit `<Compile Include>`). On SDK-style projects that already glob the file, the next `dotnet build` is NETSDK1022. Disk-sync (v1.3.30+) never `AddDocument`/`RemoveDocument`; a new or deleted `.cs` is composition-stale and needs `load_workspace` (stale skips cache) or `reset_workspace` then load. Do not “fix” this with `EnableDefaultCompileItems=false` or a `global.json` change. Intentional refactoring `AddDocument` (`extract_interface(createNewFile=true)`, `move_type_to_new_file`) is a separate follow-up.

25. **`SourceText.From(text, Encoding.UTF8)` adds a UTF-8 BOM on write** — `MSBuildWorkspace.ApplyDocumentTextChanged` writes the file with `DetermineEncoding(text, document)` = `text.Encoding`, and `Encoding.UTF8` carries the BOM preamble. `Encoding.UTF8` in `WithDocumentText`/`AddDocument` therefore put a BOM on every edited file (v1.4.15). The mirror trap is the manual `File.WriteAllTextAsync(path, text, ct)` overload: no encoding means no BOM, so it strips a BOM the file had. Both writes of one operation must resolve the same encoding — use `SourceTextEncoding.ResolveForWrite(existing, candidate, diskPath)` / `ForDiskPath`, never a bare `Encoding.UTF8`. Do not add a `RoslynMcp.jsonc` key for this; “preserve the file” is the contract, not an option.

26. **Log harness id is the immediate parent** — several stdio processes share `logs/mcp-*.log`. Each line is `[{harness}-{4}]` (`[cursor-k7m2]`, `[dsh-p9qx]`, `[opencode-4mht]`) from the parent image (`Cursor.exe`, `claude`, `codex`, `opencode`) or, when the parent is `node`/`nodejs`, from that command line (`@deepseek-ai/dsh`, `@anthropic-ai/claude-code`, `@openai/codex`, an `opencode` directory, or a segment such as `cursor-agent`). Do not walk up to the shell: a node harness's grandparent is often powershell. Do not parse `Cursor.exe` arguments: that utility host is named Cursor while the command line contains `node.mojom`. Do not write the command line into the log.

27. **`get_changed_files` pipe read** — do not read stderr only after stdout reaches EOF, and do not pass the request cancellation token into `ReadToEndAsync`. A full stderr buffer (the size is not 4 KiB on every OS) leaves `git` blocked and stdout unclosed. The same code path runs on Windows, Linux, and macOS: both pipes plus process exit share one deadline (180s), stdin is closed immediately, and cleanup is one shared 3s budget after `Kill(entireProcessTree: true)`. Do not report the tree as killed unless the process has exited and both EOFs were observed. Do not add a Windows job object without the same guarantee on Unix.

28. **Additional-file and analyzer-config text** — do not `TryApplyChanges` that text. It rewrites the additional file and does not update the published snapshot; an analyzer config throws `NotSupportedException` (`Changing analyzer config documents is not supported`) and writes nothing. Replace the text on the published snapshot. XAML, resx, Razor, and a path that is also an evaluation input stay out. Coverage unknown does not block a document already in the graph, and the replace does not make coverage complete.

29. **Non-C# read vs a newer watcher event** — `StableFileBytes` reads without the session gate. A callback during that read bumps `EventRevision`, and `ApplyContentObservation` stays `LeftPending`. Removing `_dirtyNonCSharpPaths` for that outcome used to drop the newer event for the rest of the flush, so one `GetPublishedSolutionAfterDiskSyncAsync` kept the old additional-file or `.editorconfig` text. Retry only when the revision moved, a few times, and publish only after `Changed` or `Echo`. Do not retry `UncommittedWrite`: the flush already holds `_workspaceLock`, and the writer commits under that same lock. Do not publish a read that is still pending. Do not loop until events stop arriving.