Add Language Server Protocol support via mdsmith lsp subcommand - #236
Merged
Conversation
Drafts docs/guides/editors/vscode.md covering install, settings, code actions, configuration discovery, the diagnostic-to-LSP mapping, troubleshooting, and the performance benchmark invocation. Documents the forthcoming `mdsmith lsp` subcommand and VS Code extension ahead of implementation so the user-facing contract is reviewable while the server and client land. Marks plan 121 as in progress (🔳) and refreshes the guides catalog plus the CLAUDE.md and AGENTS.md includes that mirror it. https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
Adds internal/lsp, a hand-rolled LSP server that speaks JSON-RPC 2.0 over stdio with no external dependencies. Wires the existing engine.Runner.RunSource pipeline to publish diagnostics on textDocument/didOpen and textDocument/didChange, clears them on didClose, and handles textDocument/codeAction with two action kinds: quickfix (per fixable diagnostic) and source.fixAll.mdsmith (whole-file fix). Also adds an in-memory fix entry point (internal/fix/FixSource and FixSourceWithRules) that the code-action path uses to compute the fixed buffer without touching disk. Registers `mdsmith lsp` as a subcommand. Tests cover the initialize handshake, didOpen → publishDiagnostics, didClose clearing, and the shutdown response shape. https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
Wires the remaining pieces of plan 121 end-to-end and marks acceptance criteria as met. LSP server - internal/lsp/bench_test.go: p95 latency benchmark on 1k and 5k synthetic line documents; budgets are 150 ms and 500 ms respectively. Local p95 measures ~2 ms / ~9 ms with plenty of headroom. - internal/lsp/server_test.go: tests for didChange, per-rule quickfix, source.fixAll.mdsmith, and unknown-method error handling. - internal/fix/source_test.go: pins the FixSource output to the on-disk Fixer.Fix bytes (acceptance criterion). - internal/fix: rename FixSource → Source and FixSourceWithRules → SourceWithRules to drop package-name stutter. CLI - cmd/mdsmith/lsp_test.go: end-to-end subprocess test driving `go run ./cmd/mdsmith lsp` over a pipe. VS Code extension - editors/vscode/: package.json, tsconfig.json, esbuild.js, src/extension.ts, README.md, .gitignore, .vscodeignore. - The extension spawns `mdsmith lsp`, surfaces fixOnSave via source.fixAll.mdsmith, and shows a Download / Settings prompt when the binary is missing. CI - ci.yml: lsp-bench job runs the latency benchmark on every PR; vscode-extension job builds and packages the .vsix as an artifact. - release.yml: vscode job packages mdsmith-<version>.vsix alongside the Go binaries on tag pushes. Docs - docs/reference/cli/lsp.md: new CLI reference page. - docs/background/markdown-linters.md: VS Code row flipped from "no" to "yes (LSP)". - README.md / docs/reference/cli.md / CLAUDE.md / AGENTS.md: catalogs auto-refreshed. Config - .mdsmith.yml: editors/** added to directory-structure.allowed; node_modules and dist added to ignore. Plan - plan/121_vscode-integration.md: status flipped to ✅, acceptance checkboxes ticked, design note amended to record that the server is hand-rolled rather than built on go.lsp.dev. https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #236 +/- ##
==========================================
+ Coverage 94.92% 95.14% +0.22%
==========================================
Files 144 151 +7
Lines 17148 18020 +872
==========================================
+ Hits 16278 17146 +868
- Misses 531 533 +2
- Partials 339 341 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds first-class editor integration by introducing an mdsmith lsp subcommand plus a minimal in-repo LSP implementation and a VS Code extension that consumes it. This extends mdsmith’s existing lint/fix pipelines into real-time diagnostics and code actions in LSP-capable editors.
Changes:
- Implement a JSON-RPC-over-stdio LSP server under
internal/lsp/with diagnostics + code actions. - Add in-memory fix APIs (
internal/fix/source.go) to power LSP quick-fixes and fix-all. - Add a VS Code extension (
editors/vscode/), plus docs and CI jobs to build/package and benchmark latency.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Adds lsp to the top-level CLI command table. |
| plan/121_vscode-integration.md | Marks plan 121 complete and updates design notes. |
| PLAN.md | Marks plan 121 as ✅ in the catalog table. |
| internal/lsp/transport.go | Implements LSP frame (Content-Length) transport over stdio. |
| internal/lsp/server.go | Core LSP server: lifecycle, sync, diagnostics, code actions, config watching. |
| internal/lsp/server_test.go | Unit-level harness + tests for LSP request/notification behavior. |
| internal/lsp/protocol.go | Minimal JSON-RPC + LSP type definitions used by server/tests. |
| internal/lsp/documents.go | Thread-safe store for open documents. |
| internal/lsp/diagnostics.go | Maps mdsmith diagnostics to LSP diagnostics with UTF-16 positioning. |
| internal/lsp/bench_test.go | Benchmarks didChange→publishDiagnostics latency budgets. |
| internal/fix/source.go | Adds in-memory fix entry points (whole-file and per-rule). |
| internal/fix/source_test.go | Tests that in-memory fixes match on-disk fixer behavior. |
| editors/vscode/tsconfig.json | TypeScript compiler settings for the extension. |
| editors/vscode/src/extension.ts | VS Code client wiring to spawn mdsmith lsp and provide fix-on-save. |
| editors/vscode/README.md | Extension README with install + settings overview. |
| editors/vscode/package.json | Extension manifest, contributed settings, build scripts, deps. |
| editors/vscode/esbuild.js | Bundling script to produce the VS Code extension artifact. |
| editors/vscode/.vscodeignore | Packaging exclusions for the .vsix. |
| editors/vscode/.gitignore | Ignores build artifacts and node_modules for the extension. |
| docs/reference/cli/lsp.md | CLI reference doc for mdsmith lsp. |
| docs/reference/cli.md | Adds lsp to the CLI reference command table. |
| docs/guides/index.md | Adds VS Code integration guide to the guides catalog. |
| docs/guides/editors/vscode.md | Full VS Code integration guide (settings, actions, troubleshooting, perf). |
| docs/background/markdown-linters.md | Updates comparison table to indicate VS Code support via LSP. |
| cmd/mdsmith/main.go | Registers lsp subcommand in CLI dispatch + usage text. |
| cmd/mdsmith/lsp.go | Implements mdsmith lsp CLI entrypoint (stdio server run + signals). |
| cmd/mdsmith/lsp_test.go | Subprocess integration test for mdsmith lsp over pipes. |
| CLAUDE.md | Catalog entries for the new VS Code guide + lsp CLI doc. |
| AGENTS.md | Catalog entries for the new VS Code guide + lsp CLI doc. |
| .mdsmith.yml | Allows editors/** and ignores extension build artifacts. |
| .github/workflows/release.yml | Adds a job to build/package the VS Code .vsix on release. |
| .github/workflows/ci.yml | Adds LSP benchmark job + VS Code extension build/package job. |
| .github/copilot-instructions.md | Catalog entries for the new VS Code guide + lsp CLI doc. |
Substantive bug fixes from the Copilot review:
- Route JSON-RPC responses separately (server.go). Frames
with id and no method are responses to server-initiated
requests; the previous code treated them as method-not-
found errors, which broke the workspace/configuration
and client/registerCapability reply flows.
- Honor mdsmith.run in scheduleLint. The setting was
declared but never consulted, so onSave/off were
effectively onType. didChange now skips when run=onSave;
off skips entirely; didOpen/didSave/config-change
always lint regardless of mode.
- Consume the workspace/configuration response in
fetchClientSettings so mdsmith.config and mdsmith.run
actually take effect. The settings-fetch goroutine
registers a pending-response channel, awaits the reply,
and updates s.settings under the existing mutex.
- Honor codeAction Context.Only. computeCodeActions now
short-circuits kinds the client did not request, so
source.fixAll-only requests no longer run per-rule
fix passes whose output the client would discard.
- Fix fullFileEdit's end position. The previous range
used {Line: lineCount, Character: lastLineLen} which
is invalid for documents that end with a newline.
documentEndPosition now returns {Line: lineCount,
Character: 0} for newline-terminated files, matching
the LSP convention for end-of-document edits.
Test improvements:
- internal/lsp test harness: rewrote with one dedicated
reader goroutine that demuxes frames into channels.
The previous design spawned a new reader per
awaitNotification iteration, which raced on the shared
bufio.Reader and produced rare deadlocks under
parallel runs.
- internal/lsp/bench_test.go: replaced the synthetic
testing.T with a benchmark-native harness.
awaitDiagnostics now b.Fatalf's on timeout instead of
silently returning, so a stuck server fails the
benchmark fast.
- internal/fix/source_test.go: assert Fixer.Fix had no
errors and modified the file before comparing on-disk
vs in-memory output.
- internal/lsp/documents.go: clarify that get() returns
a shallow copy whose text slice still aliases — both
copies share the underlying byte array.
New tests covering previously uncovered branches:
- TestInitializedTriggersRegistration:
workspace/configuration + client/registerCapability
fire from handleInitialized.
- TestDidChangeWatchedFiles{Relints,Ignores}: re-lint
on .mdsmith.yml change; no-op on unrelated files.
- TestDidChangeConfigurationRelintsOpenDocs: settings
refresh + re-lint.
- TestDebouncedLintCollapsesRapidChanges: debounce
collapses N didChanges into one publish.
- TestDidSaveLintsWhenRunOnSave / TestRunOffSuppressesLint:
the new run-mode behavior.
- TestCodeActionOnlyFiltersOutQuickFix: Only filter.
- TestReloadConfig{EmptyRoot,DiscoverInTempDir,Override*}:
config discovery and override paths.
- TestDocumentEndPosition{Trailing,No,Empty}: the
end-position math behind fullFileEdit.
- TestUriToPathRoundTrip / TestPickRoot* /
TestIsWholeFileOnly / TestIsFixableUsesRegistry /
TestWantsKind / TestDocumentStoreOpenURIs.
CLI:
- runLSP split into runLSPWith for testability;
end-to-end test in lsp_unit_test.go drives the CLI
entry point through in-memory pipes.
- The subprocess test in lsp_test.go now replies to
workspace/configuration with run=onType so the
didChange flow it exercises actually triggers a lint
pass under the new run-mode semantics.
Capabilities:
- Advertise textDocumentSync.save so VS Code reports
didSave events to the server.
Removed dead code: transport.readMessage and
Server.String were unused after the refactor.
Local LSP package coverage rose from 61% to 83%.
https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
Adds focused tests for previously uncovered branches in internal/lsp: - TestRegisterWatchersWritesRequest pins the registerWatchers wire format directly via a captured Writer, replacing the parallel-flaky integration test that drove handleInitialized through the harness. - TestFetchClientSettingsAppliesResponse and TestFetchClientSettingsIgnoresErrorResponse exercise the response-routing path: a synthetic deliverResponse unblocks the goroutine and the parsed values land in s.settings under the existing mutex. - TestHandleDidChangeConfigurationRelintsOpenDocs: rewritten as a direct unit test of the handler against a captured Writer. The previous version raced the server-spawned fetchClientSettings goroutine. - TestSeverityForMappings, TestCurrentLineOutOfRange, TestSplitLinesEmpty, TestUtf16ColumnSurrogatePair, TestFrontMatterEnabledExplicit: per-helper unit tests. - TestDispatchRawIgnoresInvalidJSON, TestDispatchRawRejectsWrongVersion: the request/ response routing entry point. - TestRunModeFallsBackOnUnknown, TestQuickFixForRejects*, TestRunLintIgnoredFile, TestRunLintMissingDoc: scheduling and code-action edge cases. - TestHandleDid*InvalidJSON / *UnknownURI / *EmptyContentChanges: the silent-return paths every document-sync handler takes on malformed inputs. - TestDocumentStoreGetMissing, TestUnregisterPendingResponseClearsSlot, TestDeliverResponseUnknownIDIsNoOp: state-store invariants. Subprocess test stabilization (cmd/mdsmith/lsp_test.go): - Build the binary once via `go build -o tmp/mdsmith` instead of `go run` per invocation. The previous version paid Go compilation latency on every spawn, which under parallel load consumed the per-step deadline and produced 120-second timeouts. - Each awaitDiagnostics call gets its own 30-second deadline so a slow first step doesn't starve the next. - Subprocess timeout bumped from 60s to 120s for headroom when `go test ./...` runs the entire suite in parallel. LSP package coverage went from 83% to 90%; cmd/mdsmith test runtime stays under 7 seconds in the steady state. https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
internal/lsp/transport_test.go: locks the framing contract — missing Content-Length, invalid integer, out-of-bounds value, truncated body, valid frame round-trip — plus the JSON encode/decode error paths on writeJSON, writeResponse, writeError, writeNotification, and writeRequest. Coverage of transport.go went from 68% to ~95%. server_test.go: TestHandleInitializedRunsConfigAndWatchers now polls for the workspace/configuration write rather than racing the goroutine. Added unit-test coverage for handleInitialize (empty + malformed params), handleCodeAction (unknown doc + invalid JSON), quickFixFor invalid path, dispatch on $/* notifications, and Run-on-context-cancel. cmd/mdsmith/lsp_test.go: switch the subprocess test to the shared binaryPath built by TestMain in e2e_test.go. The binary is compiled with -cover -covermode=atomic so the spawned `mdsmith lsp` execution counts toward the merged coverage profile in CI. This finally lets coverage from cmd/mdsmith/lsp.go's runLSP body land in the report. Local LSP coverage rose from 90% to 93%. https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
Code fixes:
- diagnostics.go: drop the dead first loop in
utf16Column. The result was already produced by the
second loop; the first one only incremented `idx`
with no observable effect on the diagnostics hot
path.
- cmd/mdsmith/lsp.go: treat context.Canceled as a
clean exit. SIGINT/SIGTERM cancel ctx, so srv.Run
returns context.Canceled — printing it as an error
and exiting 2 made graceful shutdowns look like
failures.
- server.go handleDidChangeConfiguration: stop
scheduling lint passes synchronously. The new
settings/config land asynchronously inside
fetchClientSettings; running lint before then
publishes diagnostics with stale config. Move the
per-document re-lint into fetchClientSettings's
success path so the published diagnostics always
reflect the post-fetch state.
- server.go handleDidOpen: clarify the lint comment.
The prior "always lints regardless of run setting"
claim was wrong — `run=off` skips even open events,
by design.
Doc fixes:
- cli/lsp.md and guides/editors/vscode.md: discovery
is workspace-wide (initialize.rootUri), not
per-document. Updated both pages to describe what
the implementation actually does.
- cli/lsp.md capabilities table: re-lint on change is
conditional on mdsmith.run, not unconditional.
Added a `mdsmith.run` summary block listing the
three modes and their lint triggers.
- guides/editors/vscode.md: `data` carries `{rule}`,
not the mdsmith `explanation` field. Re-lint on
watched-file change is immediate, not deferred to
the next edit/focus.
Tests:
- TestHandleDidChangeConfigurationRelintsOpenDocs:
now drives the workspace/configuration response
synchronously, since the post-fetch re-lint is the
thing being checked.
- cmd/mdsmith/lsp_test.go: awaitClearedDiagnostics
drains stale publishDiagnostics frames so the
fetchClientSettings re-lint can race with
didChange without flaking the assertion.
https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
Real bugs: - scheduleLint: a debounced timer that armed before shutdown/exit could fire afterward and publish stale diagnostics during teardown. The time.AfterFunc callback now re-checks s.shutdown before running runLint, and the shutdown/exit dispatch handlers call a new stopPendingLints() that cancels every armed timer and clears the pending map. - computeCodeActions used to call quickFixFor once per diagnostic, and quickFixFor ran a fresh fix.SourceWithRules pass each time. On a file with N MDS006 diagnostics that meant N full fix passes per codeAction request, blowing the latency budget. The new path runs one fix.SourceWithRules call per distinct rule and reuses the resulting WorkspaceEdit across every diagnostic carrying that rule. quickFixFor was renamed to quickFixEditFor (returning *workspaceEdit, no bool) since the calling pattern is now rule-keyed. - Quick-fix titles now read "Fix all <rule> with mdsmith". The edit replaces the entire document with the output of running just that rule, so it covers every occurrence — the old "Fix <rule> with mdsmith" wording implied a range-scoped change that mdsmith's whole-file fix pipeline cannot produce. Docs: - vscode.md and cli/lsp.md: configuration discovery walks up from the workspace root to a `.git` boundary (matches `config.Discover`). The previous text said "workspace root" without acknowledging the upward walk. - vscode.md and cli/lsp.md "Code actions" sections: describe the new whole-rule scope, the per-request dedup, and the rule exclusion list explicitly rather than implying range-scoped edits. - vscode.md troubleshooting: rewrote the "config edits do not take effect" entry now that in-workspace edits re-lint immediately. Tests: - TestComputeCodeActionsDedupesPerRule pins the invariant that N diagnostics from the same rule produce one WorkspaceEdit (asserted via pointer-identity). - TestQuickFixEditForRejectsWholeFileRule, TestQuickFixEditForUnknownRule, and TestQuickFixEditForNoOpReturnsNil replace the earlier quickFixFor tests. - TestRunLSPHelpFlag covers the Usage callback body. https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
Targeted tests for previously uncovered branches so the patch-coverage threshold stops failing: - TestDispatchRawRoutesResponseToWaiter: response routing through dispatchRaw (the unique path that delivered responses to fetchClientSettings). - TestDispatchRawRejectsWrongVersionWithID: writeError branch when an id-bearing frame uses jsonrpc < 2.0. - TestScheduleLintSkipsWhenShutdown, TestScheduleLintOnSaveSkipsChange, TestStopPendingLintsCancelsTimers: scheduleLint's shutdown / runOnSave / cancel-pending paths. - TestRunModeFallsBackOnEmpty: empty-string fallback branch in runMode. - TestFetchClientSettingsHandlesEmptyArray / HandlesMalformedResult / HonorsContextCancel: the three "no settings landed" exits. - TestComputeCodeActionsSkipsDiagnosticsWithoutData / CachesNilEdits: the early-skip and cache-miss branches in the per-rule dedup loop. - TestToLSPClampsZeroLine: the startLine clamp branch. - TestFixSourceWithRulesAcceptsZeroMaxBytes: the default-fallback for SourceOptions.MaxInputBytes. - TestRunLSPRunFailurePrintsStderr: the runLSPWith error-print branch via a synthetic failingReader. Local coverage: internal/lsp 94.6% → 97.0%; cmd/mdsmith 64.2% → 64.3% (most cmd/mdsmith lines already covered). https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
Real bugs:
- internal/fix/source.go: a nil opts.Config used to
panic inside Fixer.prepareFile because that path
derefs Config via ValidateFrontMatterKinds. Treat
nil as the default config so callers can pass a
zero-value Options without crashing the fix
pipeline.
- internal/engine/runner.go RunSource:
in-memory linting (LSP buffers, formerly stdin)
used to leave lint.File.FS nil, so default-enabled
rules that consult FS (include, catalog) silently
skipped. Added a Runner.SourceFS field that
RunSource wires onto the file along with a
GitignoreFunc rooted at RootDir, mirroring what
processFile sets up for on-disk runs. Stdin
callers (CLI) leave SourceFS nil and behave
unchanged.
- internal/lsp/server.go runLint: the LSP path comes
from file:// URIs and was passed verbatim to the
engine. Config glob matching expects
workspace-relative paths ("docs/foo.md"), so an
absolute path made `**/docs/**` ignore globs and
override entries miss. runLint now normalizes via
filepath.Rel against the workspace root before
calling RunSource, while passing the absolute
directory to dirFSForPath so include/catalog still
see the right filesystem view.
- internal/lsp/server.go scheduleLint: the
time.AfterFunc closure used to call
delete(s.pending, uri) unconditionally, which
could remove the *new* timer when an old timer's
firing raced a fresh scheduleLint. The closure
now captures its own *time.Timer and only deletes
the map entry when it still points to that timer.
Protocol corrections:
- internal/lsp/server.go dispatchRaw: malformed JSON
used to be silently dropped, leaving clients
hanging on a request whose reply never came.
dispatchRaw now emits a JSON-RPC 2.0 §5.1 parse
error (-32700, id: null) on bad input.
- handleInitialize / handleCodeAction: bad params
in well-formed JSON now return -32602 (Invalid
params), matching the JSON-RPC spec, rather than
-32700 (Parse error). Added the codeInvalidParams
constant in protocol.go.
Tests:
- TestDispatchRawInvalidJSONRespondsWithParseError
pins the new parse-error reply.
- TestHandleInitializeMalformedReturnsInvalidParams
and TestHandleCodeActionMalformedReturnsInvalidParams
pin the -32602 mapping.
- TestWorkspaceRelativePathHandling and
TestDirFSForPathRelativeIsNil cover the path
normalization helpers.
- TestScheduleLintTimerRaceLeavesNewTimer pins the
identity-checked replacement.
- TestFixSourceNilConfigUsesDefaults pins the
nil-Config path.
https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
Targeted coverage for the four largest remaining gaps: - internal/engine: TestRunSource_WiresSourceFSAndGitignore pins the new SourceFS+RootDir branch via a fileSnapRule that captures the lint.File pointer and asserts FS and GitignoreFunc are wired before Check is called. - internal/fix: TestFixSourcePropagatesPrepareError triggers ValidateFrontMatterKinds on an undeclared kind name so the prepareFile error path produces a surfaced error rather than a silent crash. - internal/lsp: TestWriteJSONBodyWriteFails uses a writer whose second Write fails to drive transport's body-write error branch (the first Write absorbs the Content-Length header). Local coverage: engine 95.2% → 96.2%, fix 91.6% → 92.1%, lsp 97.0% → 97.3%. https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
Three small but real fixes: - diagnostics.toLSP: empty-line diagnostics used to produce a range with End.Character=1 (startCol+1) even though the line had length 0. The end now derives from the line's actual UTF-16 length, with a fallback to startCol when that would be smaller, so empty lines emit a zero-width range instead of one whose end lies past the line. - diagnostics.utf16Column: utf16.RuneLen returns -1 for unpaired surrogates and other invalid code points. We were summing that into `units`, which could go negative for adversarial input. Treat invalid runes as a single UTF-16 unit so the returned column is always non-negative. - engine.RunSource doc comment: said GitignoreFunc was wired against "the SourceFS's directory when no rootDir is set", but the implementation only wires it when RootDir is set. Updated the comment to match what the code actually does (gitignore is rooted at RootDir; SourceFS without RootDir leaves gitignore unconfigured). Tests: - TestToLSPEmptyLineProducesZeroWidthRange pins the empty-line range invariant. - TestUtf16ColumnTreatsInvalidRunesAsOneUnit pins the non-negative-result guarantee on invalid runes. https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
Every subcommand had its own copy-pasted "if err := fs.Parse... return 2" block, and most of them silently exited 2 because pflag's ContinueOnError mode does not write to fs.Output(). Some also returned 2 on --help, which was the second half of the same bug — pflag returns flag.ErrHelp from Parse after printing usage, and "exit 2 after success" surprises any tool that relies on POSIX-style help conventions. New helper in cmd/mdsmith/main.go: reportFlagParseErr(err, stderr, prefix) int translates a Parse result into the canonical CLI exit code: nil → -1 (caller continues), flag.ErrHelp → 0, anything else → 2 with `<prefix>: <err>` on stderr. Call sites converted: runCheck, runFix, runQuery (via parseQueryFlags), runInit, runKinds(list/show/path/resolve/why), runMetricsList, runMetricsRank. Existing e2e tests pinned the wrong "--help exits 2" behavior on five subcommands (Check, Fix, MetricsRank, MetricsList, Init, Query); flipped them to assert exit 0 with a comment explaining the pflag.ErrHelp contract. Also closes the gap that question (b) of the round-19 LSP review surfaced: the LSP e2e test was spawning `mdsmith lsp` without `--stdio`, so it never exercised the flag vscode-languageclient actually passes. lsp_test.go now spawns with `lsp --stdio` so a future regression in runLSP's flag set fails the e2e instead of slipping through. https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
The original LSP wiring excluded catalog/toc/include/toc-directive
from per-diagnostic quick-fix actions, with the rationale that
their fixes "invite partial regenerations." In practice this
meant when a user clicked the lightbulb on a stale catalog
diagnostic in VS Code, the only entries shown in the Quick Fix
menu were the AI extension's "Fix" / "Explain" — mdsmith
disappeared from the surface where users naturally look for it.
Two corrections:
1. The "partial regeneration" worry was already addressed by the
action title ("Fix all <rule> with mdsmith") and by the
whole-file workspaceEdit shape every rule produces. Generated-
section rules regenerate the section's body; that's exactly
what users want when they click the squiggle.
2. The exclusion meant the user's expected workflow ("squiggle →
Quick Fix → run mdsmith fix") simply didn't work for the rules
most associated with mdsmith's identity (catalog/toc/include).
Removed isWholeFileOnly + its caller guard in quickFixEditFor,
plus the now-stale TestIsWholeFileOnly. Added
TestQuickFixEditForCatalogProducesEdit pinning the new behavior:
catalog must produce a non-nil WorkspaceEdit so the action shows
up in the lightbulb. Updated the vscode guide to explain the new
contract (generated-section rules regenerate the block, scope
is whole-file, and the title says so) and rewrote the
"Quick fix does nothing" troubleshooting entry now that the
"some rules don't expose quick fixes" half is no longer true.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
Two correctness fixes flagged in the latest Copilot review:
- internal/lsp/server.go scheduleLint timer callback: the
identity check protected the pending-map entry but still
called runLint(uri) when the timer was stale (i.e. a racing
scheduleLint had already replaced us). A timer that lost the
Stop() race could fire and emit stale diagnostics on top of
fresher ones. Skip runLint entirely when pending[uri] !=
timer; the newer timer (or its inline runLint when
debounce==0) is responsible for the next publish.
- internal/lsp/diagnostics.go splitLines: the doc claimed the
result indexes the same as lint.File.Lines (which is
bytes.Split-based and yields a 1-element slice for empty
input), but the implementation returned nil for empty input.
A diagnostic anchored at line 1 of an empty buffer therefore
fell through currentLineBytes's out-of-range guard and
silently clamped to the wrong column. Return [][]byte{nil}
for empty input to match lint.File.Lines and updated
TestSplitLines (and added an explicit []byte{} case).
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
Eight threads addressed in one batch: - internal/lsp/protocol.go: add Version to publishDiagnostics so clients can drop stale results when overlapping lint runs publish out of order. internal/lsp/server.go runLint now passes doc.version through. - editors/vscode/src/extension.ts: track the .mdsmith.yml file watcher in a module-level handle, dispose it on restart and on deactivate, and push it onto context.subscriptions so the watcher cannot leak across LSP-client restarts. - internal/lsp/server.go fetchClientSettings: switch the response shape to a new clientSettings struct with *string fields. Empty pointer means "client did not supply" (cached default stays); non-nil empty string means "client cleared the setting". Users can now revert mdsmith.config back to "" without restarting the editor. - internal/lsp/server.go handleInitialize/Initialized: capture the client's advertised capabilities and gate the optional follow-up requests on them. Without workspace.configuration we skip workspace/configuration; without didChangeWatchedFiles.dynamicRegistration we skip client/registerCapability. Empty-cap clients (Helix / JetBrains-LSP defaults) no longer log spurious errors. - internal/lsp/server_test.go: replace fixed time.Sleep(50ms) waits in the fetchClientSettings tests with a deadline-based deliverPendingResponse + awaitDone helper pair, so the tests are no longer flake-prone under load. Added TestHandleInitializedSkipsWhenCapabilitiesMissing to pin the new caps-gating behavior. - cmd/mdsmith/lsp_test.go: the e2e LSP test relied on the server requesting workspace/configuration to flip run=onType so didChange would lint. With caps gating in place, the empty-capabilities initialize no longer produces that flow; updated the harness to send a realistic capabilities object (workspace.configuration + dynamic file watcher registration) so the round-trip path is exercised. - docs/reference/cli/lsp.md: drop the stale "catalog/toc/include excluded from quick fixes" claim — that exclusion was removed in the previous commit and the CLI doc was diverging from the VS Code guide. - .github/workflows/ci.yml + editors/vscode/tsconfig.json: add a `bunx tsc --noEmit` step in the vscode-extension job and enable skipLibCheck. Bun's bundler accepts code tsc would reject, so without this step a broken-but-bundleable type signature could ship in the .vsix. Caught two pre-existing issues in wiring.test.ts (ServerOptions union indexing, bun's strict toBe overloads) and fixed them. https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
The CLI doc claimed the subcommand takes no arguments, but the implementation (and `mdsmith lsp --help`) accepts --stdio as a no-op for LSP-client compatibility. Updated the synopsis from `mdsmith lsp` to `mdsmith lsp [--stdio]` and added a paragraph explaining why the flag is accepted (vscode-languageclient appends it whenever stdio transport is selected; rust-analyzer and typescript-language-server document the same convention). https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
Two correctness fixes flagged in the round-26 review: - editors/vscode/build.ts watch loop: the rebuild trigger only flipped `changed=true` when an already-seen file's mtime moved. Adding a brand-new `src/**/*.ts` file or deleting one wouldn't rebuild, since `prev` was undefined for new paths and removals weren't detected at all. Fixed: treat missing `prev` (newly appearing path) as a change, and walk the seen map after the scan to drop entries that no longer show up — those count as changes too. The watch mode is now responsive to add/remove/modify, not just modify. - internal/lsp/server.go fetchClientSettings: replaced `time.After(s.fetchTimeout)` with `time.NewTimer` + `defer Stop()`. fetchClientSettings runs on every workspace/didChangeConfiguration, so a fast-replying client was leaking one runtime timer per settings change until the full fetchTimeout elapsed. Stop releases the timer eagerly when the response (or ctx.Done) wins the select. https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
- internal/lsp/server.go: drop the dead Server.clock field. It was initialized to time.Now in New() but never read anywhere in production or test code. Removed both the struct field and the init line; nothing else needed to change. - editors/vscode/src/wiring.test.ts: remove a duplicated comment block in the buildClientOptions test that accidentally repeated "The same watcher object is forwarded ..." twice. Kept the second copy that included the `as unknown` cast rationale. https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
Three robustness/consistency fixes: - internal/engine/runner.go RunSource and internal/fix/source.go fixSourceImpl: align the in-memory oversize error string with the on-disk Fixer / processFile shape. Both on-disk paths wrap lint.ReadFileLimited's "file too large" via `reading %q: %w`, producing `reading "<path>": file too large (...)`. The in-memory guards used `<path>: file too large (...)` — the same fact formatted three different ways, which made editor / log output drift between the LSP and `mdsmith check`. Both in-memory paths now use the on-disk shape. Existing tests use `Contains(..., "file too large")` substring matching, so no test changes were needed. - editors/vscode/build.ts watch loop: wrap Bun.file(abs).stat() in try/catch. glob.scan yielded the path but a delete/rename can race between the yield and the stat call; an unhandled throw used to crash the watch process entirely. Now we treat the stat failure the same as "file vanished" — skip the iteration and let the deletion sweep over `seen` (added in round 26) pick the missing entry up next tick. https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
- internal/lsp/server.go runLint: re-check s.shutdown after engine.RunSource returns. RunSource is CPU-bound and can run for hundreds of milliseconds on large buffers; if the client sends shutdown/exit while we're busy, the lint pass should not race the dispatch loop's teardown and publish to a half-closed pipe. The check covers the three downstream writes (lint-error window/logMessage, surfaceForeignDiagnostics, publishDiagnostics) so a single Load() suffices. Adds TestRunLintSkipsPublishWhenShutdown- AfterLint pinning the new behaviour. - internal/lsp/server.go surfaceForeignDiagnostics: map lint.Severity to the matching window/logMessage MessageType via a new messageTypeForLint helper. Previously every foreign diagnostic was reported as messageTypeError, which drowned out warnings. Errors → 1, Warnings → 2 per LSP §3.18.1. Added TestSurfaceForeignDiagnosticsPreservesWarning- Severity and tightened the existing test to assert the default-severity Error path produces "type":1 too. https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
- internal/lsp/server.go scheduleLint: route the immediate-trigger path (open/save/config) through time.AfterFunc(0, ...) instead of calling runLint synchronously. The dispatch goroutine reads, decodes, and routes every incoming frame on one goroutine; a synchronous runLint blocked it from processing inbound responses (workspace/configuration replies, the response to client/registerCapability) until the lint finished. On large buffers that lint can run for hundreds of milliseconds and a slow lint could deadlock against fetchClientSettings' fetchTimeout. The unified path also reuses the existing identity-checked replacement: a racing newer scheduleLint Stop()'s the prior immediate timer just like it would for a debounce timer. Updated TestScheduleLintImmediateCancels- PendingDebounce to match — the test now asserts the prior timer is replaced (not removed) and waits for the immediate timer to drain. - editors/vscode/src/extension.ts startServer: clear `client` and dispose the config watcher when client.start() throws. A partially-started LanguageClient was lingering in the module-level handle, so a subsequent deactivate() / mdsmith.restartServer would call stop() on a client that never reached the running state — vscode-languageclient throws in that case. Now the catch block tears the half-built state down before returning to the user, and a fresh startServer() call gets a clean slate. - editors/vscode/src/extension.ts deactivate: wrap client.stop() in try/catch. Even with the startServer fix above, a host-driven deactivate that races a still-starting client could land in the same throw-from-stop hole. Swallowing the error keeps deactivate clean — dropping the client reference is enough to release the client object. https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
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
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.
Implements a complete Language Server Protocol (LSP) server that exposes mdsmith's lint and fix capabilities to editors like VS Code, Neovim, and Helix.
Summary
This PR adds an
mdsmith lspsubcommand that runs an LSP server over stdio, enabling real-time Markdown linting and quick fixes in LSP-aware editors. The implementation includes a hand-rolled JSON-RPC 2.0 transport layer, full LSP message handling, and a VS Code extension that spawns the server.Key Changes
Core LSP Server (
internal/lsp/)server.go: Main LSP server implementation handling lifecycle (initialize, shutdown), document synchronization (didOpen, didChange, didClose), diagnostics publishing, and code actionsprotocol.go: LSP message types and JSON-RPC 2.0 framing definitions (no external LSP library dependencies)transport.go: HTTP-style header framing for JSON-RPC messages with concurrent write serializationdocuments.go: Thread-safe document store for tracking open editor buffersdiagnostics.go: Conversion from mdsmith diagnostics to LSP format with UTF-16 position handlingserver_test.go: Comprehensive test harness and unit tests for server behaviorbench_test.go: Performance benchmarks enforcing p95 latency budgets (150ms for 1k lines, 500ms for 5k lines)CLI Integration
cmd/mdsmith/lsp.go: Entry point for thelspsubcommand with signal handlingcmd/mdsmith/main.goto register the new subcommandFix Pipeline Enhancement
internal/fix/source.go: NewSource()andSourceWithRules()functions for in-memory fix operations (used by LSP code actions)internal/fix/source_test.go: Tests ensuring LSP-side fixes match on-disk behaviorVS Code Extension (
editors/vscode/)src/extension.ts: TypeScript extension that spawnsmdsmith lspand configures the LSP clientpackage.json: Extension manifest with settings for binary path, config override, run mode (onType/onSave), and fix-on-saveesbuild.js,tsconfig.json) for bundling the extensionREADME.md: User-facing extension documentationDocumentation
docs/guides/editors/vscode.md: Comprehensive guide covering installation, settings, code actions, and performance considerationsdocs/reference/cli/lsp.md: CLI reference documenting capabilities and diagnostic mappingCI/CD
lsp-benchjob to GitHub Actions enforcing latency budgets on every commitvscodejob to release workflow for building and publishing the.vsixextension artifactImplementation Details
source.fixAll.mdsmithactions.mdsmith.ymlchangesThe server reuses the existing lint and fix pipelines, ensuring consistency between CLI and editor behavior.
https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C