feat(apple): Refactor Xcode project primitives for snapshots#1295
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
bd6e62e to
11c97d4
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Bundle ID picks first config
- The function now resolves build setting macros using resolveBuildSettingValue and prefers Release configuration over Debug to return the correct bundle identifier.
Or push these changes by commenting:
@cursor push 64690a7812
Preview (64690a7812)
diff --git a/src/apple/xcode-manager.ts b/src/apple/xcode-manager.ts
--- a/src/apple/xcode-manager.ts
+++ b/src/apple/xcode-manager.ts
@@ -304,11 +304,48 @@
return undefined;
}
- return this.getTargetBuildSettings(target.obj)
- .map((buildSettings) => {
- return unquote(buildSettings.PRODUCT_BUNDLE_IDENTIFIER);
- })
- .find(Boolean);
+ const buildConfigurationListId = target.obj.buildConfigurationList;
+ if (!buildConfigurationListId) {
+ return undefined;
+ }
+
+ const configurationList = this.objects.XCConfigurationList?.[
+ buildConfigurationListId
+ ] as XCConfigurationList | undefined;
+ if (!configurationList || typeof configurationList === 'string') {
+ return undefined;
+ }
+
+ const configurations = configurationList.buildConfigurations ?? [];
+ let releaseBundleId: string | undefined;
+ let fallbackBundleId: string | undefined;
+
+ for (const configRef of configurations) {
+ const config = this.objects.XCBuildConfiguration?.[configRef.value];
+ if (!config || typeof config === 'string') {
+ continue;
+ }
+
+ const bundleId = resolveBuildSettingValue(
+ config.buildSettings?.PRODUCT_BUNDLE_IDENTIFIER,
+ target.obj.name,
+ );
+ if (!bundleId) {
+ continue;
+ }
+
+ if (!fallbackBundleId) {
+ fallbackBundleId = bundleId;
+ }
+
+ const configName = unquote(config.name).toLowerCase();
+ if (configName.includes('release')) {
+ releaseBundleId = bundleId;
+ break;
+ }
+ }
+
+ return releaseBundleId ?? fallbackBundleId;
}
/**You can send follow-ups to the cloud agent here.
11c97d4 to
d063349
Compare
itaybre
left a comment
There was a problem hiding this comment.
Almost LGTM, mainly the v8 is blocking here
| @@ -6,6 +6,7 @@ | |||
| import * as clack from '@clack/prompts'; | |||
There was a problem hiding this comment.
m: This might be personal preference, but feels like this source file grew a lot and could be broken into a couple files (SPM logic, types and all the utilities).
But I am sure this would also break the stacked PRs :(
There was a problem hiding this comment.
I'll address this as a separate PR if that's okay.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Variable TEST_HOST breaks hosted detection
- Modified testHostReferencesApplication to detect TEST_HOST values using Xcode macros like
$(WRAPPER_NAME) and $ (EXECUTABLE_NAME) in addition to literal names.
- Modified testHostReferencesApplication to detect TEST_HOST values using Xcode macros like
Or push these changes by commenting:
@cursor push c178b88ddf
Preview (c178b88ddf)
diff --git a/src/apple/xcode-manager.ts b/src/apple/xcode-manager.ts
--- a/src/apple/xcode-manager.ts
+++ b/src/apple/xcode-manager.ts
@@ -89,13 +89,32 @@
const referencesAppBundle = appHostCandidates.bundleNames.some((bundleName) =>
containsPathSegment(resolvedTestHost, `${bundleName}.app`),
);
- if (!referencesAppBundle) {
- return false;
+ const referencesExecutable = appHostCandidates.executableNames.some(
+ (executableName) => containsPathSegment(resolvedTestHost, executableName),
+ );
+
+ if (referencesAppBundle && referencesExecutable) {
+ return true;
}
- return appHostCandidates.executableNames.some((executableName) =>
- containsPathSegment(resolvedTestHost, executableName),
+ const hasBundleMacro =
+ /\$\(WRAPPER_NAME\)|\$\(PRODUCT_NAME\)/.test(resolvedTestHost) ||
+ (resolvedTestHost.includes('.app') &&
+ /\$\(FULL_PRODUCT_NAME\)/.test(resolvedTestHost));
+ const hasExecutableMacro = /\$\(EXECUTABLE_NAME\)|\$\(PRODUCT_NAME\)/.test(
+ resolvedTestHost,
);
+
+ if (
+ hasBundleMacro &&
+ hasExecutableMacro &&
+ appHostCandidates.bundleNames.length > 0 &&
+ appHostCandidates.executableNames.length > 0
+ ) {
+ return true;
+ }
+
+ return false;
}
function containsPathSegment(value: string, segment: string): boolean {
diff --git a/test/apple/xcode-manager.test.ts b/test/apple/xcode-manager.test.ts
--- a/test/apple/xcode-manager.test.ts
+++ b/test/apple/xcode-manager.test.ts
@@ -203,6 +203,122 @@
});
});
+ describe('getHostedUnitTestTargetNamesForApplicationTarget', () => {
+ let xcodeProject: XcodeProject;
+ let tempPbxprojPath: string;
+
+ beforeEach(() => {
+ const tempDir = fs.mkdtempSync(
+ path.join(os.tmpdir(), 'xcode-test-hosted-unit-tests-'),
+ );
+ const xcodeprojDir = path.join(tempDir, 'Project.xcodeproj');
+ fs.mkdirSync(xcodeprojDir, { recursive: true });
+ fs.mkdirSync(path.join(tempDir, 'Sources'), { recursive: true });
+ tempPbxprojPath = path.join(xcodeprojDir, 'project.pbxproj');
+ fs.copyFileSync(singleTargetProjectPath, tempPbxprojPath);
+ xcodeProject = new XcodeProject(tempPbxprojPath);
+ });
+
+ it('should find hosted unit test with literal app and executable names in TEST_HOST', () => {
+ // -- Arrange --
+ addHostedUnitTestTarget(xcodeProject, {
+ name: 'ProjectTests',
+ hostAppName: 'Project',
+ projectObjectId,
+ productsGroupId,
+ });
+
+ // -- Act --
+ const hostedTests =
+ xcodeProject.getHostedUnitTestTargetNamesForApplicationTarget(
+ 'Project',
+ );
+
+ // -- Assert --
+ expect(hostedTests).toEqual(['ProjectTests']);
+ });
+
+ it('should find hosted unit test with macro-based TEST_HOST', () => {
+ // -- Arrange --
+ addHostedUnitTestTarget(xcodeProject, {
+ name: 'ProjectMacroTests',
+ hostAppName: 'Project',
+ testHost:
+ '"$(BUILT_PRODUCTS_DIR)/$(WRAPPER_NAME)/$(BUNDLE_EXECUTABLE_FOLDER_PATH)/$(EXECUTABLE_NAME)"',
+ projectObjectId,
+ productsGroupId,
+ });
+
+ // -- Act --
+ const hostedTests =
+ xcodeProject.getHostedUnitTestTargetNamesForApplicationTarget(
+ 'Project',
+ );
+
+ // -- Assert --
+ expect(hostedTests).toEqual(['ProjectMacroTests']);
+ });
+
+ it('should find multiple hosted unit tests with different TEST_HOST formats', () => {
+ // -- Arrange --
+ addHostedUnitTestTarget(xcodeProject, {
+ name: 'ProjectLiteralTests',
+ hostAppName: 'Project',
+ projectObjectId,
+ productsGroupId,
+ });
+ addHostedUnitTestTarget(xcodeProject, {
+ name: 'ProjectMacroTests',
+ hostAppName: 'Project',
+ testHost:
+ '"$(BUILT_PRODUCTS_DIR)/$(WRAPPER_NAME)/$(BUNDLE_EXECUTABLE_FOLDER_PATH)/$(EXECUTABLE_NAME)"',
+ projectObjectId,
+ productsGroupId,
+ });
+
+ // -- Act --
+ const hostedTests =
+ xcodeProject.getHostedUnitTestTargetNamesForApplicationTarget(
+ 'Project',
+ );
+
+ // -- Assert --
+ expect(hostedTests).toContain('ProjectLiteralTests');
+ expect(hostedTests).toContain('ProjectMacroTests');
+ expect(hostedTests).toHaveLength(2);
+ });
+
+ it('should not find unit tests with wrong app name in TEST_HOST', () => {
+ // -- Arrange --
+ addHostedUnitTestTarget(xcodeProject, {
+ name: 'OtherAppTests',
+ hostAppName: 'OtherApp',
+ projectObjectId,
+ productsGroupId,
+ });
+
+ // -- Act --
+ const hostedTests =
+ xcodeProject.getHostedUnitTestTargetNamesForApplicationTarget(
+ 'Project',
+ );
+
+ // -- Assert --
+ expect(hostedTests).toEqual([]);
+ });
+
+ it('should return empty array when app target does not exist', () => {
+ // -- Act --
+ const hostedTests =
+ xcodeProject.getHostedUnitTestTargetNamesForApplicationTarget(
+ 'NonExistentTarget',
+ );
+
+ // -- Assert --
+ expect(hostedTests).toEqual([]);
+ });
+ });
+
describe('ensureSwiftPackageProductLinked', () => {
let xcodeProject: XcodeProject;You can send follow-ups to the cloud agent here.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9b38d1e. Configure here.
|
|
||
| describe('add SPM reference', () => { | ||
| const addSPMReference = true; | ||
| const swiftPackageProduct = sentrySwiftPackageProductLinkOptions; |
There was a problem hiding this comment.
l: please inline this constant
There was a problem hiding this comment.
I've fixed the review comment by inlining the sentrySwiftPackageProductLinkOptions constant.
The change:
- Removed the
swiftPackageProductvariable declaration on line 690 - Replaced all 7 usages of
swiftPackageProductwith the constantsentrySwiftPackageProductLinkOptionsdirectly in the test calls
Tests pass and the code has been formatted. Changes committed and pushed to the branch.
Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Restore inline comments to lookupXcodeProject explaining that a single discovered Xcode project is used directly, while multiple candidates require prompting the user to choose. This makes the branching intent clear to future readers without changing behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Require sentry-cocoa 9.0.0 as the minimum Swift package version so new Apple projects adopt the current major SDK release instead of the older 8.0.0 baseline. Update the corresponding XcodeManager test expectation to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace inline "com.apple.product-type" string literals with named XCODE_APPLICATION_PRODUCT_TYPE and XCODE_UNIT_TEST_PRODUCT_TYPE constants. Apply unquote() consistently when matching the application product type. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: Cameron Cooke <web@cameroncooke.com>
1d82e9b to
5a74ab4
Compare
| productDependency.productDependencyId, | ||
| product, | ||
| ) || changed; |
There was a problem hiding this comment.
Bug: The function shouldUpdatePackageRequirement incorrectly triggers an update when requirement kinds differ, causing it to silently overwrite a user's manually configured Swift Package requirement with the wizard's default.
Severity: LOW
Suggested Fix
Modify shouldUpdatePackageRequirement to handle differing kind properties more gracefully. Instead of returning true and causing an overwrite, consider returning false to preserve the user's configuration and logging a warning that the existing requirement type is different from the wizard's default.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/apple/xcode-manager.ts#L379-L381
Potential issue: The `shouldUpdatePackageRequirement` function returns `true` if an
existing Swift Package requirement has a different `kind` than the one requested by the
wizard. This causes the calling function, `ensureSwiftPackageReference`, to overwrite
the user's entire requirement object. If a user has manually configured their Sentry
package with a specific requirement kind, such as `exactVersion`, running the wizard
will silently replace it with the default `upToNextMajorVersion` kind. This overwrites a
user's explicit configuration without their consent, although the scenario is unlikely
in practice.
Merge activity
|
## Stack This is **2/3** in the Apple Snapshots stack replacing the original unified PR #1293. - 1/3: #1295 — Xcode project primitives - 2/3: #1296 — Interactive Apple Snapshots wizard (this PR) - 3/3: #1297 — Non-interactive Apple Snapshots mode This PR builds on #1295. The top of the stack is intended to be an exact, lossless re-expression of #1293. ## Summary Adds a standalone **Apple Snapshots** wizard for configuring Sentry SnapshotPreviews in Apple/Xcode projects without running the normal iOS runtime SDK setup. The new `appleSnapshots` integration is available from the wizard picker and via `--integration appleSnapshots`. It reuses the Xcode project discovery path from #1295, asks for the app target that hosts Swift previews, then selects a hosted XCTest target for running SnapshotPreviews. This is intentionally separate from the existing `ios` wizard. Snapshot setup is related to Apple projects, but it should not silently configure normal runtime SDK behavior. Keeping this as its own wizard makes the user-visible behavior narrower: project wiring plus explicit next-step guidance for snapshot export/upload. ## What changed - Adds the `appleSnapshots` integration and CLI routing. - Adds SnapshotPreviews package configuration. - Links: - `SnapshottingTests` to the hosted test target - `SnapshotPreferences` to the selected app target only when Swift previews are detected - Creates or reuses a generated `SnapshotTest` subclass. - Ensures the generated test file belongs to the selected XCTest target. - Adds scheme inference for generated `xcodebuild test` guidance only. - Prints local export/upload guidance for: - `xcodebuild` - `sentry-cli snapshots upload` - existing Fastlane snapshot upload lanes - Adds tests for SnapshotPreviews package setup, generated test files, source insertion, smoke coverage, scheme inference, preflight behavior, CLI constants, and run routing. - Updates the help snapshot and changelog for the interactive wizard surface. ## Behavior contract in this layer - App target selection follows the existing interactive wizard pattern. - Hosted XCTest target selection uses hosted test target detection from #1295. - Missing Swift previews are not fatal. Interactive mode asks whether to continue. - Snapshot scheme inference is used only to generate `xcodebuild test` guidance. - If the correct scheme cannot be inferred safely, the wizard still completes and prints guidance without a scheme. - The wizard does not configure Sentry auth, DSNs, runtime SDK initialization, dSYM upload scripts, Fastlane dSYM setup, MCP config, or CI workflow files. ## What this PR intentionally does not do - Does not add unattended/non-interactive behavior. - Does not add `--non-interactive`, `--app-target`, or `--hosted-test-target` flags for Apple Snapshots. - Does not guarantee prompt-free execution for agentic runs. Those pieces land in #1297. ## Suggested review focus - SnapshotPreviews package URL, minimum version, and product names. - `.pbxproj` mutation correctness and idempotency for package/product linking. - Generated snapshot test file placement/reuse for both classic source phases and synchronized folders. - Hosted XCTest target selection behavior in interactive mode. - Scheme inference being limited to generated `xcodebuild test` guidance. - Ensuring the existing `ios` wizard did not pick up accidental runtime behavior changes. ## Risk Medium. The new wizard mutates Xcode project graphs, links package products, and creates/reuses source files. The new `appleSnapshots` flow is isolated from auth/runtime/dSYM/CI configuration, but unusual Xcode project structures, ambiguous schemes, or non-standard hosted test target settings are the areas most likely to need reviewer scrutiny. ## Validation Before submitting the stack: - `yarn test` passed: 947 passed, 4 skipped - `yarn build` passed - Final stack tree matched original PR #1293 exactly - `gt submit --stack --dry-run` passed Co-Authored-By: GPT-5 Codex <noreply@openai.com>
## Stack This is **3/3** in the Apple Snapshots stack replacing the original unified PR #1293. - 1/3: #1295 — Xcode project primitives - 2/3: #1296 — Interactive Apple Snapshots wizard - 3/3: #1297 — Non-interactive Apple Snapshots mode (this PR) This is the top of the stack. The final tree of this stack was verified to exactly match original PR #1293. ## Summary Adds unattended/agent-friendly execution for the Apple Snapshots wizard introduced in #1296. The main goal is that non-interactive mode never waits on stdin. It either proceeds from explicit arguments and safe single-target assumptions, or it fails with actionable manual setup instructions and the CLI flag needed to continue. ## What changed - Adds Apple Snapshots CLI options: - `--non-interactive` - `--app-target` - `--hosted-test-target` - Threads non-interactive behavior through: - CLI/run routing - project lookup - git safety checks - preview/missing-preview confirmation - sentry-cli preflight - hosted XCTest target selection - Adds app-target and hosted-test-target resolution behavior for unattended runs. - Adds prompt suppression paths so agentic setup cannot hang on stdin. - Updates help output for the new flags. - Adds tests for non-interactive success/failure paths, explicit target handling, and prompt-free behavior. ## Behavior contract in this layer - App target selection uses `--app-target` when provided. - Without `--app-target`, the wizard auto-selects only when there is a single app target. - If the app target cannot be resolved safely in non-interactive mode, the wizard fails with guidance to pass `--app-target`. - Hosted XCTest target selection uses `--hosted-test-target` when provided, as long as the target exists and has a non-empty `TEST_HOST`. - Without `--hosted-test-target`, the wizard first narrows hosted XCTest targets to those whose `TEST_HOST` matches the selected app target's bundle/executable names. - If app-specific hosted-test matching finds one target, the wizard uses it. - If app-specific hosted-test matching finds multiple targets, non-interactive mode fails with guidance to pass `--hosted-test-target`. - If app-specific hosted-test matching is inconclusive but the project has exactly one hosted XCTest target, the wizard assumes the common case that this target belongs to the selected app. - If there are multiple hosted XCTest targets and no safe match, non-interactive mode fails with guidance. - Missing Swift previews are not fatal. Non-interactive mode logs the limitation and continues because agents may still want the project wiring and manual next steps. - Non-interactive mode must never wait on stdin. ## What this PR intentionally does not do - Does not configure Sentry auth, DSNs, runtime SDK initialization, dSYM upload scripts, Fastlane dSYM setup, MCP config, or CI workflow files. - Does not change the existing `ios` wizard into a SnapshotPreviews setup path. - Does not make unsafe guesses when target resolution is ambiguous. ## Suggested review focus - The target-selection contract for explicit CLI choices, single-target assumptions, and non-interactive failures. - Hosted XCTest detection via `TEST_HOST` and app bundle/executable name matching. - Confirming all non-interactive paths avoid stdin prompts. - Error messages/manual setup guidance for ambiguous target/project structures. - Help output and CLI option behavior. ## Risk Medium. This layer controls unattended behavior and is the most important one for agentic setup safety. The main risk is choosing the wrong app/test target in unusual projects or leaving a prompt path reachable in non-interactive mode. ## Validation Before submitting the stack: - `yarn test` passed: 947 passed, 4 skipped - `yarn build` passed - Final stack tree matched original PR #1293 exactly - `gt submit --stack --dry-run` passed Co-Authored-By: GPT-5 Codex <noreply@openai.com>





Stack
This is 1/3 in the Apple Snapshots stack replacing the original unified PR #1293.
The top of the stack is intended to be an exact, lossless re-expression of #1293.
Summary
This PR refactors the existing Apple/Xcode project infrastructure needed by the Apple Snapshots wizard, without exposing the new
appleSnapshotswizard yet.The goal is to make the shared Xcode helpers reviewable independently before adding the user-facing SnapshotPreviews flow. Existing Sentry Apple setup remains routed through the current
ioswizard; this layer should be shared infrastructure, not a new runtime setup behavior.What changed
XcodeProjectwith reusable helpers for:TEST_HOSTconfigureXcodeProject/updateXcodeProjectbehavior for the normal Apple SDK setup path.What this PR intentionally does not do
appleSnapshotsintegration option.Those pieces land in the upstack PRs.
Suggested review focus
.pbxprojmutation correctness and idempotency.TEST_HOST.ioswizard behavior did not accidentally change.Risk
Medium. This refactors shared Apple/Xcode project mutation code used by existing setup paths. The risk is primarily around unusual Xcode project graphs, package product linking, target discovery, and source membership handling.
Validation
Before submitting the stack:
yarn testpassed: 947 passed, 4 skippedyarn buildpassedgt submit --stack --dry-runpassedCo-Authored-By: GPT-5 Codex noreply@openai.com