feat(publish): wire watchOS publishing + tvOS/watchOS App Store distribute#5085
Merged
Conversation
…ibute
watchOS had no publish path; this mirrors the tvOS wiring for watchOS and
adds App Store Connect distribute to both tvOS and watchOS.
- server_api: add tvos_distribute + watchos_{deployment_target,
encryption_exempt,info_plist,distribute} to BuildManifest
- config_types: add distribute to TvosConfig/WatchosConfig; entry to WatchosConfig
- mod.rs: is_watchos flag; watchOS entry (src/main_watchos.ts) + bundle_id
resolution; build_number gate; tvos/watchos distribute extraction +
manifest population; target_display + summary
- credentials: tvOS/watchOS appstore/testflight credential preflight;
watchOS added to interactive target picker
- preflight: classify watchOS as a gui app
A standalone watchOS app must have its own unique bundle id, so the watchOS
resolver does NOT fall back to the iOS bundle id, and appstore/testflight
publishing hard-requires an explicit [watchos] bundle_id (App Store Connect
rejects duplicates).
📝 WalkthroughWalkthroughThe PR extends the perry publish command to recognize watchOS as a target platform. It adds watchOS configuration options, integrates watchOS into platform selection prompts, enforces credential validation for App Store Connect distribution, and wires watchOS through entry-point resolution, bundle-ID assignment, build-number auto-increment, and build-manifest serialization to the build server. ChangeswatchOS Publish Support
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/perry/src/commands/publish/mod.rs (1)
1095-1110:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWatchOS-specific signing overrides are still ignored by publish.
This new preflight is watchOS-aware, but the Apple credential resolution above still only reads TOML values from
[ios]or[macos].crates/perry/src/commands/publish/config_types.rs:166-180defines[watchos].team_idand[watchos].signing_identity, soperry publish watchoscurrently ignores those overrides and silently falls back to macOS/global credentials.Either wire watchOS to its own TOML overrides or explicitly delegate it to the iOS config path; the current macOS fallback is the wrong contract for a standalone watchOS publish flow.
🤖 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 `@crates/perry/src/commands/publish/mod.rs` around lines 1095 - 1110, The publish flow is ignoring watchOS-specific TOML overrides ([watchos].team_id, [watchos].signing_identity); update the credential resolution so validate_credentials_for_distribute receives watchOS values instead of silently falling back to macOS. Concretely, when is_watchos is true pass the watchOS distribute config (watchos_distribute.as_deref()) or explicitly delegate to the iOS path (ios_distribute.as_deref()) depending on the intended contract, so that validate_credentials_for_distribute(...) sees the watchOS overrides rather than macOS/global values.
🤖 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 `@crates/perry/src/commands/publish/mod.rs`:
- Around line 307-328: The code increments build_number (computed from
toml_build_number and written back to perry_toml_path) for tvOS/watchOS but does
not propagate that new value into the app manifest (CFBundleVersion) because the
manifest serialization currently only applies to iOS/visionOS/Android/macOS;
update the manifest-write logic so the same build_number (the variable
build_number / n) is written into CFBundleVersion for tvOS and watchOS as well
(i.e., include tvOS/watchOS in the branch that serializes CFBundleVersion or
explicitly set CFBundleVersion = build_number when handling tvOS/watchOS) so the
worker receives the incremented CFBundleVersion used for upload.
---
Outside diff comments:
In `@crates/perry/src/commands/publish/mod.rs`:
- Around line 1095-1110: The publish flow is ignoring watchOS-specific TOML
overrides ([watchos].team_id, [watchos].signing_identity); update the credential
resolution so validate_credentials_for_distribute receives watchOS values
instead of silently falling back to macOS. Concretely, when is_watchos is true
pass the watchOS distribute config (watchos_distribute.as_deref()) or explicitly
delegate to the iOS path (ios_distribute.as_deref()) depending on the intended
contract, so that validate_credentials_for_distribute(...) sees the watchOS
overrides rather than macOS/global values.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 028c9110-e83a-4b9a-b286-827acd6cab66
📒 Files selected for processing (5)
crates/perry/src/commands/publish/config_types.rscrates/perry/src/commands/publish/credentials.rscrates/perry/src/commands/publish/mod.rscrates/perry/src/commands/publish/preflight.rscrates/perry/src/commands/publish/server_api.rs
Comment on lines
+307
to
+328
| let build_number = | ||
| if is_ios || is_visionos || is_tvos || is_watchos || is_android || macos_needs_upload { | ||
| let n = toml_build_number + 1; | ||
| if let Ok(content) = fs::read_to_string(&perry_toml_path) { | ||
| let updated = if content.contains("build_number =") { | ||
| content.replace( | ||
| &format!("build_number = {}", toml_build_number), | ||
| &format!("build_number = {}", n), | ||
| ) | ||
| } else { | ||
| // Insert build_number after the version line | ||
| content.replace( | ||
| &format!("version = \"{}\"", version), | ||
| &format!("version = \"{}\"\nbuild_number = {}", version, n), | ||
| ) | ||
| }; | ||
| fs::write(&perry_toml_path, &updated).ok(); | ||
| } | ||
| n | ||
| } else { | ||
| toml_build_number | ||
| }; |
There was a problem hiding this comment.
Propagate the new tvOS/watchOS build number into the manifest too.
This block now increments build_number for tvOS/watchOS, but Lines 1281-1286 still serialize build-number semantics only for iOS/visionOS/Android/macOS uploads. The worker therefore receives the marketing version for tvOS/watchOS, so a second App Store/TestFlight publish can reuse the old CFBundleVersion and get rejected.
Suggested fix
- let (manifest_version, manifest_short_version) =
- if is_ios || is_visionos || is_android || macos_needs_upload {
+ let (manifest_version, manifest_short_version) =
+ if is_ios || is_visionos || is_tvos || is_watchos || is_android || macos_needs_upload {
(build_number.to_string(), Some(version.clone()))
} else {
(version.clone(), None)
};🤖 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 `@crates/perry/src/commands/publish/mod.rs` around lines 307 - 328, The code
increments build_number (computed from toml_build_number and written back to
perry_toml_path) for tvOS/watchOS but does not propagate that new value into the
app manifest (CFBundleVersion) because the manifest serialization currently only
applies to iOS/visionOS/Android/macOS; update the manifest-write logic so the
same build_number (the variable build_number / n) is written into
CFBundleVersion for tvOS and watchOS as well (i.e., include tvOS/watchOS in the
branch that serializes CFBundleVersion or explicitly set CFBundleVersion =
build_number when handling tvOS/watchOS) so the worker receives the incremented
CFBundleVersion used for upload.
…st call sites - Extract per-target entry/bundle_id resolution from mod.rs into resolve.rs (mod.rs was 2039 lines, over the 2000-line lint gate -> now 1954). - validate_credentials_for_distribute gained 4 params (is_tvos/tvos_distribute, is_watchos/watchos_distribute); update the 10 #[cfg(test)] call sites that cargo check skips but cargo test compiles, and add tvOS/watchOS credential validation tests.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/perry/src/commands/publish/tests.rs (1)
266-560: ⚡ Quick winReduce positional-argument brittleness in credential validation tests.
These tests repeatedly pass long
bool/Optiontuples, which is easy to misorder as the validator signature evolves. Consider a tiny test helper (or per-platform wrappers) to centralize defaults and make each case intent-focused.Refactor sketch
+fn validate_ios_case( + ios_distribute: Option<&str>, + apple_key_id: Option<&str>, + apple_issuer_id: Option<&str>, + p8_key_content: Option<&str>, +) -> anyhow::Result<()> { + validate_credentials_for_distribute( + false, None, None, // android + true, ios_distribute, apple_key_id, apple_issuer_id, p8_key_content, // ios + false, None, // macos + false, None, // tvos + false, None, // watchos + ) +}🤖 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 `@crates/perry/src/commands/publish/tests.rs` around lines 266 - 560, The tests in tests.rs call validate_credentials_for_distribute with long positional bool/Option tuples making them brittle; create a small test helper (e.g., a builder or wrapper function test_validate_args or per-platform helpers like android_args, ios_args, macos_args) that supplies sensible defaults and accepts only the values each test cares about, then update each test to call the helper with named parameters (or a short struct) and pass the resulting fields into validate_credentials_for_distribute; reference the existing validate_credentials_for_distribute call sites to replace direct long tuples with helper-produced arguments, and keep helper code local to the tests module to avoid touching production code.crates/perry/src/commands/publish/resolve.rs (1)
79-79: ⚡ Quick winConsider sanitizing app_name more thoroughly for bundle ID generation.
The default bundle ID generator only replaces spaces with hyphens but doesn't sanitize other special characters. If
app_namecontains characters like@,#,!, or Unicode, the generated bundle IDcom.perry.{app_name}could be invalid (bundle IDs should only contain alphanumeric characters, hyphens, and periods).♻️ Proposed sanitization improvement
- let default = || format!("com.perry.{}", app_name.to_lowercase().replace(' ', "-")); + let default = || { + let sanitized = app_name + .to_lowercase() + .chars() + .map(|c| if c.is_alphanumeric() || c == '-' || c == '.' { c } else { '-' }) + .collect::<String>(); + format!("com.perry.{}", sanitized) + };🤖 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 `@crates/perry/src/commands/publish/resolve.rs` at line 79, The bundle ID default closure currently only replaces spaces; update it to robustly sanitize app_name: create/inline a small sanitizer used by the default closure that lowercases app_name, normalizes or strips/transliterates diacritics if available, replaces any character not in [a-z0-9-] with a hyphen (using a regex like [^a-z0-9-]+), collapses consecutive hyphens into one, and trims leading/trailing hyphens so the resulting format is safe for "com.perry.{sanitized}". Modify the existing closure (the local variable default that references app_name) to call this sanitizer before building the final string; ensure the sanitizer function name (e.g., sanitize_bundle_component) is unique and used by the default closure.
🤖 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.
Nitpick comments:
In `@crates/perry/src/commands/publish/resolve.rs`:
- Line 79: The bundle ID default closure currently only replaces spaces; update
it to robustly sanitize app_name: create/inline a small sanitizer used by the
default closure that lowercases app_name, normalizes or strips/transliterates
diacritics if available, replaces any character not in [a-z0-9-] with a hyphen
(using a regex like [^a-z0-9-]+), collapses consecutive hyphens into one, and
trims leading/trailing hyphens so the resulting format is safe for
"com.perry.{sanitized}". Modify the existing closure (the local variable default
that references app_name) to call this sanitizer before building the final
string; ensure the sanitizer function name (e.g., sanitize_bundle_component) is
unique and used by the default closure.
In `@crates/perry/src/commands/publish/tests.rs`:
- Around line 266-560: The tests in tests.rs call
validate_credentials_for_distribute with long positional bool/Option tuples
making them brittle; create a small test helper (e.g., a builder or wrapper
function test_validate_args or per-platform helpers like android_args, ios_args,
macos_args) that supplies sensible defaults and accepts only the values each
test cares about, then update each test to call the helper with named parameters
(or a short struct) and pass the resulting fields into
validate_credentials_for_distribute; reference the existing
validate_credentials_for_distribute call sites to replace direct long tuples
with helper-produced arguments, and keep helper code local to the tests module
to avoid touching production code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 85048080-e744-4b05-95d8-c54b75e9bfa7
📒 Files selected for processing (3)
crates/perry/src/commands/publish/mod.rscrates/perry/src/commands/publish/resolve.rscrates/perry/src/commands/publish/tests.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/perry/src/commands/publish/mod.rs
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.
What
Adds first-class watchOS publishing and App Store Connect distribute for both tvOS and watchOS to
perry publish. watchOS previously had no publish path (the[watchos]manifest wiring was missing); tvOS could build but had nodistributeoption.Companion changes (separate): hub PR (license + advertise tvos/watchos) and worker deploy patches (build/sign/upload). This PR is the CLI half.
Changes
BuildManifest: addtvos_distribute+watchos_{deployment_target,encryption_exempt,info_plist,distribute}.distributeonTvosConfig/WatchosConfig;entryonWatchosConfig.is_watchos; watchOS entry (src/main_watchos.ts) + bundle-id resolution;build_numbergate; tvOS/watchOSdistributeextraction + manifest population;target_display+ publish summary.appstore/testflightcredential preflight (App Store Connect key id / issuer / .p8); watchOS added to the interactive target picker.guiapp.Standalone watchOS bundle id
perry publish watchosships an independent/standalone watchOS app, which Apple requires to have its own unique bundle id. The watchOS resolver therefore does not fall back to the iOS bundle id (unlike tvOS), andappstore/testflightpublishing hard-requires an explicit[watchos] bundle_id— otherwise App Store Connect rejects the duplicate id.Notes
ios-precompiled → ios-signbuild/sign path (they sign/package exactly like iOS), so no new server capability is introduced.Test
cargo check -p perrypasses. Manual:perry publish watchos/tvoson a sample app builds the expected manifest (targets,watchos_*/tvos_*fields) and rejects missing App Store Connect creds / missing[watchos] bundle_id.Summary by CodeRabbit
New Features
Bug Fixes / Validation
Tests