Add behavior tests for the highest-risk under-tested mutation paths: findExactText
(overlapping-match index resolution + out-of-range/empty/not-found errors),
findRegexPattern (non-overlapping index resolution + out-of-range/invalid-regex/
not-found errors), and spliceContent's CRLF pullback (an edit ending between \r
and \n must not orphan a bare LF). These resolve the byte range that gets mutated,
so wrong handling means wrong-location or line-ending-corrupting writes.
References defaulted to one line per reference; compact (one line per file,
columns collapsed) existed only as opt-in. Compact preserves every line:col pair
with zero information loss while cutting roughly one line per reference for the
LLM consumer. Default it to true; compact=false and the session compact_refs
pref still select the verbose per-reference layout (explicit override wins).
The default line prefix was the padded ' 42| ' form; the compact '42|' form
existed only as opt-in (compact_line_numbers). The padding and trailing space
carry no information an LLM needs, costing ~3 chars/line — ~300 tokens on a
400-line read, the highest-volume path in the server. Flip the default to
compact; compact_line_numbers=false, mode/line_numbers=full, and the session
'full' pref all still restore the padded alignment (explicit override wins).
Updates the two tests that pinned the old default and refreshes stale comments.
edit_apply validated the target with IsPathAllowed in the handler, then later
wrote it via os.WriteFile on the raw path after reading+parsing+diffing. A
symlink swapped in during that window could redirect the write outside the
workspace root. Inject the admission check into the edit Engine
(NewEngineWithValidator) and re-run it inside a new writeValidatedFile helper
immediately before the write — IsPathAllowed resolves symlinks at write time, so
a swapped-in link now resolving outside the root is rejected. This narrows the
TOCTOU window to near-zero while still permitting legitimate in-workspace symlink
edits (unlike a blanket O_NOFOLLOW). writeValidatedFile also dedups the
perm-preserving write shared by AST and text modes.
Tests: rejected-path edit leaves the file untouched; file mode is preserved.
lsp_query printed absolute paths from URIToFile (e.g. the full
/Users/.../internal/... prefix repeated per result) while file_search and the
AST summary already relativize. On a find_references across many files that
~60-char root prefix repeats per file group — wasted tokens for the LLM
consumer. Extract a Server.relPath helper (dedups the AST summary's inline
block) and apply it to definition and references display paths; the absolute
path is kept where the file is actually re-read for previews.
Adds a relPath unit test (in-workspace relativized, out-of-workspace unchanged).
line/col are 1-indexed and converted via uint32(line-1); a caller passing 0
wrapped to 4294967295, feeding a garbage Point into tree-sitter. Reachable from
the LSP-down AST fallback with caller-supplied coordinates. Reject non-positive
positions up front. Adds tests (the function was previously 0% covered).
file_search runs ripgrep with --follow on by default, so a symlink inside the
workspace pointing outside it (e.g. link -> /etc) caused rg to traverse out and
return matches from external files — leaking their content to the caller despite
the workspace-scoping contract. parseOutput emitted match paths verbatim with no
re-validation.
Filter every match through IsPathAllowed (resolving rg's workspace-relative path
against the root first); IsPathAllowed's EvalSymlinks rejects the escape while
still permitting legitimate worktree siblings. Defense in depth that also covers
--no-ignore and any other traversal that reaches outside the root.
Adds a security regression test (proven to fail without the filter).
parseOutput's pendingFile guard attaches before-context only when it belongs to the same file as the following match. Add tests for both the cross-file leak guard (fileA context must not attach to a fileB match) and the same-file positive case. Test-only; no behavior change.
edit_apply response=diff prefixed the fenced ```diff block with a 'Diff:'
label that the fence and the diff's own ---/+++ headers already convey — pure
token cost per diff response. Extract fencedDiff() (also dedups the diff and
default branches) and drop the label. Adds a test guard.
Also document that strip renumbers lines from 1 over the stripped content, so
the model doesn't cite original-file line numbers after a stripped read.
The page-1 walk was capped at max_results via SkipAll, but buildASTCursorFooter
derives the remaining count from the total number of matches seen. Capping
truncated that total, so on page 1 the cursor fired only when the last parsed
file happened to overshoot the cap — nondeterministic — and reported a wrong
remaining count when it did. Same failure class as the file_search first-page
cursor bug.
Always walk the full tree (pages 2+ already did). The common case where matches
fit in one page already visited everything (SkipAll never triggered), and parse
results are LRU-cached, so the extra cost falls only on multi-page queries and is
mostly file I/O.
Adds a strict regression test asserting page 1 emits a cursor with an accurate
remaining count; the existing pagination test silently skipped when no cursor.
ast_query walked every language-matching file under the search root, including
node_modules, vendor and bower_components — parsing third-party code and
flooding results with irrelevant matches (large token cost on every call,
worst for JS/TS node_modules). Skip these by name during the walk, alongside
the existing hidden-dir skip.
The walk root is now exempt from both skips, so targeting a dependency dir
explicitly (paths:["vendor/foo"]) still works — previously naming a hidden or
dependency dir as the root would have skipped everything. Documents the scope
in the tool description and help resource.
Adds a regression test covering default-skip and explicit-target inclusion.
extractCFunction/extractCFunctionDecl scanned for the first 'identifier' in the declarator, which for C++ methods grabbed a parameter name (e.g. method setName surfaced as 'n') or, when the name was a field_identifier with no parameter identifier, dropped the symbol entirely. cDeclaratorName now descends the declarator field-chain to the real name (identifier/field_identifier/qualified_identifier/destructor/operator); in-class definitions are kinded as methods and qualified Class::method (out-of-line Class::method definitions detected via '::'). Adds TestExtractCppMethods.
On the first page (offset==0) ripgrep capped results at exactly max_results
and set Truncated=true, but the cursor-footer guard (len>max_results) was
false, so the response said "(truncated...)" with no cursor — the caller
was told more matches exist but given no way to fetch them. Pagination only
worked from page 2. Emit a cursor on the first page using rg's total match
count for the remaining estimate.
Adds a strict regression test (asserts, not skips); the existing pagination
test was silently skipping its round-trip because page 1 never had a cursor.
firstLineOf (query) and truncateString (edit) sliced strings at a raw byte
offset, splitting a multibyte UTF-8 character into invalid bytes in
LLM-facing output (ast_query compact format and edit not-found errors are
hot paths). Extract the rune-boundary backoff already used by search's
truncateLine into util.RuneBoundary and route all three sites through it.
Adds table-driven tests for the primitive and a multibyte case for firstLineOf.
truncateLine byte-sliced s[:maxLen-3], which can split a multibyte UTF-8 character and emit an invalid sequence (mojibake) on long non-ASCII match lines. Back off to the nearest rune boundary via utf8.RuneStart. Adds TestTruncateLineRuneSafe.
Only max_file_size and max_parse_size were clamped after reading .mcp-filepuff.json; max_edit_size and max_search_results were unbounded, letting a config file request excessive memory. Clamp max_edit_size to 100MB and max_search_results to 100k (and floor negatives to 0). Adds TestLoadClampsExcessiveLimits.
FormatDocComment emitted leftover tags by ranging a map, so multi-tag comments (@example/@see/@deprecated) rendered in random order run-to-run — nondeterministic output that defeats caching/diffing. Sort the remaining tag names before emitting.
extractGoSymbols/extractJSSymbols matched var/const/let at any tree depth, so every function-local var, const (Go) and let/const/var (JS/TS) was emitted as a top-level symbol — flooding include_ast/ast_query symbol lists with locals (inaccurate + token-wasteful). Skip declarations nested inside a function/block body (isNestedInBody); module-level and 'export const' declarations are still kept. Adds TestExtractSymbolsExcludesLocals.
Locks the documented guarantee that inserting LF content into a CRLF file normalizes to CRLF, with no bare-LF leaks. Closes a test gap on a stated design goal.
extractElixirStruct hardcoded Name="defstruct", so every Elixir struct surfaced as a symbol literally called "defstruct" — useless for navigation and colliding across modules. Walk ancestors to the enclosing defmodule and name the struct %ModuleName{}. Adds TestExtractElixirStructNamedByModule.
drainStderr discarded all server stderr, so an LSP init failure (e.g. 'command not found', missing module) surfaced as an opaque 'initialization failed'. Retain a bounded tail (StderrCaptureMax=4096B, oldest dropped) under a mutex and attach it to the init-failure error context. Adds TestClientStderrCapture.
The fallback skeleton (c/cpp/elixir/html/vue) is not valid source, so the '// ' comment framing and '(line N)' wording just cost tokens. Use a compact 'kind name :line' form. ~15-25% fewer tokens on fallback skeletons; output stays accurate.
Per the project's error-handling rule, an ignored error needs a comment. Marshalling the two-scalar payload struct cannot fail; document why the error is dropped. No behavior change.
skeletonPythonNode took lines[0] of a decorated_definition, which is the first @decorator line, dropping the def/class signature entirely. Now emit the decorator line(s) then recurse into the wrapped definition so its signature is preserved. Adds skeleton_test.go (decorated + plain function).
readLoop sent the response on the pending channel while holding c.mu. A misbehaving server sending a duplicate response for the same id would block on the buffered(1) channel under the lock, deadlocking all future dispatches. Use a non-blocking send (matching the notification path).
Counter.Add applied any delta, so a negative value would decrement a counter that must be monotonic, corrupting rate()-style downstream math. Ignore delta<0. Adds TestCounterRejectsNegativeAdd.
Two file_search gaps:
- max_results defaulted to 0 (unbounded), so a broad pattern returned every match — unbounded memory + token blowup. The configured MaxSearchResults (default 1000) existed but was dead; wire it as the builtIn default. Explicit max_results still wins, and the cursor footer pages the rest.
- search.Request.IncludeHidden was a dead field with no tool param. Add an include_hidden bool (default false) so dotfiles (.github/, etc.) are searchable.
ast_query defaulted to verbose output (full code + up to 500 chars/match + captures), which is very token-heavy on multi-match queries. Default to compact (one line per match: file:line (kind) firstline) — roughly 5-10x fewer tokens on typical queries — while verbose remains available per-call (format=verbose) or via the default_format session pref. Updates tool description, help resource, and TestASTQueryFormatCompactDefault.
hunkBounds approximated one-sided start lines by reusing the opposite side's number, and re-assigned it every iteration inside the count==0 guard. For pure insertion/deletion hunks (e.g. writing into an empty file, or deleting a whole file) this produced wrong headers like '@@ -2,0 +1,2 @@' instead of '@@ -0,0 +1,2 @@'. flattenDiffLines now records both running line numbers on every line, so hunkBounds takes the start from the first line directly and applies GNU's zero-count convention (line-before-change, clamped to 0). Adds TestGenerateDiffHunkHeaderBounds.
handleASTQuery rebuilt the formatter's '[remaining: N]' placeholder string and strings.ReplaceAll'd it with the cursor line, plus a fallback append — fragile coupling to the formatter's exact output. New FormatResultsWithCursor appends the prebuilt '[cursor: ..., remaining: N]' footer in place of the bare hint when results are truncated; the handler just passes it through. FormatResultsWithOptions is unchanged (delegates with empty cursor). Adds TestFormatResultsWithCursor.
Two ast_query/lsp_query ergonomics improvements that share the tool registration and help files:
- lsp_query accepts symbol_name (+symbol_kind) and resolves the position via AST (resolveLSPSymbolPosition), so callers need not know line:col; line/column are no longer required. SymbolCandidate gains NameLine/NameColumn for the precise identifier position. Ambiguous/unknown names error with candidates / a 'did you mean' hint. Adds TestResolveLSPSymbolPosition.
- ast_query help + tool description now document that matching is keyword/kind-based, not structural: pattern arg/return constraints are not enforced; use name_exact/name_matches/kind_in to filter. Fixes a misleading 'functions returning error' example.
The goroutine parks in the blocking stderr.Read, so closing c.done does not interrupt it; termination relies on the read erroring at pipe EOF when the process is killed. The c.done select is only a best-effort fast path.
renderSymbolsOnly (fallback for c/cpp/elixir/html/vue) appended each symbol's raw start line plus a literal ' { ... }'. For brace-less languages (Elixir 'def f do', HTML) that is syntactically wrong and wastes tokens. Emit only the accurate symbol list; drop the fabricated line and unused params.
A misspelled or stale symbol_name returned a bare 'not found' error, forcing blind retries. nearestSymbolSuggestion fuzzy-matches the requested name against the file's actual symbols (via existing pkg/fuzzy) and appends 'did you mean: a, b, c?'. The re-parse is served from the parser cache. Adds TestReadSymbolNotFoundSuggestsNearest.
When a pagination cursor footer is emitted it already reports the remaining count; the separate '(truncated, showing subset of N)' prose line duplicated that signal. Suppress the prose line when CursorLine is set; keep it for non-paginated truncation.
A replace with empty new_content failed with a bare 'new_content is required' message, leaving the model to guess. Append a hint to use operation=delete when the intent is removal. Adds TestValidateReplaceEmptySuggestsDelete.
extractGoVarConst used ChildByFieldName("name"), which returns only the first name field, so 'var a, b = 1, 2' and 'const ( C, D = 3, 4 )' surfaced only a/C. Iterate every "name"-field child so each declared identifier becomes its own symbol. Adds TestExtractGoVarConstMultiName.
Three strip=imports/license correctness fixes (all heuristic, keyword/structure-gated):
- stripTSImports now tracks multi-line named imports (import {\n ... \n} from 'x') instead of dropping only the first line and leaving dangling members; side-effect imports without semicolons no longer trigger runaway consumption.
- stripGoImports recognizes all single-import spec forms: import _ "x", import . "x", and aliased import m "x" (previously only import "x"/backtick).
- stripLicense peels a leading shebang (#!) or Go build-constraint (//go:build, // +build) block and preserves it above the license, and now strips // line-comment license headers (not just /* */ and #). Adds tests for each.
FindSymbolRange returned the first AST node matching a name, so file_read with symbol_name silently read the wrong symbol when a name collided (e.g. two String() methods on different receivers, or a func and type sharing a name). New FindSymbolCandidates returns all matches; the file_read handler now errors on ambiguity, listing each candidate's node type and line range so the caller can disambiguate via symbol_kind or line_start/line_end. FindSymbolRange is kept (first match) for callers that don't need ambiguity detection. Adds TestFindSymbolCandidates.
We advertise textDocument.definition.linkSupport=true, so gopls and rust-analyzer return the LocationLink shape (targetUri/targetRange). The old code only tried []Location then a single Location; unmarshaling LocationLink JSON into []Location silently succeeds with zero-value URIs/ranges, producing garbage locations (or a false 'No definition found'). parseDefinitionResult now decodes a unified shape capturing both field sets and normalizes to Location, and surfaces a real parse error instead of swallowing it. Adds TestParseDefinitionResult covering all shapes.
lspHover fell back to AST symbol info on any LSP error, but the '(AST fallback)' marker only showed when verbose=true. In the default mode the model received Node type/Text with no signal the answer was a degraded AST guess. Now always emit '[AST fallback — LSP unavailable]' so the model can decide whether to retry. Includes colocated QF1012 style fixups.
Two AST-summary/query path fixes plus colocated QF1012 style fixups:
- runASTQueryWalk now checks info.Size() > MaxFileSize before os.ReadFile, avoiding a full read of files it will discard.
- generateASTSummary used len(splitLines(...)) for the '(N lines)' header, over-counting newline-terminated files by one (phantom trailing element). New countLines() helper counts via bytes.Count. Adds TestCountLines.
extractErrors is a pure function of the parsed tree (it ignores content), so re-walking the whole tree on every cache hit wasted work on the hottest path (file_read+include_ast, ast_query, edit validation). Store []SyntaxError in CachedTree and return it directly on hits. Adds TestParseCachedErrorsMatch asserting cache-hit errors equal the cache-miss parse.
Mechanical fix applied by golangci-lint --fix. Pre-existing debt that fails the staticcheck QF1012 quickfix check; no behavior change. Clears the lint gate so subsequent commits in this package set pass.
macos-13 Intel runner image retired by GitHub on 2025-12-08; darwin_amd64
build job queued indefinitely with no available runner. Switch to the
standard macos-15-intel label (native x86_64, free for public repos,
supported until Aug 2027). CGO required by go-tree-sitter, so a native
Intel runner is needed.
GoReleaser Pro --split defaults to partitioning by GOOS only, so each
linux matrix runner built both linux/amd64 and linux/arm64. With
CGO_ENABLED=1 (go-tree-sitter), the native gcc cannot assemble the other
arch's CGO runtime (gcc_arm64.S: no such instruction), failing the build.
- .goreleaser.yaml: set partial.by=target (partition by GOOS+GOARCH)
- workflow-prepare.sh: export GGOOS/GGOARCH from the matrix TARGET_* vars
so each native runner builds exactly its own target
GGOOS/GGOARCH are filter-only and do not bleed into before-hooks, unlike
GOOS/GOARCH.
- [x] Remove auto-indentation from text mode edits (caller controls whitespace)
- [x] Add line-ending detection and normalization for both AST and text modes
- [x] Share edit logic via new `spliceContent` function for both modes
- [x] Fix diff to emit "No newline at end of file" markers
- [x] Fix diff to strip raw CR from CRLF file output
- [x] Remove double-unescape of backslash sequences in new_content
- [x] Fix countDiffLines to be hunk-aware (correctly count lines starting with +/-)
- [x] Fix block-comment stripping to remove standalone lines cleanly
- [x] Fix Python license header stripping to preserve separator blank lines
lukaszraczylo
