Add HTTP firmware command headers for image URL updates and reboots.#864
Merged
Conversation
Queue reboot and image URL changes for HTTP-polling devices and deliver them via /next response headers, with matching controls on the device update page. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughAdds HTTP firmware command support for eligible devices, including queued image/reboot state, context-aware command dispatch, pending response headers, device UI controls, localization, and coverage for queueing and header delivery. ChangesHTTP firmware commands
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ManagerUI
participant Server
participant DeviceDB
participant HTTPDevice
ManagerUI->>Server: submit image URL or reboot command
Server->>DeviceDB: persist pending command state
Server->>HTTPDevice: broadcast firmware command
HTTPDevice->>Server: request next image
Server->>HTTPDevice: return pending HTTP headers
Server->>DeviceDB: clear pending command state
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/server/handlers_api_test.go (1)
1203-1209: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
requireandassertin the new tests.
internal/server/handlers_api_test.go#L1203-L1209: userequire.NoErrorfor the query andassert.EqualforPendingImageURL.internal/server/handlers_api_test.go#L1231-L1237: userequire.NoErrorfor the query andassert.TrueforPendingReboot.internal/server/handlers_device_api_test.go#L99-L135: replace new direct failure checks withrequirefor setup/fatal prerequisites andassertfor response and persisted-state assertions.As per coding guidelines,
**/*_test.go: “Use thetestifylibrary, specificallyassertandrequire, in unit tests.”🤖 Prompt for 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. In `@internal/server/handlers_api_test.go` around lines 1203 - 1209, Replace direct testing failures with testify assertions: in internal/server/handlers_api_test.go lines 1203-1209, use require.NoError for the database query and assert.Equal for PendingImageURL; in lines 1231-1237, use require.NoError and assert.True for PendingReboot. In internal/server/handlers_device_api_test.go lines 99-135, use require for setup or fatal prerequisites and assert for response and persisted-state checks.Source: Coding guidelines
🤖 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 `@internal/server/helpers.go`:
- Around line 704-729: Update the pending command handling in handleNextApp so
acknowledgement is serialized with enqueueing: reload the device state and claim
each pending update under a row lock, or use generation-aware conditional
updates. Ensure clearing pending_update_url, pending_image_url, and
pending_reboot only acknowledges the command that was sent, preserving any newer
command queued after the initial device load.
---
Nitpick comments:
In `@internal/server/handlers_api_test.go`:
- Around line 1203-1209: Replace direct testing failures with testify
assertions: in internal/server/handlers_api_test.go lines 1203-1209, use
require.NoError for the database query and assert.Equal for PendingImageURL; in
lines 1231-1237, use require.NoError and assert.True for PendingReboot. In
internal/server/handlers_device_api_test.go lines 99-135, use require for setup
or fatal prerequisites and assert for response and persisted-state checks.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 91569ee6-67c7-4e3f-864b-a10bf9fd6e22
📒 Files selected for processing (11)
internal/data/models.gointernal/data/models_test.gointernal/server/handlers_api.gointernal/server/handlers_api_test.gointernal/server/handlers_device.gointernal/server/handlers_device_api.gointernal/server/handlers_device_api_test.gointernal/server/helpers.goweb/i18n/de.jsonweb/i18n/en.jsonweb/templates/manager/update.html
Comment on lines
+704
to
+729
| if _, err := gorm.G[data.Device](s.DB).Where("id = ?", device.ID).Update(ctx, "pending_update_url", ""); err != nil { | ||
| slog.Error("Failed to clear pending update", "error", err) | ||
| } else { | ||
| device.PendingUpdateURL = "" | ||
| } | ||
| } | ||
|
|
||
| if imageURL := device.PendingImageURL; imageURL != "" { | ||
| slog.Info("Sending image URL update header", "device", device.ID, "url", imageURL) | ||
| w.Header().Set("Tronbyt-Image-URL", imageURL) | ||
|
|
||
| if _, err := gorm.G[data.Device](s.DB).Where("id = ?", device.ID).Update(ctx, "pending_image_url", ""); err != nil { | ||
| slog.Error("Failed to clear pending image URL", "error", err) | ||
| } else { | ||
| device.PendingImageURL = "" | ||
| } | ||
| } | ||
|
|
||
| if device.PendingReboot { | ||
| slog.Info("Sending reboot header", "device", device.ID) | ||
| w.Header().Set("Tronbyt-Reboot", "true") | ||
|
|
||
| if _, err := gorm.G[data.Device](s.DB).Where("id = ?", device.ID).Update(ctx, "pending_reboot", false); err != nil { | ||
| slog.Error("Failed to clear pending reboot", "error", err) | ||
| } else { | ||
| device.PendingReboot = false |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Serialize command acknowledgement with queueing.
These unconditional clears race with a new command queued after handleNextApp loaded device. A poll can send image URL A, a request can queue B, and this update then clears B—so B is never delivered. Reload and claim/ack pending commands under a row lock or use command generations/conditional acknowledgements so a later enqueue survives.
🤖 Prompt for 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.
In `@internal/server/helpers.go` around lines 704 - 729, Update the pending
command handling in handleNextApp so acknowledgement is serialized with
enqueueing: reload the device state and claim each pending update under a row
lock, or use generation-aware conditional updates. Ensure clearing
pending_update_url, pending_image_url, and pending_reboot only acknowledges the
command that was sent, preserving any newer command queued after the initial
device load.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Queue reboot and image URL changes for HTTP-polling devices and deliver them via /next response headers, with matching controls on the device update page.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation