Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ It is deliberately **thin**: Microsoft's `almcp` already compiles, runs diagnost
- `WorkspaceStartupResolver` — discovers AL projects (mirrors `almcp`'s own `DiscoverProjectPaths`: downward scan for `app.json`, depth 4, standard exclusions) and composes the child `almcp`'s `--projects` / `--codeanalyzers` / `--rulesetpath` / `--packagecachepath` args. `almcp` in MCP mode never reads `.vscode/settings.json` and has no per-call analyzer, ruleset or package-cache parameter, so this bridge at launch is the only thing keeping `al_compile` and our fix tools in agreement (`ProjectLoader` reads the same `al.packageCachePath` for the in-process compilation).
- `AlMcpProxy` — child process lifecycle plus generic tool forwarding over a single long-lived MCP client that reconnects on session expiry. `ForwardAsync` is a passthrough with **no per-tool argument rewriting**; configuration is conveyed at launch instead.
- `ProjectAnalyzerResolver` — reads `al.codeAnalyzers` and the ruleset (`.vscode/settings.json`, `.AL-Go/settings.json`, convention-named files) and builds an `AnalyzerSet`. Nothing is built in.
- `AlcopsAnalyzerProvisioner` — downloads ALCops' own analyzers from NuGet, matched to the installed DevTools TFM, and caches them under `~/.alcops/analyzers/`. `Task<string?> Ready` completes with the provisioned folder or `null`. Configured via `--alcops-analyzers` / `ALCOPS_ANALYZERS` / `ALCOPS_ANALYZERS_CACHE`.
- `AlcopsAnalyzerProvisioner` — cache-first: when a valid cached version exists, `Task<string?> Ready` completes immediately with it and a background task checks NuGet for a newer version (for the next start). `internal Task BackgroundRefresh` is that background task, drained by `ProvisionAsync` and cancelled by `AlcopsAnalyzerProvisionerStartup.StopAsync`. On a cold cache or with a pinned version, the provisioner fetches from NuGet before completing. Internal HTTP timeouts surface as `TimeoutException` and fall back to cache; only the caller's cancellation propagates. Version ordering is SemVer 2 via `SemanticVersion`. The `.in-use` lock (`FileShare.None`) is a cross-process lock via `flock` on Unix, reliable on local file systems and advisory on network mounts such as NFS home directories. Configured via `--alcops-analyzers` / `ALCOPS_ANALYZERS` / `ALCOPS_ANALYZERS_CACHE`.
- `ExternalAnalyzerLoader` — loads analyzer DLLs through `AnalyzerAssemblyLoadContext`, which resolves shared types by simple name from the default context. That type sharing is what makes `typeof(DiagnosticAnalyzer).IsAssignableFrom` work, and therefore what makes in-process code fixes possible at all.
- `ProjectSessionManager` / `ProjectLoader` — caches AL project workspaces keyed by path; `GetOrLoadProjectAsync` is the entry point tools use.
- **Models/** — record types for tool return values, serialized with `JsonDefaults.Options` (camelCase, not indented).
Expand All @@ -57,7 +57,7 @@ It is deliberately **thin**: Microsoft's `almcp` already compiles, runs diagnost

Shipping pinned cop DLLs beside whatever `Nav.CodeAnalysis` the user installed is what caused `AD0001` / `MissingMethodException` (issue #10). Microsoft cops and third-party analyzers come solely from the project's own config and the DevTools directory. `ALCops.Analyzers` is referenced by the **test project only**, so the fixtures have real cops with real code fixes to exercise; it must never move back to `src`.

ALCops' own analyzers are provisioned by `AlcopsAnalyzerProvisioner` at every startup: it detects the DevTools TFM, downloads the latest stable `ALCops.Analyzers` NuGet package (or uses a pinned/prerelease version per `--alcops-analyzers`), extracts the matching `lib/<tfm>/` folder, and caches the DLLs under `~/.alcops/analyzers/<tfm>/<version>/`. `ExternalAnalyzerLoader.ResolveDllPath` probes the provisioned folder first for `${analyzerFolder}ALCops.*.dll` specs. The DevTools themselves are never downloaded at runtime.
ALCops' own analyzers are provisioned by `AlcopsAnalyzerProvisioner` with a cache-first strategy: it detects the DevTools TFM; when a valid cached version exists, `Ready` completes immediately with it and a background task checks NuGet for a newer version (cached for the next start). On a cold cache (first run), it downloads the latest stable `ALCops.Analyzers` NuGet package (or the pinned/prerelease version per `--alcops-analyzers`), extracts the matching `lib/<tfm>/` folder, and caches the DLLs under `~/.alcops/analyzers/<tfm>/<version>/`. "Latest" means latest as of the previous run once a cache exists. `ExternalAnalyzerLoader.ResolveDllPath` probes the provisioned folder first for `${analyzerFolder}ALCops.*.dll` specs. The DevTools themselves are never downloaded at runtime.

When passing analyzers to the child `almcp`, their sibling dependencies must travel with them (`ALCops.Common.dll`, `Microsoft.Dynamics.Nav.Analyzers.Common.dll`): `almcp` resolves analyzer dependencies only among the paths it was given and does not probe the analyzer's directory. A missing one turns every rule in that assembly into an `AD0001` instead of a diagnostic.

Expand Down
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,20 +78,20 @@ This is deliberate: bundling pinned cop DLLs beside whatever `Nav.CodeAnalysis`

### ALCops analyzer provisioning

ALCops' own analyzers (`${analyzerFolder}ALCops.*.dll`) are provisioned automatically at every startup. The server detects the installed DevTools' target framework (e.g. `net10.0`), downloads the latest stable [ALCops.Analyzers](https://www.nuget.org/packages/ALCops.Analyzers) NuGet package, extracts the matching `lib/<tfm>/` folder, and caches the DLLs under `~/.alcops/analyzers/<tfm>/<version>/`. On subsequent starts a newer stable version is picked up automatically; older cached versions are left in place.
ALCops' own analyzers (`${analyzerFolder}ALCops.*.dll`) are provisioned automatically at every startup. The server detects the installed DevTools' target framework (e.g. `net10.0`), and on the first start downloads the latest stable [ALCops.Analyzers](https://www.nuget.org/packages/ALCops.Analyzers) NuGet package, extracts the matching `lib/<tfm>/` folder, and caches the DLLs under `~/.alcops/analyzers/<tfm>/<version>/`. On later starts the newest cached version is used immediately so `almcp` launches without waiting on NuGet; a NuGet check and any download run in the background and a newer version is used on the **next** start. Older cached versions are left in place.

Configure with `--alcops-analyzers` or the `ALCOPS_ANALYZERS` environment variable:

| Value | Behaviour |
|-------|-----------|
| `latest` (default) | Download the latest stable release. |
| `prerelease` | Download the highest version including prereleases. |
| `latest` (default) | Newest cached stable release; a newer one is fetched in the background for the next start (the first run downloads before starting). |
| `prerelease` | Highest version including prereleases from cache; a newer one is fetched in the background for the next start (the first run downloads before starting). |
| `<version>` (e.g. `1.2.0`) | Pin to a specific version (no index lookup). |
| `off` | Disable provisioning entirely. |

Set `ALCOPS_ANALYZERS_CACHE` to override the default cache directory (`~/.alcops/analyzers`).

When offline, the newest previously cached version for the target TFM is used with a warning. When no cache exists, the server starts without ALCops analyzers and logs a message with manual provisioning instructions.
When NuGet is unreachable or slow, the newest previously cached version for the target TFM is used with a warning. When no cache exists, the server starts without ALCops analyzers and logs a message with manual provisioning instructions.

The recommended `al.codeAnalyzers` configuration:

Expand Down
5 changes: 3 additions & 2 deletions src/ALCops.Mcp/McpHost.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@ public static async Task RunAsync(string[] args, BcToolsLocator toolsLocator, Pr
sp.GetRequiredService<RulesetLoader>(),
sp.GetRequiredService<AlcopsAnalyzerProvisioner>()));

// ALCops analyzer provisioner: downloads ALCops' own analyzers from NuGet, matched to the
// installed DevTools TFM. Runs under --no-proxy too — the native fix tools need them.
// ALCops analyzer provisioner: uses the newest cached ALCops analyzers immediately and
// refreshes from NuGet in the background, matched to the installed DevTools TFM.
// Runs under --no-proxy too — the native fix tools need them.
var analyzersOption = AlcopsAnalyzersOption.Parse(
proxyOptions.AlcopsAnalyzers
?? Environment.GetEnvironmentVariable("ALCOPS_ANALYZERS"));
Expand Down
123 changes: 95 additions & 28 deletions src/ALCops.Mcp/Services/AlcopsAnalyzerProvisioner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,18 @@ internal sealed class AlcopsAnalyzerProvisioner : IDisposable
private readonly ILogger<AlcopsAnalyzerProvisioner> _logger;
private readonly TaskCompletionSource<string?> _ready = new(TaskCreationOptions.RunContinuationsAsynchronously);
private FileStream? _inUseLock;
private Task _backgroundRefresh = Task.CompletedTask;

internal static readonly TimeSpan StaleTempDirectoryAge = TimeSpan.FromHours(1);

public Task<string?> Ready => _ready.Task;

/// <summary>
/// The NuGet check/download that runs after <see cref="Ready"/> completed from cache.
/// Never faults; <see cref="Task.CompletedTask"/> on cold-cache, pinned and off paths.
/// Awaited by <see cref="ProvisionAsync"/> so shutdown waits for it.
/// </summary>
internal Task BackgroundRefresh => _backgroundRefresh;
internal TimeSpan ResolveTimeout { get; init; } = TimeSpan.FromSeconds(10);
internal TimeSpan DownloadTimeout { get; init; } = TimeSpan.FromSeconds(60);

Expand All @@ -68,6 +76,8 @@ public void Dispose()
_httpClient.Dispose();
}

// FileShare.None is a cross-process lock via flock on Unix; reliable on local file systems,
// advisory on network mounts such as NFS home directories.
private static FileStream AcquireInUseLock(string dir) =>
new(Path.Combine(dir, InUseLockFileName), FileMode.OpenOrCreate,
FileAccess.ReadWrite, FileShare.None, bufferSize: 1, FileOptions.None);
Expand All @@ -86,6 +96,7 @@ public async Task ProvisionAsync(CancellationToken ct)
}

_ready.TrySetResult(result);
await _backgroundRefresh;
}

private async Task<string?> ProvisionCoreAsync(CancellationToken ct)
Expand All @@ -110,23 +121,29 @@ public async Task ProvisionAsync(CancellationToken ct)
_logger.LogInformation("DevTools target framework: {Tfm}", tfm);

SweepStaleTempDirectories();
ct.ThrowIfCancellationRequested();

string? version;
try
{
version = await ResolveVersionAsync(ct);
}
catch (Exception ex) when (ex is not OperationCanceledException)
if (_option.Mode is AlcopsAnalyzersMode.Latest or AlcopsAnalyzersMode.Prerelease)
{
_logger.LogWarning(ex, "Could not reach NuGet to resolve ALCops analyzer version");
return FallbackToCacheOrWarn(tfm);
var cached = FindNewestCachedVersion(tfm, includePrerelease: _option.Mode == AlcopsAnalyzersMode.Prerelease);
if (cached is not null)
{
_logger.LogInformation(
"ALCops analyzers: v{Version} ({Tfm}) from cache {Dir}; checking NuGet in the background",
Path.GetFileName(cached), tfm, cached);
_backgroundRefresh = RefreshCacheAsync(tfm, cached, ct);
return cached;
}
}

return await FetchOrFallbackAsync(tfm, ct);
}

private async Task<string?> FetchAsync(string tfm, CancellationToken ct)
{
var version = await ResolveVersionAsync(ct);
if (version is null)
{
_logger.LogWarning("No suitable ALCops analyzer version found on NuGet");
return FallbackToCacheOrWarn(tfm);
}
return null;

_logger.LogInformation("ALCops analyzers: resolved version {Version}", version);

Expand All @@ -137,14 +154,43 @@ public async Task ProvisionAsync(CancellationToken ct)
return cacheDir;
}

return await DownloadAndExtractAsync(version, tfm, ct);
}

private async Task<string?> FetchOrFallbackAsync(string tfm, CancellationToken ct)
{
try
{
return await DownloadAndExtractAsync(version, tfm, ct);
var result = await FetchAsync(tfm, ct);
if (result is not null)
return result;

_logger.LogWarning("No suitable ALCops analyzer version found on NuGet");
}
catch (Exception ex) when (ex is not OperationCanceledException)
{
_logger.LogWarning(ex, "Failed to download ALCops.Analyzers {Version}", version);
return FallbackToCacheOrWarn(tfm);
_logger.LogWarning(ex, "ALCops analyzers: NuGet provisioning failed");
}

return FallbackToCacheOrWarn(tfm);
}

private async Task RefreshCacheAsync(string tfm, string current, CancellationToken ct)
{
try
{
var result = await FetchAsync(tfm, ct);
if (result is not null && !string.Equals(result, current, StringComparison.OrdinalIgnoreCase))
_logger.LogInformation(
"ALCops analyzers: fetched v{Version} ({Tfm}); it will be used on next start",
Path.GetFileName(result), tfm);
}
catch (OperationCanceledException) when (ct.IsCancellationRequested) { }
catch (Exception ex)
{
_logger.LogWarning(ex,
"ALCops analyzers: background refresh failed; keeping v{Version}",
Path.GetFileName(current));
}
}

Expand Down Expand Up @@ -316,12 +362,26 @@ internal string ExtractPackage(string nupkgPath, string version, string tfm, str
}
}

/// <summary>
/// Falls back to the newest cached version, including prereleases: loading a prerelease
/// with a warning beats running without ALCops rules when NuGet is unreachable.
/// </summary>
private string? FallbackToCacheOrWarn(string tfm)
{
var cached = FindNewestCachedVersion(tfm);
if (cached is not null)
{
_logger.LogWarning("ALCops analyzers: using cached version from {Dir}", cached);
if (_option.Mode == AlcopsAnalyzersMode.Latest
&& SemanticVersion.TryParse(Path.GetFileName(cached), out var v) && !v.IsStable)
{
_logger.LogWarning(
"ALCops analyzers: NuGet unreachable and no stable version cached; using prerelease v{Version} from {Dir} as a last resort",
Path.GetFileName(cached), cached);
}
else
{
_logger.LogWarning("ALCops analyzers: using cached version from {Dir}", cached);
}
return cached;
}

Expand All @@ -333,7 +393,7 @@ internal string ExtractPackage(string nupkgPath, string version, string tfm, str
return null;
}

private string? FindNewestCachedVersion(string tfm)
private string? FindNewestCachedVersion(string tfm, bool includePrerelease = true)
{
var tfmDir = Path.Combine(_cacheRoot, tfm);
if (!Directory.Exists(tfmDir))
Expand All @@ -353,6 +413,8 @@ internal string ExtractPackage(string nupkgPath, string version, string tfm, str
continue;
if (!SemanticVersion.TryParse(name, out var v))
continue;
if (!includePrerelease && !v.IsStable)
continue;

if (bestVersion is null || v.CompareTo(bestVersion) > 0)
{
Expand All @@ -369,27 +431,32 @@ internal string ExtractPackage(string nupkgPath, string version, string tfm, str
return best;
}

// Three-level error isolation so a failure in one TFM folder or one temp
// directory never aborts the sweep for any other:
// 1. Outer (method-level): guards enumeration of the cache root itself.
// 2. Per-TFM-folder: guards enumeration of *.tmp-* within each TFM folder.
// 3. Per-directory: guards the probe/delete of each individual temp directory.
private void SweepStaleTempDirectories()
{
if (!Directory.Exists(_cacheRoot))
return;

var count = 0;
try
{
foreach (var tfmDir in Directory.EnumerateDirectories(_cacheRoot))
if (!Directory.Exists(_cacheRoot))
return;

var count = 0;
foreach (var tfmDir in BcToolsLocator.SafeEnumerateDirectories(_cacheRoot))
{
try
{
foreach (var dir in Directory.EnumerateDirectories(tfmDir, "*.tmp-*"))
foreach (var dir in BcToolsLocator.SafeEnumerateDirectories(tfmDir, "*.tmp-*"))
{
try
{
if (DateTime.UtcNow - Directory.GetLastWriteTimeUtc(dir) < StaleTempDirectoryAge)
continue;

try { using var probe = AcquireInUseLock(dir); }
catch (IOException)
catch (Exception ex) when (ex is IOException or UnauthorizedAccessException)
{
_logger.LogDebug("ALCops analyzers: skipping in-use extraction directory {Dir}", dir);
continue;
Expand All @@ -409,14 +476,14 @@ private void SweepStaleTempDirectories()
_logger.LogDebug(ex, "ALCops analyzers: could not enumerate extraction directories in {Dir}", tfmDir);
}
}

if (count > 0)
_logger.LogInformation("ALCops analyzers: removed {Count} stale extraction directories", count);
}
catch (Exception ex) when (ex is IOException or UnauthorizedAccessException)
{
_logger.LogDebug(ex, "ALCops analyzers: could not enumerate TFM directories in {Dir}", _cacheRoot);
_logger.LogDebug(ex, "ALCops analyzers: sweep of stale extraction directories aborted");
}

if (count > 0)
_logger.LogInformation("ALCops analyzers: removed {Count} stale extraction directories", count);
}

internal static bool IsCacheValid(string cacheDir)
Expand Down
5 changes: 5 additions & 0 deletions src/ALCops.Mcp/Services/AlcopsAnalyzerProvisionerStartup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@

namespace ALCops.Mcp.Services;

/// <summary>
/// Hosted service that runs <see cref="AlcopsAnalyzerProvisioner.ProvisionAsync"/> on a background task.
/// <see cref="StopAsync"/> cancels the token and awaits the startup task, which drains both
/// provisioning and the background NuGet refresh through <see cref="AlcopsAnalyzerProvisioner.ProvisionAsync"/>.
/// </summary>
internal sealed class AlcopsAnalyzerProvisionerStartup : IHostedService
{
private readonly AlcopsAnalyzerProvisioner _provisioner;
Expand Down
2 changes: 1 addition & 1 deletion src/ALCops.Mcp/Services/BcToolsLocator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ internal static IEnumerable<string> OrderByDescendingVersion(IEnumerable<string>
.OrderByDescending(x => x.Version, SemanticVersion.Comparer)
.Select(x => x.Path);

private static IEnumerable<string> SafeEnumerateDirectories(string root, string pattern)
internal static IEnumerable<string> SafeEnumerateDirectories(string root, string pattern = "*")
{
try
{
Expand Down
Loading
Loading