Skip to content

Gitignore entries are not reference-counted, so removing one scope strips lines another still claims #378

Description

@bguidolim

Summary

ResourceRefCounter.Resource has exactly two cases, .brewPackage and .plugin. Gitignore entries belong in that set and are missing from it.

GitignoreManager.resolveGlobalGitignorePath() resolves a single file for the whole machine — git config core.excludesFile, falling back to ~/.config/git/ignore. There is no per-project gitignore. Yet both ProjectSyncStrategy.installArtifacts and GlobalSyncStrategy.installArtifacts record the same entries into their own PackArtifactRecord.gitignoreEntries, so a pack installed in two scopes has two claims on one physical line — and Configurator.unconfigurePack deletes it on behalf of whichever scope is torn down first, with no check for the others.

Affected paths

All four route through Configurator.removeGitignoreArtifact, none of them ref-counted:

  • unconfigurePack — deselecting a pack in mcs sync while another scope still has it
  • removeNewlyExcludedComponentArtifacts — excluding the component via --customize
  • reconcileStaleArtifacts — stale-artifact cleanup
  • mcs pack remove — worst case: the federated loop runs unconfigurePack once per affected scope, so the first scope strips the shared lines out from under every scope that follows, and their removals silently no-op

Symptom

An ignore line disappears while a pack still claims it, so previously-ignored files begin showing up in git status — across every project on the machine, since the file is global. Sync is convergent, so the next mcs sync --global or mcs update restores it; until then the state is wrong.

Proposed fix

Add case gitignoreEntry(String) to ResourceRefCounter.Resource:

  • Resource.displayName — a case for the new kind
  • checkGlobalArtifacts — match against PackArtifactRecord.gitignoreEntries
  • packDeclaresResource — match against the .gitignoreEntries(entries:) install action
  • Configurator.removeGitignoreArtifact — return the same .removed / .stillNeeded / .failed tri-state as its siblings removeBrewArtifact and removePluginArtifact, and update the call sites to the switch those already model

One detail makes this safer than it looks for the other resource kinds: .gitignoreEntries(entries: [String]) carries a literal payload with no placeholder substitution (unlike MCPServerConfig.substituting), so declaration-based matching in packDeclaresResource is exact.

Known wrinkle, matching existing behavior rather than fixing it: checkProjectIndex does not consult per-scope excludedComponents, so a scope that excluded the gitignore component still counts as a referent. That biases toward keeping a line, which is the same conservative bias brew packages and plugins already accept, and the safer direction here.

Cleanup this enables

PR #377 compensates for this on the doctor path only — ScopeDuplicationCheck.restoreSharedGitignoreEntries re-adds whatever the global record still claims after unconfigurePack has stripped it. That workaround and its comment should be deleted when this lands. Its test (fixPreservesSharedGitignoreEntries) asserts the entry survives, which stays true, so it converts into a regression guard for the primitive instead of for the workaround.

Acceptance criteria

  • Deselecting a pack in one scope leaves gitignore lines another scope still claims
  • mcs pack remove removes a shared line exactly once, after the last scope releases it
  • ResourceRefCounterTests covers the new case
  • ScopeDuplicationCheck.restoreSharedGitignoreEntries and its explanatory comment are removed

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Normal prioritybugSomething isn't workingsilent-failureError handling / silent failure

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions