Repository navigation
fix: adversarial audit findings (#62–#83) - #86
Merged
Merged
Conversation
The lexer emitted an identifier-shaped token for every byte outside the grammar, so 'puzzle @ {}' checked clean, and multi-byte characters produced tokens ending mid-character which panicked position_of's str slicing. Lex unknown input as TokenKind::Unknown, consuming a whole char so spans stay on UTF-8 boundaries.
Fixes #62
line_byte_range stripped a trailing newline unconditionally, so the empty phantom line after a document-final newline got start > end and any positioned method (definition/references/rename/validateEdits) panicked on a position that spec section 6.4 declares valid. Only strip a terminator that lies inside the line's own span. Fixes #63
…ning a silent no-puzzle parse_top returned None without a diagnostic when input ended right after 'puzzle' or 'puzzle <name>', breaking compile's 'no errors implies a puzzle' invariant (panic) and making check report truncated files as clean. Emit an unexpected-end-of-input diagnostic at the end of the token stream. Fixes #64
Op simulation failure (division by zero, i64 overflow) produced no diagnostic, so compile's 'no errors implies resolvable ops' invariant broke and the provider panicked. Emit x-demo/simulation-failed at the solution range so check surfaces it and compile returns diagnostics instead of crashing. Fixes #65
…cking read_line into a String fails on non-UTF-8 bytes and the panic killed the session with no response; read_until keeps the bytes, answers Parse error for an undecodable line, and continues. Stdout write/flush failures now exit quietly instead of panicking on EPIPE. Fixes #66
Diagnostics were emitted in analysis-phase order (body errors, then missing-section at the puzzle-name range, then semantic checks), violating the spec section 9.2 sort mandate whenever a later-position diagnostic was pushed first. Stable-sort by range.start in parse_document. Fixes #68
occurrences() lists the declaration first (a references carve-out in spec section 14.2), so a puzzle with 'solution' before 'ops' produced unsorted rename edits, violating section 15.2's sorted-edits mandate. Sort rename_ranges' op result; references keeps declaration-first. Fixes #69
…counts position_of counted the '\r' of a CRLF terminator as a character, so diagnostics on CRLF documents landed past the visible line length (invalid ranges per spec section 6.5). Clamp the counted span to the line's content end: bytes in the terminator map to end-of-line. Fixes #70
references looked occurrences up by bare name, so a position on 'puzzle a' returned the occurrences of an unrelated op named 'a', and a puzzle name with no same-named op reported no declaration even with includeDeclaration. Resolve occurrences through the typed Symbol instead. Fixes #71
The rename edits loop read non-position documents with .puzzle, so a document carrying error diagnostics still contributed edits while an unparseable one was silently skipped - a partial rename violating section 15.2's all-references-covered rule (e.g. only the first of two same-named ops was edited). Parse every received document with into_puzzle and answer invalidDocument/documentHasErrors instead. Fixes #72
…dary parse_op ignored expect_punct failures and expect_x/expect_number recovered twice, so one bad op line produced extra misattributed diagnostics and swallowed the next line's valid declaration, which then surfaced as a bogus unresolved-op. Return after the first reported error; remove the redundant recover_line calls. Fixes #73
read_number mapped parse failure to 0, so 'target = 99999999999999999999' silently compiled as 0. Tokenize overflow as BadNumber and report 'integer literal out of range' in expect_number. Parsing the signed slice also keeps -9223372036854775808 representable. Fixes #74
read_dir entry errors were dropped via filter_map, .xdl-named directories were read as source files, and a canonicalize failure reported no details.uri. Collect entries fallibly, skip directories (symlinks still resolve through canonicalization), and name the failing file in details.uri per spec 8.1. Fixes #75
reconstruct_source wrote sheet names into generated source verbatim, so an artifact carrying names with spaces or newlines produced text the provider's own parser rejects. Validate every name against the identifier grammar and answer invalidArtifact instead. Fixes #76
Response parse/mismatch failures returned without killing the provider, leaking the process. A stdout-closed provider now gets killed immediately too. And the final reader_thread.join() could block forever when a provider descendant inherits stdout; the reader is detached instead - it ends on EOF or a failed send and dies with the process. Fixes #77 Fixes #78
A fractional id (1.5) was accepted and could even initialize the session; bool/array/object ids were misreported as notificationNotSupported; a missing params member silently became {}. Spec 4 requires integer/string ids and a REQUIRED params object: wrong-shaped ids now answer -32600 and missing/non-object params answer -32602.
Fixes #79
Four gaps: a provider could emit extra lines past the last step unobserved (bounded reader drain now reports them); ${PROJECT_URI} was not substituted inside rawLine steps; --provider --fixtures swallowed the next flag as a value; and a directory named *.json or a failed read_dir entry confused fixture discovery.
Fixes #82
Section 18.3 documented refusal details as { refusalCode, uri?, range? } while normative fixtures asserted format/newName/uri/within verbatim, so a provider implementing exactly the documented shape failed the suite. Refusal details are now a closed contract: refusalCode plus the echo keys each refusal's defining section requires - uri for noSymbolAtPosition, newName for rename name refusals, format for reconstruct.artifactFormatUnsupported, within for unknown lookup selectors. The mock already emits exactly these keys; no fixture changes needed.
Fixes #67
kind is defined since LPP 1.1 ('file' or omitted) with 'directory' added at 1.2; supportedProtocolVersions lists exactly the supported set; a version-gated entry field before 1.1 is -32602 (capabilityUnavailable when negotiated but unadvertised); invalidEntry covers an invalid version.
Fixes #80
Clarifications matching mock + fixture behavior: document-set results order by map key (check/compile/symbols/rename); references' declaration-first carve-out and invalidPosition on all position-taking methods; overlappingEdits reports the original-order index of the later-in-document edit with a stable sort; whitespace-only lines are -32700; the full request validation pipeline is pinned; shutdown receives no further responses; languages is a REQUIRED non-empty array of exact {id, extensions} entries; invalidLanguage covers top-level languageId params; and a spec-defined field sent in an inapplicable context is -32602, not ignorable.
Fixes #81
Two races only surfaced under Linux CI scheduling: both reaped-provider tests shared one scratch dir (parallel cleanup deleted the pidfile), and scripts that exit without reading stdin raced the request write to EPIPE.
This was referenced Oct 15, 2026
Closed
This was referenced Oct 15, 2026
Closed
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
Fixes all verified findings from the full-repo adversarial audit (tracking issue #84). Each fix was developed red-green: a failing regression test was added and observed failing, then the smallest targeted change was made and re-verified.
Mock provider crashes (H1–H5 → #62–#66)
Unknown(char)tokens and syntax diagnostics for unrecognized input instead of panicking on non-ASCII bytes or treating them as identifiers.SourceTextno longer inverts the byte range of the phantom line after a trailing newline."puzzle","puzzle p") now produce a diagnostic andartifact: nullinstead of a silentNonethat panicked inlpp/compile.-32700(null id) and the session continues, instead of aborting the process.Ordering & semantics (#68–#74)
range.start(§9.2);renametextEdits sorted by range (§15.2) while references keep the declaration-first carve-out.position_ofexcludes CRLF terminators from character counts.references/renameresolve by symbol kind, so a puzzle and an operation sharing a name no longer conflate.renamenow refuses withinvalidDocumentwhen any received document has errors (§15.2 atomic coverage).0;i64::MINstill parses.Project loading, reconstruct, transport (#75, #76, #79)
read_direntry errors, and reportsdetails.urion canonicalization failure.reconstructrejects non-identifier names instead of emitting unparseable source.params(-32602).Runner (#77, #78, #82)
${PROJECT_URI}substitution torawLinesteps, and the CLI rejects flag-like values and.jsondirectories.Spec & docs (#67, #80, #81, #83)
detailspinned to spec-owned echo keys (uri,format,newName,within) matching fixture assertions.DocumentSetkey vs innerurisemantics, whitespace-only lines, post-shutdown behavior, and equal-start edit ordering clarified.Test plan
cargo fmt --all --checkcleancargo clippy --all-targets -- -D warningscleancargo test --workspace— 61 tests pass (new regression coverage for every code fix)67 passed, 0 failedagainst rebuilt mock providergit diff --checkclean