release orchestrator pivot, architecture and planning
This commit is contained in:
602
docs/modules/release-orchestrator/implementation-guide.md
Normal file
602
docs/modules/release-orchestrator/implementation-guide.md
Normal file
@@ -0,0 +1,602 @@
|
||||
# Implementation Guide
|
||||
|
||||
> .NET 10 implementation patterns and best practices for Release Orchestrator modules.
|
||||
|
||||
**Target Audience**: Development team implementing Release Orchestrator modules
|
||||
**Prerequisites**: Familiarity with [CLAUDE.md](../../../CLAUDE.md) coding rules
|
||||
|
||||
---
|
||||
|
||||
## Overview
|
||||
|
||||
This guide supplements the architecture documentation with .NET 10-specific implementation patterns required for all Release Orchestrator modules. These patterns ensure:
|
||||
|
||||
- Deterministic behavior for evidence reproducibility
|
||||
- Testability through dependency injection
|
||||
- Compliance with Stella Ops coding standards
|
||||
- Performance and reliability
|
||||
|
||||
---
|
||||
|
||||
## Code Quality Requirements
|
||||
|
||||
### Compiler Configuration
|
||||
|
||||
All Release Orchestrator projects **MUST** enforce warnings as errors:
|
||||
|
||||
```xml
|
||||
<!-- In Directory.Build.props or .csproj -->
|
||||
<PropertyGroup>
|
||||
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
|
||||
<Nullable>enable</Nullable>
|
||||
<ImplicitUsings>disable</ImplicitUsings>
|
||||
</PropertyGroup>
|
||||
```
|
||||
|
||||
**Rationale**: Warnings indicate potential bugs, regressions, or code quality drift. Treating them as errors prevents them from being ignored.
|
||||
|
||||
---
|
||||
|
||||
## Determinism & Time Handling
|
||||
|
||||
### TimeProvider Injection
|
||||
|
||||
**Never** use `DateTime.UtcNow`, `DateTimeOffset.UtcNow`, or `DateTimeOffset.Now` directly. Always inject `TimeProvider`.
|
||||
|
||||
```csharp
|
||||
// ❌ BAD - non-deterministic, hard to test
|
||||
public class PromotionManager
|
||||
{
|
||||
public Promotion CreatePromotion(Guid releaseId, Guid targetEnvId)
|
||||
{
|
||||
return new Promotion
|
||||
{
|
||||
Id = Guid.NewGuid(),
|
||||
ReleaseId = releaseId,
|
||||
TargetEnvironmentId = targetEnvId,
|
||||
RequestedAt = DateTimeOffset.UtcNow // ❌ Hard-coded time
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
// ✅ GOOD - injectable, testable, deterministic
|
||||
public class PromotionManager
|
||||
{
|
||||
private readonly TimeProvider _timeProvider;
|
||||
private readonly IGuidGenerator _guidGenerator;
|
||||
|
||||
public PromotionManager(TimeProvider timeProvider, IGuidGenerator guidGenerator)
|
||||
{
|
||||
_timeProvider = timeProvider;
|
||||
_guidGenerator = guidGenerator;
|
||||
}
|
||||
|
||||
public Promotion CreatePromotion(Guid releaseId, Guid targetEnvId)
|
||||
{
|
||||
return new Promotion
|
||||
{
|
||||
Id = _guidGenerator.NewGuid(),
|
||||
ReleaseId = releaseId,
|
||||
TargetEnvironmentId = targetEnvId,
|
||||
RequestedAt = _timeProvider.GetUtcNow() // ✅ Injected, testable
|
||||
};
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Registration**:
|
||||
```csharp
|
||||
// Production: use system time
|
||||
services.AddSingleton(TimeProvider.System);
|
||||
|
||||
// Testing: use manual time for deterministic tests
|
||||
var manualTime = new ManualTimeProvider();
|
||||
manualTime.SetUtcNow(new DateTimeOffset(2026, 1, 10, 12, 0, 0, TimeSpan.Zero));
|
||||
services.AddSingleton<TimeProvider>(manualTime);
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### GUID Generation
|
||||
|
||||
**Never** use `Guid.NewGuid()` directly. Always inject `IGuidGenerator`.
|
||||
|
||||
```csharp
|
||||
// ❌ BAD
|
||||
var releaseId = Guid.NewGuid();
|
||||
|
||||
// ✅ GOOD
|
||||
var releaseId = _guidGenerator.NewGuid();
|
||||
```
|
||||
|
||||
**Interface**:
|
||||
```csharp
|
||||
public interface IGuidGenerator
|
||||
{
|
||||
Guid NewGuid();
|
||||
}
|
||||
|
||||
// Production implementation
|
||||
public sealed class SystemGuidGenerator : IGuidGenerator
|
||||
{
|
||||
public Guid NewGuid() => Guid.NewGuid();
|
||||
}
|
||||
|
||||
// Deterministic test implementation
|
||||
public sealed class SequentialGuidGenerator : IGuidGenerator
|
||||
{
|
||||
private int _counter;
|
||||
|
||||
public Guid NewGuid()
|
||||
{
|
||||
var bytes = new byte[16];
|
||||
BitConverter.GetBytes(_counter++).CopyTo(bytes, 0);
|
||||
return new Guid(bytes);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Async & Cancellation
|
||||
|
||||
### CancellationToken Propagation
|
||||
|
||||
**Always** propagate `CancellationToken` through async call chains. Never use `CancellationToken.None` except at entry points where no token is available.
|
||||
|
||||
```csharp
|
||||
// ❌ BAD - ignores cancellation
|
||||
public async Task<Promotion> ApprovePromotionAsync(Guid promotionId, Guid userId, CancellationToken ct)
|
||||
{
|
||||
var promotion = await _repository.GetByIdAsync(promotionId, CancellationToken.None); // ❌ Wrong
|
||||
|
||||
promotion.Approvals.Add(new Approval
|
||||
{
|
||||
ApproverId = userId,
|
||||
ApprovedAt = _timeProvider.GetUtcNow()
|
||||
});
|
||||
|
||||
await _repository.SaveAsync(promotion, CancellationToken.None); // ❌ Wrong
|
||||
await Task.Delay(1000); // ❌ Missing ct
|
||||
|
||||
return promotion;
|
||||
}
|
||||
|
||||
// ✅ GOOD - propagates cancellation
|
||||
public async Task<Promotion> ApprovePromotionAsync(Guid promotionId, Guid userId, CancellationToken ct)
|
||||
{
|
||||
var promotion = await _repository.GetByIdAsync(promotionId, ct); // ✅ Propagated
|
||||
|
||||
promotion.Approvals.Add(new Approval
|
||||
{
|
||||
ApproverId = userId,
|
||||
ApprovedAt = _timeProvider.GetUtcNow()
|
||||
});
|
||||
|
||||
await _repository.SaveAsync(promotion, ct); // ✅ Propagated
|
||||
await Task.Delay(1000, ct); // ✅ Cancellable
|
||||
|
||||
return promotion;
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## HTTP Client Usage
|
||||
|
||||
### IHttpClientFactory for Connector Runtime
|
||||
|
||||
**Never** instantiate `HttpClient` directly. Always use `IHttpClientFactory` with configured timeouts and resilience policies.
|
||||
|
||||
```csharp
|
||||
// ❌ BAD - direct instantiation risks socket exhaustion
|
||||
public class GitHubConnector
|
||||
{
|
||||
public async Task<string> GetCommitAsync(string sha)
|
||||
{
|
||||
using var client = new HttpClient(); // ❌ Socket exhaustion risk
|
||||
var response = await client.GetAsync($"https://api.github.com/commits/{sha}");
|
||||
return await response.Content.ReadAsStringAsync();
|
||||
}
|
||||
}
|
||||
|
||||
// ✅ GOOD - factory with resilience
|
||||
public class GitHubConnector
|
||||
{
|
||||
private readonly IHttpClientFactory _httpClientFactory;
|
||||
|
||||
public GitHubConnector(IHttpClientFactory httpClientFactory)
|
||||
{
|
||||
_httpClientFactory = httpClientFactory;
|
||||
}
|
||||
|
||||
public async Task<string> GetCommitAsync(string sha, CancellationToken ct)
|
||||
{
|
||||
var client = _httpClientFactory.CreateClient("GitHub");
|
||||
var response = await client.GetAsync($"/commits/{sha}", ct);
|
||||
response.EnsureSuccessStatusCode();
|
||||
return await response.Content.ReadAsStringAsync(ct);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Registration with resilience**:
|
||||
```csharp
|
||||
services.AddHttpClient("GitHub", client =>
|
||||
{
|
||||
client.BaseAddress = new Uri("https://api.github.com");
|
||||
client.Timeout = TimeSpan.FromSeconds(30);
|
||||
client.DefaultRequestHeaders.Add("User-Agent", "StellaOps/1.0");
|
||||
})
|
||||
.AddStandardResilienceHandler(options =>
|
||||
{
|
||||
options.Retry.MaxRetryAttempts = 3;
|
||||
options.CircuitBreaker.SamplingDuration = TimeSpan.FromSeconds(30);
|
||||
options.TotalRequestTimeout.Timeout = TimeSpan.FromMinutes(1);
|
||||
});
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Culture & Formatting
|
||||
|
||||
### Invariant Culture for Parsing
|
||||
|
||||
**Always** use `CultureInfo.InvariantCulture` for parsing and formatting dates, numbers, and any string that will be persisted, hashed, or compared.
|
||||
|
||||
```csharp
|
||||
// ❌ BAD - culture-sensitive
|
||||
var percentage = double.Parse(input);
|
||||
var formatted = value.ToString("P2");
|
||||
var dateStr = date.ToString("yyyy-MM-dd");
|
||||
|
||||
// ✅ GOOD - invariant culture
|
||||
var percentage = double.Parse(input, CultureInfo.InvariantCulture);
|
||||
var formatted = value.ToString("P2", CultureInfo.InvariantCulture);
|
||||
var dateStr = date.ToString("yyyy-MM-dd", CultureInfo.InvariantCulture);
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## JSON Handling
|
||||
|
||||
### RFC 8785 Canonical JSON for Evidence
|
||||
|
||||
For evidence packets and decision records that will be hashed or signed, use **RFC 8785-compliant** canonical JSON serialization.
|
||||
|
||||
```csharp
|
||||
// ❌ BAD - non-canonical JSON
|
||||
var json = JsonSerializer.Serialize(decisionRecord, new JsonSerializerOptions
|
||||
{
|
||||
Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping,
|
||||
PropertyNamingPolicy = JsonNamingPolicy.CamelCase
|
||||
});
|
||||
var hash = ComputeHash(json); // ❌ Non-deterministic
|
||||
|
||||
// ✅ GOOD - use shared canonicalizer
|
||||
var canonicalJson = CanonicalJsonSerializer.Serialize(decisionRecord);
|
||||
var hash = ComputeHash(canonicalJson); // ✅ Deterministic
|
||||
```
|
||||
|
||||
**Canonical JSON Requirements**:
|
||||
- Keys sorted alphabetically
|
||||
- Minimal escaping per RFC 8785 spec
|
||||
- No exponent notation for numbers
|
||||
- No trailing/leading zeros
|
||||
- No whitespace
|
||||
|
||||
---
|
||||
|
||||
## Database Interaction
|
||||
|
||||
### DateTimeOffset for PostgreSQL timestamptz
|
||||
|
||||
PostgreSQL `timestamptz` columns **MUST** be read and written as `DateTimeOffset`, not `DateTime`.
|
||||
|
||||
```csharp
|
||||
// ❌ BAD - loses offset information
|
||||
await using var reader = await command.ExecuteReaderAsync(ct);
|
||||
while (await reader.ReadAsync(ct))
|
||||
{
|
||||
var createdAt = reader.GetDateTime(reader.GetOrdinal("created_at")); // ❌ Loses offset
|
||||
}
|
||||
|
||||
// ✅ GOOD - preserves offset
|
||||
await using var reader = await command.ExecuteReaderAsync(ct);
|
||||
while (await reader.ReadAsync(ct))
|
||||
{
|
||||
var createdAt = reader.GetFieldValue<DateTimeOffset>(reader.GetOrdinal("created_at")); // ✅ Correct
|
||||
}
|
||||
```
|
||||
|
||||
**Insertion**:
|
||||
```csharp
|
||||
// ✅ Always use UTC DateTimeOffset
|
||||
var createdAt = _timeProvider.GetUtcNow(); // Returns DateTimeOffset
|
||||
await command.ExecuteNonQueryAsync(ct);
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Hybrid Logical Clock (HLC) for Distributed Ordering
|
||||
|
||||
For distributed ordering and audit-safe sequencing, use `IHybridLogicalClock` from `StellaOps.HybridLogicalClock`.
|
||||
|
||||
**When to use HLC**:
|
||||
- Promotion state transitions
|
||||
- Workflow step execution ordering
|
||||
- Deployment task sequencing
|
||||
- Timeline event ordering
|
||||
|
||||
```csharp
|
||||
public class PromotionStateTransition
|
||||
{
|
||||
private readonly IHybridLogicalClock _hlc;
|
||||
private readonly TimeProvider _timeProvider;
|
||||
|
||||
public async Task TransitionStateAsync(
|
||||
Promotion promotion,
|
||||
PromotionState newState,
|
||||
CancellationToken ct)
|
||||
{
|
||||
var transition = new StateTransition
|
||||
{
|
||||
PromotionId = promotion.Id,
|
||||
FromState = promotion.Status,
|
||||
ToState = newState,
|
||||
THlc = _hlc.Tick(), // ✅ Monotonic, skew-tolerant ordering
|
||||
TsWall = _timeProvider.GetUtcNow(), // ✅ Informational timestamp
|
||||
TransitionedBy = _currentUser.Id
|
||||
};
|
||||
|
||||
await _repository.RecordTransitionAsync(transition, ct);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**HLC State Persistence**:
|
||||
```csharp
|
||||
// Service startup
|
||||
public async Task StartAsync(CancellationToken ct)
|
||||
{
|
||||
await _hlc.InitializeFromStateAsync(ct); // Restore monotonicity
|
||||
}
|
||||
|
||||
// Service shutdown
|
||||
public async Task StopAsync(CancellationToken ct)
|
||||
{
|
||||
await _hlc.PersistStateAsync(ct); // Persist HLC state
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Configuration & Options
|
||||
|
||||
### Options Validation at Startup
|
||||
|
||||
Use `ValidateDataAnnotations()` and `ValidateOnStart()` for all options classes.
|
||||
|
||||
```csharp
|
||||
// Options class
|
||||
public sealed class PromotionManagerOptions
|
||||
{
|
||||
[Required]
|
||||
[Range(1, 10)]
|
||||
public int MaxConcurrentPromotions { get; set; } = 3;
|
||||
|
||||
[Required]
|
||||
[Range(1, 3600)]
|
||||
public int ApprovalExpirationSeconds { get; set; } = 1440;
|
||||
}
|
||||
|
||||
// Registration with validation
|
||||
services.AddOptions<PromotionManagerOptions>()
|
||||
.Bind(configuration.GetSection("PromotionManager"))
|
||||
.ValidateDataAnnotations()
|
||||
.ValidateOnStart();
|
||||
|
||||
// Complex validation
|
||||
public class PromotionManagerOptionsValidator : IValidateOptions<PromotionManagerOptions>
|
||||
{
|
||||
public ValidateOptionsResult Validate(string? name, PromotionManagerOptions options)
|
||||
{
|
||||
if (options.MaxConcurrentPromotions <= 0)
|
||||
return ValidateOptionsResult.Fail("MaxConcurrentPromotions must be positive");
|
||||
|
||||
return ValidateOptionsResult.Success;
|
||||
}
|
||||
}
|
||||
|
||||
services.AddSingleton<IValidateOptions<PromotionManagerOptions>, PromotionManagerOptionsValidator>();
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Immutability & Collections
|
||||
|
||||
### Return Immutable Collections from Public APIs
|
||||
|
||||
Public APIs **MUST** return `IReadOnlyList<T>`, `ImmutableArray<T>`, or defensive copies. Never expose mutable backing stores.
|
||||
|
||||
```csharp
|
||||
// ❌ BAD - exposes mutable backing store
|
||||
public class ReleaseManager
|
||||
{
|
||||
private readonly List<Component> _components = new();
|
||||
|
||||
public List<Component> Components => _components; // ❌ Callers can mutate!
|
||||
}
|
||||
|
||||
// ✅ GOOD - immutable return
|
||||
public class ReleaseManager
|
||||
{
|
||||
private readonly List<Component> _components = new();
|
||||
|
||||
public IReadOnlyList<Component> Components => _components.AsReadOnly(); // ✅ Read-only
|
||||
|
||||
// Or using ImmutableArray
|
||||
public ImmutableArray<Component> GetComponents() => _components.ToImmutableArray();
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Error Handling
|
||||
|
||||
### No Silent Stubs
|
||||
|
||||
Placeholder code **MUST** throw `NotImplementedException` or return an explicit error. Never return success from unimplemented paths.
|
||||
|
||||
```csharp
|
||||
// ❌ BAD - silent stub masks missing implementation
|
||||
public async Task<Result> DeployToNomadAsync(Deployment deployment, CancellationToken ct)
|
||||
{
|
||||
// TODO: implement Nomad deployment
|
||||
return Result.Success(); // ❌ Ships broken feature!
|
||||
}
|
||||
|
||||
// ✅ GOOD - explicit failure
|
||||
public async Task<Result> DeployToNomadAsync(Deployment deployment, CancellationToken ct)
|
||||
{
|
||||
throw new NotImplementedException(
|
||||
"Nomad deployment not yet implemented. See SPRINT_20260115_003_AGENTS_nomad_support.md");
|
||||
}
|
||||
|
||||
// ✅ Alternative: return unsupported result
|
||||
public async Task<Result> DeployToNomadAsync(Deployment deployment, CancellationToken ct)
|
||||
{
|
||||
return Result.Failure("Nomad deployment target not yet supported. Use Docker or Compose.");
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Caching
|
||||
|
||||
### Bounded Caches with Eviction
|
||||
|
||||
**Do not** use `ConcurrentDictionary` or `Dictionary` for caching without eviction policies. Use bounded caches with TTL/LRU eviction.
|
||||
|
||||
```csharp
|
||||
// ❌ BAD - unbounded growth
|
||||
public class VersionMapCache
|
||||
{
|
||||
private readonly ConcurrentDictionary<string, DigestMapping> _cache = new();
|
||||
|
||||
public void Add(string tag, DigestMapping mapping)
|
||||
{
|
||||
_cache[tag] = mapping; // ❌ Never evicts, memory grows forever
|
||||
}
|
||||
}
|
||||
|
||||
// ✅ GOOD - bounded with eviction
|
||||
public class VersionMapCache
|
||||
{
|
||||
private readonly MemoryCache _cache;
|
||||
|
||||
public VersionMapCache()
|
||||
{
|
||||
_cache = new MemoryCache(new MemoryCacheOptions
|
||||
{
|
||||
SizeLimit = 10_000 // Max 10k entries
|
||||
});
|
||||
}
|
||||
|
||||
public void Add(string tag, DigestMapping mapping)
|
||||
{
|
||||
_cache.Set(tag, mapping, new MemoryCacheEntryOptions
|
||||
{
|
||||
Size = 1,
|
||||
SlidingExpiration = TimeSpan.FromHours(1) // ✅ 1 hour TTL
|
||||
});
|
||||
}
|
||||
|
||||
public DigestMapping? Get(string tag) => _cache.Get<DigestMapping>(tag);
|
||||
}
|
||||
```
|
||||
|
||||
**Cache TTL Recommendations**:
|
||||
- **Integration health checks**: 5 minutes
|
||||
- **Version maps (tag → digest)**: 1 hour
|
||||
- **Environment configs**: 30 minutes
|
||||
- **Agent capabilities**: 10 minutes
|
||||
|
||||
---
|
||||
|
||||
## Testing
|
||||
|
||||
### Test Helpers Must Call Production Code
|
||||
|
||||
Test helpers **MUST** call production code, not reimplement algorithms. Only mock I/O and network boundaries.
|
||||
|
||||
```csharp
|
||||
// ❌ BAD - test reimplements production logic
|
||||
public static string ComputeEvidenceHash(DecisionRecord record)
|
||||
{
|
||||
// Custom hash implementation in test
|
||||
var json = JsonSerializer.Serialize(record); // ❌ Different from production!
|
||||
return SHA256.HashData(Encoding.UTF8.GetBytes(json)).ToHexString();
|
||||
}
|
||||
|
||||
// ✅ GOOD - test uses production code
|
||||
public static string ComputeEvidenceHash(DecisionRecord record)
|
||||
{
|
||||
// Calls production EvidenceHasher
|
||||
return EvidenceHasher.ComputeHash(record); // ✅ Same as production
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Path Resolution
|
||||
|
||||
### Explicit CLI Options for Paths
|
||||
|
||||
**Do not** derive paths from `AppContext.BaseDirectory` with parent directory walks. Use explicit CLI options or environment variables.
|
||||
|
||||
```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.");
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Summary Checklist
|
||||
|
||||
Before submitting a pull request, verify:
|
||||
|
||||
- [ ] `TreatWarningsAsErrors` enabled in project file
|
||||
- [ ] All timestamps use `TimeProvider`, never `DateTime.UtcNow`
|
||||
- [ ] All GUIDs use `IGuidGenerator`, never `Guid.NewGuid()`
|
||||
- [ ] `CancellationToken` propagated through all async methods
|
||||
- [ ] HTTP clients use `IHttpClientFactory`, never `new HttpClient()`
|
||||
- [ ] Culture-invariant parsing for all formatted strings
|
||||
- [ ] Canonical JSON for evidence/decision records
|
||||
- [ ] `DateTimeOffset` for all PostgreSQL `timestamptz` columns
|
||||
- [ ] HLC used for distributed ordering where applicable
|
||||
- [ ] Options classes validated at startup with `ValidateOnStart()`
|
||||
- [ ] Public APIs return immutable collections
|
||||
- [ ] No silent stubs; unimplemented code throws `NotImplementedException`
|
||||
- [ ] Caches have bounded size and TTL eviction
|
||||
- [ ] Tests exercise production code, not reimplementations
|
||||
|
||||
---
|
||||
|
||||
## References
|
||||
|
||||
- [CLAUDE.md](../../../CLAUDE.md) — Stella Ops coding rules
|
||||
- [Test Structure](./test-structure.md) — Test organization guidelines
|
||||
- [Database Schema](./data-model/schema.md) — Schema patterns
|
||||
- [HLC Documentation](../../eventing/event-envelope-schema.md) — Event ordering with HLC
|
||||
Reference in New Issue
Block a user