From c085f687a7103e224727b77c4837ce95c00e10d6 Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Sat, 19 Sep 2026 08:29:14 +0500 Subject: [PATCH] feat(upload): make --artifact optional on `upload build` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Omitting it registers the build and ships no bytes (`request_artifact_upload: false`). That mode already existed β€” `build::run` skips packing and never reads the artefact when the flag is false, and `xcode post-action` uses it β€” but no CLI caller could reach it: `--artifact` was required and the flag hard-coded `true`. It is the default path on Android and iOS, which ship bytes only when size analysis is opted into, and the only path a web build can express, having no single artefact. `--deps`/`--timings` still travel without an artefact (the build-info bundle is a separate upload). `--mapping`, `--chunked` and `--out` are rejected with exit 20 rather than ignored: dropping a `--mapping` silently would cost symbolication. Tests: 6 CLI-level, asserting both directions of the flag; 3 mutants caught; 1 e2e flow asserting the payload is intact and zero PUTs happen. First slice of web-build registration β€” docs/design/web-build-registration.md in the JS repo. πŸ€– Generated with [Claude Code](https://claude.com/claude-code) --- CHANGELOG.md | 16 +++++ README.md | 28 ++++++++ scripts/e2e_flows.py | 16 +++++ src/cli/upload.rs | 34 ++++++++-- tests/debug_files_flags.rs | 127 +++++++++++++++++++++++++++++++++++++ 5 files changed, 216 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e584bb2..5e20870 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,22 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added +- **`upload build` can register a build without shipping the artefact's bytes** β€” omit `--artifact`. + That is the normal case on every platform that has not opted into size analysis (Android unless + `sizeAnalysis.enabled`, the iOS post-action unless `BUGSEE_SIZE_ANALYSIS_ENABLED`), and it is the + only case a **web build** can express, having no single artefact to ship. The mode already existed + inside `build::Params` and `xcode post-action` used it; nothing on the command line could reach it, + because `--artifact` was required and `request_artifact_upload` was hard-coded `true`. + + `--deps` and `--timings` still travel without an artefact β€” the build-info bundle is a separate + upload. The flags that only describe how artefact bytes move (`--mapping`, `--chunked`, `--out`) are + rejected with exit 20 rather than ignored: silently dropping a `--mapping` would cost symbolication, + and a caller who passed `--chunked` is saying they expect bytes to move. + + Groundwork for web-build registration β€” the design and its decisions are in the JS repo, + `docs/design/web-build-registration.md`. + ## [0.7.11] - 2026-09-18 ### Added diff --git a/README.md b/README.md index 5cd9877..5a3bd36 100644 --- a/README.md +++ b/README.md @@ -294,6 +294,34 @@ bugsee-cli debug-files upload --type sourcemaps ... --version --build [--concurrency N] [--allow-empty] [--strip-sources-content] ``` +### `upload build` + +Registers a build record β€” the thing a symbol upload, a size analysis and a crash all hang off β€” and, +optionally, ships the build artefact's bytes. + +``` +bugsee-cli upload build --payload-json \ + [--artifact <.aab|.apk|.ipa>] # omit to REGISTER ONLY, shipping no bytes \ + [--mapping ] # needs --artifact (it rides inside the ZIP) \ + [--deps ] [--timings ] \ + [--chunked] # needs --artifact \ + [--dry-run [--out ]] # --out needs --artifact +``` + +`--payload-json` is the registration body, written by the producer (the Gradle plugin, the Xcode +post-action, a bundler plugin) and passed through verbatim apart from two fields the CLI injects: +`request_artifact_upload`, and `request_build_info_upload` when a sidecar is present. + +**Without `--artifact` the build is registered and nothing is packed or sent.** That is the normal +case wherever size analysis is not enabled β€” and the only case a web build can express, having no +single artefact to ship. `--deps`/`--timings` still travel, because the build-info bundle is a +separate upload from the artefact. The flags that only describe how artefact bytes move (`--mapping`, +`--chunked`, `--out`) are rejected with exit 20 rather than ignored: dropping a `--mapping` silently +would cost symbolication. + +Dedup is server-side on the payload's `uuid` (replace-then-create), which is why the registration POST +is retried on a transport error but never on a 5xx. + ### `xcode upload-dsyms` Uploads dSYMs from an Xcode **Run Script build phase**, with none of the diff --git a/scripts/e2e_flows.py b/scripts/e2e_flows.py index 4c8b441..2e586e0 100644 --- a/scripts/e2e_flows.py +++ b/scripts/e2e_flows.py @@ -518,6 +518,10 @@ def main(): "--mapping", os.path.join(fix, "mapping.txt")]) results["build_chunked"] = run(binpath, "build_chunked", ["upload", "build", "--payload-json", pj, "--artifact", os.path.join(fix, "app.aab"), "--chunked"]) + # No --artifact: the build is REGISTERED and no bytes move. This is the normal case on every + # platform that has not opted into size analysis, and the only one a web build can express. + results["build_register_only"] = run( + binpath, "build_register_only", ["upload", "build", "--payload-json", pj]) results["build_info"] = run(binpath, "build_info", ["upload", "build-info", "--payload-json", pj, "--deps", os.path.join(fix, "deps.json"), "--timings", os.path.join(fix, "timings.json")]) @@ -563,6 +567,18 @@ def main(): results["build_requests_artifact_upload"] = (bp.get("request_artifact_upload") is True) except Exception: results["build_requests_artifact_upload"] = False + try: + rp = json.load(open(cappath("build_register_only__builds_post.json"))) + # Registered, with the producer's payload intact, and asking for no artefact upload… + registered = (rp.get("request_artifact_upload") is False and rp.get("uuid") == bp.get("uuid")) + # …and no artefact PUT happened at all. `puts` counts artefact/symbol PUTs per flow. + results["build_register_only_ships_no_bytes"] = ( + registered and STATE["puts"].get("build_register_only", 0) == 0) + if not results["build_register_only_ships_no_bytes"]: + print(f" [FAIL] register-only payload={rp!r} puts={STATE['puts'].get('build_register_only')!r}") + except Exception as e: + print(" [FAIL] could not verify the register-only build:", e) + results["build_register_only_ships_no_bytes"] = False srv.shutdown() if not a.keep: diff --git a/src/cli/upload.rs b/src/cli/upload.rs index c1fdd88..b11b529 100644 --- a/src/cli/upload.rs +++ b/src/cli/upload.rs @@ -6,7 +6,7 @@ //! through `debug-files upload`. use clap::Subcommand; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use crate::compress; use crate::error::{config_invalid, input_not_found}; @@ -80,8 +80,14 @@ pub enum UploadCommand { payload_json: PathBuf, /// Build artefact (`.aab`/`.apk`/`.ipa`), STORED verbatim in the ZIP. + /// + /// OPTIONAL. Without it the build is REGISTERED and no bytes are shipped β€” which is the + /// normal case on every platform that does not opt into size analysis, and the only case a + /// web build can express (it has no single artefact to ship). The payload still carries the + /// producer's `uuid`, `version` and the rest, and `--deps`/`--timings` still travel, because + /// the build-info bundle is a separate upload from the artefact. #[arg(long)] - artifact: PathBuf, + artifact: Option, /// Optional R8/ProGuard mapping.txt, zstd-packed alongside the artefact. #[arg(long)] @@ -243,14 +249,32 @@ pub async fn dispatch( } } } + // Flags that only describe how ARTEFACT bytes travel, or what rides inside the + // artefact ZIP, are contradictions without one. Rejected rather than ignored: silently + // dropping a `--mapping` costs symbolication, and a caller who passed `--chunked` is + // telling us they expect bytes to move. + if artifact.is_none() { + for (flag, present) in [ + ("--mapping", mapping.is_some()), + ("--chunked", chunked), + ("--out", out.is_some()), + ] { + if present { + return Err(config_invalid(format!( + "{flag} needs --artifact: without an artefact the build is only \ + registered and no bytes are packed or sent" + ))); + } + } + } let endpoint = endpoint.unwrap_or_else(|| DEFAULT_ENDPOINT.to_string()); let params = build::Params { endpoint: &endpoint, app_token, payload_json: &payload_json, - artifact: &artifact, - // `upload build` exists to ship the artefact β€” always request it. - request_artifact_upload: true, + // Read only when bytes ship; `build::run` never touches it otherwise. + artifact: artifact.as_deref().unwrap_or(Path::new("")), + request_artifact_upload: artifact.is_some(), mapping: mapping.as_deref(), deps: deps.as_deref(), timings: timings.as_deref(), diff --git a/tests/debug_files_flags.rs b/tests/debug_files_flags.rs index 5f119e2..81c0461 100644 --- a/tests/debug_files_flags.rs +++ b/tests/debug_files_flags.rs @@ -14,6 +14,8 @@ use assert_cmd::Command; use predicates::str::contains; use std::path::PathBuf; +mod common; + const CONFIG_INVALID: i32 = 20; fn upload(kind: &str, extra: &[&str]) -> Command { @@ -202,3 +204,128 @@ fn sourcemap_only_flags_are_accepted_for_sourcemaps() { .assert() .success(); } + +/// `upload build` registers a build record AND ships the artefact's bytes. Every other platform +/// treats "register, ship no bytes" as the NORMAL case β€” it is what Android does unless +/// `sizeAnalysis.enabled` is set, and what the iOS post-action does unless +/// `BUGSEE_SIZE_ANALYSIS_ENABLED` is β€” but the flag that expresses it existed only inside the Rust +/// `build::Params`, reachable from `xcode post-action` and from nothing on the command line. A web +/// build has no `.aab`/`.ipa` to ship, so that was the one case it could not express. +mod register_only_build { + use super::*; + + fn build_cmd(extra: &[&str]) -> Command { + let tmp = std::env::temp_dir(); + let payload = tmp.join("bugsee-register-only-payload.json"); + std::fs::write( + &payload, + br#"{"uuid":"deadbeefdeadbeefdeadbeefdeadbeef","version":"1.2.3","format":"web"}"#, + ) + .unwrap(); + let mut c = common::cli(); + c.args([ + "--app-token", + "TKN", + "--endpoint", + "http://127.0.0.1:1", + "upload", + "build", + "--payload-json", + payload.to_str().unwrap(), + ]) + .args(extra); + c + } + + /// tracing styles the field name and its value separately, so `contains` over the raw stderr + /// cannot see `name=value` as one string. Stripping the escapes keeps the assertion on the VALUE + /// β€” a weaker match on the field name alone would pass for `true` just as happily. + fn plain(text: &str) -> String { + let mut out = String::with_capacity(text.len()); + let mut chars = text.chars(); + while let Some(c) = chars.next() { + if c == '\u{1b}' { + for skip in chars.by_ref() { + if skip == 'm' { + break; + } + } + } else { + out.push(c); + } + } + out + } + + /// No `--artifact`: the build registers and no bytes are packed or sent. + #[test] + fn a_build_with_no_artifact_registers_and_ships_nothing() { + build_cmd(&["--dry-run"]) + .assert() + .success() + .stderr(predicates::function::function(|s: &str| { + plain(s).contains("request_artifact_upload=false") + })); + } + + /// …and WITH one, the same log line says bytes are going β€” the other half of the same assertion, + /// so "ships nothing" cannot be satisfied by a run that never looked at the flag. + #[test] + fn a_build_with_an_artifact_still_requests_the_upload() { + let artifact = std::env::temp_dir().join("bugsee-register-only-app.aab"); + std::fs::write(&artifact, b"PK\x03\x04 fake aab").unwrap(); + let out = std::env::temp_dir().join("bugsee-register-only-out.zip"); + build_cmd(&[ + "--artifact", + artifact.to_str().unwrap(), + "--out", + out.to_str().unwrap(), + "--dry-run", + ]) + .assert() + .success() + .stderr(predicates::function::function(|s: &str| { + plain(s).contains("request_artifact_upload=true") + })); + assert!(out.is_file(), "the artefact ZIP is still packed"); + } + + /// `--mapping` rides INSIDE the artefact ZIP, so asking for one without an artefact is a + /// contradiction β€” and silently dropping the mapping would cost symbolication. + #[test] + fn mapping_without_an_artifact_is_rejected() { + let tmp = std::env::temp_dir(); + let mapping = tmp.join("bugsee-register-only-mapping.txt"); + std::fs::write(&mapping, b"a -> b\n").unwrap(); + build_cmd(&["--mapping", mapping.to_str().unwrap(), "--dry-run"]) + .assert() + .code(CONFIG_INVALID) + .stderr(contains("--mapping needs --artifact")); + } + + /// Same for the two flags that only describe how artefact bytes travel. + #[test] + fn artifact_transport_flags_without_an_artifact_are_rejected() { + build_cmd(&["--chunked", "--dry-run"]) + .assert() + .code(CONFIG_INVALID) + .stderr(contains("--chunked needs --artifact")); + + let out = std::env::temp_dir().join("bugsee-register-only.zip"); + build_cmd(&["--out", out.to_str().unwrap(), "--dry-run"]) + .assert() + .code(CONFIG_INVALID) + .stderr(contains("--out needs --artifact")); + } + + /// The artefact path is still validated when one IS given β€” the register-only path must not turn + /// a typo into a silent no-op upload. + #[test] + fn a_missing_artifact_path_is_still_an_error() { + const INPUT_NOT_FOUND: i32 = 10; + build_cmd(&["--artifact", "/nope/app.aab", "--dry-run"]) + .assert() + .code(INPUT_NOT_FOUND) + .stderr(contains("artifact does not exist")); + } +}