Skip to content

feat: extend update-agent reporting#571

Merged
vmenge merged 12 commits into
mainfrom
alex/update-progress-reporting
Aug 22, 2025
Merged

feat: extend update-agent reporting#571
vmenge merged 12 commits into
mainfrom
alex/update-progress-reporting

Conversation

@AlexKaravaev
Copy link
Copy Markdown
Contributor

@AlexKaravaev AlexKaravaev commented Jul 20, 2025

Extend update agent reporting, modify backend status connect to not depend on if update agent is alive or not. Example of status sent with new update and new field "state"

Jul 20 14:06:21 localhost.localdomain worldcoin-backend-status[627981]: 
Sending status to backend: CurrentStatus 
....update_progress: Some(UpdateProgress { download_progress: 100, processed_progress: 100, install_progress: 100, total_progress: 100, error: None, state: NoNewVersion }), ...

@AlexKaravaev AlexKaravaev force-pushed the alex/update-progress-reporting branch from cd83e4b to b648155 Compare July 20, 2025 16:53
@AlexKaravaev AlexKaravaev added the ai:supervised For PRs that make use of AI but with supervision label Jul 20, 2025
@AlexKaravaev AlexKaravaev requested a review from Copilot July 20, 2025 16:57
@AlexKaravaev AlexKaravaev force-pushed the alex/update-progress-reporting branch from 264c1cf to d65bc56 Compare July 20, 2025 16:57
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR extends the update agent's reporting capabilities by adding new progress states and overall status tracking, plus removes the backend status dependency on whether the update agent is alive. The changes enhance monitoring of update processes from "no new version" through completion and reboot.

Key changes:

  • Adds comprehensive state tracking with new UpdateAgentState enum values (NoNewVersion, Installing, Rebooting)
  • Replaces individual component property updates with unified progress reporting via update_dbus_progress
  • Migrates backend status from polling-based to signal-based update progress monitoring

Reviewed Changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
update-agent/src/main.rs Core update agent logic updated to use new progress reporting and handle "no new version" case
update-agent/src/lib.rs Adds utility mappers for state conversion between different components
update-agent/src/dbus/interfaces.rs Replaces individual property updates with unified progress reporting system
update-agent/src/component.rs Updates download progress reporting to use new unified interface
update-agent/src/claim.rs Adds NoNewVersion error type for better error handling
update-agent/dbus/src/lib.rs Extends dbus interface with new state enums and overall progress properties
backend-status/src/update_progress.rs Replaces polling mechanism with signal-based progress monitoring
backend-status/src/main.rs Updates logging messages to reflect new signal-based approach
backend-status/src/dbus/intf_impl.rs Adds immediate send logic for critical update states
backend-status/src/backend/types.rs Adds state field to update progress structure
backend-status/src/backend/status.rs Includes state in status request building
backend-status/dbus/src/types.rs Adds state field to UpdateProgress type
Comments suppressed due to low confidence (1)

backend-status/src/update_progress.rs:348

  • The test function references UpdateProgressCalculator::from_update_agent_data but this struct/method doesn't exist in the visible code. The test should reference the actual implementation or the struct should be defined.
    fn test_update_progress_calculator() {

Comment thread update-agent/src/main.rs
Comment thread update-agent/src/main.rs
Comment thread update-agent/src/lib.rs Outdated
Comment thread update-agent/src/dbus/interfaces.rs
Comment thread backend-status/src/update_progress.rs
@AlexKaravaev AlexKaravaev force-pushed the alex/update-progress-reporting branch from 5361335 to d65bc56 Compare July 20, 2025 17:03
@AlexKaravaev AlexKaravaev marked this pull request as ready for review July 20, 2025 17:20
Copy link
Copy Markdown
Contributor

@Michael-Preibisch Michael-Preibisch left a comment

Choose a reason for hiding this comment

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

Left some minor comments.

@AlexKaravaev would it be feasible to include downloaded_bytes and bytes_total instead of progress %?

Comment thread backend-status/src/dbus/intf_impl.rs Outdated
Comment thread backend-status/src/dbus/intf_impl.rs Outdated
let force = *force_flag;
if force {
*force_flag = false; // Reset the flag after checking
info!(
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.

Nit: Weird formatting

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is clippy :(

Comment thread backend-status/src/update_progress.rs Outdated
Comment thread update-agent/src/claim.rs Outdated
Comment thread backend-status/src/backend/types.rs
Comment thread backend-status/src/dbus/intf_impl.rs Outdated
@AlexKaravaev AlexKaravaev requested a review from paulquinn00 July 24, 2025 16:31
@AlexKaravaev AlexKaravaev force-pushed the alex/update-progress-reporting branch from bd3dc49 to b78d127 Compare July 24, 2025 16:31
UpdateAgentState::Downloading => {
(progress_value as u64, 0, 0, (progress_value / 3) as u64) // 1/3 of total workflow
}
UpdateAgentState::Fetched => (100, 0, 0, 66), // Downloading is still majority of time for the user
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.

This should be 33, not 66. The Fetched state means "download complete" which is 1/3 of the total update process.

Currently causes the progress bar to jump from 33% to 66% instantly when downloads finish.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This should be 33, not 66. The Fetched state means "download complete" which is 1/3 of the total update process.

I was thinking for a bit on what to do here. Download is 1/3 of the whole process, but in fact for the user download is majority of the whole update process time due to the slow connection speed. So I think that if we hardcode it for 66(for now - we should improve it better) it would reflect a bit better for the actual user the overall installation progress

Copy link
Copy Markdown
Contributor

@Michael-Preibisch Michael-Preibisch left a comment

Choose a reason for hiding this comment

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

:lfg:

AlexKaravaev and others added 9 commits August 20, 2025 15:50
This commit introduces an UpdateAgentState enum and overall progress tracking to the update-agent D-Bus interface, and propagates this state through backend-status. The backend-status service now receives and forwards the update agent's overall state, enabling immediate backend notification on critical states (e.g., Rebooting). The update-agent now emits D-Bus signals for overall status and progress, and backend-status uses a signal-based watcher for real-time updates. Additional improvements include weighted progress calculation, error handling for 'no new version', and expanded test coverage.
@paulquinn00 paulquinn00 force-pushed the alex/update-progress-reporting branch from b78d127 to 9f68215 Compare August 20, 2025 22:53
@paulquinn00 paulquinn00 requested a review from a team as a code owner August 20, 2025 22:53
Copy link
Copy Markdown
Collaborator

@vmenge vmenge left a comment

Choose a reason for hiding this comment

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

lgtm

@vmenge vmenge merged commit 79ccacb into main Aug 22, 2025
24 checks passed
@vmenge vmenge deleted the alex/update-progress-reporting branch August 22, 2025 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai:supervised For PRs that make use of AI but with supervision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants