Skip to content

[eas-cli] Upload local Simulator app bundles - #4684

Open
gwdp wants to merge 4 commits into
mainfrom
gwdp/simulator-control-app-bundles
Open

gwdp wants to merge 4 commits into
mainfrom
gwdp/simulator-control-app-bundles

Conversation

@gwdp

@gwdp gwdp commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Why

simulator:control app.install rejects local .app directories even though they are the native Simulator app format.

Splits app-bundle uploads out of #4679; targets main.

Dependency

Requires expo/expo-device-hub#389 to be merged and released in the session's serve-sim version for local .app uploads to work. The CLI checks the host's uploadArchive: 'tar.gz' capability and reports an unsupported-host error when it is absent.

expo/expo-device-hub#393 is the host staging/concurrency follow-up; #389 provides the archive-install capability this CLI feature depends on.

How

  • Package a local .app directory as a gzip tar archive when the host advertises uploadArchive: 'tar.gz'.
  • Resolve symlinked bundles while preserving the supplied .app name in the archive.
  • Remove temporary archives after packaging, upload, or installation failures and after success.
  • Use .app in Simulator install examples and retain the existing file-path policy.
  • Reject local .ipa and .zip install files before uploading, with guidance to use a Simulator .app or .tar.gz/.tgz.
  • Show app-bundle guidance only for app.install; other upload actions ask for a file.

Test Plan

Codex ran 486 simulator/help tests across 21 suites, the CLI build, package typecheck, and changed-file lint/format checks. A combined checkout with #4681, #4682, and #4683 passed 498 tests. Tests inspect real archives and cover symlinks, host capability checks, and cleanup after failures.

Earlier local validation installed four app sources on an owned iOS Simulator through real host upload/install actions with stubbed cloud discovery/transport. That device validation was not repeated for this split. CI passed on 69e6e8c5b (including Node 22 and Node 24); Devin reviewed the same head and reported no issues. The unsupported-format follow-up is at 492a80772. The action-specific folder-message correction at 23c737a0c passed all 38 control tests, package typecheck, and changed-file lint/format checks; CI passed on that head, including Node 22/24, GraphQL, changeset and coverage checks. Devin completed analysis on the same head; there are no review threads. Hosted QA: Codex installed local .app, .tgz and literal path=fixture.app inputs through the four-fix CLI build 5f147368f, then verified exact launch arguments/deep links from the app itself. A JSON import also installed a local .app through the unpublished all-CLI integration dc6bfa8d6. Both used real EAS with host tip expo/expo-device-hub#393, which includes the required archive support from expo/expo-device-hub#389.

Authorship and LLM use

Gabe directed the fixes and requested separate PRs. Codex wrote, split, reviewed, and tested the implementation and description. Devin's earlier symlink finding is retained as a regression; Codex added packaging-failure cleanup coverage during the split. Gabe subsequently requested early rejection of unsupported formats; Codex added the guard and failing-first regressions. Gabe reviewed the final changes at 23c737a0c and requested Brent and Krystof for review.

Review follow-up at 17b990cf8: Codex serialized tar packaging, added scoped cancellation cleanup and visible packaging progress, checked known local archive capabilities and kept catalog parsing tolerant. Codex ran 529 tests plus build/typecheck/lint/format, Node 24 child tests, and a 548-test local integration with #4682/#4683; all pass. The hard-link fixture completes locally but does not reproduce Brent’s reported hang. Gabe reviewed the final changes at 17b990cf8, including this follow-up. Current-head CI passed, and Devin completed analysis with no open findings. Its last written no-issues report is on the earlier head.

— Codex

@gwdp

This comment has been minimized.

@devin-ai-integration

This comment has been minimized.

@devin-ai-integration devin-ai-integration 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.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@codecov

codecov Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.71930% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.44%. Comparing base (e8ca106) to head (17b990c).

Files with missing lines Patch % Lines
packages/eas-cli/src/simulator/appArchive.ts 81.25% 6 Missing ⚠️
packages/eas-cli/src/commands/simulator/control.ts 96.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4684      +/-   ##
==========================================
+ Coverage   75.43%   75.44%   +0.02%     
==========================================
  Files        1055     1056       +1     
  Lines       52404    52455      +51     
  Branches    11437    11446       +9     
==========================================
+ Hits        39526    39571      +45     
- Misses      11891    11897       +6     
  Partials      987      987              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gwdp

This comment has been minimized.

@devin-ai-integration

This comment has been minimized.

@gwdp

This comment has been minimized.

@devin-ai-integration

This comment has been minimized.

@gwdp
gwdp marked this pull request as ready for review October 18, 2026 04:20
@gwdp

gwdp commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

Codex implemented and tested the app-bundle packaging, unsupported-format rejection, and action-specific folder guidance under Gabe’s direction. Gabe confirmed review of the final changes at 23c737a0c and requested Brent and Krystof for review. No code changed during this handoff.

CI passed on the current head, including Node 22/24; Devin completed analysis on the same head, with no review threads. The description retains the dependency on released host support from expo/expo-device-hub#389 and the hosted validation boundary.

— Codex

@github-actions

Copy link
Copy Markdown

Subscribed to pull request

File Patterns Mentions
packages/eas-cli/** @douglowder

Generated by CodeMention

Warning: The preamble and epilogue options in commentConfiguration are deprecated. Use template instead.

@gwdp

gwdp commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor Author

Hosted QA: Codex installed local .app, .tgz and literal path=fixture.app inputs through the four-fix CLI build 5f147368f, then verified exact launch arguments/deep links from the app itself. A JSON import also installed a local .app through the unpublished all-CLI integration dc6bfa8d6. Both used real EAS with host tip expo/expo-device-hub#393, which includes the required archive support from expo/expo-device-hub#389.

— Codex

@brentvatne brentvatne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed at 23c737a with Codex, Grok and Claude reviewers; I checked each finding below against the code.

1. tar.create can hang forever on bundles with hard links (medium-low)

control.ts:313-322:

await tar.create(
  { cwd: bundle, file: archive, prefix: path.basename(path.resolve(value)), gzip: true, portable: true },
  ['.']
);

With tar 7.5.19 on Node 24, a bundle containing a hard-linked file next to framework Versions/Current symlinks hung in 29 of 30 runs; with jobs: 1 it completed 30 of 30. The cause is in node-tar's pack: the pending-links loop in [JOBDONE] starts new jobs without checking the job limit, after which [PROCESS] never reaches the head job. Upgrading tar does not fix it.

Because packaging runs before the spinner starts and the WebSocket keeps the process alive, the CLI stops silently after "Connected to simulator session". Xcode output has no hard links (the simulator bundles I checked had none and packaged fine); it takes a bundle that went through a hard-link-preserving copy or dedup step.

Fix: pass jobs: 1, add a regression test with a hard link, and start the spinner before packaging.

2. Interrupting leaves the temporary archive (low)

Cleanup runs only in the catch at :324 and the finally in runActionAsync. Ctrl-C during packaging or a long upload leaves $TMPDIR/eas-simulator-app-*/app.tar.gz, a compressed copy of the app. A one-shot SIGINT/exit handler that removes the directory synchronously would cover it.

3. Smaller items (low)

  • uploadArchive: z.string().optional() (hostActions.ts:34) makes a non-string value fail Catalog.safeParse, which breaks every action, not only app.install. No current host sends one; z.unknown() with the === 'tar.gz' check at packaging time would keep the catalog tolerant.
  • A prebuilt .tar.gz/.tgz file skips the capability check (it runs only for directories), so an older host fails in simctl. That matches the previous behavior for files, but the description's "The CLI checks the host's capability" covers .app directories only.
  • The host rejects bundles with absolute symlinks, over 2 GiB expanded, or over 100,000 entries only after the full upload. A local pre-check would save the upload.

Checked and fine: the archive keeps the supplied .app name (including through a symlinked bundle), executable bits and internal symlinks; links pointing outside the bundle are archived as links, not followed; temp directories come from mkdtemp; packaging and upload stream rather than buffer.

This PR and #4683 both rewrite findLocalUploadAsync from main, so whichever lands second needs a rebase.

gwdp and others added 4 commits October 22, 2026 23:54
Start progress before packaging, require archive support for local archives, and keep unknown capability metadata from rejecting the action catalog. Add child-process interruption and packaging coverage.

Co-authored-by: Codex <codex@openai.com>
@gwdp

gwdp commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

/devin review

Thanks @brentvatne for the archive review. Codex addressed the follow-up at 17b990cf8: packaging uses jobs: 1, progress starts before packaging, a small archive helper owns normal/interrupt/exit cleanup, local archives require the host capability, and unknown archive metadata no longer rejects the whole action catalog. Directory creation and cleanup registration share a synchronous turn to close the interrupt gap. Full local preflight policy duplication is deferred; the host remains authoritative.

Codex ran 529 simulator/help tests, build, typecheck and changed-file lint/format checks. Child-process packaging and SIGINT tests also pass on Node 24. Cancellation and capability regressions failed before the fix. Our bounded hard-link/framework fixture completes with and without jobs: 1; it does not independently reproduce the reported hang, so that fix relies on your reproduction and the checked scheduler code. Please share the exact reproducer if it differs, so we can add it.

Local adversarial review checked interruption during packaging and upload, handler removal, archive name preservation, capability fallback and progress ordering. A local integration with #4682 and #4683 passes 548 tests plus typecheck; those PRs remain independent. No new hosted QA. Gabe approved the direction; human inspection of this follow-up is pending. Approvals and reviewer requests were preserved.

Please review the current head, especially cancellation ownership, archive cleanup and capability parsing.

— Codex

@gwdp
gwdp force-pushed the gwdp/simulator-control-app-bundles branch from 23c737a to 17b990c Compare October 23, 2026 07:19
@devin-ai-integration

Copy link
Copy Markdown

Starting Devin Review.

Devin Review

@github-actions

Copy link
Copy Markdown

✅ Thank you for adding a changeset!

@gwdp
gwdp requested a review from brentvatne October 23, 2026 16:18
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.

2 participants