Fix repair logic

Author: jribbink

internal/dependencymanager/dependencyinstaller.go

@@ -716,11 +716,16 @@ func (di *DependencyInstaller) handleFoundContract(dependency config.Dependency,
 		}
 	}
 
-	// Install or update the dependency
-	// This is the shared installation path for:
-	// - New dependencies (no hash mismatch)
-	// - Hash mismatch with --skip-update-prompts and no local file
-	// - Hash mismatch with --update flag
+	// Check if file exists and needs repair (out of sync with flow.json)
+	fileExists := di.contractFileExists(contractAddr, contractName)
+	fileModified := false
+	if fileExists {
+		if err := di.verifyLocalFileIntegrity(contractAddr, contractName, contractDataHash); err != nil {
+			fileModified = true
+		}
+	}
+
+	// Install or update: new deps, out-of-sync files, or network updates with --update/--skip-update-prompts
 	isNewDep := di.State.Dependencies().ByName(dependency.Name) == nil
 
 	err := di.updateDependencyState(dependency, contractDataHash)
@@ -729,26 +734,27 @@ func (di *DependencyInstaller) handleFoundContract(dependency config.Dependency,
 		return err
 	}
 
-	// Log if this was an auto-update
-	if hashMismatch && di.Update {
+	// Log if this was an auto-update (with --update flag) or file repair
+	if (hashMismatch || fileModified) && di.Update {
 		msg := util.MessageWithEmojiPrefix("✅", fmt.Sprintf("%s updated to latest version", dependency.Name))
 		di.logs.stateUpdates = append(di.logs.stateUpdates, msg)
+	} else if fileModified {
+		// File repair without --update flag (common after git clone)
+		msg := util.MessageWithEmojiPrefix("✅", fmt.Sprintf("%s synced", dependency.Name))
+		di.logs.stateUpdates = append(di.logs.stateUpdates, msg)
 	}
 
 	// Handle additional tasks for new dependencies or when contract file doesn't exist
-	if isNewDep || !di.contractFileExists(contractAddr, contractName) {
+	if isNewDep || !fileExists {
 		err := di.handleAdditionalDependencyTasks(networkName, dependency.Name)
 		if err != nil {
 			di.Logger.Error(fmt.Sprintf("Error handling additional dependency tasks: %v", err))
 			return err
 		}
 	}
 
-	// Handle file creation/update
-	fileExists := di.contractFileExists(contractAddr, contractName)
-	forceOverwrite := hashMismatch && di.Update
-
-	if !fileExists || forceOverwrite {
+	// Create or overwrite file
+	if !fileExists || fileModified || (hashMismatch && di.Update) {
 		err = di.createContractFile(contractAddr, contractName, contractData)
 		if err != nil {
 			return fmt.Errorf("error creating contract file: %w", err)
@@ -760,14 +766,6 @@ func (di *DependencyInstaller) handleFoundContract(dependency config.Dependency,
 		}
 	}
 
-	// Verify local file integrity matches stored hash (if file exists and we didn't just overwrite it)
-	if fileExists && !forceOverwrite {
-		err = di.verifyLocalFileIntegrity(contractAddr, contractName, contractDataHash)
-		if err != nil {
-			return err
-		}
-	}
-
 	return nil
 }
 

internal/dependencymanager/dependencyinstaller_test.go

@@ -750,9 +750,9 @@ func TestDependencyInstallerInstallFromFreshClone(t *testing.T) {
 		assert.Empty(t, di.logs.stateUpdates, "Should have no state updates")
 	})
 
-	t.Run("Already installed, up-to-date hash BUT modified local file", func(t *testing.T) {
-		// This is the CRITICAL test case: network hash matches flow.json hash,
-		// but local file has been tampered with. This should FAIL.
+	t.Run("Already installed, up-to-date hash BUT modified local file - user repairs", func(t *testing.T) {
+		// Network hash matches flow.json hash, but local file has been tampered with
+		// Should auto-repair WITHOUT prompting (flow.json is source of truth)
 		_, state, _ := util.TestMocks(t)
 
 		contractCode := []byte(`access(all) contract Hello { access(all) fun sayHello(): String { return "Hello, World!" } }`)
@@ -796,7 +796,7 @@ func TestDependencyInstallerInstallFromFreshClone(t *testing.T) {
 			gw.GetAccount.Return(acc, nil)
 		})
 
-		// Mock prompter - should NOT be called (no update prompt since hashes match)
+		// No prompter needed - auto-repairs when network agrees with flow.json
 		mockPrompter := &mockPrompter{responses: []bool{}}
 
 		di := &DependencyInstaller{
@@ -819,14 +819,99 @@ func TestDependencyInstallerInstallFromFreshClone(t *testing.T) {
 		}
 
 		err = di.Install()
-		// Should FAIL because local file has been modified (tampering detected)
-		assert.Error(t, err, "Should fail when local file is modified even if network hash matches")
-		assert.Contains(t, err.Error(), "local file has been modified", "Error should mention file modification")
-		assert.Contains(t, err.Error(), "hash mismatch", "Error should mention hash mismatch")
-		assert.Contains(t, err.Error(), "Hello", "Error should mention the dependency name")
+		// Should SUCCEED - auto-repaired without prompting
+		assert.NoError(t, err, "Should auto-repair when network agrees with flow.json")
 
-		// Verify no prompts occurred (integrity check happens before any prompts)
-		assert.Equal(t, 0, mockPrompter.index, "No prompts should have been shown")
+		// Verify file WAS repaired
+		fileContent, err := state.ReaderWriter().ReadFile(filePath)
+		assert.NoError(t, err)
+		assert.Contains(t, string(fileContent), "Hello, World!", "Should have correct version")
+		assert.NotContains(t, string(fileContent), "HACKED", "Should not have hacked version")
+
+		// Verify NO prompt was shown (auto-repair because network agrees with flow.json)
+		assert.Equal(t, 0, mockPrompter.index, "Should not prompt when network agrees with flow.json")
+	})
+
+	t.Run("Already installed, up-to-date hash BUT modified local file - skip prompts mode", func(t *testing.T) {
+		// Network hash matches flow.json hash, but local file has been tampered with
+		// Should auto-repair even with --skip-update-prompts (no network change)
+		_, state, _ := util.TestMocks(t)
+
+		contractCode := []byte(`access(all) contract Hello { access(all) fun sayHello(): String { return "Hello, World!" } }`)
+		modifiedContractCode := []byte(`access(all) contract Hello { access(all) fun sayHello(): String { return "Hello, HACKED!" } }`)
+
+		// Calculate the hash of the correct contract
+		hash := sha256.New()
+		hash.Write(contractCode)
+		contractHash := hex.EncodeToString(hash.Sum(nil))
+
+		// Simulate a dependency with matching hash in flow.json
+		dep := config.Dependency{
+			Name: "Hello",
+			Source: config.Source{
+				NetworkName:  "emulator",
+				Address:      serviceAddress,
+				ContractName: "Hello",
+			},
+			Hash: contractHash, // Hash matches what's on network
+		}
+
+		state.Dependencies().AddOrUpdate(dep)
+
+		// Create a MODIFIED file (different from what hash says it should be)
+		filePath := fmt.Sprintf("imports/%s/Hello.cdc", serviceAddress.String())
+		err := state.ReaderWriter().MkdirAll(filepath.Dir(filePath), 0755)
+		assert.NoError(t, err)
+		err = state.ReaderWriter().WriteFile(filePath, modifiedContractCode, 0644)
+		assert.NoError(t, err)
+
+		gw := mocks.DefaultMockGateway()
+
+		gw.GetAccount.Run(func(args mock.Arguments) {
+			addr := args.Get(1).(flow.Address)
+			assert.Equal(t, addr.String(), serviceAddress.String())
+			acc := tests.NewAccountWithAddress(addr.String())
+			acc.Contracts = map[string][]byte{
+				"Hello": contractCode, // Network has the correct version
+			}
+
+			gw.GetAccount.Return(acc, nil)
+		})
+
+		// No prompter needed - auto-repairs regardless of flags
+		mockPrompter := &mockPrompter{responses: []bool{}}
+
+		di := &DependencyInstaller{
+			Gateways: map[string]gateway.Gateway{
+				config.EmulatorNetwork.Name: gw.Mock,
+				config.TestnetNetwork.Name:  gw.Mock,
+				config.MainnetNetwork.Name:  gw.Mock,
+			},
+			Logger:            logger,
+			State:             state,
+			SaveState:         true,
+			TargetDir:         "",
+			SkipDeployments:   true,
+			SkipAlias:         true,
+			SkipUpdatePrompts: true, // Should still auto-repair (no network change)
+			dependencies:      make(map[string]config.Dependency),
+			accountAliases:    make(map[string]map[string]flow.Address),
+			pendingPrompts:    make([]pendingPrompt, 0),
+			prompter:          mockPrompter,
+		}
+
+		err = di.Install()
+		// Should SUCCEED - auto-repaired even with --skip-update-prompts
+		assert.NoError(t, err, "Should succeed even with --skip-update-prompts (no network change)")
+
+		// Verify file WAS repaired
+		fileContent, err := state.ReaderWriter().ReadFile(filePath)
+		assert.NoError(t, err)
+		assert.Contains(t, string(fileContent), "Hello, World!", "Should have correct version")
+		assert.NotContains(t, string(fileContent), "HACKED", "Should not have hacked version")
+
+		// Verify no prompts (auto-repair because network agrees with flow.json)
+		assert.Equal(t, 0, mockPrompter.index, "Should not prompt when network agrees with flow.json")
 	})
 
 	t.Run("Already installed, outdated hash - user accepts", func(t *testing.T) {