Skip to content

fix: make roles context initialization async to unblock extension host#2655

Draft
arikon wants to merge 3 commits intoansible:mainfrom
arikon:fix/async-roles-context-init
Draft

fix: make roles context initialization async to unblock extension host#2655
arikon wants to merge 3 commits intoansible:mainfrom
arikon:fix/async-roles-context-init

Conversation

@arikon
Copy link
Copy Markdown

@arikon arikon commented Mar 16, 2026

Summary

Fixes #2607

The LightSpeedManager constructor synchronously scans the entire workspace via glob.sync("**/roles", ...) during activate(), blocking the extension host for 50+ seconds on large monorepos.

This PR converts the entire call chain to async:

  • getCustomRolePathsglob.syncawait glob() with maxDepth: 6 limit
  • updateRolesContext / updateRoleContext / readVarFilesfs.*Syncfs.promises.*
  • watchRolesDirectory — async, returns Disposable[] for proper cleanup
  • LightSpeedManager.setContext — async, fire-and-forget from constructor via void this.setContext()
  • data.ts:getRoleYamlFiles — fix forEach(async ...) antipattern → for...of

Bug fixes included

  • watchers.ts:onDidDeletefs.statSync on already-deleted path → ENOENT crash
  • watchers.ts:onDidDeletedirPath in StandardRolePaths always false (in checks array indices, not values) → .includes()
  • watchers.ts — captured ansibleRolesCache reference becomes stale after resetContext() → use lightSpeedManager.ansibleRolesCache directly
  • watchers.ts — watcher leak on reInitialize() → proper dispose via _watchers array
  • Race condition guard via AbortController in setContext()

@arikon arikon requested review from a team as code owners March 16, 2026 17:04
Copilot AI review requested due to automatic review settings March 16, 2026 17:04
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 16, 2026

Codecov Report

❌ Patch coverage is 0% with 82 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/features/lightspeed/base.ts 0.00% 26 Missing ⚠️
...rc/features/lightspeed/utils/updateRolesContext.ts 0.00% 22 Missing ⚠️
src/features/lightspeed/utils/watchers.ts 0.00% 19 Missing ⚠️
src/features/lightspeed/utils/data.ts 0.00% 10 Missing ⚠️
src/features/lightspeed/utils/readVarFiles.ts 0.00% 2 Missing ⚠️
src/features/utils/ansible.ts 0.00% 2 Missing ⚠️
src/extension.ts 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@arikon
Copy link
Copy Markdown
Author

arikon commented Mar 16, 2026

@ssbarnea Could you have a look, please?

@goneri
Copy link
Copy Markdown
Collaborator

goneri commented Mar 20, 2026

Thanks @arikon. This is super cool.

arikon and others added 2 commits March 20, 2026 17:27
Replace synchronous glob.sync and fs.*Sync calls in LightSpeedManager
startup path with async equivalents. This prevents extension host freeze
on large monorepos where recursive workspace scan could take 5+ seconds.

Changes:
- Convert getCustomRolePaths to use async glob() with maxDepth limit
- Convert updateRolesContext/updateRoleContext/readVarFiles to async fs
- Fix watchRolesDirectory: return disposables, use async updateRolesContext
- Fix onDidDelete: replace broken `in` on array with `.includes()`,
  remove fs.statSync on deleted path, use live ansibleRolesCache reference
- Fix data.ts: replace forEach(async) antipattern with for...of
- Add AbortController guard for race conditions in setContext
- Add watcher cleanup on resetContext/reInitialize

Fixes ansible#2607

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Move async setContext() call from constructor to end of activate()
- Extract watchRolePaths() to reduce cognitive complexity (20 → ~12)
- Invert negated condition for clarity

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@goneri
Copy link
Copy Markdown
Collaborator

goneri commented Mar 20, 2026

note: The failing UI tests pass just fine locally.

@goneri
Copy link
Copy Markdown
Collaborator

goneri commented Mar 20, 2026

This PR will be blocked because of the "Commits must have verified signatures." rule.

@arikon could you set-up a SSH key to sign your commit and rebase your commit?

@goneri goneri enabled auto-merge (squash) March 20, 2026 22:37
@ssbarnea
Copy link
Copy Markdown
Member

@arikon The only issue that I see is what the quality gate reported coverage of new code under 80%

@ssbarnea ssbarnea marked this pull request as draft March 23, 2026 16:55
auto-merge was automatically disabled March 23, 2026 16:55

Pull request was converted to draft

@ssbarnea
Copy link
Copy Markdown
Member

Moved to draft, bring it back from draft only after all CI jobs report green.

@ssbarnea
Copy link
Copy Markdown
Member

ssbarnea commented Mar 31, 2026

@arikon Any chance you can resolve conflicts on this one please? Rebase and squash and we should be ready to merge it.

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

Labels

Projects

Status: Review

Development

Successfully merging this pull request may close these issues.

Ansible extension freezes other extensions loading because of sync fs traversal in Exntension Host

4 participants