save progress

AGENTS.md

@@ -273,6 +273,7 @@ Implementation principles:
 
 * Always follow .NET 10 and Angular v17 best practices.
 * Apply SOLID design principles (SRP, OCP, LSP, ISP, DIP) in service and library code.
+* Keep in mind the nuget versions are controlled centrally by src/Directory* files, not via csproj
 * Maximise reuse and composability.
 * Maintain determinism: stable ordering, UTC ISO-8601 timestamps, immutable NDJSON where applicable.
 
@@ -388,6 +389,396 @@ If no design decision is required, you proceed autonomously, implementing the ch
 5) **Do not defer:** Execute steps 1–4 immediately; reporting is after the fact, not a gating step.
 
 **Lessons baked in:** Past delays came from missing code carry-over and missing sprint tasks. Always move advisory code into benchmarks/tests and open the corresponding sprint rows the same session you read the advisory.
+
+---
+
+### 8) Code Quality & Determinism Rules
+
+These rules were distilled from a comprehensive audit of 324+ projects. They address the most common recurring issues and must be followed by all implementers.
+
+#### 8.1) Compiler & Warning Discipline
+
+| Rule | Guidance |
+|------|----------|
+| **Enable TreatWarningsAsErrors** | All projects must set `<TreatWarningsAsErrors>true</TreatWarningsAsErrors>` in the `.csproj` or via `Directory.Build.props`. Relaxed warnings mask regressions and code quality drift. |
+
+```xml
+<!-- In .csproj or Directory.Build.props -->
+<PropertyGroup>
+  <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
+</PropertyGroup>
+```
+
+#### 8.2) Deterministic Time & ID Generation
+
+| Rule | Guidance |
+|------|----------|
+| **Inject TimeProvider / ID generators** | Never use `DateTime.UtcNow`, `DateTimeOffset.UtcNow`, `Guid.NewGuid()`, or `Random.Shared` directly in production code. Inject `TimeProvider` (or `ITimeProvider`) and `IGuidGenerator` abstractions. |
+
+```csharp
+// BAD - nondeterministic, hard to test
+public class BadService
+{
+    public Record CreateRecord() => new Record
+    {
+        Id = Guid.NewGuid(),
+        CreatedAt = DateTimeOffset.UtcNow
+    };
+}
+
+// GOOD - injectable, testable, deterministic
+public class GoodService(TimeProvider timeProvider, IGuidGenerator guidGenerator)
+{
+    public Record CreateRecord() => new Record
+    {
+        Id = guidGenerator.NewGuid(),
+        CreatedAt = timeProvider.GetUtcNow()
+    };
+}
+```
+
+#### 8.3) ASCII-Only Output
+
+| Rule | Guidance |
+|------|----------|
+| **No mojibake or non-ASCII glyphs** | Use ASCII-only characters in comments, output strings, and log messages. No `ƒ?`, `バ`, `→`, `✓`, `✗`, or box-drawing characters. When Unicode is truly required, use explicit escapes (`\uXXXX`) and document the rationale. |
+
+```csharp
+// BAD - non-ASCII glyphs
+Console.WriteLine("✓ Success → proceeding");
+// or mojibake comments like: // ƒ+ validation passed
+
+// GOOD - ASCII only
+Console.WriteLine("[OK] Success - proceeding");
+// Comment: validation passed
+```
+
+#### 8.4) Test Project Requirements
+
+| Rule | Guidance |
+|------|----------|
+| **Every library needs tests** | All production libraries/services must have a corresponding `*.Tests` project covering: (a) happy paths, (b) error/edge cases, (c) determinism, and (d) serialization round-trips. |
+
+```
+src/
+  Scanner/
+    __Libraries/
+      StellaOps.Scanner.Core/
+    __Tests/
+      StellaOps.Scanner.Core.Tests/   <-- Required
+```
+
+#### 8.5) Culture-Invariant Parsing
+
+| Rule | Guidance |
+|------|----------|
+| **Use InvariantCulture** | Always use `CultureInfo.InvariantCulture` for parsing and formatting dates, numbers, percentages, and any string that will be persisted, hashed, or compared. Current culture causes locale-dependent, nondeterministic behavior. |
+
+```csharp
+// BAD - culture-sensitive
+var value = double.Parse(input);
+var formatted = percentage.ToString("P2");
+
+// GOOD - invariant culture
+var value = double.Parse(input, CultureInfo.InvariantCulture);
+var formatted = percentage.ToString("P2", CultureInfo.InvariantCulture);
+```
+
+#### 8.6) DSSE PAE Consistency
+
+| Rule | Guidance |
+|------|----------|
+| **Single DSSE PAE implementation** | Use one spec-compliant DSSE PAE helper (`StellaOps.Attestation.DsseHelper` or equivalent) across the codebase. DSSE v1 requires ASCII decimal lengths and space separators. Never reimplement PAE encoding. |
+
+```csharp
+// BAD - custom PAE implementation
+var pae = $"DSSEv1 {payloadType.Length} {payloadType} {payload.Length} ";
+
+// GOOD - use shared helper
+var pae = DsseHelper.ComputePreAuthenticationEncoding(payloadType, payload);
+```
+
+#### 8.7) RFC 8785 JSON Canonicalization
+
+| Rule | Guidance |
+|------|----------|
+| **Use shared RFC 8785 canonicalizer** | For digest/signature inputs, use a shared RFC 8785-compliant JSON canonicalizer with: sorted keys, minimal escaping per spec, no exponent notation for numbers, no trailing/leading zeros. Do not use `UnsafeRelaxedJsonEscaping` or `CamelCase` naming for canonical outputs. |
+
+```csharp
+// BAD - non-canonical JSON
+var json = JsonSerializer.Serialize(obj, new JsonSerializerOptions
+{
+    Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping,
+    PropertyNamingPolicy = JsonNamingPolicy.CamelCase
+});
+
+// GOOD - use shared canonicalizer
+var canonicalJson = CanonicalJsonSerializer.Serialize(obj);
+var digest = ComputeDigest(canonicalJson);
+```
+
+#### 8.8) CancellationToken Propagation
+
+| Rule | Guidance |
+|------|----------|
+| **Propagate CancellationToken** | Always propagate `CancellationToken` through async call chains. Never use `CancellationToken.None` in production code except at entry points where no token is available. |
+
+```csharp
+// BAD - ignores cancellation
+public async Task ProcessAsync(CancellationToken ct)
+{
+    await _repository.SaveAsync(data, CancellationToken.None);  // Wrong!
+    await Task.Delay(1000);  // Missing ct
+}
+
+// GOOD - propagates cancellation
+public async Task ProcessAsync(CancellationToken ct)
+{
+    await _repository.SaveAsync(data, ct);
+    await Task.Delay(1000, ct);
+}
+```
+
+#### 8.9) HttpClient via Factory
+
+| Rule | Guidance |
+|------|----------|
+| **Use IHttpClientFactory** | Never `new HttpClient()` directly. Use `IHttpClientFactory` with configured timeouts and retry policies via Polly or `Microsoft.Extensions.Http.Resilience`. Direct HttpClient creation risks socket exhaustion. |
+
+```csharp
+// BAD - direct instantiation
+public class BadService
+{
+    public async Task FetchAsync()
+    {
+        using var client = new HttpClient();  // Socket exhaustion risk
+        await client.GetAsync(url);
+    }
+}
+
+// GOOD - factory with resilience
+public class GoodService(IHttpClientFactory httpClientFactory)
+{
+    public async Task FetchAsync()
+    {
+        var client = httpClientFactory.CreateClient("MyApi");
+        await client.GetAsync(url);
+    }
+}
+
+// Registration with timeout/retry
+services.AddHttpClient("MyApi")
+    .ConfigureHttpClient(c => c.Timeout = TimeSpan.FromSeconds(30))
+    .AddStandardResilienceHandler();
+```
+
+#### 8.10) Path/Root Resolution
+
+| Rule | Guidance |
+|------|----------|
+| **Explicit CLI options for paths** | Do not derive repository root from `AppContext.BaseDirectory` with parent directory walks. Use explicit CLI options (`--repo-root`) or environment variables. Provide sensible defaults with clear error messages. |
+
+```csharp
+// BAD - fragile parent walks
+var repoRoot = Path.GetFullPath(Path.Combine(
+    AppContext.BaseDirectory, "..", "..", "..", ".."));
+
+// GOOD - explicit option with fallback
+[Option("--repo-root", Description = "Repository root path")]
+public string? RepoRoot { get; set; }
+
+public string GetRepoRoot() =>
+    RepoRoot ?? Environment.GetEnvironmentVariable("STELLAOPS_REPO_ROOT")
+    ?? throw new InvalidOperationException("Repository root not specified. Use --repo-root or set STELLAOPS_REPO_ROOT.");
+```
+
+#### 8.11) Test Categorization
+
+| Rule | Guidance |
+|------|----------|
+| **Correct test categories** | Tag tests correctly: `[Trait("Category", "Unit")]` for pure unit tests, `[Trait("Category", "Integration")]` for tests requiring databases, containers, or network. Don't mix DB/network tests into unit suites. |
+
+```csharp
+// BAD - integration test marked as unit
+public class UserRepositoryTests  // Uses Testcontainers/Postgres
+{
+    [Fact]  // Missing category, runs with unit tests
+    public async Task Save_PersistsUser() { ... }
+}
+
+// GOOD - correctly categorized
+[Trait("Category", "Integration")]
+public class UserRepositoryTests
+{
+    [Fact]
+    public async Task Save_PersistsUser() { ... }
+}
+
+[Trait("Category", "Unit")]
+public class UserValidatorTests
+{
+    [Fact]
+    public void Validate_EmptyEmail_ReturnsFalse() { ... }
+}
+```
+
+#### 8.12) No Silent Stubs
+
+| Rule | Guidance |
+|------|----------|
+| **Unimplemented code must throw** | Placeholder code must throw `NotImplementedException` or return an explicit error/unsupported status. Never return success (`null`, empty results, or success codes) from unimplemented paths. |
+
+```csharp
+// BAD - silent stub masks missing implementation
+public async Task<Result> ProcessAsync()
+{
+    // TODO: implement later
+    return Result.Success();  // Ships broken feature!
+}
+
+// GOOD - explicit failure
+public async Task<Result> ProcessAsync()
+{
+    throw new NotImplementedException("ProcessAsync not yet implemented. See SPRINT_XYZ.");
+}
+```
+
+#### 8.13) Immutable Collection Returns
+
+| Rule | Guidance |
+|------|----------|
+| **Return immutable collections** | Public APIs must return `IReadOnlyList<T>`, `ImmutableArray<T>`, or defensive copies. Never expose mutable backing stores that callers can mutate. |
+
+```csharp
+// BAD - exposes mutable backing store
+public class BadRegistry
+{
+    private readonly List<string> _scopes = new();
+    public List<string> Scopes => _scopes;  // Callers can mutate!
+}
+
+// GOOD - immutable return
+public class GoodRegistry
+{
+    private readonly List<string> _scopes = new();
+    public IReadOnlyList<string> Scopes => _scopes.AsReadOnly();
+    // or: public ImmutableArray<string> Scopes => _scopes.ToImmutableArray();
+}
+```
+
+#### 8.14) Options Validation at Startup
+
+| Rule | Guidance |
+|------|----------|
+| **ValidateOnStart for options** | Use `ValidateDataAnnotations()` and `ValidateOnStart()` for options. Implement `IValidateOptions<T>` for complex validation. All required config must be validated at startup, not at first use. |
+
+```csharp
+// BAD - no validation until runtime failure
+services.Configure<MyOptions>(config.GetSection("My"));
+
+// GOOD - validated at startup
+services.AddOptions<MyOptions>()
+    .Bind(config.GetSection("My"))
+    .ValidateDataAnnotations()
+    .ValidateOnStart();
+
+// With complex validation
+public class MyOptionsValidator : IValidateOptions<MyOptions>
+{
+    public ValidateOptionsResult Validate(string? name, MyOptions options)
+    {
+        if (options.Timeout <= TimeSpan.Zero)
+            return ValidateOptionsResult.Fail("Timeout must be positive");
+        return ValidateOptionsResult.Success;
+    }
+}
+```
+
+#### 8.15) No Backup Files in Source
+
+| Rule | Guidance |
+|------|----------|
+| **Exclude backup/temp artifacts** | Add backup patterns (`*.Backup.tmp`, `*.bak`, `*.orig`) to `.gitignore`. Regularly audit for and remove stray artifacts. Consolidate duplicate tools/harnesses. |
+
+```gitignore
+# .gitignore additions
+*.Backup.tmp
+*.bak
+*.orig
+*~
+```
+
+#### 8.16) Test Production Code, Not Reimplementations
+
+| Rule | Guidance |
+|------|----------|
+| **Helpers call production code** | Test helpers must call production code, not reimplement algorithms (Merkle trees, DSSE PAE, parsers, canonicalizers). Only mock I/O and network boundaries. Reimplementations cause test/production drift. |
+
+```csharp
+// BAD - test reimplements production logic
+[Fact]
+public void Merkle_ComputesCorrectRoot()
+{
+    // Custom Merkle implementation in test
+    var root = TestMerkleHelper.ComputeRoot(leaves);  // Drift risk!
+    Assert.Equal(expected, root);
+}
+
+// GOOD - test exercises production code
+[Fact]
+public void Merkle_ComputesCorrectRoot()
+{
+    // Uses production MerkleTreeBuilder
+    var root = MerkleTreeBuilder.ComputeRoot(leaves);
+    Assert.Equal(expected, root);
+}
+```
+
+#### 8.17) Bounded Caches with Eviction
+
+| Rule | Guidance |
+|------|----------|
+| **No unbounded Dictionary caches** | Do not use `ConcurrentDictionary` or `Dictionary` for caching without eviction policies. Use bounded caches with TTL/LRU eviction (`MemoryCache` with size limits, or external cache like Valkey). Document expected cardinality and eviction behavior. |
+
+```csharp
+// BAD - unbounded growth
+private readonly ConcurrentDictionary<string, CacheEntry> _cache = new();
+
+public void Add(string key, CacheEntry entry)
+{
+    _cache[key] = entry;  // Never evicts, memory grows forever
+}
+
+// GOOD - bounded with eviction
+private readonly MemoryCache _cache = new(new MemoryCacheOptions
+{
+    SizeLimit = 10_000
+});
+
+public void Add(string key, CacheEntry entry)
+{
+    _cache.Set(key, entry, new MemoryCacheEntryOptions
+    {
+        Size = 1,
+        SlidingExpiration = TimeSpan.FromMinutes(30)
+    });
+}
+```
+
+#### 8.18) DateTimeOffset for PostgreSQL timestamptz
+
+| Rule | Guidance |
+|------|----------|
+| **Use GetFieldValue&lt;DateTimeOffset&gt;** | PostgreSQL `timestamptz` columns must be read via `reader.GetFieldValue<DateTimeOffset>()`, not `reader.GetDateTime()`. `GetDateTime()` loses offset information and causes UTC/local confusion. Store and retrieve all timestamps as UTC `DateTimeOffset`. |
+
+```csharp
+// BAD - loses offset information
+var createdAt = reader.GetDateTime(reader.GetOrdinal("created_at"));
+
+// GOOD - preserves offset
+var createdAt = reader.GetFieldValue<DateTimeOffset>(reader.GetOrdinal("created_at"));
+```
+
 ---
 
 ### 6) Role Switching