fix(server): tolerate unknown protocol enum values in initialize

[16bit-ykiko]

Problem

LSP clients newer than the protocol schema clice was built against can fail the entire initialize handshake. The failure chain:

1. The client's ClientCapabilities carries enum values introduced after LSP 3.17 — e.g. codeActionKind.valueSet containing "notebook" / "refactor.move", new SymbolKind members in a valueSet, or unknown capability fields.
2. kotatsu's generated protocol layer decoded string enums by exact name lookup (they were generated as integer enum class) and narrowed integer enums to the smallest underlying type covering known members (SymbolKind → uint8_t).
3. The first unknown string or out-of-range integer anywhere inside InitializeParams failed the whole message parse. The server answered InvalidParams, the editor gave up, and clice never started.

This is reproducible today with vscode-languageclient 10 (which emits LSP 3.18 capabilities) against any pre-fix clice binary:

initialize rejected: code=-32602, message=unknown enum value 'create'
  at capabilities.workspace.workspaceEdit.resourceOperations[0]

It went unnoticed in CI because tests/replay.py treated error responses as a successful replay — a recorded session whose initialize was rejected still reported PASS.

Fix

The root cause is fixed in kotatsu (clice-io/kotatsu#167, merged as b4bcd3c): string-valued enumerations are now generated as std::string wrapper types with named constants, so unknown values pass through decoding unchanged; integer-valued enumerations keep the LSP wire-format width (integer/uinteger), so out-of-range values survive as well.

This PR consumes that fix and hardens clice against the failure class ever returning:

• Bump the kotatsu pin in cmake/package.cmake to the merged commit b4bcd3c, and adapt the two MarkupKind member spellings in hover to the new wrapper shape.
• Generative robustness tests (tests/integration/lifecycle/test_protocol_robustness.py + tests/integration/utils/injection.py): walk lsprotocol's type metadata and build an initialize request that injects, into every enum field reachable from InitializeParams, (a) unknown string enum values, (b) out-of-range integers, (c) unknown object fields — then assert the server answers initialize normally and shuts down cleanly. Three parametrized modes, with a floor assertion so a silently shrinking injection count fails the test.
• Real-client smoke fixture (tests/smoke/session_lsp318.jsonl): a session recorded from stock VS Code 1.124 running the clice extension with vscode-languageclient 10 — initialize with LSP 3.18 capabilities, didOpen, hover, code actions, semantic tokens, shutdown, exit. Picked up automatically by the existing smoke-test glob.
• Close the CI blind spot: tests/replay.py now fails a replay whose initialize response is an error. With this change alone, a pre-fix binary fails the new fixture with exactly the error above; cancellation errors on other requests still replay as PASS, as before.

Verification

• Unit 599 / integration 175 / smoke 3/3 all pass locally (RelWithDebInfo), with kotatsu fetched from GitHub at the pinned rev — same path CI takes.
• Negative check: a pre-fix clice binary fails the new smoke fixture with the initialize rejected error; the fixed binary passes 3/3.
• Live editor test: latest stable VS Code (1.124) with a vscode-languageclient 10 build of the extension — server starts, file opens, hover returns content.

Notes

• The shipped extension currently pins vscode-languageclient 9.0.1, which still sends LSP 3.17-era capabilities, so stock VS Code upgrades alone have not triggered this yet — any newer client library or editor does. Bumping the extension to languageclient 10 (which also requires switching to createOutputChannel(..., { log: true }) for its LogOutputChannel requirement) is left as a follow-up.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR normalizes hover markup enums, adds a hostile-params injection utility and integration test for initialize robustness, enhances replay initialization error reporting, updates a kotatsu dependency pin, and adds a smoke-session fixture.

Changes

LSP Protocol Robustness Testing and Related Updates

Layer / File(s)	Summary
Hover response markup enum variant normalization src/feature/hover.cpp, tests/unit/feature/hover_tests.cpp	Updates protocol::MarkupKind enum usage from capitalized variants (Markdown, Plaintext) to lowercase variants (markdown, plain_text) in hover response construction and corresponding test assertion.
Kotatsu dependency version bump cmake/package.cmake	Updates the pinned kotatsu dependency revision in CMake FetchContent configuration to a new commit hash.
Replay initialization failure detection tests/replay.py	Enhances the replay test harness with state tracking for initialize request IDs and collected error responses, enabling early failure reporting when initialization fails separately from existing hang/crash detection.
LSP hostile parameter injection utility tests/integration/utils/injection.py	Implements a reusable parameter injection utility that recursively walks LSP protocol type metadata and generates malformed JSON payloads by injecting unknown/out-of-range enum values and unknown object fields, with injection statistics and recursion depth limits.
Protocol robustness integration test tests/integration/lifecycle/test_protocol_robustness.py	Adds parametrized integration test validating LSP server initialize handling against three hostile input modes, verifying successful handshake structure, processing initialization/shutdown sequence, and confirming clean process termination.
LSP session smoke test fixture tests/smoke/session_lsp318.jsonl	Provides a recorded LSP session trace for hello_world workspace covering full lifecycle: initialization, document operations, multiple editor requests, cancellations, trace control, and graceful shutdown.

sequenceDiagram
  participant Test
  participant InjectionFactory
  participant Server
  Test->>InjectionFactory: build_params(InitializeParams, mode)
  InjectionFactory-->>Test: params, stats
  Test->>Server: initialize(hostile_params)
  Server-->>Test: initialize response
  Test->>Server: initialized notification
  Test->>Server: shutdown request
  Server-->>Test: shutdown response
  Test->>Server: exit

Loading

Possibly related PRs:

• clice-io/clice#444: Updates the same kotatsu pinned GIT_TAG in cmake/package.cmake.

🎯 3 (Moderate) | ⏱️ ~25 minutes

"I nibble at enums, both strange and new,
I toss unknown fields into the stew,
Replay keeps watch when init goes awry,
Tests hop in circles till failures say goodbye,
A rabbit's small cheer for robust sky-high!"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 5.26% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the main change: fixing the server to handle unknown protocol enum values in the initialize request, which is the core issue addressed across all modifications.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/lsp-string-enum-compat

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

cmake/package.cmake (1)

44-44: Track the kotatsu PR merge method to determine if pin needs updating.

As noted in the PR objectives, if clice-io/kotatsu#167 is squash-merged, this commit hash will become invalid and the pin will need to be updated to the squash commit. Monitor the merge strategy when that PR lands.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmake/package.cmake` at line 44, The GIT_TAG pin in cmake/package.cmake (the
line containing GIT_TAG 0a1141887ff3a5a48c146416e965fbf90d99e632) may become
invalid if clice-io/kotatsu#167 is squash-merged; monitor that PR's merge
strategy and, if it is squash-merged, update the GIT_TAG to the resulting squash
commit hash (or switch to a stable tag) so the pin resolves correctly; update
the comment next to GIT_TAG to note which PR it tracks (clice-io/kotatsu#167)
and why the pin may need refreshing.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@cmake/package.cmake`:
- Line 44: The GIT_TAG pin in cmake/package.cmake (the line containing GIT_TAG
0a1141887ff3a5a48c146416e965fbf90d99e632) may become invalid if
clice-io/kotatsu#167 is squash-merged; monitor that PR's merge strategy and, if
it is squash-merged, update the GIT_TAG to the resulting squash commit hash (or
switch to a stable tag) so the pin resolves correctly; update the comment next
to GIT_TAG to note which PR it tracks (clice-io/kotatsu#167) and why the pin may
need refreshing.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 370d15dd-a0ca-4fe2-b331-9eb0bb0cd0eb

📥 Commits

Reviewing files that changed from the base of the PR and between d20e897 and e83a6a6.

📒 Files selected for processing (7)

• cmake/package.cmake
• src/feature/hover.cpp
• tests/integration/lifecycle/test_protocol_robustness.py
• tests/integration/utils/injection.py
• tests/replay.py
• tests/smoke/session_lsp318.jsonl
• tests/unit/feature/hover_tests.cpp

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/integration/lifecycle/test_protocol_robustness.py`:
- Around line 59-65: The test creates the subprocess via
asyncio.create_subprocess_exec and opens stderr as a PIPE (proc.stderr) but
never reads it, risking the pipe filling and deadlocking initialize/shutdown
waits; fix by not leaving stderr unconsumed — either redirect stderr to stdout
(use stderr=asyncio.subprocess.STDOUT) or to DEVNULL, or add an async task to
continuously read proc.stderr (e.g., a reader coroutine that drains proc.stderr)
so the child cannot block on writing to stderr; update the subprocess creation
call or add the draining coroutine and ensure it runs for the lifetime of proc.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8c5791f1-3015-42b9-a787-254a0ba3aa2f

📥 Commits

Reviewing files that changed from the base of the PR and between e83a6a6 and 19314bc.

📒 Files selected for processing (7)

• cmake/package.cmake
• src/feature/hover.cpp
• tests/integration/lifecycle/test_protocol_robustness.py
• tests/integration/utils/injection.py
• tests/replay.py
• tests/smoke/session_lsp318.jsonl
• tests/unit/feature/hover_tests.cpp

✅ Files skipped from review due to trivial changes (2)

• cmake/package.cmake
• tests/smoke/session_lsp318.jsonl

🚧 Files skipped from review as they are similar to previous changes (4)

• tests/unit/feature/hover_tests.cpp
• tests/replay.py
• src/feature/hover.cpp
• tests/integration/utils/injection.py

[coderabbitai]

♻️ Duplicate comments (1)

tests/integration/lifecycle/test_protocol_robustness.py (1)

59-65: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid unconsumed stderr pipe to prevent test hangs.

Line 64 sets stderr to PIPE, but proc.stderr is never drained. If server logs are verbose, the child can block on write and hang this test.

Suggested fix

     proc = await asyncio.create_subprocess_exec(
         str(executable),
         "server",
         stdin=asyncio.subprocess.PIPE,
         stdout=asyncio.subprocess.PIPE,
-        stderr=asyncio.subprocess.PIPE,
+        stderr=asyncio.subprocess.DEVNULL,
     )

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/lifecycle/test_protocol_robustness.py` around lines 59 -
65, The test spawns a subprocess with asyncio.create_subprocess_exec and sets
stderr=PIPE but never reads proc.stderr, which can cause the child to block;
update the test to either redirect stderr to asyncio.subprocess.DEVNULL or
ensure proc.stderr is consumed (e.g., read/drain proc.stderr asynchronously or
start a task to read it) so the child's stderr buffer cannot fill and hang the
test when using proc from asyncio.create_subprocess_exec.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@tests/integration/lifecycle/test_protocol_robustness.py`:
- Around line 59-65: The test spawns a subprocess with
asyncio.create_subprocess_exec and sets stderr=PIPE but never reads proc.stderr,
which can cause the child to block; update the test to either redirect stderr to
asyncio.subprocess.DEVNULL or ensure proc.stderr is consumed (e.g., read/drain
proc.stderr asynchronously or start a task to read it) so the child's stderr
buffer cannot fill and hang the test when using proc from
asyncio.create_subprocess_exec.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0e325340-b0b5-4691-b54c-15ad78fe0873

📥 Commits

Reviewing files that changed from the base of the PR and between 19314bc and 06e58c9.

📒 Files selected for processing (7)

• cmake/package.cmake
• src/feature/hover.cpp
• tests/integration/lifecycle/test_protocol_robustness.py
• tests/integration/utils/injection.py
• tests/replay.py
• tests/smoke/session_lsp318.jsonl
• tests/unit/feature/hover_tests.cpp

✅ Files skipped from review due to trivial changes (1)

• cmake/package.cmake

🚧 Files skipped from review as they are similar to previous changes (5)

• tests/unit/feature/hover_tests.cpp
• tests/replay.py
• tests/smoke/session_lsp318.jsonl
• src/feature/hover.cpp
• tests/integration/utils/injection.py