Repository navigation
Organize SPM packages into Packages/{Shared,iOS,macOS}/ so the folder tree mirrors the Xcode workspace groups - #6278
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedToo many files! This PR contains 299 files, which is 149 over the limit of 150. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (299)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughIntroduces ChangesWorkspace SPM Package Grouping Enforcement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 20 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (20 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greptile SummaryThis PR fixes a long-standing drift in
Confidence Score: 4/5The workspace data change itself is metadata-only and safe to merge. The new generator script has one design gap worth addressing before the next example project is added. The classification logic, idempotency, and CI wiring are all well-constructed. The one concrete defect is that EXAMPLES_GROUP is hardcoded in the generator: if anyone adds a new example project to the workspace and then gets a CI --check failure, the error guidance ('run --write') will silently drop their new entry rather than preserving it. This is not a current breakage — the constant matches the workspace today — but it is a predictable workflow trap the next time an example is added. scripts/check-workspace-package-groups.py — the EXAMPLES_GROUP constant and the --check error message both need attention before an example is next added to the workspace. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[discover_packages\nPackages/*/Package.swift] --> B[build deps map\nlocal .package path edges]
B --> C1[pbx_local_roots\ncmux.xcodeproj]
B --> C2[pbx_local_roots\nios/cmux-ios.xcodeproj\n+ ios/cmuxPackage]
C1 --> D1[closure mac]
C2 --> D2[closure ios]
D1 --> E{in mac?\nin ios?}
D2 --> E
E -->|both| F[Shared]
E -->|ios only| G[iOS]
E -->|mac only| H[macOS]
E -->|neither| I[declared_platforms\nfallback]
I -->|iOS only| G
I -->|macOS only| H
I -->|both or neither| F
F --> J[render + write\ncontents.xcworkspacedata]
G --> J
H --> J
J --> K{--write or --check?}
K -->|--write| L[overwrite file]
K -->|--check| M{actual == expected?}
M -->|yes| N[OK exit 0]
M -->|no| O[error + diff exit 1]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[discover_packages\nPackages/*/Package.swift] --> B[build deps map\nlocal .package path edges]
B --> C1[pbx_local_roots\ncmux.xcodeproj]
B --> C2[pbx_local_roots\nios/cmux-ios.xcodeproj\n+ ios/cmuxPackage]
C1 --> D1[closure mac]
C2 --> D2[closure ios]
D1 --> E{in mac?\nin ios?}
D2 --> E
E -->|both| F[Shared]
E -->|ios only| G[iOS]
E -->|mac only| H[macOS]
E -->|neither| I[declared_platforms\nfallback]
I -->|iOS only| G
I -->|macOS only| H
I -->|both or neither| F
F --> J[render + write\ncontents.xcworkspacedata]
G --> J
H --> J
J --> K{--write or --check?}
K -->|--write| L[overwrite file]
K -->|--check| M{actual == expected?}
M -->|yes| N[OK exit 0]
M -->|no| O[error + diff exit 1]
Reviews (1): Last reviewed commit: "Group every SPM package in the root work..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+53
to
+61
| EXAMPLES_GROUP = ( | ||
| 'container:Examples', | ||
| 'Examples', | ||
| [ | ||
| "group:TabsVisibleSidebar/TabsVisibleSidebar.xcodeproj", | ||
| "group:SampleSidebarExtensionApp/SampleSidebarExtensionApp.xcodeproj", | ||
| "group:CmuxExtensionSidebarExamples", | ||
| ], | ||
| ) |
Contributor
There was a problem hiding this comment.
EXAMPLES_GROUP creates a data-loss trap for future example additions
render() always regenerates the Examples group from the hardcoded constant on line 53–61. If a developer adds a new example project to the workspace manually and then gets a --check failure in CI, the failure message instructs them to "Run --write" — but --write will silently drop their new entry because the constant no longer matches the workspace. The developer must also update EXAMPLES_GROUP in this script, but nothing in the error message or the CLAUDE.md doc tells them that. The safest fix is to parse the existing Examples group from the current workspace file rather than hardcoding its children, or at minimum make the --check error message explicitly warn that Examples changes also require updating EXAMPLES_GROUP in the script.
| ], | ||
| ) | ||
|
|
||
| PACKAGE_DEP_RE = re.compile(r'\.package\(\s*path:\s*"([^"]+)"') |
Contributor
There was a problem hiding this comment.
PACKAGE_DEP_RE regex matches .package(path: "...") anywhere in the file, including inside Swift line comments. A commented-out dependency whose package still exists on disk would be incorrectly included in the reachability closure and change the group classification for that package.
Suggested change
| PACKAGE_DEP_RE = re.compile(r'\.package\(\s*path:\s*"([^"]+)"') | |
| # Strip line comments before matching to avoid false positives from commented-out deps. | |
| _STRIP_COMMENT_RE = re.compile(r"//[^\n]*") | |
| PACKAGE_DEP_RE = re.compile(r'\.package\(\s*path:\s*"([^"]+)"') |
Comment on lines
+216
to
+217
| actual = open(WORKSPACE_DATA, encoding="utf-8").read() | ||
| if actual == expected: |
Contributor
There was a problem hiding this comment.
open(WORKSPACE_DATA) in --check mode will raise FileNotFoundError (unhandled, raw Python traceback) if the workspace file is absent, rather than printing a clean actionable message. This could happen in a fresh sparse checkout or if someone renames the file.
Suggested change
| actual = open(WORKSPACE_DATA, encoding="utf-8").read() | |
| if actual == expected: | |
| try: | |
| actual = open(WORKSPACE_DATA, encoding="utf-8").read() | |
| except FileNotFoundError: | |
| print( | |
| f"::error file={os.path.relpath(WORKSPACE_DATA, ROOT)}::" | |
| f"workspace file not found. Run: python3 scripts/check-workspace-package-groups.py --write", | |
| file=sys.stderr, | |
| ) | |
| return 1 | |
| if actual == expected: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/check-workspace-package-groups.py`:
- Around line 94-97: In the pbx_local_roots() function, restrict pbx root
discovery to only consider relativePath entries that are under the `Packages/`
directory. The current code in the loop iterating through PBX_RELPATH_RE.findall
results adds any basename found in names to the roots set, but it should first
verify that the raw path contains or starts with the `Packages/` directory
prefix before adding to roots. This prevents unrelated relativePath entries from
being incorrectly marked as SwiftPM roots.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 92db4548-418e-4474-8aab-f286ee34ddc3
📒 Files selected for processing (4)
.github/workflows/ci.ymlCLAUDE.mdcmux.xcworkspace/contents.xcworkspacedatascripts/check-workspace-package-groups.py
Comment on lines
+94
to
+97
| for raw in PBX_RELPATH_RE.findall(txt): | ||
| base = os.path.basename(raw.strip().rstrip("/")) | ||
| if base in names: | ||
| roots.add(base) |
There was a problem hiding this comment.
Restrict pbx root discovery to Packages/ paths only.
pbx_local_roots() currently treats any relativePath basename as a package root. That can incorrectly mark unrelated entries as SwiftPM roots and skew Shared/iOS/macOS classification.
Suggested fix
def pbx_local_roots(pbxproj: str, names: set[str]) -> set[str]:
if not os.path.exists(pbxproj):
return set()
txt = open(pbxproj, encoding="utf-8").read()
roots = set()
for raw in PBX_RELPATH_RE.findall(txt):
- base = os.path.basename(raw.strip().rstrip("/"))
+ rel = raw.strip().rstrip("/")
+ if not rel.startswith("Packages/"):
+ continue
+ base = os.path.basename(rel)
if base in names:
roots.add(base)
return roots🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/check-workspace-package-groups.py` around lines 94 - 97, In the
pbx_local_roots() function, restrict pbx root discovery to only consider
relativePath entries that are under the `Packages/` directory. The current code
in the loop iterating through PBX_RELPATH_RE.findall results adds any basename
found in names to the roots set, but it should first verify that the raw path
contains or starts with the `Packages/` directory prefix before adding to roots.
This prevents unrelated relativePath entries from being incorrectly marked as
SwiftPM roots.
azooz2003-bit
force-pushed
the
task-spm-package-groups
branch
from
June 16, 2026 23:08
bd8417a to
9217d11
Compare
azooz2003-bit
force-pushed
the
task-spm-package-groups
branch
3 times, most recently
from
June 16, 2026 23:45
bc1add9 to
3353d80
Compare
… tree mirrors the workspace groups Every Swift package now lives physically under exactly one group directory (Packages/Shared, Packages/iOS, Packages/macOS), so the repo's directory tree and the root workspace's group columns are the same shape. Opening cmux.xcworkspace shows each package under the Shared / iOS / macOS group matching the folder it lives in. Folder is the source of truth. Group = which app(s) consume the package: both apps -> Shared, iOS app only -> iOS, macOS app only -> macOS. check-workspace-package-groups.py mirrors the folders directly; --write regenerates the workspace, --check (in CI, beside check-pbxproj) fails on drift. All boundary-crossing relative paths were rewritten to keep the build intact: inter-package deps same group `../Name` / cross group `../../<Group>/Name`; escaping paths gain one level (vendor `../../../vendor/...`, GhosttyKit `../../../GhosttyKit.xcframework`); macOS project relativePaths, ios/cmuxPackage and Examples deps + project relativePaths, the file-length budget, the iOS conventions lint scopes, the namespace-type baseline, the test-ios change globs, the ci.yml per-package `swift test` loop (now resolves the group dir), and doc/skill references all updated to the nested paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
azooz2003-bit
force-pushed
the
task-spm-package-groups
branch
from
June 17, 2026 00:47
3353d80 to
eff93a4
Compare
This was referenced Jun 17, 2026
6 tasks
This branch was successfully deployed
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
The root
cmux.xcworkspacegrouped packages into Shared / iOS / macOS columns, but on disk every package sat flat underPackages/, so the group structure wasn't visible as folders in the repo. This makes the folder tree the source of truth and the same shape as the workspace: every Swift package now lives physically under exactly one group directory, and the workspace mirrors it one-to-one.Packages/Shared/<pkg>— used by both the macOS and iOS appsPackages/iOS/<pkg>— iOS app onlyPackages/macOS/<pkg>— macOS app onlyAll 74 packages were
git mvd into their group folder (Shared 5 / iOS 15 / macOS 54). The three workspace groups now havecontainer:Packages/Shared|iOS|macOSlocations, so opening the workspace shows each package under the group matching the folder it lives in. Classification (which folder) was computed from app dependency closures: reachable from both apps → Shared, iOS-app-only → iOS, macOS-app-only → macOS, orphans by declaredplatforms.Every relative path that crosses the package boundary was rewritten so the build is unaffected:
../Name, cross group../../<Group>/Name../../../vendor/..., GhosttyKit binary target../../../GhosttyKit.xcframeworkcmux.xcodeprojrelativePaths,ios/cmuxPackage+ Examples package deps and their.xcodeprojrelativePaths,.github/swift-file-length-budget.tsv, thelint-ios-package-conventions.shscopes,lint-namespace-types-baseline.txt, thetest-ios.ymlchange-detection globs, and doc/skill references all point at the nested paths.scripts/check-workspace-package-groups.pynow mirrors the folders directly (a package's group is its parent directory).--writeregeneratescontents.xcworkspacedata;--checkis wired into the CI lint job and fails on drift. The rule is documented inCLAUDE.md/AGENTS.md: to regroup a package,git mvits folder and re-run--write; never hand-edit the workspace.Testing
Metadata/path-only change (no app/runtime behavior), so no tagged reload.
python3 scripts/check-workspace-package-groups.py --check→ OK; re-running--writeis idempotent. All 74 packages covered, 0 missing, 0 dangling.swift package dump-packageresolves a package closure crossing Shared↔iOS boundaries (../../<group>/...paths), confirming the rewrites resolve through SwiftPM, not just on disk.path:in every manifest and every pbxprojrelativePathresolves to an existing directory.xcodebuild -list -workspace cmux.xcworkspacefinds every package at its new nested path; the only remaining errors are the pre-existing environmental ones (uninitializedvendor/bonsplitsubmodule + unbuiltGhosttyKitbinary target + an existingGhosttyRuntimeTestStubstarget-name clash), byte-identical to the pre-change baseline. A full Xcode resolve needsscripts/setup.sh(submodule init + zig GhosttyKit build) and isn't possible in a fresh worktree../scripts/check-pbxproj.sh(pbxproj normalized),python3 scripts/swift_file_length_budget.py(budget paths), and./scripts/lint-ios-package-conventions.sh(scopes resolve) all pass.A source-shape test asserting specific FileRefs would just restate the generator output, so the generator's deterministic
--checkin CI is the right behavior-level proof.🤖 Generated with Claude Code