Skip to content

Fix iOS App Store lane bundle fixture - #8242

Merged
lawrencecchen merged 2 commits into
mainfrom
task-ios-appstore-plistbuddy-fixture
Jul 16, 2026
Merged

lawrencecchen merged 2 commits into
mainfrom
task-ios-appstore-plistbuddy-fixture

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Jul 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • add CFBundleExecutable to fake Xcode app bundles
  • keep generated and reused archive fixtures consistent with real app metadata

Testing

  • focused test_upload_beta_lane_uses_beta_marketing_version: passed all 8 checks
  • git diff --check

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Summary by cubic

Set CFBundleExecutable on fake iOS app bundles and update the invalid framework upload regression to strip shells without a valid executable. This makes fixtures match real Xcode archives and confirms the App Store lane handles invalid embeds.

  • Bug Fixes
    • Add CFBundleExecutable: "cmux" to Info.plist for generated and reused archive fixtures.
    • Update regression: use CMUX_FAKE_EMBED_INVALID_FRAMEWORK_SHELL; expect a successful upload that strips the invalid framework, logs why, and the IPA omits Iroh.framework.

Written for commit 811f4a3. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Tests
    • Updated iOS App Store lane test fixtures to include executable metadata in the generated app Info.plist, improving archive identity validation coverage.
    • Revised framework-stripping test logic to cover an embedded framework shell with an invalid dynamic-library executable, ensuring it’s removed from the final signed IPA contents.
    • Adjusted test runner wiring to execute the updated framework-stripping test.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Fake iOS archive fixtures now include CFBundleExecutable, and framework-stripping coverage now verifies that an embedded framework with an invalid executable is removed from the signed IPA.

Changes

iOS upload fixture validation

Layer / File(s) Summary
Add executable metadata to fake archives
tests/test_ios_appstore_lane_identity.py
Both fake archive-generation paths write CFBundleExecutable as cmux in the generated app Info.plist.
Test invalid framework shell stripping
tests/test_ios_appstore_lane_identity.py
The fake export flow uses CMUX_FAKE_EMBED_INVALID_FRAMEWORK_SHELL, and the replacement test verifies the invalid framework is stripped from the final signed IPA and is invoked by main().

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: azooz2003-bit

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed The PR only changes a Python test file; no Swift sources or actor-isolation-relevant declarations were introduced or worsened.
Cmux Swift Blocking Runtime ✅ Passed HEAD only changes a Python test file; no Swift production code or new blocking/timing sync primitives were introduced.
Cmux Browser Automation Off-Main ✅ Passed Only tests/test_ios_appstore_lane_identity.py changed; it updates iOS fixture/test logic and touches no browser.* routing, mainActor, or socketWorkerMethods code.
Cmux Expensive Synchronous Load ✅ Passed Diff is test-only Python; no Swift production code or main-actor sync-load paths were added or moved.
Cmux Cache Substitution Correctness ✅ Passed Only a Python test file changed; no production Swift/TS/JS cache substitution path was introduced.
Cmux No Hacky Sleeps ✅ Passed The PR only changes test scaffolding; the diff adds no fixed sleeps, timers, polling, or wall-clock waits in runtime code.
Cmux Algorithmic Complexity ✅ Passed PR changes only tests/test_ios_appstore_lane_identity.py; no production Swift/TS/JS/shell/runtime path was introduced or worsened.
Cmux Swift Concurrency ✅ Passed The PR only changes a Python test file; it introduces no cmux-owned Swift code or async patterns to evaluate.
Cmux Swift @Concurrent ✅ Passed Only a Python test file changed; there are no Swift diffs to evaluate under the concurrency rule.
Cmux Swift Package Boundaries ✅ Passed PR diff vs origin/main changes only a Python test file; no Swift production code is touched, so the Swift package-boundary rule is not applicable.
Cmux Swiftpm Lockfiles ✅ Passed Only tests/test_ios_appstore_lane_identity.py changed; no package, Xcode project, .gitignore, workflow, or Package.resolved files were touched, so the rule isn’t triggered.
Cmux Swift Logging ✅ Passed PR only changes a Python test file; no Swift production code or logging changes are present in the diff.
Cmux User-Facing Error Privacy ✅ Passed Only tests/test_ios_appstore_lane_identity.py changed; the added vendor/path/env strings are in test fakes and assertions, not production user-facing text.
Cmux Full Internationalization ✅ Passed Only tests/fake fixtures changed; no production user-facing text or locale assets were modified, so the i18n rule isn’t violated.
Cmux Swiftui State Layout ✅ Passed Only tests/test_ios_appstore_lane_identity.py changed, and the diff has no SwiftUI state/layout patterns or view code.
Cmux Architecture Rethink ✅ Passed PR only updates a Python test fixture and test names/env flags; no Swift lifecycle, timing, observer, or duplicate-owner patterns were introduced.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS: The PR only changes tests/test_ios_appstore_lane_identity.py; no Swift window/controller code or cmuxAuxiliaryWindowIdentifiers changes, and the rule exempts test-only fixtures.
Cmux Source Artifacts ✅ Passed The only changed path is a hand-written test file, which the policy explicitly allows; no artifact, cache, or scratch directory was added.
Cmux No Test Or Debug Seam In Production Source ✅ Passed PR only changes tests/test_ios_appstore_lane_identity.py; no Swift files under production Sources/ were touched, so this seam rule is not applicable.
Cmux No Ambient Global State ✅ Passed PR only changes a Python test file; the no-ambient-global-state rule applies to production Swift code, so it is not implicated.
Title check ✅ Passed The title matches the main fixture update, though it omits the framework-stripping test change.
Description check ✅ Passed The description covers Summary and Testing, but omits the Demo Video, Review Trigger, and Checklist sections.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch task-ios-appstore-plistbuddy-fixture

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

@codex review

@greptile-apps

greptile-apps Bot commented Jul 16, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes two gaps in the iOS App Store lane test fixtures: it adds the missing CFBundleExecutable key to Info.plist in both the generated archive fixture (inside the embedded fake xcodebuild command) and the reusable _write_fake_archive helper so they match real Xcode output. It also updates the invalid-framework test to reflect a behavioral change where the upload script now strips shell-only frameworks rather than hard-failing.

Confidence Score: 5/5

Test-only change that makes fixtures more accurate and aligns a test with the updated stripping behavior; no production code is touched.

All changes are confined to the test file. Adding CFBundleExecutable closes a real gap between the fake fixtures and actual Xcode archives, and the env-var rename plus test-expectation flip are internally consistent. No production Swift or runtime code is modified.

No files require special attention.

Important Files Changed

Filename Overview
tests/test_ios_appstore_lane_identity.py Adds CFBundleExecutable to both generated and reused archive fixtures, renames the framework-stripping env var, and updates the corresponding test to verify successful stripping rather than upload rejection

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[xcodebuild -archive fake] -->|writes Info.plist| B["Info.plist\n+ CFBundleExecutable: cmux\n+ CFBundleIdentifier\n+ CFBundleVersion\n+ CFBundleShortVersionString"]
    C[_write_fake_archive helper] -->|writes Info.plist| B
    D[CMUX_FAKE_EMBED_INVALID_FRAMEWORK_SHELL=1] -->|injects bad framework| E[Iroh.framework shell\nno executable]
    B --> F[upload-testflight.sh]
    E --> F
    F -->|detects missing executable| G[Strips Iroh.framework]
    G --> H[Signs IPA without framework]
    H --> I{Test assertions}
    I -->|returncode == 0| J[✓ Upload succeeds]
    I -->|stripping message in stdout| K[✓ Reports reason]
    I -->|Iroh.framework absent in IPA| L[✓ Framework stripped]
Loading
%%{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[xcodebuild -archive fake] -->|writes Info.plist| B["Info.plist\n+ CFBundleExecutable: cmux\n+ CFBundleIdentifier\n+ CFBundleVersion\n+ CFBundleShortVersionString"]
    C[_write_fake_archive helper] -->|writes Info.plist| B
    D[CMUX_FAKE_EMBED_INVALID_FRAMEWORK_SHELL=1] -->|injects bad framework| E[Iroh.framework shell\nno executable]
    B --> F[upload-testflight.sh]
    E --> F
    F -->|detects missing executable| G[Strips Iroh.framework]
    G --> H[Signs IPA without framework]
    H --> I{Test assertions}
    I -->|returncode == 0| J[✓ Upload succeeds]
    I -->|stripping message in stdout| K[✓ Reports reason]
    I -->|Iroh.framework absent in IPA| L[✓ Framework stripped]
Loading

Reviews (2): Last reviewed commit: "Update invalid framework upload regressi..." | Re-trigger Greptile

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 34444c7072

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@lawrencecchen
lawrencecchen force-pushed the task-ios-appstore-plistbuddy-fixture branch from 34444c7 to 811f4a3 Compare July 16, 2026 05:31
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Rebased onto 43072a493d. Updated the stale invalid-framework regression for the behavior merged in #8245: the manual export now succeeds, reports stripping the framework shell, and the final IPA contains no Payload/cmux.app/Frameworks/Iroh.framework/ entries.

Verification: full python3 tests/test_ios_appstore_lane_identity.py suite passed in a CI-compatible shell environment; focused beta identity and framework-strip cases also passed; git diff --check passed.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 811f4a3789

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

env = _base_env(tmp, fakebin)
env["CMUX_IOS_UPLOAD_DIR"] = str(tmp / "upload")
env["CMUX_FAKE_EMBED_FRAMEWORK_WITHOUT_MINIMUM_OS"] = "1"
env["CMUX_FAKE_EMBED_INVALID_FRAMEWORK_SHELL"] = "1"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restore MinimumOSVersion failure coverage

By replacing the old malformed-framework fixture with CMUX_FAKE_EMBED_INVALID_FRAMEWORK_SHELL, this test now covers only the re-sign strip path: the fake framework has no executable, so it is removed before verify_ipa_framework_minimum_os_versions can check MinimumOSVersion. I checked the suite for MinimumOSVersion, and there is no remaining test that embeds a valid dynamic-library framework missing that key, even though ios/scripts/upload-testflight.sh still treats that as a hard pre-upload failure. Please keep this shell-strip regression, but add/retain a separate dynamic-framework fixture for the missing/invalid MinimumOSVersion case so that validation path cannot regress silently.

Useful? React with 👍 / 👎.

@lawrencecchen
lawrencecchen merged commit ed0b3a6 into main Jul 16, 2026
7 checks passed
@lawrencecchen
lawrencecchen deleted the task-ios-appstore-plistbuddy-fixture branch July 16, 2026 09:28
@lawrencecchen
lawrencecchen restored the task-ios-appstore-plistbuddy-fixture branch July 18, 2026 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant