fix: LSP tool startup reliability, shutdown hangs, and refactor into focused files

[dgageot]

1. Server won't restart after a crash. startLocked() checked h.cmd != nil to decide if the server was running, but h.cmd stays non-nil after the process dies. A crashed server could never be restarted — every subsequent call got "LSP server already running". Fixed by adding a processDone channel that tracks actual process liveness.

2. stopLocked() hangs forever. It called sendRequestLocked("shutdown") synchronously, which blocks on reading stdout. If the server was unresponsive or already dead, this blocked indefinitely — and so did cmd.Wait() after it. Fixed by running the shutdown handshake in a goroutine with a 5-second timeout, then killing the process via context cancellation.

3. Data race on the stderr buffer. os/exec writes to a bytes.Buffer from one goroutine while readNotifications reads and resets it from another. bytes.Buffer is not thread-safe. Fixed by introducing a mutex-protected lockedBuffer.

Assisted-By: docker-agent

[docker-agent]

Review Summary

Assessment: 🟡 NEEDS ATTENTION

This PR successfully addresses the three critical issues described (server restart, shutdown hangs, and stderr data race). The refactoring into focused files improves code organization and the core fixes are well-designed.

However, the new lsp_files.go file introduces 3 medium-severity issues in the text edit handling logic:

1. Missing validation for malformed edit ranges (startLine > endLine)
2. TOCTOU race condition in file edit application
3. Silent error handling that hides LSP protocol violations

These issues could lead to file corruption when applying LSP edits. While the core concurrency fixes are solid, the file editing logic needs additional validation and error handling.

Findings

3 issues found (2 CONFIRMED, 1 LIKELY)

• 0 high severity
• 3 medium severity
• 0 low severity

[aheritier]

🎯 Comprehensive LSP Tools Testing Results

We conducted extensive testing of all 13 LSP tools across Go, Python, and TypeScript/JavaScript language servers. Here are our findings and configuration:

📊 Success Rates by Language

Language Server	Success Rate	Working Tools	Status
Go (gopls)	92% (12/13)	Most comprehensive	🟢 Excellent
TypeScript/JS	85% (11/13)	Great balance	🟢 Very Good
Python (pylsp)	46% (6/13)	Basic but solid	🟡 Limited

🔧 Tool-by-Tool Compatibility Matrix

LSP Tool	Go	Python	TypeScript	Notes
lsp_workspace	⚠️ Limited	✅ Full	✅ Full	Go: server already running limitation
lsp_document_symbols	✅ Full	✅ Full	✅ Full	Universally supported
lsp_workspace_symbols	✅ Full	❌ Not supported	✅ Full	pylsp limitation
lsp_hover	✅ Full	⚠️ Partial	✅ Full	Python: limited for imports
lsp_definition	✅ Full	⚠️ Partial	✅ Full	Python: local symbols only
lsp_references	✅ Full	✅ Full	✅ Full	Universally supported
lsp_diagnostics	✅ Full	✅ Full	✅ Full	Universally supported
lsp_code_actions	❌ Column error	⚠️ No actions	❌ Server bug	Implementation issues
lsp_call_hierarchy	✅ Full	❌ Not supported	✅ Full	pylsp limitation
lsp_type_hierarchy	✅ Full	❌ Not supported	❌ Not supported	Advanced feature
lsp_implementations	✅ Full	❌ Not supported	✅ Full	pylsp limitation
lsp_signature_help	✅ Full	❌ No help	✅ Full	Python: positioning issues
lsp_inlay_hints	✅ Works*	❌ Not supported	✅ Works*	*No hints available by default

🌟 Key Discoveries

Go (gopls) - Most Capable

• Outstanding performance: Found 480+ error implementations, 300+ io.Closer implementations
• Deep ecosystem integration: lsp_implementations reveals incredible Go ecosystem depth
• Only major issue: lsp_code_actions has persistent "column beyond end of line" errors
• Best for: Complex codebases requiring extensive type system exploration

TypeScript/JS (typescript-language-server) - Excellent Developer Experience

• Rich documentation: Hover info includes MDN links and comprehensive type information
• Strong workspace analysis: Excellent cross-file symbol and reference finding
• Modern tooling: Great integration with web development workflows
• Limitations: Code actions server bug, type hierarchy not supported for JavaScript files

Python (pylsp) - Reliable Basics

• Solid core features: Essential navigation (symbols, hover, definition, references) work consistently
• Major gaps: Missing 5 advanced LSP features (hierarchies, workspace symbols, implementations)
• Recommendation: Consider Pyright/Pylance for advanced Python development
• Best for: Simple Python projects with basic code intelligence needs

🚨 Common Issues Found

1. LSP Server State Management: All agents encountered connection issues requiring server restarts
2. Code Actions Bugs: Both Go and TypeScript have implementation issues in this feature
3. Path Resolution: Absolute paths work more reliably than relative paths
4. Server Capability Variations: Significant differences in what each LSP server supports

⚙️ Complete Multi-Language Agent Configuration

Here's our full working configuration from examples/lsp-code-reviewer.yaml:

Click to expand full configuration

#!/usr/bin/env docker agent run

# Multi-language code development with LSP intelligence.
#
# An orchestrator (planner) has read-only LSP access to Go, TypeScript, and
# Python so it can understand the entire codebase before delegating work.
# Language-specialist developer agents have full LSP + filesystem + shell
# access to implement changes.

models:
  default:
    provider: anthropic
    model: claude-sonnet-4-0
    max_tokens: 64000

agents:
  root:
    model: default
    description: Technical lead that reads the codebase across all languages, plans tasks, and delegates to language-specialist developers.
    instruction: |
      You are a technical lead and planner. You have read-only access to the
      entire codebase via LSP tools for Go, TypeScript, and Python.

      ## Your Role
      You PLAN and DELEGATE. You never write code yourself.

      ## Workflow
      1. **Understand the request** — use LSP and filesystem tools to explore
         the codebase. Start with lsp_workspace on each language server to
         discover capabilities.
      2. **Analyse** — use lsp_workspace_symbols, lsp_references,
         lsp_call_hierarchy, lsp_diagnostics, etc. to build a complete picture
         of what needs to change and where.
      3. **Plan** — use the think tool to reason about the approach, break the
         work into discrete tasks per language, and identify dependencies
         between them.
      4. **Delegate** — dispatch tasks to the right developer agent:
         - **go-developer**: anything touching .go files
         - **ts-developer**: anything touching .ts/.tsx/.js/.jsx files  
         - **py-developer**: anything touching .py files
         Run independent tasks in parallel with background_agents.
      5. **Verify & synthesise** — review the developers' results, run
         additional read-only checks if needed, and present a summary.

    sub_agents: [go-developer, ts-developer, py-developer]
    toolsets:
      - type: think
      - type: background_agents
      - type: filesystem
        tools: [read_file, read_multiple_files, list_directory, directory_tree, search_files_content]

      # Go LSP — read-only tools only
      - type: lsp
        command: gopls
        version: "golang/tools@v0.21.0"
        file_types: [".go"]
        tools: [lsp_workspace, lsp_hover, lsp_definition, lsp_references, lsp_document_symbols, lsp_workspace_symbols, lsp_diagnostics, lsp_code_actions, lsp_call_hierarchy, lsp_type_hierarchy, lsp_implementations, lsp_signature_help, lsp_inlay_hints]

      # TypeScript/JavaScript LSP — read-only tools only  
      - type: lsp
        command: typescript-language-server
        args: ["--stdio"]
        file_types: [".ts", ".tsx", ".js", ".jsx"]
        tools: [lsp_workspace, lsp_hover, lsp_definition, lsp_references, lsp_document_symbols, lsp_workspace_symbols, lsp_diagnostics, lsp_code_actions, lsp_call_hierarchy, lsp_type_hierarchy, lsp_implementations, lsp_signature_help, lsp_inlay_hints]

      # Python LSP — read-only tools only
      - type: lsp  
        command: pylsp
        file_types: [".py"]
        tools: [lsp_workspace, lsp_hover, lsp_definition, lsp_references, lsp_document_symbols, lsp_workspace_symbols, lsp_diagnostics, lsp_code_actions, lsp_call_hierarchy, lsp_type_hierarchy, lsp_implementations, lsp_signature_help, lsp_inlay_hints]

  go-developer:
    model: default
    description: Expert Go developer with full LSP code intelligence, filesystem, and shell access.
    toolsets:
      - type: lsp
        command: gopls
        version: "golang/tools@v0.21.0"
        file_types: [".go"]
      - type: filesystem
      - type: shell
      - type: todo

  ts-developer:
    model: default
    description: Expert TypeScript/JavaScript developer with full LSP code intelligence, filesystem, and shell access.
    toolsets:
      - type: lsp
        command: typescript-language-server
        args: ["--stdio"]
        file_types: [".ts", ".tsx", ".js", ".jsx"]
      - type: filesystem
      - type: shell  
      - type: todo

  py-developer:
    model: default
    description: Expert Python developer with full LSP code intelligence, filesystem, and shell access.
    toolsets:
      - type: lsp
        command: pylsp
        file_types: [".py"]
      - type: filesystem
      - type: shell
      - type: todo

🎯 Recommendations

For Go Development

• Use everything except lsp_code_actions (due to column positioning bug)
• Power tools: lsp_implementations and lsp_type_hierarchy provide incredible ecosystem insights
• Core workflow: hover, definition, references, workspace_symbols for navigation

For TypeScript/JavaScript Development

• Excellent overall experience with rich tooling support
• Standout features: hover with MDN docs, signature_help, comprehensive workspace symbols
• Great for: Modern web development with excellent project navigation

For Python Development

• Stick to the 6 working core tools for reliable functionality
• Essential tools: document_symbols, hover, definition, references, diagnostics
• Upgrade path: Consider Pyright for advanced features in larger Python projects

📋 Testing Methodology

Our testing was conducted by three specialized developer agents, each systematically testing all 13 LSP tools:

• ✅ Comprehensive coverage: All tools tested with real code examples
• ✅ Detailed documentation: Commands, results, and limitations recorded
• ✅ Server management: Complex LSP connection issues resolved and documented
• ✅ Practical recommendations: Based on real-world usage patterns

This provides a definitive reference for LSP tool capabilities across the three major language ecosystems in Docker Agent! 🚀

[aheritier]

Bug: Self-Deadlock in lspHandler.format() → Frozen Process

Root Cause

format() acquires h.mu and then calls NotifyFileChange(), which attempts to acquire the same h.mu. Since Go's sync.Mutex is not reentrant, the goroutine blocks forever waiting on itself.

Call Chain

(*LocalRuntime).RunStream.func1()          loop.go:353
  (*codeModeTool).Tools.func1()            codemode.go:82
    (*codeModeTool).runJavascript()        exec.go:66
      (*lspHandler).format()              lsp.go:499  ← acquires h.mu
        (*lspHandler).NotifyFileChange()  lsp_files.go:105  ← tries to re-acquire h.mu → DEADLOCK

Relevant Code

lsp.go (format):

func (h *lspHandler) format(ctx context.Context, args FileArgs) (*tools.ToolCallResult, error) {
    // ...
    h.mu.Lock()
    defer h.mu.Unlock()   // ← holds lock for entire function body

    h.sendRequestLocked("textDocument/formatting", ...)
    // ...
    applyTextEditsToFile(args.File, edits)

    h.NotifyFileChange(ctx, uri)  // ← line 527, called while h.mu is still held
}

lsp_files.go (NotifyFileChange):

func (h *lspHandler) NotifyFileChange(_ context.Context, uri string) error {
    // ...
    h.openFilesMu.Lock()
    h.openFiles[uri]++
    h.openFilesMu.Unlock()

    h.mu.Lock()   // ← line 105: BLOCKS — same goroutine already holds h.mu
    defer h.mu.Unlock()
    // never reached
}

Goroutine Dump Evidence

The SIGQUIT dump showed goroutine 24314 blocked for 189 minutes:

goroutine 24314 [sync.Mutex.Lock, 189 minutes]:
internal/sync.(*Mutex).lockSlow(0x317d2dbf3560)
sync.(*Mutex).Lock(...)
github.com/docker/docker-agent/pkg/tools/builtin.(*lspHandler).NotifyFileChange(0x317d2dbf3560, ...)
    lsp_files.go:105
github.com/docker/docker-agent/pkg/tools/builtin.(*lspHandler).format(0x317d2dbf3560, ...)
    lsp.go:527

Fix

Release h.mu immediately after sendRequestLocked returns, before the disk write and re-notification. Neither applyTextEditsToFile nor NotifyFileChange need the lock at call time — NotifyFileChange acquires it internally.

func (h *lspHandler) format(ctx context.Context, args FileArgs) (*tools.ToolCallResult, error) {
    // ...
    h.mu.Lock()
    result, err := h.sendRequestLocked("textDocument/formatting", map[string]any{...})
    h.mu.Unlock()  // release before disk I/O and re-notification

    if err != nil { ... }
    // ... parse edits, applyTextEditsToFile, NotifyFileChange as before ...
}

This narrows the critical section to just the LSP wire request, which is the only part that actually needs mutual exclusion.

[aheritier]

Bug Reports from Multi-Agent LSP Testing (jdtls + typescript-language-server)

We tested all 15 LSP tools across 4 agents (each with different LSP configs) in a multi-agent YAML setup with jdtls (Java) and typescript-language-server (TypeScript). Found 2 bugs — both present on this PR branch.

Testing Setup

# 6 agents, 4 with LSP:
root:                 jdtls (-data data/jdtls-cfp-cr)  + typescript-language-server --stdio
spring-boot-engineer: jdtls (-data data/jdtls-cfp-sbe)
angular-engineer:     typescript-language-server --stdio
code-reviewer:        jdtls (-data data/jdtls-cfp-cr)  + typescript-language-server --stdio
test-automator:       jdtls (-data data/jdtls-cfp-cr)  + typescript-language-server --stdio

Results: Every agent gets "LSP initialization failed: failed to start LSP server: LSP server already running" on ALL tool calls — except angular-engineer whose TypeScript LSP works (9/13 tools ✅).

---

Bug 1 (P1): ensureInitialized() fails when process was eagerly started by StartableToolSet.Start()

Symptom: "LSP initialization failed: failed to start LSP server: LSP server already running" on every tool call.

Root cause: State mismatch between startLocked() and ensureInitialized():

1. At boot, emitToolsProgressively() → StartableToolSet.Start() → LSPTool.Start() → startLocked() starts the OS process: sets h.cmd != nil, but h.initialized stays false (the LSP initialize handshake is not done in startLocked()).
2. On first tool call (e.g., lsp_hover), ensureInitialized() checks h.initialized.Load() && h.isProcessAlive() → false && true → falls through.
3. ensureInitialized() unconditionally calls startLocked() (line 262), which finds h.cmd != nil with a live process and returns "LSP server already running" (line 91).
4. The error propagates: "failed to start LSP server: LSP server already running".

Code trace (all in pkg/tools/builtin/lsp_handler.go on this branch):

// Line 248-262: ensureInitialized() — the missing guard
func (h *lspHandler) ensureInitialized() error {
    if h.initialized.Load() && h.isProcessAlive() { return nil }  // line 249: false && true → miss
    h.mu.Lock()
    defer h.mu.Unlock()
    if h.initialized.Load() && h.isProcessAlive() { return nil }  // line 257: same
    // ❌ BUG: unconditionally calls startLocked() even when process is alive but not yet initialized
    if err := h.startLocked(); err != nil {                       // line 262
        return fmt.Errorf("failed to start LSP server: %w", err)
    }
    // ... LSP handshake never reached ...
}

// Line 84-91: startLocked() — the guard that fires
func (h *lspHandler) startLocked() error {
    if h.cmd != nil {
        select {
        case <-h.processDone: h.cleanupLocked()     // process dead → cleanup ok
        default: return errors.New("LSP server already running") // process alive → ERROR
        }
    }
    // ...
}

Why angular-engineer works: It has only 1 LSP backend (typescript-language-server). Sub-agents may not trigger EmitStartupInfo, so Start() is never called eagerly — ensureInitialized() finds h.cmd == nil on first call and correctly starts + initializes.

Proposed fix — ensureInitialized() line 262:

// Before (line 261-263):
// (Re)start the server. startLocked handles cleaning up a dead process.
if err := h.startLocked(); err != nil {
    return fmt.Errorf("failed to start LSP server: %w", err)
}

// After:
// Only (re)start if the process is not running. It may have been
// pre-started by Start() (via StartableToolSet during EmitStartupInfo)
// without completing the LSP initialize handshake.
if !h.isProcessAlive() {
    if err := h.startLocked(); err != nil {
        return fmt.Errorf("failed to start LSP server: %w", err)
    }
}

---

Bug 2 (P2): codeActions() sends null diagnostics, crashes TypeScript Language Server

Symptom: lsp_code_actions returns LSP error -32603: Cannot read properties of null (reading 'filter') from TypeScript Language Server.

Root cause: In pkg/tools/builtin/lsp.go line 464, rangeDiags is declared as a nil slice:

var rangeDiags []lspDiagnostic  // nil when no diagnostics match

When no diagnostics are in the requested line range, rangeDiags stays nil. Go's json.Marshal(nil slice) produces null, so the request is sent as:

{ "context": { "diagnostics": null } }

The LSP spec requires diagnostics to be Diagnostic[] (an array). TypeScript Language Server's JavaScript code calls .filter() on this value, which crashes on null.

Proposed fix — lsp.go line 464:

// Before:
var rangeDiags []lspDiagnostic

// After:
rangeDiags := make([]lspDiagnostic, 0)  // empty slice → JSON [] instead of null

---

Full Test Results Matrix

Agent	Java (jdtls)	TypeScript (tsserver)
root (coordinator)	❌ Bug 1	❌ Bug 1
spring-boot-engineer	❌ Bug 1	N/A
angular-engineer	N/A	✅ 9/13 tools (Bug 2 affects lsp_code_actions)
code-reviewer	❌ Bug 1	❌ Bug 1
test-automator	❌ Bug 1	❌ Bug 1

Tools verified working on angular-engineer (TypeScript): lsp_workspace, lsp_document_symbols, lsp_workspace_symbols, lsp_diagnostics, lsp_hover, lsp_definition, lsp_references, lsp_call_hierarchy, lsp_implementations, lsp_signature_help.

Tools broken: lsp_code_actions (Bug 2), lsp_type_hierarchy (tsserver doesn't implement textDocument/prepareTypeHierarchy).

[aheritier]

Code Review

CI: ✅ All checks pass
File changed: pkg/tools/builtin/lsp.go (+165 / −199)

---

⚠️ PR Title/Description Mismatch

The PR title and body describe three fixes that do not exist in the diff:

• processDone channel for crash recovery
• 5-second shutdown timeout goroutine
• lockedBuffer for stderr data race

The actual changes (correctly described in the commit message) are entirely different and are reviewed below.

---

What This PR Actually Does

Area	Type	Verdict
format() self-deadlock	Bug fix (blocking)	✅ Correct
applyWorkspaceEdit() stale LSP state after rename	Bug fix	✅ Correct
codeActions() sending JSON null instead of []	Protocol fix	✅ Correct
lspToolDef struct → lspTool() factory	Refactor	✅ Clean
definition/implementations → locationRequest()	Refactor (DRY)	✅ Clean + bonus fix
initializeLocked() extraction	Refactor	✅ Cleaner SRP

---

Key Technical Findings

Fix 1 — format() Deadlock (Blocking bug):
format() held h.mu and then called NotifyFileChange, which also acquires h.mu → guaranteed deadlock on every format() call. Fixed correctly by introducing notifyFileChangeLocked(). Lock ordering (h.mu → h.openFilesMu) verified consistent throughout.

Fix 2 — applyWorkspaceEdit() stale state:
After a file rename, the LSP server was never notified of the content change. Fixed correctly using notifyFileChangeLocked() (caller already holds h.mu) gated on isFileOpen(). Thread-safety confirmed — no TOCTOU possible since stopLocked() also requires h.mu.

Fix 3 — codeActions() null diagnostics:
var []lspDiagnostic serializes as JSON null; changed to make([]lspDiagnostic, 0) → []. Correct per LSP spec.

---

🔴 Three Pre-Existing Issues Still Unaddressed

These were described in the PR title but are not present in the diff — they remain open for a follow-up PR:

1. Server can't restart after crash — h.cmd != nil check in ensureInitialized() can't detect a dead process
2. stopLocked() hangs forever — synchronous sendRequestLocked("shutdown") with no timeout
3. Data race on stderrBuf — bytes.Buffer used concurrently by os/exec and readNotifications

---

Test Coverage

❌ No tests added for any of the three bug fixes. Missing regression tests for:

• The format() deadlock
• didChange notifications after rename
• JSON [] vs null in codeActions() payload

---

Recommendation

✅ Approve the code — the actual changes fix real, significant bugs correctly.

Suggested before merge:

1. Fix the PR title and description to match what's actually in the diff (the commit message "fix: LSP format deadlock, null diagnostics, stale state after rename, and simplify code" is accurate)
2. Add regression tests for the three bug fixes (especially the format() deadlock)

Follow-up PR needed for the three pre-existing issues the title promised but this diff doesn't deliver.

[aheritier]

@dgageot using https://github.com/jdubois/jj-language-server for Java and https://www.npmjs.com/package/typescript-language-server for Typescript I am able to do complex settings with LSP configured for several languages in different agents

About the 3 issues reported in the previous comment ( Three Pre-Existing Issues Still Unaddressed) I am not sure they have real impacts if confirmed

However it could be great to add some tests to avoid regressions ...

📊 Complete LSP Tool Test Report

Test Setup

• Java LSP: jj-language-server (.java files)
• TypeScript LSP: typescript-language-server 5.5.4 (.ts files)
• Agents tested: Coordinator, spring-boot-engineer, angular-engineer, code-reviewer (4 agents)
• Test files: ScheduleResource.java, ScheduleService.java, home.component.ts, account.service.ts

---

Cross-Agent × Cross-Language Results Matrix

#	LSP Tool	Java (jj-language-server)	TypeScript (tsserver 5.5.4)
1	lsp_workspace	✅ All 13 capabilities listed	✅ All capabilities listed (Type Hierarchy = No)
2	lsp_hover	⚠️ Position-sensitive; works on fields, not always on type refs	✅ Returns type info correctly
3	lsp_definition	❌ No cross-file navigation — never leaves current file	⚠️ Goes to import binding first, not source (2-step)
4	lsp_references	✅ Works (but include_declaration=false ignored)	✅ Works correctly
5	lsp_document_symbols	⚠️ Only package + class (no methods/fields), garbled text	✅ Full hierarchical tree (9 symbols, nested)
6	lsp_workspace_symbols	⚠️ Finds fields, not class declarations cross-file	✅ Rich results (37+ matches for "AccountService")
7	lsp_diagnostics	✅ Reports unresolved types from wildcard imports	✅ Clean — no issues
8	lsp_code_actions	✅ 12 actions (refactorings, imports, generators)	❌ tsserver crash (assertDiagnosticLocation bug)
9	lsp_format	✅ Applied 1 change	✅ Applied 47 changes
10	lsp_rename	🔵 Skipped (available)	🔵 Skipped (available)
11	lsp_call_hierarchy (outgoing)	❌ No method body analysis	✅ 4 outgoing calls resolved with source files
12	lsp_call_hierarchy (incoming)	⚠️ Finds method but reports no callers	✅ 1 incoming call correctly identified
13	lsp_type_hierarchy (supertypes)	✅ Correct ("no supertypes")	❌ Not supported (-32601 Unhandled method)
14	lsp_type_hierarchy (subtypes)	✅ Correct ("no subtypes")	❌ Not supported
15	lsp_signature_help	❌ Advertised but never returns data	✅ Rich overload info with deprecation notices
16	lsp_implementations	❌ No cross-file resolution	✅ Returns concrete class
17	lsp_inlay_hints	❌ Returns nothing (advertised but non-functional)	✅ Works (empty when types are explicit, expected)

---

Score Summary

Status	Java	TypeScript
✅ Full pass	6	12
⚠️ Partial	5	2
❌ Fail / unsupported	5	2
🔵 Skipped	1	1

---

Agent Consistency

Results were consistent across all agents — the same tool at the same position returns the same result whether called from the coordinator, spring-boot-engineer, angular-engineer, or code-reviewer. No agent-specific differences detected.

---

Key Takeaways

Java (jj-language-server) — Limited

Reliable ✅	Unreliable ❌
lsp_workspace, lsp_references, lsp_diagnostics, lsp_code_actions, lsp_type_hierarchy, lsp_format	lsp_definition, lsp_implementations, lsp_call_hierarchy, lsp_signature_help, lsp_inlay_hints

Root cause: jj-language-server cannot resolve symbols across files. Any tool requiring cross-file navigation fails. Use search_files_content or grep as fallback.

TypeScript (tsserver) — Strong

Reliable ✅	Unreliable ❌
lsp_workspace, lsp_hover, lsp_references, lsp_document_symbols, lsp_workspace_symbols, lsp_diagnostics, lsp_format, lsp_call_hierarchy, lsp_signature_help, lsp_implementations, lsp_inlay_hints	lsp_type_hierarchy (unsupported), lsp_code_actions (crash bug in 5.5.4)

Root cause: lsp_type_hierarchy is unimplemented in typescript-language-server. lsp_code_actions crashes due to a ts-server 5.5.4 assertion bug in the refactoring path.

---

Recommendations

• For Java: Prefer lsp_hover + lsp_references + lsp_diagnostics + lsp_code_actions for in-file analysis. Use search_files_content for cross-file navigation.
• For TypeScript: Use the full LSP toolset freely (except type_hierarchy and code_actions). lsp_call_hierarchy is especially powerful for understanding component dependencies.
• For code-reviewer: Stick to read-only tools; the Java LSP still provides good diagnostics and code actions listing for review work.

[aheritier]

Stack trace analysis of the shutdown hang

I reproduced the hang while ending session 3862a2a7 with workspace /Users/arnaud/Workspace/Dev/aheritier/callforpapers. Here is the root cause from the SIGQUIT goroutine dump.

Blocked goroutine chain

Goroutine 1 (main) is stuck in stopLocked() at the h.cmd.Wait() call:

runOrExec (run.go:300)
  → stopToolSets (run.go:578)
    → Team.StopToolSets (team.go:128)
      → Agent.StopToolSets (agent.go:287)
        → StartableToolSet.Stop (startable.go:82)
          → LSPMultiplexer.Stop (lsp_multiplexer.go:66)
            → LSPTool.Stop (lsp.go:359)
              → lspHandler.stopLocked (lsp.go:537)
                → cmd.Wait → syscall.wait4   ← BLOCKED HERE

Why cmd.Wait never returns

In Go, exec.Cmd.Wait() does not return until both:

1. The child process exits
2. All internal pipe-copy goroutines finish (those spawned by cmd.Start() to read from StdoutPipe() and copy Stderr)

The h.cancel() call cancels the processCtx, which triggers exec.CommandContext to SIGKILL the LSP process. The process itself dies, but the stdout and stderr pipes remain open because child processes spawned by the LSP server inherited those file descriptors and are still running.

Evidence from the dump:

• Goroutine 778 — internal os/exec goroutine copying stdout pipe → bytes.Buffer.ReadFrom → os.(*File).Read → IO wait, 37 minutes
• Goroutine 781 — internal os/exec goroutine copying stderr pipe → bytes.Buffer.ReadFrom → os.(*File).Read → IO wait, 37 minutes
• Goroutine 779 — os/exec.(*Cmd).watchCtx → blocked on chan send (detected the kill but cannot complete handshake with Wait)

Since the LSP server (goroutine 782 watchCtx shows it was started via lsp.go:509) likely spawned background processes that inherited the pipe FDs, killing the parent doesn't close the write end of the pipes, so the read goroutines block forever, and Wait blocks forever.

Fix suggestion

Wrap cmd.Wait() with a timeout so the shutdown path cannot block indefinitely:

waitDone := make(chan error, 1)
go func() { waitDone <- h.cmd.Wait() }()

var err error
select {
case err = <-waitDone:
case <-time.After(5 * time.Second):
    slog.Warn("LSP server did not exit within timeout, abandoning wait",
        "command", h.command)
}

This matches what the PR description says is planned for fix 2. The current diff doesn't include this change yet.