Skip to content

core:frontend: Update major tom together with BlueOS - #3988

Open
joaomariolago wants to merge 4 commits into
bluerobotics:masterfrom
joaomariolago:update-major-tom-with-blueos
Open

core:frontend: Update major tom together with BlueOS#3988
joaomariolago wants to merge 4 commits into
bluerobotics:masterfrom
joaomariolago:update-major-tom-with-blueos

Conversation

@joaomariolago

@joaomariolago joaomariolago commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

This PR removes major_tom from the default extensions list to avoid background work to be performed without user interaction as well as avoids the update process to think that extension is already installed while is being currently pulled in background

Summary by Sourcery

Integrate automatic installation and updating of the optional Major Tom extension as part of BlueOS version changes and cloud actions, with dedicated progress reporting and centralized utility logic.

New Features:

  • Add UI support to show dedicated progress for Major Tom installation and update operations in the version chooser.
  • Introduce utility helpers to fetch latest Major Tom extension data, detect installed versions, decide whether to install or update, and perform the installation via the Kraken v2 API.

Enhancements:

  • Trigger Major Tom installation or update when changing or pulling BlueOS versions without blocking the core update if Major Tom fails.
  • Refactor version application logic in the version chooser into a reusable method used by both pull-and-set and direct set flows.
  • Unify Major Tom extension identifiers and installation data fetching across the frontend, and update cloud tray installation to use the new shared utility and Kraken v2 extension API.

@joaomariolago
joaomariolago force-pushed the update-major-tom-with-blueos branch from 66bada5 to 2e58215 Compare July 16, 2026 10:29
@joaomariolago
joaomariolago force-pushed the update-major-tom-with-blueos branch from 2e58215 to 3c70c7e Compare July 22, 2026 16:33
@joaomariolago
joaomariolago force-pushed the update-major-tom-with-blueos branch from 3c70c7e to ef45860 Compare July 22, 2026 16:47
@joaomariolago
joaomariolago marked this pull request as ready for review July 22, 2026 17:44
@github-actions

Copy link
Copy Markdown

Automated PR Review

0. Summary

  • Verdict: MINOR SUGGESTIONS ✏️

Removes Major Tom from DEFAULT_EXTENSIONS and instead installs/updates it as part of the version-chooser pullAndSetVersion / setVersion flows. Adds a shared core/frontend/src/utils/major_tom.ts used by both the version chooser and the cloud tray menu, plus a new step prop on PullProgress to differentiate the "Installing/Updating Major Tom" and "Updating BlueOS/Bootstrap" phases.

1. Correctness & Implementation Bugs

  • 1.1 [minor] core/frontend/src/components/version-chooser/VersionChooser.vue:360-379runMajorTomInstallOrUpdate wraps installOrUpdateMajorTom in a new Promise executor that resolves/rejects from two sources: (a) the PullTracker onready/onerror callbacks and (b) .then(() => { resolve() }).catch(reject) on the axios promise. Subsequent settlements are no-ops in JS so it "works", but the flow is confusing and the outer async + return new Promise(...) is redundant. Consider either using only the tracker callbacks (matching pullVersion on line 558) or just await installOrUpdateMajorTom(...) and letting the tracker only update UI state.
  • 1.2 [minor] core/frontend/src/utils/major_tom.ts:35 — the URL gained a trailing slash: previously 'https://blueos.cloud/major_tom/install' (see the old MAJOR_TOM_CLOUD_URL in CloudTrayMenu.vue:204 and core/services/kraken/config.py:16), now 'https://blueos.cloud/major_tom/install/'. If the cloud endpoint doesn't follow redirects transparently (some CDNs 301 vs 308), this could break both flows silently since updateMajorTomIfNeeded swallows fetch errors. Please confirm the trailing slash is intentional and the endpoint returns the same JSON.
  • 1.3 [minor] core/frontend/src/utils/major_tom.tsresolveMajorTomAction calls fetchMajorTomInstallData(), then installOrUpdateMajorTom calls it again internally. That's two round-trips to blueos.cloud/major_tom/install/ per apply-version. Pass the already-fetched latest from resolveMajorTomAction into installOrUpdateMajorTom (or return it from resolveMajorTomAction).

4. Performance

  • 4.1 [minor] Cross-refs 1.3 — duplicate external fetch each time a user switches BlueOS versions is avoidable.

5. UI / UX

  • 5.1 [minor] core/frontend/src/components/utils/PullProgress.vue:11 — the dialog <v-card-title> still hardcodes "Pulling new version" even though the new step prop can now display "Installing Major Tom" or "Updating Bootstrap". The card header no longer matches the actual operation. Consider promoting step (or a new title prop) into the title so the dialog reads coherently in the Major Tom / bootstrap cases.
  • 5.2 [minor] core/frontend/src/components/version-chooser/VersionChooser.vue:385 — when resolveMajorTomAction throws (blueos.cloud unreachable, DNS failure, 5xx, etc.), the user gets only console.warn; the surrounding updateMajorTomIfNeeded returns silently and the BlueOS pull proceeds without Major Tom. That matches the "don't block core update" intent, but a notifier.pushWarning (like the failure path in the second try/catch) would give the user visible feedback that Major Tom wasn't touched.

6. Code Quality & Style

  • 6.1 [minor] core/frontend/src/components/version-chooser/VersionChooser.vue:297-301 — six new flat data properties (show_major_tom_progress, major_tom_pull_output, major_tom_download_percentage, major_tom_extraction_percentage, major_tom_status_text, major_tom_step) for a single operation would read better as a single reactive object (e.g. major_tom_progress: { show, output, download, extraction, status, step }), keeping the template bindings terse and letting you reset with a single assignment.
  • 6.2 [nit] core/frontend/src/components/cloud/CloudTrayMenu.vue:206const MAJOR_TOM_EXTENSION_IDENTIFIER = MAJOR_TOM_IDENTIFIER is a needless local rename; the three call sites (lines 262, 265, 871) can just use the imported MAJOR_TOM_IDENTIFIER directly and this alias can be dropped.
  • 6.3 [nit] core/frontend/src/components/version-chooser/VersionChooser.vue:544this.pull_step = 'Updating BlueOS' is set immediately before pullVersion internally re-writes pull_output = 'Fetching remote image...' and show_pull_output = true (lines 555-556). The redundant local pull_output/show_pull_output assignments at 545-546 are now duplicated with pullVersion; the whole prep block could live inside pullVersion for cleanliness.

8. Documentation

  • 8.1 [nit] core/frontend/src/utils/major_tom.ts:20progressHandler: (event: { event: { currentTarget: { response: string } } }) => void describes the fields as required, but the only caller in runMajorTomInstallOrUpdate guards with progressEvent.event?.currentTarget?.response — meaning at least one caller expects them optional. Either loosen the type to optional (matches the guard) or drop the guard (matches the type). Consistency with core/frontend/src/store/disk.ts:196 (optional shape) would be simpler.

9. Nitpicks / Optional

  • 9.1 [nit] core/services/kraken/config.py:3 — imports Dict, List from typing only to annotate an empty list. Matches the file style used elsewhere so no change needed, just noting that on Python 3.11 this could be list[dict[str, str]] without the extra import.

Generated by PR Review Bot. This is advisory, a human reviewer must still approve.

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.

1 participant