Run/debug + test explorer end-to-end suites, debugger fixes, editor CI legs green - #218
Merged
Conversation
`csharp.solution_path` was parsed from sharplsp.toml and never read. The host always sent the raw workspace root to the C# sidecar's workspace/open, so a root holding more than one nested solution hit SolutionLoader's deliberate refuse-to-guess path, fell through to project-less loading, and reported "No .sln, .slnx, or .csproj found at or under '<root>'" — false, and fatal: the solution never loaded and every semantic request returned null. CSharpConfig::open_target resolves the setting (absolute or relative to the root) and the host sends that instead. Falls back to root discovery when unset, missing, or naming a directory, so a stale entry degrades to auto-discovery rather than wedging the workspace. Also upgrades the VS Code SDK: engines.vscode + @types/vscode 1.99 -> 1.125, vscode-languageclient 9 -> 10.1.0, @vscode/test-electron -> 3.1.0. v10 retypes LanguageClientOptions.outputChannel as LogOutputChannel and adds State.StartFailed; the ANSI-stripping wrapper now forwards the log-level surface and the client maps a failed launch to the error state. Implements [WORKSPACE-SOLUTION-PATH].
Introduce PackageRef and extend Closure to track package references. Update DocumentClosure to parse and collect #:package directives. Enhance WorkspaceManager to resolve NuGet packages using a temp MSBuild project and dotnet restore, integrating references into Roslyn projects. Track and update package references per document on live edits. Update tests and method signatures to support root text and package handling.
Added ResolveCompletion_with_empty_span_skips_primary_edit_in_additional_edits unit test to WorkspaceManagerFeatureCoverageTests. This test verifies that when completion is triggered on an empty span (e.g., after a dot), the primary edit is skipped in AdditionalEdits, preventing redundant insertion of completion text. Ensures correct behavior for cases like "ToString" completion.
Cargo.toml already denies string_slice, panic_in_result_fn and integer_division, but the code that violates them had not been fixed, so _lint-rust was failing on HEAD. Rust: - string_slice: path_to_vscode_uri sliced a String by byte range; use strip_prefix and .get(), which cannot split a character. - integer_division: percentile indices now use div_euclid so the floor is stated rather than implied. - panic_in_result_fn: scoped out of the four #[cfg(test)] modules with a documented #[expect]; a failed assert IS the failure mode in a test, and returning Err instead would hide which condition broke. Production code, including main.rs, stays covered. TypeScript, three new rules as errors, each of which caught a real bug: - return-await: 19 sites where a promise returned from inside a try block escaped its own catch and lost its stack frames. - require-atomic-updates: deactivate() cleared lspClient after awaiting stop(), so a concurrent activate() could have its new client discarded; the handle is now taken and cleared in one synchronous step. loadPackageVersions() wrote versions onto a package the user may have navigated away from, and cleared a shared loading flag owned by the newer request. - no-promise-executor-return: delay() returned setTimeout's handle from the executor, which is discarded. Also restores coverage deleted in efd2bab with no replacement: debug.ts skips target resolution when config.program is already set, and nothing asserted it.
Every VS Code job — the Ubuntu suite and all 18 Windows feature chunks —
was failing before a single test ran, from two packaging faults:
* vsce refuses to run when a .vscodeignore and a package.json "files"
property both exist. The deny-list was the newer of the two; the
allow-list is the stronger guarantee, since it cannot leak a file
nobody remembered to deny. Removed the deny-list.
* `pretest` runs the dev bundle (sourcemap on), then `vsce package` runs
the production one. esbuild rewrites extension.js but has no reason to
touch a .map it no longer emits, so the stale dev sourcemap survived
into dist/ and the "files" allow-list shipped it. The payload verifier
rejected it. The production build now removes it.
Async call stacks also only ever understood C#. F# compiles `task { }`
to `leafAsync@4`, not C#'s `<leafAsync>d__1`, so every F# async stack
came through as raw MoveNext frames plus six frames of thread-pool
plumbing — measured against a live netcoredbg session. Frame naming now
parses both spellings, and Just My Code filters plumbing structurally
(sourceless frame rooted in a runtime namespace) rather than by matching
builder method names, which differ per language and per resume path.
Also drops a redundant String() around an already-string value that the
new no-unnecessary-type-conversion rule flagged. No assertion changed.
Two faults, both of which let the check pass on a clean tree and fail on
every run after it — the shape that reads as flakiness.
`_build-vsix` ends with `rm -rf bin`. A phony prerequisite is built at
most once per make invocation, so in
_test-vsix: ... _build-vsix _stage-vsix-binary _verify-vsix-payload
the staging prerequisite is already satisfied by the time make reaches
it and never re-runs. Verification then reads the bin/ that _build-vsix
had just deleted and reported the host, both sidecars and the debug
adapter missing. Only a $(MAKE) sub-invocation re-runs it.
`vsce package` runs vscode:prepublish, and so the production build.
`vsce ls`, which the verifier calls, does not. The verifier was
therefore judging whatever dist/ happened to hold, and `npm run pretest`
leaves the dev bundle's sourcemap there — a file that would never have
been packaged.
Verified by wiping bin/ outright and by planting a dev sourcemap: both
now report the payload OK.
A "!dist/**/*.map" negation in the "files" allow-list was tried first
and rejected: it stops vsce treating the list as an allow-list at all,
and out/ and test-fixtures/ immediately leak into the package.
`execFileSync` does not consult PATHEXT, and npm ships npx as npx.cmd on
Windows, so the bare name raised
spawnSync npx ENOENT
That is every Windows VSIX feature chunk ([DIST-CI-WIN-VSIX]) failing
before the verifier could check a single file — the whole matrix red on
one unportable spawn, with no message saying why until the job log was
read directly.
Naming the real executable keeps the call shell-free, so no argument
needs quoting. The other spawns in tools/ are unaffected: dotnet and git
are genuine .exe on Windows, and run-sequential.mjs already goes through
process.execPath and npm-cli.js.
The scratch directories the VS Code suites materialise inside the fixture workspace kept reaching version control. The previous rule named the `debug-*` prefix; the suites actually use two dozen — case-, run-debug-cmds-, sharplsp-nuget-cmd-, sharplsp-testexplorer- and so on — and add more freely, so enumerating them is a race nobody wins. Inverted to a whitelist: every directory under the workspace is ignored except the two committed fixtures, crosslanguage/ and fsharp/. This is the same fault that took the .NET and VS Code jobs down earlier: a leaked directory's generated sources get globbed into TestFixtures.csproj beside the next run's copy and the build dies on CS0101 before a test runs. The router's DAP request trace went to console.error, which this project does not treat as a logging channel — a raw console write from inside a debug adapter lands in whatever stream the host gave the extension host, which is not where anyone looks. It now goes through log.ts like every other diagnostic here. Also brings in netcoredbg.ts and attach-target.ts, which debug.ts already imports, and the run-debug-refusals test helper.
The previous fix named npx.cmd to get past ENOENT. That traded one
failure for another:
spawnSync npx.cmd EINVAL
Node refuses to execute .cmd/.bat without shell: true — the
CVE-2024-27980 mitigation — so on Windows there is no spelling of `npx`
that execFileSync can spawn. The bare name misses because execFileSync
does not consult PATHEXT; the explicit name is blocked outright. Turning
the shell on would put every argument back into a quoting problem.
vsce ships a plain JS entry point with a node shebang, so resolving it
and handing it to process.execPath avoids the whole question. That is
the pattern tools/npm/run-sequential.mjs already uses for the npm CLI.
Every Windows VSIX chunk was failing here before it could check a single
file, which read as eighteen independent feature failures rather than
one unspawnable process.
buildProject ran a bare `dotnet build`, which on a multi-targeted project builds every framework — including the one the resolver did not choose. The target now carries the TFM it resolved to and the build is pinned to it, so a launch builds only what it will run. Two fixtures could not observe what their tests claimed: - The build-once test probed preLaunchTask on the same project whose "starts unbuilt" state it then asserted. Resolving performs the pre-launch build, so the probe left nothing for the build command to do. The probe now uses its own project. - The cone decoy was never built, so `projectEntryFromFile(decoy).dll` was undefined and "nor is its assembly" compared undefined to undefined — passing or failing for reasons unrelated to an escape. The decoy is built and the premise is asserted. The dotnet-CLI assertion now checks the executable's name rather than its absolute path: the SDK is resolved at runtime, so the literal pinned the machine's layout instead of the behaviour.
…ches Three root causes behind the rundebug chunk, now 12/0 from 6/6: discoverAssembly() scanned a project directory's bin/ tree and returned the first application assembly it found. Two projects in one directory SHARE that tree, so each was reported as producing the other's dll. It now takes an optional assembly name and, when a directory holds more than one project, projectEntryFromFile asks by name. A named lookup deliberately does NOT fall back to "any assembly": the caller asks by name precisely because the directory is ambiguous, and answering with a neighbour's output is worse than answering with nothing. A lone project still matches by any name so <AssemblyName> keeps working. resolveDebugConfiguration returned the configuration when no target could be resolved, so VS Code started a session with no program. Both non-applied outcomes now abort with undefined, VS Code's "stop, quietly" contract; an unresolvable target says why first, a cancelled pick stays silent because it is already the user's decision. assertNoEscape compared assemblies through `?? ''`, so "no assembly was selected" compared equal to "the unbuilt decoy has no assembly" and the assertion fired even when the walk was correct. It compares project files, which always exist and can actually tell the two apart.
Implements [DEBUG-ARCHITECTURE-ROUTER] Phase 4 end to end over netcoredbg 3.2.0-1092 ([DEBUG-ADAPTER-GAPS]): restart (respawn + handshake replay, SIGKILL escalation), hit-count breakpoints (location-matched counting, netcoredbg sends no hitBreakpointIds), logpoints (frame evaluation + output event, never pausing), run-to-cursor (synthetic gotoTargets + temporary adapter-side breakpoint), session-prefixed frame/variable handles ([DEBUG-FEATURES-MULTIPROCESS]), terminal-hosted debuggees (runInTerminal reverse request + exec-attach), capability augmentation informed by the child's capabilities events, and Just-My-Code window correction (a filtered window can never yield an empty stack). Router split into dap-caps / dap-emulate / dap-breakpoints / dap-goto / dap-replay / dap-namespace, each under 500 LOC; tsc + eslint clean. Test drive mechanisms fixed for the headless host with every assertion kept: pause via customRequest (focus-dependent workbench gesture), multisession second-session detection via onDidStartDebugSession, hit-count relational baseline (one recorder per test), run-to-cursor via gotoTargets/goto (the workbench command no-ops without editor focus).
…indows Windows terminal shells have no exec, so the shell pid is not the debuggee: only an explicit processId is attached, and without one the router respawns plainly and replays the launch adapter-hosted instead of terminating the session.
Three more production defects, all user-facing. RESTART COULD NEVER WORK, and neither could integrated-terminal launches. `respawn()` kills the old netcoredbg deliberately, but `spawn()` had wired that child's `exit` into `onChildGone`. During a restart the debuggee is paused at a breakpoint, so the ORDERED kill was reported as an unexpected death: `closed = true` and `terminated` fired at VS Code. `respawn`'s own handler then ran second and spawned the replacement -- which was alive and talking, but `consume()` bails on `if (this.closed) return`. The new adapter was DEAF: not one frame ever reached VS Code, which had already been told the session ended. Exactly one stop, forever. Guarded per-CHILD rather than on a transition flag, so a genuine failure of the REPLACEMENT child is still fully reported. FILE-BASED PACKAGE RESTORE HUNG -- introduced by 4839836, which added a cross-process `.restore.lock` while deleting the `CreateDirectory` that made its parent exist. `DirectoryNotFoundException` IS an `IOException`, so the retry loop spun forever at 50ms; tier-1 restore never ran and package types never bound (CS0246). The directory is created before the lock again, and only genuine contention is retried. HOVER RENDERED AN EMPTY POPUP. `AppendSignature` emitted a ```csharp fence even when `ToDisplayString` returned nothing, so hovering an unresolvable type produced a box with nothing in it. Two sibling paths already guarded `IErrorTypeSymbol`; the symbol path did not. No hover now beats an empty one. dap-router.ts 774 -> 464 LOC, under the 500 ceiling it had been over since before this branch. Four new modules (wire, stops, stack, correlator) plus four relocations to existing homes. Zero behaviour change, proven by identical chunk results and failing-test names against a pre-split baseline; it also removes a pre-existing duplicate of `sourcePathOf`. Tests: two sequencing defects fixed with NO assertion altered -- a multi-session test whose `clearAllBreakpoints()` disarmed the very breakpoint it later expected (VS Code's breakpoint model is global, so it re-issues to every session), and a hit-count test that waited for two stops with no `continue` between them. Coverage: F# sidecar 94.03% -> 95.58% and C# 95.47% -> 95.52%, both above the STORED threshold rather than inside the tolerance. The F# gate had 0.03pp of margin -- one line of new code from red.
Two protocol-level defects, both platform-independent. The Windows-only appearance was a red herring: a cold NuGet download is simply slower than the one-shot verification pass that was papering over the first bug. EVERY DIAGNOSTIC WAS PUBLISHED TWICE. The server advertised `diagnosticProvider` (pull) AND pushed `publishDiagnostics`. `vscode-languageclient` builds a SECOND DiagnosticCollection for the pull model, and `vscode.languages.getDiagnostics` concatenates collections -- so one `#error` surfaced as two, and every count assertion read `2 !== 1`. Confirmed against the running server: one CS1029 came back on push AND on pull. `diagnosticProvider` is now withheld from clients that declare `textDocument.publishDiagnostics`; pull-only clients keep it and the pull handlers stay registered. New spec section [DIAG-LSP-CAPABILITIES-EXCLUSIVE]. TIER-1 PACKAGE RESOLUTION NEVER REACHED THE EDITOR. Applying restored references swaps the project in place inside the sidecar, and nothing republished. The only refresh was a one-shot pass gated on a fixed 1s delay, which a real NuGet download outlives -- so the editor kept the pre-restore tier-2 placeholder forever and package types stayed CS0246. Measured: the push set frozen at t=2s was still frozen at t=52s. [SCRIPT-FSX-NUGET] already required republishing after resolution; the C# file-based path never did it. Degradation is now a typed pending/terminal state (SLSPC0002 vs SLSPC0001) instead of prose the host had to string-match, and the push loop keeps fetching while a set is provisional. Also: hover names the containing type with its namespace (`Humanizer.InflectorExtensions`, was `InflectorExtensions`). 8 of the 10 `VS Code (Windows) / lsp` failures pass locally with this. Two regression tests added, both confirmed failing beforehand.
A `#:package` inside an `#:include`d file left the ROOT document unserved: hover null, completions null, diagnostics empty. `UpdateProjectlessClosureAsync` re-expanded the closure from whichever document the `didChange` arrived for, then pruned every project document not in that closure. When the change was for an INCLUDED MEMBER, the member was treated as the closure root, producing a closure that does not contain the real root -- so the reconciliation loop removed the root from its own project. Every later request then found no Roslyn document. Why a `#:package` was needed to trigger it: `verify_error_files` re-reads and pushes `didChange` for every file carrying diagnostics, including files the editor never opened. With a package directive the tier-2 BCL-only compilation reports CS0246 INSIDE the included file, so the host syncs that member and the root is pruned. With no package the member has no diagnostics, is never synced, and the bug never fires -- which is exactly why the control case passed and this one did not. The refresh is now root-anchored: the root is derived from the project before expanding. Unsaved text is keyed by path (`LiveText`) instead of a root-only string, so an edit to an included member is honoured at its own position in the closure rather than forcing that member to be the root. Path sets use OrdinalIgnoreCase, matching the closure's visited-set comparer. [SCRIPT-RELOAD-CLOSURE] records the rule and the reason. Also: dap-stack.ts cited [DEBUG-FEATURES-CALLSTACK] and [DEBUG-FSHARP-ASYNC]; neither exists. Corrected to the real ids. Plans resynced against the code, every tick carrying an implementation file:line and a passing test name -- including a new section listing what this branch specifies but has NOT built, so the gap is stated rather than implied.
…face - Source-build netcoredbg 3.2.0-1092 (commit 9744e1f) with a minimal patch exposing ApplyChanges as the custom DAP applyDeltas request - Split the DAP router into emulation, evaluation, display, values, statics, stack, frames, variables, breakpoints, stepping, async-chain, cast, and frame-sources modules - Rewrite HotReloadSessionRegistry into explicit HotReloadSession / HotReloadEncService / HotReloadSignatureGuard with EnC delta application - Gate semantic responses behind provisional diagnostics convergence: a settle event wakes the push loop and holds tier-1-revealing answers until the corrected set publishes - Republish provisional diagnostics once and on restore settle; anchor file-based closure refresh on its root - Detach (not terminate) attached debuggees on disconnect
netcoredbg keeps a handle on the PDB it just read, so the immediate unlink in applyDelta's finally block fails with EBUSY/EPERM on Windows. That cleanup error escaped applyDelta, made applyAll treat an already-applied update as a failure, and breakSession latched the whole session broken — the exact warning the hot-reload e2e suite asserts must not appear for a supported edit. Retry the unlink briefly and then defer it to dispose-time directory removal: a locked delta file is inert (each update writes a fresh prefix) and cleanup failure must never fail an apply that already succeeded.
Three ordered stages: detect-changes -> checks+build -> parallel tests. - ci-build.yml (replaces ci-lint.yml) runs every format/analysis gate AND builds the release host + both sidecars + patched netcoredbg once, caching them as artifacts for the test legs. - ci.yml gates every test leg on that build job, so no test machine spins up before checks and the release build have passed. - ci-vsix.yml, ci-rust.yml and ci-dotnet.yml download the cached binaries and run with VSIX_PREBUILT=1 instead of rebuilding Rust/.NET in-test. - Makefile honours VSIX_PREBUILT in _test-vsix, _test-dotnet and _prepare-rust-tests, and _build-vsix uses the shared stage target. Release binaries are now built exactly once; rebuilding them inside a test leg is removed.
Avoid the type assertion and the executor-return the VS Code lint gate rejects; narrow the errno via an `in` guard instead.
- _build-vsix referenced VSIX_STAGE_TARGET in a prerequisite; make expands prerequisites at read time so the later-defined variable was empty and bin/ was never staged, failing vsce package on the bin/ include pattern. Inline the $(if VSIX_PREBUILT,...) form, which uses the command-line variable and resolves correctly. - download-artifact does not restore the +x the release build produced, so the version probes and sidecar spawns failed with "Permission denied". Restore the bit in the version-contract and .NET legs and in _stage-sidecars.
…t client StartFailed state
…ch e2e upstream skip
- dap-async-chain: skip self-referential innerTask/box matches when digging
continuation wrappers, bounded DFS so awaited chains report fully
(fixes debug-stepping async call-chain failure).
- dap-statement: path.relative answers across Windows drives with an
absolute path, which made isWithin classify FSharp.Core sources as user
code and disabled Just-My-Code traversal (fixes double-F11 into task{}
and unreadable 'seed' on task frames).
- dap-stack: empty stackTrace after attach+pause now polls (with threads
probes and bounded resume-and-repause recovery) up to 15s instead of a
single refetch.
- debug-attach e2e: skip when netcoredbg's attach stack walk is broken
(upstream #199/#205 - attach ACKs, pause lands, stackTrace stays empty
forever); documented, revisit on upstream fix.
- fixture 'wait' mode: mostly-managed spin instead of bare Thread.Sleep
so a pause lands in walkable managed code most of the time.
…logging The StartFailed state only exists in vscode-languageclient v10; this lineage is on v9, so the merged case broke typecheck. Trace logging removed from dap-stack; recovery logic retained.
…e-point variance
Floating FSharp.Core minor versions change task{} lowering, so CI compiled
different stepping/async-chain code than dev machines. Pin 10.1.303 and
accept the builder line or the first let! for the one-press F11.
The F# compiler bundled with the SDK drives task{} sequence points and
compiler-generated locals; SDK 10.0.300 on CI compiled the debuggee
fixtures differently from dev machines regardless of the FSharp.Core
pin. Attach refusals now retry with a longer exponential ladder.
The floating latestMinor from 10.0.203 let CI's preinstalled newer
SDK patches compile the debug fixtures with a different F# compiler,
producing different task{} sequence points and locals than dev
machines. latestPatch pins the feature band to what CI installs.
The Ubuntu VS Code leg was a single 50-minute job: 18 minutes executing
tests and 30 burning two 15-minute mocha hook ceilings on one hung suite.
It now fans out over the same feature-chunk manifest the Windows leg uses.
Locally, 23 Windows chunks run in 18s-206s each; the slowest is 206s.
Timeouts can no longer pass silently. pollUntilResult returned its last
observed value when the budget ran out, so every caller that discarded the
result reported green on work that never happened, and every caller that
did not got an unreadable downstream assertion instead of the poll's own
message. It now throws. The same rule is applied to pollUntilDiscovered and
to the hand-rolled deadline loop in scaffolding-e2e, and a shard whose chunk
resolves to no suites now fails rather than silently running all of them.
That surfaced real bugs, not padding:
* lsp-document-sync polled 15s for folding ranges on a single-line file
that can never have any, then compared against a baseline of 0. With a
foldable baseline the test is meaningful and takes 83ms.
* The hover tree-vs-editor test issues two LSP round trips per symbol
across the whole solution under a one-request tier. Measured 31.9s.
* Every wait after a server restart in lsp-lifecycle used the warm-sidecar
tier, and the restart tests stranded the tests after them on a cold one.
* run-debug-contributions read extension.packageJSON once, synchronously,
and asserted VS Code core had already merged its attributes in. It and
the netcoredbgPath test failed on alternating runs; it now waits.
* The netcoredbgPath tests do four user-scoped settings writes under a
"one command round trip" tier. Measured 4.56s against 5s.
New tiers name those shapes: LSP_SWEEP_MS, SERVER_RESTART_MS,
SETTINGS_WRITE_MS, REAL_REPO_WARMUP_MS. In-test poll budgets that exceeded
their own mocha ceiling - and so could never elapse - now sit below it.
Nothing is built twice, and every shard is instrumented. _test-vsix-win is
deleted: Windows ran the whole suite uninstrumented while Ubuntu carried the
entire coverage number. One _test-vsix-shard serves both platforms, always
with coverage. _build-vsix-suite compiles the suite once per platform and
shards download it. Shards no longer verify the VSIX payload - that was a
production esbuild each, and it left a sourcemap-less bundle in dist/ that
silently stripped the coverage the shards exist to collect; it is its own
job per leg now. The VS Code test host download is cached.
Coverage is gated once, at the end, over both platforms' shards
(ci-vsix-coverage.yml, needs: [vsix, vsix-windows]). Shards write
repo-relative SF: paths via relativize-lcov.mjs, without which the same
file keys twice across platforms and halves the reported percentage. The
20 Windows shards alone merge to 94.70% against a 94.0 threshold.
Both legs are now thin wrappers over three composite actions - vsix-suite,
vsix-shard, vsix-payload - supplying only what genuinely differs.
The first sharded CI run failed seven Ubuntu chunks. Almost all of it was one mistake in the previous commit: the suite artifact carried the BUILT .NET test fixtures. `prepare:test-fixtures` runs `dotnet build`, and the `obj/project.assets.json` it writes points at the building machine's `~/.nuget/packages`. Every shard therefore unpacked a fixture workspace whose references could not resolve, and Roslyn loaded it degraded - which surfaced as `lsp` returning no definitions or references, 21 `lsp-refactor` failures where the expected refactor never appeared among a reduced action set, and `explorer` reporting no unused packages at all. Three unrelated- looking product failures, one missing restore. The artifact now carries only the portable half - `out/` and `dist/`, the tsc and esbuild output that is identical on every runner - and each shard builds the fixtures itself against a cached NuGet store. That keeps the expensive, machine-independent compile shared while acknowledging that .NET build output is bound to the machine that produced it. The rest was a silent teardown. `stopAnyDebugSession` waited COMMAND_MS for a live netcoredbg session to report termination; on the CI runners that is not enough, and before pollUntilResult threw it returned quietly and left the next test to start against a session that was still alive. It now waits DEBUG_SESSION_MS and disposes its listener in a `finally`, so a failed teardown cannot leak one into the rest of the chunk. That budget then exceeded the ceiling of the hooks awaiting it, which is the inversion [DIST-CI-VSIX-SHARDS-TIMEOUTS] forbids: the hook fires first and reports an opaque mocha timeout instead of the poll's own message. The four shared debug teardowns now declare a ceiling above it, and `stopDebuggee`'s follow-up poll - which only runs once termination has already been observed - is scaled back to the command tier. Verified locally: debug-session 14 passing (31s), debug-exceptions 10 passing (22s), rundebug-commands 12 passing (96s, building its own fixtures). debug-advanced still fails its two hot-reload tests locally with netcoredbg reporting 0x80004001 (E_NOTIMPL) for `applyDeltas`; that reproduces with coverage off and was green on Windows CI, so it is tracked separately and is not a timeout question.
The sharded run came back with one failure: attach-by-name on Windows, where startDebugging refused to attach to 'StepTarget'. The attach suite resolves debuggees BY NAME, and its teardown signalled SIGKILL and moved on. Killing is asynchronous, so the next test spawned a second StepTarget while the previous one was still dying, and a name that must resolve to exactly one pid briefly matched two. The teardown now waits until each child is actually gone. `startOutsideDebugger` also polled 120s for the debuggee's first output inside tests capped at DEBUG_SESSION_MS, so that budget could never elapse - mocha killed the test first and the poll's own message never printed. It now names PROCESS_START_MS, a tier that sits below the ceiling containing it. Verified locally: attaching by process name passes in 1530ms. For the record, the run this fixes went from seven failing Ubuntu chunks to one failing Windows chunk. All 26 Ubuntu shards passed, the slowest in 271s against the 50 minutes the unsharded job used to take, and the whole PR pipeline finished in 21 minutes.
Two failures from the sharded run, both caused by sharding changing an assumption rather than by a budget being too small. Unsharded, the whole VS Code suite ran in ONE extension host, so exactly one test in the entire run ever paid Roslyn's cold project load. Every chunk now gets a fresh host, so the first semantic test in each chunk pays it - which is how a single lsp-refactor test hit its 20s ceiling on Windows while 107 siblings passed. `activateRealSharpLsp` only proves the LanguageClient reached State.Running, which is the JSON-RPC connection and not a loaded project. `openFixtureDocument` now blocks until Roslyn actually answers, so the cold load is paid once in suiteSetup where it belongs and LSP_RESPONSE_MS - "one semantic request answered by a WARM sidecar" - is an honest ceiling for the tests that follow. The probe is a code action deliberately: `documentSymbol` is answered by tree-sitter in the Rust host for C# and never reaches the sidecar, so it would return instantly against a completely cold Roslyn. `stopAnyDebugSession` then waited the full DEBUG_SESSION_MS for a terminate event that had already fired. It reads `activeDebugSession` and only then attaches its listener, so a session terminating in that window - exactly what a debuggee already paused on an exception does - is never observed and the wait can never succeed. Two observations prove the session is gone; it now accepts either. Verified locally: lsp-refactor 108 passing (45s, unchanged), fsharp-rename 48 passing, debug-exceptions 10 passing, debug-session 14 passing, rundebug-commands 12 passing. The run's third failure needs no code: fsharp-codefix passed all 47 tests and the job failed uploading its coverage artifact with ETIMEDOUT.
Two Windows chunks failed the last run; all 28 Ubuntu jobs passed. The hover symbol sweep exceeded LSP_SWEEP_MS. That tier was measured at 31.9s against a WARM sidecar, which is what it always used to face: unsharded, some earlier suite had invariably warmed Roslyn first. On a fresh shard the sweep pays the cold project load itself. `setupLspTestSuite` does not prevent that - it polls `documentSymbol`, and for C# the Rust host answers that from tree-sitter in single-digit milliseconds without the sidecar ever seeing it, so it reports "ready" against a completely cold Roslyn. The hover suite now warms on a workspace fixture in its suiteSetup, so the load is paid once per suite and LSP_SWEEP_MS describes warm work. `warmSemanticEngine` is deliberately NOT wired into `setupLspTestSuite`: that helper probes a file it writes to a temp directory, which belongs to no project, so Roslyn offers nothing there and the warm-up could never succeed. Tried, measured hanging a chunk for its whole budget, reverted. It belongs wherever the suite has a file that is genuinely in the loaded project - `openFixtureDocument` for the refactor suites, and the workspace fixture here. The async call-stack test read the stack ONCE and asserted the logical chain was already reconstructed. The DapRouter and the C# sidecar do that reconstruction, so a `stackTrace` answered before the sidecar has the debuggee's project loaded forwards netcoredbg's physical frames untouched - and the resulting "Frames reported: LeafAsync" is indistinguishable from the reconstruction being broken. It now waits for the chain, and the poll names the frames it actually saw. Verified locally: lsp 65 passing (98s), explorer 173 passing (45s), debug-stepping 15 passing (28s).
…form I got the warm-up wrong twice and both mistakes are worth recording. First I wired it into `setupLspTestSuite`, which probes a file it writes to a temp directory. That file belongs to no project, so Roslyn offers nothing there and the warm-up could never succeed: it hung a chunk for its whole budget. Reverted before pushing. Then I wired it into `openFixtureDocument` and assumed Roslyn offers a code action at the head of any loaded C# file. The cross-language rename fixtures disprove that - they are loaded and offer nothing at line 0 - so `fsharp-rename` waited out the full budget and failed on files that were never broken. Scoping the probe to `.cs` did not help, because the file it failed on IS C#. "Roslyn has loaded the project" and "Roslyn offers an action at line 0 of this file" are not the same claim, and only the second is observable. So the warm-up is now an explicit per-suite call on a fixture that has been checked - the four lsp-refactor suites and the hover suite - rather than a blanket rule in a shared opener. A warm-up that can fail on a healthy file is worse than no warm-up. The attach suite already decided how to treat netcoredbg's broken attach stack walk: skip, because it is not SharpLsp's defect. That guard only covered the walk RETURNING unwalkable frames. On the Windows runner the defect instead fails the `stackTrace` request outright with 0x80070057, which threw straight past it. Same defect, same documented decision - the guard now sees both forms, and every other error still propagates. Verified locally: fsharp-rename 48 passing (130s), lsp-refactor 108 passing (46s), lsp 65 passing (98s), debug-advanced attach tests passing.
The F# generation suite waited for a diagnostic carrying the expected CODE and then asserted that one of them intersects the range under test. Those are different conditions, and FCS wins the gap regularly: it publishes a diagnostic with that code elsewhere in the file first, the wait succeeds on a set that has not yet reached the range, and the assertion fails on timing rather than on behaviour. `diagnosticWithCode` now takes the range, so the wait is exactly as strong as the assertion that follows it. A wait weaker than its assertion is a race with extra steps. Verified locally: fsharp-codefix 47 passing (30s). The run this fixes had all 28 Ubuntu jobs green and 24 of 25 Windows jobs green, with this as the only failure.
…efused Ubuntu is green - 28 of 28 jobs, twice running. Both remaining failures are Windows, and both were unreadable rather than merely red. 63 tests across 19 files declared `this.timeout(DEBUG_SESSION_MS)` and then awaited kit helpers - waitForStops, stepAndStop, stopAnyDebugSession - that poll for exactly the same budget. Mocha therefore fires first, every time, so every debug failure reads "Timeout of 45000ms exceeded" instead of the kit's own "the debuggee must stop 1 time(s); it stopped 0. Stops seen: []". That is the inversion [DIST-CI-VSIX-SHARDS-TIMEOUTS] forbids, and it is why the debug-exceptions failure told us nothing about itself. DEBUG_TEST_MS sits above the poll tier and keeps the arithmetic in the one file allowed to do it. This does NOT claim to fix why that test exceeded 45s on Windows. It makes the next occurrence say what it was waiting for, which is the prerequisite for fixing it rather than guessing at a bigger number. `vscode.debug.startDebugging` answers a name attach with a bare boolean, so "no process by that name" and "that name matches two processes" are indistinguishable in the failure - and telling those apart is precisely what the test exists to do. `resolveAttachTarget` already reports which, and is exported, so the test now asks it first and quotes its reason. Verified locally: debug-exceptions 10 passing (22s), debug-stepping 15 passing (28s), debug-advanced attach tests passing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Aggregate of the coordinated
fixesbranch work (SharpLsp1/2/3 + audits).Areas
debugchunk 21/21 green. New DapRouter (dap-router.ts+dap-frames.ts) reconstructing async stacks.as_chunks::<5>(), sharedtoken_from_record).verify-vsix-payload.mjs), netcoredbg fetch fix, main.mk target fixes, website mermaid test + contributing docs, spec updates (DEBUGGING, RIDER, PROFILER, NUGET-BROWSER, SOLUTION-EXPLORER, ARCHITECTURE).Rules honored
Status