mirror of
https://github.com/azaion/annotations.git
synced 2026-06-21 20:01:06 +00:00
03f879206e
This commit captures everything produced during autodev existing-code Steps 1 (Document), 2 (Architecture Baseline Scan), and 3 (Test Spec), together with the targeted auth + CORS re-sync triggered on 2026-05-14 when codebase drift was detected at Step 4 entry. None of this work was previously committed. Step 1 (Document) — 50+ _docs/02_document/ files: problem, solution, architecture, system flows, glossary, module-layout, per-component specs (01..06), modules, deployment, diagrams, data model, FINAL report, verification log, discovery. Step 2 (Architecture Baseline) — architecture_compliance_baseline.md. Verdict PASS_WITH_WARNINGS (0 Critical, 0 High, 1 Medium, 2 Low). No High/Critical findings; auto-chained to Step 3 per existing-code flow. Step 3 (Test Spec) — _docs/02_document/tests/* (67 scenarios across blackbox, security, resilience, resource-limit, performance), plus e2e/docker-compose.test.yml, e2e/seed/run.sh, scripts/run-tests.sh, scripts/run-performance-tests.sh. Coverage 88% over the active scope (40 of 45 items covered, 6 RB-deferred, 5 documented-as-uncovered). Targeted auth + CORS re-sync — replaces the deleted in-house token issuer with a JWKS-verifier model. AuthController and TokenService removed; JwtExtensions switched from HS256 symmetric to ES256 over admin's JWKS. ConfigurationResolver and CorsConfigurationValidator added under src/Infrastructure/. ADR-002 and ADR-006 retired; SEC-01, SEC-02, SEC-03 marked Closed. One new testability risk recorded in architecture.md Open Risks Section 6 (JWKS HTTPS gating). Source changes: - src/Auth/JwtExtensions.cs (modified) — ES256, JWKS, alg pinning - src/Program.cs (modified) — DI wiring for ConfigurationResolver and CorsConfigurationValidator - src/Controllers/AuthController.cs (deleted) — no in-service issuance - src/Services/TokenService.cs (deleted) — same - src/Infrastructure/ConfigurationResolver.cs (new) - src/Infrastructure/CorsConfigurationValidator.cs (new) - .env.example (new) — required env var documentation - .gitignore (updated) Cross-repo coordination: _docs/cross-repo/flights_h1_h2_h3_change_spec captures the change-spec for downstream services that consumed the now deleted /auth endpoints. Co-authored-by: Cursor <cursoragent@cursor.com>
106 lines
7.8 KiB
Markdown
106 lines
7.8 KiB
Markdown
# Architecture Compliance Baseline — Azaion.Annotations
|
|
|
|
**Mode**: code-review baseline (Phase 1 + Phase 7 only)
|
|
**Scope**: full codebase under `src/` (57 C# files)
|
|
**Date**: 2026-05-14
|
|
**Verdict**: PASS_WITH_WARNINGS
|
|
|
|
This is the **one-time architecture baseline** for the existing-code flow's Step 2. Future per-batch code-review runs partition findings against this baseline (carried over / resolved / newly introduced) per `.cursor/skills/code-review/SKILL.md` → "Baseline delta".
|
|
|
|
## Inputs (verified loaded)
|
|
|
|
- `_docs/02_document/architecture.md` — layering rules, ADRs, refactor backlog
|
|
- `_docs/02_document/module-layout.md` — per-component file ownership, Public API, Allowed Dependencies table
|
|
- `_docs/02_document/components/*/description.md` — six component specs
|
|
- `_docs/00_problem/restrictions.md` — operational constraints
|
|
- `_docs/01_solution/solution.md` — solution overview
|
|
|
|
## Method
|
|
|
|
Per Phase 7 of the code-review skill:
|
|
|
|
1. Mapped all 57 C# files to one of six logical components per `module-layout.md`.
|
|
2. Parsed every `using Azaion.Annotations.*` directive and every constructor-injection / static reference between domain types (`AnnotationService`, `AnnotationEventService`, `FailsafeProducer`, `MediaService`, `DatasetService`, `SettingsService`, `PathResolver`, `AppDataConnection`). Note: `TokenService` and `AuthController` were removed in the auth refactor and are no longer part of the reference graph.
|
|
3. Resolved each cross-file reference against the Allowed Dependencies table (Layer 1 → Layer 2 → Layer 3) and the per-component Public API list.
|
|
4. Applied the five Phase 7 checks: layer direction, Public API respect, cyclic dependencies, duplicate symbols, cross-cutting concerns.
|
|
|
|
## Findings summary
|
|
|
|
| Severity | Count | Categories |
|
|
|----------|-------|------------|
|
|
| Critical | 0 | — |
|
|
| High | 0 | — |
|
|
| Medium | 1 | Architecture |
|
|
| Low | 2 | Architecture, Maintainability |
|
|
|
|
**No new High or Critical findings.** Per the existing-code flow Step 2 auto-chain rule, this allows direct progression to Step 3 (Test Spec).
|
|
|
|
## Findings detail
|
|
|
|
### F1 — `04 dataset` writes directly to the annotation domain (Medium / Architecture)
|
|
|
|
- **Location**: `src/Services/DatasetService.cs:75-94` (`UpdateStatus`, `BulkUpdateStatus`).
|
|
- **Description**: `DatasetService` mutates `db.Annotations.Set(a => a.Status, …)` directly. The `annotations` row is part of `01 annotations-rest`'s domain (per `module-layout.md` → DTO ownership table and component spec). Today this is technically allowed because the only cross-component reference is `AppDataConnection` (a `06_platform` foundation type), but it duplicates ownership of the annotation lifecycle: there are now two paths that mutate `annotations.status` — `AnnotationService.UpdateStatus` (in 01) and `DatasetService.UpdateStatus` / `BulkUpdateStatus` (in 04) — and only the former is wired (post RB-01) into the lifecycle observability contract (SSE + outbox).
|
|
- **Architecture vision impact**: ADR-009 + RB-01 require every mutation to emit lifecycle events. As long as 04 has its own DB write path, that contract cannot be enforced from one place — RB-08 fixes this by routing 04's status writes through `AnnotationService`.
|
|
- **Suggestion**: Track via RB-08; no inline action required at this baseline. Confirms the refactor backlog item is well-grounded.
|
|
- **Module-layout reference**: section "Allowed Dependencies → Rules" — "today: only via shared AppDataConnection in same assembly — acceptable but treat as tight coupling; prefer domain services for new code."
|
|
|
|
### F2 — `ClassesController` bypasses the service layer (Low / Architecture)
|
|
|
|
- **Location**: `src/Controllers/ClassesController.cs:11`.
|
|
- **Description**: `ClassesController` injects `AppDataConnection` directly and queries the `detection_classes` table inline. Every other controller in the codebase (`AnnotationsController`, `MediaController`, `DatasetController`, `SettingsController`) routes through a service. Allowed by the layering rules (06 is a foundation), but inconsistent with the project convention.
|
|
- **Architecture vision impact**: RB-06 introduces admin-managed CRUD for detection classes plus an in-memory cache with `Reset()`. That work will naturally land a `DetectionClassService` (or extend `SettingsService` to own this surface), retiring the direct-DB pattern.
|
|
- **Suggestion**: Defer to RB-06; do not address inline.
|
|
|
|
### F3 — `FailsafeProducer.EnqueueAsync` is a static method that performs DB I/O (Low / Maintainability)
|
|
|
|
- **Location**: `src/Services/FailsafeProducer.cs:195` (the static helper) and the call site `src/Services/AnnotationService.cs:102`.
|
|
- **Description**: `FailsafeProducer.EnqueueAsync(AppDataConnection db, string annotationId, QueueOperation operation)` is a static method on the same type as the hosted-service producer, called from `AnnotationService` to insert a row into `annotations_queue_records`. `coderule.mdc` discourages static methods that touch resources; the user has explicitly accepted this as technical debt during the verification stakeholder review (no RB item — keep as-is).
|
|
- **Architecture impact**: The Public API of `02 annotations-realtime-sync` includes both the running `FailsafeProducer` *and* this static helper; that is documented in `module-layout.md` Component 02. So there is no boundary violation — it is a deliberate API choice the user has confirmed.
|
|
- **Suggestion**: No action. Recorded for future reviewers so the pattern is not flagged again as a finding.
|
|
|
|
## Phase 7 checklist (full traversal)
|
|
|
|
| Check | Result | Notes |
|
|
|-------|--------|-------|
|
|
| 1. Layer direction (Layer 3 → Layer 2 → Layer 1 only) | PASS | All Layer 3 components import only `06` (and `01` additionally `02`'s `AnnotationEventService` + `FailsafeProducer.EnqueueAsync`). No reverse imports detected. |
|
|
| 2. Public API respect | PASS | Every cross-component constructor injection or static call targets a type listed in `module-layout.md` → Public API. No internal-file imports across components. |
|
|
| 3. No cyclic module dependencies | PASS | DI graph: `06 ← {01, 02, 03, 04, 05}`, `02 ← 01`. No cycles. |
|
|
| 4. Duplicate symbols across components | PASS (with F1) | No class/function name collisions. F1 is a *logical* duplication of write authority over `annotations.status`, captured separately. |
|
|
| 5. Cross-cutting concerns not locally re-implemented | PASS | Logging via `ILogger<T>`; auth in `06_platform/Auth`; error envelope in `06_platform/Middleware/ErrorHandlingMiddleware`; config / env reading concentrated in `Program.cs`. No per-component re-implementations. |
|
|
|
|
## Static reference graph (verified)
|
|
|
|
```mermaid
|
|
graph LR
|
|
P06[06 platform]
|
|
P02[02 realtime sync]
|
|
P01[01 annotations rest]
|
|
P03[03 media]
|
|
P04[04 dataset]
|
|
P05[05 settings metadata]
|
|
|
|
P02 --> P06
|
|
P01 --> P06
|
|
P01 --> P02
|
|
P03 --> P06
|
|
P04 --> P06
|
|
P05 --> P06
|
|
```
|
|
|
|
Edges represent constructor-injection or static-call dependencies. `Program.cs` (composition root, in 06) is excluded — it touches everything by definition.
|
|
|
|
## Cross-references
|
|
|
|
- ADR-008 (transactional outbox) and RB-01, RB-03, RB-08 in `_docs/02_document/architecture.md` cover the lifecycle / coupling concerns.
|
|
- `module-layout.md` → "Allowed Dependencies" table is the source of truth for layer membership.
|
|
- `_docs/02_document/04_verification_log.md` documents the stakeholder acceptance of `FailsafeProducer.EnqueueAsync` as tech debt (F3).
|
|
|
|
## Auto-chain decision
|
|
|
|
Per `.cursor/skills/autodev/flows/existing-code.md` → Step 2 auto-chain rule:
|
|
|
|
> If the baseline report contains High or Critical Architecture findings: append to Step 4 testability inputs OR surface to user. If clean (no High/Critical): auto-chain directly to Step 3.
|
|
|
|
This baseline contains **0 Critical, 0 High, 1 Medium, 2 Low** Architecture findings. → **Auto-chain to Step 3 (Test Spec)** without user gate.
|