LSP Phase 2: warm kotlin-lsp subprocess + per-konstruction workspace.json + stdio bridge#42
Conversation
…json + stdio bridge Replace the Phase 1 stub KsrpcLanguageServer with a REAL bridge to a JetBrains kotlin-lsp (intellij-server --stdio) subprocess, producing genuine kcsg-aware diagnostics for the open konstruction. Positions stay in wrapped-.kt space (the ±3-line csgs↔kotlin translation is Phase 3 / #38). Bridge (two legs): - Frontend↔backend: the Phase 1 nested KonstructionService.lsp(client) is kept; the editor's KsrpcLanguageClient is stashed. - Backend↔subprocess: KotlinLspProcess spawns one warm intellij-server, wraps its stdin/stdout with lsp-ksrpc's asLspConnection, and connectAsLspClient(forwarder) yields a subprocess-facing KsrpcLanguageServer stub. BridgeLanguageServer delegates each request to that stub. Pull→push diagnostics: kotlin-lsp does NOT push publishDiagnostics; it answers PULL textDocument/diagnostic. The editor client (Phase 1 design) listens for pushes, so on didOpen the bridge polls the engine (retrying through the ~120s cold index) and forwards items up as publishDiagnostics, rewriting the wrapped-.kt URI back to the editor's content.csgs URI. Per-konstruction setup: KonstructionLspWorkspace synthesizes a workspace dir with (a) the wrapped .kt via KonstructionControllerImpl.copyContentToScript (compiler- exact wrapping) and (b) a workspace.json matching the proven PoC recipe (JAVA_MODULE + moduleSource + one project library pointing at lib.jar as a single root_itself CLASSES root). Built with JSON object builders so the backend needs no serialization compiler plugin. Lifecycle gating (per the lsp-kotlin maintainer): LifecycleState is driven by hand — initialized/shutdown/exit advance it, and awaitInitialized() gates the first server→client emit. No blocking client-bound request from inside initialize(). On exit the poll is cancelled and the subprocess-facing stub is closed. Config: KONSTRUCTOR_KOTLIN_LSP (or -Dkonstructor.kotlinLsp) points at the binary; KONSTRUCTOR_KOTLIN_LSP_SYSTEM_PATH persists the warm index. Unset/missing binary ⇒ LSP stays off (bridge degrades to an inert server), never a crash. CI has no binary, so this degrades to off and stays green. Also: fix the #41 carry-over nit in EditorPane (catch Throwable → Exception). Tests: - KonstructionLspWorkspaceTest (CI-safe): asserts the synthesized workspace.json matches the strict PoC schema and the wrapped .kt reuses the compiler wrapping. - BridgeLanguageServerIntegrationTest (LOCAL-only, double-gated on -Dintegration and KONSTRUCTOR_KOTLIN_LSP): drives the real bridge against the live engine. - LspPipeTest updated: the canned-diagnostic stub is gone, so with no engine on CI the flag-ON path must establish cleanly and surface zero diagnostics. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
monkopedia-reviewer
left a comment
There was a problem hiding this comment.
Reviewed to full tier-1 standard (correctness + quality + CI-green); approving.
Flag-OFF safety (load-bearing): airtight. EditorPane.produceState returns early on if (!lspEnabled) before any service.lsp() call, so BridgeLanguageServer is never constructed when the flag is off — the editor is byte-for-byte unchanged. Graceful-off (flag ON, no binary = CI): Config.isKotlinLspAvailable gates KotlinLspProcess.connect() → null → delegate=null → initialize() returns empty ServerCapabilities() and every notification handler does delegate ?: return. Inert, never a crash.
Lifecycle: correct. Hand-driven LifecycleState starts at INITIALIZING; advanceTo(INITIALIZED/SHUTTING_DOWN/EXITED) are all legal transitions and advanceTo is the idempotent no-throw variant. awaitInitialized() gates the first push; no blocking client-bound call inside initialize(). exit() cancels the poll + scope Job and exit()/close()s the subprocess stub. The warm subprocess being kept across konstructions is the documented Phase-5/#40 deferral, not a leak.
Tests meaningful. KonstructionLspWorkspaceTest (CI-safe) asserts the strict workspace.json schema (JAVA_MODULE, moduleSource+library deps, lowercase compile, root_itself CLASSES root, lib.jar path) + wrapped-.kt reuse of copyContentToScript. BridgeLanguageServerIntegrationTest genuinely drives the real engine, correctly double-gated and CI-skipped. LspPipeTest updated to assert the inert flag-ON path surfaces zero diagnostics and the app stays healthy.
Quality. Idiomatic Kotlin; dead Phase-1 StubLanguageServer removed with no orphan refs; Apache headers present; scope limited to Phase 2 (no #38 position translation); the #41 Throwable→Exception nit is fixed. Two non-blocking nits: one inline-FQN com.monkopedia.lsp.ServerCapabilities() against the file's import convention, and messages computed a line before the empty-check in the integration test — neither worth blocking.
CI: build-and-test green, MERGEABLE/CLEAN (verified via gh pr checks). Approving and merging.
Part of #35; Fixes #37. Builds on Phase 1 (#41). Additive + flag-gated (
SettingsViewModel.lspEnabled, default OFF): with the flag off (and on CI, which has no engine binary), the editor behaves byte-for-byte as before.Replaces Phase 1's stub
KsrpcLanguageServerwith a REAL bridge to a JetBrains kotlin-lsp (intellij-server --stdio) subprocess, producing genuine kcsg-aware diagnostics for the open konstruction. Positions stay in WRAPPED-.ktspace for now — the ±3-line csgs↔kotlin translation is the next phase (#38).Bridge architecture (two LSP legs)
KonstructionService.lsp(client): KsrpcLanguageServeris kept; the editor'sKsrpcLanguageClientis stashed.KotlinLspProcessspawns ONE warmintellij-server --stdio --system-path <dir>, wraps its stdin/stdout with lsp-ksrpc'sasLspConnection, andSingleChannelConnection<String>.connectAsLspClient(forwarder)yields a subprocess-facingKsrpcLanguageServerstub.BridgeLanguageServer(aDefaultLanguageServersubclass) delegates each request to that stub. Theforwarderis aKsrpcLanguageClientwhosetextDocumentPublishDiagnosticsforwards to the stashed frontend client (with the wrapped-.ktURI rewritten back to the editor'scontent.csgsURI).Pull→push diagnostics (key finding)
kotlin-lsp does not proactively push
publishDiagnostics; it answers PULLtextDocument/diagnostic(confirmed against the binary: push=None across a full warm cycle). kodemirror's editor client (the Phase 1 design) listens for PUSHED diagnostics. So on didOpen the bridge polls the engine via pull (retrying through the ~120s cold index, then a slow steady-state refresh) and forwards the resulting items up aspublishDiagnostics. Any push the engine does send is also forwarded — they coexist.Per-konstruction setup
KonstructionLspWorkspacesynthesizes, under the konstruction's script dir, a workspace dir containing:.ktviaKonstructionControllerImpl.copyContentToScript(konstructor's EXACT compiler wrapping — header swap + footer — so LSP positions match the compiler), andworkspace.jsonmirroring the proven PoC recipe: aJAVA_MODULEwithmoduleSource+ alibrarydep, and one project-level library pointing at the resolvedlib.jaras a singleCLASSESroot withinclusionOptions: root_itself, lowercasecompilescope, inline polymorphictypediscriminators. Built with JSON object builders (not@Serializableclasses) so the backend needs no serialization compiler plugin.Lifecycle gating (per the lsp-kotlin maintainer)
LifecycleStateis driven by hand:initialized()/shutdown()/exit()→lifecycle.advanceTo(...), andlifecycle.awaitInitialized()gates the first server→client emit (no diagnostics before the client sentinitialized). No blocking client-bound request is made from insideinitialize()(deadlock risk). Onexitthe poll is cancelled and the subprocess-facing stub isclose()d (cheap leak guard; full teardown/leak test is Phase 5 / #40).Config / graceful-off
KONSTRUCTOR_KOTLIN_LSP(or-Dkonstructor.kotlinLsp) points at the binary;KONSTRUCTOR_KOTLIN_LSP_SYSTEM_PATHpersists/warms the index. If the binary is unset or missing,Config.isKotlinLspAvailableis false,KotlinLspProcess.connectreturns null, andBridgeLanguageServerdegrades to an inert server (empty capabilities, notifications dropped) — LSP stays off, never a crash. CI has no binary, so this degrades to off.Also fixes the #41 carry-over nit:
EditorPane'sproduceStatenow usescatch (_: Exception)(notThrowable).Verification
Flag-OFF / CI-equivalent — GREEN (JDK 21, clean):
clean shadowJar— whole project compiles + bundles../gradlew test— backend + protocol unit/integration tests pass (incl. the new CI-safeKonstructionLspWorkspaceTestasserting the workspace.json matches the strict PoC schema + wrapped-.ktreuse).:backend/:frontend/:e2e/:protocol spotlessCheck— clean.:e2e:test -Pe2e(Xvfb + Mesa-EGL) — all green.LspPipeTestwas updated: the Phase-1 canned diagnostic is gone, so with no engine on CI the flag-ON path must establish cleanly and surface zero diagnostics (asserted), and the app stays healthy; the flag-OFF suite is unchanged.Real-engine — LOCAL ONLY (CI does NOT run the 393MB binary):
Pointed config at
/tmp/kotlin-lsp-poc/.../bin/intellij-server(warm--system-path) and ranBridgeLanguageServerIntegrationTest(double-gated on-Dintegration=true+KONSTRUCTOR_KOTLIN_LSP, so it SKIPS on CI). A deliberate-error konstruction (val broken = thisSymbolDoesNotExist123above a validcube {}DSL line) drove the real bridge end to end:Exactly 1 real kcsg-aware diagnostic, routed to the editor's
content.csgsURI, zero false positives on the validcube {}/primitive {}DSL. This proves: synthesize workspace.json + wrapped.kt→ spawn/connect subprocess → initialize/initialized/didOpen → pull diagnostics → push to the frontend client. This engine path is not CI-tested (no binary, no bundled lib resource on CI) — stated explicitly.🤖 Generated with Claude Code