arch(lsp): split server.go into focused sibling files - #680
Merged
Conversation
server.go was 1007 lines. Split the session management and utility helpers into two new sibling files within the lsp package: - server_session.go: rebuildSession, currentSession, snapshotConfig, reloadConfig, resolveConfig, fetchClientSettings, registerPendingResponse, unregisterPendingResponse, deliverResponse - server_util.go: workspaceRelative, dirFSForPath, frontMatterEnabled, isFixable, uriToPath, uriToPathOnOS, hasDriveLetterPrefix, pickRoot server.go trimmed to 555 lines (core struct, types, New, Run, recoverPanic, logPanic, and the dispatch functions). Pure reorganisation — zero logic changes. Closes plan/2606211909_arch-fix-lsp-server-split.md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BhyAzFuAuKB7s5N1FQAx9e
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…generate PLAN.md
The agent dropped `defer s.recoverPanic("fetch client settings")` when
extracting fetchClientSettings to server_session.go, causing panics to
escape instead of being logged. Restored the recover, trimmed the
trailing blank line that golangci-lint flagged, and regenerated the
PLAN.md catalog for the new plan entry.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BhyAzFuAuKB7s5N1FQAx9e
The `1. - [x]` nested-list structure in the Tasks section caused TestListscanMatchesASTCorpus to diverge on this file — goldmark saw 9 lists (7 nested unordered sublists inside each ordered item) while listscan saw only 2. Use the plain numbered list format from the proto.md template instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BhyAzFuAuKB7s5N1FQAx9e
The godoc said "disposes the previous one" but the function body explicitly does the opposite: it intentionally leaves the superseded session alive for GC so that in-flight lint goroutines (which hold a reference obtained before the swap) never observe a disposed session. Point the godoc at the in-body explanation instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BhyAzFuAuKB7s5N1FQAx9e
…ndows drive root
- watchedFilesTreeChanged: non-file:// URIs (e.g. git://, untitled:)
returned "" from uriToPath, which bypassed the .mdsmith.yml filter
and could trigger a spurious InvalidateWikilinks on Created/Deleted
events from non-conformant clients. Now skip URI="" entries.
- uriToPathOnOS: file:///Z: (bare Windows drive root) produced "Z:."
after filepath.Clean("Z:") because the stripped path "Z:" is treated
as a relative drive path. Append "\" before Clean when the result
is exactly two chars (X:).
- Corrected the url.Parse error comment: the branch is reachable on
invalid percent-encoded hosts (e.g. file://%GH/path) and is covered
by TestUriToPathInvalidURL; it is not "unreachable in practice".
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BhyAzFuAuKB7s5N1FQAx9e
… fix UNC stub
Tests:
- TestUriToPathOnWindowsBareRoot: drives the `len(p)==2` early-return
branch red/green — file:///C: must resolve to "C:\" not "C:."
- TestUriToPathOnWindowsHostOnlyReturnsEmpty: drives the new UNC
host-only guard — file://server (no share) must return ""
- TestWatchedFilesTreeChangedSkipsNonFileURI: drives the path==""
continue branch — a git:// Created event must not trigger a
wikilink-index rebuild
Fixes:
- uriToPathOnOS: return early for bare Windows drive root ("C:") to
bypass filepath.Clean's platform-dependent "C:." behaviour; use
backslash directly so the result is the canonical Windows root "C:\"
- uriToPathOnOS: UNC construction now returns "" when the path has no
share component (len(p)<=1), preventing "\\server" stubs from
passing callers' path!="" guards and reaching filesystem operations
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BhyAzFuAuKB7s5N1FQAx9e
All lint findings are pre-existing prealloc warnings in files not touched by this PR — no new issues introduced by the server.go split. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BhyAzFuAuKB7s5N1FQAx9e
jeduden
added
queue
Owner
Author
|
🟢 Merge Queue — picked up This PR is in the queue and will be batched with other Next: No action needed — you'll get another comment when CI starts on the batch. View merge queue run. |
Owner
Author
|
🔵 Merge Queue — CI running Merged into batch branch Next: No action needed — you'll be notified when CI completes. |
3 tasks
jeduden
removed
the
queue:active
Owner
Author
|
✅ Merge Queue — merged This PR landed on Next: Done — nothing more to do here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
internal/lsp/server.gowas 1007 lines (flagged by the 2026-06-21 architecture audit). Split session management and utility helpers into two new sibling files within thelsppackage, trimmingserver.goto 555 lines.internal/lsp/server_session.go:rebuildSession,currentSession,snapshotConfig,reloadConfig,resolveConfig,fetchClientSettings,registerPendingResponse,unregisterPendingResponse,deliverResponseinternal/lsp/server_util.go:workspaceRelative,dirFSForPath,frontMatterEnabled,isFixable,uriToPath,uriToPathOnOS,hasDriveLetterPrefix,pickRoot2606211909marked complete.Test plan
go build ./...passesgo test ./...passes (all packages)wc -l internal/lsp/server.gois 555 (under 800)go tool golangci-lint run— no new issues🤖 Generated with Claude Code
https://claude.ai/code/session_01BhyAzFuAuKB7s5N1FQAx9e
Generated by Claude Code