Skip to content

fix(api-core): forward compute tray firmware force update - #5282

Open
behroozrafii wants to merge 2 commits into
NVIDIA:mainfrom
behroozrafii:fix/5216-forward-compute-firmware-force-update
Open

fix(api-core): forward compute tray firmware force update#5282
behroozrafii wants to merge 2 commits into
NVIDIA:mainfrom
behroozrafii:fix/5216-forward-compute-firmware-force-update

Conversation

@behroozrafii

Copy link
Copy Markdown
Contributor

Summary

  • Forward the request's force_update value when compute-tray firmware updates use direct dispatch.
  • Keep existing access-token handling and other firmware-update defaults unchanged.
  • Add handler-level regression coverage using a recording compute-tray backend.
  • Assert that the fixture resolves the endpoint successfully and that both force_update=false and force_update=true reach the backend.

Fixes #5216

Testing

Passed:

  • cargo fmt --all -- --check

  • git diff --check

  • cargo check -p carbide-api-core --no-default-features --lib

  • cargo clippy -p carbide-api-core --no-default-features --lib -- -D warnings

  • cargo test -p component-manager direct_rms_firmware_object_json_request_defaults_missing_access_token_to_noauth

  • Linux Docker/PostgreSQL targeted test:

    cargo test -p carbide-api-core --lib \
      compute_tray_direct_dispatch_forwards_force_update \
      --locked -- --nocapture
    

    Result: 1 passed, 0 failed.

Additional package-suite validation:

cargo test -p carbide-api-core --lib --locked

Result: 1,723 passed, 9 failed, 4 ignored. The issue-specific regression test passed. The nine failures were unrelated to this change and occurred in unchanged tests: three required Git-derived version metadata unavailable in the container, and six required the missing tpm2 executable.

Full Linux all-targets/all-features Clippy was not run because the available container did not include the Clippy component. Production-library Clippy passed locally with warnings denied.

@behroozrafii
behroozrafii requested a review from a team as a code owner August 22, 2026 01:02
@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e48a6fed-644a-4606-911a-e3507b974022

📥 Commits

Reviewing files that changed from the base of the PR and between 2266b58 and 19fd092.

📒 Files selected for processing (1)
  • crates/api-core/src/tests/firmware_component_manager.rs

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


Summary by CodeRabbit

  • New Features

    • Compute-tray firmware updates now honor the selected “force update” option.
  • Bug Fixes

    • Fixed direct firmware update requests that previously ignored the force-update setting.
  • Tests

    • Added coverage for both forced and standard firmware update requests.

Walkthrough

The compute-tray direct firmware dispatch now forwards the request’s force_update value. Test fixtures support manager injection, and integration tests verify both force-update states.

Changes

Compute-tray firmware options

Layer / File(s) Summary
Forward force-update option
crates/api-core/src/handlers/component_manager.rs
Direct compute-tray firmware dispatch preserves the request’s force_update value instead of using the default false value.
Add forwarding regression test
crates/api-core/src/tests/common/api_fixtures/mod.rs, crates/api-core/src/tests/firmware_component_manager.rs, crates/api-core/src/tests/mod.rs
Test fixtures support an injected compute-tray manager. The integration test records forwarded options for both force-update states and verifies that access_token remains unset.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 19fd0

The change forwards the requested firmware update flag while preserving existing defaults and access-token handling; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the compute-tray firmware force-update forwarding fix.
Description check ✅ Passed The description directly explains the fix, preserved behavior, regression coverage, and validation results.
Linked Issues check ✅ Passed The implementation forwards request.force_update in direct compute-tray dispatch and adds tests for both false and true values, satisfying issue #5216.
Out of Scope Changes check ✅ Passed The changes remain within the issue scope: handler behavior, test fixtures, regression tests, and test registration.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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.

bug: CM update-firmware ignores options when bypassing state controller

1 participant