Support PBXFileSystemSynchronizedRootGroup (Xcode 16 buildable folders)

[Ryan Zulkoski (rzulkoski)]

Summary

Adds complete support for merging projects that use Xcode 16 buildable folders, stored as PBXFileSystemSynchronizedRootGroup. These are referenced both from the main group tree and from a target's fileSystemSynchronizedGroups; Kintsugi had no handling for the isa, so any conflict touching such a folder failed with Trying to add unsupported component type PBXFileSystemSynchronizedRootGroup.

Builds on #139 by ivoidcat and implements the "proper fix" described in Noam Freeman (@noamfreeman)'s review there (built with Claude Code's help — the commit is co-authored accordingly):

• Handle the group in the main-group addition/removal pipeline (apply_group_additions / apply_group_removals) instead of dropping it — the missing-from-the-group-tree case from review. Also fixes a phantom empty-children key that would otherwise block removals.
• Attach it to a target by resolving and reusing the object already created in the group tree (mirroring add_variant_group → find_variant_group), so one folder never yields two root groups. Guarded by Settings.allow_duplicates.
• Dropped the respond_to? fallback and the no-op branches, per review.
• Coverage beyond the minimum: exceptions (target- and build-phase-membership), removals, and attribute diffs.

The exceptions support needed a to_tree_hash monkeypatch (in the existing xcodeproj_extensions.rb) to break a reference cycle — the stock to_tree_hash expands the exception set's target/build_phase reference, which recurses back through the owning group forever. It also works around an xcodeproj bug where PBXFileSystemSynchronizedGroupBuildPhaseMembershipExceptionSet#display_name calls build_phase.name, which the referenced build phases (e.g. PBXSourcesBuildPhase) don't implement — present through the latest release (1.28.1) and master.

No dependency changes; validated against the pinned xcodeproj range (>= 1.26.0, <= 1.27.0).

Closes #139 (supersedes it — happy to keep it open instead if you'd rather).

Test plan

• Unit specs for both references, explicit file types/folders, exceptions, removal, and the allow_duplicates guard — including an object-identity assertion, since the tree-hash matcher can't detect a duplicated object.
• End-to-end integration spec reproducing the original merge failure via a real git conflict (across rebase / cherry-pick / merge).
• bundle exec rake (RuboCop + full suite): 150 examples, 0 failures; 0 offenses.

🤖 Generated with Claude Code

[Noam Freeman (noamfreeman)]

Hi Ryan Zulkoski (@rzulkoski) ! thanks for the implementation.

i found 2 things worth fixing before merging(and some other minor issues - in the comments):

• When a target and its synchronized-folder exception are added together, the group is processed before the target exists, so the target or build-phase reference can remain unset. Deferring reference resolution until targets are created—or doing a second pass—may help.
• Groups and build phases are resolved globally by display name. Common names such as Sources can silently select an object belonging to another target or parent group. Target/hierarchy information or source-object identity may provide a safer match.

tests for reproduction

it "resolves an exception target added in the same change" do
  theirs_project = create_copy_of_project(base_project, "theirs")
  target = theirs_project.new_target(
    "com.apple.product-type.library.static", "bar", :ios
  )
  group = add_synchronized_root_group(theirs_project, "SyncedSources")
  target.file_system_synchronized_groups << group

  klass = Xcodeproj::Project::PBXFileSystemSynchronizedBuildFileExceptionSet
  exception = theirs_project.new(klass)
  exception.target = target
  exception.membership_exceptions = ["Excluded.swift"]
  group.exceptions << exception

  changes = get_diff(theirs_project, base_project)
  described_class.apply_change_to_project(base_project, changes, theirs_project)

  resolved = base_project.objects.find { |object| object.is_a?(klass) }
  expected_target = base_project.targets.find { |item| item.name == "bar" }

  expect(resolved.target&.uuid).to eq(expected_target.uuid)
end

it "resolves a build-phase exception against the correct target" do
  base_project.new_target(
    "com.apple.product-type.library.static", "bar", :ios
  )
  theirs_project = create_copy_of_project(base_project, "theirs")
  target = theirs_project.targets.find { |item| item.name == "bar" }
  group = add_synchronized_root_group(theirs_project, "SyncedSources")
  target.file_system_synchronized_groups << group

  klass =
    Xcodeproj::Project::PBXFileSystemSynchronizedGroupBuildPhaseMembershipExceptionSet
  exception = theirs_project.new(klass)
  exception.build_phase = target.source_build_phase
  exception.membership_exceptions = ["OnlyInSources.swift"]
  group.exceptions << exception

  changes = get_diff(theirs_project, base_project)
  described_class.apply_change_to_project(base_project, changes, theirs_project)

  resolved = base_project.objects.find { |object| object.is_a?(klass) }
  expected_phase =
    base_project.targets.find { |item| item.name == "bar" }.source_build_phase

  expect(resolved.build_phase.uuid).to eq(expected_phase.uuid)
end

the inline comments are of lower severety. all the issues are in the new feature, not regressions, so this current implementation is also a big improvement.

[Noam Freeman (noamfreeman)]

a delete vs. modify conflict may reach the else error case,

can we include PBXFileSystemSynchronizedRootGroup in this recreation path? maybe pass it to apply_group_additions?

[Ryan Zulkoski (rzulkoski)]

Done — create_nonexistent_groupable_component now routes PBXFileSystemSynchronizedRootGroup to apply_group_additions (alongside PBXGroup), so a delete-vs-modify conflict recreates the folder instead of hitting the else. Covered by the "recreates a synchronized root group modified on theirs and deleted on ours" spec.

[Noam Freeman (noamfreeman)]

small edge case: if base doesn't have an exception set, but the 2 branches do, we should reuse/merge the existing one.
same for the build-phase exception below.

[Ryan Zulkoski (rzulkoski)]

Done for both variants — exception sets are now deduped before being added, so an equivalent set already on the group is a no-op. Because a folder is referenced from both the main group and its target(s), the same addition is visited through several graph paths; the dedup handles that too (and stays correct while the reference is still deferred/unresolved). Specs: "does not duplicate an exception set added on both sides", "...deferred exception added to an existing buildable folder", plus the allow_duplicates variants.

[Noam Freeman (noamfreeman)]

Can we set a display_name in the same manner as PBXFileSystemSynchronizedGroupBuildPhaseMembershipExceptionSet?
if non is supplied, the inherited target.name may be dereferenced on a nil target.

[Ryan Zulkoski (rzulkoski)]

Done — PBXFileSystemSynchronizedBuildFileExceptionSet now has a nil-safe display_name override mirroring the membership variant (using target&.name), so an unset target no longer dereferences nil. Spec: "serializes a build file exception set with an unset target without crashing".

[Ryan Zulkoski (rzulkoski)]

Thanks for the thorough review, Noam Freeman (@noamfreeman) — I've pushed an update addressing all of it.

The two main issues:

• Same-change ordering — exception target/build_phase references are now resolved in a deferred pass that runs after the whole change is applied (once targets exist and folders are linked to them), so an exception whose target is created in the same change resolves correctly. Your first repro test is included as a spec.
• Global-by-name resolution — groups are now resolved by hierarchy path (matched against the change's target in the source project), so same-named folders in different parent groups no longer collide; build phases are resolved via their owning target rather than by bare name, so a shared folder's Sources-phase exception can't select an unrelated target's phase. Your second repro test is included.

The inline comments:

• Delete-vs-modify recreation path — PBXFileSystemSynchronizedRootGroup is now handled in create_nonexistent_groupable_component (routed to apply_group_additions), so it no longer falls through to the else error.
• Reusing/merging an existing exception set — exception sets are now deduped; because a folder is referenced from both the main group and its target(s), the same exception addition is visited through several graph paths, and adding an equivalent set is now a no-op.
• display_name for the build-file exception set — it now has a nil-safe display_name override (mirroring the membership variant), so a transiently-unset target can't raise during serialization.

While in there I also ran an adversarial pass over the whole feature and fixed a few pre-existing edges it turned up — notably a data-loss case where removing a target's link to a shared folder deleted the folder object project-wide (it now detaches the reference and lets the object survive for other targets), plus the reference-cycle and display_name recursion guards needed to serialize exceptions safely.

One known limitation I left documented in the code: relocating a folder between parent groups on one side can drop its target membership on merge, because buildable folders have no stable identity in the diff (a move reads as remove-here + add-there). A proper fix would need to correlate removals with additions across the change, which felt out of scope here.

bundle exec rake is green (RuboCop + full suite). Happy to adjust anything — and thanks again for the detailed repros, they made this much easier to get right.

[Noam Freeman (noamfreeman)]

Thanks for addressing these.

[Noam Freeman (noamfreeman)]

we may have some issue with 2 folders with the same name in the same target, or folders with a slash in the name.
but overall, looks good to release.

[Ryan Zulkoski (rzulkoski)]

Sounds good, thanks!