feat(upload): pre-flight validate-app, body-aware success, precise package errors
Implements the 3 biggest wins from the Siti-AI release findings plan: - #1 altool --validate-app pre-flight: new AltoolOp enum shares the argv builder between validate/upload; validate_app() and an `apps validate` CLI subcommand run App Store Connect's checks in seconds without delivering the binary. --wait is upload-only and dropped for validation. - #3 body-aware success: outcome() no longer trusts exit 0 alone; a non-empty product-errors/errors array in altool's JSON body now marks the outcome as failed. - #4 precise package errors: validate_package() names an .app bundle and distinguishes a missing path from a directory instead of a generic "does not exist". Tests cover the validate argv, the .app error, and body-error-on-exit-0. Co-Authored-By: siGit Code v1.6.6-acp <noreply@sigit.si>
Seto Elkahfi committed
Oct 11, 2026 at 14:24 UTC
9c5cd2b7f5508027084d05a2cb05a63c09f66cb8
2 files changed
+166
-10
crates/cli/src/main.rs
+31
-2
@@ -16,7 +16,7 @@ use smbcloud_ascapi_aso::bundle_id::{BundleIdCreateAttributes, BundleIdPlatform}
16
use smbcloud_ascapi_aso::prelude::*;
17
use smbcloud_ascapi_core::{ApiKey, Client};
18
use smbcloud_ascapi_frontend::review;
19
-use smbcloud_ascapi_frontend::upload::{self, upload_package, AltoolAuth};
19
+use smbcloud_ascapi_frontend::upload::{self, upload_package, validate_app, AltoolAuth, AltoolOp};
20
use smbcloud_ascapi_pricing::app_price::PriceKind;
21
use smbcloud_ascapi_pricing::prelude::*;
22
use smbcloud_ascapi_reports::prelude::*;
@@ -427,6 +427,13 @@ enum AppsCommand {
427
#[arg(long, value_enum)]
428
platform: Option<CliPlatform>,
429
},
430
+ /// Pre-flight an exported `.ipa`/`.pkg` with `xcrun altool --validate-app`:
431
+ /// run App Store Connect's checks without delivering the binary. Catches
432
+ /// most of what a failed upload would, in seconds. Run this before `upload`.
433
+ Validate {
434
+ /// The exported package (`xcodebuild -exportArchive` output).
435
+ package: PathBuf,
436
+ },
437
/// Upload an exported `.ipa`/`.pkg` with `xcrun altool --upload-package`,
438
/// authenticated with the same key and `.p8` as every other command.
439
/// Afterwards, find the build with `apps builds <APP_ID> --version <N>`.
@@ -942,11 +949,33 @@ async fn run_apps(
949
let platform = platform.map(Platform::from);
950
print_json(&client.list_pre_release_versions(&app_id, platform).await?)
951
}
952
+ AppsCommand::Validate { package } => {
953
+ if dry_run {
954
+ let mut argv = vec!["xcrun".to_string()];
955
+ argv.extend(
956
+ upload::altool_args(&package, altool_auth, AltoolOp::Validate, false)
957
+ .iter()
958
+ .map(|arg| arg.to_string_lossy().into_owned()),
959
+ );
960
+ return print_json(&serde_json::json!({ "dry_run": true, "command": argv }));
961
+ }
962
+ let auth = altool_auth.clone();
963
+ let outcome = tokio::task::spawn_blocking(move || validate_app(&package, &auth))
964
+ .await?
965
+ .map_err(anyhow::Error::msg)?;
966
+ print_json(&outcome)?;
967
+ match outcome.exit_code {
968
+ _ if outcome.success => {}
969
+ Some(code) => anyhow::bail!("altool validation failed with exit code {code}"),
970
+ None => anyhow::bail!("altool validation was killed by a signal"),
971
+ }
972
+ Ok(())
973
+ }
974
AppsCommand::Upload { package, wait } => {
975
if dry_run {
976
let mut argv = vec!["xcrun".to_string()];
977
argv.extend(
949
- upload::altool_args(&package, altool_auth, wait)
978
+ upload::altool_args(&package, altool_auth, AltoolOp::Upload, wait)
979
.iter()
980
.map(|arg| arg.to_string_lossy().into_owned()),
981
);
crates/frontend/src/upload.rs
+135
-8
@@ -45,12 +45,34 @@ pub struct UploadOutcome {
45
pub output: serde_json::Value,
46
}
47
48
+/// Which altool operation to run against the package: the pre-flight
49
+/// `--validate-app` or the real `--upload-package`. They take the same
50
+/// authentication and the same output format, so they share one argv
51
+/// builder.
52
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
53
+pub enum AltoolOp {
54
+ /// `--validate-app`: run App Store Connect's checks without delivering
55
+ /// the binary. Catches most of what a failed upload would, in seconds.
56
+ Validate,
57
+ /// `--upload-package`: deliver the binary for real.
58
+ Upload,
59
+}
60
+
61
+impl AltoolOp {
62
+ fn flag(self) -> &'static str {
63
+ match self {
64
+ AltoolOp::Validate => "--validate-app",
65
+ AltoolOp::Upload => "--upload-package",
66
+ }
67
+ }
68
+}
69
+
70
/// The arguments after `xcrun`. Separate from [`upload_package`] so the
71
/// exact command line is testable and `--dry-run` can print it.
50
-pub fn altool_args(package: &Path, auth: &AltoolAuth, wait: bool) -> Vec<OsString> {
72
+pub fn altool_args(package: &Path, auth: &AltoolAuth, op: AltoolOp, wait: bool) -> Vec<OsString> {
73
let mut args: Vec<OsString> = vec![
74
"altool".into(),
53
- "--upload-package".into(),
75
+ op.flag().into(),
76
package.as_os_str().to_owned(),
77
"--api-key".into(),
78
auth.key_id.clone().into(),
@@ -61,7 +83,9 @@ pub fn altool_args(package: &Path, auth: &AltoolAuth, wait: bool) -> Vec<OsStrin
83
"--output-format".into(),
84
"json".into(),
85
];
64
- if wait {
86
+ // --wait only means anything for an upload; validation is already
87
+ // synchronous, so don't pass a flag altool would reject there.
88
+ if wait && op == AltoolOp::Upload {
89
args.push("--wait".into());
90
}
91
args
@@ -75,6 +99,16 @@ pub fn validate_package(package: &Path) -> Result<(), String> {
99
.map(str::to_ascii_lowercase);
100
match extension.as_deref() {
101
Some("ipa") | Some("pkg") => {}
102
+ // A `.app` is a directory, so the generic "not a file" check below
103
+ // would mislabel the most common mistake — pointing altool at the
104
+ // built app instead of the exported archive. Name it precisely.
105
+ Some("app") => {
106
+ return Err(format!(
107
+ "{} is an .app bundle, not an uploadable package; export the \
108
+ archive first (xcodebuild -exportArchive) to get an .ipa/.pkg",
109
+ package.display()
110
+ ))
111
+ }
112
_ => {
113
return Err(format!(
114
"{} is not an .ipa or .pkg; export the archive first \
@@ -83,9 +117,17 @@ pub fn validate_package(package: &Path) -> Result<(), String> {
117
))
118
}
119
}
86
- if !package.is_file() {
120
+ // Distinguish "nothing here" from "here, but a directory" so the error
121
+ // points at the real problem instead of a generic "does not exist".
122
+ if !package.exists() {
123
return Err(format!("{} does not exist", package.display()));
124
}
125
+ if !package.is_file() {
126
+ return Err(format!(
127
+ "{} is a directory, not a package file",
128
+ package.display()
129
+ ));
130
+ }
131
Ok(())
132
}
133
@@ -103,6 +145,27 @@ pub fn upload_package(
145
package: &Path,
146
auth: &AltoolAuth,
147
wait: bool,
148
+) -> Result<UploadOutcome, String> {
149
+ run_altool(package, auth, AltoolOp::Upload, wait)
150
+}
151
+
152
+/// Run `xcrun altool --validate-app` and report what happened.
153
+///
154
+/// A pre-flight check that runs App Store Connect's validation without
155
+/// delivering the binary: it catches most of what a failed upload would
156
+/// (bad entitlements, missing icons, bundle-id mismatches) in seconds
157
+/// instead of minutes, before anything is uploaded. Same success/`Err`
158
+/// contract as [`upload_package`].
159
+pub fn validate_app(package: &Path, auth: &AltoolAuth) -> Result<UploadOutcome, String> {
160
+ // --wait is upload-only; validation is already synchronous.
161
+ run_altool(package, auth, AltoolOp::Validate, false)
162
+}
163
+
164
+fn run_altool(
165
+ package: &Path,
166
+ auth: &AltoolAuth,
167
+ op: AltoolOp,
168
+ wait: bool,
169
) -> Result<UploadOutcome, String> {
170
validate_package(package)?;
171
if !auth.private_key_path.is_file() {
@@ -113,7 +176,7 @@ pub fn upload_package(
176
}
177
178
let output = Command::new("xcrun")
116
- .args(altool_args(package, auth, wait))
179
+ .args(altool_args(package, auth, op, wait))
180
.stdin(Stdio::null())
181
.stderr(Stdio::inherit())
182
.output()
@@ -124,15 +187,37 @@ pub fn upload_package(
187
188
fn outcome(package: &Path, exit_code: Option<i32>, stdout: &[u8]) -> UploadOutcome {
189
let output = parse_output(stdout);
190
+ // A zero exit is necessary but not sufficient: altool has been seen to
191
+ // exit 0 while reporting errors in its JSON body (e.g. the
192
+ // `product-errors` array). Trust the exit code only when the body
193
+ // doesn't contradict it.
194
+ let success = exit_code == Some(0) && !has_errors(&output);
195
UploadOutcome {
196
package: package.display().to_string(),
129
- success: exit_code == Some(0),
197
+ success,
198
exit_code,
199
delivery_id: find_delivery_id(&output),
200
output,
201
}
202
}
203
204
+/// altool reports failures in its JSON body under a handful of keys
205
+/// (`product-errors` / `product_errors` for upload, `errors` for
206
+/// validation). A non-empty array under any of them means the operation
207
+/// failed regardless of the exit code.
208
+fn has_errors(value: &serde_json::Value) -> bool {
209
+ const ERROR_KEYS: [&str; 3] = ["product-errors", "product_errors", "errors"];
210
+ match value {
211
+ serde_json::Value::Object(map) => {
212
+ ERROR_KEYS.iter().any(|key| {
213
+ matches!(map.get(*key), Some(serde_json::Value::Array(errors)) if !errors.is_empty())
214
+ }) || map.values().any(has_errors)
215
+ }
216
+ serde_json::Value::Array(items) => items.iter().any(has_errors),
217
+ _ => false,
218
+ }
219
+}
220
+
221
/// With `--wait`, altool writes two JSON documents back to back: the build's
222
/// processing status, then the upload result. Read them as a stream and
223
/// return an array when there is more than one. Anything that isn't JSON
@@ -183,7 +268,12 @@ mod tests {
268
269
#[test]
270
fn passes_the_resolved_key_file_rather_than_letting_altool_search() {
186
- let args = strings(altool_args(Path::new("/out/Klepon.ipa"), &auth(), false));
271
+ let args = strings(altool_args(
272
+ Path::new("/out/Klepon.ipa"),
273
+ &auth(),
274
+ AltoolOp::Upload,
275
+ false,
276
+ ));
277
assert_eq!(
278
args,
279
[
@@ -204,11 +294,38 @@ mod tests {
294
295
#[test]
296
fn never_puts_key_material_on_the_command_line() {
207
- let args = strings(altool_args(Path::new("/out/Klepon.ipa"), &auth(), true));
297
+ let args = strings(altool_args(
298
+ Path::new("/out/Klepon.ipa"),
299
+ &auth(),
300
+ AltoolOp::Upload,
301
+ true,
302
+ ));
303
assert!(!args.iter().any(|arg| arg == "--auth-string"));
304
assert_eq!(args.last().map(String::as_str), Some("--wait"));
305
}
306
307
+ #[test]
308
+ fn validation_uses_validate_app_and_ignores_wait() {
309
+ let args = strings(altool_args(
310
+ Path::new("/out/Klepon.ipa"),
311
+ &auth(),
312
+ AltoolOp::Validate,
313
+ true,
314
+ ));
315
+ assert_eq!(args[1], "--validate-app");
316
+ assert!(
317
+ !args.iter().any(|arg| arg == "--wait"),
318
+ "--wait is upload-only: {args:?}"
319
+ );
320
+ }
321
+
322
+ #[test]
323
+ fn points_at_the_archive_when_given_an_app_bundle() {
324
+ let error = validate_package(Path::new("/out/Klepon.app")).unwrap_err();
325
+ assert!(error.contains(".app"), "unhelpful error: {error}");
326
+ assert!(error.contains("exportArchive"), "unhelpful error: {error}");
327
+ }
328
+
329
#[test]
330
fn rejects_an_unexported_archive() {
331
let error = validate_package(Path::new("/out/Klepon.xcarchive")).unwrap_err();
@@ -251,6 +368,16 @@ mod tests {
368
assert_eq!(documents[0]["build-status"], "VALID");
369
}
370
371
+ #[test]
372
+ fn reports_failure_when_altool_exits_zero_but_the_body_has_errors() {
373
+ let body = br#"{"product-errors":[{"message":"Invalid bundle"}]}"#;
374
+ let outcome = outcome(Path::new("/out/Klepon.ipa"), Some(0), body);
375
+ assert!(
376
+ !outcome.success,
377
+ "a non-empty product-errors array means failure even on exit 0"
378
+ );
379
+ }
380
+
381
#[test]
382
fn keeps_non_json_output_as_text_and_reports_failure() {
383
let outcome = outcome(