Skip to content

feat: support remote ALCops configuration - #500

Merged
Arthurvdv merged 7 commits into
ALCops:mainfrom
someC0d3r:feat/configuration-extends
Sep 8, 2026
Merged

Arthurvdv merged 7 commits into
ALCops:mainfrom
someC0d3r:feat/configuration-extends

Conversation

@someC0d3r

@someC0d3r someC0d3r commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds Extends.Source so projects can share one centrally maintained alcops.json and override selected settings locally, as proposed in #483.

Integrates the central CM0001 diagnostic from #512 and addresses the maintainer review: failed inheritance uses complete defaults, HTTP responses have a size limit, and rejected URL credentials are removed from diagnostics.

HTTP failures use a 30-second cooldown per workspace, allowing recovery within the session while avoiding repeated downloads during offline editing. Cancellation propagation, consistent compilation snapshots and shared JSON handling keep settings and diagnostics aligned.

Behavior

  • Loads one anonymously accessible HTTP(S) URL or one absolute local file path. The project must trust its referenced source.
  • Local scalar values and arrays replace inherited values; nested objects merge property by property. Inheritance chains are rejected.
  • HTTP responses are limited to 1 MiB (1,048,576 bytes) during buffering, including chunked responses and responses without Content-Length. The five-second timeout also covers the response body.
  • HTTP(S) URLs containing username/password information are rejected before network access. That information is omitted from the CM0001 diagnostic source.
  • If declared inheritance fails, both the base and local overrides are discarded and built-in defaults apply, with CM0001 explaining the failure. For example, a local complexity threshold of 41 becomes the default 8 if its base cannot be loaded.
  • Unknown top-level setting names remain non-fatal: recognized values apply and CM0001 identifies each unknown name. Invalid inherited values cannot be hidden by local overrides.
  • Each compilation uses one consistent settings snapshot. Retryable HTTP failures are cached per workspace path for 30 seconds after the failed request completes. Compilations within the window reuse defaults and CM0001 without another request. Cache hits do not extend the window; a new compilation requesting settings after expiry makes one shared retry. Successes and deterministic configuration errors remain cached until process restart.
  • Cancellation propagates through all settings consumers and the HTTP body read without caching an error or starting a new cooldown. The first uncached HTTP lookup and eligible retries still wait for their response, bounded by the five-second timeout; all HTTP continuations avoid synchronization-context capture.
  • Empty, whitespace-only, comment-only or JSON-null local input uses defaults without CM0001. An inherited source must still contain an object.

The JSON schema, README, internal guidance, configuration tests and CM0001 tests describe and verify the same behavior. Companion documentation: ALCops/alcops.dev#161.

Local validation

CI follow-up: the concurrent HTTP recovery test now uses dedicated callers and a bounded start barrier, avoiding starvation of its in-process server on small runners. The production cache and timeouts are unchanged. All 159 Common tests passed on AL 12/16/18 with two logical processors, plus 75 isolated repetitions with a four-worker pool; the old task scheduling reproduced the failure under the same constraints.

The cooldown regression test failed before production changes: subsequent compilations fetched again during the intended cooldown. After the correction and deterministic clock coverage, all 159 Common tests pass on each SDK. Tests advance an isolated monotonic clock without real cooldown sleeps or modifying global clock state.

SDK / analyzer target Passed Failed
AL 18.0.36.33307 / net10.0 1,765 0
AL 16.0.27.57058 / net8.0 1,749 0
AL 12.0.13.24028 / netstandard2.1, tests hosted on .NET 10 1,528 0
  • All seven production projects built for all three supported TFMs before the CI-mode tests.
  • Coverage includes the 29,999/30,000 ms expiry boundary, cooldown measured after request completion, repeated failures, cache hits that do not extend expiry, concurrent recovery, permanently cached successes/deterministic errors, stable compilation snapshots and cancelled retries without a new cooldown. Existing active-request cancellation, independent caller cancellation, SDK token-identity and synchronization-context tests remain green.
  • Existing size-boundary, UTF-8, chunked/no-length, credentials, merge, invalid-value and CM0001 tests remain green. Local and inherited type validation are retained intentionally; parsed documents are reused for key scanning and merging.
  • Empty local input and strict inherited-object validation are covered on both JSON implementations. All settings consumers use the compilation captured at CompilationStart; the existing FC0007 semantic-classifier fixtures pass after its registration adjustment.
  • Final dotnet format --verify-no-changes, schema JSON validation and .claude/scripts/Validate-Rules.ps1 passed; the latter checked 52 files.
  • Existing MSB3277 test-reference warnings and NU1900 warnings while retrieving NuGet vulnerability metadata remain. The table reports local validation, not GitHub CI results.
  • Companion docs built with Hugo Extended: 167 pages, no errors; existing Hugo/Docsy deprecation warnings remain.

Codex disclosure

The initial implementation was generated using Codex. Original author note: 5.6 Sol, Reasoning: Very High (4/5).

The follow-up review and these corrections were performed using Codex with GPT-6 Astra, Reasoning: Very High.


Generated via Codex using GPT-6 Astra with Very High reasoning.

@someC0d3r someC0d3r changed the title feat: support extending ALCops configuration feat: support remote ALCops configuration Sep 1, 2026
@someC0d3r someC0d3r closed this Sep 2, 2026
@Arthurvdv

Copy link
Copy Markdown
Member

@someC0d3r, I was planning to review your PR, but I see you've closed it. Is this by accident or do you want to create a new PR?

@someC0d3r

Copy link
Copy Markdown
Contributor Author

@Arthurvdv I'm gonna reopen it then, then you can review it! :)

@someC0d3r someC0d3r reopened this Sep 3, 2026

@Arthurvdv Arthurvdv 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 this — the proof-of-concept is solid and this work shouldn't be wasted. The resolver core matches what was agreed in #483: merge semantics, single-level inheritance, separate validation of the inherited document, and the Lazy cache fix are all keepers. After reviewing this against the NAV SDK's external-ruleset handling, we've settled on requirements that change the failure behavior, so the plan is to sequence this rather than polish it now:

  1. Sequencing: configuration-load diagnostics (#328) must land first — a failure to load alcops.json or an Extends source must surface as a diagnostic, never silently. I'll create a PR for #328 myself; once that's merged, this PR can be rebased onto it and build on that mechanism.
  2. All-or-nothing fallback: when Extends is declared and cannot be fully resolved (unreachable, timeout, malformed, or the source itself declares Extends), the effective configuration falls back to built-in defaults entirely — local overrides are ignored too, to avoid a confusing half-applied state. The diagnostic from #328 makes the drop visible.
  3. Hardening: add a response size cap on the HTTP fetch (e.g. 1 MB). No URL gating beyond the existing credentials-in-URL rejection; we'll document that committing alcops.json implies trusting the referenced source.
  4. Everything else stays as designed here: single source, HTTP(S) or absolute local path, no inheritance chains, once-per-process caching, local-over-remote merge with array replacement.

So concretely: I'll keep this PR open, get #328 in first, and ping you here when it's merged — then a rebase plus the failure-behavior changes (points 2 and 3, with the silent-fallback tests updated to assert the diagnostic + defaults behavior) should get this over the line.


This review was created with Claude.

@Arthurvdv

Copy link
Copy Markdown
Member

@someC0d3r I've implemented the warning mechanism when we can't load the ALCops settings, where I the idea was make it extendable for the Extends.Source feature.

If you take the changes from the main branch ideally you could build this on top of the improvements from the #512.

Ping me if i can assist or help on anything. Would be great to bring this into the next version of ALCops 🤗

@someC0d3r

Copy link
Copy Markdown
Contributor Author

No worries! I'll merge the behinds and give you feedback here when I'm done. :)

…tends

# Conflicts:
#	.claude/rules/common-library.md
#	src/ALCops.Common/Settings/ALCopsSettingsProvider.cs
@someC0d3r

someC0d3r commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Following the model switch from GPT-5.6 Sol to GPT-6 Astra (both using Very High reasoning), GPT-6 Astra reviewed this PR and merge commit 33c2bb1, including the integration of #512 and the requirements in the maintainer review.

Decision basis for findings 1 and 2: I used the explicit requirements published by Arthurvdv in his maintainer review of 4 September 2026.

For the fallback, point 2 states:

local overrides are ignored too

The review treats the declared base and local overrides as one configuration and asks for complete built-in defaults if inheritance fails, to avoid applying only part of that configuration. Continuing with valid local settings was the original design and is technically possible. I classified it as a finding because it differed from this newly requested policy; CM0001 itself does not require discarding local settings.

For HTTP hardening, point 3 requests:

add a response size cap

The review suggests approximately 1 MB. The credential disclosure in finding 3 was an additional issue identified during this Codex review. The linked maintainer review itself discloses that it was created with Claude; I treated the requirements published under Arthurvdv's account as the maintainer's instructions.

The CM0001 foundation from #512 is present: the Common-hosted analyzer, diagnostic metadata, compilation-level reporting, cached failures, and original tests were preserved. However, three issues remain:

  1. Failed inheritance still applies local overrides. The provider records the failure but returns the local settings:

    if (inheritanceFailure is not null)
        localFailures = localFailures.Add(inheritanceFailure);
    
    return new ALCopsSettingsLoadResult(localSettings, localFailures);

    With a missing base file and a local CyclomaticComplexityThreshold of 41, the effective value remains 41, instead of the built-in default 8. This does not meet the requested all-or-nothing fallback. Existing tests currently assert the old behavior and need to change.

  2. The HTTP response has no application-level size cap. The resolver uses GetStringAsync, and a local reproduction accepted a valid response of 2,097,200 bytes without a failure. The five-second timeout does not enforce the requested response-size limit.

  3. Rejected URL credentials are exposed through CM0001. The credentials check correctly prevents the request, but the unmodified URL is stored as SettingsLoadFailure.Source. The diagnostic then includes its username/password in IDE output or build logs. This was reproduced using synthetic credentials through the existing CM0001 test harness.

All 116 existing Common tests passed on each of .NET 10, .NET 8, and the legacy AL 12 setup, which confirms that these specific gaps need additional regression coverage.

I am addressing these findings with failing regression tests first, followed by the implementation and framework checks. The configuration guide and companion documentation PR ALCops/alcops.dev#161 will be updated to describe the resulting behavior. A follow-up comment will report the changes and actual validation results once the fixes are pushed.


Generated via Codex using GPT-6 Astra with Very High reasoning.

@someC0d3r

someC0d3r commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

The three findings from the review are fixed and pushed in 35e96a8.

Decision basis for changes 1 and 2: These implement the explicit requirements published by Arthurvdv in his maintainer review of 4 September 2026.

For the fallback, point 2 states:

local overrides are ignored too

Its stated rationale is to avoid applying only part of the intended base-plus-overrides configuration. I therefore changed the original local-fallback behavior to complete built-in defaults. Using valid local settings would also be technically possible; this change implements the requested policy rather than a technical requirement imposed by CM0001.

For HTTP hardening, point 3 requests:

add a response size cap

The review suggests approximately 1 MB; the concrete limit selected in this implementation is 1 MiB (1,048,576 bytes). Credential redaction in change 3 addresses an additional issue identified during this Codex review. The linked maintainer review itself discloses that it was created with Claude; I treated the requirements published under Arthurvdv's account as the maintainer's instructions.

  1. Complete fallback: a failed declared Extends now returns built-in defaults and CM0001. Local overrides are discarded too, including arrays and nested settings. A local threshold of 41 with an unavailable base now correctly becomes 8.
  2. Bounded HTTP responses: HttpClient.MaxResponseContentBufferSize limits response content to 1 MiB (1,048,576 bytes) before JSON deserialization. This covers chunked responses and missing Content-Length, while retaining the five-second timeout.
  3. Credential redaction: URL username/password information is removed from the recorded diagnostic source. Credential-bearing URLs are still rejected before any connection is made, including percent-encoded credentials.

TDD and validation: the regression run against the previous implementation produced 23 expected failures and 110 passes before production code changed. After the fix, all 133 Common tests pass on each of AL 18, AL 16, and AL 12. Coverage includes HTTP size boundaries, UTF-8 byte counting, HTTP errors/body timeouts, invalid bases and source declarations, and actual CM0001 messages.

All seven cop suites were then run:

SDK / analyzer target Passed Skipped Failed
AL 18 / net10.0 1,739 2 0
AL 16 / net8.0 1,723 18 0
AL 12 / netstandard2.1, tests hosted on .NET 10 1,502 239 0

All seven production projects built for all three supported TFMs. Formatting verification and validation of all 52 internal rule guides passed. Existing NU1900/MSB3277 warnings remain in the local test logs; these results are local validation, not GitHub CI results.

The integration of #512 through merge 33c2bb1 remains intact: the central CM0001 analyzer, metadata, compilation-level reporting, cached failures and regression coverage are preserved. Unknown setting names still report CM0001 while recognized settings remain effective.

The README, schema and internal guidance are aligned with the fixes. Companion docs PR ALCops/alcops.dev#161 now updates both the configuration guide and CM0001 page and builds successfully with Hugo Extended (167 pages). Both PR descriptions have been refreshed to match the final behavior.

No remaining blocker was found in the reviewed changes. This PR remains a Draft.


Generated via Codex using GPT-6 Astra with Very High reasoning.

@someC0d3r

Copy link
Copy Markdown
Contributor Author

@Arthurvdv Codex performed the merge and I reviewed the changes afterwards. Since GPT-6 Astra was rolled out, I switched models and had it perform an AI review of this PR. It identified a few additional changes and created a fix commit to address the points you mentioned
Feel free to review it again. :)

FYI Also the docs PR got updated: ALCops/alcops.dev#161

@Arthurvdv
Arthurvdv marked this pull request as ready for review September 6, 2026 06:03
@Arthurvdv

Copy link
Copy Markdown
Member

Code review — remote ALCops configuration

Scope reviewed: ALCopsSettingsInheritanceResolver.cs (new), ALCopsSettingsProvider.cs, SettingsLoadFailure.cs, alcops.schema.json. Schema parity is fine — Extends is allowlisted in _knownTopLevelKeys and the enum parity tests are unaffected.

Substantive (correctness / latency)

1. Transient network failure permanently poisons a workspace's settingsALCopsSettingsProvider.cs:78
A reachable remote base with one timed-out/blipped fetch makes LoadSettingsFromFileSystem return defaults + an Unreadable failure, which the Lazy caches per directoryPath with no invalidation. Every later compilation silently uses built-in defaults (e.g. CognitiveComplexity 15 instead of the configured value) until the language server restarts. Local-only alcops.json cached deterministic parse outcomes; remote fetch makes the poisoning non-deterministic.

2. Synchronous blocking network I/O on the analyzer threadALCopsSettingsInheritanceResolver.cs:142
GetStringAsync().GetAwaiter().GetResult() blocks the analyzer thread up to the full 5s timeout on first compilation of a workspace with a remote Extends. Stalls interactive analysis, and (finding 4) can't be cancelled early. Deadlock-safe only because Roslyn analyzers run without a SynchronizationContext — a fragile assumption that deserves at least a comment.

Lower-severity behavioral edges

3. Degenerate-but-parseable local JSON now emits CM0001ALCopsSettingsInheritanceResolver.cs:60
An alcops.json that is null/whitespace/comment-only used to deserialize to defaults silently. TryResolve now hits JObject.Parse/ParseObject on a non-object root, which throws outside TryResolve's try/catch, propagating to the outer catch and emitting an Invalid (CM0001) the user didn't get before. Behavior regression / false positive.

4. Remote fetch ignores the analyzer's CancellationTokenALCopsSettingsInheritanceResolver.cs:142
Only the fixed 5s Timeout applies; no CancellationToken is threaded through GetSettings/GetLoadResult into the HTTP call. A cancelled compilation (edit, workspace reload) keeps a thread until the timeout elapses.

Cleanup

5. Redundant deserializationALCopsSettingsProvider.cs:166
localSettings is computed but unused on the inheritance-success path, and the local JSON is parsed 3× per load (DeserializeSettingsCore, GetUnknownSettingFailures, and TryResolve).

6. Duplicated TFM-split JSON handlingALCopsSettingsInheritanceResolver.cs:1
The resolver re-implements the #if NETSTANDARD2_1 Newtonsoft-vs-System.Text.Json stack and case-insensitive key lookup that the provider already owns. Two parallel stacks risk drift (e.g. AllowTrailingCommas/comment-skipping must stay in lockstep).

Verified as not bugs

1 MiB boundary uses > so exactly-1048576 is allowed (matches tests); array-replace/object-merge semantics correct; credential-bearing URLs rejected before any network call and redacted in the diagnostic; file:///relative/drive-relative sources rejected; chain detection works; all-or-nothing fallback to new ALCopsSettings() is intentional per the design memo.


We generated this review with the help of Claude.

@Arthurvdv

Copy link
Copy Markdown
Member

@someC0d3r awesome work and great to see GPT-6 Astra working in the wild 😄

I’ve let Claude run a code review and it came back with a few remarks we should have a look at, but overall, it looks great! With these finishing touches, I think we're good to get this into the main branch for the next ALCops release.

@someC0d3r

someC0d3r commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

@someC0d3r awesome work and great to see GPT-6 Astra working in the wild 😄

Yea! Needed to try it asap 😁 I tried also other stuff with it... incredible what this model can do and in what precision.. kinda scary ngl 🥲

I'm gonna fix your mentioned points:)

@someC0d3r

Copy link
Copy Markdown
Contributor Author

The follow-up to Arthurvdv's review is implemented in 7828117.

  1. Transient HTTP failures: failed requests are no longer retained in the workspace cache for the entire process. A compilation keeps a consistent snapshot, including defaults and CM0001 after a failure; a later compilation retries. A successful retry is cached across subsequent compilations. This covers transport failures, timeouts, HTTP error statuses and response-size rejection. Deterministic configuration errors remain cached.

  2. Synchronous wait: the synchronization-context assumption has been removed: every await in the HTTP helper uses ConfigureAwait(false), with a regression test using a caller synchronization context. The first uncached lookup still waits for the response. The NAV SDK exposes synchronous Action callbacks, and the rule needs its settings before it can report correctly. The wait is bounded by the five-second timeout and is now cancellable. Eliminating that initial wait would require host-side preloading or a change to the agreed configuration semantics; moving the same wait to a background task would not remove it.

  3. Empty local input: JSON null, empty/whitespace-only and comment-only local files now use defaults without CM0001 on both JSON implementations. Other invalid roots, malformed comments and invalid values still report CM0001. An inherited document must still be a JSON object, so the existing all-or-nothing contract is preserved.

  4. Cancellation: every production settings consumer passes its current callback token. Cancellation stops the HTTP body read and does not cache defaults, an exception or a failure diagnostic. A waiting caller can cancel independently of the active workspace load. Tests also check that cancellation exceptions carry the token recognized by the SDK.

  5. Repeated work: local/base documents are parsed into a shared representation reused for key scanning and merging. Local type validation is deliberately retained before network access: an invalid local value must not trigger a download. Independent base validation also remains, so a local override cannot hide an invalid inherited value.

  6. JSON implementation drift: ALCopsSettingsDocument now owns parsing options, case-insensitive key lookup, validation and merge for both TFMs. The provider and resolver use that shared implementation.

All settings consumers use the compilation captured at CompilationStart, because the SDK can expose a different object through SemanticModel.Compilation. For FC0007, the SDK's missing CompilationStart operation-registration API required registering standalone invocation syntax and binding it to the same semantic operation classifier. Its existing positive, negative and incomplete-call coverage passes across the SDK matrix.

TDD: before production changes, the regression run had 7 expected failures and 138 passes. After the corrections and additional coverage, all 152 Common tests pass on every SDK. Full cop-suite results:

SDK / analyzer target Passed Skipped Failed
AL 18.0.36.33307 / net10.0 1,758 2 0
AL 16.0.27.57058 / net8.0 1,742 18 0
AL 12.0.13.24028 / netstandard2.1, tests hosted on .NET 10 1,521 239 0

All seven production projects built for all three TFMs. The final formatting gate, schema JSON check and validation of all 52 internal rule guides passed. Existing MSB3277 test-reference warnings remain. These are local validation results.

The README, schema descriptions and internal guidance are updated. The configuration guide and CM0001 page in docs PR #161 now explain retries, cancellation, empty local input and the remaining initial wait; Hugo Extended built 167 pages successfully.


Development and validation were performed with Codex, GPT-6 Astra, reasoning 'Very High'.

@Arthurvdv

Copy link
Copy Markdown
Member

Circling back on the remote-config caching (7828117) — we looked specifically at how often the remote URL actually gets fetched while someone is editing in VS Code, and traced it against how the editor host drives analysis and how AL handles the equivalent (al.ruleSetPath → remote ruleset).

The success path is already smart 👍

The durable workspace cache (keyed by app directory, process lifetime) short-circuits every later analysis pass, so on the happy path the remote URL is fetched once per process, per app directory — editing files does not re-fetch. That's the property that matters, and it's already better than AL, which re-fetches its remote ruleset on every save (RuleSetLoader.FetchRulesetBodyFromUrl, no caching at all, 15 s timeout, fail-soft to defaults).

The failure path retries too often

Your point 1 in 7828117 is that a failed fetch is no longer retained process-wide, so "a later compilation retries." That's the right fix for the poisoning problem — but the cadence of "a later compilation" turns out to be the catch. In the editor host, a new Compilation is created roughly every 800 ms of debounced editing (DiagnosticServiceOptions.DelayAfterLastDocumentChange = 800 ms; AnalyzerDriverBase allocates a fresh Compilation per run, so the ConditionalWeakTable<Compilation,…> snapshot misses every pass). Since the failure isn't held at the durable layer, each of those passes re-attempts the fetch, each attempt costing up to the 5 s timeout. So an offline developer editing a file gets stalled repeatedly — much more aggressively than AL, which only re-attempts on save.

The overlap of using a remote config and working offline is admittedly a narrow case, but it's a bad enough experience when it hits (repeated multi-second stalls while typing) that it's worth closing.

Suggestion: cool down the failure retry

Keep everything else as-is; just bound the retry. At the durable (per-directory) layer, cache a retryable failure for a short cooldown (~30 s) instead of not caching it at all:

  • within a pass → all callbacks share one attempt (already true);
  • across passes inside the window → no re-fetch, so no per-800 ms stall while offline;
  • after the window → one retry, so a transient blip still recovers within the session, no restart.

Deterministic failures (invalid JSON, unknown setting, credentials) and successes keep caching as they do now. Mechanically it's an expiry alongside the memoized result in the cache entry (retryable failure → now + cooldown; everything else → never expires). Heads-up: a couple of the recovery tests assert the immediately next compilation retries — those would move to advancing an injectable clock past the cooldown, which also gives clean coverage of the new cadence.

What do you think?


Arthur and Claude worked together on this review.

@someC0d3r

Copy link
Copy Markdown
Contributor Author

Agreed with Arthurvdv's retry-cadence review: removing process-wide failure caching allowed a new request on every compilation. Following the proposed 30-second cooldown, this is fixed in 8395257.

  • A retryable HTTP failure is now cached per app directory for 30 seconds from the end of the failed request. New compilations inside that window reuse defaults and CM0001 without another fetch. Cache hits do not extend the window.
  • After expiry, the first new compilation requesting settings retries under the workspace lock. Concurrent callers share the attempt; another failed request starts a fresh cooldown. Successful loads and deterministic errors retain their existing process-lifetime caching.
  • Existing compilation snapshots stay unchanged, even after the window expires or another compilation recovers. Cancellation does not cache a failure or start a new cooldown.
  • The result and its failure timestamp are published together as an immutable cache value. A monotonic Stopwatch clock avoids wall-clock adjustments; tests inject a clock into an isolated cache instance without changing global state.

TDD: the new analyzer regression failed before production changes because subsequent compilations fetched again during the cooldown. After the fix, all 159 Common tests pass on each supported SDK. Coverage includes 12 subsequent analyzer runs sharing the original failure, the 29,999/30,000 ms boundary, cooldown starting after request completion, repeated failures, concurrent recovery, stable snapshots, permanent success/deterministic-error caching and cancellation during an expired retry.

SDK / analyzer target Passed Failed
AL 18.0.36.33307 / net10.0 1,765 0
AL 16.0.27.57058 / net8.0 1,749 0
AL 12.0.13.24028 / netstandard2.1, tests hosted on .NET 10 1,528 0

All seven production projects built for all three targets. The formatting gate, schema JSON check and validation of 52 internal guides passed. Existing MSB3277 test-reference warnings and NU1900 warnings while retrieving NuGet vulnerability metadata remain. These are local validation results.

The first uncached request and each eligible retry can still wait for the five-second timeout; the cooldown prevents a new request on each editor pass during an outage. There is no background refresh. The README, schema, internal guidance and docs PR #161 now describe this behavior. Hugo Extended built 167 pages successfully.


Development and validation were performed with Codex, GPT-6 Astra, Reasoning: Very High.

@Arthurvdv

Copy link
Copy Markdown
Member

@someC0d3r this turned out really nice, thank you for sticking with me and working through all the feedback (and shout-out to Codex too 😄)

A remote alcops.json config is a great feature for teams sharing a central setup and I'm happy to see this come together. Looks production-ready to me, awesome work! 🤗

@Arthurvdv

Copy link
Copy Markdown
Member

@someC0d3r, oops, the CI/CD is failing where I’m seeing random versions fail. My experience tells me we might be running into a concurrency issue, as analyzers run in parallel.

We’re almost at the finish line, would you mind taking a look at this last issue?

@someC0d3r

Copy link
Copy Markdown
Contributor Author

@Arthurvdv No worries, uno momento :)

@someC0d3r

Copy link
Copy Markdown
Contributor Author

Fixed the intermittent CI failure reported by Arthur in 00115d5.

I inspected all 50 SDK test reports from the failing run. All 11 failures were the same test, HttpFailure_IsRetriedAfterCooldown_AndSuccessIsCached, often at the five-second timeout; some also reported a broken pipe from the loopback server.

The test launched 12 synchronous lookups with Task.Run. One caller waited for HTTP while the others waited for the workspace lock, occupying the worker pool also needed by the in-process server. That can starve the server on a small runner and produce a real timeout. [NonParallelizable] controls NUnit scheduling, not the tasks created inside the test.

The competing callers now use dedicated threads with TaskCreationOptions.LongRunning and TaskScheduler.Default, plus a bounded barrier so all 12 still contend together. The test server explicitly queues a continuation to prevent inline loopback I/O from hiding this dependency. Failure assertions now print the actual recorded error reasons.

Red/green evidence: in an isolated process with four worker threads and queued server continuations, the old callers stalled until the watchdog released the pool and reproduced the connection failure. The corrected test passed under the same constraints without watchdog intervention. All 159 Common tests passed on AL 12, AL 16 and AL 18 with two logical processors, followed by 25 constrained-pool repetitions per SDK (75 total). Formatting and validation of all 52 internal guides passed.

GitHub CI: the new run for this commit is currently action_required with zero jobs started and needs maintainer approval. The local results above are not a claim that the new GitHub matrix has passed.

This correction changes the test harness and internal testing guidance. The production cache, 30-second cooldown, five-second HTTP timeout and assertion coverage remain intact; no public documentation change is needed.


Development and validation were performed with Codex, GPT-6 Astra, Reasoning: Very High.

@someC0d3r

Copy link
Copy Markdown
Contributor Author

@someC0d3r this turned out really nice, thank you for sticking with me and working through all the feedback (and shout-out to Codex too 😄)

Happy to help! :) thx for all the thoughtful feedback, it’s helped make the implementation much more robust.
This should make it easier for us to maintain a shared configuration across our AL projects and hopefully other teams will find it useful too:)

@Arthurvdv

Copy link
Copy Markdown
Member

Awesome, let's get this merged! 🎉

@Arthurvdv
Arthurvdv merged commit 3e22fad into ALCops:main Sep 8, 2026
55 checks passed
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.

2 participants