fix: use >= for bundle cache mtime comparison to handle equal timestamps#1155
fix: use >= for bundle cache mtime comparison to handle equal timestamps#1155
Conversation
…mps (#38) The bundle cache fallback used strict > when comparing bundle mtime against source mtime. When both files share the same mtime (e.g. written in the same second during a pre-commit hook), the cache was skipped and a rebundle attempted — which fails when imports are missing. Changed to >= across all 5 extension loaders so equal mtimes correctly use the cached bundle. Closes systeminit/swamp-club#38 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
Clean, well-scoped bug fix. The > → >= change is correct: when source and bundle share the same mtime (e.g., both written in the same second during a pre-commit hook), the cached bundle should be used rather than triggering a rebundle that may fail.
Blocking Issues
None.
Suggestions
- The comment at
src/domain/models/user_model_loader.ts:1138("If the bundle is newer than all source files, use it directly") is now slightly imprecise — could say "newer than or equal to" to match the>=operator. Very minor; not blocking.
Verified:
- All 5 loader files have the identical
>=change applied consistently - New unit test properly validates the equal-mtime scenario with assertions on both content and mtime preservation
- Test follows project conventions (naming, assertions, temp dir cleanup)
- No import boundary violations — all changes are within domain-internal code
- No DDD or security concerns
LGTM ✅
There was a problem hiding this comment.
Adversarial Review
Critical / High
None.
Medium
None.
Low
- Theoretical stale-cache window with
>=(all 5 loaders, e.g.user_model_loader.ts:1159)
With>, if a source file is modified in the same filesystem time quantum as the existing bundle's mtime (e.g., same second on ext4), the old>check would correctly fall through to rebundle. The new>=would serve the stale cache. Example: bundle was written at time T, then a user edits the source within the same second —bundle.mtime (T) >= source.mtime (T)→ stale cache used.
Why this is LOW, not higher: (a) The scenario requires a source edit landing in the exact same second as a prior bundle write, which is far less likely than the pre-commit hook scenario this PR fixes. (b) The fallback path (catch (bundleError) { if (bundleExists) ...}) already serves the stale cache on rebundle failure anyway, so the blast radius is limited. (c) This is the same tradeoffmakehas used for decades. (d) The next source edit with a different mtime will correct it.
No action needed — just documenting the tradeoff.
Verdict
PASS — Minimal, correct fix. The > to >= change across all 5 extension loaders is semantically sound for the target scenario (source and bundle written in the same second during a pre-commit hook). The test directly validates the equal-mtime case with Deno.utime and confirms no rebundle occurs. All changes are mechanically consistent. The theoretical stale-cache window introduced by >= is a reasonable tradeoff.
Summary
>to>=in all 5 user extension loaders (models, reports, datastores, vaults, drivers)Closes systeminit/swamp-club#38
Test Plan
bundleWithCache uses cache when source and bundle have equal mtimes— sets source and bundle to identical mtime viaDeno.utime, verifies cache is used without rebundlinguser_model_loader_test.tstests pass/tmp/swamp-repro-issue-38— with equal mtimes the compiled binary now logsUsing cached bundleinstead of rebundlingdeno check,deno lint,deno fmtall pass🤖 Generated with Claude Code