feat(lsp): ast-grep LSP as the rule engine — auxiliary adopter (Phase 1) then consolidate/retire napi runner (Phase 2)

[apmantza]

Goal

Consolidate pi-lens''s rule-based ast-grep onto a single full-engine path: the ast-grep lsp server, driven by the repo''s own sgconfig.yml when present, else by pi-lens''s shipped rules via --config. This retires the napi subset interpreter (which ignores field:, has no bare-regex/anon-kind candidates) for rule scanning, expands language coverage (~7 napi file types → ~15 LSP languages), and unlocks engine-driven autofix via codeAction.

Verified feasibility: ast-grep lsp -c, --config <CONFIG_FILE> exists (checked the installed binary). So the no-sgconfig fallback — launch the LSP pointed at a pi-lens-synthesized sgconfig of our shipped rules — is supported. pi-lens already synthesizes an sgconfig into a session dir (sg-runner.ts:470), so the machinery exists.

NOT in scope

• The pilens_ast_grep_search / pilens_ast_grep_replace MCP tools stay on napi (ad-hoc pattern queries; the LSP doesn''t expose those). "Replace the runner" != "remove napi."

Phase 1 — ast-grep LSP as auxiliary-diagnostic-LSP adopter (sgconfig repos only) — LOW RISK, additive

Reuse the Opengrep auxiliary-LSP capability (clients/dispatch/auxiliary-lsp.ts, role:"auxiliary", with-auxiliary touchFile scope, profile registry, allowBlocking(cwd)).

• Register ast-grep lsp server gated on repo-root sgconfig.y[a]ml (its root_markers + workspace_required).
• Profile: map LSP source -> tool:"ast-grep" + semantic policy; blocking-eligible (sgconfig = curated repo rules, same logic as Opengrep''s curated-rules gate).
• Wire ast-grep rule fix: -> LSP codeAction -> pi-lens autofix / fixSuggestion -> actionable-warnings routing.
• Smoke: add a fixture to scripts/smoke-tools.mjs --lsp (auxiliaryServerIds) — sgconfig + a rule + a violating file; assert the ast-grep-sourced diagnostic returns alongside the primary.
• Does not touch the napi runner — purely adds the team-rules surface for sgconfig repos.

Phase 2 — consolidate the baseline + retire the napi runner — GATED

• Extend to the no-sgconfig path: ast-grep lsp --config <synthesized sgconfig of shipped rules> (advisory baseline, mirroring Opengrep auto-set = advisory).
• Port the napi runner''s policy (the bulk of the work): MAX_BLOCKING_RULE_COMPLEXITY, isOverlyBroadPattern, MAX_MATCHES_PER_RULE/MAX_TOTAL_DIAGNOSTICS caps, classifyDefect taxonomy, fixable/fixSuggestion routing, hasEslintConfig gating → into the auxiliary-LSP profile/aggregation.
• GATE A — latency: measure warm-LSP single-file edit latency vs in-process napi. napi is in-process (no spawn); the LSP must not regress the hot path. Retire napi only if comparable/better.
• GATE B — binary-absent degrade: napi is bundled (offline, zero-install); routing the universal baseline through the LSP makes @ast-grep/cli a hard dependency. Define the fallback when the binary is missing (degrade to napi vs skip).
• Only after both gates: retire the napi runner (keep napi for search/replace tools).
• Bonus once on the full engine: shipped rules can use field:/regex the napi subset couldn''t.

Acceptance (Phase 1)

• ast-grep lsp registered role:"auxiliary", sgconfig-gated, with strategy (reopenOnResync if it ignores didChange like Opengrep)
• auxiliary-lsp profile: source routing + semantic policy + blocking-on-curated-rules
• rule fix: surfaces as a fixSuggestion/autofix
• smoke-tools --lsp fixture validates end-to-end (auto-install, spawn, scan, sourced diagnostic alongside primary)
• registry-consistency guard passes

(Origin: "utilize the ast-grep LSP" thread. Sibling LSP work: #235/#236/#238; auxiliary-LSP capability shipped with Opengrep #111.)

[apmantza]

Audit update: rule-engine equivalence, Phase 1 status, Finding A fixed, runner gaps

Spent a session characterizing whether pi-lens's shipped rules actually behave the same across the napi runner and the ast-grep lsp engine (the Phase 2 question). Summary below; commits referenced inline.

Phase 1 is functionally complete (checkboxes were never ticked)

All five acceptance items are implemented and pass live:

• ast-grep registered role:"auxiliary", sgconfig-gated, with strategy — server.ts + server-strategies.ts
• aux-lsp profile (source routing + semantic + blocking-on-curated) — auxiliary-lsp.ts:66-82, consumed at dispatch/runners/lsp.ts:264-277
• rule fix: → codeAction → fixSuggestion — lsp.ts:236-245
• smoke --lsp fixture — ast-grep-aux, passes live (1/1, source:"ast-grep")
• registry-consistency guard — 17/17

Engine equivalence: PROVEN (napi ≡ ast-grep CLI ≡ ast-grep LSP)

The current napi runner is not a subset interpreter — since #206 it delegates to napi's native engine via root.findAll({rule, constraints}), the same Rust core as the CLI and the LSP. With rule sets and file traversal aligned, napi and the CLI produce identical matches across all 224 repo TS files (539 == 539, zero diff either direction). field:/regex:/has:/inside: all honored equally. So Phase 2's engine swap is diagnostic-neutral — provided the rule loading matches (see Finding A). Corrected a stale "subset interpreter" comment in server.ts (commit 517a739).

Finding A (FIXED — commit fb9f35d): the sgconfig wasn't loading all the rules

rules/ast-grep-rules/.sgconfig.yml is ruleDirs: ./rules, but the 29 slop rules lived in the parent as a multi-document slop-patterns.yml. ast-grep's native loader cannot read multi-doc rule files (only the napi runner's custom splitter could), so driving the LSP via this sgconfig would have silently dropped all 29.

• Split slop-patterns.yml → 29 single-rule files under ./rules (loaded identically by both engines).
• Fixed 2 with invalid kind conditional_expression → ternary_expression — these were dead in napi (findAll threw, silently caught) and would have hard-aborted the entire ast-grep LSP loader (it fails on the first bad rule in ruleDirs).
• Added tests/clients/dispatch/runners/ast-grep-rule-validity.test.ts — validates every shipped rule against the native engine (incl. utils:) so a future invalid-kind/malformed rule can't silently die or abort the loader.

Rule cleanup (commit 39a7f50): 4 rules with wrong/misleading advice removed

Surfaced now that these run through the full engine:

• ts-array-length-check (was the noisiest rule, 206 hits) — flags arr.length > 0, suggests if (arr), but [] is truthy.
• ts-typeof-equality — typeof x === 'string' IS the right primitive guard; instanceof is wrong for primitives.
• ts-explicit-undefined-check — the typeof x === 'undefined' form it flags is the safer one.
• ts-array-map-ceremony — Array.from(arr)/arr.map(x=>x) make a copy (often intentional).

183 rules remain. Opinionated-but-correct rules kept. The -js twins are kept and are correct for the LSP (grammar-strict: a language: TypeScript rule fires on .ts but not .js, so the twin is complementary — they double-fire only in the napi runner, which the LSP fixes).

napi-runner gaps the LSP would fix (~15 rules dead in production napi)

Discovered while validating — the napi runner's invocation drops things the LSP gets from the rule file:

1. utils: not passed — ast-grep-napi.ts:285 passes {rule, constraints} only, so the 3 utils:-based rules (no-dupe-keys, no-dupe-keys-js, no-dupe-class-members) throw in findAll → dead. The CLI/LSP loads utils from the file natively and runs them. (no-dupe-keys is severity error/blocking — activating it is a behavior change.)
2. TSX rules skipped (5) — runner skips non-ts/js languages (:262); 4 need Tsx grammar (JSX kinds), so they can't be relabeled to TypeScript. LSP-only.
3. Python rules skipped (7) — same language gate. LSP-only.

These are concrete arguments for Phase 2: the LSP runs ~15 rules the napi runner can't.

Gate B reframe

The issue text says routing the baseline through the LSP "makes @ast-grep/cli a hard dependency." That premise is already false — @ast-grep/cli is already a bundled dependencies entry and already required at runtime by sg-runner (tempScan) and the pilens_ast_grep_search/replace MCP tools. So Phase 2 adds no new dependency. The real Gate B is narrower: degrade to in-process napi when the CLI subprocess can't launch (spawn-restricted/sandboxed env, broken platform binary). napi is the right fallback precisely because it's a separate, in-process artifact pinned to the same engine version (0.43) — spawn-free and diagnostic-neutral.

Gate A (latency): PASS

ast-grep LSP warm latency, measured in isolation, is 549ms — the fastest auxiliary, inside the primary band. (The old "20557ms" was a benchmark artifact: primary+aux co-spawn under a max()-floor deadline, now fixed in #242.)

Remaining Phase 2 work

1. Policy port (the bulk): move the napi runner's caps / isOverlyBroadPattern / eslint-overlap suppression / classifyDefect / MAX_* limits into the aux-LSP profile/aggregation before the napi runner can be retired.
2. Gate B fallback: degrade-to-napi when the CLI can't spawn.
3. No-sgconfig baseline: ast-grep lsp --config <synthesized sgconfig of shipped rules>. Note: the synthesized config must load the same ./rules dir (now that slop is there) so coverage matches the napi runner.
4. (Optional, independent of Phase 2) fix the napi runner's utils/TSX/Python gaps so the current runner stops silently dropping ~15 rules.

[apmantza]

Scope correction: the "policy port (the bulk)" is mostly unnecessary

Re-examined the napi runner's policy layer against what the aux-LSP path already does. The earlier "port the bulk of the policy" framing is overstated — ~6 of 8 pieces are already handled or made moot:

napi policy (ast-grep-napi.ts)	through the LSP
classifyDefect / defect taxonomy	✅ already done — profile.defectClass (lsp.ts:277)
severity → blocking/warning, curated gate	✅ already done — profile.semantic + allowBlocking (lsp.ts:268-273)
fix: → fixSuggestion	✅ already done, better — engine codeAction (lsp.ts:230)
language gating (ts/js only)	✅ moot — sgconfig is grammar-strict (and correctly runs TSX/Python the runner can't)
dedup by rule id	✅ moot — one ./rules dir, per-file LSP
isOverlyBroadPattern / complexity	⚠️ defense vs bad rules — largely redundant with curated set + new validity guard
MAX_MATCHES_PER_RULE / MAX_TOTAL caps	⚠️ flood insurance — LSP already has file-size guards + is per-file
LINTER_OVERLAP(+eslint) / TREE_SITTER_OVERLAP	⚠️ the one real gap — avoids double-reporting what eslint / the tree-sitter runner already flag

The aux-LSP profile IS the policy layer and already exists. The genuine residue is overlap suppression, which is really a cross-runner dedup concern (don't let ast-grep + eslint + tree-sitter triple-report no-eval) — better solved generically at aggregation than ported rule-by-rule. Caps are optional.

Revised Phase 2 scope

1. No-sgconfig baseline — ast-grep lsp --config <synthesized sgconfig of ./rules> when no project sgconfig (the real feature). Coupling to be designed: must not double-fire with the still-present napi runner (see below).
2. Gate B fallback — degrade to in-process napi when the CLI can't spawn.
3. Overlap dedup — small; suppress ast-grep findings duplicating eslint/tree-sitter (ideally generic).

Supersedes the "Remaining Phase 2 work → policy port (the bulk)" line in the previous comment.

[apmantza]

Record correction: search/replace already CLI; overlap already deduped; Phase 2 core done

Two corrections to earlier notes, after reading the code:

pilens_ast_grep_search / pilens_ast_grep_replace are already CLI, not napi

AstGrepClient (which both tools wrap) routes every op through SgRunner → the ast-grep CLI subprocess (runner.exec/tempScanAsync/execRaw); it has no @ast-grep/napi import. The "MCP tools stay on napi" line in the issue body is wrong — nothing to convert. Corollary: the only production napi consumer is the dispatch lint runner (ast-grep-napi.ts), which is now LSP-superseded (dormant when the LSP is present), so @ast-grep/napi is needed solely as the Gate-B fallback.

"Overlap dedup" is largely already handled — not net-new Phase 2 work

dispatcher.ts already has two layers:

1. dedupeOverlappingDiagnostics — collapses same (file,line,col,defectClass,rule) across ALL tools, keeping highest priority.
2. suppressLintOverlapsWithLsp — drops lint-tool findings overlapping an lsp/ts-lsp finding by (file,line,defectClass).

The napi runner also skipped LINTER_OVERLAP (15 rules, under eslint) + TREE_SITTER_OVERLAP (5 rules) at rule-selection time. Those are the same shared-id/same-span cases layer 1 already collapses (ast-grep and eslint/tree-sitter share rule ids like no-cond-assign, nested-ternary). Residual is only the column/defectClass-mismatch edge — small. If ever wanted airtight: teach layer 2 to recognize the auxiliary LSP tools (opengrep/ast-grep), NOT port the skip-lists.

Revised Phase 2 status — core complete

Shipped this thread:

• Engine equivalence proven (napi ≡ CLI ≡ LSP; same Rust core via findAll)
• Finding A — sgconfig now loads all shipped rules (slop split; fb9f35d)
• No-sgconfig baseline — ast-grep lsp --config <shipped baseline>, attaches everywhere (da4b988)
• LSP supersedes napi; napi resumes as Gate-B fallback when the binary is absent (da4b988)
• Gate A (latency) PASS — 549ms warm; Gate B fallback in place
• Overlap — handled by existing dedup layers (above)

Remaining (optional / low-urgency):

• Airtight overlap for the column/defectClass-mismatch edge (≤20 rules) — teach suppressLintOverlapsWithLsp about the aux LSP tools.
• Formal retirement of the napi runner (it's already dormant when the LSP is available; keep napi for search/replace + Gate-B fallback).
• Independent: the napi runner's utils/TSX/Python invocation gaps (only affect the fallback path now).

So the "policy port (the bulk)" framing is retired: the aux-LSP profile already is the policy layer, and dedup already handles overlap.