Skip to content

fix: handle RPE BIOS status when delayed response from BIOS - #3655

Open
nmgaston wants to merge 3 commits into
mainfrom
handleRPEstatusWhenDelayed
Open

nmgaston wants to merge 3 commits into
mainfrom
handleRPEstatusWhenDelayed

Conversation

@nmgaston

@nmgaston nmgaston commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

NOTE: Don't merge without console and cloud fixes.

PR Checklist

  • Unit Tests have been added for new changes
  • API tests have been updated if applicable
  • All commented code has been removed
  • If you've added a dependency, you've ensured license is compatible with Apache 2.0 and clearly outlined the added dependency.

What are you changing?

Hardens BIOS status handling for the RPE flow when BIOS responses are delayed, so the UI/state management doesn’t incorrectly fail or show stale status while waiting.

Anything the reviewer should know when reviewing this PR?

If the there are associated PRs in other repositories, please link them here (i.e. device-management-toolkit/repo#365 )

device-management-toolkit/console#1338

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Polling can mix device results after route changes and repeatedly display transient timeout alerts.

2 open issues
What changed in this PR

Improves Remote Platform Erase feedback by distinguishing request submission from confirmed BIOS completion.

Changes:

  • Adds enterprise BIOS-status polling and pending-state controls.
  • Limits SSD passwords to 32 characters and 64 UTF-8 bytes.
  • Updates tests and localized messages.
File Description
src/​models/​models.ts Adds erase-status model.
src/​assets/​i18n/​sv.json Updates Swedish messages.
src/​assets/​i18n/​ru.json Updates Russian messages.
src/​assets/​i18n/​nl.json Updates Dutch messages.
src/​assets/​i18n/​ja.json Updates Japanese messages.
src/​assets/​i18n/​it.json Updates Italian messages.
src/​assets/​i18n/​he.json Updates Hebrew messages.
src/​assets/​i18n/​fr.json Updates French messages.
src/​assets/​i18n/​fi.json Updates Finnish messages.
src/​assets/​i18n/​es.json Updates Spanish messages.
src/​assets/​i18n/​en.json Updates English messages.
src/​assets/​i18n/​de.json Updates German messages.
src/​assets/​i18n/​ar.json Updates Arabic messages.
src/​app/​devices/​remote-platform-erase/​remote-platform-erase.component.ts Adds polling and password validation.
src/​app/​devices/​remote-platform-erase/​remote-platform-erase.component.spec.ts Tests polling and password limits.
src/​app/​devices/​remote-platform-erase/​remote-platform-erase.component.html Shows progress and disables pending actions.
src/​app/​devices/​devices.service.ts Adds status endpoint request.

Review effort: Balanced

Comment thread src/app/devices/remote-platform-erase/remote-platform-erase.component.ts Outdated
Comment thread src/app/devices/remote-platform-erase/remote-platform-erase.component.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

A delayed submission response can restart polling for the previous device after navigation, blocking the current device’s controls and displaying the wrong outcome.

1 open issue
2 resolved since last review

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The destructive workflow needs human validation against the Console API contract, and a delayed-response lifecycle issue remains unresolved.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Guard pending erase response after component destruction

src/​app/​devices/​remote-platform-erase/​remote-platform-erase.component.ts:330

If the component is destroyed while setRemoteEraseOptions is pending, its subscription still handles the response. The device ID remains unchanged, so this guard passes, shows a submission snackbar, and calls watchEraseResult after destruction. Check destroyRef.destroyed before these side effects, and add a regression test that destroys the component before the submission response arrives; the existing destruction test only covers an already-started watcher.

1 resolved since last review

Review effort: Balanced

@nmgaston
nmgaston force-pushed the handleRPEstatusWhenDelayed branch from 8f457b6 to 261e6e9 Compare October 20, 2026 01:33
@nmgaston
nmgaston requested a balanced review from Copilot October 20, 2026 01:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The destructive erase workflow needs human integration validation against the Console BIOS-status API and across both deployment flavors.

No open issues 🎉

Review effort: Balanced

@nmgaston
nmgaston force-pushed the handleRPEstatusWhenDelayed branch from b3e9e14 to ab8f122 Compare October 20, 2026 19:43
@nmgaston
nmgaston marked this pull request as ready for review October 20, 2026 20:49

@rsdmike rsdmike left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for this. @nmgaston! Two things I see. First, the removed setAmtFeatures(..., { ocr, rpe: true }) call was what turned OCR back on after console sets the boot service to an OCR-disabled state, so users with OCR enabled will lose it after an erase. Second, the 5 minute limit can show "Unable to confirm" as an error after an erase that potentially worked, because a reboot plus a long SSD erase can run past 5 minutes or hide the pending state from the polls. A longer or configurable limit and a warning instead of an error would fix that. Small cleanups: csmeEraseSuccess and eraseSuccess are now unused in all the i18n files, and the 5000/10000/300000 values in watchEraseResult should be named constants.

@nmgaston
nmgaston force-pushed the handleRPEstatusWhenDelayed branch from b31d112 to ebd78b4 Compare October 23, 2026 17:32
@nmgaston

Copy link
Copy Markdown
Contributor Author

thanks for this. @nmgaston! Two things I see. First, the removed setAmtFeatures(..., { ocr, rpe: true }) call was what turned OCR back on after console sets the boot service to an OCR-disabled state, so users with OCR enabled will lose it after an erase. Second, the 5 minute limit can show "Unable to confirm" as an error after an erase that potentially worked, because a reboot plus a long SSD erase can run past 5 minutes or hide the pending state from the polls. A longer or configurable limit and a warning instead of an error would fix that. Small cleanups: csmeEraseSuccess and eraseSuccess are now unused in all the i18n files, and the 5000/10000/300000 values in watchEraseResult should be named constants.

@rsdmike Thanks for the feedback. I've fixed all of the mentioned issues.

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.

3 participants