Commit 5e36d99
Add Language Server Protocol support via
* docs(plan 121): add VS Code integration guide
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
* lsp: add minimal Language Server Protocol server
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
* plan 121: complete VS Code LSP integration
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
* lsp: address review feedback and lift coverage
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
* lsp: lift test coverage and stabilize subprocess test
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
* lsp: add transport unit tests + reuse e2e binary
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
* lsp: address Copilot review round 2
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
* lsp: address Copilot review round 3
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
* lsp: lift coverage to 97% on internal/lsp
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
* lsp: address Copilot review round 4
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
* lsp: cover SourceFS+gitignore wiring and body-write error
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
* lsp: address Copilot review round 5
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
* lsp: address Copilot review round 6
- scheduleLint dropped its unused ctx parameter. The
function had only `_ = ctx` at the end, which made
the API misleading and added noise at every call
site. Callers no longer need to forward ctx purely
for this hop.
- uriToPath now respects the URI host component:
- "localhost" maps to empty per RFC 8089 §3.
- On Windows, a non-empty/non-localhost host
produces a UNC path (\\server\share\...).
- On non-Windows, a remote host returns "" because
we have no way to mount the share; the caller
will skip the document instead of treating the
share as a local path.
- fullFileEdit's doc comment was wrong about the
end position. It claimed counting `after` lines;
the code uses documentEndPosition(before). Comment
rewritten to describe what the code actually does
and why (clients apply edits by replacing the
named range in their existing document).
Tests:
- TestUriToPathLocalhostHostIsTreatedAsEmpty
pins the RFC 8089 localhost equivalence.
- TestUriToPathRemoteHostRejectedOnUnix
pins the conservative "" return on Unix.
https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
* lsp: address Copilot review round 7
- splitLines: stop trimming trailing "\n" before the
split. The previous version dropped trailing empty
lines, which made currentLine() return "" for any
diagnostic anchored at len(f.Lines) (e.g.
single-trailing-newline emitting on the final
blank line). The fix matches lint.NewFile's
semantics — bytes.Split keeps trailing empties —
so toLSP can map EOF/blank-line diagnostics
without clamping to a position past the document.
- dispatchRaw: classify a frame as a response only
when result or error is present. Per JSON-RPC 2.0,
{jsonrpc:2.0, id:1} alone is an invalid request
and must get -32600. Without this guard a buggy
client could send such a frame and either get
silently dropped or have it misrouted to a
pending waiter.
- uriToPath: gate the "/C:/..." → "C:/..." fix-up on
runtime.GOOS == "windows" plus a real
drive-letter pattern. On non-Windows platforms a
legitimate absolute path whose third byte is ':'
(e.g. "/a:/tmp/file.md") used to be silently
rewritten into a relative path; now it stays
intact. Added hasDriveLetterPrefix to keep the
test for the pattern explicit and reusable.
Tests:
- TestSplitLines covers the new semantics including
trailing-newline preservation and CRLF stripping.
- TestDispatchRawIDOnlyFrameIsInvalidRequest pins
the -32600 reply.
- TestDispatchRawErrorOnlyResponseIsRouted verifies
error-only responses still route correctly.
- TestUriToPathLeavesNonDriveColonPathUnchanged
pins the fix on Unix, plus TestHasDriveLetterPrefix
for the helper itself.
https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
* lsp: address Copilot review round 8
Performance / hot-path fixes:
- computeCodeActions: replaced two `string(fixed) == string(doc.text)`
comparisons with bytes.Equal. The string conversions allocated and
copied the full document twice per codeAction request, which on
large files added noticeable latency to a path that already runs
one fix.Source pass per distinct rule.
- diagnostics.splitLines: now operates entirely on []byte. The
previous version round-tripped the entire document through string
on every diagnostics publish; bytes.Split + per-line CR trim
avoids the full-document copy and matches how lint.File.Lines is
built.
Config-glob parity with the CLI:
- computeCodeActions and quickFixEditFor now pass the
workspace-relative path to fix.Source / SourceWithRules. Absolute
file URIs were preventing repo-style globs (ignore, overrides,
kind-assignment) from matching, so LSP fixes could differ from
`mdsmith fix` on disk for the same file.
Error surfacing:
- internal/fix/source.go fixSourceImpl: settings errors from
Fixer.fixableRules used to be discarded (`fixable, _ := ...`).
They are now joined via errors.Join and returned, so callers can
decide whether to surface the failed code action / report it. LSP
callers already check for err != nil before producing an Edit, so
buggy settings now silently disable the affected quick fix
instead of running with a half-configured rule set.
Test cleanup:
- transport_test.itoaTransport now delegates to strconv.Itoa. The
hand-rolled five-digit decimal helper would have rendered
Content-Length incorrectly for any frame body ≥ 100 000 bytes —
no current test trips that, but it was a footgun for future
ones.
Tests: TestSplitLines also asserts CRLF behaviour now that the
function works directly on []byte.
https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
* lsp: lift coverage on dispatch arms and discover fallback
- TestDispatchRoutesInitializedToHandler,
TestDispatchRoutesDidChangeConfigurationToHandler,
TestDispatchRoutesDidChangeWatchedFilesToHandler:
exercise the dispatch case statements directly so
the routing arms count toward coverage. The
underlying handlers are already pinned by other
tests; these just walk the switch.
- TestReloadConfigDiscoverEmptyFallsBack: hits the
discover-returned-empty branch in reloadConfig by
pointing rootDir at a tempdir that has no
.mdsmith.yml ancestor.
Local lsp coverage rose 96.5% → 97.6%.
https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
* lsp: address Copilot review round 9
Two real bugs:
- internal/fix/source.go and Fixer: when callers pass
a workspace-relative Path (which the LSP server
does, so config glob matching uses repo-style
paths), Fixer.prepareFile used to derive the dirFS
via os.DirFS(filepath.Dir(path)) — i.e. resolved
against the process working directory. Editor
invocations would silently mis-locate
include/catalog neighbour files when launched from
outside the repo root.
Added Fixer.SourceFS plus SourceOptions.SourceFS
so callers can supply an explicit
fs.FS rooted at the document's real on-disk
directory. prepareFile uses it when set; the
disk-based Fix() loop leaves it nil and behaves
unchanged. The LSP server now passes
dirFSForPath(doc.path) alongside the relative
path it already computed.
- internal/lsp/server.go handleCodeAction: code
actions ignored cfg.Ignore. Because VS Code's
editor.codeActionsOnSave can fire
source.fixAll.mdsmith on any saved file (even one
that never produced diagnostics), an ignored
buffer would have been rewritten by the LSP path
even though `mdsmith fix` skips it on disk. The
handler now short-circuits to an empty action
array when config.IsIgnored matches the
workspace-relative path.
Tests:
- TestFixSourceWithSourceFSResolvesIncludeRelativePath
pins the SourceFS plumbing.
- TestHandleCodeActionRespectsIgnoreList pins the
ignore-list short-circuit.
https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
* lsp: gofmt fix for source_test.go
Stray double blank line between import block and the
first test function tripped CI's `go tool golangci-lint run`
gofmt check. `gofmt -w` removes the empty line.
https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
* lsp: switch extension to bun, add unit tests, address review
User-requested changes:
- Switched the VS Code extension toolchain from
Node + esbuild to Bun. Bundling now goes through
`bun run build.ts`, tests through `bun test`,
packaging through `bunx --bun @vscode/vsce`. The
esbuild script and devDep are gone; @types/bun is
in. CI's vscode-extension job and the release
workflow's vscode job both pinned to bun 1.3.11
via oven-sh/setup-bun.
- editors/vscode/src/extension.ts: extracted four
pure helpers (buildServerOptions,
buildClientOptions, startupErrorMessage,
collectFixAllEdits) into editors/vscode/src/wiring.ts
so they can be unit-tested without booting a real
VS Code host. The runtime wiring in extension.ts
is now a thin glue layer over those.
- editors/vscode/src/wiring.test.ts: 8 bun-test
cases pin the spawn shape, document-selector +
watcher binding, error-message phrasing, and the
fix-on-save edit filter (kind/document/missing-
edit handling, edit-order preservation).
- bun.lock committed so CI can use `--frozen-lockfile`.
- README.md and plan/121 updated to describe the
bun-based build/test workflow.
Copilot review round 10:
- engine.RunSource: GitignoreFunc was only wired
when RootDir was set, so in-memory linting with
an absolute path but no RootDir diverged from
Run()'s processFile(). Now mirrors processFile:
anchor at RootDir when set, fall back to
filepath.Dir(path) when path is absolute, leave
unset only for the bare "<stdin>"-style case
where there is no useful root.
- internal/lsp/server.go registerWatchers comment
said clients without dynamic registration "fall
back to the polled config"; no such polling
exists. Comment now describes the actual
behavior: the server only sees a config change
on the next initialize / didChangeConfiguration
/ explicit didChangeWatchedFiles event.
- TestRunOffSuppressesLint comment claimed didOpen
"still produces an initial snapshot"; the test
actually exercises the harness's onType default
for the open and only flips to off afterward.
Comment rewritten to describe what the test
really does and to point at the runMode docs for
the full table.
https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
* ci: install bun directly to avoid setup-bun cache use
zizmor's cache-poisoning rule flags `oven-sh/setup-bun`
because the action enables the GitHub Actions tool cache
by default — a mutable surface that can be poisoned by
any other workflow on the repo. The action exposes no
flag to disable caching, so replace the action with a
pinned `curl https://bun.sh/install | bash -s bun-v...`
step in both ci.yml and release.yml. Output of the
installer is added to GITHUB_PATH so the rest of the
job can call `bun` and `bunx` as before.
https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
* lsp: surface transport write errors and anchor server CWD
Two real bugs from Copilot review round 11:
- transport.writeJSON used to discard write failures
via `_ = s.t.write*`. If the client dropped its
stdout pipe (EPIPE) while stdin remained open, the
server kept running silently — the documented
exit-code 2 on transport failure never triggered.
The transport now records the first write error
in an atomic.Pointer[error]; transport.WriteError()
exposes it. Server.Run polls before/after every
dispatch and returns the recorded error so
`mdsmith lsp` exits non-zero on broken pipes.
- editors/vscode/src/wiring.ts buildServerOptions
now accepts an optional `cwd` and threads it
through to Executable.options on both the run and
debug launch shapes. extension.ts looks up the
first workspace folder and passes it down so the
spawned server runs anchored at the project root.
Several rules still call os.Stat on paths derived
from f.Path (cross-file links, git-hook-sync
repo discovery); without a stable CWD they would
resolve against whatever directory VS Code's
extension host happens to start from, producing
drift from CLI behavior.
Tests:
- TestRunSurfacesTransportWriteError pins that
Run() returns the recorded transport error rather
than looping forever.
- TestWriteJSONRecordsFirstError +
TestWriteJSONPreservesFirstError pin the
first-error-wins semantics on the new
WriteError() accessor.
- editors/vscode/src/wiring.test.ts gains two
cases: cwd is wired onto both run and debug when
supplied, and the options field is omitted
entirely when no cwd is given (so clients that
reject empty options shapes stay happy).
https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
* lsp: address Copilot review round 12
Real bugs:
- internal/lsp/server.go Run(): on ctx cancellation
the server returned immediately without setting
shutdown or stopping pending debounce timers, so
an armed time.AfterFunc could fire after Run() had
exited and write publishDiagnostics into a
half-closed pipe. Run now defers shutdown=true +
stopPendingLints() on every exit path.
- internal/lsp/server.go dispatch(): once `shutdown`
has been requested, LSP §3.16 says only `exit` may
follow; everything else gets InvalidRequest. The
dispatch switch now short-circuits non-exit
methods with a -32600 reply (or silent drop for
notifications) once s.shutdown is set.
Bench fidelity:
- internal/lsp/bench_test.go used to register only
linelength + notrailingspaces, so the latency
budget was measured against ~5% of the real rule
surface. Switched to the new
internal/rules/all blank-import barrel, which
pulls in every production rule. Local p95 on the
full set is 13 ms (1k) / 76 ms (5k) — comfortably
inside the 150 / 500 ms budgets.
- internal/rules/all/all.go consolidates the
blank-imports that cmd/mdsmith/main.go used to
enumerate inline. Both now share the barrel so
"what rules ship in mdsmith" has a single source
of truth.
Supply chain:
- .github/workflows/ci.yml + release.yml: the bun
install used to pipe `https://bun.sh/install` to
bash without integrity verification. Replaced
with a pinned download of the
bun-v1.3.11/bun-linux-x64.zip release archive
whose SHA-256 is checked before unzip. No more
remote-script execution on CI.
Docs:
- docs/reference/cli/lsp.md and
docs/guides/editors/vscode.md said the diagnostic
end column was "derived per-rule"; the
implementation always sets it to the line's UTF-16
length. Tables now describe the actual squiggle →
end-of-line behavior.
https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
* lsp: address Copilot review round 13
- handleDidSave: doc comment claimed didSave was the
only event linted under run=onSave, but
scheduleLint's own table notes that open/save/
config-change all lint in onSave; only didChange
is filtered. Comment rewritten to match
scheduleLint's behaviour.
- scheduleLint: when an immediate trigger
(open/save/config) fires while a debounce timer
for the same URI is still armed, the timer used
to fire after the synchronous lint and republish
diagnostics for the same buffer version. Stop
and remove the pending timer before running
immediately so the immediate publish is the only
one the client sees.
- internal/lsp/server_test.go: was blank-importing
only linelength + notrailingspaces while a
comment claimed rule.All() returned the
production set. Switched to the
internal/rules/all barrel so the test exercises
what an editor actually loads.
- internal/lsp/bench_test.go: newBenchHarness
registers `b.Cleanup(h.close)`; benchLatency was
also `defer h.close()`, which made the second
close hang for the 2-second srvDone watchdog.
Kept only b.Cleanup. Bench wall-clock dropped
from ~10 s to ~2 s on the local 20-iteration
config.
- internal/lsp/bench_test.go: per-iteration
document grew unbounded under the default
time-based benchtime, so later p95 samples
measured a much larger file than earlier ones.
Switched to two same-length buffers swapped
between iterations (`flipFirstParagraph` keeps
the byte length identical), so every sample
measures the same workload.
https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
* lsp: convert lint.Diagnostic.Column from byte to UTF-16
lint.Diagnostic.Column is a 1-based UTF-8 byte column (see
lint.File.ColumnOfOffset, which derives column from byte offsets
inside the source). toLSP was passing Column-1 to utf16Column as
if it were a rune offset — for ASCII files the two coincide, but
any line with multi-byte runes preceding the diagnostic produced
a squiggle/code-action range that was off by N-1 positions per
preceding multi-byte rune.
Replaced utf16Column with utf16FromByteOffset, which walks the
line's bytes via utf8.DecodeRune and sums each rune's
utf16.RuneLen until it hits the requested byte offset. Out-of-
range offsets clamp to [0, utf16Length(line)] so a malformed
mdsmith column cannot produce a negative or past-end position.
Removed the now-unused currentLine / runeLen helpers; the only
caller (documentEndPosition) and the new toLSP path operate on
byte slices directly via currentLineBytes / utf16Length.
Tests:
- TestToLSPMultiByteRuneBeforeColumn pins the byte-vs-rune
contract for a 2-byte rune (é) — Column 3 must map to UTF-16
character 1, not 2.
- TestToLSPSurrogatePairBeforeColumn pins the same contract for a
4-byte / 2-UTF-16-unit non-BMP rune (😀): Column 5 maps to
UTF-16 character 2.
- TestUtf16FromByteOffsetSurrogatePair, ClampsNegative, and
InvalidRunes cover the helper directly.
- TestCurrentLineBytesOutOfRange replaces the string-returning
currentLine test.
https://claude.ai/code/session_01LMJUseK9vHBNdZYMFwpz1C
* ci: create bunx symlink in vscode-extension and release jobs
The bun release zip ships only the bun binary, but the workflow's
"Package .vsix" step calls `bunx --bun @vscode/vsce package`. Because
bunx isn't on PATH, that step exits 127 (command not found) while the
earlier `bun install`/`bun test`/`bun run` steps succeed. The official
bun installer creates a bunx -> bun symlink, so reproduce that after
moving bun into $HOME/.bun/bin.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* lsp: address Copilot review round 14
- cmd/mdsmith/lsp.go: handle pflag.ErrHelp and exit 0 to match
the rest of the CLI (kinds, etc.). The unit test that
documented the prior behavior is updated to assert exit 0.
- plan/121_vscode-integration.md: replace stale npm/esbuild
wording with the actual Bun-based toolchain
(`bun install --frozen-lockfile`, `bun test`,
`bun run build.ts --production`, `bunx @vscode/vsce package`)
in the design section, the task list, and the acceptance
criterion.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* test(lsp): wait for subprocess exit so coverage counters flush
The LSP e2e test races the CommandContext's `defer cancel()` against
the subprocess's natural exit. After `pipe.shutdown(t)` sends the
`exit` notification, the test function returns and `defer cancel()`
fires; CommandContext SIGKILLs the still-exiting subprocess before
its runtime atexit handler writes the coverage counter file. The
coverage profile contained covmeta but no covcounters for cmd/mdsmith,
so codecov reported 0% for every line in cmd/mdsmith/lsp.go even
though the e2e test had executed them.
The shutdown helper now closes stdin and waits for the subprocess
before returning, so by the time `defer cancel()` runs the process
has already flushed its counters and exited.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* lsp: lift coverage on shutdown, debounce, and source-fix branches
Targets the Codecov patch+project gates: cover the post-shutdown
request rejection path in dispatch, the immediate-trigger
cancellation of a pending debounce timer, the post-shutdown
re-check inside the debounce callback, the nil-error guard in
recordWriteErr, and the settings-error branch in fixSourceImpl.
https://claude.ai/code/session_0145LDPJNBdgWFs8RxuTsSEA
* ci: install bun via oven-sh/setup-bun (pinned, no-cache)
Replaces the hand-rolled curl + sha256sum + manual symlink dance with
the canonical install path documented in
https://bun.com/docs/guides/runtime/cicd. The action is pinned by
commit SHA (v2.2.0 → 0c5077e5...), and `no-cache: true` keeps the
GitHub Actions tool cache disabled so zizmor's cache-poisoning rule
stays satisfied — `bun` and `bunx` are downloaded and installed fresh
on every run.
The symlink workaround (introduced in cab2819) is no longer needed
because setup-bun installs both `bun` and `bunx`.
Verified locally: `zizmor --persona=auditor` reports no findings
against the two updated workflow files.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* lsp+engine: address Copilot review round 15
- internal/lsp/server.go workspaceRelative: a bare
HasPrefix(rel, "..") incorrectly rejected in-root files whose
names happen to start with two dots (e.g. "..foo.md") as
parent traversals, so they fell back to the absolute path and
no longer matched repo-style globs/ignores. Match only true
traversals: rel == ".." or rel starting with ".." +
filepath.Separator. Adds two regression cases to
TestWorkspaceRelativePathHandling.
- internal/engine/runner.go RunSource doc: the comment claimed
GitignoreFunc is wired only when Runner.RootDir is set, but
the implementation also wires it when RootDir is empty and
path is absolute (using filepath.Dir(path) as the gitignore
root). Restate the actual two-case ordering so callers know
when gitignore is consulted.
PR description updated separately to replace the misleading
"rune-based columns" wording with "1-based UTF-8 byte columns",
matching what lint.Diagnostic.Column actually carries.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* lsp: address Copilot review round 16
- internal/lsp/server.go snapshotConfig: anchor the effective
project root at filepath.Dir(s.configPath) when a config is
loaded, falling back to the workspace folder otherwise. This
matches the CLI's rootDirFromConfig in cmd/mdsmith/main.go,
so ignore globs, override patterns, and Runner.RootDir behave
identically whether a file is checked via `mdsmith check` or
via the LSP server. Without this, opening a workspace folder
that is a subdirectory of the repo (or pointing
`mdsmith.config` at a config one level up) silently produced
different diagnostics. Adds two regression tests.
- internal/lsp/documents.go set: take ownership of d.text via a
deep copy so callers may safely reuse or mutate their own
slice after the call. The previous shallow copy aliased the
caller's backing array; concurrent get() readers could then
observe an in-flight mutation. Updates the get() doc to
reflect the new ownership contract and adds a regression test
that mutates the caller's slice and asserts the stored bytes
are unchanged.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* lsp+engine+fix: enforce MaxInputBytes on in-memory sources
Mirror the on-disk size cap that lint.ReadFileLimited and
readStdinLimited apply at read time, so in-memory callers (LSP
buffers, code-action fixes, future integrations) cannot bypass
the file-size limit by sending bytes directly through the
pipeline.
- internal/engine/runner.go RunSource: fast-fail with a
"<path>: file too large (<n> bytes, max <max>)" error when
len(source) exceeds Runner.MaxInputBytes. MaxInputBytes <= 0
and math.MaxInt64 keep the existing "unlimited" semantics.
- internal/fix/source.go fixSourceImpl: same guard against the
effective maxBytes (defaulting to lint.DefaultMaxInputBytes
when opts.MaxInputBytes is 0). LSP code actions now surface
a clear error instead of silently fixing buffers larger than
`mdsmith fix` would accept.
- internal/lsp/server.go runLint: stop discarding
Runner.RunSource errors. Each error is logged via s.logger
and surfaced to the editor via a window/logMessage
notification (LSP §3.18.1, MessageType.Error), so users see
pipeline failures (parse errors, oversized buffers,
config-target rule errors) instead of an editor that looks
silently broken. Added the messageType / logMessageParams
types in protocol.go.
- Tests:
- TestRunSource_RejectsOversizedSource and
TestRunSource_UnlimitedMaxInputBytes pin the engine guard.
- TestFixSourceRejectsOversizedSource pins the fix guard.
- TestRunLintSurfacesRunnerErrorsViaLogMessage pins the LSP
error-surfacing path.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* ci+codecov: measure TypeScript coverage and tag uploads by language
The vscode-extension job now runs `bun test --coverage` (text +
lcov reporters) and uploads `editors/vscode/coverage/lcov.info`
to Codecov under the `typescript` flag. The existing Go upload
in the test job is tagged with the `go` flag so the two reports
do not overwrite each other.
codecov.yml learns flag_management (with `paths:` so each flag
only covers its own language) and component_management so the
Codecov UI separates Go vs TypeScript and the PR comment shows
a per-component breakdown.
vscode-extension also gains `permissions: { contents: read,
id-token: write }` for codecov-action's use_oidc flow, matching
the existing test job. Verified zizmor reports no new findings.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* editors/vscode: ignore bun's coverage/ output dir
`bun test --coverage` writes coverage/lcov.info under the
working directory; the new CI step consumes it but leaves the
artifact behind on local runs. Ignore it next to dist/ and
node_modules/ so it doesn't surface as untracked.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* lsp+engine+vscode: address Copilot review round 18
- internal/engine/runner.go RunSource: run runConfigTargetRules
before the size guard. Run() runs them up front regardless of
any individual file's size; the previous ordering meant an
oversized in-memory buffer would skip config-level diagnostics
that `mdsmith check` would have surfaced.
- internal/lsp/server.go runLint: wire Runner.ConfigPath from
snapshotConfig. Without it, engine.Runner skipped config-target
rules entirely, so LSP linting silently dropped those checks
and the comment claiming "config-target rule errors" are
surfaced was a lie.
- internal/lsp/server.go reloadConfig: surface load/discover
failures via s.logger and a window/logMessage notification
(LSP §3.18.1, MessageType.Error). Previously a malformed
.mdsmith.yml or unreadable mdsmith.config path silently fell
back to defaults and the editor user had no way to diagnose
why settings appeared to be ignored. The notification is sent
in a deferred block after configMu is released to avoid
serializing transport writes against future readers.
Adds TestReloadConfigSurfacesLoadFailure pinning the new
behaviour.
- editors/vscode/build.ts watch mode: resolve glob.scan results
against import.meta.dir before passing to Bun.file().stat().
When build.ts is invoked from a working directory other than
its own (e.g. via `bun run`), the relative paths returned by
glob.scan would otherwise be statted against process.cwd and
miss change detection.
- editors/vscode/package.json: add the "Formatters" marketplace
category (and a "formatter" keyword) so the extension surfaces
in formatter searches — mdsmith ships an auto-fix pipeline,
not just a linter.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* lsp+fix: 100% coverage for server.go, diagnostics.go, fix/source.go
CI's lint gate failed earlier because reloadConfig had grown past
funlen's 40-statement limit. Split it into reloadConfig (lock,
notify) and a new resolveConfig helper that returns
(cfg, path, errMsg). Same observable behaviour, smaller
functions, plus the seam lets us inject a fake discoverConfig
for the previously-unreachable Discover failure path.
Removed defensive branches that no test could ever reach:
- internal/lsp/diagnostics.go toLSP: dropped the `if endCol <
startCol` clamp — utf16FromByteOffset and utf16Length both
cap their result at the line's UTF-16 length, so endCol is
always >= startCol.
- internal/lsp/diagnostics.go utf16FromByteOffset: dropped the
`size == 0` and `w < 0` branches. utf8.DecodeRune returns
size >= 1 on every non-empty slice, and utf16.RuneLen returns
-1 only for surrogate code points which DecodeRune never
produces (invalid UTF-8 yields RuneError, whose RuneLen is 1).
- internal/lsp/server.go documentEndPosition: dropped the empty
splitLines fallback — the function already returned (0,0)
on empty source, so splitLines always yields at least one
element by the time we reach that line.
- internal/lsp/server.go fetchClientSettings /
registerWatchers: dropped the `json.Marshal(int64) err`
checks. Marshaling an int can't fail.
- internal/lsp/server.go uriToPath: dropped the redundant
`u.Scheme != "file"` check after the `strings.HasPrefix(uri,
"file://")` guard.
- internal/fix/source.go fixSourceImpl: dropped the dead
applyFixPasses error pass-through. The only path that
populates that errs slice is lint.NewFile's error return,
and NewFile is currently infallible.
Made Windows-only paths testable by extracting uriToPathOnOS,
which takes a goos parameter. Added tests:
- TestUriToPathOnWindowsHostBecomesUNC
- TestUriToPathOnWindowsStripsDriveLetterSlash
- TestUriToPathOnLinuxLeavesDriveLetterAlone
Made the previously-unreachable transport error / channel-full /
fetch timeout / Discover error paths testable:
- Server gained `fetchTimeout` (defaults to 2s; tests dial it to
ms) and `discoverConfig` (defaults to config.Discover; tests
inject a stub).
- TestDeliverResponseDropsWhenChannelFull: fills the buffer-1
pending channel and asserts the second deliver is dropped
rather than blocking the dispatch loop.
- TestFetchClientSettingsTimeoutLeavesSettings: drives the
time.After branch with a 5 ms timeout.
- TestFetchClientSettingsWriteRequestFailureReturnsEarly: pins
the early-return when the transport writeRequest fails.
- TestRunReturnsErrorRecordedBeforeFirstIteration: pre-records a
transport write error and runs Run, hitting the top-of-loop
WriteError check on iteration 0.
- TestRunReturnsRecordedWriteError: drives two real frames into
Run with a failing writer so the post-dispatch WriteError
check returns the recorded io.ErrShortWrite.
- TestReloadConfigSurfacesDiscoverFailure: stubs discoverConfig
to error, asserts the failure surfaces via window/logMessage.
Also extended TestWorkspaceRelativePathHandling with the
relative-root case (filepath.Rel returns an error there) so the
err branch is covered.
Final coverage as reported by `go tool cover -func`:
- internal/lsp/server.go 100% (700/700 stmts)
- internal/lsp/diagnostics.go 100% (71/71 stmts)
- internal/fix/source.go 100% (per-function: 100/100/100)
go test ./... is green; golangci-lint reports 0 issues.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* lsp+vscode: address Copilot review round 19
- editors/vscode/src/extension.ts: cast collectFixAllEdits's
result to vscode.TextEdit[] at the call site so
event.waitUntil sees the type it expects. wiring.ts stays
decoupled from the `vscode` runtime package via the
structural TextEditLike interface; the cast is safe because
the runtime objects are real vscode.TextEdit instances
forwarded from executeCodeActionProvider.
- internal/lsp/server.go documentEndPosition: rewrote the doc
to spell out the exact end coordinates per case so the doc
no longer drifts from the return value (the previous
"{lineCount, 0}" wording was ambiguous: lineCount could mean
splitLines's length, but the function actually returns the
newline count, which differs by 1 for newline-terminated
input). Updated fullFileEdit's adjacent doc to defer to the
function's own doc instead of restating it.
- internal/lsp/protocol.go: removed the dead
titleQuickFixPattern constant. quickFixTitle in server.go
builds its own "Fix all <rule> with mdsmith" string and
never referenced this template, which used a different
("Fix %s with mdsmith") shape.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* lsp: route config-target diagnostics off the markdown squiggle path
engine.RunSource fires both document rules AND config-target
rules (gated by Runner.ConfigPath, which the LSP wires from
snapshotConfig). Config-target findings carry
lint.Diagnostic.File = .mdsmith.yml, not the markdown document.
toLSPAll has no notion of File, so without filtering, every
config-file finding shows up as a squiggle inside the markdown
buffer that happened to trigger the lint pass — at the config
file's line/column, against the wrong file.
Split res.Diagnostics through a new partitionDocDiagnostics
helper:
- Diagnostics whose File matches relPath (or is empty, the
legacy single-file shape) flow into the textDocument/
publishDiagnostics for the markdown buffer.
- Everything else is logged via s.logger and surfaced via
window/logMessage with a "<file>:<line> <message> [<rule>]"
prefix so the user can navigate to the actual config-file
issue.
Adds TestPartitionDocDiagnosticsRoutesByFile pinning the three
cases (doc-scoped, empty-File, foreign-File) so a future change
can't regress the routing.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* lsp+fix: align max-input-size semantics + restore utf16 guard
Round-21 review:
- internal/lsp/diagnostics.go: restored the negative-width guard
in utf16FromByteOffset, extracted as nonNegativeUTF16RuneLen so
the defense is unit-testable. utf8.DecodeRune normally maps
invalid bytes to RuneError (width 1), but a future runtime
change that yielded a surrogate code point through DecodeRune
would otherwise let utf16.RuneLen's -1 decrement the running
total and emit a negative LSP character offset on the wire. New
TestNonNegativeUTF16RuneLen pins both the normal and clamped
branches.
- internal/fix/source.go fixSourceImpl: aligned MaxInputBytes
semantics with lint.ReadFileLimited / cmd resolveMaxInputBytes.
Previously <= 0 forced the 2 MB default, which made it
impossible for callers to honor `max-input-size: 0`
(unlimited). Now <= 0 (and math.MaxInt64) means unlimited and
the default is the caller's responsibility. The unused
internal/lint import dropped with that change. SourceOptions
doc updated to spell out the contract; existing tests still
pass because they either set an explicit cap or already passed
buffers small enough that the (formerly default, now unlimited)
zero value never triggered the guard.
- internal/lsp/server.go runLint + quickFixEditFor +
source.fixAll: replaced the hard-coded lint.DefaultMaxInputBytes
with a new s.resolveMaxInputBytes(cfg) helper that mirrors the
CLI's resolution: unset → default, "0" → unlimited, otherwise
parse via config.ParseSize. Parse errors are logged and
surfaced via window/logMessage so a typo in
`max-input-size:` does not silently break linting. LSP fixes
now match `mdsmith fix` on disk for any project-wide cap.
- Pulled the foreign-diagnostic forwarding out of runLint into
surfaceForeignDiagnostics so the routing is unit-testable
without driving a full lint pipeline.
- Tests: TestSurfaceForeignDiagnosticsEmitsLogMessage,
TestResolveMaxInputBytesSurfacesParseError,
TestResolveMaxInputBytesUnlimited.
Coverage: server.go, diagnostics.go, fix/source.go all back to
100% statements covered.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* test(fix): rewrite misleading source_test.go tests
Round-22 review caught two tests whose names/comments did not
match what they actually verified:
- TestFixSourceWithRulesAcceptsZeroMaxBytes was named after the
old "<=0 means default" semantics. Round 21 aligned the
contract with lint.ReadFileLimited so <=0 means unlimited, but
the test only asserted no-trailing-spaces' fix output — it
would still pass if the size guard regressed. Renamed to
TestFixSourceWithRulesUnlimitedMaxBytes and fed it a >2 MB
buffer (one larger than DefaultMaxInputBytes) so a regression
to the old "default fallback" behavior surfaces as a
"file too large" error rather than a successful fix.
- TestFixSourceWithSourceFSResolvesIncludeRelativePath claimed to
pin SourceFS-driven include resolution, but it only registered
no-trailing-spaces and single-trailing-newline — neither rule
reads f.FS, so SourceFS could be dropped on the floor and the
test would still pass. Replaced with a small fsSpyRule that
records f.FS during Check, plus two assertions:
- TestFixSourceWiresSourceFSIntoLintFile pins that
SourceOptions.SourceFS reaches the lint.File the rule sees.
- TestFixSourceFallsBackToDirFSWhenSourceFSNil pins the
dirFS-from-Path fallback when no SourceFS is supplied.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* docs(vscode): fix misleading mdsmith.path PATH wording
The setting description ("A bare name is resolved against \$PATH")
implied the user shell's PATH. In practice the
vscode-languageclient spawns the server through Node's
child_process, which inherits the extension host's PATH —
the container/login-shell environment, NOT the interactive
shell that sourced ~/.bashrc / ~/.zshrc. A user with
\`/go/bin\` on PATH in their terminal hit \`spawn mdsmith ENOENT\`
because that PATH was added in ~/.bashrc and never reached
the extension host.
Updated three surfaces with the same accurate wording:
- editors/vscode/package.json: setting description now
spells out which PATH is consulted and points users at
the absolute-path workaround when ENOENT shows up.
- editors/vscode/README.md: settings table mirrors it.
- docs/guides/editors/vscode.md: settings table mirrors it,
and the troubleshooting section gains a dedicated
"spawn mdsmith ENOENT" entry with both the absolute-path
fix and the symlink-into-/usr/local/bin fix.
mdsmith fix repaired the table-format alignment so check
stays clean.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* vscode: add Restart / Show Output commands
The extension previously had no way to recover from a startup
failure (bad mdsmith.path, stale binary, server crash) short of
"Developer: Reload Window". Adds two commands surfaced via the
Command Palette under the `mdsmith:` category:
- `mdsmith: Restart Language Server` stops the active client
(best-effort — a half-started client may refuse to stop, in
which case dropping the reference is enough) and spawns a
fresh one through the same startServer flow as activate().
Useful after editing `mdsmith.path`, rebuilding the binary,
or pointing at a freshly-installed VS Code extension.
- `mdsmith: Show Output Channel` reveals the "mdsmith" output
channel where the language client logs RPC traffic and the
server's stderr. Quickest way to read a startup error
message.
extension.ts is restructured so:
1. Commands and the willSave fix-on-save handler are
registered FIRST, unconditionally — they remain usable
even when the server fails to start (the most useful one
then is "Show Output Channel").
2. The server-spawn flow is extracted into startServer() so
restartServer() can call it again without duplicating the
error-dialog logic. The startup error dialog gains a
"Show Output" button next to the existing "Download
mdsmith" / "Open Settings" choices.
docs/guides/editors/vscode.md gains a "Commands" section
documenting both commands; mdsmith fix tightened the
column-padding on the new tables to keep the markdown
linter clean.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* lsp: address Copilot review round 23 (LSP spec compliance)
Three correctness fixes flagged in the latest Copilot review:
- internal/lsp/protocol.go initializeParams: change ProcessID
and RootURI to pointer types (`*int`, `*string`). Per LSP
§3.16 these fields are typed `integer | null` and
`DocumentUri | null`; clients (including VS Code) really do
send JSON `null` when no parent process or root is
available. The previous concrete `int` / `string` made
`json.Unmarshal` fail with "cannot unmarshal null into ...",
causing handleInitialize to reject the very first request
for any client that sent null. pickRoot in server.go
follows the pointer through, treating nil as "no rootUri".
TestHandleInitializeAcceptsNullProcessIDAndRootURI pins
the new behavior end-to-end.
- internal/lsp/server.go Run + dispatch: track whether the
client sent a `shutdown` request via a new
`shutdownReceived` atomic bool. Per LSP §3.16, an `exit`
notification without a prior successful `shutdown` is an
abnormal termination — the CLI must exit non-zero. Run now
returns errExitWithoutShutdown in that case;
cmd/mdsmith/lsp.go's existing "non-Canceled error → exit
2" branch translates that into the correct exit code.
Two new tests pin both directions:
TestRunExitWithoutShutdownReturnsError and
TestRunShutdownThenExitReturnsNil.
- cmd/mdsmith/lsp_test.go lspPipe: guard cmd.Wait with
sync.Once so the cleanup-time wait and shutdown()-time
wait can't both call Wait (which is single-shot and
returns "Wait was already called" the second time,
obscuring real failures). Added a wait() helper; the
cleanup hook now calls p.wait() instead of cmd.Wait
directly. shutdown() also routes through p.wait() so the
cleanup is a no-op when the happy path already waited.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* vscode: bump extension to 0.1.1
Re-installing the same version of a .vsix is a silent no-op in
VS Code unless --force is passed, which makes "rebuild and
install" cycles confusing — users see the old binary keep
running. Bump to 0.1.1 so each iteration during the
plan-121 stabilization shows up as a real upgrade.
Functional change: includes the round-19 commands
(mdsmith.restartServer, mdsmith.showOutput) and the round-23
LSP spec fixes (nullable processId/rootUri, exit-without-
shutdown returning non-zero), plus the wiring renames.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* vscode: stage repo LICENSE during build to silence vsce warning
vsce package emits
WARNING LICENSE, LICENSE.md, or LICENSE.txt not found
unless one of those files lives next to package.json. The
repo's MIT LICENSE is at the repo root; rather than duplicate
it under editors/vscode/, build.ts now copies the root file
into editors/vscode/LICENSE before bundling. The staged copy
is git-ignored so the repo root stays the single source of
truth.
Verified locally: vsce now reports the LICENSE.txt entry in
the .vsix file list and no longer prints the warning.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* vscode: replace default ErrorHandler with permissive recovery prompt
vscode-languageclient's stock ErrorHandler stops restarting the
server after 5 close events in 3 minutes and prints
"The mdsmith server crashed 5 times in the last 3 minutes. The
server will not be restarted." Once that fires, the only way to
recover is reloading the entire VS Code window — hostile during
local development (rebuild loops, transient ENOENT while
iterating on `mdsmith.path`, etc.).
Replace it with MdsmithErrorHandler:
- error(): always returns ErrorAction.Continue — RPC errors do
not kill the process, so there's nothing to do but keep going.
- closed(): allows up to 25 close events per 3-minute sliding
window before falling back to DoNotRestart. Rolling-window
state filters out stale entries on every event so the limiter
recovers naturally once the storm passes.
- When the limiter does trip, surface
vscode.window.showErrorMessage with "Restart Language Server"
and "Show Output" buttons. The first dispatches the existing
mdsmith.restartServer command (so the recovery path matches
the palette command), the second reveals the output channel
where the server's stderr lives.
Bumped the extension to 0.1.2 so re-installs replace the prior
0.1.1 binary instead of being silent no-ops, and added a
troubleshooting entry in docs/guides/editors/vscode.md
documenting the new "crashed too many times in a row"
notification path.
https://claude.ai/code/session_01CydtcNJ8MghUNxEn6pKMTx
* lsp: accept --stdio flag, surface fs.Parse errors
VS Code's vscode-languageclient appends `--stdio` whenever the
client uses TransportKind.stdio. Other LSP servers
(rust-analyzer, typescript-language-server, …) document the
same flag the same way — it's the de facto convention for
"transport explicitly requested as stdio". Our `mdsmith lsp`
flag set didn't define it, so fs.Parse returned
"unknown flag: --stdio" and we exited 2 silently — the
extension would crashloop on every VS Code launch in
Codespaces (and anywhere else the spawn surface adds the
flag).
cmd/mdsmith/lsp.go now declares `--stdio` as a no-op flag
(transport is always stdio) so the parse succeeds. As a
companion fix, the parse-error branch now writes
"mdsmith: lsp: <err>" to stderr instead of returning 2 with
nothing on the wire — pflag's ContinueOnError path doesn't
write to fs.Output reliably, so the debug trail used to
end at "exit code 2" …mdsmith lsp subcommand (#236)1 parent a024cae commit 5e36d99
48 files changed
Lines changed: 8384 additions & 154 deletions
File tree
- .github
- workflows
- cmd/mdsmith
- docs
- background
- guides
- editors
- reference
- cli
- editors/vscode
- src
- internal
- engine
- fix
- lsp
- rules/all
- plan
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
38 | 38 | | |
39 | 39 | | |
40 | 40 | | |
| 41 | + | |
41 | 42 | | |
42 | 43 | | |
43 | 44 | | |
| |||
47 | 48 | | |
48 | 49 | | |
49 | 50 | | |
| 51 | + | |
50 | 52 | | |
51 | 53 | | |
52 | 54 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
62 | 62 | | |
63 | 63 | | |
64 | 64 | | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
65 | 129 | | |
66 | 130 | | |
67 | 131 | | |
| |||
120 | 184 | | |
121 | 185 | | |
122 | 186 | | |
123 | | - | |
| 187 | + | |
124 | 188 | | |
125 | 189 | | |
126 | 190 | | |
127 | 191 | | |
128 | 192 | | |
| 193 | + | |
129 | 194 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
48 | 48 | | |
49 | 49 | | |
50 | 50 | | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
51 | 88 | | |
52 | | - | |
| 89 | + | |
53 | 90 | | |
54 | 91 | | |
55 | 92 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
71 | 71 | | |
72 | 72 | | |
73 | 73 | | |
| 74 | + | |
74 | 75 | | |
75 | 76 | | |
76 | 77 | | |
| |||
232 | 233 | | |
233 | 234 | | |
234 | 235 | | |
| 236 | + | |
| 237 | + | |
235 | 238 | | |
236 | 239 | | |
237 | 240 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
44 | 44 | | |
45 | 45 | | |
46 | 46 | | |
| 47 | + | |
47 | 48 | | |
48 | 49 | | |
49 | 50 | | |
| |||
53 | 54 | | |
54 | 55 | | |
55 | 56 | | |
| 57 | + | |
56 | 58 | | |
57 | 59 | | |
58 | 60 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
| 33 | + | |
33 | 34 | | |
34 | 35 | | |
35 | 36 | | |
| |||
39 | 40 | | |
40 | 41 | | |
41 | 42 | | |
| 43 | + | |
42 | 44 | | |
43 | 45 | | |
44 | 46 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
32 | 32 | | |
33 | 33 | | |
34 | 34 | | |
35 | | - | |
| 35 | + | |
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
108 | 108 | | |
109 | 109 | | |
110 | 110 | | |
| 111 | + | |
111 | 112 | | |
112 | 113 | | |
113 | 114 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
354 | 354 | | |
355 | 355 | | |
356 | 356 | | |
357 | | - | |
| 357 | + | |
358 | 358 | | |
359 | 359 | | |
360 | 360 | | |
361 | 361 | | |
362 | 362 | | |
363 | 363 | | |
364 | | - | |
| 364 | + | |
365 | 365 | | |
366 | 366 | | |
367 | 367 | | |
| |||
562 | 562 | | |
563 | 563 | | |
564 | 564 | | |
565 | | - | |
| 565 | + | |
566 | 566 | | |
567 | 567 | | |
568 | 568 | | |
569 | 569 | | |
570 | 570 | | |
571 | 571 | | |
572 | | - | |
| 572 | + | |
573 | 573 | | |
574 | 574 | | |
575 | 575 | | |
| |||
636 | 636 | | |
637 | 637 | | |
638 | 638 | | |
639 | | - | |
| 639 | + | |
640 | 640 | | |
641 | 641 | | |
642 | 642 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
130 | 130 | | |
131 | 131 | | |
132 | 132 | | |
133 | | - | |
| 133 | + | |
134 | 134 | | |
135 | 135 | | |
136 | 136 | | |
| |||
0 commit comments