-
Notifications
You must be signed in to change notification settings - Fork 0
feat(phase-02a): packet 3 — cross-cutting foundation (ADR-0032) #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
cemililik
merged 9 commits into
main
from
feat/phase-02a-packet-3-cross-cutting-foundation
Aug 8, 2026
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
b1e1306
feat(phase-02a): packet 3 — cross-cutting foundation per ADR-0032
cemililik 6023f67
fix(phase-02a): packet 3 review-1 — ADR-0032 contract gaps + cleanups
cemililik a194b77
fix(phase-02a): packet 3 review-2 — secret-provider seam + sensitive-…
cemililik 0fd16d5
fix(phase-02a): packet 3 review-3 — L1 robustness, redaction parity, …
cemililik b4c98f0
fix(phase-02a): packet 3 review-3/4 — analyzer crash, redaction depth…
cemililik 0d1b81e
style(phase-02a): packet 3 — dotnet format the recursive redaction sw…
cemililik 7d5983d
docs(roadmap): thread Packet 3 deferred follow-ups into their owning …
cemililik 3a0552e
fix(phase-02a): update DefaultTokens to return a read-only snapshot a…
cemililik 2c2099b
fix(phase-02a): enhance error tracking and resilience handling, impro…
cemililik File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 2 additions & 0 deletions
2
backend/analyzers/LearnStack.Analyzers/AnalyzerReleases.Shipped.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| ; Shipped analyzer releases. | ||
| ; See https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md |
8 changes: 8 additions & 0 deletions
8
backend/analyzers/LearnStack.Analyzers/AnalyzerReleases.Unshipped.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| ; Unshipped analyzer release. | ||
| ; See https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md | ||
|
|
||
| ### New Rules | ||
|
|
||
| Rule ID | Category | Severity | Notes | ||
| --------|----------|----------|------------ | ||
| LS0001 | Design | Warning | Rule "LearnStackException-DomainExceptionThrow" (ADR-0032 § Sub-decision 4 + Amendment 1). DomainException is reserved for programmer errors; expected business-rule violations return Result.Fail(business_rule_violation, ...). Roslyn diagnostic ids must be valid identifiers, so the wire id is LS0001; the hyphenated string is the human-readable rule name. Severity escalates to Error after Phase 03 exit. |
126 changes: 126 additions & 0 deletions
126
backend/analyzers/LearnStack.Analyzers/DomainExceptionThrowAnalyzer.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| using System.Collections.Immutable; | ||
| using Microsoft.CodeAnalysis; | ||
| using Microsoft.CodeAnalysis.CSharp; | ||
| using Microsoft.CodeAnalysis.CSharp.Syntax; | ||
| using Microsoft.CodeAnalysis.Diagnostics; | ||
|
|
||
| namespace LearnStack.Analyzers; | ||
|
|
||
| /// <summary> | ||
| /// Rule "LearnStackException-DomainExceptionThrow" (Roslyn diagnostic id | ||
| /// <see cref="DiagnosticId"/> = <c>LS0001</c>) — flags every | ||
| /// <c>throw new DomainException(...)</c> in Domain / Application code per | ||
| /// ADR-0032 § Sub-decision 4. Expected business-rule violations return | ||
| /// <c>Result.Fail(business_rule_violation, ...)</c>; the exception is | ||
| /// reserved for programmer errors. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// The Roslyn diagnostic id MUST be a valid identifier (letters/digits, no | ||
| /// hyphens) — Roslyn throws <c>AD0001</c> at report time otherwise. The | ||
| /// human-readable rule name <c>LearnStackException-DomainExceptionThrow</c> | ||
| /// (ADR-0032 / Standards 21 naming convention) is carried in the title and | ||
| /// help text; the wire-level id is <c>LS0001</c>. See ADR-0032 Amendment 1. | ||
| /// </para> | ||
| /// <para> | ||
| /// Severity: <see cref="DiagnosticSeverity.Warning"/> in Phase 02a. Per | ||
| /// ADR-0032 the severity escalates to <see cref="DiagnosticSeverity.Error"/> | ||
| /// after Phase 03 exit when every existing call site has been migrated. | ||
| /// Until then <c>LS0001</c> is listed in <c>WarningsNotAsErrors</c> | ||
| /// (Directory.Build.props) so a legitimate aggregate-invariant throw does | ||
| /// not break CI under <c>TreatWarningsAsErrors</c> before the documented | ||
| /// escalation point. | ||
| /// </para> | ||
| /// </remarks> | ||
| [DiagnosticAnalyzer(LanguageNames.CSharp)] | ||
| public sealed class DomainExceptionThrowAnalyzer : DiagnosticAnalyzer | ||
| { | ||
| public const string DiagnosticId = "LS0001"; | ||
|
|
||
| /// <summary>The human-readable rule name (ADR-0032 / Standards 21).</summary> | ||
| public const string RuleName = "LearnStackException-DomainExceptionThrow"; | ||
|
|
||
| private static readonly LocalizableString Title = | ||
| "Avoid throwing DomainException for expected business-rule violations (LearnStackException-DomainExceptionThrow)"; | ||
|
|
||
| private static readonly LocalizableString MessageFormat = | ||
| "DomainException is reserved for programmer errors. " + | ||
| "Return Result.Fail(business_rule_violation, ...) instead."; | ||
|
|
||
| private static readonly LocalizableString Description = | ||
| "ADR-0032 § Sub-decision 4 reserves DomainException for aggregate-invariant " + | ||
| "violations that signal a programming mistake. Expected business-rule " + | ||
| "violations are an outcome — return Result.Fail(business_rule_violation, ...) " + | ||
| "from the domain method."; | ||
|
|
||
| private static readonly DiagnosticDescriptor Rule = new( | ||
| id: DiagnosticId, | ||
| title: Title, | ||
| messageFormat: MessageFormat, | ||
| category: "Design", | ||
| defaultSeverity: DiagnosticSeverity.Warning, | ||
| isEnabledByDefault: true, | ||
| description: Description, | ||
| helpLinkUri: "https://github.com/cemililik/LearnStack/blob/main/docs/decisions/0032-exception-handling-logging-and-observability.md"); | ||
|
|
||
| public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics | ||
| => ImmutableArray.Create(Rule); | ||
|
|
||
| public override void Initialize(AnalysisContext context) | ||
| { | ||
| if (context is null) throw new System.ArgumentNullException(nameof(context)); | ||
| context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); | ||
| context.EnableConcurrentExecution(); | ||
| context.RegisterSyntaxNodeAction(AnalyzeThrowExpression, SyntaxKind.ThrowExpression); | ||
| context.RegisterSyntaxNodeAction(AnalyzeThrowStatement, SyntaxKind.ThrowStatement); | ||
| } | ||
|
|
||
| private static void AnalyzeThrowStatement(SyntaxNodeAnalysisContext context) | ||
| { | ||
| var node = (ThrowStatementSyntax)context.Node; | ||
| if (node.Expression is null) return; | ||
| InspectThrown(context, node.Expression); | ||
| } | ||
|
|
||
| private static void AnalyzeThrowExpression(SyntaxNodeAnalysisContext context) | ||
| { | ||
| var node = (ThrowExpressionSyntax)context.Node; | ||
| InspectThrown(context, node.Expression); | ||
| } | ||
|
|
||
| private static void InspectThrown(SyntaxNodeAnalysisContext context, ExpressionSyntax expression) | ||
| { | ||
| if (expression is not ObjectCreationExpressionSyntax creation) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| var typeInfo = context.SemanticModel.GetTypeInfo(creation, context.CancellationToken); | ||
| var symbol = typeInfo.Type; | ||
| if (symbol is null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| if (!IsDomainException(symbol)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| context.ReportDiagnostic(Diagnostic.Create(Rule, creation.GetLocation())); | ||
| } | ||
|
|
||
| private static bool IsDomainException(ITypeSymbol type) | ||
| { | ||
| for (var current = type; current is not null; current = current.BaseType) | ||
| { | ||
| if (current.Name == "DomainException" && | ||
| current.ContainingNamespace?.ToDisplayString() == "LearnStack.SharedKernel.Errors") | ||
| { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
| } | ||
32 changes: 32 additions & 0 deletions
32
backend/analyzers/LearnStack.Analyzers/LearnStack.Analyzers.csproj
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
|
|
||
| <PropertyGroup> | ||
| <!-- Roslyn analyzers must target netstandard2.0 — the analyzer host | ||
| (Visual Studio, dotnet build, Roslyn) only loads netstandard2.0 | ||
| analyzer assemblies regardless of the project's TargetFramework. | ||
| See: https://learn.microsoft.com/dotnet/csharp/roslyn-sdk/tutorials/how-to-write-csharp-analyzer-code-fix#analyzer-target-framework --> | ||
| <TargetFramework>netstandard2.0</TargetFramework> | ||
| <Nullable>enable</Nullable> | ||
| <LangVersion>latest</LangVersion> | ||
| <IsPackable>false</IsPackable> | ||
| <RootNamespace>LearnStack.Analyzers</RootNamespace> | ||
| <EnforceExtendedAnalyzerRules>true</EnforceExtendedAnalyzerRules> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.CodeAnalysis.CSharp"> | ||
| <PrivateAssets>all</PrivateAssets> | ||
| </PackageReference> | ||
| <PackageReference Include="Microsoft.CodeAnalysis.Analyzers"> | ||
| <PrivateAssets>all</PrivateAssets> | ||
| </PackageReference> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <!-- Roslyn release-tracking rule (RS2008) requires these markdown files | ||
| to live alongside the analyzer assembly. --> | ||
| <AdditionalFiles Include="AnalyzerReleases.Shipped.md" /> | ||
| <AdditionalFiles Include="AnalyzerReleases.Unshipped.md" /> | ||
| </ItemGroup> | ||
|
|
||
| </Project> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| using System.Net; | ||
| using LearnStack.SharedKernel.Errors; | ||
| using LearnStack.SharedKernel.Results; | ||
|
|
||
| namespace LearnStack.Api.Common; | ||
|
|
||
| /// <summary> | ||
| /// Maps an <see cref="Error.Code"/> (or a <see cref="LearnStackException"/> | ||
| /// subclass) to the HTTP status the API surface returns. The table mirrors | ||
| /// <see href="../../../../docs/standards/09-error-handling.md">Standards 09 | ||
| /// § Result Type</see>; adding a new error code requires updating both | ||
| /// places so the contract stays in sync. | ||
| /// </summary> | ||
| public static class HttpStatusMap | ||
| { | ||
| public static int For(Error error) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(error); | ||
| return For(error.Code); | ||
| } | ||
|
|
||
| public static int For(string code) => code switch | ||
| { | ||
| "validation_failed" => (int)HttpStatusCode.BadRequest, | ||
| "unsupported_locale" => (int)HttpStatusCode.BadRequest, | ||
| "unauthorized" => (int)HttpStatusCode.Unauthorized, | ||
| "forbidden" => (int)HttpStatusCode.Forbidden, | ||
| "resource_scope_violation" => (int)HttpStatusCode.Forbidden, | ||
| "feature_disabled" => (int)HttpStatusCode.Forbidden, | ||
| "not_found" => (int)HttpStatusCode.NotFound, | ||
| "tenant_mismatch" => (int)HttpStatusCode.NotFound, | ||
| "concurrency_conflict" => (int)HttpStatusCode.Conflict, | ||
| "business_rule_violation" => (int)HttpStatusCode.Conflict, | ||
| "recording_consent_required" => (int)HttpStatusCode.Conflict, | ||
| "rate_limited" => (int)HttpStatusCode.TooManyRequests, | ||
| "dependency_unavailable" => (int)HttpStatusCode.ServiceUnavailable, | ||
| _ => (int)HttpStatusCode.InternalServerError, | ||
| }; | ||
|
|
||
| public static int For(Exception exception) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(exception); | ||
|
|
||
| return exception switch | ||
| { | ||
| // 499 "client closed request" is an Nginx convention, not an | ||
| // IETF code — Standards 09 § Result Type does not pin a status | ||
| // for client disconnects. We pick 499 (rather than 408 / 503 / | ||
| // 500) because: | ||
| // * IIS, Nginx, Envoy, and APISIX all emit 499 for pre- | ||
| // response client aborts; log dashboards and SLO calculators | ||
| // already treat it as "not our fault". | ||
| // * 408 implies a server-side timeout (we did not time out — | ||
| // the client left). | ||
| // * 5xx codes would inflate error-budget metrics and trip | ||
| // PagerDuty rotations for nothing. | ||
| // L1 handler skips both Sentry capture and the response body | ||
| // write for OperationCanceled per ADR-0032 § Sub-decision 7; | ||
| // the status is set here for parity with the upstream proxy's | ||
| // behaviour. If a future ADR pins a different code, change | ||
| // this line. | ||
| OperationCanceledException => 499, | ||
|
|
||
| // Every LearnStackException carries a structured Error; the HTTP | ||
| // status is derived from that Error.Code so the response status | ||
| // and the Problem Details `code` field can NEVER disagree. In | ||
| // particular `ProviderException.IsClientError` is an | ||
| // observability concern (it gates Sentry capture in | ||
| // ShouldCapture), NOT an HTTP-status concern: a bare provider | ||
| // failure carries `dependency_unavailable` → 503, and an adapter | ||
| // that wants to surface a provider 4xx as a client-actionable | ||
| // status passes an explicit Error (e.g. validation_failed → 400). | ||
| // Deriving from the code keeps body+status consistent for both. | ||
| LearnStackException known => For(known.Error), | ||
| _ => (int)HttpStatusCode.InternalServerError, | ||
| }; | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
Repository: HodeTech/LearnStack
Length of output: 50375
🏁 Script executed:
Repository: HodeTech/LearnStack
Length of output: 28394
🏁 Script executed:
Repository: HodeTech/LearnStack
Length of output: 21419
🏁 Script executed:
Repository: HodeTech/LearnStack
Length of output: 339
Align LS0001 with aggregate-invariant guards
InspectThrownreports every matchingDomainExceptionconstruction. It does not detect aggregate-invariant guards. After Phase 03, valid invariant throws can fail the build.📍 Affects 2 files
backend/analyzers/LearnStack.Analyzers/DomainExceptionThrowAnalyzer.cs#L91-L110(this comment)backend/src/LearnStack.Domain/LearnStack.Domain.csproj#L10-L16🤖 Prompt for AI Agents