fix(lsp): convert semantic-range (and code-action) bounds per position encoding

Addresses the round-5 finding: the whole-document range that serves a
RANGE-ONLY semantic-token provider derived its columns from UTF-8 byte
counts and sent them unchanged — unlike the inlay path, it skipped
outbound_position. A UTF-16 server receives an invalid end character
for non-ASCII text ("é" is two bytes, one UTF-16 unit) and may reject
the request; since /range is a range-only provider's ONLY pull path,
that means no semantic styling at all.

Both bounds of request_semantic_tokens_range now go through
outbound_position. request_code_action had the identical bug (byte
columns, no conversion) and is fixed in the same stroke — same class,
same one-line shape, commented as such.

Fixture: `rangeonly16` fake mode = rangeonly + negotiated UTF-16 +
STRICT UTF-16 bounds validation on /range (fail-closed: a missing
didOpen record or absent uri also rejects, so the fixture can never
pass vacuously). An env-gated PMACS_FAKE_RANGE_SINK records the
received range for debugging. Test opens a file whose last line ends
in non-ASCII and asserts tokens arrive; verified it bites — with the
conversion removed the wire carries the byte column (13 vs the valid
11), the fake rejects, and the test fails.

Honest note: an earlier bite-check in this session produced a vacuous
pass because short, non-unique edit patterns hit the WRONG json! block
(temporarily regressing the inlay conversion and accidentally
converting code-action). The final diff is anchored uniquely and
verified: inlay unchanged (whitespace only), semantic + code-action
converted, bite-check red/green confirmed against the exact lines.

Gates: fmt + workspace clippy clean; lib 1500; crdt 1672; m4 99;
killring 30; completion 9; GPU 58; git diff --check clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Levi Neuwirth 2026-07-09 21:17:50 -04:00
parent 625128c139
commit 685547f2a7
3 changed files with 147 additions and 10 deletions

View File

@ -38,9 +38,13 @@
//! error — a conforming full-only server, for testing that the
//! client never requests delta without the negotiated capability.
//! * If launched with `PMACS_FAKE_LSP_MODE=rangeonly`: advertises a
//! range-only `semanticTokensProvider` (`"range": true`, `full`
//! null) and rejects `semanticTokens/full` — per LSP, `full` and
//! `range` are optional, independent capabilities.
//! range-only `semanticTokensProvider` (`"range": true`, no `full`)
//! and rejects `semanticTokens/full` — per LSP, `full` and `range`
//! are optional, independent capabilities.
//! * If launched with `PMACS_FAKE_LSP_MODE=rangeonly16`: `rangeonly`
//! plus UTF-16 position encoding, with strict UTF-16 bounds
//! validation on `/range` — rejects a client that sent raw byte
//! columns for non-ASCII text.
//! * If launched with `PMACS_FAKE_LSP_MODE=sighelp`: additionally
//! advertises `signatureHelpProvider` with `(` / `,` triggers, so a
//! test can drive the Arc 1d auto-trigger. Every other mode omits the
@ -187,13 +191,20 @@ fn main() {
// `range` WITHOUT `full` — the /full arm below rejects
// in this mode, so a client that ignores the split gets
// a visible failure instead of silent staleness.
if mode == "rangeonly" {
if mode.starts_with("rangeonly") {
let p = &mut resp["result"]["capabilities"]["semanticTokensProvider"];
if let Some(obj) = p.as_object_mut() {
obj.remove("full");
obj.insert("range".into(), serde_json::Value::from(true));
}
}
// `rangeonly16` additionally negotiates UTF-16, so the
// /range arm can validate that the client converted its
// byte columns to UTF-16 code units.
if mode == "rangeonly16" {
resp["result"]["capabilities"]["positionEncoding"] =
serde_json::Value::from("utf-16");
}
// Arc 1d: advertise signature help only in `sighelp`, so
// every other mode keeps the no-auto-trigger path (the
// `textDocument/signatureHelp` arm below still answers
@ -790,7 +801,7 @@ fn main() {
("textDocument/semanticTokens/full", Some(idv)) => {
// `rangeonly`: a range-only provider rejects /full —
// the client should have sent a range request.
if mode == "rangeonly" {
if mode.starts_with("rangeonly") {
let resp = serde_json::json!({
"jsonrpc": "2.0",
"id": idv,
@ -834,6 +845,27 @@ fn main() {
write_frame(&mut stdout, &resp);
}
("textDocument/semanticTokens/range", Some(idv)) => {
// `rangeonly16`: strict bounds validation in UTF-16
// units. A client that sent raw byte columns for
// non-ASCII text overshoots the last line's UTF-16
// length and is rejected — the fixture for the
// outbound-position conversion.
if mode == "rangeonly16"
&& let Ok(sink) = std::env::var("PMACS_FAKE_RANGE_SINK")
{
let _ = std::fs::write(&sink, format!("{params}"));
}
if mode == "rangeonly16"
&& let Some(message) = utf16_range_error(&params, &open_docs)
{
let resp = serde_json::json!({
"jsonrpc": "2.0",
"id": idv,
"error": { "code": -32602, "message": message }
});
write_frame(&mut stdout, &resp);
continue;
}
// T M4.5: same shape as /full, scoped to a range.
// One token: line 1 col 0 len 3, variable.
let resp = serde_json::json!({
@ -1053,6 +1085,47 @@ fn document_end_position(text: &str) -> (u64, u64) {
(line, col)
}
/// `rangeonly16` bounds validation: the request's end position must not
/// exceed the document end measured in UTF-16 code units (the
/// negotiated encoding). Byte-column overshoot on non-ASCII text is
/// exactly the client bug this catches.
fn utf16_range_error(
params: &serde_json::Value,
open_docs: &HashMap<String, String>,
) -> Option<String> {
// Fail-CLOSED: a fixture that silently skips validation on an
// unexpected state (missing uri / unrecorded doc) reads as a pass.
let Some(uri) = params
.get("textDocument")
.and_then(|t| t.get("uri"))
.and_then(serde_json::Value::as_str)
else {
return Some("range request carried no textDocument.uri".into());
};
let Some(text) = open_docs.get(uri) else {
return Some(format!("no didOpen text recorded for {uri}"));
};
let (mut line, mut col) = (0u64, 0u64);
for ch in text.chars() {
if ch == '\n' {
line += 1;
col = 0;
} else {
col += ch.len_utf16() as u64;
}
}
let end = params.get("range")?.get("end")?;
let end_line = end.get("line")?.as_u64()?;
let end_col = end.get("character")?.as_u64()?;
if end_line > line || (end_line == line && end_col > col) {
Some(format!(
"invalid utf-16 range end {end_line}:{end_col}; document ends at {line}:{col}"
))
} else {
None
}
}
fn inlay_range_error(
params: &serde_json::Value,
open_docs: &HashMap<String, String>,

View File

@ -2003,11 +2003,16 @@ impl LspManager {
diagnostics: &[Value],
) -> Result<JobId, String> {
let uri = uri.into();
// Same encoding rule as every outbound range: columns are pmacs
// byte offsets and must convert to the negotiated units — a
// UTF-16 server receiving byte columns resolves the wrong range
// for non-ASCII lines (same bug class as the semantic-token
// range fix in this change).
let params = json!({
"textDocument": { "uri": uri.clone() },
"range": {
"start": { "line": start_line, "character": start_col },
"end": { "line": end_line, "character": end_col },
"start": self.outbound_position(sid, &uri, start_line, start_col),
"end": self.outbound_position(sid, &uri, end_line, end_col),
},
"context": { "diagnostics": diagnostics },
});
@ -2038,7 +2043,7 @@ impl LspManager {
"textDocument": { "uri": uri.clone() },
"range": {
"start": self.outbound_position(sid, &uri, start_line, start_col),
"end": self.outbound_position(sid, &uri, end_line, end_col),
"end": self.outbound_position(sid, &uri, end_line, end_col),
},
});
let req_id = self.send_request(sid, "textDocument/inlayHint", params)?;
@ -2081,11 +2086,17 @@ impl LspManager {
end_col: u32,
) -> Result<JobId, String> {
let uri = uri.into();
// Columns arrive as pmacs byte offsets; convert per the
// negotiated position encoding, exactly like the inlay-hint
// range. A UTF-16 server receiving raw byte columns gets an
// invalid end character for non-ASCII text ("é" is two bytes
// but one UTF-16 unit) and may reject the request — fatal for
// a range-only provider whose ONLY pull path this is.
let params = json!({
"textDocument": { "uri": uri.clone() },
"range": {
"start": { "line": start_line, "character": start_col },
"end": { "line": end_line, "character": end_col },
"start": self.outbound_position(sid, &uri, start_line, start_col),
"end": self.outbound_position(sid, &uri, end_line, end_col),
},
});
let req_id = self.send_request(sid, "textDocument/semanticTokens/range", params)?;

View File

@ -4582,6 +4582,59 @@ fn arc1c_range_only_server_is_served_range_requests() {
);
}
/// Arc 1c review fix — a range-only server that negotiated UTF-16.
/// The whole-document range's columns are derived from pmacs byte
/// offsets; they must go through `outbound_position` like every other
/// outbound position. The last line ends in non-ASCII ("é" = 2 bytes,
/// 1 UTF-16 unit), and the fake validates the end bound strictly in
/// UTF-16 units — raw byte columns overshoot and are rejected, leaving
/// the store empty and this test failing.
#[test]
fn arc1c_range_only_utf16_server_gets_converted_bounds() {
use pmacs::editor::EditorState;
let dir = tempfile::tempdir().expect("tempdir");
let a_path = dir.path().join("a.rs");
std::fs::write(&a_path, "fn a() {}\nlet x = \u{e9}\u{e9};".as_bytes()).expect("write a");
let a_disp = a_path.display().to_string();
let mut state = EditorState::new();
let fake = fake_lsp_path();
state
.lua_host
.lua()
.load(format!(
"pmacs.lsp.config.rust = {{
command = '{fake}',
env = {{ PMACS_FAKE_LSP_MODE = 'rangeonly16' }},
}}"
))
.exec()
.expect("override rust config");
state
.lua_host
.lua()
.load(format!("pmacs.buffer.find_or_open('{a_disp}')"))
.exec()
.expect("open a.rs");
let has_tokens = format!(
"(function() \
local sid \
for _,r in ipairs(pmacs.lsp.list()) do \
if r.state and r.state.kind=='initialized' then sid=r.id end \
end \
if not sid then return false end \
local t = pmacs.semantic_tokens.tokens(sid, 'file://{a_disp}') \
return t ~= nil and #t > 0 \
end)()"
);
assert!(
pump_lua_flag(&mut state, &has_tokens, 5),
"a UTF-16 range-only server must receive converted (not byte) columns"
);
}
/// Arc 1d — a server-declared NON-ASCII trigger character works. LSP
/// trigger characters are strings; the fake declares "«" (2 UTF-8
/// bytes), and the codepoint-aware `char_before` must match it.