Skip to content

Fix unbounded recipe-local file discovery - #538

Merged
deribaucourt merged 4 commits into
yoctoproject:stagingfrom
gcomneno:fix/issue-537-recipe-local-search
Aug 6, 2026
Merged

Fix unbounded recipe-local file discovery#538
deribaucourt merged 4 commits into
yoctoproject:stagingfrom
gcomneno:fix/issue-537-recipe-local-search

Conversation

@gcomneno

@gcomneno gcomneno commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix unbounded recipe-local discovery that could recursively scan a configured build directory when editing documents such as build.conf.

Fixes #537.

Root cause

Recipe-local discovery was triggered eagerly from document changes and used separate traversal paths for files and directories.

The client also attempted recipe-local discovery for .conf documents even though recipe-local links are not applicable there.

Changes

  • add a standalone non-regression commit covering the existing recipe-local document-link behavior;
  • skip recipe-local discovery for .conf documents before requesting links or starting a scan;
  • centralize recipe-local roots and glob-pattern construction for both document links and the language client;
  • replace separate file and directory traversal with one asynchronous fast-glob.stream() scan;
  • return files and directories from the same object-mode stream using absolute paths;
  • limit traversal concurrency to 4 and avoid following symbolic links;
  • cancel active discovery through CancellationToken and stream.destroy();
  • avoid adding a separate timeout;
  • request recipe-local files lazily only while completing SRC_URI;
  • cache successful recipe-local discovery results;
  • preserve compatibility with clients that do not support the custom request.

Commit structure

  1. test: cover recipe-local document links
  2. fix: bound recipe-local file discovery
  3. optim: defer recipe-local discovery to completion

Validation

  • ESLint on the modified client and server files;
  • npm run compile;
  • targeted document-link provider suite: 4 tests passed;
  • targeted server completion suite: 27 tests passed;
  • 31 targeted tests passed in total.

@gcomneno
gcomneno force-pushed the fix/issue-537-recipe-local-search branch from 2f9be4b to ccf0d4d Compare July 31, 2026 14:21
@gcomneno
gcomneno marked this pull request as ready for review July 31, 2026 14:23

@deribaucourt deribaucourt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for rapidly taking a hold of this issue with lots of distributed changes!
I do have a few major remarks on code performance and maintainability. Please take a look.

Comment thread client/src/__tests__/unit-tests/document-link-provider.test.ts
Comment thread client/src/__tests__/unit-tests/document-link-provider.test.ts
Comment thread client/src/documentLinkProvider.ts Outdated
Comment thread client/src/documentLinkProvider.ts Outdated
Comment thread client/src/documentLinkProvider.ts Outdated
Comment thread client/src/language/languageClient.ts Outdated
Comment thread server/src/connectionHandlers/onCompletion.ts
@gcomneno
gcomneno force-pushed the fix/issue-537-recipe-local-search branch from ccf0d4d to 23014cb Compare August 3, 2026 11:12
@gcomneno

gcomneno commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

@deribaucourt I have addressed the review feedback and updated the PR history:

  • added the existing-behavior regression test in its own commit;
  • skipped recipe-local discovery for .conf documents;
  • replaced the separate traversals and timeout with one cancellable fast-glob.stream() scan;
  • centralized the recipe-local roots and pattern construction;
  • renamed the server-side commit to optim: defer recipe-local discovery to completion;
  • updated the PR description and resolved the seven review threads.

The targeted client and server suites pass (31 tests in total), along with ESLint and the full TypeScript compilation.

Could you please take another look when convenient?

@deribaucourt deribaucourt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Very minor remarks with the new implementation. It provides great results, thanks a lot!

Comment thread client/src/documentLinkProvider.ts Outdated
Comment thread client/src/documentLinkProvider.ts Outdated
Comment thread client/src/documentLinkProvider.ts
Comment thread client/src/documentLinkProvider.ts Outdated
Comment thread client/src/documentLinkProvider.ts Outdated
Comment thread client/src/documentLinkProvider.ts
@deribaucourt
deribaucourt merged commit 0d1d6ff into yoctoproject:staging Aug 6, 2026
5 checks passed
@gcomneno

gcomneno commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

@deribaucourt Thank you for the follow-up review, for testing the responsiveness of the new search, and for merging the contribution.

All five minor remarks were addressed in 461bb3fa:

  • corrected the method indentation and spacing;
  • removed the redundant cancellation check;
  • errors are now always logged;
  • stream cleanup is guaranteed from finally;
  • stream destruction is centralized and idempotent.

CI run #1633 passed completely, including lint, compilation, the full test project, the standalone language-server package, and the VSIX build.

I also performed a final focused review of the resulting implementation. I did not identify any further change that should have been folded into this PR. A possible additional test for a non-cancellation stream error remains a future, non-blocking improvement so the merged change stays focused.

@gcomneno
gcomneno deleted the fix/issue-537-recipe-local-search branch August 6, 2026 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Editing build.conf recursively scans the configured build directory and can crash VS Code with renderer OOM

2 participants