Improve ecosystem stats refresh resilience

[PrzemyslawKlys]

Summary

• add project-catalog telemetry sync to the ecosystem-stats pipeline task
• refresh known PowerShell Gallery modules by package ID when the owner-wide Gallery query times out
• cover the stats/catalog sync path with focused pipeline tests

Validation

• dotnet test .\PowerForge.Tests\PowerForge.Tests.csproj -c Release --no-restore --filter "FullyQualifiedNameWebPipelineRunnerEcosystemStatsTests|FullyQualifiedNameWebEcosystemStatsGeneratorTests"
• dotnet build .\PowerForge.Web.Cli\PowerForge.Web.Cli.csproj -c Release --no-restore
• git diff --check

[claude]

Code Review — PR #344: Improve ecosystem stats refresh resilience

Overview

This PR adds two improvements to the ecosystem-stats pipeline task:

1. Project-catalog telemetry sync — after stats are generated, telemetry from the stats file can be merged into catalog.json and published.
2. Per-module PSGallery fallback refresh — when the owner-wide Gallery query times out, each known module is fetched individually by package ID using the PSGallery OData v2 API.

The structure is clear, the new methods are well-scoped, and XML parsing is hardened against XXE. Test coverage is a genuine positive. A few concerns are worth addressing before merge.

---

Issues

1. refreshPowerShellGalleryByIdOnFallback is always hardcoded to true

In ExecuteEcosystemStats, the new parameter is passed unconditionally:

refreshPowerShellGalleryByIdOnFallback: true,

The parameter was added to TryPreserveEcosystemSources but the call site never passes false, so the parameter adds noise without adding flexibility. Either expose it as a pipeline-step config key (alongside syncProjectCatalogTelemetry) or remove the parameter and inline the behavior. The current state is the worst of both worlds.

---

2. Synchronous HTTP (GetAwaiter().GetResult()) in a per-module loop

using var response = client.GetAsync(url).GetAwaiter().GetResult();

GetAwaiter().GetResult() on a hot async task is a sync-over-async pattern that blocks the calling thread and can deadlock in contexts with a SynchronizationContext. More practically: requests are entirely sequential, so an org with 80 modules makes 80 serial HTTP roundtrips under a 10-second-per-call timeout — worst case 800 s. Consider Task.WhenAll with bounded parallelism (SemaphoreSlim) or just use .Result explicitly (same risk, but at least signals intent clearly). If the pipeline already runs on a threadpool thread with no sync context, document that assumption.

---

3. Bare catch silently discards all exceptions

try
{
    catalog = JsonSerializer.Deserialize<ProjectCatalogDocument>(...);
}
catch
{
    return 0;
}

A catch (JsonException) (and perhaps IOException) would make intent clear and let unexpected exceptions (e.g., OutOfMemoryException) propagate. Silent swallowing makes CI failures hard to diagnose.

---

4. URL construction mixes OData escaping and URI encoding

var escapedId = Uri.EscapeDataString(id.Replace("'", "''", StringComparison.Ordinal));
var url = $"...?$filter=Id%20eq%20'{escapedId}'%20and%20IsLatestVersion%20eq%20true";

The intent is correct (double-quote for OData, then percent-encode), but Uri.EscapeDataString will also encode the doubled apostrophes to %27%27, which the OData endpoint may not decode as a string escape sequence. In practice the PS Gallery module IDs are alphanumeric so this path is never hit, but a module ID with a special character could silently fall back to the existing data instead of refreshing. Worth a unit test or at least a comment explaining the encoding contract.

---

5. Test relies on network timeout to drive the fallback path

"githubOrg": "pf-test-org-{{Guid.NewGuid():N}}",
"timeoutSeconds": 1,

Using a random org name with a 1-second timeout is clever, but it makes the test depend on network latency. In an air-gapped CI environment or a very fast one that gets a quick 404, the behavior may differ. A small abstraction allowing the stats generator to be replaced with a stub would make this test fully deterministic.

---

Minor / Style

• module.Id! null-forgiving operator after an IsNullOrWhiteSpace guard is technically correct but the ! is surprising to readers. A local variable var id = module.Id; avoids it.
• SyncProjectCatalogTelemetryFromStats first param is string (non-nullable) but internally guarded with IsNullOrWhiteSpace — inconsistent with params 2 and 3 which are string?. Align the signatures.
• The projectCatalogTelemetry block in the summary output only appears when syncProjectCatalogTelemetry == true. If a consumer reads the summary and the key is absent, they need a null check. A merged: 0 with catalogPath: null might be less surprising than a missing key.

---

Positive Notes

• XML parsing correctly sets DtdProcessing.Prohibit and XmlResolver = null — good XXE protection.
• MergeModule is a clean value-level merge: refreshed wins unless it's empty/zero, falling back to existing. Easy to reason about.
• The warning messages added to generated.Warnings give good operational visibility into when the fallback path fires.
• The test covers the happy path for catalog telemetry sync end-to-end and the TryPreserveEcosystemSources arg update is correctly reflected.

---

Summary

The feature logic is sound. The blocking items are the hardcoded refreshPowerShellGalleryByIdOnFallback: true (either expose or remove the parameter), and the bare catch hiding deserialization errors. The sync-over-async loop is worth flagging for large module sets. Everything else is polish.

[claude]

Code Review — PR #344: Improve ecosystem stats refresh resilience

Overview

This PR adds two distinct features to the WebPipelineRunner.Tasks.EcosystemStats.cs pipeline task:

1. Per-package PSGallery refresh on fallback — when the owner-wide PSGallery query times out, known module IDs are used to re-fetch stats individually via the OData API.
2. Project catalog telemetry sync — after generating/preserving stats, optionally merges the updated stats into a project catalog.json and copies it to a publish path.

The test coverage covers both paths. The XML parsing hardening (DTD prohibition) and OData escaping are solid. A few concerns below.

---

Issues and Suggestions

🟡 Behavioral default change — refreshPowerShellGalleryByIdOnFallback defaults to true

var refreshPowerShellGalleryByIdOnFallback =
    GetBool(step, "refreshPowerShellGalleryByIdOnFallback") ??
    ...
    true;  // <-- opt-in behavior, but shipped as opt-out

This is a silent breaking change for existing pipeline configs. Previously, a fallback silently reused existing PSGallery data (instant). Now it silently makes up to N concurrent HTTP requests to powershellgallery.com. For users with many modules or strict network policies this could be surprising. Consider defaulting to false and requiring explicit opt-in, or at minimum documenting the change clearly.

---

🟡 HttpClient instantiated per call — potential socket exhaustion

using var client = new HttpClient
{
    Timeout = TimeSpan.FromSeconds(10)
};

For a CLI/pipeline tool with one run per invocation this is unlikely to cause socket exhaustion in practice, but it's the well-known HttpClient antipattern. If this method is ever called more frequently (e.g., parallel pipeline steps), TIME_WAIT socket exhaustion can occur. An IHttpClientFactory or a static/shared HttpClient field would be safer.

---

🟡 No cancellation propagation into HTTP refresh

The inner HTTP loop uses CancellationToken.None:

var refreshed = await TryFetchPowerShellGalleryModuleByIdAsync(client, id, CancellationToken.None)
    .ConfigureAwait(false);

The pipeline step already has a timeoutSeconds parameter, but that timeout isn't threaded into the per-ID refresh. With a semaphore of 8 and, say, 80 modules, the worst case is (80 / 8) * 10s = 100s of extra wall-clock time regardless of what timeoutSeconds says. A CancellationTokenSource derived from the pipeline's timeout budget would cap this.

---

🟡 Blocking async with .GetAwaiter().GetResult() in synchronous path

results = RefreshPowerShellGalleryModulesByIdAsync(client, existing.Modules).GetAwaiter().GetResult();

In a pure CLI context this is fine, but it's worth tracking: if this code ever runs on a thread pool thread (e.g., inside Task.Run) with a synchronization context, this can deadlock. A comment noting "called only from sync pipeline entry point" would make the intent clear.

---

🟡 Silent catalog deserialization failures — no user-visible signal

catch (JsonException) { return 0; }
catch (IOException)   { return 0; }
catch (UnauthorizedAccessException) { return 0; }

When the catalog file can't be read or parsed, the pipeline step reports projectCatalogTelemetry=0 in its message — indistinguishable from "sync was disabled" or "nothing matched." Adding the exception message to warnings (same pattern used elsewhere) would surface the failure.

---

🟠 Test expectation may be incorrect — PSGallery totalDownloads not updated

In RunPipeline_EcosystemStats_SyncsProjectCatalogTelemetry:

// catalog.json has: "totalDownloads": 1
// stats.json has:   "downloadCount": 123456

Assert.Equal(42, project.GetProperty("metrics").GetProperty("github").GetProperty("stars").GetInt32());
Assert.Equal(1L,  project.GetProperty("metrics").GetProperty("powerShellGallery").GetProperty("totalDownloads").GetInt64());

GitHub stars are expected to update (1 → 42) but PSGallery totalDownloads stays at 1, not 123456. Either MergeProjectTelemetry intentionally skips PSGallery download counts in fallback scenarios, or this assertion is wrong. A comment explaining why the download count isn't updated here would prevent future confusion.

---

🟢 Positives worth noting

• XXE prevention: DtdProcessing.Prohibit and XmlResolver = null correctly guard against XML External Entity attacks when parsing the OData feed.
• OData injection: Single-quote escaping (id.Replace("'", "''")) before Uri.EscapeDataString is correct and tested.
• Graceful per-module fallback: Failures for individual modules fall back to existing data rather than aborting the entire batch — good resilience design.
• $select optimization opportunity: The OData URL could add $select=Id,Version,DownloadCount,... to reduce response size, though this is a minor optimization.
• Test isolation: Temp directories with Guid.NewGuid() and try/finally cleanup — solid test hygiene.
• Partial-class reuse: Calling MergeProjectTelemetry from the stats task (defined in the catalog task partial class) avoids duplication cleanly.

---

Summary

The core logic is sound and the resilience goal is well-motivated. The two items most worth addressing before merge are:

1. Default opt-in behavior for refreshPowerShellGalleryByIdOnFallback — this changes the network behavior of all existing pipeline configs silently.
2. The totalDownloads assertion — clarify whether it's intentionally not synced or a bug in the test.

The other items are lower priority but worth tracking.

[claude]

Code Review — PR #344: Improve ecosystem stats refresh resilience

Overview

This PR adds two resilience features to the ecosystem-stats pipeline task:

1. Per-ID PowerShell Gallery fallback refresh — when the owner-wide Gallery query times out, known modules are individually fetched by package ID via the OData v2 API.
2. Project-catalog telemetry sync — after stats are generated, download counts and GitHub stars are merged back into catalog.json and optionally published.

Both features are opt-in (false by default), and the PR includes focused tests covering the pipeline-level path, the direct sync method, and OData URL escaping. The general structure is solid and follows existing patterns.

---

Issues & Suggestions

⚠️ HttpClient created per invocation (socket exhaustion risk)

// TryRefreshPowerShellGalleryModulesById
using var client = new HttpClient { Timeout = Timeout.InfiniteTimeSpan };

A new HttpClient is allocated on every call. Even with using, this can exhaust TCP sockets under load because sockets enter TIME_WAIT after disposal. The standard fix is a static readonly HttpClient (acceptable for a CLI/batch context) or IHttpClientFactory. Given this is a CLI pipeline runner, a static readonly client is the simplest safe path.

---

⚠️ .GetAwaiter().GetResult() sync-over-async

results = RefreshPowerShellGalleryModulesByIdAsync(client, existing.Modules, cancellation.Token)
    .GetAwaiter().GetResult();

The comment acknowledges this is a deliberate sync bridge. In a pure CLI/batch context without an active SynchronizationContext this is usually safe, but it is fragile if the call site ever moves into an async host (e.g., Blazor, ASP.NET). Worth a brief note in the AGENTS.md "Working Agreements" that this method must only be called from a thread-pool context.

---

ℹ️ Significant file growth — check line-count gate

The AGENTS.md calls out a 800-line file limit (node .\Build\linecount.js . 800). WebPipelineRunner.Tasks.EcosystemStats.cs gains ~192 lines in this PR. It would be worth running the line-count check locally to confirm the file stays under budget.

---

ℹ️ Test JSON fixtures are duplicated verbatim

SyncsProjectCatalogTelemetry and MergesPowerShellGalleryDownloads_WhenStatsIncludeGallery contain nearly identical 50-line JSON blobs for stats.json and catalog.json. A shared private helper (e.g., WriteDefaultStatsAndCatalog(root, ...)) would make both tests shorter and easier to update when the schema evolves.

---

✅ OData escaping is correct

var odataLiteral = id.Replace("'", "''", StringComparison.Ordinal);
var filter = $"Id eq '{odataLiteral}' and IsLatestVersion eq true";
return "https://www.powershellgallery.com/api/v2/Packages?$filter=" + Uri.EscapeDataString(filter);

Single-quote doubling before URL-encoding is the correct approach for OData v2 string literals. The dedicated unit test for this (BuildPowerShellGalleryPackageByIdUrl_EscapesODataLiteralBeforeUrlEncoding) confirms the behavior. Good.

---

✅ XML security hardening

var settings = new XmlReaderSettings
{
    DtdProcessing = DtdProcessing.Prohibit,
    XmlResolver = null,
    IgnoreComments = true
};

DtdProcessing.Prohibit + XmlResolver = null correctly prevents XXE attacks against the Gallery Atom feed. No issues here.

---

✅ Concurrency and timeout design

• SemaphoreSlim(8, 8) keeps per-ID requests bounded.
• Math.Clamp(timeoutSeconds, 5, 300) prevents extreme values.
• Setting HttpClient.Timeout = Timeout.InfiniteTimeSpan while driving cancellation via CancellationTokenSource is the right pattern — avoids the client's internal timeout racing with the token.

---

ℹ️ MergeProjectTelemetry not visible in diff

SyncProjectCatalogTelemetryFromStats calls MergeProjectTelemetry(catalog.Projects, statsPath, serializerOptions) but this method isn't in the diff. Presumably it exists already — worth confirming there are no unresolved changes hidden in a dependent branch.

---

ℹ️ projectCatalogPath always resolved even when sync is disabled

var projectCatalogPath = ResolvePath(baseDir, GetString(step, "projectCatalogPath") ?? ... ?? "./data/projects/catalog.json");

These paths are resolved unconditionally even when syncProjectCatalogTelemetry = false. This is harmless but slightly noisy in dry-run or summary output, since the resolved paths appear in the summary JSON regardless of whether sync ran. Consider gating the summary fields on syncProjectCatalogTelemetry:

catalogPath = syncProjectCatalogTelemetry ? Path.GetFullPath(projectCatalogPath) : null,

(Already done for catalogPath and publishPath — but the path resolution at the top is still unconditional.)

---

Summary

Category	Status
Correctness	✅ No functional bugs found
Security	✅ XXE protected; OData injection prevented
Performance	⚠️ HttpClient lifecycle needs attention
Test coverage	✅ Three targeted tests; minor duplication
Conventions	✅ Follows existing multi-alias config pattern
Line budget	⚠️ Check linecount.js before merging

The core logic is sound. The HttpClient lifetime is the one change I'd recommend making before merge; the rest are low-priority improvements.

[claude]

Code Review - PR 344: Improve ecosystem stats refresh resilience

This PR adds two capabilities to the ecosystem-stats pipeline task: (1) PSGallery per-module fallback refresh when the owner-wide Gallery query times out, re-fetching modules individually via the OData v2 API; and (2) Project catalog telemetry sync that merges stats back into a project catalog JSON after each run. The implementation is well-structured. A few areas worth discussing:

CORRECTNESS: The outer catch blocks for HttpRequestException and TaskCanceledException in TryRefreshPowerShellGalleryModulesById are effectively dead code - per-module exceptions are already caught per-task and converted to Warning strings, so Task.WhenAll rarely throws. Also, if the CancellationToken fires mid-batch, partial refresh results (some modules refreshed, others not) can be returned silently; a comment clarifying the best-effort intent would help.

DESIGN: The static HttpClient with Timeout.InfiniteTimeSpan is the correct pattern. However the default for powerShellGalleryByIdRefreshTimeoutSeconds falls back to timeoutSeconds, coupling the two timeouts unless explicitly overridden - worth documenting. The sync-over-async GetAwaiter().GetResult() bridge is fine given the single-threaded pipeline runner with no SynchronizationContext. MergeProjectTelemetry and ProjectCatalogDocument are not visible in this diff - a cross-reference to their location would help reviewers.

CODE QUALITY: (1) In SyncProjectCatalogTelemetryFromStats all three catch blocks (JsonException, IOException, UnauthorizedAccessException) share an identical body - collapse into a single type-pattern handler. (2) The null-forgiveness operator on result.Warning! is redundant after the IsNullOrWhiteSpace check. (3) summary.json now always emits a projectCatalogTelemetry object even when the feature is disabled - soft breaking change for strict-schema consumers.

TESTS: (1) Tests for SyncProjectCatalogTelemetryFromStats, TryPreserveEcosystemSources, and BuildPowerShellGalleryPackageByIdUrl use NonPublic reflection with positional argument arrays. This is brittle - TryPreserveEcosystemSources already required a positional update (6 to 8 args) in this PR. Consider InternalsVisibleTo instead. (2) RunPipeline_EcosystemStats_SyncsProjectCatalogTelemetry uses timeoutSeconds=1 and a random non-existent GitHub org to force fallback. Potentially flaky in CI if PSGallery returns an empty success fast enough.

SECURITY: DTD processing correctly disabled (DtdProcessing.Prohibit, XmlResolver=null) - no XXE risk. OData single-quote escaping in BuildPowerShellGalleryPackageByIdUrl is correct and verified by the dedicated unit test. No unsanitised user-controlled paths beyond what ResolvePath covers.

SUMMARY: Solid, defensively-implemented feature. Actionable: (1) Collapse the three identical catch blocks. (2) Use InternalsVisibleTo over NonPublic reflection for private-method tests. (3) Document that powerShellGalleryByIdRefreshTimeoutSeconds can be set independently of timeoutSeconds. (4) Cross-reference MergeProjectTelemetry and ProjectCatalogDocument. Good work on the OData escaping test and secure XML reader settings.

[claude]

Code Review — PR #344: Improve ecosystem stats refresh resilience

Overview

This PR adds two resilience features to the ecosystem-stats pipeline task:

1. Per-package-ID PSGallery refresh fallback — when the owner-wide Gallery query times out, each preserved module is re-queried individually using the OData v2 Packages endpoint.
2. Project-catalog telemetry sync — after the stats run, download/star counts can be merged back into catalog.json and published to a static path.

Both features are opt-in (refreshPowerShellGalleryByIdOnFallback: false, syncProjectCatalogTelemetry: false by default) and are covered by new focused tests.

---

Code Quality & Style

Strengths:

• Static HttpClient reuse (PowerShellGalleryPackageClient) is correct; Timeout = Timeout.InfiniteTimeSpan is right because cancellation is handled via CancellationTokenSource.
• OData single-quote escaping (id.Replace("'", "''", ...)) is correct and is verified by BuildPowerShellGalleryPackageByIdUrl_EscapesODataLiteralBeforeUrlEncoding.
• DtdProcessing = DtdProcessing.Prohibit, XmlResolver = null properly defends against XXE.
• SemaphoreSlim(8) concurrency cap on parallel HTTP calls is sensible.
• The async-to-sync bridge (.GetAwaiter().GetResult()) is used once at the outer edge with a clear comment — acceptable for a synchronous CLI context.
• Conservative opt-in defaults mean existing pipelines are unaffected.

---

Specific Issues

1. Dead catch clauses in TryRefreshPowerShellGalleryModulesById (minor)

catch (HttpRequestException ex)
{
    warnings.Add(...);
    return null;
}
catch (TaskCanceledException ex)
{
    warnings.Add(...);
    return null;
}

The inner per-module lambda already catches HttpRequestException or TaskCanceledException or XmlException or IOException, so these exceptions never escape individual tasks. Task.WhenAll will not throw either of those types when all tasks have already swallowed them — making the outer catches effectively dead code. Either remove them or replace with a broader catch (Exception) if you want a true safety net.

2. Significant JSON fixture duplication across tests (minor)

RunPipeline_EcosystemStats_SyncsProjectCatalogTelemetry and SyncProjectCatalogTelemetryFromStats_MergesPowerShellGalleryDownloads_WhenStatsIncludeGallery write nearly identical stats/catalog JSON blobs (~80 lines each). Consider extracting a WriteStatsWithGitHubAndGallery(string path) / WriteCatalogWithSecurityPolicy(string path) helper — consistent with the existing WriteStats(...) helper at the bottom of the file.

3. Reflection-based test is now 8-argument (fragile, pre-existing pattern)

var args = new object?[] { File.ReadAllText(statsPath), generatedPath, true, true, true, false, 30, null };

The update from 6 to 8 args shows the risk: a future signature change will fail silently until the test runs. This is a pre-existing pattern in the file, but worth noting that the two new bool/int parameters (refreshPowerShellGalleryByIdOnFallback, powerShellGalleryByIdRefreshTimeoutSeconds) would benefit from a dedicated TryPreserveEcosystemSources_RefreshesModulesByIdOnFallback test rather than being validated only through the preserved-existing-gallery test path.

4. Config-key alias proliferation (minor)

Each new path gets 4–5 config aliases:

GetString(step, "projectCatalogPath") ??
GetString(step, "project-catalog-path") ??
GetString(step, "catalog") ??
GetString(step, "catalogPath") ??
GetString(step, "catalog-path") ??

catalog and catalogPath are very generic — they could collide with unrelated pipeline step config in the future, or cause surprising resolution if a step happens to have a top-level catalog key for another purpose. Prefer the prefixed form (projectCatalogPath / project-catalog-path) and document the aliases that are kept, or drop the un-prefixed ones.

5. Test asserts Equal(1L, ...) with misleading comment

// This run requests only GitHub stats, so catalog sync preserves the existing PSGallery metrics.
Assert.Equal(1L, project.GetProperty("metrics").GetProperty("powerShellGallery").GetProperty("totalDownloads").GetInt64());
Assert.Equal(1L, project.GetProperty("metrics").GetProperty("downloads").GetProperty("total").GetInt64());

The comment is accurate but the assertion value 1L is the stale catalog seed value — it could be confused with "1 module". Naming the constant or adding // stale seed value, not updated because gallery refresh is disabled would clarify intent.

---

Security

• XXE: handled correctly (DtdProcessing.Prohibit, XmlResolver = null). ✅
• OData injection: single-quote doubling is correct and tested. ✅
• No user-controlled data flows into command execution or file paths beyond what already existed.

---

Performance

• HTTP client reuse: correct. ✅
• Per-module concurrency capped at 8: reasonable for a fallback path. ✅
• Catalog file is read, merged, and written in the synchronous pipeline path — fine for small catalogs (<1000 entries), but worth keeping in mind if the catalog grows significantly.

---

Test Coverage

Coverage is good for the happy paths and the OData escaping edge case. Gaps worth considering:

• refreshPowerShellGalleryByIdOnFallback: true path is not integration-tested end-to-end (the unit tests call the private method directly, but there's no pipeline-level test that exercises the fallback + per-ID refresh together).
• SyncProjectCatalogTelemetryFromStats when stats file is missing — the early-return guard exists but isn't tested.
• Malformed XML from PSGallery — XmlException is caught per-module, but there's no test verifying the module falls back gracefully to the existing record.

---

Summary

Solid implementation with sensible defaults and good security hygiene. The three items most worth addressing before merge are:

1. Remove or fix the dead HttpRequestException/TaskCanceledException catch clauses.
2. Extract the duplicated JSON test fixtures into helpers.
3. Reconsider the un-prefixed catalog/catalogPath config aliases.

Everything else is minor polish or future test coverage.

[claude]

Code Review — PR #344: Improve ecosystem stats refresh resilience

Overview

This PR adds two independent features to the ecosystem-stats pipeline task:

1. Per-ID PowerShell Gallery fallback refresh — when the owner-wide Gallery query times out, known modules are re-fetched individually using OData filters.
2. Project-catalog telemetry sync — after ecosystem stats are generated/preserved, download counts and GitHub metrics are merged back into catalog.json and optionally published.

Both features are gated behind new bool step options (refreshPowerShellGalleryByIdOnFallback, syncProjectCatalogTelemetry) so existing behaviour is unchanged by default.

---

Code Quality & Best Practices

Strengths

• OData single-quote escaping (id.Replace("'", "''")) before Uri.EscapeDataString is correct and verified by a dedicated unit test (BuildPowerShellGalleryPackageByIdUrl_EscapesODataLiteralBeforeUrlEncoding).
• XML parsing uses DtdProcessing.Prohibit and XmlResolver = null — correctly prevents XXE attacks.
• Concurrency is capped with SemaphoreSlim(8, 8) during per-ID fetches.
• The static HttpClient with Timeout.InfiniteTimeSpan is intentional — per-operation cancellation is handled by the caller's CancellationTokenSource. This is the right pattern for a synchronous CLI that bridges async code once at the edge.
• IsLatestVersion eq true (not IsAbsoluteLatestVersion) correctly excludes pre-release packages.
• Dual camelCase/kebab-case aliases for new step keys are consistent with the existing config convention.

Concerns

1. Missing error isolation for optional catalog sync (medium)

// ExecuteEcosystemStats — no try/catch around this block
if (syncProjectCatalogTelemetry)
{
    projectCatalogTelemetryMerged = SyncProjectCatalogTelemetryFromStats(
        outputPath, projectCatalogPath, projectCatalogPublishPath,
        out projectCatalogTelemetryWarning);
}

SyncProjectCatalogTelemetryFromStats guards against JsonException/IOException/UnauthorizedAccessException during catalog deserialization, but MergeProjectTelemetry (called just below the guarded block) has no wrapper. An unexpected exception there propagates out of ExecuteEcosystemStats and marks the whole step as failed — even though the ecosystem stats file was already successfully written and published. Since the catalog sync is optional/additive, consider wrapping the entire if (syncProjectCatalogTelemetry) block with a try/catch that sets projectCatalogTelemetryWarning rather than failing the step.

2. Undocumented timeout clamping interaction (minor)

var boundedTimeoutSeconds = Math.Clamp(timeoutSeconds, 5, 300);

When powerShellGalleryByIdRefreshTimeoutSeconds falls through to timeoutSeconds and timeoutSeconds is small (e.g. 1, as used in tests), the clamp silently upgrades the fallback to 5 s. This is probably intentional for reliability, but it's surprising: a step with timeoutSeconds: 1 effectively gives the per-ID refresh a 5× longer budget. A short comment at the clamp site or in docs would help operators understand this.

3. Reflection-based private-method test is fragile (minor)

var syncMethod = typeof(WebPipelineRunner)
    .GetMethod("SyncProjectCatalogTelemetryFromStats", BindingFlags.NonPublic | BindingFlags.Static);
var args = new object?[] { statsPath, catalogPath, publishedCatalogPath, null };
var merged = Assert.IsType<int>(syncMethod!.Invoke(null, args));
Assert.Null(args[3]); // checks the out-warning

Reflection tests are fragile across refactors and provide no compile-time safety. Since SyncProjectCatalogTelemetryFromStats has clear internal-only semantics, an [assembly: InternalsVisibleTo("PowerForge.Tests")] + internal visibility would be cleaner than reflection. This is a pattern decision for the project, so flagging rather than blocking.

4. Duplicated JSON fixtures across tests (minor)

The statsPath/catalogPath JSON blobs (ecosystem stats + catalog with SecurityPolicy) are written verbatim in both RunPipeline_EcosystemStats_SyncsProjectCatalogTelemetry and SyncProjectCatalogTelemetryFromStats_MergesPowerShellGalleryDownloads_WhenStatsIncludeGallery. Extracting these into private helper methods (e.g. WriteStandardStats(path) / WriteStandardCatalog(path)) would reduce duplication and make each test's intent clearer.

---

Missing Test Coverage

The following paths have no automated coverage:

Scenario	Impact
refreshPowerShellGalleryByIdOnFallback: true end-to-end (or via reflection)	Core new feature is untested at the module level
ParseFirstPowerShellGalleryModule with valid/empty/malformed Atom XML	XML parsing logic has no unit tests
Catalog sync failure propagating (to validate isolation proposal above)	Edge case around optional feature failure
MergePowerShellGalleryModule with various null/zero field combinations	Merge logic is untested directly

The per-ID fallback refresh is the headline feature of this PR; not having a test that exercises it (even with a pre-canned XML response) is a gap worth closing before shipping.

---

Summary

The implementation is thoughtful — security is well handled (XXE, OData injection), the async-to-sync bridge is correctly contained, and the feature flags keep the default behaviour stable. The two items worth addressing before merge:

1. Wrap the optional catalog sync in a try/catch so a bug in MergeProjectTelemetry can't fail a step whose primary work is already done.
2. Add at least one test for the per-ID Gallery refresh path — even a minimal test with a stubbed HTTP response or a checked reflection call would close the coverage gap for the headline feature.

Everything else is minor polish.