fix(server): block files that are not ready from being published

[yuvrajjsingh0]

Problem

bulk_create_files inserts each row as a placeholder and fills it in later:

let new_file = NewFileEntry { ..., size: 0, checksum: "".to_string(), ... };

task::spawn(async move {
    if let Ok((file_size, file_checksum)) = utils::download_and_checksum(&file_url).await {
        ...update the row...
    } else {
        Err(ABError::InternalServerError("Failed to download file".into()))  // nothing awaits this
    }
});

Nothing awaits that task, so a failed download left the placeholder row in place with no record of why.

Nothing downstream distinguished a placeholder from a real file:

• create_package checked only files.len() != request.files.len() — that the rows exist, not that they are usable.
• build_overrides (shared by release create and update) checked only that none were missing.
• serve_release_handler copied file.checksum straight into the manifest.

So a placeholder could be packaged, released, and served, and the manifest reaching devices carried checksum: "". Both SDKs skip integrity verification when the checksum is empty (UpdateTask.kt:865, AJPRemoteFileUtil.swift:139), so the device installs bytes that were never verified against anything. That is what turns an incomplete upload into a supply-chain problem rather than just a broken build.

Audit reference: S3 (and it closes the server half of S2).

Change

A readiness predicate — bytes present and checksum present — enforced at three points:

Point	Behaviour
create_package	400, naming the offending files
build_overrides (release create and update)	400, before anything is persisted to Superposition
serve_release_handler	500 + error log, rather than emitting a placeholder

Serving refuses the request instead of emitting a manifest with an empty checksum. That leaves devices on the bundle they already booted, which is the safe outcome; the alternative ships unverified bytes. This also covers releases created before this gate existed, which the create-time gates cannot retroactively fix.

The detached bulk task now logs why a file failed to become ready, distinguishing download failure from a DB failure after a successful download.

Scope notes

• create_file validates size and checksum synchronously (64 hex chars, size > 0) and upload_file deletes its row when the S3 upload fails — so bulk_create_files is the only source of persistent placeholder rows.
• The read paths (get_release, list_releases) are deliberately not gated. They are authenticated dashboard reads, and an operator needs to be able to see a broken release in order to diagnose it.
• I used a derived predicate rather than adding a status column. That needs no migration and no backfill, and it correctly classifies rows that already exist. An explicit state machine would be the better long-term model but is a bigger change than this fix warrants.

Operational impact

A deployment that already has placeholder rows referenced by a live release will now get a 500 on that release's config, with the offending file keys in the log:

[SERVE RELEASE] Refusing to serve <app>: release references files with no checksum: broken.js@version:7

That is intentional and worth calling out in review: those releases were already shipping unverifiable bundles. Devices stay on their current bundle until the file is re-created. Worth a check for size = 0 rows against live releases before rolling this out.

Testing

7 unit tests on the predicate and gate, all passing:

file::utils::tests::a_downloaded_file_is_ready ... ok
file::utils::tests::a_bulk_created_placeholder_is_not_ready ... ok
file::utils::tests::half_populated_rows_are_not_ready ... ok
file::utils::tests::a_whitespace_checksum_is_not_ready ... ok
file::utils::tests::ready_files_pass_the_gate ... ok
file::utils::tests::the_gate_rejects_and_names_pending_files ... ok
file::utils::tests::not_ready_keys_lists_only_unready_files ... ok

a_bulk_created_placeholder_is_not_ready asserts against the exact row shape bulk_create_files inserts. The half-populated and whitespace cases cover a checksum with no bytes, bytes with no checksum, and a whitespace-only checksum that would pass a naive is_empty().

Verified by inspection that each gate sits before its write: build_overrides is awaited at release.rs:421 well before create_experiment().send(), and both public routes (serve_release, serve_release_v2) funnel through the guarded serve_release_handler.

Full suite: 17 passed. cargo fmt --check and cargo clippy --all-targets -- -Dwarnings pass.

Docs

No documentation change applies. bulk_create_files is not in the Smithy service (only CreateFile is) and is not described anywhere in airborne_docs/; the new 400/500 responses fall under the BadRequestError/InternalServerError already declared service-wide in smithy/models/main.smithy, so the API reference does not need regenerating. The CLI flows documented in react-native-cli/remote-files-and-packages.md use CreateFile/UploadFile, both of which already produce a real checksum or fail, so they are unaffected.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Prevented incomplete or failed file downloads from being included in packages and releases.
  • Added validation to ensure files have valid size and checksum information before publishing.
  • Package creation now reports which files are not ready instead of creating an unusable package.
  • Release serving is blocked when referenced files are incomplete, improving reliability and preventing invalid release responses.
  • Download and database update failures are logged for improved troubleshooting.

[semanticdiff-com]

Review changes with  

Changed Files

File	Status
airborne_server/src/file.rs	22% smaller
airborne_server/src/file/utils.rs	0% smaller
airborne_server/src/package.rs	0% smaller
airborne_server/src/release/utils.rs	0% smaller

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2e458453-685e-40c4-af82-a0924325fbe3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Walkthrough

File download tasks now log failures explicitly, while shared readiness checks prevent incomplete files from being used during package creation, release construction, or release serving.

Changes

File readiness enforcement

Layer / File(s)	Summary
Download status and readiness contract airborne_server/src/file.rs, airborne_server/src/file/utils.rs	Download failures and database update errors are logged, and utilities identify files with missing size or checksum data.
Package and release readiness gates airborne_server/src/package.rs, airborne_server/src/release.rs, airborne_server/src/release/utils.rs	Package creation, release override construction, and release serving now validate file readiness before continuing.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant PackageOrReleaseHandler
  participant FileDatabase
  participant FileReadinessUtils
  Client->>PackageOrReleaseHandler: create or serve release
  PackageOrReleaseHandler->>FileDatabase: fetch referenced files
  FileDatabase-->>PackageOrReleaseHandler: file metadata
  PackageOrReleaseHandler->>FileReadinessUtils: validate readiness
  FileReadinessUtils-->>PackageOrReleaseHandler: success or pending keys
  PackageOrReleaseHandler-->>Client: continue or return error

Loading

Poem

A rabbit watched the downloads land,
With checksums neat and sizes grand.
If files were faint, the gates said “wait,”
No fragile release passed the state.
Hop, hop—readiness keeps things straight!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: server-side readiness checks now block unready files from being published.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ 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 fix/s3-block-pending-files

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🤖 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 `@airborne_server/src/release.rs`:
- Around line 1385-1393: Update the refusal log in the release-serving readiness
check to describe every failed readiness condition, including missing or invalid
size as well as missing checksum. Keep the existing not_ready reporting and
error return behavior unchanged, but revise the message so zero-sized files are
diagnosed accurately.
- Around line 1383-1394: Update the release file-lookup flow around
not_ready_file_keys to retain the complete set of requested file keys and verify
every requested key was resolved. Treat lookup errors and partial Ok(files)
results as failures, log the missing or lookup error, and return
ABError::InternalServerError instead of falling through to the placeholder
response or serving the release.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d5d42fe4-6e5e-44ef-a3fb-880d6abd5a17

📥 Commits

Reviewing files that changed from the base of the PR and between a706e7f and 18b8f18.

📒 Files selected for processing (5)

• airborne_server/src/file.rs
• airborne_server/src/file/utils.rs
• airborne_server/src/package.rs
• airborne_server/src/release.rs
• airborne_server/src/release/utils.rs

[yuvrajjsingh0]

Pushed 2399209 addressing the file-lookup review.

The finding was valid. The readiness gate only inspected rows the lookup returned, so not_ready_file_keys was structurally blind to a row that never came back. Three paths still reached a device with an empty checksum:

1. Lookup error — the else branch fell through to ServeFile { checksum: String::new(), .. } and empty file lists. The readiness gate never ran at all.
2. Unresolved index — unwrap_or_else substituted the same empty-checksum placeholder.
3. Partial resolution — filter_map silently dropped unresolved files, producing an incomplete manifest.

Change

• Retain requested_file_keys before the lookup consumes the vector.
• Err from the lookup now logs and returns InternalServerError instead of falling through.
• New unresolved_file_keys helper (next to not_ready_file_keys) reports requested keys that resolved to no row; any non-empty result logs the missing keys and fails the request.
• The index uses ok_or_else(...)? rather than a placeholder, so an empty checksum is no longer constructible on this path.
• Order is deliberate: resolution completeness is checked before readiness, since readiness cannot speak to rows that are absent.

Matching on the parsed file_path (rather than comparing lengths) means duplicate keys and @version: / @tag: suffixes don't produce false failures — the SQL UNION dedupes, so a length comparison would have.

Validation

cargo fmt --check and cargo clippy --all-targets -- -Dwarnings pass; the suite is green.

I verified unresolved_file_keys against five cases, including the one that matters — readiness reporting clean while a requested key resolved to nothing:

unresolved_is_caught_where_readiness_is_blind ... ok
fully_resolved_reports_nothing ... ok
empty_result_leaves_everything_unresolved ... ok
version_and_tag_suffixes_do_not_break_matching ... ok
duplicate_keys_do_not_cause_false_missing ... ok

Those are not in the commit — the amend on this branch removed the test module from file/utils.rs, so I took that as deliberate and did not reinstate it. Say the word and I'll add them back.

Note

The amend introduced a stray character in a doc comment — /// Rejects the request unless every file is ready to publish.= in file/utils.rs. Left alone to keep this diff to the review comment.