Skip to content

feat: use cached ALCops analyzers immediately and refresh from NuGet in the background - #29

Merged
Arthurvdv merged 5 commits into
mainfrom
feat/provisioner-cache-first-startup
Sep 20, 2026
Merged

Arthurvdv merged 5 commits into
mainfrom
feat/provisioner-cache-first-startup

Conversation

@Arthurvdv

@Arthurvdv Arthurvdv commented Sep 20, 2026 •

Copy link
Copy Markdown
Member

Summary

  • When a valid cached ALCops analyzer version exists, Ready completes immediately from cache and almcp launches without waiting on NuGet. A background task checks NuGet for a newer version; if found, it is downloaded and cached for the next start.
  • On a cold cache (first run) or with a pinned version, the behaviour is unchanged: fetch from NuGet, then cache.
  • WorkspaceStartupResolver.GetConfigAsync() is now memoized via Lazy<Task<WorkspaceStartupConfig>>, so the settings.json walk, analyzer resolution and PE metadata reads happen once instead of on every call.

What changed

Cache-first provisioning (AlcopsAnalyzerProvisioner)

  • ProvisionCoreAsync checks FindNewestCachedVersion before any network call for Latest/Prerelease modes.
  • New FetchAsync / FetchOrFallbackAsync / RefreshCacheAsync split the old monolithic resolve-download block into composable pieces.
  • RefreshCacheAsync never throws; it catches cancellation and exceptions, logging a warning on failure.
  • ProvisionAsync awaits BackgroundRefresh after setting Ready, so AlcopsAnalyzerProvisionerStartup.StopAsync drains both provisioning and the background refresh.
  • FindNewestCachedVersion accepts includePrerelease: Latest skips cached prereleases; Prerelease uses the highest version of any kind. FallbackToCacheOrWarn keeps includePrerelease: true (a cached prerelease with a warning beats nothing when offline).

Carry-overs from review

  • AcquireInUseLock documents the FileShare.None cross-process lock assumption (flock on Unix).
  • SweepStaleTempDirectories lock probe now catches UnauthorizedAccessException alongside IOException.
  • SweepStaleTempDirectories uses BcToolsLocator.SafeEnumerateDirectories (now internal) instead of duplicating the safe-enumerate pattern.
  • SweepStaleTempDirectories has three-level error isolation: outer (guards cache root enumeration), per-TFM-folder (guards *.tmp-* enumeration within each TFM folder), and per-directory (guards probe/delete). A failure in one TFM folder never aborts the sweep for any other.
  • FallbackToCacheOrWarn has a doc comment explaining the deliberate prerelease inclusion; when the fallback is a prerelease under Latest mode, it logs a specific "no stable version cached" warning instead of the generic "using cached version" line.

Config memoization (WorkspaceStartupResolver)

  • _fullConfig is a Lazy<Task<WorkspaceStartupConfig>> initialized in the constructor.
  • GetConfigAsync() returns the memoized task; BuildAlMcpArgs uses the completed result or throws if the provisioner is still running.
  • With _provisioner == null (the public constructor tests use) the task completes synchronously — existing tests are unaffected.

Behaviour changes

  • A freshly published ALCops.Analyzers version is first used on the second start after it ships (visible through the Information log line). First start after publication downloads it in the background.
  • Persistent offline runs log one Warning per start (background refresh failure).
  • The sync BuildAlMcpArgs now throws InvalidOperationException when a provisioner is attached and not yet ready; no production caller does that (AlMcpProxy.StartCoreAsync uses BuildAlMcpArgsAsync).

Test plan

  • dotnet build --configuration Release — 0 warnings, 0 errors
  • dotnet test --configuration Release — 159 tests pass (11 new, 1 extended, 1 renamed)
  • New: Provision_WarmCache_ReadyFromCache_RefreshDownloadsNewerForNextStart — verifies Ready completes from cache while background refresh is in-flight, then the newer version is cached
  • New: Provision_WarmCache_Latest_UsesNewestCachedStable_NotPrerelease — Latest mode skips cached prereleases
  • New: Provision_WarmCache_Prerelease_UsesNewestCachedAny — Prerelease mode uses highest cached version of any kind
  • New: Provision_WarmCache_RefreshFailure_NeverThrows — background refresh failure is swallowed with a warning
  • New: Provision_WarmCache_ShutdownCancelsRefresh — cancelling the token stops the background refresh
  • New: Provision_Pinned_Cached_ZeroRequests — pinned version in cache makes zero HTTP requests
  • Extended: Provision_ExtractsCorrectTfm_And_WritesManifest — asserts BackgroundRefresh.IsCompleted on cold-cache path
  • New: GetConfigAsync_IsMemoized — config resolution is not repeated after settings.json changes
  • New: Sweep_EnumerationFailure_DoesNotFailProvisioning — locked stale dir survives sweep; a second TFM folder's stale dir is still cleaned; on Unix, unreadable TFM folder exercises the per-folder catch
  • New: Fallback_Latest_OnlyPrereleaseCached_UsesItWithExplicitWarning — prerelease-only cache under Latest logs "no stable version cached"
  • Renamed: Provision_Latest_StableAndPrereleaseCached_FastPathUsesStable_NoLastResortWarning (was Fallback_…PrefersStable) — verifies the cache-first fast path, not fallback; asserts zero NuGet requests
  • New: Fallback_Prerelease_PicksHighestCachedAcrossStableAndPrerelease — Pinned mode with unresolvable version falls back to highest cached (1.3.0-preview.1 > 1.2.0 in SemVer 2); logs "using cached version"
  • All existing provisioner and startup resolver tests remain green
  • [AlMcpFact] tests (almcp-backed) remain green

🤖 Generated with Claude Code

Arthurvdv and others added 5 commits September 20, 2026 18:13
…Get in the background

When a valid cached version exists for the target TFM, Ready completes
from it without any network I/O. A background task then checks NuGet for
a newer version; if found, it is downloaded and cached for the next
start. On a cold cache (first run) or with a pinned version, the
behaviour is unchanged: fetch-then-cache.

Other changes in this commit:
- FindNewestCachedVersion accepts includePrerelease: Latest mode skips
  cached prereleases; Prerelease mode uses the highest cached version of
  any kind. FallbackToCacheOrWarn keeps includePrerelease: true so a
  cached prerelease with a warning beats nothing when offline.
- SweepStaleTempDirectories uses BcToolsLocator.SafeEnumerateDirectories
  (now internal) instead of duplicating the safe-enumerate pattern.
- The lock probe in SweepStaleTempDirectories catches
  UnauthorizedAccessException alongside IOException.
- AcquireInUseLock documents the FileShare.None cross-process lock
  assumption (flock on Unix, reliable on local FS, advisory on NFS).
- AlcopsAnalyzerProvisionerStartup documents that StopAsync drains the
  background refresh through ProvisionAsync.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
GetConfigAsync and BuildAlMcpArgs now return the same memoized
WorkspaceStartupConfig instance instead of re-walking settings.json,
re-resolving every analyzer spec and re-reading PE metadata on each
call. With _provisioner == null (the public constructor tests use) the
task completes synchronously, so existing tests are unaffected.

The sync BuildAlMcpArgs now throws InvalidOperationException when a
provisioner is attached and not yet ready; no production caller does
that (AlMcpProxy.StartCoreAsync uses BuildAlMcpArgsAsync).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Update README.md and AGENTS.md to reflect the cache-first provisioning
strategy: later starts use the newest cached version immediately and
check NuGet in the background; a freshly published version is first
used on the second start. Document the BackgroundRefresh task, the
.in-use lock assumption, TimeoutException fallback behaviour, and
SemVer 2 version ordering.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…release offline fallback

- Wrap SweepStaleTempDirectories body in outer try/catch so lazy
  enumeration failures (concurrent deletion, permission change, AV lock)
  cannot set Ready to null; the per-directory catch is kept so one bad
  directory does not abort the rest
- Add doc comment on FallbackToCacheOrWarn explaining the deliberate
  prerelease inclusion; log a specific "no stable version cached" warning
  when the fallback is a prerelease under Latest mode

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rerelease test accurately

- Restore per-TFM-folder try/catch in SweepStaleTempDirectories so a
  failure enumerating one folder (e.g. unreadable) does not abort the
  sweep for remaining folders. Three-level error isolation: method-level
  (cache root enumeration), per-TFM-folder (*.tmp-* enumeration), and
  per-directory (probe/delete). Extend the sweep test to verify a stale
  dir in a separate TFM folder is still cleaned when another folder
  fails.

- Rename Fallback_Latest_StableAndPrereleaseCached_PrefersStable to
  Provision_Latest_StableAndPrereleaseCached_FastPathUsesStable_NoLastResortWarning:
  the test exercises the cache-first fast path, not the FallbackToCacheOrWarn
  code path. Add Assert.Empty(handler.RequestUrls) to make the fast-path
  claim explicit.

- Add Fallback_Prerelease_PicksHighestCachedAcrossStableAndPrerelease:
  a real fallback-ordering test that reaches FallbackToCacheOrWarn via a
  Pinned mode with an unresolvable version, proving the last-resort
  behaviour picks the highest cached version (1.3.0-preview.1 > 1.2.0
  in SemVer 2) and logs a "using cached version" warning.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Arthurvdv
Arthurvdv merged commit 4b03679 into main Sep 20, 2026
5 checks passed
@Arthurvdv
Arthurvdv deleted the feat/provisioner-cache-first-startup branch September 20, 2026 17:49
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.

1 participant