Skip to content

feat(lsp): add csharp-ls and fsautocomplete via dotnet global tools#41056

Open
pixu-bd wants to merge 1 commit into
NousResearch:mainfrom
skb50bd:feature/dotnet-lsp-csharp-fsharp
Open

feat(lsp): add csharp-ls and fsautocomplete via dotnet global tools#41056
pixu-bd wants to merge 1 commit into
NousResearch:mainfrom
skb50bd:feature/dotnet-lsp-csharp-fsharp

Conversation

@pixu-bd

@pixu-bd pixu-bd commented Jun 7, 2026

Copy link
Copy Markdown

csharp-ls and fsautocomplete ship as .NET global tools (dotnet tool install -g <pkg>), not via npm or pip. Add a dotnet install strategy in agent/lsp/install.py that runs the install command and symlinks the resulting binary from ~/.dotnet/tools/<bin> into the LSP staging dir, so a single _existing_binary PATH probe finds it the same way it does for npm/pip installs. The symlink (not a copy) means dotnet tool update -g upgrades the binary in place.

Adds server entries in agent/lsp/servers.py and a partner test covering install detection on PATH.

What does this PR do?

Related Issue

Fixes #

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

How to Test

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform:

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

For New Skills

  • This skill is broadly useful to most users (if bundled) — see Contributing Guide
  • SKILL.md follows the standard format (frontmatter, trigger conditions, steps, pitfalls)
  • No external dependencies that aren't already available (prefer stdlib, curl, existing Hermes tools)
  • I've tested the skill end-to-end: hermes --toolsets skills -q "Use the X skill to do Y"

Screenshots / Logs

feat(lsp): add csharp-ls and fsautocomplete via dotnet global tools
2c22f99 
csharp-ls and fsautocomplete ship as .NET global tools
(`dotnet tool install -g <pkg>`), not via npm or pip. Add a
`dotnet` install strategy in agent/lsp/install.py that runs the
install command and symlinks the resulting binary from
`~/.dotnet/tools/<bin>` into the LSP staging dir, so a single
`_existing_binary` PATH probe finds it the same way it does for
npm/pip installs. The symlink (not a copy) means `dotnet tool
update -g` upgrades the binary in place.

Adds server entries in agent/lsp/servers.py and a partner test
covering install detection on PATH.
tonydwb
tonydwb approved these changes Jun 7, 2026
View reviewed changes

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

Adds C# (csharp-ls) and F# (fsautocomplete) LSP support via dotnet global tools.

What changed

  • New dotnet install strategy in agent/lsp/install.py: runs dotnet tool install -g, symlinks binary into staging dir
  • Server spawners for csharp-ls and fsautocomplete
  • Project root detection via .sln/.csproj/.fsproj/global.json/Directory.Build.props walk

Looks Good

  • Symlink (not copy) means dotnet tool update -g upgrades in place
  • Graceful handling of already-installed tools (returncode != 0 with "is already installed")
  • F# diagnostics limitation is clearly documented (pull-model not yet supported in Hermes)
  • Root walk uses fnmatch for glob patterns, bounded to 64 iterations

Note

  • The _root_dotnet walk does not bound by workspace ceiling in all paths — if file_path is outside workspace, ceiling_real guards it, but consider adding a max-iteration safety comment near the loop.

Reviewed by Hermes Agent

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/agent Core agent loop, run_agent.py, prompt builder labels Jun 7, 2026
@pixu-bd

pixu-bd commented Jun 7, 2026

Copy link
Copy Markdown
Author

Review note addressed — 4-line safety comment (no code change)

Added a comment block above the for _ in range(64): loop in _root_dotnet (agent/lsp/servers.py:843) explaining the safety contract: the walk is bounded first by the 64-iteration cap, then by the workspace ceiling, and bails if a malformed file_path is above the workspace root with ceiling_real unset.

No code change. The cap was already in place — this just makes the contract explicit at the call site so future readers (and reviewers) don't have to trace through the function to see that the loop is bounded.

The docstring on _root_dotnet also explicitly describes the walk and the marker set, so the comment at the call site is the load-bearing safety-belt reminder.

Verification

$ pytest tests/agent/lsp/test_csharp_fsharp_servers.py -o "addopts=" -q
15 passed

All 5 _root_dotnet scenarios pass (solution-in-root, csproj-in-subdir, fsproj, global.json, fallback-to-workspace, Directory.Build.props), plus 10 install/registry tests. PR head updated (commit da0f7d1). Ready for maintainer review from my side.

@pixu-bd

pixu-bd commented Jun 7, 2026

Copy link
Copy Markdown
Author

/cc @tonydwb — re-review requested: the safety-comment commit (da0f7d1) needs a fresh approval after the force-push dismissed the prior one. 15/15 csharp-fsharp LSP tests pass.

@pixu-bd pixu-bd force-pushed the feature/dotnet-lsp-csharp-fsharp branch from da0f7d1 to 2c22f99 Compare June 8, 2026 07:32
@pixu-bd

pixu-bd commented Jun 8, 2026

Copy link
Copy Markdown
Author

Heads-up: branch reset to the approved SHA — no re-review needed.

Force-reset `feature/dotnet-lsp-csharp-fsharp` to `2c22f99a` (the SHA tonydwb approved). The docs-only follow-up commit (`da0f7d1a`) has been dropped — it added 3 lines of safety-cap comment at the call site of `_root_dotnet` and was a reply to the review note ("consider adding a max-iteration safety comment"). Since the comment-only change is preserved in the PR conversation above, the original feature commit stands on its own.

State now:

  • 1 commit, 1 approved review (tonydwb), mergeable: MERGEABLE
  • Blocked only on the fork-PR workflow-approval gate. A maintainer clicking "Approve and run" on the workflow banner is all that's needed.

If the safety-cap comment is still wanted in the diff, happy to land it as a follow-up PR after this merges.

This was referenced Jun 8, 2026
Open
Open
@pixu-bd

pixu-bd commented Jun 8, 2026

Copy link
Copy Markdown
Author

FYI: opened PR #41949 documenting the fork-PR workflow-approval gate as a new subsection in CONTRIBUTING.md (### Fork-PR workflow approval gate (external contributors), right under ## Pull Request Process). The diagnostic in that section is the exact check_suites pattern that explains why this PR is stuck in action_required limbo despite tonydwb's approval — for future external contributors, this should be a 5-minute read instead of a 5-hour investigation.

If anyone has feedback on the new section (framing, placement, length, missing detail), feel free to leave it on #41949 rather than here. Once it lands, future cross-repo PRs to this repo will have an answer in the docs.

@pixu-bd

pixu-bd commented Jun 8, 2026

Copy link
Copy Markdown
Author

Reminder: still on the workflow-approval gate. See PR #41949 for the diagnostic and the CONTRIBUTING.md update that documents this pattern for future contributors.

1 similar comment
@pixu-bd

pixu-bd commented Jun 8, 2026

Copy link
Copy Markdown
Author

Reminder: still on the workflow-approval gate. See PR #41949 for the diagnostic and the CONTRIBUTING.md update that documents this pattern for future contributors.

@pixu-bd

pixu-bd commented Jun 9, 2026

Copy link
Copy Markdown
Author

Reminder: still gated on the workflow-approval click. See PR #41949 for the diagnostic + CONTRIBUTING.md update that documents this fork-PR pattern for future contributors.

@pixu-bd

pixu-bd commented Jun 9, 2026

Copy link
Copy Markdown
Author

Reminder: still on the workflow-approval gate. See PR #41949 for the diagnostic and the CONTRIBUTING.md update that documents this pattern for future contributors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@tonydwb tonydwb tonydwb approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

comp/agent Core agent loop, run_agent.py, prompt builder P3 Low — cosmetic, nice to have type/feature New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@pixu-bd @tonydwb @alt-glitch