refactor(domain): lift UnifiedDataRepository port into domain layer (swamp-club#229)

[stack72]

Summary

Resolves swamp-club#229. The DDD dependency-inversion fix for the data services added in swamp-club#181 — the UnifiedDataRepository interface, OwnershipValidationError class, and GarbageCollectionResult type now live in the domain layer (src/domain/data/repositories.ts), and the infrastructure file imports them back and re-exports for non-domain consumers.

The issue body scoped this to the two new data services (rename, delete) and a −2 ratchet decrement. After review we expanded scope to include all production-code domain importers of the interface — the move is mechanical and the canonical-source correctness is worth the slightly larger blast radius. Net ratchet decrement: 31 → 22 (−9).

What changed

• New src/domain/data/repositories.ts — the lifted interface, exception, and result type. Naming matches the existing src/domain/definitions/repositories.ts convention.
• New src/infrastructure/persistence/unified_data_repository_re_export_test.ts — locks the value re-export contract: asserts OwnershipValidationError class identity is preserved across both import paths and that instanceof works from either side. Guards against a future regression that converts the value re-export to a type-only one or duplicates the class definition.
• Modified src/infrastructure/persistence/unified_data_repository.ts — imports the moved symbols from domain and re-exports them. FileSystemUnifiedDataRepository keeps implements UnifiedDataRepository against the now-domain-side interface. Drops 354 lines of moved declarations.
• Modified src/domain/data/data_rename_service.ts and data_delete_service.ts — switch to domain UnifiedDataRepository and DefinitionRepository. The constructor parameter type changes from concrete YamlDefinitionRepository to the abstract DefinitionRepository; findDefinitionByIdOrName already accepts the interface so no other call-site changes are needed.
• Modified 11 expanded-scope production importers and 10 domain test files — each is a single-line import-path change for the UnifiedDataRepository interface. Test files are excluded from the DDD ratchet (skip pattern), so they don't affect the count, but updating them keeps the canonical source consistent.
• Modified integration/ddd_layer_rules_test.ts — KNOWN_DOMAIN_INFRA_VIOLATIONS 31 → 22 and removed the now-resolved swamp-club#229 tracking comment.

Out of scope

• Files that retain other infrastructure imports (CatalogStore, CatalogRow, YamlOutputRepository, the concrete FileSystemUnifiedDataRepository class) update their UnifiedDataRepository import path for canonical-source correctness but stay on the violations list. Those are deeper refactors that need an injection point at the construction call-site.
• Other consumers (cli, libswamp, integration tests, infrastructure tests) keep importing from the infra path and continue to work via the re-export. The infra re-export should stay until those layers are also tidied — likely a separate PR.

Note for reviewers

src/domain/workflows/execution_service.ts ends up with two import lines targeting the infra/domain split: import type { UnifiedDataRepository } from "../data/repositories.ts" (interface from domain) and import { FileSystemUnifiedDataRepository } from "../../infrastructure/persistence/unified_data_repository.ts" (concrete class from infra). This is intentional — interface and implementation belong on opposite sides of the boundary.

The DDD ratchet baseline of 31 was current at branch time. If another PR lands first and bumps the count, the target moves to (new baseline − 9). The −9 delta is what this PR delivers.

Test Plan

• deno fmt --check, deno lint, deno check all clean
• deno run test integration/ddd_layer_rules_test.ts — passes; reports 22 violations
• deno run test src/infrastructure/persistence/unified_data_repository_re_export_test.ts — class identity and instanceof both verified
• deno run test integration/data_ownership_test.ts integration/unified_data_test.ts — 27/27 pass; confirms instanceof OwnershipValidationError still works through the infra re-export
• deno run test integration/data_delete_test.ts — 4/4 pass; exercises the rebuilt DataDeleteService with the DefinitionRepository constructor type
• All data-touching integration tests (76/76 pass): versioning, CEL access, cross-model, expressions, tagging, output specs, vary, workflow query
• Full suite: 5385/5385 pass, 0 failed
• deno run compile succeeds
• Compiled binary smoke-tested: swamp repo init, data list, data delete, data versions, data rename, data gc, workflow run, doctor audit — all parse, route through the changed services, and return clean JSON
• Byte-diff: moved interface block is identical to the original (only difference is one extraction-boundary blank line)
• Audit: no missed dynamic imports or barrel re-exports; extensions/ does not reference these types

[github-actions]

Code Review

Clean, well-structured dependency-inversion refactor. The PR correctly lifts the UnifiedDataRepository interface, OwnershipValidationError class, and GarbageCollectionResult type from the infrastructure layer into src/domain/data/repositories.ts, following the established repositories.ts convention used by definitions, models, workflows, and telemetry.

Blocking Issues

None.

Suggestions

None — this is a textbook DDD port extraction:

• Domain ownership is correct: The interface (port) lives in the domain; the implementation stays in infrastructure. The FileSystemUnifiedDataRepository keeps its implements clause against the domain-side interface.
• Re-export contract is locked: The infra file re-exports the moved symbols for backward compatibility, and the new unified_data_repository_re_export_test.ts verifies class identity and instanceof across both import paths — a thoughtful guard against a future regression to type-only re-exports.
• Constructor type narrowing is safe: DataDeleteService and DataRenameService now accept the abstract DefinitionRepository instead of the concrete YamlDefinitionRepository. findDefinitionByIdOrName already accepts the interface, and the call sites in libswamp pass YamlDefinitionRepository (which implements the interface), so this is a sound widening.
• Ratchet math checks out: 9 files where UnifiedDataRepository was the sole infrastructure import now import from the domain layer. The remaining 22 violations retain other infra imports (CatalogStore, CatalogRow, YamlOutputRepository, FileSystemUnifiedDataRepository) — correctly left for future PRs.
• No libswamp import boundary violations: CLI and libswamp code continue to import these types through the infra re-export path, not from the new domain internal path.
• License headers present on both new files.
• Naming matches convention: src/domain/data/repositories.ts aligns with src/domain/definitions/repositories.ts, src/domain/models/repositories.ts, etc.

[github-actions]

Adversarial Review

Critical / High

None found.

Medium

None found.

Low

1. src/domain/data/data_query_service.ts:97-100 — This file still imports CatalogRow and CatalogStore from the infra layer (../../infrastructure/persistence/catalog_store.ts). The PR description explicitly calls this out as intentionally out of scope, but it's worth noting that the domain repositories.ts file could eventually absorb a CatalogStore port too. No action needed now — just confirming the tech debt is tracked.

2. integration/ddd_layer_rules_test.ts:84 — The ratchet constant KNOWN_DOMAIN_INFRA_VIOLATIONS = 22 is correct for this PR, but the comment above it still says "added 4 transitional domain→infrastructure imports" referencing issue #223. If another PR in flight also changes the count, whoever merges second will get a test failure. This is the existing ratchet design working as intended — not a bug. Mentioning because the PR body itself flags this possibility.

Verdict

PASS — This is a textbook dependency-inversion refactor. The UnifiedDataRepository interface, OwnershipValidationError class, and GarbageCollectionResult type move cleanly from infrastructure to domain with zero behavioral change. Key things verified:

• The new src/domain/data/repositories.ts is a byte-identical lift of the declarations from the infra file (confirmed via diff).
• The infra module correctly re-exports OwnershipValidationError as a value re-export (not type-only), preserving instanceof identity across both import paths. The new unified_data_repository_re_export_test.ts locks this contract.
• DataDeleteService and DataRenameService constructor types widen from concrete YamlDefinitionRepository to abstract DefinitionRepository. All 4 instantiation sites (2 production in libswamp, 2 in tests) pass compatible arguments. findDefinitionByIdOrName only uses methods defined on the DefinitionRepository interface (findByNameGlobal, findById), so no hidden method-resolution failures.
• The DDD ratchet count of 22 was independently verified by enumerating all remaining domain→infra imports (excluding logging/tracing cross-cutting concerns).
• All 27 changed files are mechanical single-line import-path swaps with no logic changes.