fix: [#1776] prevent phantom projects from dbt_packages/ during cold-start race#1884
fix: [#1776] prevent phantom projects from dbt_packages/ during cold-start race#1884ralphstodomingo wants to merge 1 commit intomasterfrom
Conversation
…d-start race
`DBTWorkspaceFolder.createConfigWatcher` fires `onDidCreate` for every
`dbt_project.yml` that appears inside the workspace, and before this fix the
only gate against registering packages as projects was a dynamic check
against the `packages-install-path` of already-registered projects. During
cold start the watcher is active immediately, but `getPackageInstallPath()`
is only populated once the root project's `refreshProjectConfig` has run
its Python bridge round-trip — typically 2-5 seconds, longer on slow
adapters. In that window, `notInDBtPackages` was called with an empty
`packagesInstallPaths` list and trivially returned `true`, so every file
that `dbt deps` wrote into `dbt_packages/` got registered as a phantom
`DBTCoreProjectIntegration` with `projectName = "unknown_" + uuid`.
Each phantom spawns its own Python bridge, tries to initialize an adapter
against a non-existent profile, and either hangs or fails silently. The
status bar's `event.projects.length > 1` branch then trips and shows
"Parsing projects" plural forever.
Fix adds a static path-segment gate to the watcher filter: any path
containing `dbt_packages`, `dbt_internal_packages`, or `site-packages` as
a segment is rejected regardless of the dynamic packages-install-path
list. The dynamic check remains for custom `packages-install-path`
overrides. The gate is extracted into a free function
`isDBtProjectFileOutsidePackageDir` so it can be unit-tested directly.
- 16 new Jest tests in `src/test/suite/dbtWorkspaceFolder.test.ts`,
including a characterization of the pre-fix behavior (inlined copy of
the old filter) and the post-fix behavior for the same inputs.
- Full Jest suite still green (271 passed, 1 pre-existing skipped).
- Docker E2E before/after verified against `origin/master` `9a0ba3ad` vs
this branch with the same fixture and the same timed `dbt_project.yml`
spew into `dbt_packages/`:
- master: 5 phantom `unknown_<uuid>` projects registered under
`dbt_packages/phantom_pkg_1..5`, 6 simultaneous `inProgress=true`
rebuild events, status bar in "Parsing projects" plural.
- fix: 0 phantom projects registered, 1 `inProgress=true` event,
status bar correctly says "Parsing jaffle_shop".
- Happy-path regression check on the fix branch with a clean project
(no `packages.yml`): root project parses normally, name transitions
from `unknown_<uuid>` to `jaffle_shop`, NodeParser runs, rebuild
completes.
Closes #1776.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Raw log evidenceSaved from the two containers ( Pre-fix (
|
Bundle Size Reportdarwin-arm64: 70.1 MB
linux-x64: 71.8 MB
win32-x64: 72.7 MB
|
| async discoverProjects() { | ||
| // Ignore dbt_packages and venv/site-packages/dbt project folders | ||
| const excludePattern = | ||
| "**/{dbt_packages,site-packages,dbt_internal_packages}"; |
There was a problem hiding this comment.
Isnt this supposed to take care of this?
Summary
Fixes #1776 (extension stuck in "Parsing projects" state).
The
DBTWorkspaceFolderfile-system watcher registers phantomDBTCoreProjectIntegrationinstances for everydbt_project.ymlthatdbt depswrites intodbt_packages/during the cold-start window, before the root project'srefreshProjectConfighas had a chance to populatepackagesInstallPath. Each phantom spawns a Python bridge, tries to initialize an adapter against a non-existent profile, and either hangs or fails silently, leaving the status bar permanently showing"Parsing projects"with multipleunknown_<uuid>entries in the Diagnostics dump. This PR adds a race-independent static path-segment gate to the watcher filter so package directories are rejected immediately regardless of registration state.Root cause
In
src/dbt_client/dbtWorkspaceFolder.ts, the constructor creates aFileSystemWatcheron**/${DBT_PROJECT_FILE}at the moment the workspace folder is registered, beforediscoverProjects()is even called. TheonDidCreatehandler filters out package paths vianotInDBtPackages(uri, this.dbtProjects.map(p => p.getPackageInstallPath())).On the pre-fix code path,
getPackageInstallPath()returnsundefineduntil the root project has completed its asynchronousrefreshProjectConfig→createPythonDbtProject→project.init_project()chain. That chain round-trips through the Python bridge and typically takes 2-5 seconds even on Linux + DuckDB, longer on Windows + Snowflake (reporter environments). During that window, the dynamic check trivially returnedtruebecause the list was empty:Meanwhile, the extension itself auto-runs
dbt deps --project-dir ... --profiles-dir ... --target devas part ofrebuildManifest— visible in any cold-start log as"Installing dbt dependencies before first manifest rebuild". On a fresh clone with apackages.ymland no pre-existingdbt_packages/, that call creates onedbt_project.ymlper package, and the watcher'sonDidCreatefires on each. Because the filter has no idea about the staticdbt_packages/convention, every one of those package files is accepted andregisterDBTProject()is called on its parent directory. Each resultingDBTCoreProjectIntegrationstarts withprojectName = "unknown_" + crypto.randomUUID()(seealtimate-dbt-integration/src/dbtCoreIntegration.ts:194), spawns its own Python bridge, attempts to resolve a profile (there is none — it's a package dir, not a project), and wedges.The status bar in
src/statusbar/versionStatusBar.ts:70-76shows"$(sync~spin) Parsing projects"(plural) exactly whenevent.projects.length > 1, andevent.projectsis filtered byinProgress=true. So the reporter's screenshot of the plural label is hard evidence that 2+DBTProjectinstances were simultaneously inrebuildManifest. Under the race described above, that's exactly what happens: root project + N phantom package projects.Commenter tphillip33's Diagnostics dump in #1776 shows the expected downstream symptoms:
The
unknown_<uuid>name, thePython bridge is connectedline, and the entirely unpopulated target/packages/manifest/catalog paths match the state of aDBTCoreProjectIntegrationwhoserefreshProjectConfighas not completed. Commenter dnabb independently observed the literal phenomenon: "from time to time the extension still seems to parse the dbt_project.yml files from extensions installed inside dbt_packages and dbt_internal_packages instead of the dbt_project.yml from the main file."Fix
notInDBtPackagesnow delegates to a new exported free functionisDBtProjectFileOutsidePackageDirthat combines two checks:Static path-segment gate (new). Splits the path on both separators and rejects the URI if any segment equals
dbt_packages,dbt_internal_packages, orsite-packages. This matches the same directory-name list thatdiscoverProjects()already passes toworkspace.findFiles()as an exclude pattern, so both the async discovery path and the watcher path now reject the same set of package directories. The check is race-independent — it runs regardless of whether any project has finishedrefreshProjectConfig.Dynamic packages-install-path gate (preserved). Still iterates the registered projects'
getPackageInstallPath()values to catch custom package directories configured via thepackages-install-pathkey indbt_project.yml, which aren't in the static list. This only kicks in after at least one project has finished initializing, but that's fine because custom-path users also get the race-independent static gate for the common name.The static gate uses segment matching, not prefix matching, to avoid false positives on directories that merely start with
dbt_packages_backupetc. Both Windows (\) and POSIX (/) separators are handled.Extracting to a free function is purely for testability — it has no dependency on
this,vscode, or anything injected through DI, so the unit tests don't need to mock the class.Tests
16 new Jest tests in
src/test/suite/dbtWorkspaceFolder.test.ts, including a characterization test that inlines the verbatim pre-fix filter and asserts it returnedtruefor a package path during the race window:Full Jest suite: 271 passed, 1 pre-existing skipped.
Docker E2E replication
Reproduction and verification were done by building both
origin/master9a0ba3adand this branch, deploying each into an isolated code-server container against the same fixture, and running the same file-creation sequence during the cold-start race window. The fixture and the spew harness are identical between the two runs — only the extension dist differs.Fixture
/tmp/fx-1776: a copy oftest-fixtures/jaffle-shop-duckdb/with an emptydbt_packages/directory and apackages.ymlthat referencesdbt-labs/dbt_utils@1.1.1. This simulates a fresh clone wheredbt depshas not yet been run — the real-world cold-start condition reported in the issue.Reproduction harness
While the extension is activating (via Playwright against
http://localhost:$PORT/?folder=/home/coder/project), a backgrounddocker execloop creates 12 fake package directories insidedbt_packages/, each with a validdbt_project.ymland a noop macro, spaced 0.3 s apart. This is a direct substitute for what the extension's own auto-dbt depsrun does — just with deterministic content so the log output can be correlated precisely by package name:Result on
origin/master9a0ba3ad(pre-fix)Container:
dbtpu-1776m. Log file:exthost1/innoverio.vscode-dbt-power-user/Log - dbt.log.DBTCoreProjectIntegration:Registering dbt core projectentries:DbtProject:Created core dbt project unknown_<uuid>entries:rebuildManifestStatusChange inProgress=trueevents: 6 total, occurring within a ~1.6 s window (02:01:31.227through02:01:32.863). Withevent.projects.length > 1for that entire duration, the status bar inversionStatusBar.ts:70-76shows"$(sync~spin) Parsing projects"plural — the exact phrase from Vgrunert's screenshot.Only 5 phantoms landed instead of 12 because the later files (
phantom_pkg_6onward) were created after the root project'srefreshProjectConfighad begun populatingpackagesInstallPath, at which point the dynamic gate started catching them. This is consistent with the "from time to time" intermittency dnabb described: the race window has a variable edge that depends on exactly whendbt depsfinishes writing each package file.Result on this branch (post-fix)
Container:
dbtpu-1776f. Same fixture. Same reproduction harness. Same 12 phantom packages created inside the same timing window.DBTCoreProjectIntegration:Registering dbt core projectentries:DbtProject:Created core dbt project unknown_<uuid>entries:rebuildManifestStatusChange inProgress=trueevents: exactly 1. The status bar seesevent.projects.length === 1and shows"Parsing jaffle_shop"singular (which then clears normally).Zero phantom registrations. Zero phantom
unknown_<uuid>projects. The 12 phantom files still exist on disk, the watcher still firesonDidCreatefor each of them, but the new static path-segment gate rejects them all before they reachregisterDBTProject.Extra: happy-path regression check on the fix branch
Same container,
packages.ymlremoved,dbt_packages/cleaned up, extension restarted. Root project with no packages at all:The transient
unknown_<uuid>placeholder correctly transitions to the real project namejaffle_shop, NodeParser runs, rebuild completes, status bar clears. The fix does not regress the happy path.Extra-extra: phantom spew on a clean root on the fix branch
Same container, clean root reloaded, then the same 12-phantom spew replayed. Result:
Root project parses normally. Zero phantoms despite 12 phantom files created during the race window. One clean
inProgresstransition. This test is the strictest version because it also proves the root project's refresh path still works while the static gate is filtering the phantom spew in parallel.Test plan
dbtWorkspaceFolder.test.ts— 16 tests including pre-fix characterization + post-fix verification (npx jest --testPathPattern="dbtWorkspaceFolder.test")origin/master9a0ba3ad: 5 phantoms observed, plural status bar confirmedinProgresstransitionpackages.ymlto confirm the cold-start symptoms are resolved end-to-end in the reporter environmentRelated findings not in this PR
Two secondary issues were identified during the root-cause analysis and are not fixed here to keep the patch narrow. They are worth follow-up issues:
Unbounded Python bridge calls in
dbtCoreIntegration.ts:355-367.refreshProjectConfigawaitspython.ex\project.init_project()`with no timeout. python-bridge's Bluebird promises expose a.timeout(ms)method (python-bridge/index.d.ts:54-58) that no caller uses. Any Python-side hang — slow adapter init, DNS timeout, SSO flow — wedges the whole project indefinitely and thetry/catchindbtIntegrationAdapter.ts:285` can't fire because no error is thrown. Even with Extension 'stuck' in Parsing projects state #1776 fixed, a real root project whose adapter is genuinely slow will still look "stuck" to users.Silent error swallowing in
dbtIntegrationAdapter.ts:285-329.refreshProjectConfigcatches errors but only adds user-visible diagnostics forYAMLErrorandPythonException. Other error types (ERR_IPC_DISCONNECTED, IPC timeouts, network errors) are logged at debug level and the function returns silently, leaving the project stuck in theunknown_<uuid>placeholder state with no surfaced diagnostic. After this PR the phantom-project amplifier is gone, but a single genuine root failure still has poor visibility.Both findings belong in
altimate-dbt-integration, not this repo.