feat(extensions): W1a per-extension-aggregate-v3 schema migration (swamp-club#211)

[stack72]

Summary

Implements W1a of the extension catalog rearchitecture (swamp-club#211). Pure plumbing — no user-visible behaviour change. Groundwork for #201, not a fix for it.

What lands

• Three new bundle_types columns: state, extension_name, extension_version. Schema-level changes idempotent via the existing pragma_table_info probe pattern.
• Single-transaction data migration with explicit ROLLBACK on post-condition failure. Ordered TS-driven per-row UPDATEs: canonicalize source_path → backfill state from validation_failed → backfill extension_name via deriveExtensionIdentity → drop unmatched rows → verify post-condition (extension_name non-empty). On failure: cold-start rebuild (DELETE all rows, clear populated:* keys, mark migration applied) so loaders repopulate from disk on next access. A migration_applied:per-extension-aggregate-v3 marker key in bundle_meta makes subsequent process restarts skip the data phase.
• Two new helpers under src/infrastructure/persistence/: canonicalizePath (lowercased + forward-slash on Windows; raw on POSIX) and deriveExtensionIdentity (path → {name, version} for pulled, local, source-mounted; null for unmatched).
• BUNDLE_LAYOUT_VERSION bumped to per-extension-aggregate-v3, triggering a one-time model-loader rescan on first run after upgrade. ExtensionCatalogStore.upsert SQL changed from INSERT OR REPLACE to INSERT ... ON CONFLICT(source_path) DO UPDATE SET (legacy columns + state) — extension_name/extension_version intentionally NOT in the SET list so the migration's identity backfill survives the rescan.
• markCatalogValidationFailed migrated from writing validation_failed=true to state='ValidationFailed'. All six readers migrated: five loaders (model:947, driver:538, vault:539, datastore:528, report:386) plus bundle_freshness.ts:303 (the rebundle-loop guard from swamp-club#209). validation_failed column survives W1a as vestigial — no production code reads or writes it. W1b drops the column via SQLite recreate-table pattern.

What does NOT land

• extension_name/extension_version are NOT surfaced on ExtensionTypeRow. Write-once-by-migration (preserved on UPDATE via the ON CONFLICT SET-list exclusion); read by W1b's ExtensionRepository via direct SQL. Loaders' upsert payloads do not include identity columns, so loader rescans cannot clobber them.
• No domain value objects (Extension, Source, RowState, SourceLocation, BundleLocation), no ExtensionRepository, no findRepoRoot helper, no cold-start guard unification, no forceCatalogRescan migration, no validation_failed column drop. All deferred to W1b per the plan.

Why this PR is bigger than the issue's literal "schema migration only" scope

The issue's literal W1a/W1b boundary creates a window where validation_failed is writable by W1a-vintage helpers but no longer trusted by W1b-vintage readers (or vice versa). The only safe boundary is one where the column is genuinely vestigial during the gap: writers migrate to state (this PR), readers migrate to state (this PR), and the drop happens after both have shipped (W1b). Coupling all three changes in one PR is larger; decoupling is unsafe; this carve is the smallest safe carve.

Found-during-implementation corrections from the v5 plan

1. Pulled-extensions paths do NOT encode version. The on-disk layout is .swamp/pulled-extensions/<name>/<kind>/<file> with no version segment; version is owned by upstream_extensions.json (the lockfile) and consulted at read time by W1b's repository fallback (Option A). Migration backfills extension_name only for pulled rows; extension_version stays empty. Post-condition narrowed to check extension_name non-empty only.
2. Source-mounted extensions (swamp extension source add <externalDir>) have absolute paths OUTSIDE repoRoot. The original heuristic only matched <repoRoot>/extensions/<kind>/ and missed them — would have caused W1b's repository fallback to skip+DELETE every source-mounted row. The generalised matcher recognises any **/extensions/<known-kind>/ segment and rolls source-mounted into the same @local/<basename(repoRoot)> aggregate as repo-internal locals.

Pre-work contracts pinned

1. Path canonicalization: lowercased + forward-slash on Windows; raw on POSIX.
2. Repo-root identification: nearest ancestor of the source path containing a .swamp/ directory; first match wins; lexical-only walk (no realpath); innermost wins for nested worktree-in-repo cases. (findRepoRoot helper itself lands in W1b.)
3. Unmatched-row backfill: drop the row; cold-start guard repopulates from disk.
4. Migration ordering: canonicalize source_path FIRST, then backfill identity. Backfill SQL reads source_path; canonicalizing after would match against legacy strings.

Out-of-scope guardrails

• extension rm row pruning → W2
• findStaleFiles algorithm changes / bundleWithCache → W3
• ReconcileFromDisk service → W3
• Loader unification (KindAdapter) → W4
• Per-fingerprint import URLs → W5
• swamp doctor extensions changes → W6
• Two pulled versions of the same extension coexisting on disk (interrupted upgrade) — would correctly trigger I-Repo-1 in W1b; repair belongs to W3's reconcile.
• Windows correctness is NOT a W1 merge gate (parallel workstream).

Reversibility

• W1a revert: rolling back the binary triggers a layout-version mismatch on next start; model loader cold-start guard invalidates populated:model and rebuilds from disk. New columns sit unread by old code. Mild loss: rows broken during the W1a window get re-tried on revert (markCatalogValidationFailed wrote state, not validation_failed; old binary reads validation_failed=0). Not silent corruption — just re-attempted.
• W1b revert (future): forward-only. Reverting the binary after W1b drops validation_failed leaves the schema without the column; old loaders that read validation_failed see undefined → false → broken rows leak through. Revert path requires deleting _extension_catalog.db (manual ops or via forceCatalogRescan-equivalent).

Plan record divergence

The @swamp/issue-lifecycle model record has plan v5; this PR ships v6 (Option A + source-mounted handling). The lifecycle model has no revise_plan method post-approve, so the canonical record is stale. Documentation chain: issue spec → v5 (approved) → discovered constraint during implementation → v6 (this PR).

Test plan

• deno fmt --check passes
• deno lint passes
• deno check src/ passes
• deno run test — 5270 passed, 0 failed, 28 ignored
• deno run compile succeeds
• swamp-uat uat:cli against the W1a-compiled binary — 388 passed, 0 failed, 2 ignored (57s)
• swamp-uat uat:adversarial against the W1a-compiled binary — 92 passed, 0 failed, 1 ignored (3m9s) — covers state corruption, concurrency, resource exhaustion, process lifecycle, security tiers
• Manual smoke test docs: add README with project logo #1: fresh repo + swamp extension pull @swamp/aws/s3 + swamp extension pull @swamp/digitalocean (43 pulled catalog rows). W1a binary migrates cleanly: bundle_layout bumps per-extension-v2 → per-extension-aggregate-v3, all 43 rows preserved with extension_name correctly grouped (10 + 33), extension_version empty per Option A, state='Indexed'. Idempotent on second run.
• Manual smoke test feat: implement model domain layer with Echo reference model #2: fresh repo + swamp extension source add /tmp/external-srcdir (source-mounted). W1a binary migrates correctly: source-mounted row resolves to @local/<repo>/0.0.0, no rows dropped, no infinite churn.
• ADV-V3-1 canary test in suite: explicit assertion that loader rescan after layout-version bump does NOT clobber migration-backfilled extension_name/extension_version. Catches future regression to INSERT OR REPLACE.
• fix: Provide default dataOutputSpecs for user models #209 regression canary in suite: findStaleFiles + ValidationFailed → not stale test passes under new state semantics.
• Cold-start rebuild path test in suite (called via private-method cast).
• Mixed pulled + local + source-mounted + unmatched in single migration test.

🤖 Generated with Claude Code

[github-actions]

Code Review

Thorough review of all 14 files (1699 additions, 111 deletions). This PR implements W1a of the extension catalog rearchitecture — schema migration, data backfill, and reader/writer migration from validation_failed to state.

Blocking Issues

None.

Suggestions

1. state typed as bare string on ExtensionTypeRow (extension_catalog_store.ts:102) — a string literal union ('Indexed' | 'ValidationFailed' | ...) would catch typos at compile time. Acknowledged this is explicitly deferred to W1b when all 7 tags land; just flagging it as the natural follow-up.

2. Identical multi-line comments across 5 loaders — the // Skip ValidationFailed rows (swamp-club#209) — see equivalent guard in user_model_loader.ts:registerLazyFromCatalog. Migrated to read state instead of validation_failed per W1a. comment is repeated verbatim in user_datastore_loader.ts, user_driver_loader.ts, user_vault_loader.ts, user_report_loader.ts, and user_model_loader.ts. The CLAUDE.md convention is "default to writing no comments" — a single-line // Skip schema-broken rows (swamp-club#209) would suffice given the code is self-documenting (entry.state === "ValidationFailed").

What looks good

• SQL injection safety: All SQL uses parameterized queries with ? bindings — no string interpolation in SQL anywhere.
• Transaction design: Single-transaction migration with explicit ROLLBACK + cold-start rebuild fallback is robust. The recovery path is well-tested.
• ON CONFLICT preservation (ADV-V3-1): The critical design decision — excluding extension_name/extension_version from the UPDATE SET list — is correct and backed by the canary test at extension_catalog_store_test.ts:1227.
• Cross-platform handling: canonicalizePath is properly parameterized for testing both POSIX/Windows branches without host dependency. Tests use canonicalizePath() in lookup keys so assertions pass on both platforms.
• Test coverage: 40+ new test cases covering every migration sub-step, edge cases (duplicate rows on Windows, zero-length extension names, source-mounted external dirs, corrupt paths), and the cold-start rebuild recovery path.
• DDD alignment: New helpers are correctly placed in infrastructure/persistence. ExtensionIdentity is a proper value object (readonly, equality by value). Domain loaders only changed their filter predicate — no persistence concerns leaked into the domain layer.
• Libswamp import boundary: No violations. The new helpers are only imported within the infrastructure layer and their own tests.
• License headers: Present on all 4 new files.
• Migration idempotency: Schema changes via pragma_table_info probes; data migration gated by marker key in bundle_meta. Re-opening the catalog after migration is verified as a no-op in multiple tests.

[github-actions]

Adversarial Review

Reviewed all 14 changed files (1699 additions, 111 deletions). Traced every code path through the migration transaction, the ON CONFLICT upsert pattern, the path canonicalization, the identity derivation heuristic, the cold-start rebuild recovery, and the six reader-site migrations from validation_failed to state.

Critical / High

None found.

Medium

1. extractPulledExtensionName greedy first-match on kind segments (src/infrastructure/persistence/derive_extension_identity.ts:169-178): The function scans left-to-right for the first path segment matching a known kind name (models, vaults, etc.). An extension whose scoped name contains a kind-like segment — e.g., @scope/models/foo with on-disk path .swamp/pulled-extensions/@scope/models/foo/models/x.ts — would be misparsed as @scope (stopping at the first models) instead of @scope/models/foo. This is inherent to the path-based approach (the on-disk layout is ambiguous for such names), but worth documenting as a known limitation. In practice the extension registry controls namespace conventions and no existing extensions use kind-like segments in their names, so this is a warning, not a blocker.

Low

1. state column typed as bare string (src/infrastructure/persistence/extension_catalog_store.ts:102): The state field on ExtensionTypeRow is string rather than a discriminated union. A typo like "validationfailed" (wrong case) would silently pass the === "ValidationFailed" checks and leak a broken row into the registry. This is accepted W1a scope per the PR description (W1b introduces proper value objects), but a RowState string literal union would catch this class of bug at compile time today.

2. FakeCatalog.upsert stale parameter type (src/domain/extensions/bundle_freshness_test.ts:51): The test fixture still declares validation_failed: boolean in its upsert signature rather than state?: string. This compiles due to TypeScript's bivariant method parameter checking and works at runtime because of the as unknown as ExtensionTypeRow cast. No test is affected (the FakeCatalog is only used with findStaleFiles which reads, never writes via markCatalogValidationFailed), but it's a vestigial type that could confuse future maintainers.

3. extractPulledExtensionName with trailing-slash-only input (src/infrastructure/persistence/derive_extension_identity.ts:169): extractPulledExtensionName("") (empty nameAndRest after prefix strip) produces parts = [""], no kind match, returns null — correct. extractPulledExtensionName("/") produces parts = ["", ""], no kind match, returns null — also correct. Edge cases handled.

What I verified is correct

• Transaction discipline: runDataMigrationTransaction uses BEGIN/ROLLBACK/COMMIT correctly. The ROLLBACK on error leaves the schema columns in place (they were added outside the transaction) while reverting all row changes. The subsequent runColdStartRebuild opens a clean transaction.
• ON CONFLICT preservation (ADV-V3-1): extension_name and extension_version are excluded from the UPDATE SET list, so loader rescans cannot clobber migration-backfilled identity. The canary test at line 1227 pins this.
• Migration ordering: canonicalizeAllSourcePaths runs before backfillExtensionIdentity, so the identity derivation matches against canonical paths. The repoRoot is also canonicalized.
• Duplicate handling in canonicalization: When two rows canonicalize to the same PK, the UNIQUE constraint failure is caught, the duplicate is deleted, and the survivor proceeds through identity backfill.
• Cold-start rebuild: Correctly deletes all rows, clears populated:* keys, and marks migration as applied. The marker prevents infinite retry loops.
• Reader migration consistency: All six reader sites (five loaders + bundle_freshness.ts) now check entry.state === "ValidationFailed" instead of entry.validation_failed. The writer (markCatalogValidationFailed) writes state: "ValidationFailed". No writer/reader schism during W1a → W1b.
• joinForward avoids @std/path/join: Correctly uses forward slashes for cross-platform consistency with canonicalized paths.

Verdict

PASS — No blocking issues. The migration logic is well-structured with proper error handling, recovery paths, and comprehensive test coverage (including the architect-specified ADV-V3-1 canary). The medium finding about kind-segment ambiguity is a known design limitation, not a code defect.