Skip to content

feat(cli) wire CLI under Aspire and end three dead-config items#54

Merged
jkeeley2073 merged 1 commit into
mainfrom
Dev-AspireWiringAndDeadConfigFixes
May 3, 2026
Merged

feat(cli) wire CLI under Aspire and end three dead-config items#54
jkeeley2073 merged 1 commit into
mainfrom
Dev-AspireWiringAndDeadConfigFixes

Conversation

@jkeeley2073
Copy link
Copy Markdown
Contributor

Summary

Bundled fix for the three critical dead-config items surfaced by PR #53's Bicep deploy-prep audit. All three were the exact pattern that motivated feedback_pre_pr_self_audit.md: production code that exists but is never called, so a deploy alone gets nothing useful at runtime.

Three logical pieces, one PR (single "make the runtime deploy-ready" theme; tightly coupled wiring):

1. Aspire wiring + Cosmos refactor

PinballWizard.Cli/Program.cs now calls builder.AddServiceDefaults() (OTel + service discovery + standard HTTP resilience + health checks). Cosmos / OPDB / Cosmos-backed-politeness registrations are gated on ConnectionStrings:cosmos (Aspire) OR Cosmos:AccountEndpoint (Managed-Identity) being present. Standalone CLI runs without Cosmos behave exactly as before; Aspire-orchestrated runs (or production with Cosmos:AccountEndpoint from Bicep outputs) auto-wire everything.

AddCosmosPersistence accepts an externally-registered CosmosClient — all registrations switched to TryAddSingleton so Aspire's AddAzureCosmosClient("cosmos") registration is preserved. CosmosOptions.AccountEndpoint is now string? (was [Required]); the fallback throws InvalidOperationException with a clear remediation message if both registrations are absent.

2. OPDB CLI dispatch via --source opdb

Special-cased in Program.cs rather than wrapped in an ISourceScraper adapter — OPDB writes to IMachineRepository instead of yielding ScrapedItems. Returns exit code 2 with a clear message when OPDB / Cosmos aren't configured. OPDB is added to ScraperOrchestrator.SourceAliases so SourceAliasContractTests recognises the alias. The --source flag description explicitly warns that all does NOT include opdb (run --source opdb explicitly).

3. IPerSourcePolitenessResolver + PolitenessGate rewire

IPerSourcePolitenessResolver is the new abstraction returning the effective PolitenessOptions for a request URL.

  • DefaultPerSourcePolitenessResolver always returns the global defaults (registered via TryAddSingleton in AddPoliteScraping)
  • IngestionSourcePolitenessResolver reads IngestionSource.PolitenessOverrides from IIngestionSourceRepository on first lookup, caches the resulting host -> effective-options map, and degrades safely to defaults when Cosmos is unreachable so a transient outage never blocks scraping. Wired by AddCosmosBackedPolitenessOverrides.

ApplyOverrides is public static for direct test access. PolitenessGate consumes the resolver per-request for effective delay + 429-streak limit; the previously-cached _options field is removed.

Configuration-key constants

CosmosOptions / OpdbOptions expose AccountEndpointKey / CosmosConnectionName / BaseUrlKey constants so Program.cs can presence-check the configuration without duplicating the section strings — avoids exactly the dead-config drift the project's pre-PR self-audit was created to catch.

Test Plan

  • dotnet build PinballWizard.slnx -> 7 / 7 projects, 0 warnings, 0 errors
  • dotnet test PinballWizard.slnx -> 496 / 496 passing (was 486)
  • New tests pin every override-merge shape, host-keyed lookup, graceful degradation on repository failure, and load-once caching:
    • ApplyOverrides_NullOverrides_ReturnsDefaultsUnchanged
    • ApplyOverrides_AllNullFields_ReturnsValueEqualToDefaults
    • ApplyOverrides_RequestDelayOverride_AppliesOnly / Max429StreakOverride / UserAgentSuffix / RobotsTxtPathOverride
    • ResolveAsync_KnownHost_ReturnsEffectiveOptionsWithOverrides
    • ResolveAsync_UnknownHost_ReturnsDefaults
    • ResolveAsync_RepositoryThrows_FallsBackToDefaults (load-bearing — pins the resilience invariant)
    • ResolveAsync_CalledTwice_LoadsRepositoryOnce
  • Three existing test files updated for the new PolitenessGate ctor (PolitenessGateTests / OpdbSyncServiceTests / OpdbClientTests) — each constructs a DefaultPerSourcePolitenessResolver from the existing Options.Create(politenessOptions) and passes that to the gate

Out of Scope

  • AppHost adding the CLI as an orchestrated resource — the CLI is one-shot; making it work cleanly under AddProject<Projects.PinballWizard_Cli> requires arg-handling design (per-invocation args vs per-launch args). UX nicety, not a critical fix; future PR.
  • Cosmos containers in Bicep — the audit recommended adding machines and ingestion_sources containers with their partition keys to infra/modules/shared.bicep. This PR's runtime path uses CosmosBootstrapper.EnsureCreatedAsync to create containers on first connect (works with both the local emulator and a deployed account); declaring them in Bicep is a defence-in-depth follow-up.
  • Seed script for ingestion_sources — until containers exist somewhere (deployed Cosmos OR local emulator running), the seed has nowhere to land. Future PR after the first deploy.
  • Concurrent-first-call test for IngestionSourcePolitenessResolver — flagged minor by the local review. The _initLock semaphore is a textbook init-once pattern; a Task.WhenAll two-parallel-resolves test would be defensive but the cost-of-deferring is low.
  • Carrying effective options on the LeaseReportResponseAsync currently re-resolves rather than reusing the options from AcquireForRequestAsync. ~50ns per call extra; not measurable against actual network latency. Defer.

Checklist

  • CI is green (build + test + coverage + CodeQL + sanitization)
  • PR title follows the Conventional Commits format above
  • If this is a new architectural decision, an ADR has been added under docs/adr/ — N/A (this PR implements the existing ADR 0007 design that defined PolitenessOverrides; no new architecture)
  • If user-visible behavior changes, README.md and/or docs/ are updated in the same PR — README gains a Local development with .NET Aspire section and the --source table is updated for opdb
  • If a memory in ~/.claude/projects/c--projects-PinballWizard/memory/ is now stale, it has been updated or removed in the same PR — handoff memory updated separately
  • No TODO / FIXME / commented-out code committed
  • No new entries in <NoWarn> without a comment explaining why and the removal criterion

Pre-push self-audit

Step 0 — /local-review (qualitative)

  • Ran /local-review and addressed every critical finding before push
  • Local review outcome: 0 critical / 5 minor (3 fixed, 2 deferred-with-justification) / 5 categories clean
    • Fixed: --source description now explicitly warns all excludes opdb (was a silent UX surprise)
    • Fixed: Program.cs block comment adds a one-line note that gating is by presence not validation (threat-model accepts env-var redirection as strictly weaker than the RCE the attacker would already need)
    • Fixed: CosmosOptions.AccountEndpointKey / CosmosConnectionName / OpdbOptions.BaseUrlKey constants prevent dead-config drift on section renames (the exact failure mode feedback_pre_pr_self_audit.md is meant to catch)
    • Deferred: concurrent-first-call test (init pattern is textbook; cost-of-deferring low)
    • Deferred: redundant ResolveAsync in ReportResponseAsync (~50ns/call; not measurable)

Step 1 — Mechanical checklist

  • Every new *Options property has at least one real getter call in src/CosmosOptions.AccountEndpoint is read by AddCosmosPersistence fallback; AccountEndpointKey / CosmosConnectionName / BaseUrlKey constants are read by Program.cs gating
  • Sibling-diffed against the closest existing implementation — the resolver pattern is novel; verified ApplyOverrides matches every documented PolitenessOverrides field semantic
  • No bare catch { } — the only catch in new code (IngestionSourcePolitenessResolver.cs:84) is catch (Exception ex) when (ex is not OperationCanceledException)
  • New ISourceScraper? — N/A (OPDB is special-cased rather than adapted)
  • Tests assert behavior, not just structure — 10 new tests pin every override shape, host-keyed lookup, degradation, and caching
  • Build is zero-warning
  • git log -1 --format='%an <%ae>' shows personal noreply, not work email

Bundles the three critical dead-config fixes surfaced by PR #53's Bicep
deploy-prep audit:

1. PinballWizard.Cli.Program.cs now calls builder.AddServiceDefaults()
   and gates Cosmos/OPDB/Cosmos-backed-politeness registrations on
   ConnectionStrings:cosmos OR Cosmos:AccountEndpoint presence. Standalone
   CLI runs without Cosmos behave exactly as before; Aspire-orchestrated
   runs (or production with Cosmos:AccountEndpoint set from Bicep
   outputs) automatically wire the persistence layer + OPDB + per-source
   politeness overrides.

2. AddCosmosPersistence accepts an externally-registered CosmosClient.
   All registrations switched to TryAddSingleton so an Aspire-injected
   client (built from the connection string for the local emulator) is
   preserved. CosmosOptions.AccountEndpoint is now string? — required
   only when the Managed-Identity fallback is in play; the fallback
   throws InvalidOperationException with a clear remediation message
   if both registrations are absent.

3. OPDB CLI dispatch via --source opdb. Special-cased rather than
   wrapped in an ISourceScraper adapter because OPDB writes to
   IMachineRepository instead of yielding ScrapedItems. Returns exit
   code 2 with a clear message when OPDB / Cosmos aren't configured.
   OPDB added to ScraperOrchestrator.SourceAliases so the contract
   test recognises the alias. The --source flag description explicitly
   warns that 'all' does NOT include 'opdb' (run --source opdb
   explicitly).

4. IPerSourcePolitenessResolver + DefaultPerSourcePolitenessResolver
   + IngestionSourcePolitenessResolver. New abstraction in the polite-
   scraping namespace that returns the effective PolitenessOptions for
   a request URL. The Cosmos-backed implementation reads
   IngestionSource.PolitenessOverrides on first lookup, caches the
   resulting host -> effective-options map, and degrades safely to
   defaults when Cosmos is unreachable so a transient outage never
   blocks scraping. ApplyOverrides is public static for direct test
   access. PolitenessGate consumes the resolver per-request for
   effective delay + 429-streak limit; the previously-cached _options
   field is removed.

5. CosmosOptions / OpdbOptions expose AccountEndpointKey /
   CosmosConnectionName / BaseUrlKey constants so Program.cs can
   presence-check the configuration without duplicating the section
   strings (avoids exactly the dead-config drift the project's
   pre-PR self-audit was created to catch).

Three test files updated for the new PolitenessGate ctor (each
constructs DefaultPerSourcePolitenessResolver from the existing
politenessOptions). +10 new tests in IngestionSourcePolitenessResolverTests
pinning ApplyOverrides math, host-keyed lookup, graceful degradation
on repository failure, and load-once caching.

Tests: 486 -> 496. Build clean, zero warnings.

README gains a 'Local development with .NET Aspire' section and the
--source flag table is updated to enumerate every manufacturer scraper
plus opdb. Pre-existing staleness in other README sections is left for
a future docs PR.

Pre-push self-audit: /local-review (0 critical / 5 minor — 3 fixed,
2 deferred-with-justification) plus 7-item mechanical checklist (all
pass).
@jkeeley2073 jkeeley2073 added the claude-code Generated with Claude Code label May 3, 2026
@jkeeley2073 jkeeley2073 merged commit a9a0deb into main May 3, 2026
5 checks passed
@jkeeley2073 jkeeley2073 deleted the Dev-AspireWiringAndDeadConfigFixes branch May 3, 2026 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

claude-code Generated with Claude Code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant