LSP Phase 1: flag-gated ksrpc transport pipe (stub server)#41
Merged
Conversation
Prove the LSP message pipe end-to-end with a STUB server (no real Kotlin LSP engine yet). Additive and flag-gated: with the lspEnabled flag OFF the editor behaves byte-for-byte as before. protocol/ - Add com.monkopedia.lsp:lsp-ksrpc:1.0.0 (api; the @KsService LSP interfaces appear in the public KonstructionService surface). - KonstructionService gains a nested LSP sub-service: @KsMethod("/lsp") suspend fun lsp(client: KsrpcLanguageClient): KsrpcLanguageServer Passing KsrpcLanguageClient as a PARAM lets ksrpc multiplex the reverse publishDiagnostics channel onto the same WebSocket. KonstructionService already extends RpcBidiService (required because lsp() both returns and accepts a sub-service), so the tier bump did not ripple. backend/ - StubLanguageServer (subclass DefaultLanguageServer): canned InitializeResult advertising hover + completion; on textDocument/didOpen pushes one canned publishDiagnostics back through the held KsrpcLanguageClient. - KonstructionServiceImpl.lsp(client) stashes the client and returns the stub, served through the existing ksrpc graph. frontend/ (wasmJs) — behind a persisted lspEnabled flag, default OFF - Add kodemirror lsp-client + lsp-ksrpc deps. - EditorLspClient (KsrpcLanguageClient) routes server-pushed diagnostics into kodemirror's LSPClient.languageClient. - EditorPane builds LSPClient(server = lsp(EditorLspClient())) and appends languageServerSupport(...) to the extensions chain only when the flag is on and the session is ready — OFF leaves the extensions identical to before. - SettingsViewModel.lspEnabled (PersistedStateFlow, default false). e2e/ - LspPipeTest: with the flag ON, opening a konstruction round-trips the canned publishDiagnostics back to the editor (asserted via a JS-bridge hook). Verified: whole project compiles; full clean :e2e:test green (flag-OFF unchanged + flag-ON round-trip); spotlessCheck clean; backend/protocol unit tests pass. Part of #35; Fixes #36. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
monkopedia-reviewer
left a comment
monkopedia-reviewer
left a comment
There was a problem hiding this comment.
@Monkopedia — I've reviewed this entire PR to merge standard and everything bot-verifiable is sound; the only thing I'm leaving for your judgment is the shape of Phase 1's LSP transport pipe before this foundational surface lands.
Reviewed & sound (vetted at tier-1 standard):
- Flag-OFF safety is airtight. With
lspEnableddefault false,produceStatereturns null immediately,session'srememberkeys onlspExtension(null when OFF), and theif (lspExtension != null)append is skipped — the editor extensions chain is byte-for-byte identical to before. Real gating, not hidden UI. The default-OFF e2e suite passing green is the evidence. - ksrpc nested-sub-service usage is correct.
KonstructionServicealready extendedRpcBidiService, so the bidi requirement forlsp(client): KsrpcLanguageServerdid not ripple toKonstructor/Workspace. Passed/returned@KsServicetypes stay plainRpcService; backend stashes the client inStubLanguageServerand emitspublishDiagnosticson it over the multiplexed reverse channel. No glaring leak (full session teardown is explicitly deferred to a later phase). - Round-trip test is meaningful (not a tautology):
LspPipeTestcreates a real konstruction, flips the flag ON, and waits for a diagnostic that can only appear via a genuine backend→editor push, assertingcount >= 1. - CI green — independently verified:
build-and-testpassed (includes the:e2e:testgate with both flag-OFF suite and the new flag-ON round-trip). - Quality/scope: idiomatic, Apache-2022 headers on new files, scope limited to a stub (no real Kotlin engine creeping in).
One non-blocking nit (not gating): EditorPane's produceState uses catch (_: Throwable), where the codebase overwhelmingly uses catch (_: Exception) (40+ vs 2); Throwable also swallows CancellationException — benign here since the coroutine is torn down on re-key, but Exception would match house style. Flagging for a possible follow-up, not blocking this PR.
For your judgment: approve the foundational LSP transport pipe to land behind a default-off flag, as Phase 1's shape for epic #35?
monkopedia-reviewer
approved these changes
Jun 3, 2026
Collaborator
monkopedia-reviewer
left a comment
monkopedia-reviewer
left a comment
There was a problem hiding this comment.
Tier-3 signoff by Monkopedia: foundational LSP transport pipe lands behind the default-off lspEnabled flag. Reviewed to merge standard — flag-OFF airtight, ksrpc nested-sub-service usage correct (independently confirmed by the lsp-kotlin maintainer), meaningful round-trip test, CI green. The Throwable→Exception nit tracked as a Phase-2 follow-up.
This was referenced Jun 3, 2026
monkopedia-reviewer
pushed a commit
that referenced
this pull request
Jun 4, 2026
…json + stdio bridge (#42) 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First phase of the LSP epic (#35). Proves the LSP message pipe end-to-end with a stub server — no real Kotlin LSP engine yet. Additive + flag-gated: with
lspEnabledOFF the editor behaves byte-for-byte as today.protocol/
com.monkopedia.lsp:lsp-ksrpc:1.0.0(asapi, since the@KsServiceLSP interfaces appear in the publicKonstructionServicesurface). Built against ksrpc 1.1.0 / Kotlin 2.3.20 = our versions.KonstructionServicegains the nested LSP sub-service:KsrpcLanguageClientas a param lets ksrpc multiplex the reversepublishDiagnosticschannel onto the same WebSocket (no second connection, noconnectAsLspServer).KonstructionServicealready extendedRpcBidiService(required becauselsp()both returns and accepts a sub-service), so the tier bump did not ripple intoKonstructor/Workspace.backend
StubLanguageServer(subclassDefaultLanguageServer): cannedInitializeResultadvertising hover + completion; ontextDocument/didOpenpushes one cannedpublishDiagnostics(a single fake warning) back through the heldKsrpcLanguageClient.KonstructionServiceImpl.lsp(client)stashes the client and returns the stub, served through the existing ksrpc graph.frontend (wasmJs) — behind the
lspEnabledflag, default OFFlsp-client+lsp-ksrpcdeps.EditorLspClient(aKsrpcLanguageClient) routes server-pushed diagnostics into kodemirror'sLSPClient.languageClient.EditorPanebuildsLSPClient(server = lsp(EditorLspClient()))and appendslanguageServerSupport(...)to the editor extensions only when the flag is on and the session is ready — OFF leaves the extensions chain identical to before.SettingsViewModel.lspEnabled(PersistedStateFlow, defaultfalse).e2e
LspPipeTest: with the flag ON, opening a konstruction round-trips the cannedpublishDiagnosticsback to the editor (count=1, asserted via a JS-bridge hook).Verification
:e2e:testgreen — flag-OFF suite unchanged + flag-ON round-trip proven (LSP diagnostics received: count=1 uri=file:///0/0/content.csgs).spotlessCheckclean; backend + protocol unit tests pass.Part of #35; Fixes #36.
🤖 Generated with Claude Code