fix(extensions): preserve catalog fingerprint when bundle build fails (swamp-club#265)

[stack72]

Summary

• When bundleWithCache returns a stale cached bundle (via isExpectedBundleFailure fast-path or error fallback), rebundleAndUpdateCatalog now preserves the catalog's stored source_fingerprint instead of writing the new one. This prevents permanent fingerprint poisoning where findStaleFiles sees matching fingerprints and never retries the bundle.
• Adds kind-agnostic BundleResult type to bundle_freshness.ts with { js, fromCache } — shaped for W4's KindAdapter to absorb when it consolidates the loaders.
• Emits a structured warning ("source fingerprint preserved, will retry on next command") only on the fallback case (fromCache && fingerprint differs), not on legitimate cache hits.
• Applied mechanically across all 5 extension loaders (models, drivers, vaults, datastores, reports).
• bundleAndIndexOne surfaces fromCache as an additive return field — no caller changes needed (reconcile and install services create Sources independently).
• findStaleFiles signature unchanged (W2's RemoveExtensionService safety-net preserved).
• Filed deferred tracker issues: swamp-club#270 (warm-start state oscillation debt) and swamp-club#271 (sourceToRow empty mtime).

Test Plan

• Added warm-start regression integration test (integration/source_extension_rebundle_test.ts) with three assertion groups:
  • (a) Happy path: source-mounted extension primed with V1, modified to V2 with bare specifiers
  • (b) Permanent-failure determinism: two consecutive warm-starts assert identical catalog fingerprint and warning emitted both times
  • (c) Warning-log precision: assert warning fires for fallback case, assert NO warning for unchanged-source warm-start
• Verified before/after against compiled binary vs swamp on PATH:
  • Before: fingerprint poisoned (faa89ac09cda → 69ead2141011), bundle stale, no warning, permanently stuck
  • After: fingerprint preserved (faa89ac09cda), bundle stale but retryable, warning emitted, deterministic
• All 5562 existing tests pass, 0 failures
• Existing bundle_cache_freshness_test.ts (issue Issue 10: Create data migration tool #125 regression net) passes

🤖 Generated with Claude Code

[github-actions]

Code Review

Blocking Issues

None.

Suggestions

1. No unit test for findBySourcePath (src/infrastructure/persistence/extension_catalog_store.ts:695): The new public method on ExtensionCatalogStore has no dedicated unit test. It is exercised by the integration test, and it's a simple PK lookup, but the existing test file has tests for the sibling methods removeBySourcePath and findByKind — adding a test for consistency would be cheap.

2. Early-load warning may be noisy (all 5 loaders, e.g. user_model_loader.ts:440-442): In the loadModels/loadAll cold-start path, the fromCache warning fires on every cache fallback, including legitimate hits where the source hasn't changed. The rebundleAndUpdateCatalog path correctly distinguishes these cases (only warns when fingerprints differ), but the cold-start path lacks catalog access to make the same distinction. This could cause warn-level noise on every invocation for extensions with bare specifiers. Worth considering gating the cold-start warning on some condition in W4's loader consolidation.

3. Duplicated fingerprint preservation logic: The ~15-line effectiveFingerprint block is copy-pasted across all 5 loaders' rebundleAndUpdateCatalog methods. The PR description correctly notes this is intentional temporary duplication pending W4's KindAdapter consolidation — just flagging it to ensure it's on the W4 radar.

Overall

Solid bug fix. The root cause analysis is clear — writing the new source fingerprint alongside a stale cached bundle permanently masked the staleness from findStaleFiles. The fix correctly preserves the catalog's stored fingerprint when bundleWithCache returns a cache fallback, ensuring retry on the next warm-start. The BundleResult type is well-placed in the domain layer (bundle_freshness.ts) as a value object. The integration test covers the three critical scenarios (unchanged source → no warning, failed rebuild → warning + fingerprint preserved, second retry → deterministic). Design doc updated. No import boundary violations, no security concerns, correct LogTape interpolation.

[github-actions]

Adversarial Review

Critical / High

None found.

Medium

1. First-run catalog bootstrap still writes new fingerprint with stale cache (src/domain/models/user_model_loader.ts:1064-1079)

   The fingerprint preservation logic is correctly applied in all 5 loaders' rebundleAndUpdateCatalog methods (warm-start path). However, populateCatalogFromDir (line 1064-1079) — the first-run catalog bootstrap path that runs when the catalog is empty — unconditionally computes and writes the current source fingerprint even when the bundle is stale from cache.

   Breaking scenario: User deletes the .swamp database (or first initializes a repo that already has cached .js bundles from a prior environment). Source has since changed to use bare specifiers. loadModels → bundleWithCache → returns stale cache (fromCache: true). The warning fires (line 440-442) but nothing preserves the old fingerprint. Then populateCatalogFromDir writes the new fingerprint → poisoned. Subsequent warm-starts see matching fingerprints and never retry.

   This is the same class of bug as #265 on a less-common path. The warm-start fix in this PR is the right priority since it covers the issue's primary scenario. Noting this as a gap for awareness — the populateCatalogFromDir and ReconcileFromDiskService (which recomputes fingerprints at line 376) paths would need similar treatment for complete coverage.

2. loadModels/loadDrivers/etc. initial-load warning fires on legitimate cache hits (src/domain/models/user_model_loader.ts:440-442)

   In the initial full-load path, the warning Using cached bundle for ${file} — source may have changed but bundle could not be regenerated fires unconditionally whenever fromCache is true — even when the source hasn't changed and the cached bundle is perfectly valid (e.g., source-mounted extension with bare specifiers that was bundled externally, nothing modified since). The rebundleAndUpdateCatalog path correctly gates its warning on existing.source_fingerprint !== sourceFingerprint, but the initial-load path lacks this guard because there's no catalog to compare against yet.

   This produces false-alarm warnings on every cold start with source-mounted extensions that use bare specifiers, which could cause user confusion or alert fatigue.

Low

1. bundleAndIndexOne returns fromCache but no caller uses it (src/domain/models/user_model_loader.ts:1158)

   The PR adds fromCache to the return type of bundleAndIndexOne across all 5 loaders, but neither ReconcileFromDiskService (line 369 in reconcile_from_disk_service.ts) nor InstallExtensionService (line 239 in install_extension_service.ts) reads it. The field is additive and doesn't break anything, but it's dead data in the current state. The PR description notes this is intentional ("no caller changes needed"), which is fine — just noting it's unused.

2. existing?.source_fingerprint truthy guard skips empty-string fingerprints (src/domain/vaults/user_vault_loader.ts:821 and all loaders)

   if (existing?.source_fingerprint) evaluates to false when source_fingerprint is "" (empty string), which is the default for rows created without a computed fingerprint. In that case, the preservation logic is skipped and the new fingerprint is written. This is arguably correct behavior (nothing to preserve), but it means a row that was bootstrapped with an empty fingerprint won't get the protection. In practice, all current catalog write sites compute a fingerprint before writing, so this is a theoretical-only gap.

Verdict

PASS — The core fix is sound: rebundleAndUpdateCatalog correctly preserves the stored fingerprint when bundleWithCache returns stale cache, preventing the permanent fingerprint poisoning described in #265. The BundleResult type is clean, the findBySourcePath SQL is parameterized and uses the existing mapRow pattern, and the integration test covers the key warm-start scenarios with proper assertions. The medium findings are gaps in adjacent code paths, not errors in the changed code.