Make lsp resolve files external to a workspace

[mfelsche]

e.g. files from stdlib. Previously pony-lsp only checked if a file was within the workspace folder, now it checks first if it is within a workspace folder, if not it looks into all the modules ponyc picked up for all workspaces until it finds one.

This solves #4932

[ponylang-main]

Hi @mfelsche,

The changelog - fixed label was added to this pull request; all PRs with a changelog label need to have release notes included as part of the PR. If you haven't added release notes already, please do.

Release notes are added by creating a uniquely named file in the .release-notes directory. We suggest you call the file 5238.md to match the number of this pull request.

The basic format of the release notes (using markdown) should be:

## Title

End user description of changes, why it's important,
problems it solves etc.

If a breaking change, make sure to include 1 or more
examples what code would look like prior to this change
and how to update it to work after this change.

Thanks.

[orien]

These are review points offered by Claude:

C2. Zero test coverage for the core feature

• Location: tools/pony-lsp/test/ — all 195 tests use in-workspace files
• Evidence: The chain fallback path, has_module(), dispose() chain management, multi-workspace routing, and the end-of-chain error response are all untested. Counterfactual: if handle_request_chained silently dropped every request instead of forwarding, all 195 tests would still pass.
• Suggested fix: Tests needed for at minimum: (1) external file routed to correct workspace via chain, (2) has_module returning true for a compiled dependency, (3) end-of-chain error response, (4) dispose() chain integrity.

M4. Package.find_module() does full-file hashing on every chain hop

• Location: tools/lib/ponylang/pony_compiler/pony_compiler/package.pony:42-53 (called from state.pony:69-73)
• Evidence: _ModuleIter.next() constructs a new Module val including String.copy_cstring() (heap alloc) and @ponyint_hash_block (hashes entire source file contents). Called per-module, per-package, per-workspace, per-request for every external file lookup.
• Suggested fix: Cache file-to-workspace resolution in the router (M5 below). Fixing find_module() itself is in a different library (out of scope for this PR — file a separate issue).

Low

L1. var should be let

• Location: workspace_manager.pony:65, workspace_manager.pony:97
• Suggested fix: Change var file_path to let file_path in both handle_request_chained and handle_notification_chained.

L2. Redundant Uris.to_path() calls

• Location: workspace_router.pony:76, workspace_manager.pony:65, :97
• Evidence: Called once in find_workspace(), once per chained handler, once in the final handler. Each allocates.
• Suggested fix: Compute once and pass the resolved path through the chain.

L3. Same method names, different semantics on router vs. manager

• Location: workspace_router.pony and workspace_manager.pony
• Personas: API & Design
• Finding: handle_request_chained on the router does path-prefix dispatch; on the manager it does module-level chain traversal. Same name, different contract.

L4. New public behaviors on WorkspaceManager lack docstrings

• Location: workspace_manager.pony — handle_request_chained, handle_notification_chained, set_next_workspace, set_prev_workspace

L5. Pre-existing: ensure_open_document docstring incorrect

• Location: workspace_manager.pony — docstring says "returns a tuple together with a boolean" but function returns DocumentState
• Note: Pre-existing. Out of scope for this PR — file a separate issue.

L6. Chain traversal order is an implicit undocumented policy

• Location: Chain traversal always starts from head_workspace (first registered)
• Finding: When multiple workspaces share a dependency (e.g., same stdlib), the first-registered workspace always wins. This is undocumented policy — could produce surprising results if projects use different compiler flags.

[orien]

C1. Core feature does not work end-to-end for external files

The chain routes a request to a workspace via has_module() (checks compiled module data). But the dispatched handler (e.g., hover, goto_definition) calls _find_node_and_module → get_open_document, which returns None for external files unless did_open was previously processed by that workspace AND compilation completed. For the primary use case — user triggers goto-definition from workspace code into stdlib, editor opens the stdlib file, sends did_open then immediately sends hover — the hover returns nothing because compilation hasn't completed yet. Even if timing is perfect, the subsequent request returns null because open_documents is empty for that file. The chain routes correctly but the feature doesn't work.

Suggested fix: When a workspace claims a file via has_module, it needs a fallback path in the handlers that reads compiled module data directly, without requiring get_open_document. This is a design-level issue that likely needs a conversation with the author about the intended architecture.

[orien]

H1. dispose() corrupts chain when replacing a workspace for the same folder

dispose() only handles (let prev, let next) — middle nodes. Head-node (None, let next), tail-node (let prev, None), and single-node (None, None) are silently unhandled. Combined with add_workspace ordering: new manager is appended at tail before old is disposed. Concrete scenario: workspace A is head and tail; re-register same folder with A'. A' links after A. A is disposed. dispose() sees (None, A') — no match arm fires. head_workspace still points to disposed A. All chained requests now go through a dead actor first.

Suggested fix: Handle all four cases in dispose(). Also, add_workspace should insert the new manager at the old one's chain position rather than always appending to tail. After unlinking, set _next_workspace = None and _prev_workspace = None.

[orien]

H2. Async race in chain setup during startup

Location: workspace_router.pony:104-108, workspace_manager.pony:115-119

Evidence: set_next_workspace and set_prev_workspace are be (async). add_workspace queues last_mgr.set_next_workspace(mgr) but sets tail_workspace = mgr synchronously. If a request arrives before the queued message is processed, head manager's _next_workspace is still None. Chain terminates early; request returns "No workspace found" even though the new workspace has the file. Most likely during startup with multiple workspace folders.

Suggested fix: One option: router passes the next-workspace reference when it calls handle_request_chained, rather than relying on the actor's internal _next_workspace being up-to-date. This may require architectural discussion.

[orien]

H3. Silent request drop when head_workspace is None

try (head_workspace as WorkspaceManager)... end — no else clause. When find_workspace returns None and head_workspace is None, the cast fails and is swallowed. No ResponseMessage is sent back to the client, which hangs waiting indefinitely — a protocol violation. The notification analog at :62-65 silently drops with no log entry.

Suggested fix: Add an else branch sending an error response (e.g., ServerNotInitialized) for requests; add logging for the notification case.

[orien]

M1. did_open for external file triggers standalone compilation of the external package

Location: workspace_manager.pony — did_open handler

Evidence: When did_open chains to a workspace that owns the file, the handler calls _find_workspace_package, which fails for external files. The else branch creates a FilePath from Path.dir(document_path) and calls _compile(package) — triggering compilation of e.g. /usr/local/lib/pony/stdlib/builtin/ as a standalone package. This is wrong: the file is a dependency, and its compilation data already exists from the workspace's own build.

Suggested fix: Skip the compilation trigger for files claimed via the chain (not via folder prefix match).

[orien]

M2. Docstring copy-paste errors

Location: workspace_router.pony:21-33 (and the handle_notification_chained docstring)

Evidence: handle_request_chained docstring says "Handle the provided notification" and "handled by this WorkspaceManager" — both wrong. Both docstrings have typo "each on is checking" → "each one".

Suggested fix: Fix "notification" → "request", "WorkspaceManager" → "WorkspaceRouter", and the typo in both docstrings.

[orien]

M3. Repeated boilerplate in language_server.pony

Location: language_server.pony:82-366

Evidence: 11 nearly identical lambda blocks (~14 lines each) where only the method name differs. Adding a new handler requires copying 14 lines of scaffolding and updating the error message.

Suggested fix: Extract a _dispatch_request(r, handler) helper on LanguageServer. Could be deferred to a follow-up.

[orien]

M5. No caching of file-to-workspace resolution

Location: workspace_router.pony / workspace_manager.pony

Evidence: Every request for the same external file re-traverses the full chain and re-runs has_module() (with the hashing cost from M4).

Suggested fix: Add a Map[String, WorkspaceManager] cache in WorkspaceRouter, keyed by resolved file path. Invalidate on workspace add/remove/recompilation.

[SeanTAllen]

Suggested change

	## Pony-lsp: Resolve files external to workspace directories
	## Make lsp resolve files external to a workspace

[mfelsche]

I am about to address all the review comments. They just lead me down a rabbithole full of cans of worms. just wanted to let you know that this is not yet abandoned, but requires a little more rethinking and refactoring.