fix(extensions): exclude regenerable bundles from auto-resolve truncation check#1199
Merged
fix(extensions): exclude regenerable bundles from auto-resolve truncation check#1199
Conversation
…tion check The tri-state inspectInstallation introduced in #1197 stat'd every path in the lockfile's file list, which includes .swamp/bundles/** and the sibling vault/driver/datastore/report bundle caches. Clearing those caches (a normal hygiene operation) flipped the resolver to the truncated branch and emitted "incomplete — missing N file(s)" instead of the alreadyInstalledButFailed path that protects user WIP (#121). Filter bundle prefixes out of the stat loop so only source-tree files under .swamp/pulled-extensions/<name>/ drive truncation. Bundle prefixes are derived from SWAMP_SUBDIRS so a future bundle-dir addition stays in sync. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
There was a problem hiding this comment.
Code Review
Blocking Issues
None.
Suggestions
None — this is a well-executed fix.
Summary: The change correctly scopes the inspectInstallation truncation predicate to source files only, excluding regenerable bundle artifacts under .swamp/{bundles,vault-bundles,driver-bundles,datastore-bundles,report-bundles}/. This prevents clearing the bundle cache from incorrectly flipping an intact extension into the truncated branch, which would steal the user-WIP error path from #121.
What I checked:
- Correctness:
BUNDLE_ARTIFACT_PREFIXESis derived fromSWAMP_SUBDIRSconstants, so it stays in sync automatically. The string prefix check is valid because lockfile paths are POSIX-normalized (forward slashes). The single-linecontinuein the stat loop is the minimal change needed. - Import boundary:
SWAMP_SUBDIRSis imported frominfrastructure/persistence/paths.ts(not a libswamp internal path) — matches the existing import already in the file, no boundary violation. - DDD: The bundle-vs-source distinction is an adapter-level concern correctly placed in the CLI adapter, not leaked into the domain's
InstallationInspectiontype. - Test coverage: Two new regression tests cover both key scenarios — all 5 bundle subdirs excluded (stays intact) and mixed missing source + missing bundles (only source drives truncation). Good use of the existing
seedLockfile/seedPulledTreehelpers. - License headers: Present on both
.tsfiles. Noanytypes. All promises awaited. Named exports used. - Design doc: Updated to explain the rationale — accurately matches the implementation.
- Security: No new attack surface. Bundle prefixes are derived from constants, not user input.
Clean fix with good test coverage and documentation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The tri-state
inspectInstallationintroduced in #1197 stats every path in the lockfile's file list to detect a truncated install. That list includes compiled bundles under.swamp/bundles/**,.swamp/vault-bundles/**,.swamp/driver-bundles/**,.swamp/datastore-bundles/**, and.swamp/report-bundles/**— regenerable build artifacts, not source.When a user clears any of those caches (normal hygiene, stale rebuild, fresh worktree), auto-resolve flipped from
intact→truncatedand emitted:…instead of the
alreadyInstalledButFailed("already installed at … run--forceto recover") path that protects user WIP per #121. The motivating swamp-club#133 symptoms are all source-tree artifacts (manifest, kind subdirs, declared source), so the fix is to scope the truncation predicate to source files only.This filters bundle prefixes out of the stat loop in
inspectInstallation. Prefixes are derived fromSWAMP_SUBDIRS.{bundles, vaultBundles, driverBundles, datastoreBundles, reportBundles}so a future bundle-dir addition stays in sync. No filesystem mutation, no API/event/wire-format changes.Test Plan
deno fmt --check,deno lint,deno checkcleansrc/cli/auto_resolver_adapters_test.ts— 18/18 pass, including 2 new regression cases:inspectInstallation ignores absent bundle artifacts and stays intact(covers all 5 bundle subdirs)inspectInstallation reports truncated only for missing source, not missing bundlesintegration/auto_resolver_truncated_test.ts— passes (fix(extensions): detect truncated pulled-extension trees in auto-resolve (swamp-club#133) #1197/feat: add Definition entity with CEL expression support (#117) #133 source-truncation behaviour preserved)integration/auto_resolver_no_clobber_test.ts— passes (Issue 6: Migrate echo model to new architecture #121 no-clobber invariant preserved)deno run compilesucceededswamp extension pull @stack72/[email protected]→ edit pulled.tswith WIP marker + syntax error →rm -rf .swamp/bundles→swamp model create @stack72/system-usage instance1now produces:incomplete — missing 1 file(s): ".swamp/bundles/91139983/system_usage.js".tests/cli/extension/pull_wip_preservation_test.ts:156in swamp-uat (CI gate — local run blocked by membership auto-trust collision unrelated to this fix)Cross-references