Skip to content

refactor: remove the LSP subsystem (#144) #191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
byapparov merged 3 commits into from
Oct 18, 2026
Merged

refactor: remove the LSP subsystem (#144) #191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
acd62bd
refactor: remove the LSP subsystem (#144)
byapparov Oct 17, 2026
e95c2d0
fix: keep deprecated lsp config on rewrite and scope review follow-up…
byapparov Oct 17, 2026
546f163
fix: warn on an lsp rule set through AICTRL_PERMISSION (#144)
byapparov Oct 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions .opencode/agent/translator.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,6 @@ JSONC
K2
LLM
LM
LSP
M2
MCP
MR
Expand Down Expand Up @@ -417,7 +416,6 @@ aictrl.server.close()
/init
/instance/dispose
/log
/lsp
/mcp
/mnt/
/mnt/c/
Expand Down Expand Up @@ -575,16 +573,13 @@ AICTRL_DISABLE_AUTOCOMPACT
AICTRL_DISABLE_AUTOUPDATE
AICTRL_DISABLE_DEFAULT_PLUGINS
AICTRL_DISABLE_FILETIME_CHECK
AICTRL_DISABLE_LSP_DOWNLOAD
AICTRL_DISABLE_MODELS_FETCH
AICTRL_DISABLE_PRUNE
AICTRL_ENABLE_EXA
AICTRL_ENABLE_EXPERIMENTAL_MODELS
AICTRL_EXPERIMENTAL
AICTRL_EXPERIMENTAL_BASH_DEFAULT_TIMEOUT_MS
AICTRL_EXPERIMENTAL_EXA
AICTRL_EXPERIMENTAL_LSP_TOOL
AICTRL_EXPERIMENTAL_LSP_TY
AICTRL_EXPERIMENTAL_OUTPUT_TOKEN_MAX
AICTRL_EXPERIMENTAL_OXFMT
AICTRL_EXPERIMENTAL_PLAN_MODE
Expand Down
1 change: 0 additions & 1 deletion .opencode/agent/triage.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ Use for core server issues in `packages/aictrl/`, excluding `packages/aictrl/src

Examples:

- LSP server behavior
- Harness behavior (agent + tools)
- Feature requests for server behavior
- Agent context construction
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
We want to make it easy for you to contribute to Aictrl. Here are the most common type of changes that get merged:

- Bug fixes
- Additional LSPs / Formatters
- Additional formatters
- Improvements to LLM performance
- Support for new providers
- Fixes for environment-specific quirks
Expand Down
4 changes: 0 additions & 4 deletions bun.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions packages/cli/BUN_SHELL_MIGRATION_PLAN.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ Current baseline from audit:
- Largest hotspots:
- `src/cli/cmd/github.ts` (33)
- `src/worktree/index.ts` (22)
- `src/lsp/server.ts` (21)
- `src/installation/index.ts` (20)
- `src/snapshot/index.ts` (18)

Expand Down Expand Up @@ -80,7 +79,6 @@ Migrate these first:

- `src/cli/cmd/github.ts`
- `src/worktree/index.ts`
- `src/lsp/server.ts`
- `src/installation/index.ts`
- `src/snapshot/index.ts`

Expand All @@ -101,7 +99,6 @@ Migrate git-centric call sites to `Process.git*` helpers:
Migrate residual non-git usages:

- `src/cli/cmd/tui/util/clipboard.ts`
- `src/util/archive.ts`
- `src/file/ripgrep.ts`
- `src/tool/bash.ts`
- `src/cli/cmd/uninstall.ts`
Expand Down
2 changes: 0 additions & 2 deletions packages/cli/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@
"drizzle-kit": "1.0.0-beta.12-a5629fb",
"drizzle-orm": "1.0.0-beta.12-a5629fb",
"typescript": "catalog:",
"vscode-languageserver-types": "3.17.5",
"why-is-node-running": "3.2.2",
"zod-to-json-schema": "3.24.5"
},
Expand Down Expand Up @@ -121,7 +120,6 @@
"tree-sitter-bash": "0.25.0",
"turndown": "7.2.0",
"ulid": "catalog:",
"vscode-jsonrpc": "8.2.1",
"web-tree-sitter": "0.25.10",
"xdg-basedir": "5.1.0",
"yargs": "18.0.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/script/schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function generate(schema: z.ZodType) {
allowTrailingCommas?: boolean
}

// used for json lsps since config supports jsonc
// config files are JSONC, so editors validating against this schema must allow comments
result.allowComments = true
result.allowTrailingCommas = true

Expand Down
2 changes: 0 additions & 2 deletions packages/cli/src/cli/cmd/debug/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { bootstrap } from "../../bootstrap"
import { cmd } from "../cmd"
import { ConfigCommand } from "./config"
import { FileCommand } from "./file"
import { LSPCommand } from "./lsp"
import { RipgrepCommand } from "./ripgrep"
import { ScrapCommand } from "./scrap"
import { SkillCommand } from "./skill"
Expand All @@ -16,7 +15,6 @@ export const DebugCommand = cmd({
builder: (yargs) =>
yargs
.command(ConfigCommand)
.command(LSPCommand)
.command(RipgrepCommand)
.command(FileCommand)
.command(ScrapCommand)
Expand Down
52 changes: 0 additions & 52 deletions packages/cli/src/cli/cmd/debug/lsp.ts
View file
Open in desktop

This file was deleted.

61 changes: 19 additions & 42 deletions packages/cli/src/config/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
printParseErrorCode,
} from "jsonc-parser"
import { Instance } from "../project/instance"
import { LSPServer } from "../lsp/server"
import { BunProc } from "@/bun"
import { ConfigMarkdown } from "./markdown"
import { constants, existsSync } from "fs"
Expand Down Expand Up @@ -197,7 +196,9 @@ export namespace Config {
}

if (Flag.AICTRL_PERMISSION) {
result.permission = mergeDeep(result.permission ?? {}, JSON.parse(Flag.AICTRL_PERMISSION))
const permission = JSON.parse(Flag.AICTRL_PERMISSION)
warnDeprecatedLsp({ permission }, "AICTRL_PERMISSION")
result.permission = mergeDeep(result.permission ?? {}, permission)
}

// Backwards compatibility: legacy top-level `tools` config
Expand Down Expand Up @@ -630,7 +631,9 @@ export namespace Config {
webfetch: PermissionAction.optional(),
websearch: PermissionAction.optional(),
codesearch: PermissionAction.optional(),
lsp: PermissionRule.optional(),
lsp: PermissionRule.optional().describe(
"@deprecated The LSP subsystem was removed; this rule has no effect.",
),
doom_loop: PermissionAction.optional(),
skill: PermissionRule.optional(),
})
Expand Down Expand Up @@ -1087,42 +1090,7 @@ export namespace Config {
),
])
.optional(),
lsp: z
.union([
z.literal(false),
z.record(
z.string(),
z.union([
z.object({
disabled: z.literal(true),
}),
z.object({
command: z.array(z.string()),
extensions: z.array(z.string()).optional(),
disabled: z.boolean().optional(),
env: z.record(z.string(), z.string()).optional(),
initialization: z.record(z.string(), z.any()).optional(),
}),
]),
),
])
.optional()
.refine(
(data) => {
if (!data) return true
if (typeof data === "boolean") return true
const serverIds = new Set(Object.values(LSPServer).map((s) => s.id))

return Object.entries(data).every(([id, config]) => {
if (config.disabled) return true
if (serverIds.has(id)) return true
return Boolean(config.extensions)
})
},
{
error: "For custom LSP servers, 'extensions' array is required.",
},
),
lsp: z.unknown().optional().describe("@deprecated The LSP subsystem was removed; this key is ignored."),
instructions: z.array(z.string()).optional().describe("Additional instruction files or patterns to include"),
layout: Layout.optional().describe("@deprecated Always uses stretch layout."),
permission: Permission.optional(),
Expand Down Expand Up @@ -1211,6 +1179,16 @@ export namespace Config {
return load(text, { path: filepath })
}

// The LSP subsystem was removed (#144). Configs written before then may still set the
// `lsp` block or an `lsp` permission rule. The schema accepts both as no-ops and keeps
// them, so rewriting a config file preserves them; each config file that sets one warns once.
function warnDeprecatedLsp(config: Record<string, unknown>, source: string) {
const permission = config.permission

@aictrl-dev aictrl-dev Bot Oct 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚪ lsp deprecation warns per config layer, not once.

🤖 Fix with your agent 
Fix this code review finding (aictrl-dev/cli PR #191, packages/cli/src/config/config.ts:1183-1190):

Problem: lsp deprecation warns per config layer, not once
Detail: withoutDeprecatedLsp() logs inside load(), and state() (config.ts:74-186) fans load() across well-known, global (config.json + aictrl.json + aictrl.jsonc), AICTRL_CONFIG, each project config, .aictrl files, AICTRL_CONFIG_CONTENT and managed files. A user with `lsp` in both global and project config gets 2+ warnings instead of the "one warning on load" the PR contract and tests state (tests only cover the single-file case). Behavior matches the existing tui-keys deprecation precedent (per-file warn at config.ts:1208), so impact is cosmetic duplication; each warning does carry its own path.

Implement the fix on the PR head branch and add a regression test that fails before the fix and passes after.
Why this matters

withoutDeprecatedLsp() logs inside load(), and state() (config.ts:74-186) fans load() across well-known, global (config.json + aictrl.json + aictrl.jsonc), AICTRL_CONFIG, each project config, .aictrl files, AICTRL_CONFIG_CONTENT and managed files. A user with lsp in both global and project config gets 2+ warnings instead of the "one warning on load" the PR contract and tests state (tests only cover the single-file case). Behavior matches the existing tui-keys deprecation precedent (per-file warn at config.ts:1208), so impact is cosmetic duplication; each warning does carry its own path.

  // The LSP subsystem was removed (#144). Configs written before then may still set
  // the `lsp` block or an `lsp` permission rule; both load as no-ops with one warning.
  function withoutDeprecatedLsp(config: Record<string, unknown>, source: string) {
    const permission = config.permission
    const hasLspPermission = typeof permission === "object" && permission !== null && "lsp" in permission
    if (!("lsp" in config) && !hasLspPermission) return config
    log.warn("lsp config is deprecated and ignored; the LSP subsystem was removed", { path: source })

const hasLspPermission = typeof permission === "object" && permission !== null && "lsp" in permission
if (!("lsp" in config) && !hasLspPermission) return
log.warn("lsp config is deprecated and ignored; the LSP subsystem was removed", { path: source })
}

async function load(text: string, options: { path: string } | { dir: string; source: string }) {
const original = text
const source = "path" in options ? options.path : options.source
Expand All @@ -1223,6 +1201,7 @@ export namespace Config {
const normalized = (() => {
if (!data || typeof data !== "object" || Array.isArray(data)) return data
const copy = { ...(data as Record<string, unknown>) }
warnDeprecatedLsp(copy, source)
const hadLegacy = "theme" in copy || "keybinds" in copy || "tui" in copy
if (!hadLegacy) return copy
delete copy.theme
Expand Down Expand Up @@ -1292,9 +1271,7 @@ export namespace Config {
}

function globalConfigFile() {
const candidates = ["aictrl.jsonc", "aictrl.json", "config.json"].map((file) =>
path.join(Global.Path.config, file),
)
const candidates = ["aictrl.jsonc", "aictrl.json", "config.json"].map((file) => path.join(Global.Path.config, file))
for (const file of candidates) {
if (existsSync(file)) return file
}
Expand Down
3 changes: 0 additions & 3 deletions packages/cli/src/flag/flag.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ export namespace Flag {
export const AICTRL_DISABLE_PRUNE = truthy("AICTRL_DISABLE_PRUNE")
export const AICTRL_PERMISSION = process.env["AICTRL_PERMISSION"]
export const AICTRL_DISABLE_DEFAULT_PLUGINS = truthy("AICTRL_DISABLE_DEFAULT_PLUGINS")
export const AICTRL_DISABLE_LSP_DOWNLOAD = truthy("AICTRL_DISABLE_LSP_DOWNLOAD")
export const AICTRL_ENABLE_EXPERIMENTAL_MODELS = truthy("AICTRL_ENABLE_EXPERIMENTAL_MODELS")
export const AICTRL_DISABLE_AUTOCOMPACT = truthy("AICTRL_DISABLE_AUTOCOMPACT")
export const AICTRL_DISABLE_MODELS_FETCH = truthy("AICTRL_DISABLE_MODELS_FETCH")
Expand All @@ -31,8 +30,6 @@ export namespace Flag {
export const AICTRL_EXPERIMENTAL_BASH_DEFAULT_TIMEOUT_MS = number("AICTRL_EXPERIMENTAL_BASH_DEFAULT_TIMEOUT_MS")
export const AICTRL_EXPERIMENTAL_OUTPUT_TOKEN_MAX = number("AICTRL_EXPERIMENTAL_OUTPUT_TOKEN_MAX")
export const AICTRL_EXPERIMENTAL_OXFMT = AICTRL_EXPERIMENTAL || truthy("AICTRL_EXPERIMENTAL_OXFMT")
export const AICTRL_EXPERIMENTAL_LSP_TY = truthy("AICTRL_EXPERIMENTAL_LSP_TY")
export const AICTRL_EXPERIMENTAL_LSP_TOOL = AICTRL_EXPERIMENTAL || truthy("AICTRL_EXPERIMENTAL_LSP_TOOL")
export const AICTRL_DISABLE_FILETIME_CHECK = truthy("AICTRL_DISABLE_FILETIME_CHECK")
export const AICTRL_EXPERIMENTAL_PLAN_MODE = AICTRL_EXPERIMENTAL || truthy("AICTRL_EXPERIMENTAL_PLAN_MODE")
export const AICTRL_MODELS_URL = process.env["AICTRL_MODELS_URL"]
Expand Down
Loading
Loading