feat: discovery-first tool resolution (make Mason optional)#21
Open
charliie-dev wants to merge 44 commits into
Open
feat: discovery-first tool resolution (make Mason optional)#21charliie-dev wants to merge 44 commits into
charliie-dev wants to merge 44 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements a discovery-first tool resolution model so that tools already available on $PATH (system-provided or Mason-provided) are used directly, while Mason becomes an optional installer backend rather than a hard runtime gate. This supports environments like NixOS/FreeBSD where Mason-installed binaries may be unavailable or undesirable.
Changes:
- Add shared helpers for
$PATHprobing, Masonspec.binextraction, and aggregated “missing tools” warnings. - Rework LSP and DAP setup to be driven by desired dependency lists with discovery-first resolution and deferred installs.
- Update Mason bootstrap for formatters/linters to only install when the tool isn’t already on
$PATH, aggregating missing-tool warnings.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| lua/modules/utils/tools.lua | New helper module for executable discovery + aggregated missing-tool warnings. |
| lua/modules/configs/tool/dap/init.lua | Switch DAP setup to discovery-first resolution instead of mason-nvim-dap’s installed-only handler gating. |
| lua/modules/configs/completion/servers/shuck.lua | Update server comment to reflect the new discovery-first resolution model. |
| lua/modules/configs/completion/mason.lua | Only install formatter/linter packages when not already available on $PATH; aggregate missing warnings. |
| lua/modules/configs/completion/mason-lspconfig.lua | Drive LSP setup via discovery-first resolution + deferred installs, using mason-lspconfig mappings. |
| lua/modules/configs/completion/lsp.lua | Remove external_lsp_deps wiring; rely on centralized discovery-first LSP setup. |
| lua/core/settings.lua | Collapse external/system LSP list into unified lsp_deps list with discovery-first semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
conform wrapped its whole resolve in vim.schedule and had to remember to pre-call ensure_mason_on_path itself — a resolver-internal pairing enforced only by a comment. resolve()/resolve_runtime_tools now accept a defer option that pays the same-tick $PATH guarantee synchronously and schedules the rest, with phase-1 configures still counting as trigger-backed exactly like the caller-side wrapper did
dap/init eagerly required mason-registry (loading all of mason.nvim) and four mason-nvim-dap mapping tables on the interactive command tick, even when every adapter self-validates from $PATH. mason-nvim-dap moves out of nvim-dap's dependencies into its own lazy spec, the registry becomes a thunk, and mappings/setup load on first use — only the factory fallback and phase-2 classification. the client opts table keeps its historical mapping fields through a lazy __index, so a user override reading opts.adapters still gets them on demand
phase 2 (the full mason mappings decode plus package hydration) ran synchronously on the BufReadPre lazy-load tick whenever any lsp dep was unresolved, adding 10-20ms to the session's first file. it now runs deferred: vim.lsp.enable() attaches already-open matching buffers (verified on this nvim), so the one same-tick beneficiary still reaches the triggering buffer
only successful resolution was cached, so every launch attempt while debugpy stayed missing re-ran the blocking python import spawns. a closure flag now short-circuits after one failed round; every recovery path already bypasses or resets it — the fast probes run first each call (a mason install takes over there), a resolver re-configure rebuilds the closure, and :ToolsRetry re-runs the client config for a pip-installed debugpy
package_for_binary re-read and re-decoded the whole registry.json on every call while the registry was not fully bootstrapped; the unfrozen index is now kept for a single event-loop tick (nothing can change within one), so a batch of lookups decodes once and the re-refresh self-heal is preserved. mason_root's guess path re-ran the user-override module search on every $PATH miss and recheck; that existence check is memoized (it only gates the default-dir guess), while the fs_stat recheck stays per call
refresh_linter read the rebuilt linter through lint.linters' __index, whose pcall swallows a reload failure into nil — the old linter was silently restored and configure reported success, clearing the hand-off pending gate over stale state. the reload now requires the module itself and raises the real reason on failure (the resolver keeps the name pending and the warning carries it); the synchronous reload inside configure is documented as deliberate — deferring it would report success before the rebuild happened
both overridden linters (selene, markdownlint-cli2) are plain-table modules upstream, so reapply_factory_override's non-function early-return always fired and the per-call wrapper plus apply_override's second parameter were ~30 lines of speculation. sync configures apply the override directly; late configures rely on refresh_linter, which re-applies it itself. if upstream ever converts one to a factory the override degrades to not-applied (documented) — the probe's factory branch still resolves the linter
the fast path hardcoded mason's packages/debugpy/venv layout — the same class of internal knowledge this branch already deleted from delve — while the next probe resolves the spec-declared debugpy-adapter shim that exists exactly when the venv does. the venv probe is gone (accepted nuance: windows spawns via the .cmd shim, no pythonw console suppression), the hint wording follows, and the import probe now uses vim.system():wait().code per repo convention instead of round-tripping through v:shell_error
autoformat_allowed re-tested format_on_save_enabled, but its only caller is installed exclusively when that setting is truthy — the condition could never be false there and pinned the gate in two places. the setting's gate now lives solely where the callback is installed, and the predicate says so
the discovery-first refactor removed the setting, but a user settings.lua still defining it merges cleanly into the table and feeds nothing — its servers vanish with no notification, since the aggregated warning only covers lsp_deps names. the merge tail now detects the removed key and schedules one warning that lists the map's keys and says to move the server names (not the executable values) into lsp_deps
the comment promised resolution when nvim-lint lazy-loads (first BufReadPost), but a by_ft-mapped linter only resolves on its filetype's first matching event — the resolve-only FileType autocmd or a lint event — so a tool like hadolint is neither installed nor warned about until a dockerfile buffer opens; only unmapped names get the immediate deferred pass
the contract list and the implementation both know the probe may
return { unresolved = true }, but the @param type still described the
pre-group-5 shape, misleading lua-ls-driven tooling
the official docs and the site config consistently publish https://gh-dash.dev/schema.json; the www form we pointed at is only the current hosting redirect target, so the documented url is the stable contract (today it 301s to www — the dependency direction still favors what the project documents)
the config-time negative probe latched for the whole closure lifetime, so a pip-installed debugpy (no mason shim) was never picked up by later launches. re-probe on a launch attempt after a 10s window, at most 3 rounds per miss-epoch, then the latch is permanent again (keeps the round-2 f19 bound of finite blocking spawns per session). any successful resolution resets the epoch in found(), so a later vanished resolved command re-probes with a fresh budget instead of latching on spent state.
… drift the factory-branch opts proxy only exposed `name` as a real key, so a user override enumerating opts (vim.tbl_deep_extend / pairs) silently lost the mason mapping fields the pre-branch plain table carried. user-authored overrides now get a materialized plain table (load_first_usable reports the winning module); repo clients keep the lazy proxy so a provisioned session still never loads mason on the :Dap tick. failed mason-nvim-dap mapping requires are now recorded and surfaced where they previously vanished: the resolver's reason-less missing branch (new missing_reason_of spec hook — a drifted mappings.source dead-ends there), the factory nil-adapter error (actionable client-config guidance; no drift claim for legitimate source-only names like js/elixir), and a one-shot warn when a materialized override field is nil. all three sites are gated on mason presence evidence (sibling mapping load, package.loaded, module_path) so a mason-less setup keeps its silent degradation.
the migration warning assumed a map residue: a half-migrated list surfaced its numeric indices as "KEYS (1, 2)", an empty table produced a warning with nothing actionable, and no shape ever said the dead key itself must go. classify the residue first — map keys, list entries, both (neither group may suppress the other, or a listed server is silently lost on migration), or nothing — and always end with the delete-the-key step.
the anchored lazy pattern ate the leading prose of any prefix-less level-0
raise whose message contains ":<digits>: " — error("parse failed at 12:30:
bad token", 0) reached the aggregated warning as just "bad token".
strip only shapes that are actual chunknames ([string ...], [C]:N:, a path
ending in .lua), first match wins; a custom non-lua chunk keeps its prefix,
which is more information, never less.
resolve_runtime_tools hardcoded conform's "resolves per buffer" phrasing in its unresolvable_of while nvim-lint shares the helper; the probe result now carries an optional reason and the shared code falls back to a neutral "config could not be verified at startup". conform supplies its own wording on the probe return — user-visible text is unchanged.
the probe's guard returned {binary=nil} when conform drops its @Private
get_formatter_config, silently classifying every formatter as self-resolving
— no install, no warning, provisioning off wholesale.
report unresolved with a reason naming the missing api instead: every
formatter_deps entry lands in the missing bucket via the phase-1
unresolvable_of short-circuit, loud and actionable.
…t it
expand_braces was single-group: a multi-group v2 claim like
{traefik,traefik-static}.{yml,yaml} slipped the prune and both schemas
claimed traefik.yml silently.
the claim check now judges leaves during a bounded expansion (cap 64, no
cartesian blow-up on the startup path; a claimed leaf short-circuits even
when the product exceeds the cap), and a glob still beyond the matcher that
mentions a claimed stem is dropped rather than trusted — with a scheduled
warn listing the dropped globs so over-pruning stays reviewable. stem-less
unjudgeable globs stay kept and silent.
phase 1 walked every lsp_deps entry through server_info on the first BufReadPre tick, loading all ~18 server modules regardless of the opened filetype — including jsonls/yamlls whose bodies deepcopy the 1362-entry schemastore catalog twice. resolve_deps now partitions the raw dep list: invalid entries pass through verbatim (the resolver's re-scan keeps reporting them); names with either user hook (a user.configs.lsp-servers module or recorded user.configs.lsp ops) and the repo modules that override filetypes (eager set, exported for the consistency harness) keep today's load-tick semantics; the rest defer to per-filetype buckets from vim.lsp.config[name].filetypes, resolved on that filetype's first buffer via a self-cleaning FileType autocmd plus an already-loaded-buffer scan. a 120s one-shot sweep (exported as resolve_remaining) classifies whatever never opened, keeping the missing-tool warning and install parity once per session. opening a lua file now loads only lua_ls's module; jsonls/yamlls stay unloaded until a json/yaml buffer appears.
package_for_binary froze its bin->package index once the registry was bootstrapped, with no invalidation: a mid-session registry update adding a package left the reverse lookup answering stale nils until restart. attach_registry_events now also subscribes update:success and clears both index tiers synchronously — mason emits the event before update callbacks run, so a scheduled clear would leave a same-tick stale window; clearing two locals is pure lua and fast-event-safe. the next lookup rebuilds and re-freezes against the fresh specs.
lspconfig_to_package froze its first non-empty get_mappings() snapshot; a mapping appearing later (registry update, partial-bootstrap edge) stayed invisible for the session. setup now subscribes update:success (pcall guarded, synchronous pure-lua clear) so package_of refetches on the next consult.
four verifier-confirmed equivalences, no behavior change: - the collector's pending counter moved in lockstep with the unsettled set on every path; the flush gate now reads next(unsettled) and the dead nil-name branches are gone (track is only ever called with a string name) - phase-1 validate outcomes collapse from two parallel maps plus a false-vs-string sentinel into one validates[name] record whose existence is the failure mark - split_dep_names(valid, invalid) is now the one definition of a valid dep entry; run() and nvim-lint consume the invalid list instead of inversely re-scanning raw deps (normalize_names stays a single-return wrapper, so the public alias's arity is unchanged) - resolve() asserts spec.configure is a function (every call site passes one) and the timeout fallback is a named constant cross-referenced from the settings doc comment
tools.load_first_usable had exactly one caller: dap's memoized load_client_config. the two-candidate loop now lives at that call site on top of the public load_module_or_report, preserving the contract exactly (first success wins, a higher-precedence broken candidate's reason survives alongside a lower-precedence success, any_exists accumulates); the export and its docblock are gone.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
lua/modules/configs/completion/nvim-lint.lua:152
refresh_linter()re-requireslint.linters.<name>directly but never writes the returned module back intolint.linters[name]. In nvim-lint, the cachingrawset(lint.linters, name, linter)happens in thelint.lintersmetatable__index, which this bypasses—so a successfulrequire()can still leavelint.linters[name]nil and break latertry_lintcalls.
rawset(lint.linters, name, nil)
-- Require it OURSELVES: the __index loader pcall-swallows a reload
-- failure into nil, which would read as configure success and clear
-- the hand-off pending gate over a stale linter.
local ok, err = pcall(require, module)
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
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.
Feasibility POC — RFC ayamir/nvimdots#1293
Implements the "separate tool discovery from tool installation" idea from this comment: Mason becomes one installer backend, not a hard requirement. On FreeBSD/NixOS/etc. — where Mason's prebuilt binaries don't run — system-provided tools are used directly, with no manual symlinking and no startup notification spam.
No new toggle, no OS detection. Behaviour is driven purely by availability.
Resolution model (per tool)
Applied to LSP, DAP, formatters, and linters through one shared loop (
modules/utils/tools.lua). Everything resolvable without an install registers synchronously (lazy-load trigger replay still works); only installs defer.Changes
modules/utils/tools.lua(new)$PATH+ Mason-bin-dir probe, install-then-configure tracking, bin→package reverse lookup, per-subsystem warning aggregator (one message, not N) with a configurable deadline (settings.tool_install_timeout)mason-lspconfig.lua,lsp.lualsp_deps;external_lsp_depsmerged intolsp_deps; function-cmdservers (e.g.jsonls) probed via the Mason package's declared bins; off-$PATHcmds rewritten to absolute paths beforevim.lsp.enable()(covers MasonPATH = "skip")conform.lua,nvim-lint.lua,mason.luamason.lua'sensure_installedloop removed (UI-only now)dap/init.lua,dap/clients/*dap_deps; mason-nvim-dap's private mappings guarded; client configs self-validate and resolve binaries lazily on the spawn path, so remote attach works without local binariessettings.lualsp_deps: absorbsnil_ls,nixd, andshuck— one list, every entry is wanted. Package-less servers (nixd,shuck) are enabled from their$PATHbinary; when absent they join the aggregated warning (the fix is provisioning via home-manager/mise, not Mason)external_lsp_deps: removedformatter_deps/linter_deps: entries are now the subsystem's tool names, matchinglsp_depssemantics (cmakelang→cmake_format,golangci-lint→golangcilint); the Mason package name is derived, not configuredtool_install_timeout(new): deadline before the aggregated warning is flushed despite unsettled installsVerification
stylua+selene+nix flake check, green.lua_lsvia the new resolver, no errors.Known POC limitations
$PATHdetection leans on the Mason package's declared binaries (nvim-dap has no uniform command registry); adapters without a mason-nvim-dap mapping need a client config.