Skip to content

Commit cf5aad5

Browse files
author
Yogthos
committed
LSP Phase 9: README + CONFIG docs + manual test plan
README.md - New ## LSP integration section above semantic-tools. Covers the tool matrix (read warms / write+edit surface diagnostics / lsp tool for ad-hoc queries), the four built-in servers + their root resolution, and the disable mechanism (--no-lsp / lsp:false in config). CONFIG.md - New ## LSP configuration section between MCP and ACP. Documents the three accepted forms (bool / per-server map), per-server fields, CLI flag, built-in server commands, lazy spawn + dedupe behavior, known limitations (extensions override currently ignored; 4 built-ins in v1). - Links to docs/LSP_MANUAL_TEST.md for the live-server smoke test. docs/LSP_MANUAL_TEST.md (new) - Six scenarios for verifying the end-to-end LSP path that unit tests can't reach (real rust-analyzer process): 1. Diagnostic surfacing on a deliberately broken edit 2. lsp tool: hover at a known position 3. workspaceSymbol search returns matches 4. Concurrent edits don't spawn duplicate servers 5. --no-lsp + --no-default-features path 6. Broken spawn doesn't retry (broken-set behavior) - Failure-mode notes for each scenario. - Explicit list of what the manual test does NOT cover (already exercised by unit tests). No source changes. Tests still pass (426). End of the LSP stack.
1 parent f80c293 commit cf5aad5

3 files changed

Lines changed: 212 additions & 0 deletions

File tree

CONFIG.md

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,62 @@ adds a default Exa Web Search MCP server at `https://mcp.exa.ai/mcp` with the
122122
`x-api-key` header set to `EXA_API_KEY` when that environment variable is set.
123123
Set `"mcp_servers": {}` to disable all MCP servers.
124124

125+
## LSP configuration
126+
127+
When compiled with the `lsp` feature (default-on), dirge spawns language
128+
servers on demand to surface compile errors in tool output. The `lsp` config
129+
key accepts three forms:
130+
131+
```json
132+
// Default-on, built-in commands for rust/typescript/pyright/clojure-lsp.
133+
{ "lsp": true }
134+
135+
// Off entirely. Same as the --no-lsp CLI flag.
136+
{ "lsp": false }
137+
138+
// Default-on with per-server overrides.
139+
{
140+
"lsp": {
141+
"rust": {
142+
"command": ["rust-analyzer"],
143+
"env": { "RA_LOG": "rust_analyzer=debug" },
144+
"initialization": { "cargo": { "buildScripts": { "enable": true } } }
145+
},
146+
"typescript": { "disabled": true }
147+
}
148+
}
149+
```
150+
151+
Per-server fields (all optional):
152+
153+
| Field | Type | Description |
154+
| ---------------- | ---------------- | ----------- |
155+
| `command` | string[] | argv to launch the server. Replaces the built-in default. |
156+
| `extensions` | string[] | *Reserved.* Currently ignored — see "Known limitations" below. |
157+
| `env` | object | extra env vars for the child process. |
158+
| `initialization` | object | sent as `initializationOptions` in the LSP `initialize` request. |
159+
| `disabled` | boolean | `true` removes the server entirely. |
160+
161+
CLI flag: `--no-lsp` (overrides the config; same effect as `lsp: false`).
162+
163+
### Built-in server commands
164+
165+
| Server id | Default command |
166+
| ------------- | -------------------------------------------- |
167+
| `rust` | `rust-analyzer` |
168+
| `typescript` | `typescript-language-server --stdio` |
169+
| `pyright` | `pyright-langserver --stdio` |
170+
| `clojure-lsp` | `clojure-lsp` |
171+
172+
Servers are spawned lazily on first file touch and cached per `(workspace_root, server_id)` pair. Concurrent agent tool calls for the same file deduplicate so dirge never races two `rust-analyzer` processes against one workspace.
173+
174+
### Known limitations
175+
176+
- The `extensions` override is currently ignored. The claimed-extensions list lives in the static `builtin_servers()` registry at `src/lsp/server.rs`. Adding new extensions today requires editing that file. Follow-up.
177+
- v1 has four built-in servers. Additional servers can be added by extending `builtin_servers()` + `ProcessSpawner::default_commands()` in source.
178+
179+
For an end-to-end smoke test against a real `rust-analyzer` process, see [`docs/LSP_MANUAL_TEST.md`](docs/LSP_MANUAL_TEST.md).
180+
125181
## ACP (Agent Communication Protocol) configuration
126182

127183
When compiled with the `acp` feature, dirge can act as an ACP agent server.

README.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,30 @@ Create `~/.config/dirge/plugins/my-plugin.janet`:
255255
"\n- Authentication bypass"))))
256256
```
257257

258+
## LSP integration
259+
260+
When built with the `lsp` feature (on by default), dirge attaches Language Server Protocol clients to your project and surfaces compile-time diagnostics directly in the agent's tool output. After every `write` or `edit`, the LSP server gets a `didChange`, waits for a fresh diagnostic publish, and any ERRORs land in the tool result as a `<diagnostics file="...">` block — so the agent corrects compile errors on the same turn instead of writing broken code and discovering it later via `cargo check`.
261+
262+
| Tool | Effect |
263+
|------|--------|
264+
| `read` | Fire-and-forget `didOpen` so the server has the file in memory by the time the agent edits it. No diagnostic block in `read` output. |
265+
| `write` | After write: `didChange` + wait for diagnostics + append errors-block. |
266+
| `edit` | Same as `write`. |
267+
| `lsp` | Agent-facing tool that exposes `definition`, `references`, `hover`, `documentSymbol`, `workspaceSymbol`, `implementation`, `prepareCallHierarchy`, `incomingCalls`, `outgoingCalls`. 1-based coordinates. |
268+
269+
Built-in server set:
270+
271+
| Server id | Binary | Extensions |
272+
|-----------|--------|------------|
273+
| `rust` | `rust-analyzer` | `.rs` |
274+
| `typescript` | `typescript-language-server --stdio` | `.ts`, `.tsx`, `.mts`, `.cts`, `.js`, `.jsx`, `.mjs`, `.cjs` |
275+
| `pyright` | `pyright-langserver --stdio` | `.py`, `.pyi` |
276+
| `clojure-lsp` | `clojure-lsp` | `.clj`, `.cljs`, `.cljc`, `.edn`, `.bb` |
277+
278+
Workspace root resolution is per-server: rust-analyzer walks past nested member crates to the workspace `Cargo.toml` declaring `[workspace]`; typescript stops at the nearest `package.json`/`tsconfig.json` and yields to deno when a `deno.json` is closer; pyright looks for `pyproject.toml`/`setup.py`/etc.; clojure-lsp looks for `deps.edn`/`project.clj`/`shadow-cljs.edn`/`bb.edn`/`.clj-kondo`.
279+
280+
Disable: `--no-lsp` flag or `{ "lsp": false }` in the config. Per-server overrides (custom command, env, init options) live in the config — see [CONFIG.md](CONFIG.md).
281+
258282
## Semantic code tools
259283

260284
When built with `--features "semantic,semantic-ts,semantic-python"`, dirge gains AST-powered code analysis:

docs/LSP_MANUAL_TEST.md

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
# LSP integration — manual end-to-end test
2+
3+
The LSP integration is covered by ~120 unit tests against mock spawners and
4+
duplex pipes. This document captures the manual smoke test against a real
5+
`rust-analyzer` process — the one thing CI can't reproduce.
6+
7+
## Prerequisites
8+
9+
- `rust-analyzer` on `$PATH`. Verify with:
10+
```bash
11+
which rust-analyzer && rust-analyzer --version
12+
```
13+
If absent, `rustup component add rust-analyzer` from the default toolchain.
14+
- A Rust project on disk. The dirge repo itself works.
15+
16+
## Scenario 1 — diagnostic surfacing on edit
17+
18+
**Goal**: after a deliberately broken edit, the `edit` tool's output
19+
contains a `<diagnostics file="...">` block with the compile error.
20+
21+
1. Start dirge in the dirge repo: `cargo run --release`.
22+
2. Wait for the initial prompt.
23+
3. Type:
24+
```
25+
Edit src/agent/builder.rs and change `pub async fn build_agent_inner` to
26+
`pub async fnn build_agent_inner` (typo: fnn instead of fn). After the
27+
edit, show me the exact tool output you got back.
28+
```
29+
4. **Expected behavior**:
30+
- The agent calls the `edit` tool, the file is mutated.
31+
- Within ~10 seconds (the `DIAGNOSTIC_WAIT` constant), the agent receives
32+
the tool result containing a section like:
33+
```
34+
LSP errors detected in this file, please fix:
35+
<diagnostics file="/Users/.../src/agent/builder.rs">
36+
ERROR [N:M] expected one of `(`, `[`, `;`, or `<`, found `build_agent_inner`
37+
...
38+
</diagnostics>
39+
```
40+
- The agent then proposes a corrective edit (`fn` not `fnn`).
41+
5. Ctrl+C to abort, then revert: `git checkout src/agent/builder.rs`.
42+
43+
**Failure modes to watch for**:
44+
- No diagnostic block → check `rust-analyzer` is on PATH and the project has
45+
built at least once.
46+
- Diagnostic block but with WARN entries → bug: only ERRORs should surface.
47+
- Block appears but takes >15 seconds → bug: bounded wait isn't firing.
48+
49+
## Scenario 2 — `lsp` tool: hover at a known position
50+
51+
**Goal**: `lsp` tool dispatches `hover` and returns server-side info.
52+
53+
1. From inside dirge:
54+
```
55+
Use the lsp tool to hover at src/main.rs line 1 character 1.
56+
```
57+
2. **Expected behavior**:
58+
- The agent calls `lsp` with
59+
`{"operation": "hover", "file_path": "src/main.rs", "line": 1, "character": 1}`.
60+
- Result is pretty-printed JSON from rust-analyzer: a `contents` field
61+
with the hovered-token info. Even at "mod agent;" position 1, the
62+
server may return null hover info; the tool reports
63+
`(no results from hover)`.
64+
- **Validation**: cursor at a real identifier (say, "build_agent" inside
65+
`src/main.rs`) should return non-empty hover content.
66+
67+
## Scenario 3 — workspace symbol search
68+
69+
**Goal**: `lsp.workspaceSymbol` returns matches across the workspace.
70+
71+
1. From inside dirge:
72+
```
73+
Use the lsp tool with workspaceSymbol operation to find symbols
74+
matching "build_agent_inner". Pass src/main.rs as file_path (just to
75+
pick the workspace).
76+
```
77+
2. **Expected behavior**:
78+
- Pretty-printed JSON array with at least one entry pointing to
79+
`src/agent/builder.rs`.
80+
81+
## Scenario 4 — concurrent file edits don't spawn duplicate servers
82+
83+
**Goal**: the inflight-spawn dedupe works end-to-end.
84+
85+
1. Open two dirge sessions in different terminals, same repo.
86+
2. In both, ask the agent to read `src/main.rs`. The first read triggers a
87+
spawn; the second should reuse the cached client (verifiable via
88+
`ps aux | grep rust-analyzer` — only one process per workspace within
89+
a single dirge session; across sessions there's one process per).
90+
3. **Expected behavior**: across the lifetime of one dirge session,
91+
`ps aux | grep rust-analyzer | wc -l` returns 1 (the workspace root
92+
stays the same, so one server services all .rs touches).
93+
94+
## Scenario 5 — `--no-lsp` actually disables
95+
96+
**Goal**: feature gate / CLI flag work.
97+
98+
1. Run `cargo run --release -- --no-lsp` and ask the agent to edit a Rust
99+
file. The tool output must NOT contain any `<diagnostics>` block.
100+
2. Run `cargo run --release --no-default-features --features 'loop git-worktree mcp'`
101+
and verify the binary builds and runs. The `lsp` tool and diagnostic
102+
block both should be absent.
103+
104+
## Scenario 6 — broken spawn doesn't retry
105+
106+
**Goal**: failed spawn marks (root, server_id) as broken so subsequent
107+
file touches don't re-spawn.
108+
109+
1. Disable rust-analyzer temporarily:
110+
```bash
111+
mv "$(which rust-analyzer)" "$(which rust-analyzer).bak"
112+
```
113+
2. Start dirge, ask to read a `.rs` file. First touch triggers a spawn;
114+
it fails (binary missing).
115+
3. Ask to read another `.rs` file. Watch for log lines (run with
116+
`RUST_LOG=warn`): the second read should NOT log "spawn failed" — the
117+
broken-set blocks the retry.
118+
4. Restore: `mv "$(which rust-analyzer).bak" "$(which rust-analyzer)"`.
119+
120+
## What this test plan deliberately doesn't cover
121+
122+
These are exercised by unit tests against the mock spawner and don't need
123+
manual verification:
124+
- JSON-RPC framing edge cases (multi-message buffers, partial reads).
125+
- Request correlation by id under concurrent in-flight requests.
126+
- Diagnostic dedupe + MAX_PER_FILE caps.
127+
- Push-vs-pull diagnostic merge.
128+
- URI ↔ path round-tripping with special characters.
129+
- Config schema parsing.
130+
131+
See the corresponding tests under `src/lsp/**/tests::` for the contracts
132+
those features guarantee.

0 commit comments

Comments
 (0)