Protect pre-existing user files from copyPackFile overwrite#300
Merged
Protect pre-existing user files from copyPackFile overwrite#300
Conversation
bguidolim
changed the title
- Add filesystem-aware collision resolver: hooks always namespace into <pack-id>/ subdirectories; other types namespace only when a pre-existing untracked file is detected at the destination - Centralize tracked-files computation and namespacing rule into shared helpers (PackArtifactRecord.allTrackedFiles, namespacedDestination) - Add unit tests (HookAlwaysNamespaceTests, UserFileConflictTests) and integration tests (UserFileProtectionTests) covering all scenarios
bguidolim
force-pushed
the
bruno/300-protect-preexisting-user-files
branch
from
9328ce5 to
4548eec
Compare
…test - Align phase numbering in DestinationCollisionResolver comments (Phase 0/1a/1b/2) - Remove default nil from SyncStrategy.makeCollisionContext to enforce implementation - Add .generic file type user-conflict test in DestinationCollisionResolverTests
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
1 participant
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
When
mcs syncinstalls a pack'scopyPackFilecomponent, it previously overwrote any existing file at the destination — including files the user created manually. This PR adds filesystem awareness to theDestinationCollisionResolverso pre-existing user files are never touched: hooks from packs are always namespaced into<pack-id>/subdirectories, and other file types (commands, skills, agents) are namespaced only when a conflict with an untracked file is detected.Changes
CollisionFilesystemContextprotocol +ProjectCollisionContext/GlobalCollisionContextconcrete types — enable the resolver to check filesystem state and distinguish mcs-tracked files from user filesnamespacedDestinationhelper centralizes the skill-suffix vs subdirectory-prefix namespacing rule in one placePackArtifactRecord.allTrackedFiles(from:)shared static method ensures sync and doctor compute tracked files identicallySyncStrategy.makeCollisionContextandCheckScope.makeCollisionContextprovide scope-appropriate context to the resolver from both sync and doctor pathsConfigurator.configure()/dryRun()reordered: state loaded before collision resolution so tracked files are availableDoctorRunnerpasses filesystem context per scope for consistent namespaced-path doctor checksHookAlwaysNamespaceTests(4 tests),UserFileConflictTests(5 tests) withMockCollisionContextUserFileProtectionTests(4 tests) — pre-existing hook/command preserved, tracked file not false-positive, hook always namespacedLifecycleIntegrationTests,GlobalConfiguratorTests,ProjectConfiguratorTestsupdated for always-namespaced hooksTest plan
swift testpasses locally (889 tests, 0 failures)swiftformat --lint .andswiftlintpass without violationsmcs sync,mcs doctor)Checklist for engine changes
LifecycleIntegrationTests—UserFileProtectionTests, updatedCrossPackCollisionTests)