Skip to content

Commit cbb1394

Browse files
author
Yogthos
committed
feat(lsp): B3-10 — pull-diagnostic fallback for lazy LSP servers
lsp/client.rs:261-289 wait_for_push is push-only. For servers that don't push on demand — clojure-lsp's lazy semantic check, jdtls during cold-start, clangd before background indexing — the 10s DIAGNOSTIC_WAIT times out and the post-write/edit diagnostic block reports CLEAN even when errors actually exist. opencode races push against textDocument/diagnostic (LSP 3.17) at lsp/client.ts:540-582. New wait_for_push_or_pull: 1. Send textDocument/diagnostic pull request with 3s bound. 2. On success: inject items into state.push + last_push_at, bump push_signal so concurrent waiters wake. Return Ok. 3. On error (method not found, transport): fall back to push wait for the remaining time budget (min 100ms). Net behaviour for push-only servers is identical to the prior wait_for_push. Sequential pull-then-push rather than tokio::select! because the select! version had a future-move issue — pull-success cases don't need the push at all, so the sequential path is simpler and avoids re-pinning. State injection is the key piece: callers downstream (e.g. write/edit's append_lsp_block) read state via diagnostics_for(path) which is agnostic to push vs pull origin. After this fix, lazy servers' diagnostics surface through the same code path as push servers. manager.rs touch_file() switches from wait_for_push to wait_for_push_or_pull so all tool-end diagnostic waits benefit. Tests: existing 123 lsp tests still pass; no new tests added since the pull path requires a mock LSP server (no such fixture in tree). The control-flow shape is straightforward and the fallback to push preserves prior behaviour on unsupported servers. Closes dirge-ims. Batch 3 complete — all 10 items closed. Audit cycle done: - Batch 1 (8 critical security/data-loss): 4 commits - Batch 2 (3 context-fidelity): 1 commit - Batch 3 (10 polish + features): 5 commits - 21 audit issues closed across 10 commits.
1 parent 927b9b6 commit cbb1394

3 files changed

Lines changed: 105 additions & 3 deletions

File tree

.beads/issues.jsonl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
{"_type":"issue","id":"dirge-9f1","title":"Chat history ignores 120-col content_width cap","description":"max_line_width and wrap_line use raw content_cols, so on wide terminals scrollback overflows the centered band into divider/panel margin.","status":"closed","priority":1,"issue_type":"bug","assignee":"Yogthos","owner":"yogthos@gmail.com","created_at":"2026-05-21T22:17:33Z","created_by":"Yogthos","updated_at":"2026-05-21T22:26:36Z","started_at":"2026-05-21T22:17:42Z","closed_at":"2026-05-21T22:26:36Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
1717
{"_type":"issue","id":"dirge-woq","title":"R1: fix 3 critical plugin bugs (FFI panic, dialog deadlock, init hang)","description":"From the plugin subsystem audit: (1) wrap JanetCFunctions in catch_unwind so Rust panics don't unwind across the C-FFI boundary into Janet; (2) cancel send_dialog's reply_rx.recv() on worker shutdown so the worker thread doesn't block forever when the UI exits mid-dialog; (3) add timeout to the init handshake so a worker panic before init_tx.send() doesn't hang the main thread. Also: (4) bounds-assert wrap_string's i32 cast for the unlikely \u003e2GB case, (5) make take_string_slot atomic to close the race window, (6) don't eat unrelated user events in the dialog arm.","status":"closed","priority":1,"issue_type":"bug","owner":"yogthos@gmail.com","created_at":"2026-05-20T14:59:57Z","created_by":"Yogthos","updated_at":"2026-05-20T15:30:28Z","started_at":"2026-05-20T15:00:10Z","closed_at":"2026-05-20T15:30:28Z","dependency_count":0,"dependent_count":1,"comment_count":0}
1818
{"_type":"issue","id":"dirge-84j","title":"B3-9: edit tool fuzzy-match cascade","description":"edit.rs:155-160 returns 'old_text not found' on whitespace/indent/CRLF drift. LLMs frequently hit this. opencode edit.ts:222-432 has simple → lineTrimmed → whitespace-normalized → indentation-flexible → levenshtein. pi edit-diff.ts:91-132 has fuzzyFindText. Port the cascade.","status":"closed","priority":2,"issue_type":"feature","assignee":"Yogthos","owner":"yogthos@gmail.com","created_at":"2026-05-23T03:24:44Z","created_by":"Yogthos","updated_at":"2026-05-23T03:43:39Z","started_at":"2026-05-23T03:37:16Z","closed_at":"2026-05-23T03:43:39Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
19-
{"_type":"issue","id":"dirge-ims","title":"B3-10: LSP pull-diagnostic fallback for lazy servers","description":"lsp/client.rs:261-289 wait_for_push is push-only. clojure-lsp/jdtls/clangd may not push on demand. 10s timeout = 'clean' diagnostic block reported when errors exist. opencode lsp/client.ts:540-582 races push-wait against requestDocumentDiagnostics. Add pull fallback.","status":"in_progress","priority":2,"issue_type":"feature","assignee":"Yogthos","owner":"yogthos@gmail.com","created_at":"2026-05-23T03:24:44Z","created_by":"Yogthos","updated_at":"2026-05-23T03:43:41Z","started_at":"2026-05-23T03:43:41Z","dependency_count":0,"dependent_count":0,"comment_count":0}
19+
{"_type":"issue","id":"dirge-ims","title":"B3-10: LSP pull-diagnostic fallback for lazy servers","description":"lsp/client.rs:261-289 wait_for_push is push-only. clojure-lsp/jdtls/clangd may not push on demand. 10s timeout = 'clean' diagnostic block reported when errors exist. opencode lsp/client.ts:540-582 races push-wait against requestDocumentDiagnostics. Add pull fallback.","status":"closed","priority":2,"issue_type":"feature","assignee":"Yogthos","owner":"yogthos@gmail.com","created_at":"2026-05-23T03:24:44Z","created_by":"Yogthos","updated_at":"2026-05-23T04:40:13Z","started_at":"2026-05-23T03:43:41Z","closed_at":"2026-05-23T04:40:13Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
2020
{"_type":"issue","id":"dirge-yep","title":"B3-8: soft-wrap + mouse selection display-width awareness","description":"renderer.rs:1457-1496 wrap_input measures chars().count() instead of UnicodeWidthChar. Cursor mis-lands on CJK/emoji wrapped lines. Mouse selection columns map 1:1 char-to-display rather than width-aware. Pi interactive-mode.ts uses string-width per cell.","status":"closed","priority":2,"issue_type":"bug","owner":"yogthos@gmail.com","created_at":"2026-05-23T03:24:43Z","created_by":"Yogthos","updated_at":"2026-05-23T03:37:14Z","closed_at":"2026-05-23T03:37:14Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
2121
{"_type":"issue","id":"dirge-0nk","title":"B3-7: Python find_callees captures method calls","description":"semantic/adapters/python.rs:280 only matches (call function:(identifier)); misses obj.method(). Go/Java/C++ all capture selector_expression equivalent. Python users get hollow callee lists.","status":"closed","priority":2,"issue_type":"bug","owner":"yogthos@gmail.com","created_at":"2026-05-23T03:24:42Z","created_by":"Yogthos","updated_at":"2026-05-23T03:33:51Z","closed_at":"2026-05-23T03:33:51Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
2222
{"_type":"issue","id":"dirge-8lj","title":"B3-5: doom-loop + allowlist reset on /cd","description":"permission/checker.rs:568-571 only updates cwd. recent_calls and session_allowlist survive across /cd — 'cd *' allowlist follows user to new project. Clear both on set_working_dir.","status":"closed","priority":2,"issue_type":"bug","owner":"yogthos@gmail.com","created_at":"2026-05-23T03:24:41Z","created_by":"Yogthos","updated_at":"2026-05-23T03:33:50Z","closed_at":"2026-05-23T03:33:50Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}

src/lsp/client.rs

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,97 @@ impl LspClient {
288288
}
289289
}
290290

291+
/// B3-10 (audit fix): race the push-wait against a pull request to
292+
/// `textDocument/diagnostic` (LSP 3.17). For servers that don't
293+
/// push diagnostics on demand (clojure-lsp's lazy semantic check,
294+
/// jdtls during cold-start, clangd before background indexing),
295+
/// the push-only wait times out and the agent reports a clean
296+
/// diagnostic block when errors actually exist. Pull diagnostics
297+
/// give the agent an authoritative answer.
298+
///
299+
/// On pull success the response's diagnostics are injected into
300+
/// the same state.push that the notification handler uses, so
301+
/// `diagnostics_for(path)` afterwards returns them transparently.
302+
/// On pull failure (method not found, server doesn't support
303+
/// 3.17, transport error) we fall back to the push-only wait —
304+
/// no regression versus the prior behaviour. Opencode races the
305+
/// same way (lsp/client.ts:540-582 `waitForDocumentDiagnostics`).
306+
pub async fn wait_for_push_or_pull(
307+
&self,
308+
path: &Path,
309+
after: Instant,
310+
timeout: Duration,
311+
) -> Result<(), LspError> {
312+
// Sequential: try the pull first with a short bound; if
313+
// unsupported / transport error, fall back to push-only
314+
// for the remaining time. Net behaviour for push-only
315+
// servers is identical to the prior wait_for_push call.
316+
// Pull-supporting servers (LSP 3.17) return an answer
317+
// synchronously and we skip the push wait entirely.
318+
let pull_started = Instant::now();
319+
let pull_timeout = timeout.min(Duration::from_secs(3));
320+
if let Ok(true) = self.try_pull_diagnostics(path, pull_timeout).await {
321+
return Ok(());
322+
}
323+
// Pull either failed (method not found, transport error)
324+
// or returned no useful info. Wait on push for whatever
325+
// budget remains.
326+
let elapsed = pull_started.elapsed();
327+
let push_budget = timeout.saturating_sub(elapsed);
328+
// If pull burned the full timeout, still give push at
329+
// least a brief window (100ms) — better than instant-fail.
330+
let push_budget = push_budget.max(Duration::from_millis(100));
331+
self.wait_for_push(path, after, push_budget).await
332+
}
333+
334+
/// Send a `textDocument/diagnostic` (LSP 3.17) pull request and
335+
/// inject any returned items into the client's push state so
336+
/// downstream callers see them via `diagnostics_for`. Returns
337+
/// `Ok(true)` if the server responded with diagnostics (full or
338+
/// unchanged), `Ok(false)` if the server responded with no items
339+
/// or doesn't support pull. `Err` only for transport-level
340+
/// failures the caller should surface.
341+
async fn try_pull_diagnostics(&self, path: &Path, timeout: Duration) -> Result<bool, LspError> {
342+
use serde_json::json;
343+
let uri = crate::lsp::uri::path_to_file_uri_string(path);
344+
let params = json!({
345+
"textDocument": { "uri": uri },
346+
});
347+
// The reply shape can be one of several Document Diagnostic
348+
// Report kinds; just deserialize to Value and pick `items`.
349+
let resp: Result<serde_json::Value, _> = self
350+
.rpc
351+
.request("textDocument/diagnostic", params, timeout)
352+
.await;
353+
let value = match resp {
354+
Ok(v) => v,
355+
Err(_) => return Ok(false), // unsupported / errored — let push handle it
356+
};
357+
// Full report: { kind: "full", items: [...] }.
358+
// Unchanged report: { kind: "unchanged", resultId: ... } —
359+
// no new diagnostics, but successful response means "no
360+
// errors right now" → return true so the caller doesn't
361+
// block on push.
362+
let kind = value.get("kind").and_then(|k| k.as_str()).unwrap_or("");
363+
if kind == "unchanged" {
364+
return Ok(true);
365+
}
366+
let items: Vec<lsp_types::Diagnostic> = value
367+
.get("items")
368+
.and_then(|i| serde_json::from_value(i.clone()).ok())
369+
.unwrap_or_default();
370+
let abs = path.to_path_buf();
371+
{
372+
let mut state = self.inner.lock().unwrap_or_else(|e| e.into_inner());
373+
state.push.insert(abs.clone(), items);
374+
state.last_push_at.insert(abs, Instant::now());
375+
}
376+
// Wake any concurrent wait_for_push waiters since we just
377+
// populated state — they'll detect a fresh push.
378+
self.push_signal.send_modify(|v| *v = v.wrapping_add(1));
379+
Ok(true)
380+
}
381+
291382
fn has_fresh_push(&self, path: &Path, after: Instant) -> bool {
292383
let state = self.inner.lock().unwrap_or_else(|e| e.into_inner());
293384
state

src/lsp/manager.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -423,8 +423,19 @@ impl LspManager {
423423
}
424424
if let TouchMode::AwaitPush { after, timeout } = mode {
425425
let after = std::cmp::max(after, send_at);
426-
if let Err(e) = entry.client.wait_for_push(path, after, timeout).await {
427-
tracing::debug!(server = %entry.server_id, path = %path.display(), "wait_for_push: {e}");
426+
// B3-10 (audit fix): race push against textDocument/
427+
// diagnostic pull so lazy servers (clojure-lsp,
428+
// jdtls, clangd cold-start) still surface errors
429+
// instead of reporting clean diagnostics on
430+
// timeout. Pull silently falls back to push-only
431+
// when unsupported; net behaviour for push-only
432+
// servers is identical.
433+
if let Err(e) = entry
434+
.client
435+
.wait_for_push_or_pull(path, after, timeout)
436+
.await
437+
{
438+
tracing::debug!(server = %entry.server_id, path = %path.display(), "wait_for_push_or_pull: {e}");
428439
}
429440
}
430441
}

0 commit comments

Comments
 (0)