refactor(workspace): find the project of a file in the workspace by psteinroe · Pull Request #992 · supabase-community/postgres-language-server

psteinroe · 2026-10-26T14:41:58Z

Stacked on #991.

Groundwork for #936, where the review said "lsp should not know about symlinks". Move only, no behaviour change.

The LSP session picked the project of a document by matching its path against its own list of workspace folders, while the workspace already knows the folder of every project and resolves paths below it for the settings. Workspace::find_project(FindProjectParams { path }) now returns the project of the deepest folder that contains a path; a project without a folder contains every path. Session::project_key asks the workspace first and only decides what a file outside every folder belongs to (the only project with a user-chosen configuration file or without folders, otherwise the folderless project). The session keeps its folder list to unregister removed folders and to find the folder of a changed configuration file.

The method is part of the workspace protocol (pgls/find_project) like the other project methods, but not of the generated TypeScript bindings, which only list the methods the JS clients use.

Cost: one read lock and a pass over the projects per request, so O(workspace folders).

Tests

files_belong_to_the_project_of_the_deepest_folder: nested folders, a path outside every folder before and after a folderless project is registered, and after the inner folder is unregistered.
The existing LSP tests for workspace folders pass unchanged.

The LSP session matched a file against its own list of project folders to pick the project. The workspace already knows the folder of every project, so Workspace::find_project now returns the one of the deepest folder that contains a path, and the session only decides what a file outside every folder belongs to. It keeps its folder list to unregister removed folders. No behaviour change.

…ly extends transitively, replace on reload (#915) Stacked on #992 → #991. Three ways the root configuration didn't behave like the docs or like a nested configuration. **Relative paths resolved against the process's working directory (C7).** `update_settings` got `workspace_directory: fs.working_directory()` from both the LSP and the CLI, and `LoadedConfiguration::directory_path` was dropped. With the server started outside the workspace (Neovim from another cwd, the daemon, any workspace folder but the first) or `check` run from a subfolder, the root `migrations.migrationsDir`, `files.ignore`/`include` and linter globs pointed at the wrong folder, while nested configurations already used their own folder: ```jsonc // /repo/postgres-language-server.jsonc, server started in /home/alice { "migrations": { "migrationsDir": "migrations", "after": 5 } } // before: /repo/migrations/0009_later.sql is not a migration (looked in /home/alice/migrations) // after: it is, and migrations up to 0005 are ignored ``` Now both pass the folder of the configuration file (the project folder for the user configuration or no configuration). The CLI also uses that folder as the VCS base, like the LSP already did. **`extends` was one level deep (C10).** The pattern #881 documents, a nested file with `"extends": ["../../postgres-language-server.jsonc"]`, lost whatever the root itself extended (a shared file, the user configuration). Extended files' own `extends` are now applied first, resolved relative to the extended file. A cycle fails with `The configuration files extend each other: a -> b -> a`, and more than 10 levels fail too. **Reloading merged into the old root settings (C11, self-noted in #881).** Removing `"linter": { "rules": { "banDropColumn": "error" } }` from the file kept the `error` until a restart. `UpdateSettingsParams` gets `replace` (`#[serde(default)]`, so `false` for existing JSON clients). The LSP and the CLI replace: the settings are rebuilt from the defaults, and only swapped in if the configuration applies, so a broken file keeps the previous settings. The wasm `format` call keeps merging, since it only sends the formatter section. ## Changelog (behaviour changes) - Relative paths and globs in the root configuration are resolved against the folder of the configuration file, not the working directory. This affects `check`/`format` run from a subfolder of the configuration and editors whose server runs elsewhere. The CLI also looks for `.gitignore` from the configuration folder. - `extends` applies the `extends` of extended files; cycles and chains deeper than 10 are errors. - A section removed from the configuration file goes back to its default on reload. - `UpdateSettingsParams.replace` is new in the workspace protocol (optional, defaults to merging). ## Tests - `configuration::tests`: `extends_is_applied_transitively` (three levels, own `db.database` wins, inherited `db.host` kept, extended files listed), `extends_fails_on_a_cycle`, `extends_fails_when_too_deep`. - `pgls_workspace`: `updating_the_settings_replaces_or_merges_them`. - LSP: `relative_paths_of_the_root_configuration_are_relative_to_its_folder` (server cwd outside the workspace, root and nested `migrationsDir`), `a_section_removed_from_the_configuration_goes_back_to_its_defaults`. - CLI: `relative_paths_are_relative_to_the_configuration_file` (`check ../migrations/0009_later.sql` from `sub/`). - `just gen-lint` regenerated both `backend-jsonrpc` `workspace.ts` files. ## Not covered - `files.ignore` globs still aren't anchored to the configuration folder by the matcher (pre-existing, see #881). **Update:** reworked on top of #991 and #992 (base is now #992), which move configuring a project out of the LSP and the CLI into `pgls_workspace::configuration::configure_project`. - The fixes now live in one place instead of two: `configure_project` resolves relative paths and looks for the VCS ignore file from the folder of the configuration file, or the project folder without one, and replaces the root settings. `CliSession` no longer tracks a `configuration_directory`, and the LSP no longer computes the folder itself. A relative `--config-path` / `configurationPath` is made absolute against the working directory first (`check_stdin_applies_fixes`, added on main since, caught that). - `VcsIntegration::FromWorkingDirectory`, which #991 kept for the old CLI behaviour, is gone, and the CLI uses the shared `VcsIntegration`. - `extends` is applied with a stack instead of recursion (engineering rules from #933), with the limit named `EXTENDS_DEPTH_MAX`. New test `extends_applies_later_files_over_earlier_ones`. - With the root settings replaced on every load, the LSP no longer needs to remember which sections the client overrides set to reset them on clear (`SectionMask`), so the overrides are a plain optional layer. `clearing_client_overrides_restores_the_baseline` still covers it. - The test of `extends` transitively applied lives next to the loader in `pgls_workspace`, which both frontends use; nothing about it is in the LSP layer. **Update:** three follow-ups from review. - Relative path arguments of the CLI (`--migrations-dir`, `--vcs-root`) are made absolute against the working directory before they are laid over the configuration, so `check migrations/0001_init.sql --migrations-dir migrations` from a subfolder of the configuration finds `sub/migrations` again. Test: CLI `relative_path_arguments_are_relative_to_the_working_directory`. - An extended file resolves the packages in its own `extends` from its folder, like `require` in Node.js, so `@acme/pgls-config` can extend `@acme/pgls-base` from its own `node_modules`. The root file still resolves from the project folder. Test: `extended_files_resolve_packages_from_their_folder` (real `node_modules` in a temp folder). - `EXTENDS_DEPTH_MAX` counts files, as its doc says, so a chain allows 10 configuration files, not 11. Tests: `extends_applies_a_chain_as_deep_as_the_limit` (10 files) and `extends_fails_when_too_deep` (11 files).

Stacked on #915 → #992 → #991. With nested configurations (#881), one project can point at several databases, and one bad one stalled everything (C8): - `SnapshotManager::load` took the write lock of the whole snapshot map and kept it while `Snapshot::load` ran against the database. Until an unreachable database timed out (10 s by default), every snapshot lookup for every database waited, including cache hits, so hover, completions and diagnostics stalled in all projects of the language server. - `check` verified the databases one after the other and stopped at the first failure. Three unreachable databases with `connTimeoutSecs: 2` took **6.15 s** and reported one error, `Database error: pool timed out while waiting for an open connection`, without saying which database or which configuration. ## What changed - **Per-connection lock.** The map holds a slot per connection; the map lock is only taken to find or create the slot. The slot's mutex is held while that connection loads, so callers for the same database wait for one load instead of starting their own, and other databases aren't affected. A failed load leaves the slot empty for the next attempt, as before. - **Concurrent check.** `check_database_connection` collects the distinct connections of the root and nested settings (by host, port, user, database, connection string, password and timeout), checks them on scoped threads, and returns `errors: Vec<String>` instead of `error: Option<String>`, one per database it can't reach: ``` database/connection ━━━━━━━━━━━━━━━━━━━━━━━━━━━━ × Could not connect to postgres@10.255.255.1:5001/postgres, the database of /repo/db1: Database error: pool timed out while waiting for an open connection ``` The CLI turns each into its own `database/connection` diagnostic. The same three databases now take **2.07 s** (one timeout) and all three are reported. ## Tests - CLI `check_reports_every_unreachable_database_at_once` (Linux): three blackholed databases, each reported with its address and folder, in under 5 s. - The `check_fails_when_database_login_fails` snapshot now has the address prefix; the test output normalizes the address (it names the database `sqlx::test` creates) and the working directory. ## Not covered - The per-connection lock in the language server has no dedicated test; it needs a database that answers slowly. The CLI test covers the concurrent check. - The workspace protocol change (`errors` instead of `error`) has no TS counterpart; the type isn't exported to the JS bindings. **Update:** brought up to date with main and the reworked #915 (merge commits). Main had since added a check for an invalid `db.connectionString` to `check_database_connection`, which returned that one error; it now goes into `errors` like the connection failures, once per distinct message. The LSP's refresh of inlay hints and semantic tokens, also new on main, checks `errors.is_empty()`. No other changes.

…r discovery (#936) Stacked on #921 → #915 → #992 → #991. The path-matching edge cases from the release review (C19). Settings are picked by comparing a file's path with the workspace and nested folders as strings, which went wrong in three cases that are clearly bugs. **`..` picked the folder it leaves.** `/repo/db1/../db2/a.sql` starts with `/repo/db1`, so it got `db1`'s settings (and database). The CLI produces such paths from `check db1/../db2/a.sql`, and editors can too. Paths are now normalized lexically (`.`/`..` removed, without touching the disk) before the settings lookup in the workspace and before the LSP picks the project. `normalize_path` moved from `pgls_workspace::configuration` to `pgls_fs` for that. **Symlinked workspace folders.** With a folder opened as `/home/a/link` (a symlink to `/data/repo`) and a file opened by its real path, e.g. from a "go to definition" or a terminal link, the file matched no folder and got the defaults of a file outside every folder (since #902). The workspace now remembers the real path of each project folder and maps paths below it back onto the folder; the LSP does the same when picking the project. On Windows the `\\?\` prefix that `canonicalize` adds is stripped. **`.gitignore` with `vcs.enabled: false`.** The nested configuration search always honoured `.gitignore`, while the settings only use it with `vcs.enabled` and `vcs.useIgnoreFile` (and this repo's own config sets `enabled: false`). With VCS off, a config in a gitignored folder was silently not loaded, although the files there were checked. `FindFiles` gets `vcs_ignore_files`, which the search sets the way the settings would; `.ignore` files are always honoured. **Documented, not changed** (`docs/configuration.md`): - The reverse symlink case: a folder opened by its real path and a file opened through a symlink. That would need a `canonicalize` per file and request. - Case-insensitive file systems: `/Users/a/Repo` and `/Users/a/repo` are different folders to the server. Editors send the spelling of the workspace folder, so this only bites when paths are typed by hand. - A nested configuration inside the parent's `migrationsDir` replaces the `migrations` settings too (nothing is inherited), so its files aren't migrations unless it sets `migrations` itself or extends the parent. This follows from the no-inheritance design of #881. ## Tests - `pgls_workspace`: `files_below_the_real_path_of_a_symlinked_folder_use_its_settings` (Unix), and `..` cases in `files_use_the_nearest_configuration_file`. - LSP: `files_opened_by_their_real_path_belong_to_a_symlinked_folder` (Unix). - `pgls_fs`: `honours_gitignore_files_only_if_asked`. - CLI: `nested_configurations_ignore_gitignore_without_vcs`. ## Risks - With VCS off (the `init` default), discovery now walks gitignored folders such as `build/` or `dist/`. The entry limit from #895 still bounds it; ignoring them in `files.ignore` makes it faster. **Update:** reworked after "lsp should not know about symlinks", on top of #992, which moved the project lookup into the workspace (`Workspace::find_project`). - The LSP session no longer has `real_folders` or normalizes paths; it asks the workspace for the project of a file. The workspace matches a path, without `.` and `..`, against each project folder and its real path, and picks the deepest folder it is below. - The real path no longer comes from `std::fs::canonicalize` inside `pgls_workspace::settings`. `FileSystem::canonicalize` is new in `pgls_fs` (the OS implementation strips the Windows `\\?\` prefix; `MemoryFileSystem` resolves the symlinks added with `insert_symlink`, at most 40 like Linux), `configure_project` uses it, and passes the result as `RegisterProjectFolderParams::realPath` (optional, so existing clients are unaffected). `pgls_fs::real_path` is gone. - `files_below_the_real_path_of_a_symlinked_folder_use_its_settings` now runs on every platform with a `MemoryFileSystem` and checks `find_project` too; `canonicalize_resolves_symlinks_and_parent_folders` covers the memory implementation. The Unix LSP test is unchanged and passes through the workspace now.

Stacked on #936 → #921 → #915 → #992 → #991. With nested configurations every database a configuration points at gets a connection pool and a schema snapshot, and nothing ever dropped them. Editing a nested config to point at another database, renaming a folder, or removing a workspace folder left the old pool (with its open connections) and its snapshot in memory until the server stopped. The existing idle cleanup of pools only ran when a *new* pool was created, and a cache hit didn't count as a use, so a pool in constant use could be evicted while an idle one stayed. ## How big is a snapshot `crates/pgls_workspace/tests/snapshot_memory.rs` (ignored, needs the test DB) counts the bytes a loaded `Snapshot` keeps allocated, with a counting global allocator: | Database | Tables | Columns | Functions | Types | Snapshot | | --- | --- | --- | --- | --- | --- | | local `postgres` (mostly the system catalog) | 212 | 2,020 | 3,285 | 619 | 2.8 MiB | | + 500 tables × 10 columns, 200 functions, 50 views, 20 enums | 758 | 8,055 | 3,444 | 1,751 | 4.7 MiB | So each database costs about 3 MiB before it has any user objects, plus a few KiB per table. That's fine for a handful of databases, but it adds up when stale ones are never dropped. ## Caches and their bounds | Cache | Key | Grows with | Dropped when | | --- | --- | --- | --- | | `ConnectionManager.pools` | host, port, user, database | distinct databases in use | **new:** no project's root or nested settings use it (after `update_settings`, `update_nested_settings`, `unregister_project_folder`); unused for 5 min (**fixed:** hits count, the check runs at most once a minute on any use); settings fingerprint changes | | connections inside a pool | — | concurrent queries | **new:** sqlx closes a connection idle for 5 min (`idle_timeout`, was sqlx's 10 min default) | | `ConnectionManager.failures` | same | failing databases | success, settings change, **new:** unused | | `SnapshotManager.db_schemas` | same | distinct databases loaded, ~3–5 MiB each | **new:** unused (as above); **new:** not used for 30 min, loaded again on the next use; `invalidate_schema_cache` | | `catalog_base`, `snapshot_before` | — | one entry each | replaced on the next use (unchanged) | | session `root_files`, `configuration_files`, `real_folders` | workspace folder | folders | rebuilt on every reload | Evictions are logged at debug level; a dropped pool or snapshot is created again on the next use, so results don't change. `LastUsed` (an atomic timestamp) lets a cache hit under the read lock record a use. ## Tests - `connection_manager`: `unused_connections_are_dropped`, `idle_pools_are_closed` (incl. the once-a-minute check). - `snapshot_manager`: `unused_snapshots_are_dropped`, `idle_snapshots_are_dropped`. - `pgls_workspace`: `the_connection_of_a_removed_configuration_is_dropped`, against the test DB. A nested config that connects keeps one pool and one snapshot; moving it to another folder with the same database keeps them; removing it drops both. ## Not covered - The second pool per database from #907 is in another stack. When the two meet, its map needs the same `retain` call in `drop_unused_connections`. - During a reload, the root settings are applied before the nested ones. A database that moves from the root to a nested file in one edit is dropped and loaded again once. **Update:** brought up to date with main and the reworked stack below (merge commits). Main had since split each database into two pools, one for the language server's queries and one for executed statements (#907). Both live in the same cache entry, so dropping a database drops both; they are now also closed explicitly, so a clone that is still in use opens no new connections. `unused_connections_are_dropped`, `idle_pools_are_closed` and `the_connection_of_a_removed_configuration_is_dropped` check that both pools of a dropped database are closed.

psteinroe added this pull request to stack #1091 November 1, 2026 09:16

Base automatically changed from refactor/configure-project to main November 1, 2026 13:42

psteinroe force-pushed the refactor/find-project branch from 5fba4a4 to 9fc5414 Compare November 1, 2026 13:42

psteinroe merged commit 970c42e into main Nov 1, 2026
12 checks passed

psteinroe deleted the refactor/find-project branch November 1, 2026 14:00

Navigation Menu

Repository navigation

refactor(workspace): find the project of a file in the workspace - #992

refactor(workspace): find the project of a file in the workspace#992
psteinroe merged 1 commit into
mainfrom
refactor/find-project

psteinroe commented Oct 26, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

psteinroe commented Oct 26, 2026

Tests

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant