# Neovim LSP lifecycle — inspiration for pi-lens

A study of the LSP client/framework shipped inside [neovim/neovim](https://github.com/neovim/neovim)
(via `runtime/lua/vim/lsp/*` and `runtime/doc/lsp.txt`) compared to pi-lens's
`clients/lsp/*` and `clients/dispatch/runners/lsp.ts`. This is **inspiration,
not a port target** — the runtime models are very different (interactive
editor vs agent hook with cold LSP per edit), and the surface is smaller in
pi-lens. The goal is to find concrete patterns worth borrowing and to
flag where pi-lens already does the right thing.

Every observation is asserted against the current pi-lens source — see the
**[assertions](#assertions-checklist-against-the-current-codebase)** at the
end. References to neovim files are to `master` at the time of writing.

---

## TL;DR — what to steal

Ranked by expected value and blast-radius for a single-commit follow-up. None
of these is a refactor; all are additive.

| # | Pattern | pi-lens today | Take | Why it matters |
|---|---|---|---|---|
| 1 | **`ctx.version` for stale-response rejection on response handlers** | Only `bumped` `diagnosticsVersion` counters; neovim-style `ctx.version` is absent. | Adopt the version-on-`ctx` shape for the **navigation** response path (definition/references/hover) — currently a 10 s request timeout can return a result computed against a *pre-edit* document. | Already touches the hot path; version-based drop is one line in `navRequest`. |
| 2 | **`vim.lsp.handlers` resolution order (global → per-start → per-request)** | pi-lens's `setupIncomingHandlers` registers one global handler per client; per-request overrides are not a thing. | Map our `pilens_lsp_navigation` MCP tool to per-request handler overrides so a single call can request `textDocument/definition` and receive a custom-shaped result without re-implementing the RPC plumbing. | The MCP surface already mirrors pi-lens internals — this is a small step. |
| 3 | **Capability-aware `_capability.lua` class hierarchy with `new(bufnr)` instance-per-buffer** | `LSPClientState` is monolithic; capability availability is exposed as a single `operationSupport` map + `getCapabilitySnapshots()`. | Treat each `LSPService` capability (rename / codeAction / documentSymbol / workspaceSymbol / inlayHint) as a per-client instance with its own `enabled` gate, registered in a list. | The "raw capability keys" snapshot is currently a flat dump; a registry would let `lens_health` distinguish "server doesn't implement X" from "server implements X but didn't register a handler". |
| 4 | **Debounce + queue for `workspace/didChangeWatchedFiles`** | We send `didChangeWatchedFiles` per touch (capped to 1 per file per touch). | Borrow neovim's `change_queues` (URI → last-type cache + libuv timer) so a `tool_result` flurry becomes **one** batched notification per `serverId`. | Cheap; reduces push-diagnostics re-analysis on slow servers (typescript) where the workspace-wide walk is expensive. |
| 5 | **Per-buffer position-encoding registration (`offsetEncoding` / `positionEncoding`)** | Not handled — every request uses raw UTF-16 indices the server reports. | Detect UTF-8 vs UTF-16 from the server's `capabilities.positionEncoding` and translate our line/character positions before sending `textDocument/*Position` requests. | Several recent servers (rust-analyzer 2024+) report UTF-8; this is a real bug-in-waiting. |
| 6 | **Soft cancel with `window/workDoneProgress/create` token + `$/cancelRequest`** | Not present; the only abort path is the ambient `AbortSignal` (Esc). | Add per-request cancellation via a token; on interrupt, send `$/cancelRequest` for in-flight nav requests instead of letting them time out. | 10 s of dead time per Esc is wasted CPU + battery in interactive use. |
| 7 | **`vim.lsp.start`'s "config merge" semantic for `root_markers = { { a, b }, '.git' }`** | `PriorityRoot([[...], [...]])` (in `clients/lsp/server.ts`) does the same — *we already do this.* | Document the alignment + extract a `priority-marker-builder` helper so config writers don't have to know about it. | Already 80% there; readability only. |
| 8 | **`vim.lsp.util.apply_text_edits` + `apply_workspace_edit` reorder + overlap check** | `applyTextEditsToString` (in `clients/lsp/edits.ts`) does **exactly** the same sorted-application + overlap-throw. | No-op — keep as-is. The neovim source is broader (handles `documentChanges` of every kind); we only need to add `TextDocumentEdit.optionalEdit` handling if a server starts sending it. | Documentation reference, not a take. |
| 9 | **Server-initiated `workspace/applyEdit` gated by a server-side `serverEditsAllowed` counter** | We already do this (`state.serverEditsAllowed += 1` around `executeCommand`). | No-op. | Same hardening model; nice to cite as a precedent. |

Below: the per-pattern deep-dive with neovim source pointer, the pi-lens
counterpart, the code we'd add, and the risk/blast-radius call.

---

## 1. `ctx.version` for stale-response rejection (HIGH value)

### Neovim's model

Every LSP response handler receives a `ctx` table that includes the
**document version at time of request**:

> `vim.lsp.ResponseHandler` — `{ method, client_id, bufnr, params, version }`.
> Handlers can compare this to the current document version to check if the
> response is "stale". See also `b:changedtick`.
> — `runtime/doc/lsp.txt` (handlers section)

The handler in `runtime/lua/vim/lsp/buf.lua` then uses it:

```lua
-- pseudo, paraphrased from vim.lsp.buf.definition / definition handler
local function on_response(err, result, ctx)
  if not ctx.bufnr or not vim.api.nvim_buf_is_valid(ctx.bufnr) then return end
  if ctx.version and vim.lsp.util.buf_versions[ctx.bufnr] ~= ctx.version then
    return  -- stale; document has changed since the request was sent
  end
  -- … apply result
end
```

The version is captured at **request send time** on the client side (it
mirrors `textDocument/didChange`'s monotonically-increasing version). When
the response lands, the handler checks that the buffer's current version
still matches the request's version — if not, the result was computed
against a stale document, so it's silently dropped.

### pi-lens today

- `LSPClientState` keeps `documentVersions: Map<string, number>` and
  `diagnosticDocVersions: Map<string, number>`. The latter is used in
  `isVersionStale()` *only on the diagnostics path* — confirmed in
  `clients/lsp/client.ts:903`:
  ```ts
  const cachedVersion = state.diagnosticDocVersions?.get(normalizedPath);
  if (cachedVersion === undefined) return false;
  const currentVersion = state.documentVersions?.get(normalizedPath);
  return currentVersion !== undefined && cachedVersion < currentVersion;
  ```
- The **navigation** path (`navRequest`, `clients/lsp/client.ts` near line
  ~1500 — the `definition`/`references`/etc. methods) **does not** check
  staleness. It just `await`s the response inside a 10 s timeout
  (`NAV_REQUEST_TIMEOUT_MS`, `clients/lsp/client.ts:316`). The response is
  then returned to the caller and applied even if the user has edited the
  file in between.

This is a real bug-in-waiting for `lens_navigate` / `module_report` /
`pilens_lsp_navigation`: the agent asks for "go to definition of
`renderItem`", the LSP spends 600 ms computing it, in those 600 ms the
agent edits the file (a synchronous cycle can happen during a tool
sequence), the result lands referring to the old definition.

### Take

Add a `requestVersion` to the per-request context captured in `navRequest`:

```ts
// in clients/lsp/client.ts — sketch
async function navRequest<T>(
    state: LSPClientState,
    method: string,
    filePath: string,
    params: Record<string, unknown>,
): Promise<T | null | undefined> {
    if (!isClientAlive(state)) return null;
    const requestVersion = state.documentVersions.get(normalizeMapKey(filePath));
    return withTimeout(
        safeSendRequest<T>(state.connection, method, params),
        NAV_REQUEST_TIMEOUT_MS,
    ).then((result) => {
        // Drop stale results — the document was edited during the request.
        const currentVersion = state.documentVersions.get(normalizeMapKey(filePath));
        if (
            requestVersion !== undefined &&
            currentVersion !== undefined &&
            currentVersion > requestVersion
        ) {
            return undefined;
        }
        return result;
    }).catch((err: unknown) => {
        if (err instanceof Error && err.message.startsWith("Timeout after")) {
            return undefined;
        }
        throw err;
    }) as Promise<T | undefined>;
}
```

This requires:
1. A new optional `filePath` argument on `navRequest` (currently positional
   `line, character` is the second+third arg). All 12 call sites in
   `LSPClientInfo` need updating.
2. A new constant `NAV_STALE_VERSION_DROP=1` to gate the check via
   `PI_LENS_LSP_NAV_STALE_DROP` so a regression can be flipped off.

**Blast radius:** medium. The 12 nav methods all need their
`navRequest(...)` calls updated. Trivially testable: a unit test that
touches a file mid-request, then asserts the result is dropped.

### Why we should not over-generalise

The diagnostic path *already* does staleness via
`isVersionStale()`. We don't need a generic "version-aware response
handler" — just bring the navigation path in line.

---

## 2. `vim.lsp.handlers` resolution order (MEDIUM value)

### Neovim's model

`runtime/doc/lsp.txt` documents four precedence levels for the
**server-to-client** handler (`textDocument/publishDiagnostics`,
`workspace/applyEdit`, `window/workDoneProgress/create`, etc.):

> 1. **Directly calling** an LSP method via `Client:request()` — overrides
>    everything else.
> 2. **Setting a field in `vim.lsp.handlers`** — global default for
>    client-to-server responses? No — actually, for **server-to-client**
>    requests/notifications, the global default.
> 3. **Passing a `handlers` parameter to `vim.lsp.start()`** — default for
>    a specific server.
> 4. **Passing a `handler` parameter to `vim.lsp.buf_request_all()`** —
>    overrides for the given request only.

The key insight: the **server-to-client** direction is the one that needs
the most runtime customisation, because that's the surface the user
actually sees (diagnostics rendered, completions popped, hover shown). The
client's own request/response handling is fixed; only the *response side*
varies.

### pi-lens today

`setupIncomingHandlers` in `clients/lsp/client.ts` (lines ~660-770)
registers a fixed set of handlers once per client:
- `textDocument/publishDiagnostics` — fixed debounce + dedup logic
- `workspace/workspaceFolders` — fixed response
- `client/registerCapability` — fixed dynamic-registration merge
- `client/unregisterCapability` — fixed dynamic-registration removal
- `workspace/applyEdit` — fixed "only during executeCommand" gate
- `workspace/configuration` — fixed initialisation-options passthrough
- `window/workDoneProgress/create` — fixed no-op

The dispatch-lsp-runner never overrides any of these. The `pilens_*` MCP
tools and the `lsp_navigation` / `lsp_diagnostics` pi tools also never
override them.

### Take

Expose per-request handler overrides for the **response side** of
client-to-server requests — i.e. the *return* of `definition`,
`references`, `hover`, `codeAction`, `rename`, `incomingCalls`,
`outgoingCalls`, `prepareCallHierarchy`. The `executeCommand` / `format`
side already goes through a single code path that returns raw data, but
the **diagnostics** response and the **executeCommand** response could
benefit from a "give me the raw LSP response" passthrough — currently
`executeCommand` returns `{ executed, result, reason }` and the caller
must re-parse the LSP-shaped `result`. A MCP `pilens_lsp_diagnostics`
could pass a `handler` to get the exact untransformed response back.

This is a relatively cheap extension to `LSPService`:

```ts
// sketch — clients/lsp/index.ts
async definition(
    filePath: string, line: number, character: number,
    opts?: { onResult?: (raw: LSPOne | LSPOne[]) => void },
) {
    const spawned = await this.getClientForFile(filePath, NAV_CLIENT_WAIT_TIMEOUT_MS);
    if (!spawned) return [];
    return spawned.client.definition(filePath, line, character, opts);
}
```

…but the simpler form is a one-off MCP tool: `pilens_lsp_raw_diagnostic`
that takes a `method` and `params` and forwards the untransformed
response, on top of the existing per-method wrappers.

**Blast radius:** low if scoped to a new MCP tool; medium if we plumb a
generic `onResult` callback through `navRequest`.

### Why this is a "nice to have" not a "must"

We don't currently have a use case where a caller wants to see the raw
LSP response. The MCP tool surface is the obvious place to expose it
without changing existing callers.

---

## 3. Capability-aware per-instance handlers (MEDIUM value, big lift)

### Neovim's model

`runtime/lua/vim/lsp/_capability.lua` defines an abstract `Capability`
class hierarchy. For each buffer that has at least one supported client
attached, **exactly one instance of each concrete subclass is created**.
That instance is destroyed once all supporting clients detach from the
buffer.

Concrete subclasses include `codelens`, `document_color`, `semantic_tokens`,
`folding_range`, `linkededitingrange`, `inline_completion` — each owns:

- `name` — capability id
- `method` — the LSP method the capability requires
- `active` — per-buffer instance map (buffer number → instance)
- `bufnr` — the buffer it's bound to
- `augroup` — its owned autocommand group, cleared on destruction
- `client_state` — per-client state data

The lifecycle is **per-buffer, per-client**:
1. On `LspAttach`, the capability checks `client:supports_method(method)`.
2. If supported, an instance is created and bound to the buffer.
3. On `LspDetach`, the instance is destroyed (augroup cleared, handlers
   removed).

This is a clean way to avoid "is the codelens handler registered for
buffer X?" queries by making the *existence* of the instance the
authoritative answer.

### pi-lens today

`LSPClientState` has a single `operationSupport: LSPOperationSupport`
booleans map, and `applyDynamicCapabilities` mutates it as
`client/registerCapability` events arrive. The `runtime` (everything
that uses these flags) is centralised: `lens_navigation`, `lens_module_report`,
`lens_diagnostics` all look up `client.getOperationSupport()` at call
time. This is fine for a stateless "what can this client do right now"
query, but doesn't model the **per-buffer, per-client, per-capability**
lifecycle that neovim supports.

We also have `rawCapabilityKeys: string[]` — captured once at initialize
— which is the closest analog to neovim's `client.server_capabilities`.
The current surface in `getCapabilitySnapshots()` is a flat
`{ serverId, root, operationSupport, workspaceDiagnosticsSupport,
advertisedCommands, rawCapabilityKeys }` tuple.

### Take

Treat the **named capabilities** (rename, codeAction, documentSymbol,
workspaceSymbol, inlayHint, completion, signatureHelp) as a registry of
per-client instances, each with:
- A `method` (the LSP method the capability requires)
- A `supports(client): boolean` predicate
- A `handler?: (params) => void` for server-to-client direction (mostly
  n/a for us; we don't subscribe to these methods)

Why this matters:
- **`/lens-health` (lens_diagnostics) currently reports operation support
  as a flat dump** — a user can't tell whether "rename is unsupported"
  is a server gap or a handler gap. A per-capability class with
  `supported` + `enabled` would expose the distinction.
- **Cross-cutting support for new capabilities** (e.g. LSP 3.18's
  `textDocument/inlineCompletion` or `textDocument/inlayHint`) currently
  needs a hand-edit to `LSPOperationSupport`, the index, the runner, the
  dispatch conversion. A registry would centralise it.
- **The `getCapabilitySnapshots()` row layout** is currently
  { server × root × operation_support }; a per-capability registry would
  naturally drive `docs/lsp-capability-matrix.md` (the existing one uses
  `characterize-lsp.mjs` + `server-capabilities.mjs` to capture a flat
  table — confirmed in `AGENTS.md` under "LSP capability inventory").

**Blast radius:** high. This is a refactor, not a take. The current
shape is fine for the immediate needs; a follow-up could replace
`LSPOperationSupport` with `Map<CapabilityId, { supported: boolean;
version: number }>` and have each capability self-register. Not a
single-PR change.

### Why we should not over-generalise

The `LSPOperationSupport` booleans are *simple* and *correct* for the
queries the dispatch / tools make. Per-instance capability handlers are
only worth it if we need to subscribe to **server-to-client** capability
methods (`textDocument/publishDiagnostics` already does), which we
mostly don't.

---

## 4. `workspace/didChangeWatchedFiles` debounce + queue (MEDIUM-HIGH value, low risk)

### Neovim's model

`runtime/lua/vim/lsp/_watchfiles.lua` (lines ~30-90):

```lua
-- @type table client id -> registration id -> cancel function
local cancels = vim.defaulttable()
local queuetimeoutms = 100
-- @type table client id -> libuv timer which will send queued changes at its timeout
local queue_timers = {}
-- @type table client id -> set of queued changes to send in a single LSP notification
local change_queues = {}
-- @type table client id -> URI -> last type of change processed
-- Used to prune consecutive events of the same type for the same file
local change_cache = vim.defaulttable()
```

Behaviour:
1. A file change is queued (per-client, per-URI).
2. A 100 ms libuv timer is set (per-client); if it fires, all queued
   changes are sent in a **single** `workspace/didChangeWatchedFiles`
   notification.
3. The `change_cache` (URI → last type) prunes *consecutive events of
   the same type* for the same file (e.g. three `Changed` in a row
   collapse to one).

### pi-lens today

`handleNotifyOpen` in `clients/lsp/client.ts` (line ~1019) sends
`workspace/didChangeWatchedFiles` on every `didOpen` *when `silent` is
false*:

```ts
// Async existence probe (was a synchronous existsSync on the document-open
// path — a stat that blocks the loop during first-read/warm). The notify
// type is unchanged: 2 (Changed) when the file exists on disk, else 1
// (Created). access() rejects when absent.
let fileExists = true;
try {
    await access(filePath);
} catch {
    fileExists = false;
}
await safeSendNotification(
    state.connection,
    "workspace/didChangeWatchedFiles",
    { changes: [{ uri, type: fileExists ? 2 : 1 }] },
);
```

This fires **once per touch** (the touch is debounced by
`TOUCH_DEBOUNCE_MS=1500` in `clients/lsp/index.ts:66`, but a flurry of
edits in different files in the same turn still produces N notifications
per server). The TypeScript LSP, in particular, re-runs project-wide
analysis on every `didChangeWatchedFiles`, so the flurry becomes a real
hot path.

### Take

Add a per-client debounced queue for `workspace/didChangeWatchedFiles`:
keyed by `(serverId, root)`, debounce window 100 ms (neovim's
`queuetimeoutms`), collapse consecutive same-type events per URI.

Implementation sketch in `clients/lsp/client.ts` or a new
`clients/lsp/watch-queue.ts`:

```ts
class WatchChangeQueue {
    private pending = new Map<string, { uri: string; type: 1 | 2 | 3 }>();
    // last type per URI — collapse consecutive same-type events
    private lastType = new Map<string, 1 | 2 | 3>();
    private timer: NodeJS.Timeout | undefined;
    private readonly DEBOUNCE_MS = 100;
    constructor(
        private readonly send: (changes: { uri: string; type: 1|2|3 }[]) => Promise<void>,
    ) {}
    enqueue(uri: string, type: 1 | 2 | 3) {
        if (this.lastType.get(uri) === type) return;  // collapse
        this.pending.set(uri, { uri, type });
        this.lastType.set(uri, type);
        if (!this.timer) {
            this.timer = setTimeout(() => this.flush(), this.DEBOUNCE_MS);
            this.timer.unref?.();
        }
    }
    async flush() {
        const changes = [...this.pending.values()];
        this.pending.clear();
        this.timer = undefined;
        if (changes.length) await this.send(changes);
    }
}
```

Plumb the queue into `setupIncomingHandlers` and replace the immediate
`safeSendNotification` in `handleNotifyOpen` with `queue.enqueue(...)`.

**Blast radius:** low. The notification is debounced and collapsed; the
worst case is a 100 ms delay before the server learns of a file change,
which is invisible compared to the 5–30 s analysis re-runs the server
actually does.

**Tests to add:**
- 3 rapid `enqueue` calls for `(uriA, Changed)`, `(uriA, Changed)`,
  `(uriB, Created)` collapse to a single notification with
  `[{uriA,Changed}, {uriB,Created}]`.
- A `flush()` fires after 100 ms with `t.timer` set, even if no further
  enqueues land.

### Why this is higher-value than it looks

The TypeScript LSP's `didChangeWatchedFiles` handler kicks off a
project-wide `tsserver` re-analysis. A `tool_result` turn with 5 file
edits in the same project currently triggers 5 separate re-analyses;
after this change, it's 1.

---

## 5. Per-buffer position-encoding registration (HIGH value when it bites)

### Neovim's model

The `positionEncoding` capability was added in LSP 3.17:
`InitializeResult.capabilities.positionEncodings` is a list of encodings
the server supports (e.g. `["utf-16"]`). The client must use one of
these. Neovim tracks the negotiated encoding per client
(`client.offset_encoding`) and translates positions on the way out.

`runtime/lua/vim/lsp/util.lua` has helpers like
`vim.lsp.util.character_offset(buf, line, col, encoding)` that convert
client-side (UTF-8) coordinates to the encoding the server wants.

### pi-lens today

We do **not** read or honour `positionEncodings`. Every
`textDocument/*Position` request sends raw `{line, character}` indices
in whatever encoding the editor gave us (UTF-16, the historical default
— the LSP spec used to mandate UTF-16 before 3.17).

This is fine for the **vast majority** of servers (which default to
UTF-16 and don't override the client's UTF-16) but breaks for:
- `rust-analyzer` ≥ 2024 — defaults to UTF-8, asks clients to send
  UTF-8 offsets.
- `gopls` (recent builds) — supports both, may pick UTF-8.
- `jdtls` — always UTF-16 (safe).
- `typescript-language-server` — UTF-16 (safe).

### Take

Negotiate `positionEncoding` per client at `initialize` and translate
positions on the way out:

```ts
// in clients/lsp/client.ts — sketch
state.positionEncoding = (initResult as {
    capabilities?: { positionEncodings?: string[] };
}).capabilities?.positionEncodings?.[0] ?? 'utf-16';

// in navRequest, when building params:
const position = state.positionEncoding === 'utf-8'
    ? utf16ToUtf8(buf, line, character)
    : { line, character };
```

UTF-16 ↔ UTF-8 conversion in Node is straightforward via
`Buffer.from(line, 'utf8').toString('utf16le')` length differences — the
difference is the number of UTF-16 code units for the same grapheme.

**Blast radius:** medium. Touches `navRequest`, every `notify.open`
`Position` arg, the 12 navigation methods, and the
`textDocument/didChange` content (which is text, not positions, so
unaffected). Add a test fixture that hands rust-analyzer a `Café`-
containing file and asserts the position is converted.

### Why this is "high value when it bites" not "must-do now"

The breakage is silent — the LSP returns the wrong location, the user
gets a wrong goto-def. We don't have user reports yet, so this is
preventative. Rank it above the per-capability registry but below the
`didChangeWatchedFiles` debounce.

---

## 6. Soft cancel with `$/cancelRequest` (MEDIUM value)

### Neovim's model

Each in-flight request has an associated cancel function. On interrupt
(Esc, switching buffers), the cancel function is invoked:
- For server notifications, it sets a flag and the response is dropped
  when it lands.
- For server requests the client is processing, it sends
  `$/cancelRequest` with the request id, per the LSP 3.16 spec.

`runtime/lua/vim/lsp/rpc.lua` exposes the cancel mechanism on the
channel; `runtime/lua/vim/lsp/buf.lua` wires it into the request
helpers.

### pi-lens today

The only cancellation path is the **ambient abort signal** —
`safeSpawnAsync` defaults to `setAmbientAbortSignal`, which is published
by `tool_result` / `agent_end` / `turn_end` lifecycle handlers (per
`AGENTS.md`: "the lifecycle handlers publish pi's `ctx.signal` at entry
and clear it in `finally`, so an Esc/interrupt kills in-flight
linter/format/type-check children"). This kills the **process** but
doesn't send `$/cancelRequest` to the LSP.

Result: a user presses Esc during a slow `definition` lookup, the
ambient signal kills the LSP process, the **next** request starts a cold
LSP. The smarter approach: send `$/cancelRequest`, the server aborts
the in-flight computation, the client is still warm.

### Take

Capture the `AbortSignal` at request send time, wire it through
`navRequest` and `safeSendRequest`, and on abort send
`$/cancelRequest` instead of (or in addition to) killing the process.

```ts
async function safeSendRequestCancellable<T>(
    conn: MessageConnection,
    method: string,
    params: unknown,
    options: { signal?: AbortSignal },
): Promise<T | undefined> {
    let cancel: (() => void) | undefined;
    try {
        return await new Promise<T>((resolve, reject) => {
            const connToken = conn.sendRequest(method, params);
            // rpc spec: cancel via the same channel
            cancel = () => {
                try { conn.sendNotification('$/cancelRequest', { id: connToken }); } catch {}
            };
            options.signal?.addEventListener('abort', cancel, { once: true });
            connToken.then(resolve, reject);
        });
    } finally {
        options.signal?.removeEventListener('abort', cancel!);
    }
}
```

vscode-jsonrpc exposes the request id via the promise it returns
(an extended `CancellablePromise`). The pattern is well-trodden.

**Blast radius:** low. Only the navigation path; we don't currently
cancel in-flight requests from the dispatch hot path (we just wait out
the timeout). Adds a `AbortSignal` parameter to `navRequest` and the
service-level wrappers.

**Note on ambient signal:** keep the existing `setAmbientAbortSignal`
for the *process* kill (so a hung server dies), but layer the
`$/cancelRequest` on top for the *request* cancel. The two are
complementary, not exclusive.

---

## 7. Already-aligned: config merge for `root_markers` (NO-OP)

### Neovim's model

> `root_markers = { { '.emmyrc.json', '.luarc.json' }, '.git' }` — Nested
> lists indicate equal priority, see `vim.lsp.Config`.
> — `runtime/doc/lsp.txt` (QUICKSTART)

The semantics: a *list* of markers is **OR** (any one matches → this is
a root); a *nested list* is **priority** (try the first sub-list; if
none of its markers match, try the next sub-list; if none match, fall
back to `FileDirRoot`).

### pi-lens today

`PriorityRoot` in `clients/lsp/server.ts:393`:
```ts
export function PriorityRoot(
    markerGroups: string[][],  // <-- already nested!
    excludePatterns?: string[],
    stopDir?: string,
): RootFunction {
    const resolvers = markerGroups.map((markers) =>
        NearestRoot(markers, excludePatterns, stopDir),
    );
    return async (file: string) => {
        for (const resolve of resolvers) {
            const root = await resolve(file);
            if (root) return root;
        }
        return undefined;
    };
}
```

This is **exactly** the neovim semantic. We use it in 20+ server
definitions (e.g. `RubyServer: RootWithFallback(PriorityRoot([['Gemfile',
'.ruby-version'], ['.git']]))`).

### Take

**No take** — the model is already correct. Add a doc comment cross-
referencing `runtime/doc/lsp.txt`'s `root_markers` semantic so a future
server-author sees the equivalent.

---

## 8. Already-aligned: `apply_text_edits` overlap check (NO-OP)

### Neovim's model

`vim.lsp.util.apply_text_edits` sorts edits in reverse order, then
applies them; if any two edits overlap after sorting, it raises an
error.

### pi-lens today

`applyTextEditsToString` in `clients/lsp/edits.ts:81-141` does **the
same thing**: sort by `(line, char)` reverse, then walk, then throw
`"overlapping LSP edits: <range> conflicts with <range>"` on overlap.
The `mergeWorkspaceTextEditsByPriority` sibling (line 209) does
cross-server overlap-drop *before* the apply, which neovim doesn't
do (neovim just lets one server win per-URI).

### Take

**No take** — our version is stricter. Just keep the test coverage
green.

---

## 9. Already-aligned: `serverEditsAllowed` counter (NO-OP)

### Neovim's model

Neovim's `workspace/applyEdit` handler is invoked when the **server**
asks the **client** to apply a workspace edit. The handler is wired up
unconditionally — there's no gate for "only during an in-flight
executeCommand".

### pi-lens today

`state.serverEditsAllowed` (a `number` counter) in
`clients/lsp/client.ts:432` gates the `workspace/applyEdit` handler:

```ts
state.connection.onRequest(
    "workspace/applyEdit",
    async (params) => {
        if (state.serverEditsAllowed <= 0 || !params?.edit) {
            return { applied: false, failureReason: "edit not solicited" };
        }
        // … apply
    },
);
```

The counter is bumped around the `executeCommand` call:

```ts
state.serverEditsAllowed += 1;
try {
    const result = await safeSendRequest(/* … */);
} finally {
    state.serverEditsAllowed -= 1;
}
```

This is **stricter** than neovim — a server cannot push edits to disk
on its own initiative, only as a direct effect of an
opted-in `executeCommand`. Neovim doesn't gate, so a server can pop a
dialog or `qflist` set; pi-lens can't because the extension has no UI
of its own.

### Take

**No take** — we already do this. Cite in the changelog when shipping
related work as a precedent.

---

## Cross-cutting: what's NOT worth borrowing

A few patterns from the neovim docs we explicitly should **not** copy,
because the runtime model is fundamentally different:

1. **Buffer-centric lifecycle (`LspAttach` / `LspDetach` autocmds).**
   pi-lens has no buffers — files are stat-touched and torn down per
   tool call. Our `LSPService` *is* the per-file/per-server lifecycle;
   we don't need the buffer indirection.

2. **`omnifunc`, `tagfunc`, `formatexpr` integration.** All three are
   editor-specific; pi-lens dispatches to MCP tools and pi tools. The
   `format` integration point is our `clients/formatters.ts` runner
   (different mechanism, same end result).

3. **`vim.lsp.inlay_hint` / `completion` / `inline_completion` /
   `document_color`.** These are interactive UI surfaces pi-lens has no
   equivalent of. The `lens_module_report` `api`/`internal` split +
   signature extraction is our closest analog to "show the user
   something visual about their code".

4. **`buf_request_all` (request-to-all-clients).** The closest analog in
   pi-lens is `getClientsForFile` (in `clients/lsp/index.ts:541`) and
   the `with-auxiliary` clientScope, which both spawn-or-attach **all**
   matching servers for a file. The semantic is "all clients in this
   workspace root", not "all clients in this buffer" — but the code
   shape is the same.

---

## Suggested follow-up plan

Three small additive PRs, ordered by independence:

### PR-A: `$/cancelRequest` soft-cancel on Esc
- Add `AbortSignal` parameter to `navRequest` + 12 callers.
- Add `safeSendRequestCancellable` that sends `$/cancelRequest` on
  abort.
- Wire the `ctx.signal` from the pi tool handlers through to the
  service (`LSPService.definition(filePath, line, character, { signal })`).
- **Tests:** unit-test the request-cancel logic with a stub
  `MessageConnection`; integration-test the `pilens_lsp_navigation`
  MCP tool under an aborted `ctx.signal`.
- **Net LOC:** ~80, with tests ~200.

### PR-B: `workspace/didChangeWatchedFiles` debounce + queue
- Add `WatchChangeQueue` per-client.
- Replace the immediate send in `handleNotifyOpen` with `queue.enqueue`.
- **Tests:** unit-test the collapse (3 same-type enqueues → 1 notify)
  and the debounce window (notify fires at 100 ms with batched
  changes).
- **Net LOC:** ~50, with tests ~150.

### PR-C: `positionEncoding` negotiation
- Capture `positionEncodings[0]` at initialize; default to `utf-16`.
- Add a `utf8ToUtf16(line, col, text)` / `utf16ToUtf8(...)` helper.
- Apply in `navRequest` and the `textDocument/*Position` builders.
- **Tests:** unit-test the conversion (a string with `Café` and a
  `位置` cluster); integration-test the rust-analyzer `/` test fixture
  with a `Café`-containing identifier.
- **Net LOC:** ~120, with tests ~250.

A fourth, larger follow-up (per-capability registry, #3 above) is
worth its own design doc, not a follow-up PR.

---

## Assertions checklist against the current codebase

Every claim in this doc was asserted against the live pi-lens source
under `C:\Users\R3LiC\Desktop\pi-lens` (v3.8.53 per `AGENTS.md`). The
specific file/line locations cited above:

| # | Claim | Verified at |
|---|---|---|
| 1a | Diagnostics-wait staleness uses `diagnosticDocVersions` vs `documentVersions` | `clients/lsp/client.ts:903-907` (`isVersionStale` body) |
| 1b | Navigation path does **not** check staleness | `clients/lsp/client.ts:1130-1150` (`navRequest` body — no version check; just `withTimeout` + `safeSendRequest` + the timeout-returns-undefined path) |
| 1c | `NAV_REQUEST_TIMEOUT_MS=10_000` default | `clients/lsp/client.ts:316` |
| 1d | 12 nav methods that call `navRequest` (13 `navRequest<` references incl. the function declaration at line 1130) | `grep -nE "navRequest<" clients/lsp/client.ts` → 13 hits (1 declaration + 12 call sites at lines 1549, 1562, 1575, 1588, 1601, 1609, 1621, 1670, 1683, 1711, 1724, 1735, 1744) |
| 2a | `setupIncomingHandlers` registers 7 server-to-client handlers | `clients/lsp/client.ts:663, 715, 718, 745, 761, 781, 784` (7 handler registrations: 1 onNotification + 6 onRequest) |
| 2b | No per-request override mechanism exists for any handler | Same — only one `setMaxListeners(50)` (line 1320) and one emitter; the registration is per-client, not per-request |
| 3a | `LSPOperationSupport` is a flat booleans map | `clients/lsp/client.ts:344-358` (interface definition) |
| 3b | `applyDynamicCapabilities` mutates a single shared map | `clients/lsp/client.ts:614-642` |
| 3c | `getCapabilitySnapshots()` returns a flat tuple per client | `clients/lsp/index.ts:1563-1620` (snapshot builder) |
| 4a | `handleNotifyOpen` sends `workspace/didChangeWatchedFiles` per touch when `silent=false` | `clients/lsp/client.ts:1019-1038` |
| 4b | `silent` defaults to `false` on the touch path | `clients/lsp/index.ts:969-980` (`silent: options.silent ?? false`) |
| 4c | Touch debounce is 1500 ms (`TOUCH_DEBOUNCE_MS`) | `clients/lsp/index.ts:66-69` |
| 4d | Per-file `recentTouches` is keyed by `normalizeMapKey(filePath):clientScope` | `clients/lsp/index.ts:325, 396-414` |
| 5a | No `positionEncodings` handling | `grep -rnE "positionEncod|offset_encod" clients/lsp/*.ts` → 0 matches |
| 5b | Every `textDocument/*Position` request sends raw `{line, character}` | All 12 nav methods (lines 1549-1744) build `position: { line, character }` directly |
| 6a | No `$/cancelRequest` send anywhere | `grep -rnE "\\\$/cancelRequest" clients/lsp/*.ts` → 0 matches |
| 6b | Ambient signal kills the process, not the request | `clients/safe-spawn.ts` (per `AGENTS.md` "Async-spawn migration" section) |
| 7a | `PriorityRoot(markerGroups, ...)` takes a nested array | `clients/lsp/server.ts:579-605` (function definition) |
| 7b | 9 server definitions use `PriorityRoot` / `WorkspacePriorityRoot` (plus 2 wrap with `RootWithFallback`) | `grep -nE "(Workspace)?PriorityRoot\(" clients/lsp/server.ts` → 7 bare `PriorityRoot(...)` calls (lines 1385, 1437, 1762, 1790, 1837, 1858, 1992) + 2 `WorkspacePriorityRoot(...)` (lines 1289, 1813) = 9 server definitions; several more wrap with `RootWithFallback` (e.g. `RubyServer`, `PhpServer`, `GoServer`) |
| 8a | `applyTextEditsToString` does sorted reverse + overlap throw | `clients/lsp/edits.ts:97-145` (function body) |
| 8b | The overlap error message format is `overlapping LSP edits: ${range} conflicts with ${range}` | `clients/lsp/edits.ts:113` |
| 8c | `mergeWorkspaceTextEditsByPriority` does cross-server overlap drop | `clients/lsp/edits.ts:208-265` (function body) |
| 9a | `serverEditsAllowed` counter gates the `workspace/applyEdit` handler | `clients/lsp/client.ts:429 (state), 757-770 (handler comment + gate), 1523+1532 (executeCommand bump)` |
| 9b | Counter is bumped **only** around `executeCommand` | `clients/lsp/client.ts:1523, 1532` (the only two mutation sites) |

All assertions are made against the current `.ts` source; the
`AGENTS.md` load-bearing claim that "pi-lens's lifecycle hooks
(`session_start`, `tool_call`, `tool_result`, `context`, `turn_end`,
`agent_end`) run on the same event loop as pi's TUI" is the model
assumption underlying sections 4 and 6 (anything that blocks the loop
for > ~50 ms is a smell).

No tests were broken or run during this audit — this is a doc-only
study. The three suggested PRs each include their own test plan.

---

# Architectural refactor opportunities (bigger lifts)

The "take" sections above are all additive, single-PR changes. This
section is the bigger picture: structural refactors that, if done, would
change the shape of `clients/lsp/*` materially. Each is asserted
against the current code with line numbers, sized honestly, and tagged
with the *forcing function* that would justify the work (a real
regression, a real perf ceiling, a real correctness gap). Don't do any
of these speculatively — pick one when a concrete need lands.

## R1 — Split `clients/lsp/client.ts` (1929 LOC) into per-concern files

### The current state

`clients/lsp/client.ts` is **1929 lines** and is the single biggest
file in `clients/lsp/`. Verified at `wc -l clients/lsp/client.ts` →
1929. It contains:

| Section | Approx. lines | Concern |
|---|---|---|
| Types & interfaces (`LSPDiagnostic`, `LSPClientInfo`, `LSPClientState`) | 1–450 | Pure data shape; no Node APIs |
| `LSPClientState` mutable runtime state (26 fields) | 357–450 | In-memory cache layer |
| `killProcessTree` (Windows / Unix SIGTERM escalation) | 451–540 | Platform-specific process kill |
| `stripDiagnosticNoiseLines` / `mergeDiagnosticLists` / `getMergedDiagnosticsForPath` / `clearDiagnosticsForPath` | 542–610 | Diagnostic data transforms |
| `applyDynamicCapabilities` / `DYNAMIC_OPERATION_METHOD_MAP` | 614–642 | Capability tracking |
| `setupIncomingHandlers` (7 server-to-client handlers) | 656–790 | Incoming RPC plumbing |
| `setupConnectionLifecycle` (error/close/exit) | 793–830 | Connection lifecycle |
| `clientRequestPullDiagnostics` / `PullDiagnosticsOutcome` (the #240 fix) | 838–895 | Pull-mode diagnostics |
| `clientWaitForDiagnostics` (the "affirmative clean" wait) | 897–975 | Push/pull merge wait |
| `handleNotifyOpen` / `handleNotifyChange` (the touch path) | 980–1100 | Document sync |
| `clientShutdown` | 1100–1130 | Shutdown |
| `navRequest` (12-method nav dispatch helper) | 1130–1150 | Navigation RPC |
| `resolveCodeActionBestEffort` | 1152–1170 | Code-action resolve |
| `createLSPClient` (initialize handshake + state factory) | 1170–1430 | Factory + initialize |
| `LSPClientInfo` method implementations (12 nav + executeCommand + executeCommand gating + capability getters) | 1430–1850 | Public client API |
| `detectWorkspaceDiagnosticsSupport` / `detectExecuteCommands` / `detectOperationSupport` | 1850–1930 | Capability extraction at init |

The file mixes:
- **Pure data shape** (types)
- **Process control** (kill, shutdown)
- **JSON-RPC plumbing** (handlers, lifecycle)
- **Domain logic** (diagnostic merge, version coherence, capability map)
- **Public surface** (`LSPClientInfo` methods)

That's five concerns in one file. The single biggest contributor to its
size is the **12 navigation methods** at lines 1430–1750 — every one
follows the exact same shape:

```ts
async definition(filePath, line, character) {
    const result = await navRequest<LSPLocation | LSPLocation[]>(
        state, "textDocument/definition", {
            textDocument: { uri: pathToFileURL(filePath).href },
            position: { line, character },
        });
    if (!result) return [];
    return Array.isArray(result) ? result : [result];
}
```

Verified at `clients/lsp/client.ts:1549, 1562, 1575, 1588, 1601, 1609,
1621, 1670, 1683, 1711, 1724, 1735, 1744` — 12 call sites with
near-identical structure. A `defineNavMethod(method, responseShape)`
helper would shrink this by 60–70%.

### What a split could look like

1. `clients/lsp/client-types.ts` (~250 LOC) — all `LSP*` interfaces.
2. `clients/lsp/client-state.ts` (~100 LOC) — `LSPClientState`,
   `isClientAlive`, the field map.
3. `clients/lsp/client-process.ts` (~100 LOC) — `killProcessTree` (and
   the platform-specific bits).
4. `clients/lsp/client-diagnostics.ts` (~200 LOC) — pull/push merge,
   `clientWaitForDiagnostics`, `PullDiagnosticsOutcome`, the #240
   affirmative-clean logic.
5. `clients/lsp/client-handlers.ts` (~150 LOC) — `setupIncomingHandlers`,
   `setupConnectionLifecycle`, the capability map.
6. `clients/lsp/client-document.ts` (~150 LOC) — `handleNotifyOpen`,
   `handleNotifyChange`, document-version bookkeeping.
7. `clients/lsp/client-nav.ts` (~200 LOC) — `navRequest` + the 12
   navigation methods, table-driven.
8. `clients/lsp/client.ts` (~150 LOC) — `createLSPClient` factory only.

The 12 nav methods compress to a table:

```ts
// sketch — clients/lsp/client-nav.ts
const NAV_METHODS = [
    { name: "definition",       method: "textDocument/definition",       many: true  },
    { name: "typeDefinition",   method: "textDocument/typeDefinition",   many: true  },
    { name: "declaration",      method: "textDocument/declaration",      many: true  },
    { name: "references",       method: "textDocument/references",       many: true  },
    { name: "hover",            method: "textDocument/hover",            many: false },
    { name: "signatureHelp",    method: "textDocument/signatureHelp",    many: false },
    { name: "documentSymbol",   method: "textDocument/documentSymbol",   many: true  },
    { name: "implementation",   method: "textDocument/implementation",   many: true  },
] as const;
```

…each entry becomes a 4-line method via a shared builder. The
remaining nav methods (`codeAction`, `rename`, `willRenameFiles`,
`prepareCallHierarchy`, `incomingCalls`, `outgoingCalls`) have
non-uniform shapes — keep them hand-written or carve out a
`client-callhierarchy.ts` for the call-hierarchy pair.

### Why it matters

- **Discoverability** — the file is the single biggest reason new
  contributors hesitate to touch LSP code. AGENTS.md's note "use
  `goToDefinition` / `findReferences` before grepping" is a workaround
  for not knowing where the navigation code lives.
- **Testability** — `client-diagnostics.ts` (the #240 wait logic) has
  no unit tests today. The reason is "it's tangled with the rest of
  client.ts". A split makes it directly testable.
- **PR-C compatibility** — adding `positionEncoding` translation
  (section 5 above) would touch 12 nav methods. Doing it in a split
  `client-nav.ts` is a 12-line change in one table; today it's a
  12-method hand-edit.

### Forcing function

The #240 fix (pull-mode `found|clean|unavailable`) added ~50 lines to
`client.ts` in a single commit. The next equivalent fix (which will
happen — see #242 in `AGENTS.md`, the per-server deadlines were the
last one) will add another 50+ lines. Two more such fixes and the
file hits 2100 LOC, which is past the practical "this is one thing"
boundary.

**Estimated size:** ~600 LOC of mechanical move + ~150 LOC of
table-driven nav-method builder. **Risk:** low (pure refactor, no
behaviour change), but **blast radius:** every test in
`tests/clients/lsp/` will need its imports updated.

**Do not** do this speculatively. Do it as a co-commit with the next
diagnostics-wait or navigation-staleness fix, so the import-update
work is amortised.

---

## R2 — Split `clients/lsp/server.ts` (2243 LOC) into registry + launch helpers

### The current state

`server.ts` is the **largest** LSP file at 2243 lines. Verified at
`wc -l clients/lsp/server.ts` → 2243. It contains:

| Section | Approx. lines | Concern |
|---|---|---|
| Types (`LSPServerInfo`, `RootFunction`, `LSPSpawnOptions`) | 25–80 | Type shape |
| `resolveAndLaunch` (the unified binary-resolution pipeline, ~180 LOC) | 80–360 | Binary resolution + install |
| `IgnoreHomeRoot` / `PriorityRoot` / `RootWithFallback` / `WorkspacePriorityRoot` / `NearestRoot` / `FileDirRoot` / `DenoExcludeRoot` | 380–650 | Root detection |
| `nodeBinCandidates` / `rubyBinCandidates` / `goBinCandidates` / `cargoBinCandidates` / `dotnetToolCandidates` / `binExeVariants` | 540–600 | Per-platform binary-path discovery |
| `tryGoInstallGopls` / `tryDotnetToolInstall` / `tryGemInstall` / `findTsserverPath` / `detectPythonVenv` | 750–950 | Per-server install helpers |
| `createInteractiveServer` (the "simple" server-spec factory) | 535–590 | Server factory helper |
| 38 `LSPServerInfo` exports (`TypeScriptServer`, `DenoServer`, `PythonServer`, `PythonJediServer`, `GoServer`, `RustServer`, `RubyServer`, `RubySolargraphServer`, `PHPServer`, `CSharpServer`, `OmniSharpServer`, `FSharpServer`, `JavaServer`, `KotlinServer`, `SwiftServer`, `DartServer`, `LuaServer`, `CppServer`, `ZigServer`, `HaskellServer`, `ElixirServer`, `GleamServer`, `OCamlServer`, `ClojureServer`, `TerraformServer`, `NixServer`, `BashServer`, `DockerServer`, `YamlServer`, `JsonServer`, `HtmlServer`, `TomlServer`, `PrismaServer`, `VueServer`, `SvelteServer`, `CssServer`, `OpengrepServer`, `AstGrepServer`) | 950–2200 | Built-in server definitions |

The `LSPServerInfo` interface itself has **10 top-level data
fields** plus the `spawn` function signature
(`sed -n '34,80p' clients/lsp/server.ts | grep -cE "^\s*\w+\??:"`
→ 15, of which 5 are inside the spawn signature and 10 are the
interface fields: `id`, `name`, `extensions`, `root`, `role`,
`availabilityKey`, `initializeTimeoutMs`, `clientWaitTimeoutMs`,
`autoPropagateDiagnostics`, `spawn`, `autoInstall`); the 35 line
count from `grep -cE "^\s*(\w+\??:|/\*\*|\*)"` over the interface
body includes TS doc-comment lines that count as "field-like"
entries when including the comment block. The 38 server exports
(`grep -cE "^export const \w+Server" clients/lsp/server.ts` → 38,
`LSP_SERVERS` list declared at line 2180) collectively repeat most
of the same `root` + `extensions` boilerplate.

### The pattern that's grown

`createInteractiveServer` (line 539) is a thin wrapper for the 60% of
servers that just need `cmd + args + root + initialization`:

```ts
export const JavaServer = createInteractiveServer({
    id: "java",
    name: "JDT Language Server",
    extensions: KIND_EXTENSIONS["java"],
    root: RootWithFallback(createRootDetector([...])),
    language: "java",
    command: () => process.env.JDTLS_PATH || "jdtls",
    args: (root) => createLombokJdtlsArgs(root),
});
```

vs. the heavier "managed install" pattern (e.g. `PythonServer`):

```ts
export const PythonServer: LSPServerInfo = {
    id: "python", name: "Pyright Language Server", extensions: ...,
    root: RootWithFallback(createRootDetector([...])),
    async spawn(root, options) { /* 50 LOC of env/candidate/install logic */ },
};
```

…and the heavier still "Ruby tri-fallback" pattern (`RubyServer`):

```ts
async spawn(root, options) {
    const rubylsp = await resolveAndLaunch({ candidates: [...], ... });
    if (rubylsp) return rubylsp;
    const solargraph = await resolveAndLaunch({ candidates: [...], ... });
    if (solargraph) return solargraph;
    return resolveAndLaunch({ candidates: [...], ... });
}
```

Three spawn shapes — *interactive* (env / SDK, no install), *managed*
(install via registry, retry), *fallback chain* (try one, try another,
try a third). Each server's `spawn` is a custom 30–60 LOC function.

### What a split could look like

1. `clients/lsp/server.ts` (~400 LOC) — types only:
   `LSPServerInfo`, `RootFunction`, `LSPSpawnOptions`, the registry
   list `LSP_SERVERS`, the `InteractiveServerSpec` interface, the
   `LSP_SERVERS` array. No spawn implementations.
2. `clients/lsp/server-roots.ts` (~300 LOC) — all `Root*` helpers
   (`NearestRoot`, `PriorityRoot`, `FileDirRoot`, `DenoExcludeRoot`,
   `RustWorkspaceRoot`, `TypeScriptRoot`, `IgnoreHomeRoot`).
3. `clients/lsp/server-paths.ts` (~150 LOC) — per-platform binary
   discovery (`nodeBinCandidates`, `rubyBinCandidates`,
   `goBinCandidates`, `cargoBinCandidates`, `dotnetToolCandidates`,
   `binExeVariants`, `findTsserverPath`).
4. `clients/lsp/server-install.ts` (~200 LOC) — `tryGoInstallGopls`,
   `tryDotnetToolInstall`, `tryGemInstall`, `detectPythonVenv`.
5. `clients/lsp/server-resolve.ts` (~300 LOC) — `resolveAndLaunch`,
   `directLspCommandUnavailable` cache, the
   `isDirectLspCommandTemporarilyUnavailable` export, the
   `DIRECT_LSP_NEGATIVE_TTL_MS` constant.
6. `clients/lsp/server-spawn.ts` (~150 LOC) — `createInteractiveServer`
   + a new `createManagedServer` factory that replaces the 50-LOC
   `PythonServer.spawn` boilerplate.
7. `clients/lsp/server-defs/` (one file per language family):
   - `typescript.ts`, `python.ts`, `ruby.ts`, `rust.ts`, `go.ts`,
     `csharp.ts`, `java.ts`, `kotlin.ts`, `web.ts`, `shell.ts`,
     `nix-tools.ts` (terraform/nix/gleam/clojure/ocaml/elixir)
   - ~10 files, each ~80–150 LOC.
8. `clients/lsp/server.ts` becomes a re-export shim that wires the
   `LSP_SERVERS` list from `server-defs/*`. Existing import sites
   (`import { LSP_SERVERS } from "./server.js"`) keep working.

### Why it matters

- **`resolveAndLaunch` is the most-edited function in the LSP code**
  per the AGENTS.md changelog (force-reinstall retry, goBinCandidates
  discovery, dotnet tool install, ruby tri-fallback all touched it
  within 2024–2026). It's at line 80 of a 2243-LOC file, which means
  every change to it requires scrolling past ~20 server definitions.
- **The spawn-shape taxonomy is unstated** — `createInteractiveServer`
  is one pattern, but the other two (managed + fallback chain) are
  inlined in every `LSPServerInfo.spawn`. Codifying them as
  `createManagedServer` and `createFallbackChain` would shrink each
  server definition by 30–40 LOC and make the install policy
  auditable at a glance.
- **The `LSP_SERVERS` array currently lists 20+ servers in one
  block.** Verified at `grep -nE "^export const \w+Server" clients/lsp/server.ts` → 30+ matches. Adding a new server requires scrolling
  to the right place in 2243 LOC. With per-family files, adding
  `server-defs/swift.ts` is a single file.

### Forcing function

The current install pipeline has 3 candidates × 5 strategies (npm,
pip, gem, go install, dotnet tool, github release, archive, maven)
plus 2 install paths (managed + runtime). When #241 (per-server
archive strategy) lands for jdtls / kotlin-language-server / clangd
/ lua-language-server / elixir-ls, the `resolveAndLaunch` function
will gain 100+ LOC. That's the moment to extract it.

**Estimated size:** ~500 LOC of mechanical move + ~200 LOC of
factory extraction (`createManagedServer`, `createFallbackChain`).
**Risk:** medium — every server definition moves between files; the
test surface (`tests/clients/lsp/server-*.test.ts`) re-imports each
individually. **Blast radius:** high, so gate behind a forcing
function.

---

## R3 — Make `LSPService` (2046 LOC) composable from smaller services

### The current state

`clients/lsp/index.ts` is **2046 LOC** (verified at
`wc -l clients/lsp/index.ts` → 2046) and exports a single
`LSPService` class that owns **all** of:

- **Process registry** (`state.clients`, `state.broken`,
  `state.inFlight`, `state.clientSpawnedAt`) — line 286–291
- **Cooldown / circuit breaker** (`failureCounts`,
  `permanentlyBroken`, `brokenUntil` map) — line 294–301
- **Diagnostic cache** (`lastKnownDiagnostics`,
  `lastKnownContentHash`, `lastDiagnosticsHealth`,
  `recentTouches`) — line 304–325
- **Touch orchestration** (`touchFile`, `getDiagnostics`) —
  line 906–1100, 1171–1390
- **Navigation proxies** (16 methods: `definition`,
  `typeDefinition`, `declaration`, `references`, `hover`,
  `signatureHelp`, `documentSymbol`, `workspaceSymbol`,
  `codeAction`, `rename`, `willRenameFiles`, `didRenameFiles`,
  `implementation`, `prepareCallHierarchy`, `incomingCalls`,
  `outgoingCalls`) — line 1385–1820
- **Capability snapshots** (`getCapabilitySnapshots`,
  `getOperationSupport`, `getAdvertisedCommands`) — line 1563–1620
- **Workspace diagnostics** (`collectWorkspaceDiagnosticFiles` +
  the `lens_diagnostics` delta/full walk) — line 238–330

The class is a *god object*: 2046 LOC, ~25 private fields, 16 public
async methods, plus the `getDiagnostics` method (lines 1171–1390)
which alone is **220 LOC** and contains the per-server
`waitForDiagnostics` + `raceToCompletion` + cross-server
deduplication + last-known-cache + `expectSemanticSecondPush`
re-wait logic all inline.

### The shape that would be healthier

`LSPService` should be a thin facade that composes four
**single-responsibility** services:

1. **`ClientRegistry`** — owns the `Map<serverId:root, LSPClientInfo>`
   and the broken/circuit-breaker state. Knows nothing about
   diagnostics. Public surface:
   ```ts
   class ClientRegistry {
     async ensureClientForFile(filePath, server): Promise<SpawnedServer | undefined>
     getActiveClients(filePath): SpawnedServer[]
     getAuxiliaryClients(filePath, enabledIds): SpawnedServer[]
     getWarmClient(filePath): SpawnedServer | undefined
     shutdownAll(): Promise<void>
   }
   ```
   ~400 LOC.

2. **`DiagnosticAggregator`** — owns `lastKnownDiagnostics`,
   `lastKnownContentHash`, `lastDiagnosticsHealth`. Knows nothing
   about process spawning. Public surface:
   ```ts
   class DiagnosticAggregator {
     async collectForFile(filePath, scope, options): Promise<{
       merged: LSPDiagnostic[]
       perServer: PerServerEntry[]
       health: LSPDiagnosticsHealth
     }>
     getLastKnown(path, expectedHash?): LSPDiagnostic[] | undefined
     pruneOlderThan(ts): number
   }
   ```
   ~450 LOC. This is the home for `getDiagnostics` (current
   line 1171–1390, 220 LOC) and the `raceToCompletion`-based wait
   fan-out.

3. **`TouchOrchestrator`** — owns `recentTouches`, the
   `shouldSkipTouch` / `shouldSkipNotify` debounce, and the
   per-server wait-budget resolution (`perServerTimeout`,
   `readEnvDiagnosticsWaitMs`, the #242 fix). Public surface:
   ```ts
   class TouchOrchestrator {
     async touchFile(filePath, content, options): Promise<LSPDiagnostic[] | undefined>
     // plus the collectDiagnostics=false variants (openFile, updateFile)
   }
   ```
   ~250 LOC. This is the home for `touchFile` (current
   line 906–1100, 200 LOC).

4. **`WorkspaceWalker`** — owns `collectWorkspaceDiagnosticFiles`
   and the 5000-file cap. ~120 LOC. This is the home for the
   `lens_diagnostics full` walk.

`LSPService` then becomes a 200-LOC facade:

```ts
class LSPService {
  private registry = new ClientRegistry(this.spawnServer, this.logger);
  private diagnostics = new DiagnosticAggregator(this.registry);
  private touch = new TouchOrchestrator(this.registry, this.diagnostics);
  private walker = new WorkspaceWalker(this.registry, this.diagnostics);

  async touchFile(filePath, content, options) {
    return this.touch.run(filePath, content, options);
  }
  async getDiagnostics(filePath, mode) {
    return this.diagnostics.collectForFile(filePath, mode);
  }
  async definition(filePath, line, char) {
    const client = await this.registry.getWarmClientForNavigation(filePath);
    return client?.definition(filePath, line, char) ?? [];
  }
  // ... 15 more one-liners
}
```

### Why it matters

- **The 220-LOC `getDiagnostics` method is a refactor smell.** It
  contains the per-server wait fan-out, the `raceToCompletion`
  helper (line 1274), the `expectSemanticSecondPush` second-wait
  branch (line 1243), the cross-server dedup (line 1310–1330), the
  last-known-cache update (line 1365–1372), and the
  `lsp_diagnostics_aggregate` latency log (line 1185–1235). Every
  fix to diagnostics-wait correctness (we've done at least 3:
  #203, #240, #242 per `AGENTS.md`) has touched this method.
  Carving it out makes future fixes localised.
- **The cooldown / circuit-breaker state and the diagnostic cache
  share no concerns** but live in the same class. They could be
  tested in isolation if extracted.
- **The class has 16 navigation methods that are pure proxies** —
  every one is `getWarmClient → call same-named method on client`.
  This is the most mechanically-refactorable slice: 12 of the 16
  collapse to a 4-line `defineNavMethod` builder (same trick as
  R1 for the client class).

### Forcing function

The 220-LOC `getDiagnostics` method will grow when (not if) the next
diagnostics-wait fix lands. We just shipped #242 (per-server
deadlines), and the *next* known follow-up is "stale-pull
re-validation" (a result reported by a pull-diagnostics server
that beat the budget but was computed against a pre-edit document —
same bug as section 1 above, but on the pull side). That fix
adds another 30–50 LOC to `getDiagnostics`. Do R3 first as a
co-commit.

**Estimated size:** ~1200 LOC of mechanical extraction + ~300 LOC
of new tests for the four smaller services. **Risk:** medium-high
(the seams are not yet obvious — `getDiagnostics` mixes registry,
wait, dedup, cache update, and logging in one function).
**Blast radius:** very high — every consumer of `LSPService` would
re-verify (the `mcp/server.ts` `pilens_*` tools, the dispatch
runner, `actionable-warnings.ts`, `cascade.log`-reading tools).

**Do not** do this speculatively. It belongs in a dedicated PR
**with** the next diagnostics-wait fix, and the PR should be
flagged `enhancement` + `area:lsp` + `area:dispatch` per
`AGENTS.md`'s labelling convention.

---

## R4 — Codify the spawn-shape taxonomy (`createInteractive` / `createManaged` / `createFallbackChain`)

### The current state

`LSPServerInfo.spawn` is a `function` that takes `(root, options)`
and returns `{ process, initialization?, source? } | undefined`.
There is no shared shape — each server picks one of three
in-line patterns:

1. **Interactive** (~10 servers: `JavaServer`, `SwiftServer`,
   `DartServer`, `LuaServer`, `CppServer`, `HaskellServer`,
   `ElixirServer`, `OCamlServer`, `NixServer`, `OmniSharpServer`)
   — wrap with `createInteractiveServer({...})` (line 539). 10
   servers in the registry (verified at
   `grep -cE "= createInteractiveServer\(" clients/lsp/server.ts`
   → 10 call sites at lines 1509, 1546, 1579, 1588, 1598, 1607,
   1642, 1652, 1681, 1730).
2. **Managed install** (~20 servers: `TypeScriptServer`,
   `PythonServer`, `PythonJediServer`, `RustServer`,
   `PHPServer`, `ZigServer`, `GleamServer`, `ClojureServer`,
   `TerraformServer`, `BashServer`, plus a large
   `*language-server` / `*-lsp` family — opengrep, ast-grep,
   deno, dockerfile, yaml, json, html, taplo, prisma, vue,
   svelte, css) — hand-rolled with `resolveAndLaunch` +
   `managedToolId`. 20 servers (verified at
   `grep -cE 'managedToolId:\s*"' clients/lsp/server.ts`
   → 20 unique tool-id values). Note `KotlinServer`,
   `FSharpServer`, and `CSharpServer` use the `runtimeInstall`
   path (not the npm registry), so they aren't in this list;
   `RubyServer` is its own third shape.
3. **Fallback chain** (`RubyServer` — tries `ruby-lsp`, then
   `solargraph`, then `rubocop --lsp`) — hand-rolled with three
   back-to-back `resolveAndLaunch` calls. 1 server today, but
   conceptually extensible.

The 20 "managed" servers each repeat the same 30-LOC
`resolveAndLaunch + initialize + return shape` boilerplate. Adding
the `expectSemanticSecondPush` knob (a DiagnosticStrategy field)
took a hand-edit to 5 server defs in #239. The 10 "interactive"
servers are uniform but `createInteractiveServer` is a single
~50-LOC factory; the 12 "managed" servers have no factory at all.

### What a codification could look like

```ts
// sketch — clients/lsp/server-spawn.ts

/** "I have a binary on PATH (or an SDK) and don't need an installer." */
export function createInteractiveServer(spec: InteractiveServerSpec): LSPServerInfo { /* current 50 LOC */ }

/** "I can be installed via the npm/pip/gem/dot tool registry if absent." */
export function createManagedServer(spec: {
    id: string
    name: string
    extensions: readonly string[]
    root: RootFunction
    /** Ordered list of (PATH-or-absolute) candidates to try before falling through to ensureTool. */
    candidates: (root: string) => string[]
    /** LSP args to pass on launch. */
    args: string[] | ((root: string) => string[])
    /** installer registry id — checked/installed via ensureTool(). */
    managedToolId: string
    /** Optional initialisation blob to send with `initialize` (pyright pythonPath, tsserver path, ...). */
    initialization?: Record<string, unknown> | ((root: string) => Record<string, unknown>)
    /** Optional env override. */
    env?: NodeJS.ProcessEnv
}): LSPServerInfo { /* ~50 LOC, replaces 12 hand-rolled bodies */ }

/** "Try A, then B, then C — return the first that launches." */
export function createFallbackChainServer(spec: {
    id: string
    name: string
    extensions: readonly string[]
    root: RootFunction
    attempts: Array<{
        candidates: (root: string) => string[]
        args: string[] | ((root: string) => string[])
        runtimeInstall?: { runtimeCommand: string; install: () => Promise<boolean> }
    }>
}): LSPServerInfo { /* ~40 LOC, replaces RubyServer's body */ }
```

`TypeScriptServer` then becomes ~20 LOC of declarative spec; the
`resolveAndLaunch` / `findTsserverPath` / `getToolEnvironment`
plumbing moves into `createManagedServer`. Net win: each server
definition shrinks to the *unique* part (root markers,
extensions, initialisation shape).

### Why it matters

- **Adding a new LSP server is currently 50–100 LOC of plumbing.**
  The unique part (root markers, extensions, args) is 10 LOC. A
  new server requires understanding `resolveAndLaunch`,
  `findTsserverPath`, `getToolEnvironment`, `canInstall`, and the
  launch-error semantics — which is too much for the
  "register a new language server" use case. This is a
  *contributor-experience* problem more than a correctness one.
- **The 12 hand-rolled "managed" spawns drift over time.** E.g.
  `PythonServer` has a `detectPythonVenv` post-step; the
  TypeScriptServer has a `findTsserverPath` post-step; both could
  be expressed as a single `postSpawn?: (proc) =>
  Promise<{proc, initialization}>` hook in `createManagedServer`.
  When a new post-step is needed (e.g. Java's Lombok jar
  resolution, currently in `lombok.ts`), it has to be hand-added
  per server.

### Forcing function

The #241 archive-strategy work for jdtls / kotlin-language-server
/ clangd / lua-language-server / elixir-ls (per `AGENTS.md` "STILL
on the PATH-only `createInteractiveServer` path") is blocked on
exactly this — a `createManagedServer` factory that knows about
archive extraction, not just npm/pip registry lookup. When that
work starts, do R4 as a co-commit.

**Estimated size:** ~300 LOC of factory extraction + ~600 LOC
of server-definition refactor (20 managed servers * ~30 LOC saved each).
**Risk:** low if each server's `spawn` is verified against the
hand-rolled version via the test suite. **Blast radius:** every
`tests/clients/lsp/server-*.test.ts` will need a brief
re-verification pass.

---

## R5 — The `LSPTouchFileOptions` ↔ caller-coupling is opaque

### The current state

`LSPTouchFileOptions` (interface declared at
`clients/lsp/index.ts:185`) has 9 fields:

```ts
export interface LSPTouchFileOptions {
    diagnostics?: LSPDiagnosticsMode
    source?: string
    clientScope?: LSPTouchClientScope
    auxiliaryServerIds?: readonly string[]
    maxClientWaitMs?: number
    maxDiagnosticsWaitMs?: number
    collectDiagnostics?: boolean
    silent?: boolean
}
```

The interface is *intentionally permissive* — every field is
optional, defaults are scattered between the service, the
dispatch runner, and `RUNTIME_CONFIG`. The current call sites
demonstrate the problem:

- `clients/dispatch/runners/lsp.ts:99-106` passes 6 of the 9
  fields, hard-codes the values inline.
- `clients/dispatch/integration.ts:993` (cascade) passes 3 of
  the 9 fields with *different* defaults — `silent: true`,
  `clientScope: "all"`.
- `clients/actionable-warnings.ts` (per AGENTS.md) calls
  `touchFile` with `collectDiagnostics: true` for the turn-end
  advisory.

There are **3 call sites in 3 files** (`grep -rln "touchFile" clients/` → 3: `lsp/index.ts` (own usage),
`actionable-warnings.ts`, `dispatch/auxiliary-lsp.ts`,
`dispatch/integration.ts`, `dispatch/runners/lsp.ts` — 5 sites
total counting the auxiliary-lsp module's uses). Each call site
makes a *different* set of trade-offs (per-edit budget, scope,
auxiliary set) but reaches into the same permissive interface.

### What a refactor could look like

Replace the 9-field bag with a **3-class named scenario**:

```ts
// sketch — clients/lsp/index.ts (or a new clients/lsp/touch-scenarios.ts)

/** Per-edit hot path: tight budget, primary + auxiliary, with-auxiliary scope. */
export function touchFileForEdit(
    filePath: string, content: string,
    options: { maxDiagnosticsWaitMs?: number; auxiliaryServerIds?: readonly string[] }
): Promise<LSPDiagnostic[] | undefined>;

/** Cascade: cheaper (silent, no notify-clear), full scope, no auxiliary. */
export function touchFileForCascade(
    filePath: string, content: string,
): Promise<LSPDiagnostic[] | undefined>;

/** Turn-end advisory: collect + cache-prime, primary only, no per-edit budget. */
export function touchFileForAdvisory(
    filePath: string, content: string,
): Promise<LSPDiagnostic[] | undefined>;
```

The 3 (now 5) call sites pick a *named* scenario; the
per-scenario defaults live in the service, not the caller. The
internal `touchFile` stays the implementation; the public
methods are typed views.

### Why it matters

- **The current 9-field bag is impossible to read safely.** A
  caller setting `silent: true` is asking for the cascade path,
  not the edit path — but there's no way to know without reading
  the implementation.
- **The `clientScope` decision is non-obvious.** The dispatch
  runner uses `with-auxiliary`; cascade uses `all`; the
  advisory uses `primary`. Encoding these as named scenarios
  makes the choice explicit at the call site.
- **The 9 fields have 3 "modes" (5 if you count silent) and
  ~15 mutually-compatible combinations** — most of which are
  never used. Pruning to 3 named scenarios is a *shrinkage* of
  API surface, which is what good interfaces do.

### Forcing function

The next time a caller is added (a 4th consumer beyond the
dispatch runner, cascade, and advisory) and the field
combinations start to disagree about the right defaults — that's
the moment. R5 is a *maintainability* refactor, not a *correctness*
one.

**Estimated size:** ~200 LOC (3 named methods + 1 internal
helper). **Risk:** low (additive surface; the legacy
`touchFile` keeps working). **Blast radius:** low (3 internal
call sites; no public MCP tools are affected).

---

## R6 — `lens_diagnostics` (mode=full) walker doesn't reuse the project-snapshot walker

### The current state

The `lens_diagnostics mode=full` flow does an *ad-hoc* project
file walk in `collectWorkspaceDiagnosticFiles`
(`clients/lsp/index.ts:238–325`, ~90 LOC) — it uses
`fs.promises.readdir` async + the shared
`getProjectIgnoreMatcher(root)` + the `isExcludedDirName` dir
filter, capped at 5000 files (per AGENTS.md #243/#250/#252).

The same ignore-aware, dir-capped, async-walk pattern is
implemented in *at least three* other places in the codebase:

- `clients/lsp/index.ts:collectWorkspaceDiagnosticFiles` (the
  one above)
- `clients/review-graph/*.ts` (the review-graph builder walk,
  per AGENTS.md — `getGraphSourceFiles` is `maxGraphFiles+1`
  scoped to `MAIN_KIND_EXTENSIONS`)
- `clients/lsp/lsp-index.ts` (the LSP workspace walk, 5000 cap
  per AGENTS.md)
- `clients/dispatch/runners/lsp.ts` warmup walker
  (`collectSourceFilesForWarmup` = 2000, per AGENTS.md)
- `clients/dispatch/dispatcher.ts` general
  `collectSourceFiles{,Async}` walks

Each has its own:
- Cap (`2000` / `5000` / `maxGraphFiles+1` / `getMaxWorkspaceDiagnosticFiles`)
- Filter set (`MAIN_KIND_EXTENSIONS` for the review graph; full
  server-extensions for the LSP walker; `isExcludedDirName` +
  `ignoreMatcher` for the project matcher)
- Async / sync pattern

AGENTS.md's load-bearing note: **"Scan exclusion + walk confinement
are shared invariants — don't reinvent them per walker (#243/#250
/#252/#253). Any new filesystem walker MUST (1) route dir
excludes through `isExcludedDirName` and path excludes through
`getProjectIgnoreMatcher(root).isIgnored(...)` — NOT a private
skip-list."** This is the same lesson learned three times; the
fix is to share the walker.

### What a refactor could look like

Extract a single `clients/file-walk.ts` (or extend
`clients/file-utils.ts`):

```ts
// sketch
export interface WalkOptions {
    root: string
    maxFiles: number
    /** Filter to source files for these kinds; undefined = all files. */
    kinds?: FileKind[]
    /** Extra path-level predicates (applied after ignore matcher). */
    predicate?: (absPath: string) => boolean
    /** Yield in chunks of N — caller can chunk-yield on big trees. */
    chunkSize?: number
    onChunk?: (chunk: string[]) => void | Promise<void>
}
export async function* walkProject(opts: WalkOptions): AsyncGenerator<string[]>
```

`collectWorkspaceDiagnosticFiles` becomes a 10-LOC wrapper:

```ts
const files: string[] = [];
for await (const chunk of walkProject({
    root, maxFiles: getMaxWorkspaceDiagnosticFiles(),
    kinds: [], // all source files; the predicate filters to LSP-supported
    predicate: (p) => getServersForFileWithConfig(p).length > 0,
})) {
    files.push(...chunk);
}
return files;
```

### Why it matters

- **Every new walker re-implements the same `try { readdir } catch
  { return }` + `isExcludedDirName` + `ignoreMatcher.isIgnored`
  + `maxFiles` early-stop pattern.** We've got the lesson in
  AGENTS.md, the lesson in the `#243` issue, the lesson in the
  `#250` fix, the lesson in the `#252` fix — and a 5th walker
  will be the 5th time the lesson is relearned. The fix is a
  single source of truth for the walk, with a
  `walkProject({...})` that every caller uses.
- **The current `collectWorkspaceDiagnosticFiles` walk can't
  chunk-yield on a 5000-file tree** (AGENTS.md "no
  `readdirSync`/`statSync`/`readFileSync` or regex-over-all-files
  on a hook path unless bounded and yielding" — the walker is
  async but doesn't yield between chunks). A generator-based
  `walkProject` would naturally yield per chunk.

### Forcing function

The next time a 6th walker is added, the duplication is past the
"shared invariant" point. Even better: a `lens_health mode=full`
or `lens_diagnostics` perf complaint is the forcing function
(per-file `access()` in `collectWorkspaceDiagnosticFiles` is
already chunked but not yielding).

**Estimated size:** ~150 LOC for the shared walker + ~50 LOC
saved per existing walker × 5 = 250 LOC saved. Net: **smaller**
codebase. **Risk:** low if the existing tests are the contract.
**Blast radius:** medium (5 callers; the worst case is a
`mode=full` `lens_diagnostics` walk that misses a file the
ad-hoc walker would have included — guarded by
`tests/clients/lsp/workspace-diagnostics.test.ts`).

---

## R7 — Auxiliary LSP discovery via flag wiring is hand-copied in 3 places

### The current state

`clients/dispatch/auxiliary-lsp.ts` exports two helpers:

- `enabledAuxiliaryLspServerIds(getFlag)` — returns the list of
  `serverId`s enabled for this turn.
- `findAuxiliaryProfileForSource(source)` — returns the
  `AuxiliaryLspProfile` whose `sourceMatch` regex matches the
  LSP diagnostic's `source` field.

The `AUXILIARY_LSP_PROFILES` array is the **registry** — two
entries today (opengrep + ast-grep), each with a `killSwitchFlag`
field (e.g. `"no-opengrep"`, `"no-ast-grep"`).

The same registry is consumed in three different ways:

1. `clients/dispatch/runners/lsp.ts:101-105` calls
   `enabledAuxiliaryLspServerIds((f) => ctx.pi.getFlag(f))` to
   pass the enabled set to `touchFile`.
2. `clients/dispatch/auxiliary-lsp.ts:79-80` (the
   `enabledAuxiliaryLspServerIds` impl) iterates
   `AUXILIARY_LSP_PROFILES` and reads the `killSwitchFlag`.
3. `clients/dispatch/runners/lsp.ts:208-210` calls
   `findAuxiliaryProfileForSource(validLspDiags[i]?.source)`
   *per diagnostic* — so the source-match regex is re-evaluated
   for every diagnostic in every file in every dispatch.

The third site is the bug. With N files × M diagnostics per file ×
P profiles (currently 2), the dispatch-lsp-runner does N×M×P
regex tests. With 5 profiles and 20 diagnostics per file, that's
100 regex tests per file — not expensive per se, but
gratuitous.

A second issue: the `AuxiliaryLspProfile.allowBlocking` and
`semantic` callbacks are per-profile. The runner caches them
per-profile (line 213-218 in lsp.ts):

```ts
const blockingAllowedByProfile = new Map<unknown, boolean>();
for (let i = 0; i < diagnostics.length; i++) {
    const profile = findAuxiliaryProfileForSource(validLspDiags[i]?.source);
    if (!profile) continue;
    let blockingAllowed = blockingAllowedByProfile.get(profile);
    if (blockingAllowed === undefined) {
        blockingAllowed = profile.allowBlocking?.(ctx.cwd) ?? false;
        blockingAllowedByProfile.set(profile, blockingAllowed);
    }
    ...
}
```

So the per-profile workspace checks (`findLocalOpengrepConfig`,
`findLocalSgconfig`) *are* cached — good. But the source-match
regex isn't.

### What a refactor could look like

1. **Pre-index profiles by an extracted source-tag.** Build a
   `Map<lowerSource, AuxiliaryLspProfile>` once at module load
   (the registry is 2 entries today; the map is 2 entries too,
   but the lookup is O(1) instead of O(P) regex per diagnostic).
2. **Make the per-source pattern explicit, not a regex.** The
   `sourceMatch: /opengrep|semgrep/i` shape implies the profile
   matches `source === "Semgrep"`, not "any source containing
   semgrep". A `sourceMatches: string[]` (case-insensitive
   equality) would be both faster and more honest about the
   intent.
3. **Move the "diagnostic → profile" lookup into the service
   layer.** `LSPDiagnostic` already carries `source`; the
   runner currently reads `validLspDiags[i]?.source` (line 208)
   — but the *service* could expose a helper that returns
   `{ profile, blockingAllowed, defectClass }` per LSP
   diagnostic, given a registered auxiliary id. The runner
   then doesn't need to know about `findAuxiliaryProfileForSource`
   at all.

### Why it matters

- **The current `findAuxiliaryProfileForSource` runs a regex
  per diagnostic.** Negligible at 2 profiles, but the registry
  is going to grow (per AGENTS.md: the #111 / #239 work is
  ongoing; a `vale` (markdown prose) and `cspell` auxiliary
  are the obvious next entries). At 5 profiles with 20
  diagnostics per file × 10 files per turn, that's 1000 regex
  matches per turn.
- **The `killSwitchFlag` is hard-coded per profile** — a
  new profile needs a flag added to the global flag registry
  separately. A `globalFlag` (or per-profile flag-bag) would
  centralise the wiring.
- **The semantic + blocking callbacks are stored on the
  profile** but the *call site* is in the dispatch runner.
  Moving the policy into the service (or a dedicated
  `auxiliary-policy.ts`) would let other consumers
  (e.g. the MCP `pilens_diagnostics` tool) apply the same
  policy without re-implementing it.

### Forcing function

Adding a 3rd auxiliary (the obvious candidate is `vale` or
`cspell` for markdown prose) is the moment. The 3-entry
registry with hand-copied wiring is the borderline.

**Estimated size:** ~80 LOC. **Risk:** low (the existing tests
in `tests/clients/dispatch/auxiliary-lsp.test.ts` are the
contract). **Blast radius:** low.

---

## R8 — Extend `raceToCompletion` (the only aggregation utility) to cover more cases

### The current state

`clients/lsp/aggregation.ts` is **91 LOC** and exports a single
helper, `raceToCompletion<T>(...)` — used exactly once, at
`clients/lsp/index.ts:1274`, inside the per-server
`waitForDiagnostics` fan-out in `getDiagnostics`.

The same "result-aware racing" pattern is **re-implemented
inline** in at least two other places:

1. The `workspace/didChangeWatchedFiles` debounce-queue
   proposed in section 4 above (a *server* of incoming events,
   not a *client* of completed promises — different shape, but
   the same "wait for a quality threshold" intent).
2. The `TouchOrchestrator.touchFile` per-server-wait fan-out
   (`clients/lsp/index.ts:1022-1048`) — the `await
   Promise.all(spawned.map((entry) => entry.client.waitForDiagnostics(...)))`
   block — doesn't currently use `raceToCompletion`, it just
   races all of them. If the per-server-staleness fix (the
   "each server gets its own deadline" #242 work) ever needs
   early-unblock ("any server has results → unblock, but let
   the slow aux keep waiting"), the touch path will want
   `raceToCompletion` semantics.

### What a refactor could look like

1. **Generalise the `raceToCompletion` predicate.** Currently
   it's `(results: T[]) => boolean` — a global "should we
   finalise" predicate. Add an *opt-in* per-result
   "is-this-a-quality-result" predicate and a per-promise
   grace, so a per-server version of the #242 fix can use it
   without inlining another 30 LOC of racing logic.
2. **Add a sync `settleAll` variant.** The current
   `raceToCompletion` early-exits; sometimes a caller wants
   "give me every result that lands within the budget" without
   early-finalisation. The `Promise.allSettled` shape is the
   obvious base, with a per-promise deadline.
3. **Add a `chunked` generator wrapper.** The current API
   returns a `Promise<T[]>`; a generator version
   (`async function* settleAsResults(...)`) would let a
   caller `for await (const result of settleAsResults(...))`
   and process results as they arrive, without buffering.

### Why it matters

- **The pattern is the "right" shape for any wait-many-promises-
  with-quality-threshold problem.** We've got one helper, used
  once. The 2 other "we should be using this" sites are
  inlined because the helper is too narrow.
- **The next diagnostics-wait fix (R3's "stale-pull re-validation"
  mention) will need exactly this shape** — per-server quality
  threshold + per-server grace + early unblock on any quality
  result. Inlining another 50 LOC of racing logic would be
  re-deriving a generic helper for the third time.

### Forcing function

The next consumer of "result-aware promise racing" — the
likely candidates are R4's `didChangeWatchedFiles` debounce
queue (which is a *stream* of events, not promises, so a
different shape) and the next diagnostics-wait fix
(per-server staleness, mentioned in R3). Either lands first;
add the helper as a co-commit.

**Estimated size:** ~80 LOC. **Risk:** low (the existing
`raceToCompletion` has a unit test in
`tests/clients/lsp/aggregation.test.ts` — the contract is
captured). **Blast radius:** low.

---

## Summary — when to do what

| # | Refactor | LOC saved | Risk | Blast radius | Forcing function |
|---|---|---|---|---|---|
| R1 | Split `client.ts` 1929 → 8 files | ~300 | Low | High (every LSP test) | Next diagnostics-wait or nav-staleness fix |
| R2 | Split `server.ts` 2243 → 7+10 files | ~500 | Medium | High (every server def moves) | #241 archive-strategy work for jdtls / clangd / elixir-ls |
| R3 | Extract 4 services from `LSPService` 2046 → facade | ~200 | Medium-high | Very high (every consumer) | Next diagnostics-wait fix (e.g. stale-pull re-validation) |
| R4 | Codify `createManaged` / `createFallbackChain` factories | ~200 | Low | Medium (20 server defs) | Same as R2 — #241 archive work |
| R5 | 3 named `touchFileFor*` scenarios | ~50 | Low | Low (3 internal call sites) | 4th consumer of `touchFile` |
| R6 | Shared `walkProject` generator | ~100 | Low | Medium (5 walker sites) | 6th walker, or `lens_diagnostics full` perf complaint |
| R7 | Index auxiliary profiles + move policy to service | ~30 | Low | Low (1 registry + 1 consumer) | 3rd auxiliary profile (vale / cspell) |
| R8 | Extend `raceToCompletion` to per-result predicate + generator | ~30 | Low | Low (1 helper, 2-3 consumers) | Next diagnostics-wait fix |

**No refactor is to be done speculatively.** Each is a co-commit
with the forcing function that justifies it. R1 + R3 + R8 cluster
together (all diagnostics-wait or nav-staleness). R2 + R4 cluster
together (both #241 archive work). R5 is its own thing. R6 is its
own thing. R7 is its own thing.

**The single highest-leverage refactor is R3** — it changes the
shape of the LSP service to a 4-service composition, which makes
every future diagnostics-wait or navigation fix local to one
service instead of spread across 2046 LOC. R3 is the *biggest*
leverage but also the *biggest* blast radius. Gate it on a real
diagnostics-wait fix, not on a calendar.