diff --git a/.github/workflows/lint-and-test.yml b/.github/workflows/lint-and-test.yml index 38a3c88e5..8fcfe0cf3 100644 --- a/.github/workflows/lint-and-test.yml +++ b/.github/workflows/lint-and-test.yml @@ -18,7 +18,29 @@ permissions: packages: read security-events: write +env: + # Pin to avoid current latest's nilness recover panic: dominikh/go-tools#1725. + GOLANGCI_LINT_VERSION: v2.12.2 + jobs: + actionlint: + name: Actionlint + runs-on: ubuntu-latest + timeout-minutes: 5 + + steps: + - name: Checkout code + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Set up Go + uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version-file: go.mod + cache: false + + - name: Run actionlint + run: go run github.com/rhysd/actionlint/cmd/actionlint@v1.7.12 + ci: name: CI timeout-minutes: 15 @@ -106,7 +128,7 @@ jobs: - name: Run golangci-lint uses: golangci/golangci-lint-action@ba0d7d2ec06a0ea1cb5fa41b2e4a3ab91d21278a # v9.3.0 with: - version: latest + version: ${{ env.GOLANGCI_LINT_VERSION }} args: --timeout=5m - name: Install govulncheck @@ -296,8 +318,9 @@ jobs: -e CXX="${TARGET_CXX}" \ -e CGO_CFLAGS="${TARGET_CGO_CFLAGS}" \ -e CGO_LDFLAGS="${TARGET_CGO_LDFLAGS}" \ + -e GOLANGCI_LINT_VERSION \ ${{ env.ZIGCC_IMAGE }} \ - bash -c 'curl -sSfL https://golangci-lint.run/install.sh | sh -s -- -b /home/build/bin 2>&1 | tail -1 && PATH="/home/build/bin:$PATH" golangci-lint run --timeout=5m' + bash -c 'curl -sSfL https://golangci-lint.run/install.sh | sh -s -- -b /home/build/bin "$GOLANGCI_LINT_VERSION" 2>&1 | tail -1 && PATH="/home/build/bin:$PATH" golangci-lint run --timeout=5m' native-pr-tests: name: Native PR Tests @@ -417,11 +440,15 @@ jobs: ci-status: name: CI Status if: always() && !cancelled() - needs: [ci, cross-lint, native-pr-tests] + needs: [actionlint, ci, cross-lint, native-pr-tests] runs-on: ubuntu-latest steps: - name: Check CI results run: | + if [ "${{ needs.actionlint.result }}" != "success" ]; then + echo "CI failed: actionlint=${{ needs.actionlint.result }}" + exit 1 + fi if [ "${{ needs.ci.result }}" != "success" ]; then echo "CI failed: ci=${{ needs.ci.result }}" exit 1 diff --git a/docs/api/index.md b/docs/api/index.md index 4db11473c..c60f3c858 100644 --- a/docs/api/index.md +++ b/docs/api/index.md @@ -213,9 +213,9 @@ This access is also allowed when a connection is made over a WebSocket Secure (w Core evaluates permissions for each request from connection locality and paired-client role: - **Localhost** requests originate from Core's device and have full access. -- Paired **admin** clients have the `profiles.manage` and `settings.write` capabilities. -- Paired **member** clients can use day-to-day methods, but cannot manage profiles or change protected settings. -- **Unpaired remote** clients are possible only when encryption is disabled. For backward compatibility they receive admin capabilities, but methods explicitly restricted to localhost or to a paired admin still reject them. +- Paired **admin** clients have the `profiles.manage`, `settings.write`, and `update.apply` capabilities. +- Paired **member** clients can use day-to-day methods, but cannot manage profiles, change protected settings, or apply updates. +- **Unpaired remote** clients are possible only when encryption is disabled. For backward compatibility they receive admin capabilities, but methods explicitly restricted to localhost or to a paired admin still reject them. They do not get the `update.apply` capability, and `update.check` is closed to them too. Call [`clients.current`](./methods.md#clientscurrent) to inspect current connection's role and capabilities. Every method in [API Methods](./methods) states its access requirements. Some read methods return additional sensitive fields to privileged clients; those fields are identified in their result contracts. @@ -339,8 +339,8 @@ Methods execute actions and return data from Core. This catalog documents **88 n | settings.auth.link | Start online account link. | Local/admin | | settings.auth.link.status | Return online account link flow status. | Tiered | | settings.auth.link.cancel | Cancel online account link flow. | Local/admin | -| update.check | Check for newer Core version. | All clients | -| update.apply | Apply latest update and restart gracefully. | All clients | +| update.check | Check for newer Core version. | Localhost or any paired client | +| update.apply | Apply latest update and restart gracefully. | `update.apply` | ## Notifications @@ -362,3 +362,4 @@ Notifications let a server or client know an event has occurred. See the [API No | playtime.limit.reached | A playtime limit (session or daily) has been reached and enforced. | | playtime.limit.warning | A playtime warning notification sent at configured intervals before limit reached. | | inbox.added | A new inbox message was added to the server. | +| update.state | Progress of an update being applied. | diff --git a/docs/api/methods.md b/docs/api/methods.md index e16334050..1485e25fc 100644 --- a/docs/api/methods.md +++ b/docs/api/methods.md @@ -7,8 +7,10 @@ Methods are used to execute actions and request data back from the API. Each method below identifies which clients may call it: - **All clients:** localhost, paired admin, paired member, and unpaired remote clients accepted by the API transport. +- **Localhost or any paired client:** localhost and any paired client, member included. Unpaired remote clients are rejected. - **`profiles.manage`:** localhost and clients with the `profiles.manage` capability. Paired admins have this capability; paired members do not. Unpaired remote clients retain it for backward compatibility when encryption is disabled. - **`settings.write`:** localhost and clients with the `settings.write` capability. Paired admins have this capability; paired members do not. Unpaired remote clients retain it for backward compatibility when encryption is disabled. +- **`update.apply`:** localhost and clients with the `update.apply` capability. Paired admins have this capability; paired members and unpaired remote clients do not. - **Localhost or paired admin:** localhost and authenticated paired admins only. Paired members and unpaired remote clients are rejected. - **Localhost only:** requests originating from Core's device. All remote clients are rejected. @@ -2540,6 +2542,9 @@ None. | systemDefaults | [SystemDefault](#system-default-object)[] | Yes | Per-system overrides for default launcher and exit ZapScript. | | profilesRequireForLaunch | boolean | Yes | Whether media launches are blocked while no personal profile is active. | | profilesSwapData | boolean | Yes | Whether profile switches also swap profile-scoped data (saves, save states) on supported platforms. Defaults to true. | +| updateChannel | string | Yes | Release channel used for update checks: `stable` or `beta`. Defaults to `stable`. | +| updateCheck | boolean | Yes | Whether the service looks for new releases on its own. Defaults to true on every platform, including installs a package manager owns. | +| updateInstall | boolean | Yes | Whether the device downloads and installs updates on its own, rather than only telling the user one exists. Defaults to false, and is always false while `updateCheck` is off. | | backupRemoteEnabled | boolean | No | Whether automatic remote backup scheduling is enabled. Only returned to localhost and paired admin clients. | | playtimeSyncEnabled | boolean | No | Whether the user explicitly enabled play history sync. Defaults to false. Only returned to localhost and paired admin clients. | | backupRemoteSchedule | string | No | Remote backup schedule: `daily`, `weekly`, or `manual`. Only returned to localhost and paired admin clients. | @@ -2628,6 +2633,9 @@ An object containing any of the following optional keys: | systemDefaults | [SystemDefault](#system-default-object)[] | No | Replace the full list of per-system launcher/exit-script overrides. Each `launcher` value, if non-empty, must match a known launcher ID or group (case-insensitive). | | profilesRequireForLaunch | boolean | No | Whether media launches are blocked while no personal profile is active. | | profilesSwapData | boolean | No | Whether profile switches also swap profile-scoped data. Turning it off converges data back to the shared state immediately. | +| updateChannel | string | No | Release channel used for update checks: `stable` or `beta`. | +| updateCheck | boolean | No | Whether the service looks for new releases on its own. | +| updateInstall | boolean | No | Whether the device installs updates on its own. Setting it to true while update checking is off is refused; send `updateCheck: true` in the same call to turn both on. | | backupRemoteEnabled | boolean | No | Enable automatic remote backup scheduling. Requires a localhost or paired admin client. | | playtimeSyncEnabled | boolean | No | Explicitly enable or disable play history sync. The first enabled sync uploads retained local history. Disabling stops future uploads. Requires a localhost or paired admin client. | | backupRemoteSchedule | string | No | Remote backup schedule: `daily`, `weekly`, or `manual`. Requires a localhost or paired admin client. | @@ -3712,7 +3720,7 @@ A profile's **switch ID is a bearer credential**: presenting it — by scanning A swap requested while media is running is deferred until it stops, so the running session keeps the data it launched with. Progress and failures are reported by the [`profiles.data`](notifications.md#profilesdata) notification; the `profilesSwapData` setting turns swapping off. Deleting a profile does not delete its profile-owned platform data. -**Administration and trust model.** The first profile is created as `admin` and must have a PIN; later profiles default `member`. The first paired client is `admin`; later pairings default `member`. Sensitive local UIs call `profiles.verify`, confirm the returned profile has the `admin` role, then send the ordinary management request. This is a client-side nuisance gate for parental and kiosk controls, not cryptographic request authorization; no unlock session is retained. Admin paired clients use their client capability directly. The last admin profile/client cannot be removed or demoted. Profiles remain a household convenience boundary, comparable to TV parental controls — not OS account security. Anyone with OS access still owns the device, and while `service.encryption` is off an unpaired remote client retains legacy admin API capability. Enabling encryption makes paired-client restrictions enforceable. +**Administration and trust model.** The first profile is created as `admin` and must have a PIN; later profiles default `member`. The first paired client is `admin`; later pairings default `member`. Sensitive local UIs call `profiles.verify`, confirm the returned profile has the `admin` role, then send the ordinary management request. This is a client-side nuisance gate for parental and kiosk controls, not cryptographic request authorization; no unlock session is retained. Admin paired clients use their client capability directly. The last admin profile/client cannot be removed or demoted. Profiles remain a household convenience boundary, comparable to TV parental controls — not OS account security. Anyone with OS access still owns the device, and while `service.encryption` is off an unpaired remote client retains legacy admin API capability, apart from the capabilities that require an authenticated connection — currently `update.apply`. Enabling encryption makes paired-client restrictions enforceable. ### Profile object @@ -4662,7 +4670,7 @@ None. Return pairing status, authenticated role, and effective capabilities for the current connection. This method is available to every connection accepted by the API transport. -`role` is `admin` or `member` for paired connections and `null` otherwise. Unpaired plaintext connections retain their legacy effective capabilities. Clients should use capability presence for corresponding UI gates and treat role as display-only. Capability names currently include `profiles.manage` and `settings.write`; the array does not enumerate every callable RPC method. +`role` is `admin` or `member` for paired connections and `null` otherwise. Unpaired plaintext connections retain their legacy effective capabilities, except those that require an authenticated connection — currently `update.apply`, which such a connection never receives. Clients should use capability presence for corresponding UI gates and treat role as display-only. Capability names currently include `profiles.manage`, `settings.write`, and `update.apply`; the array does not enumerate every callable RPC method. #### Parameters @@ -4960,9 +4968,13 @@ None. ### update.check -**Access:** All clients. +**Access:** Localhost or any paired client. + +Check if a newer version of Zaparoo Core is available. Returns version information, release notes, and everything a client needs to decide what to offer: whether the device is eligible for updates at all, whether the release has reached this device yet, and what is currently stopping one being installed. -Check if a newer version of Zaparoo Core is available. Returns version information and release notes. On development builds, always returns `updateAvailable: false`. +A check makes the device fetch and verify signed release metadata and write the result to its data directory, which is why it is not open to unpaired remote clients. + +On development builds, `updateAvailable` is always `false` and `eligibility` is `development`. #### Parameters @@ -4970,12 +4982,58 @@ None. #### Result -| Key | Type | Required | Description | -| :-------------- | :------ | :------- | :------------------------------------------------- | -| currentVersion | string | Yes | The currently running version. | -| latestVersion | string | No | The latest available version (if check succeeded). | -| updateAvailable | boolean | Yes | Whether a newer version is available. | -| releaseNotes | string | No | Release notes for the latest version. | +| Key | Type | Required | Description | +| :-------------- | :------ | :------- | :------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| currentVersion | string | Yes | The currently running version. | +| updateAvailable | boolean | Yes | Whether a newer version is available. | +| autoInstall | boolean | Yes | Whether the device installs updates on its own. Mirrors the `updateInstall` setting. | +| latestVersion | string | No | The latest available version (if the check succeeded). | +| releaseNotes | string | No | Release notes for the latest version. | +| channel | string | No | The update channel the check used: `stable` or `beta`. | +| eligibility | string | No | Whether this install can take OTA updates: `eligible`, `development`, `unsupported` (the platform has no OTA path), or `managed` (a package manager owns the install, so it should do the installing). A platform with no OTA path reports `unsupported` even when a package manager owns it, because that is the one an install is actually refused for. | +| checkedAt | string | No | RFC3339 timestamp of when the release metadata was last fetched. | +| rolloutHeld | boolean | No | The release is newer but has not reached this device's share of the fleet yet. Applying it by hand still works; automatic installs wait. | +| blockedBy | object | No | What is stopping an update being applied right now. Absent when nothing is. | +| deferredReason | string | No | Why an automatic install has been putting this version off. Same values as `blockedBy.reason`. | +| deferredSince | string | No | RFC3339 timestamp of when this version was first put off. After 24 hours an automatic install goes ahead through the signals that expire. | +| lastResult | object | No | How the previous update finished. Present until a newer result replaces it. | + +##### blockedBy + +| Key | Type | Required | Description | +| :-------- | :------ | :------- | :------------------------------------------------------------------------------------------------------------- | +| reason | string | Yes | Machine-readable reason, from the table below. | +| message | string | Yes | Human-readable explanation, suitable for showing as-is. | +| forceable | boolean | Yes | Whether `update.apply` with `force: true` goes ahead anyway. False means the refusal stands whatever is passed. | + +Reasons: + +| Reason | Forceable | Meaning | +| :---------------- | :-------- | :--------------------------------------------------------------- | +| mediaIndexing | No | The media database is being generated. | +| mediaOptimizing | No | The media database is being optimised. | +| mediaScraping | No | Media metadata is being scraped. | +| backupActive | No | A backup, restore or upload is running. | +| readerWriting | No | A reader is part-way through writing a token. | +| restoreActive | No | A restore is holding the databases. | +| activeMedia | Yes | Media is playing and a restart would close it. | +| backgroundMedia | Yes | Media is playing in the background. | +| activePlaylist | Yes | A playlist is running. | +| powerLow | No | The battery is below the level an install needs. | +| powerUnknown | Yes | The battery level could not be read. | +| apiBusy | Yes | The API has not been idle long enough. Automatic installs only. | + +`blockedBy` is what a client should read before offering an update: hide or disable the button when `forceable` is false, and offer to go ahead when it is true. + +##### lastResult + +| Key | Type | Required | Description | +| :---------- | :----- | :------- | :--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| at | string | Yes | RFC3339 timestamp of when the update finished. | +| outcome | string | Yes | `succeeded`, `rolledBack` (the new build would not start and the old one was put back), `rollbackBlocked` (the rollback could not be completed), or `recoveryRequired`. | +| fromVersion | string | No | The version before the update. | +| toVersion | string | No | The version the update was to. | +| detail | string | No | What went wrong, when something did. | #### Example @@ -4999,20 +5057,41 @@ None. "currentVersion": "2.9.1", "latestVersion": "2.10.0", "updateAvailable": true, - "releaseNotes": "..." + "autoInstall": false, + "releaseNotes": "...", + "channel": "stable", + "eligibility": "eligible", + "checkedAt": "2026-08-18T09:30:00Z", + "blockedBy": { + "reason": "activeMedia", + "message": "media is playing", + "forceable": true + } } } ``` ### update.apply -**Access:** All clients. +**Access:** Requires `update.apply`. + +Download and apply the latest available update, then gracefully restart the service. The response is sent to the client before the restart occurs. + +Before anything is downloaded the device checks that it is safe to install: nothing writing to the databases, no backup or token write in progress, nothing playing, and enough battery. A refusal comes back as an error whose message is the same text `update.check` reports in `blockedBy.message`. Call `update.check` first to know in advance, and whether `force` would get past it. -Download and apply the latest available update, then gracefully restart the service. The response is sent to the client before the restart occurs. Returns an error if media indexing is in progress or if running a development build. +The battery is checked twice — once before the download and again immediately before the install begins — because a download long enough to matter is also long enough to outlive a charger being unplugged. + +This method has no request timeout: the download and install run to completion or unwind on their own. Applying an update is treated as low priority, so it does not delay reader scans or playback control. + +While it runs, the device sends [`update.state`](notifications.md#updatestate) notifications. #### Parameters -None. +| Key | Type | Required | Description | +| :---- | :------ | :------- | :---------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| force | boolean | No | Go ahead through the signals `update.check` reports as `forceable`, such as media playing that the restart will close. It does not get past anything that risks data or a device without the power to finish. Defaults to false. | + +Parameters may be omitted entirely, which is the same as `force: false`. #### Result @@ -5029,7 +5108,10 @@ None. { "jsonrpc": "2.0", "id": "a1b2c3d4-1234-5678-9abc-def012345678", - "method": "update.apply" + "method": "update.apply", + "params": { + "force": true + } } ``` diff --git a/docs/api/notifications.md b/docs/api/notifications.md index 5f981f03d..db10d48fd 100644 --- a/docs/api/notifications.md +++ b/docs/api/notifications.md @@ -612,3 +612,57 @@ The `finished` notification is terminal for that operation, whatever its outcome } } ``` + +## Updates + +### update.state + +Sent while an update is being applied, so a client can show progress instead of a spinner that outlasts a large download. + +The stages that happen after the restart — `confirming`, `succeeded` and `rolledBack` — are not sent here, because no client is connected while they run. Read them from `update.check`'s `lastResult` once the service is back. + +Download progress is reported at most a few times a second, and the final byte always produces an event. + +#### Parameters + +| Key | Type | Required | Description | +| :-------------- | :----- | :------- | :---------------------------------------------------------------------------------------- | +| stage | string | Yes | `checking`, `downloading`, `verifying`, `probing`, `installing`, `restarting`, or `failed`. | +| version | string | No | The version being installed. | +| trigger | string | No | Who asked for it: `manual` or `auto`. | +| error | string | No | What went wrong. Only present on `failed`. | +| bytesDownloaded | number | No | Bytes downloaded so far. Only present during `downloading`. | +| bytesTotal | number | No | Total bytes to download, when the server reported a length. | + +#### Examples + +##### Downloading + +```json +{ + "jsonrpc": "2.0", + "method": "update.state", + "params": { + "stage": "downloading", + "version": "2.10.0", + "trigger": "manual", + "bytesDownloaded": 4194304, + "bytesTotal": 12582912 + } +} +``` + +##### Failed + +```json +{ + "jsonrpc": "2.0", + "method": "update.state", + "params": { + "stage": "failed", + "version": "2.10.0", + "trigger": "auto", + "error": "verifying the download: checksum mismatch" + } +} +``` diff --git a/pkg/api/methods/clients_test.go b/pkg/api/methods/clients_test.go index d2d7a4202..0d1331e11 100644 --- a/pkg/api/methods/clients_test.go +++ b/pkg/api/methods/clients_test.go @@ -120,6 +120,13 @@ func TestHandleClientsCurrent(t *testing.T) { adminCapabilities := []string{ string(permissions.CapProfilesManage), string(permissions.CapSettingsWrite), + string(permissions.CapUpdateApply), + } + // An unpaired remote client gets the admin capabilities apart from + // update.apply. + unpairedCapabilities := []string{ + string(permissions.CapProfilesManage), + string(permissions.CapSettingsWrite), } tests := []struct { name string @@ -144,8 +151,8 @@ func TestHandleClientsCurrent(t *testing.T) { wantPaired: true, }, { - name: "remote unpaired keeps legacy grant", - wantCapabilities: adminCapabilities, + name: "remote unpaired has no update.apply", + wantCapabilities: unpairedCapabilities, }, { name: "local unpaired gets local grant", diff --git a/pkg/api/methods/permissions.go b/pkg/api/methods/permissions.go index 3d9644c09..b0ce8b883 100644 --- a/pkg/api/methods/permissions.go +++ b/pkg/api/methods/permissions.go @@ -48,6 +48,17 @@ func requireCapability(env *requests.RequestEnv, capability permissions.Capabili return nil } +// requireAuthenticated returns a client error unless the request came from +// the device itself or from a paired client. Any paired client passes, +// member included. Use it for methods anyone in the household may call but +// a stranger on the network may not. +func requireAuthenticated(env *requests.RequestEnv) error { + if !requestGrant(env).Authenticated() { + return models.ClientErrf("%w", ErrForbidden) + } + return nil +} + // requireProfileManagement permits trusted local UI requests and requires // the profile-management capability from remote clients. Local profile PIN // prompts are a UI nuisance barrier, not API authorization. diff --git a/pkg/api/methods/run_test.go b/pkg/api/methods/run_test.go index d7335a4bc..11eef5f2f 100644 --- a/pkg/api/methods/run_test.go +++ b/pkg/api/methods/run_test.go @@ -46,9 +46,9 @@ func TestHandleStopWaitsForLaunchAndMediaReadiness(t *testing.T) { st, _ := state.NewState(mockPlatform, "test-boot") defer st.StopService() - releaseLaunch, err := st.AcquireMediaLaunch() + launchAccess, err := st.AcquireMediaLaunch() require.NoError(t, err) - st.SetActiveMedia(models.NewActiveMedia("snes", "SNES", "game.sfc", "Game", "RASNES")) + launchAccess.SetActiveMedia(models.NewActiveMedia("snes", "SNES", "game.sfc", "Game", "RASNES")) readyGen, active := st.ActiveMediaReadyGeneration() require.True(t, active) @@ -72,7 +72,7 @@ func TestHandleStopWaitsForLaunchAndMediaReadiness(t *testing.T) { } mockPlatform.AssertNotCalled(t, "StopActiveLauncher", platforms.StopForMenu) - releaseLaunch() + launchAccess.Release() select { case result := <-resultCh: require.NoError(t, result.err) @@ -99,9 +99,9 @@ func TestHandleStopCanceledWhileLaunchInFlight(t *testing.T) { st, _ := state.NewState(mockPlatform, "test-boot") defer st.StopService() - releaseLaunch, err := st.AcquireMediaLaunch() + launchAccess, err := st.AcquireMediaLaunch() require.NoError(t, err) - defer releaseLaunch() + defer launchAccess.Release() ctx, cancel := context.WithCancel(context.Background()) cancel() diff --git a/pkg/api/methods/settings.go b/pkg/api/methods/settings.go index 0c349dbc1..2eb776678 100644 --- a/pkg/api/methods/settings.go +++ b/pkg/api/methods/settings.go @@ -60,6 +60,8 @@ func HandleSettings(env requests.RequestEnv) (any, error) { //nolint:gocritic // resp := models.SettingsResponse{ UpdateChannel: env.Config.UpdateChannel(), + UpdateCheck: env.Config.UpdateCheck(), + UpdateInstall: env.Config.UpdateInstall(), RunZapScript: env.State.RunZapScriptEnabled(), DebugLogging: env.Config.DebugLogging(), AudioScanFeedback: env.Config.AudioFeedback(), @@ -149,6 +151,9 @@ func HandleSettingsUpdate(env requests.RequestEnv) (any, error) { } } + releaseConfig := env.Config.AcquireUpdateLock() + defer releaseConfig() + // Pre-flight validation of inputs that depend on runtime state. Run before // any mutations are applied so a validation failure here does not leave // the in-memory config partially updated. @@ -162,13 +167,27 @@ func HandleSettingsUpdate(env requests.RequestEnv) (any, error) { } // Reload config from disk before applying mutations so that external - // edits (e.g. user hand-editing config.toml) are not lost on save. - // TODO: Load+Set+Save is not atomic — concurrent handler calls can - // interleave. Needs a config-level transaction lock to fix properly. + // edits (e.g. user hand-editing config.toml) are not lost on save or + // validated against stale in-memory values. if err := env.Config.Load(); err != nil { log.Warn().Err(err).Msg("failed to reload config before settings update, using in-memory values") } + // Installing updates without checking for them is not a state the device + // can be in, so the combination is refused rather than stored and quietly + // ignored. A client asking for both at once is fine. + if params.UpdateInstall != nil && *params.UpdateInstall { + checking := env.Config.UpdateCheck() + if params.UpdateCheck != nil { + checking = *params.UpdateCheck + } + if !checking { + return nil, models.ClientErrf( + "installing updates automatically needs automatic update checking turned on", + ) + } + } + if params.RunZapScript != nil { log.Debug().Bool("runZapScript", *params.RunZapScript).Msg("updating setting") env.State.SetRunZapScript(*params.RunZapScript) @@ -179,6 +198,16 @@ func HandleSettingsUpdate(env requests.RequestEnv) (any, error) { env.Config.SetUpdateChannel(*params.UpdateChannel) } + if params.UpdateCheck != nil { + log.Debug().Bool("updateCheck", *params.UpdateCheck).Msg("updating setting") + env.Config.SetUpdateCheck(*params.UpdateCheck) + } + + if params.UpdateInstall != nil { + log.Debug().Bool("updateInstall", *params.UpdateInstall).Msg("updating setting") + env.Config.SetUpdateInstall(*params.UpdateInstall) + } + if params.DebugLogging != nil { log.Debug().Bool("debugLogging", *params.DebugLogging).Msg("updating setting") env.Config.SetDebugLogging(*params.DebugLogging) diff --git a/pkg/api/methods/settings_test.go b/pkg/api/methods/settings_test.go index 01e8221d4..7053bb81a 100644 --- a/pkg/api/methods/settings_test.go +++ b/pkg/api/methods/settings_test.go @@ -25,6 +25,7 @@ import ( "os" "path/filepath" "strings" + "sync/atomic" "testing" "time" @@ -40,6 +41,7 @@ import ( "github.com/ZaparooProject/zaparoo-core/v2/pkg/testing/helpers" "github.com/ZaparooProject/zaparoo-core/v2/pkg/testing/mocks" "github.com/jonboulle/clockwork" + "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -55,6 +57,22 @@ func drainCh(ch <-chan models.Notification) { } } +type configOpenTrackingFS struct { + afero.Fs + opens chan struct{} + track atomic.Bool +} + +func (fs *configOpenTrackingFS) Open(name string) (afero.File, error) { + if fs.track.Load() { + select { + case fs.opens <- struct{}{}: + default: + } + } + return fs.Fs.Open(name) //nolint:wrapcheck // test wrapper preserves the backing filesystem error +} + // TestHandlePlaytimeLimitsUpdate_ReEnableWithActiveMedia tests that re-enabling // playtime limits while a game is already running correctly triggers a session start. // This is a regression test for the bug where disabling then re-enabling limits @@ -203,6 +221,7 @@ func TestHandleSettings_ReaderConnections(t *testing.T) { mockPlatform := mocks.NewMockPlatform() mockPlatform.On("ID").Return("test-platform").Maybe() + mockPlatform.On("ManagedByPackageManager").Return(false).Maybe() tmpDir := t.TempDir() cfg, err := config.NewConfig(tmpDir, config.Values{ @@ -247,10 +266,11 @@ func TestHandleSettings_ReportsEncryptionSetting(t *testing.T) { }) require.NoError(t, err) mockPlatform := mocks.NewMockPlatform() + mockPlatform.On("ManagedByPackageManager").Return(false).Maybe() appState, ns := state.NewState(mockPlatform, "test-boot-uuid") t.Cleanup(func() { drainCh(ns) }) - result, err := HandleSettings(requests.RequestEnv{Config: cfg, State: appState}) + result, err := HandleSettings(requests.RequestEnv{Platform: mockPlatform, Config: cfg, State: appState}) require.NoError(t, err) resp, ok := result.(models.SettingsResponse) require.True(t, ok) @@ -264,6 +284,7 @@ func TestHandleSettings_EmptyReaderConnections(t *testing.T) { mockPlatform := mocks.NewMockPlatform() mockPlatform.On("ID").Return("test-platform").Maybe() + mockPlatform.On("ManagedByPackageManager").Return(false).Maybe() tmpDir := t.TempDir() cfg, err := config.NewConfig(tmpDir, config.Values{}) @@ -386,6 +407,7 @@ func TestHandleSettings_ErrorReportingDefault(t *testing.T) { mockPlatform := mocks.NewMockPlatform() mockPlatform.On("ID").Return("test-platform").Maybe() + mockPlatform.On("ManagedByPackageManager").Return(false).Maybe() tmpDir := t.TempDir() cfg, err := config.NewConfig(tmpDir, config.Values{}) @@ -418,6 +440,7 @@ func TestHandleSettings_ErrorReportingEnabled(t *testing.T) { mockPlatform := mocks.NewMockPlatform() mockPlatform.On("ID").Return("test-platform").Maybe() + mockPlatform.On("ManagedByPackageManager").Return(false).Maybe() tmpDir := t.TempDir() cfg, err := config.NewConfig(tmpDir, config.Values{ @@ -556,6 +579,69 @@ func TestHandleSettingsUpdate_UpdateChannel(t *testing.T) { assert.Equal(t, config.UpdateChannelBeta, cfg.UpdateChannel()) } +// Checking is stored as a tri-state so an untouched install can follow the +// default, but the API has to report a plain answer either way. The default is +// on everywhere, including where a package manager owns the install, because a +// check only tells the user a newer release exists. +func TestHandleSettings_UpdateCheckRoundTrip(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + packaged bool + }{ + {name: "standalone install defaults to on"}, + {name: "package manager install also defaults to on", packaged: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + mockPlatform := mocks.NewMockPlatform() + mockPlatform.On("ID").Return("test-platform").Maybe() + mockPlatform.On("ManagedByPackageManager").Return(tt.packaged).Maybe() + + cfg, err := config.NewConfig(t.TempDir(), config.Values{}) + require.NoError(t, err) + + appState, ns := state.NewState(mockPlatform, "test-boot-uuid") + t.Cleanup(func() { drainCh(ns) }) + + env := requests.RequestEnv{ + Context: context.Background(), + Platform: mockPlatform, + Config: cfg, + State: appState, + } + + result, err := HandleSettings(env) + require.NoError(t, err) + resp, ok := result.(models.SettingsResponse) + require.True(t, ok) + assert.True(t, resp.UpdateCheck) + + // An explicit choice has to survive, including when it matches the + // default it is overriding. + for _, want := range []bool{false, true} { + paramsJSON, err := json.Marshal(models.UpdateSettingsParams{UpdateCheck: &want}) + require.NoError(t, err) + env.Params = paramsJSON + + _, err = HandleSettingsUpdate(env) + require.NoError(t, err) + assert.Equal(t, want, cfg.UpdateCheck()) + + result, err = HandleSettings(env) + require.NoError(t, err) + resp, ok = result.(models.SettingsResponse) + require.True(t, ok) + assert.Equal(t, want, resp.UpdateCheck) + } + }) + } +} + // TestHandleSettingsUpdate_ReaderConnectionsWithIDSource tests that IDSource // field is preserved when updating reader connections. func TestHandleSettingsUpdate_ReaderConnectionsWithIDSource(t *testing.T) { @@ -661,6 +747,7 @@ func TestHandleSettings_ReaderConnectionsEnabled(t *testing.T) { mockPlatform := mocks.NewMockPlatform() mockPlatform.On("ID").Return("test-platform").Maybe() + mockPlatform.On("ManagedByPackageManager").Return(false).Maybe() f := false tmpDir := t.TempDir() @@ -749,6 +836,7 @@ func TestHandleSettings_LaunchGuardDefaults(t *testing.T) { mockPlatform := mocks.NewMockPlatform() mockPlatform.On("ID").Return("test-platform").Maybe() + mockPlatform.On("ManagedByPackageManager").Return(false).Maybe() tmpDir := t.TempDir() cfg, err := config.NewConfig(tmpDir, config.Values{}) @@ -879,6 +967,7 @@ func TestHandleSettings_AudioVolumeDefault(t *testing.T) { mockPlatform := mocks.NewMockPlatform() mockPlatform.On("ID").Return("test-platform").Maybe() + mockPlatform.On("ManagedByPackageManager").Return(false).Maybe() tmpDir := t.TempDir() cfg, err := config.NewConfig(tmpDir, config.Values{}) @@ -1018,6 +1107,7 @@ func TestHandleSettings_SystemDefaults(t *testing.T) { mockPlatform := mocks.NewMockPlatform() mockPlatform.On("ID").Return("test-platform").Maybe() + mockPlatform.On("ManagedByPackageManager").Return(false).Maybe() tmpDir := t.TempDir() cfg, err := config.NewConfig(tmpDir, config.Values{ @@ -1322,10 +1412,12 @@ func TestHandleSettings_BackupRemoteBaseURLGatedToLocal(t *testing.T) { cfg, err := config.NewConfig(t.TempDir(), config.BaseDefaults) require.NoError(t, err) mockPlatform := mocks.NewMockPlatform() + mockPlatform.On("ManagedByPackageManager").Return(false).Maybe() appState, ns := state.NewState(mockPlatform, "test-boot-uuid") t.Cleanup(func() { drainCh(ns) }) - result, err := HandleSettings(requests.RequestEnv{Config: cfg, State: appState, IsLocal: true}) + env := requests.RequestEnv{Platform: mockPlatform, Config: cfg, State: appState, IsLocal: true} + result, err := HandleSettings(env) require.NoError(t, err) resp, ok := result.(models.SettingsResponse) require.True(t, ok) @@ -1334,10 +1426,317 @@ func TestHandleSettings_BackupRemoteBaseURLGatedToLocal(t *testing.T) { require.NotNil(t, resp.PlaytimeSyncEnabled) assert.False(t, *resp.PlaytimeSyncEnabled) - result, err = HandleSettings(requests.RequestEnv{Config: cfg, State: appState, IsLocal: false}) + env.IsLocal = false + result, err = HandleSettings(env) require.NoError(t, err) resp, ok = result.(models.SettingsResponse) require.True(t, ok) assert.Nil(t, resp.BackupRemoteBaseURL) assert.Nil(t, resp.PlaytimeSyncEnabled) } + +func TestHandleSettings_UpdateInstallRoundTrip(t *testing.T) { + t.Parallel() + + mockPlatform := mocks.NewMockPlatform() + mockPlatform.On("ID").Return("test-platform").Maybe() + mockPlatform.On("ManagedByPackageManager").Return(false).Maybe() + + cfg, err := config.NewConfig(t.TempDir(), config.Values{}) + require.NoError(t, err) + + appState, ns := state.NewState(mockPlatform, "test-boot-uuid") + t.Cleanup(func() { drainCh(ns) }) + + env := requests.RequestEnv{ + Context: context.Background(), + Platform: mockPlatform, + Config: cfg, + State: appState, + } + + // Installing on its own is off until someone asks for it, even on a + // platform where checking for updates is on by default. + result, err := HandleSettings(env) + require.NoError(t, err) + resp, ok := result.(models.SettingsResponse) + require.True(t, ok) + assert.True(t, resp.UpdateCheck) + assert.False(t, resp.UpdateInstall) + + enabled := true + paramsJSON, err := json.Marshal(models.UpdateSettingsParams{UpdateInstall: &enabled}) + require.NoError(t, err) + env.Params = paramsJSON + + _, err = HandleSettingsUpdate(env) + require.NoError(t, err) + + result, err = HandleSettings(env) + require.NoError(t, err) + resp, ok = result.(models.SettingsResponse) + require.True(t, ok) + assert.True(t, resp.UpdateInstall) + + disabled := false + paramsJSON, err = json.Marshal(models.UpdateSettingsParams{UpdateInstall: &disabled}) + require.NoError(t, err) + env.Params = paramsJSON + + _, err = HandleSettingsUpdate(env) + require.NoError(t, err) + + result, err = HandleSettings(env) + require.NoError(t, err) + resp, ok = result.(models.SettingsResponse) + require.True(t, ok) + assert.False(t, resp.UpdateInstall) +} + +func TestHandleSettingsUpdate_UpdateInstallNeedsChecking(t *testing.T) { + t.Parallel() + + enabled := true + disabled := false + + tests := []struct { + updateCheck *bool + name string + storedChecking bool + wantErr bool + }{ + { + name: "checking already on", + storedChecking: true, + }, + { + name: "turned on in the same call", + updateCheck: &enabled, + }, + { + name: "checking off and left off", + wantErr: true, + }, + { + name: "turned off in the same call", + updateCheck: &disabled, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + mockPlatform := mocks.NewMockPlatform() + mockPlatform.On("ID").Return("test-platform").Maybe() + mockPlatform.On("ManagedByPackageManager").Return(false).Maybe() + + cfg, err := config.NewConfig(t.TempDir(), config.Values{}) + require.NoError(t, err) + cfg.SetUpdateCheck(tt.storedChecking) + // The handler reloads config from disk before it writes, so a + // stored choice has to be on disk to still be there afterwards. + require.NoError(t, cfg.Save()) + + appState, ns := state.NewState(mockPlatform, "test-boot-uuid") + t.Cleanup(func() { drainCh(ns) }) + + paramsJSON, err := json.Marshal(models.UpdateSettingsParams{ + UpdateCheck: tt.updateCheck, + UpdateInstall: &enabled, + }) + require.NoError(t, err) + + env := requests.RequestEnv{ + Context: context.Background(), + Platform: mockPlatform, + Config: cfg, + State: appState, + Params: paramsJSON, + } + + _, err = HandleSettingsUpdate(env) + if tt.wantErr { + require.Error(t, err) + assert.Contains(t, err.Error(), "automatic update checking") + // The refusal comes before anything is written, so the device + // is not left installing updates it never checks for. + assert.False(t, cfg.UpdateInstall()) + return + } + require.NoError(t, err) + assert.True(t, cfg.UpdateInstall()) + }) + } +} + +func TestHandleSettingsUpdate_ConcurrentIndependentChanges(t *testing.T) { + t.Parallel() + + fs := &configOpenTrackingFS{ + Fs: afero.NewMemMapFs(), + opens: make(chan struct{}, 1), + } + cfg, err := config.NewConfigWithFs(t.TempDir(), config.Values{}, fs) + require.NoError(t, err) + + volumeEntered := make(chan struct{}) + releaseVolume := make(chan struct{}) + player := mocks.NewMockPlayer() + player.On("SetVolume", 0.25).Run(func(mock.Arguments) { + close(volumeEntered) + <-releaseVolume + }).Return().Once() + + volume := 25 + debugLogging := true + firstParams, err := json.Marshal(models.UpdateSettingsParams{ + AudioVolume: &volume, + DebugLogging: &debugLogging, + }) + require.NoError(t, err) + errorReporting := true + secondParams, err := json.Marshal(models.UpdateSettingsParams{ErrorReporting: &errorReporting}) + require.NoError(t, err) + + firstResult := make(chan error, 1) + go func() { + _, updateErr := HandleSettingsUpdate(requests.RequestEnv{ + Context: context.Background(), Config: cfg, Player: player, Params: firstParams, IsLocal: true, + }) + firstResult <- updateErr + }() + + select { + case <-volumeEntered: + case <-time.After(time.Second): + t.Fatal("first settings update did not reach its runtime side effect") + } + + fs.track.Store(true) + secondStarted := make(chan struct{}) + secondResult := make(chan error, 1) + go func() { + close(secondStarted) + _, updateErr := HandleSettingsUpdate(requests.RequestEnv{ + Context: context.Background(), Config: cfg, Params: secondParams, IsLocal: true, + }) + secondResult <- updateErr + }() + <-secondStarted + + select { + case <-fs.opens: + close(releaseVolume) + t.Fatal("second settings update loaded config before first update completed") + case <-time.After(100 * time.Millisecond): + } + close(releaseVolume) + + select { + case updateErr := <-firstResult: + require.NoError(t, updateErr) + case <-time.After(time.Second): + t.Fatal("first settings update did not complete") + } + select { + case updateErr := <-secondResult: + require.NoError(t, updateErr) + case <-time.After(time.Second): + t.Fatal("second settings update did not complete") + } + + require.NoError(t, cfg.Load()) + assert.True(t, cfg.DebugLogging()) + assert.True(t, cfg.ErrorReporting()) + assert.Equal(t, volume, cfg.AudioVolume()) + player.AssertExpectations(t) +} + +func TestHandleSettingsUpdate_UpdateInstallReloadsChecking(t *testing.T) { + t.Parallel() + + enabled := true + disabled := false + tests := []struct { + requestCheck *bool + name string + diskChecking bool + memoryChecking bool + wantChecking bool + wantErr bool + }{ + { + name: "disk enabled overrides stale disabled memory", + diskChecking: true, + memoryChecking: false, + wantChecking: true, + }, + { + name: "disk disabled overrides stale enabled memory", + diskChecking: false, + memoryChecking: true, + wantErr: true, + }, + { + name: "request enable overrides disabled disk", + diskChecking: false, + memoryChecking: true, + requestCheck: &enabled, + wantChecking: true, + }, + { + name: "request disable overrides enabled disk", + diskChecking: true, + memoryChecking: false, + requestCheck: &disabled, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + mockPlatform := mocks.NewMockPlatform() + mockPlatform.On("ID").Return("test-platform").Maybe() + + cfg, err := config.NewConfigWithFs(t.TempDir(), config.Values{}, afero.NewMemMapFs()) + require.NoError(t, err) + cfg.SetUpdateCheck(tt.diskChecking) + require.NoError(t, cfg.Save()) + cfg.SetUpdateCheck(tt.memoryChecking) + + appState, ns := state.NewState(mockPlatform, "test-boot-uuid") + t.Cleanup(appState.StopService) + t.Cleanup(func() { drainCh(ns) }) + + paramsJSON, err := json.Marshal(models.UpdateSettingsParams{ + UpdateCheck: tt.requestCheck, + UpdateInstall: &enabled, + }) + require.NoError(t, err) + + _, err = HandleSettingsUpdate(requests.RequestEnv{ + Context: context.Background(), + Platform: mockPlatform, + Config: cfg, + State: appState, + Params: paramsJSON, + IsLocal: true, + }) + if tt.wantErr { + require.Error(t, err) + assert.Contains(t, err.Error(), "automatic update checking") + assert.False(t, cfg.UpdateInstall()) + assert.Equal(t, tt.diskChecking, cfg.UpdateCheck()) + return + } + + require.NoError(t, err) + assert.True(t, cfg.UpdateInstall()) + assert.Equal(t, tt.wantChecking, cfg.UpdateCheck()) + }) + } +} diff --git a/pkg/api/methods/update.go b/pkg/api/methods/update.go index de812ed3c..9e4484d0b 100644 --- a/pkg/api/methods/update.go +++ b/pkg/api/methods/update.go @@ -28,9 +28,13 @@ import ( "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models" "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models/requests" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/notifications" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/permissions" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/validation" "github.com/ZaparooProject/zaparoo-core/v2/pkg/config" - "github.com/ZaparooProject/zaparoo-core/v2/pkg/database/mediadb" "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/power" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/platforms" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/updater" "github.com/rs/zerolog/log" ) @@ -86,11 +90,14 @@ func (g *updateRestartGuard) finish(fallback bool) { } // updaterOptions describes the device to the updater. -func updaterOptions(env *requests.RequestEnv) updater.Options { +func updaterOptions(env *requests.RequestEnv, mode updater.Mode) updater.Options { opts := updater.Options{ PlatformID: env.Platform.ID(), Channel: env.Config.UpdateChannel(), DataDir: helpers.DataDir(env.Platform), + DeviceID: env.Config.DeviceID(), + Managed: env.Platform.ManagedByPackageManager(), + Mode: mode, } if env.Database != nil { opts.UserDB = env.Database.UserDB @@ -98,14 +105,84 @@ func updaterOptions(env *requests.RequestEnv) updater.Options { return opts } +// updateGateDeps wires the gate up to what this device is currently doing. +// Anything the request environment does not carry is left nil, which the gate +// reads as nothing to report rather than as a reason to refuse. +func updateGateDeps(env *requests.RequestEnv) *updater.GateDeps { + pl := env.Platform + deps := &updater.GateDeps{ + Power: func() power.Status { return platforms.PowerStatus(pl) }, + } + + if env.Database != nil { + deps.IndexingStatus = env.Database.MediaDB.GetIndexingStatus + deps.OptimizationStatus = env.Database.MediaDB.GetOptimizationStatus + deps.ScrapingStatus = env.Database.MediaDB.GetScrapingStatus + } + + st := env.State + if st == nil { + return deps + } + if coordinator := st.BackupCoordinator(); coordinator != nil { + deps.BackupActive = func() bool { + _, _, active := coordinator.Active() + return active + } + } + deps.ReaderWriteActive = st.AnyReaderWriteActive + deps.ActiveMedia = func() bool { return st.ActiveMedia() != nil } + deps.BackgroundMedia = func() bool { return st.BackgroundMedia() != nil } + deps.ActivePlaylist = func() bool { return st.GetActivePlaylist() != nil } + return deps +} + +// updateProgressFn forwards the updater's progress to every connected client. +func updateProgressFn(env *requests.RequestEnv) updater.ProgressFn { + if env.State == nil || env.State.Notifications == nil { + return nil + } + ns := env.State.Notifications + return func(progress updater.Progress) { + notifications.UpdateState(ns, models.UpdateStateNotification{ + Stage: string(progress.Stage), + Version: progress.Version, + Trigger: progress.Trigger, + Error: progress.Error, + BytesDownloaded: progress.BytesDownloaded, + BytesTotal: progress.BytesTotal, + }) + } +} + +// HandleUpdateCheck asks the release server what the newest build for this +// device is. Any local or paired client may call it, member included. A check +// makes the device fetch and verify signed metadata and write the result to +// its data directory, so unpaired remote clients are refused: otherwise +// anyone on the network could drive repeated flash writes and outbound +// requests. func HandleUpdateCheck( env requests.RequestEnv, //nolint:gocritic // hugeParam checkFn func(ctx context.Context, opts updater.Options) (*updater.Result, error), ) (any, error) { - result, err := checkFn(env.Context, updaterOptions(&env)) + if err := requireAuthenticated(&env); err != nil { + return nil, err + } + + autoInstall := env.Config.UpdateInstall() + + opts := updaterOptions(&env, updater.ModeManual) + // The gate is read here so a client knows what is in the way before it + // offers an update button, rather than finding out from a failed apply. + opts.Gate = updateGateDeps(&env) + + result, err := checkFn(env.Context, opts) if errors.Is(err, updater.ErrDevelopmentVersion) { return models.UpdateCheckResponse{ CurrentVersion: config.AppVersion, + Eligibility: updater.EligibilityDevelopment, + Channel: env.Config.UpdateChannel(), + AutoInstall: autoInstall, UpdateAvailable: false, }, nil } @@ -113,12 +190,42 @@ func HandleUpdateCheck( return nil, fmt.Errorf("update check failed: %w", err) } - return models.UpdateCheckResponse{ + resp := models.UpdateCheckResponse{ CurrentVersion: result.CurrentVersion, LatestVersion: result.LatestVersion, UpdateAvailable: result.UpdateAvailable, ReleaseNotes: result.ReleaseNotes, - }, nil + Channel: result.Channel, + Eligibility: result.Eligibility, + RolloutHeld: result.RolloutHeld, + AutoInstall: autoInstall, + DeferredReason: result.DeferredReason, + } + if !result.CheckedAt.IsZero() { + checkedAt := result.CheckedAt + resp.CheckedAt = &checkedAt + } + if !result.DeferredSince.IsZero() { + since := result.DeferredSince + resp.DeferredSince = &since + } + if result.BlockedReason != "" { + resp.BlockedBy = &models.UpdateBlockedBy{ + Reason: result.BlockedReason, + Message: result.BlockedMessage, + Forceable: result.BlockedForceable, + } + } + if result.LastResult != nil { + resp.LastResult = &models.UpdateLastResult{ + At: result.LastResult.At, + Outcome: result.LastResult.Outcome, + FromVersion: result.LastResult.FromVersion, + ToVersion: result.LastResult.ToVersion, + Detail: result.LastResult.Detail, + } + } + return resp, nil } func HandleUpdateApply( @@ -126,38 +233,63 @@ func HandleUpdateApply( applyFn func(ctx context.Context, opts updater.Options) (string, error), restartFn func(), ) (any, error) { - // Reject updates while media indexing is in progress to avoid - // interrupting database writes mid-transaction. - if env.Database != nil { - if status, err := env.Database.MediaDB.GetIndexingStatus(); err == nil { - if status == mediadb.IndexingStatusRunning || status == mediadb.IndexingStatusPending { - return nil, models.ClientErrf("cannot apply update while media indexing is in progress") - } + // Refuses paired members and unpaired remote clients alike. + if err := requireCapability(&env, permissions.CapUpdateApply); err != nil { + log.Warn(). + Str("clientId", env.ClientID). + Bool("local", env.IsLocal). + Str("role", env.ClientRole). + Msg("rejected update apply request") + return nil, err + } + + var params models.UpdateApplyParams + if len(env.Params) > 0 { + if err := validation.ValidateAndUnmarshal(env.Params, ¶ms); err != nil { + log.Warn().Err(err).Msg("invalid params") + return nil, models.ClientErrf("invalid params: %w", err) } } - releaseMediaGate := func() {} + // The gate takes the restore and media gates itself, in the order the rest + // of the service takes them, and holds them until the restart. + deps := updateGateDeps(&env) if env.State != nil { - release, err := env.State.AcquireUpdateMediaGate(env.Context) - if err != nil { - return nil, fmt.Errorf("waiting for media activity to settle before update: %w", err) - } - releaseMediaGate = release - if env.State.ActiveMedia() != nil { - releaseMediaGate() - return nil, models.ClientErrf("cannot apply update while media is active") - } + deps.AcquireRestore = env.State.TryAcquireRestoreAccess + deps.AcquireMediaGate = env.State.AcquireUpdateMediaGate + } + decision, err := updater.CanApplyUpdate(env.Context, deps, updater.ModeManual, params.Force) + if err != nil { + return nil, fmt.Errorf("preparing the device for an update: %w", err) + } + if !decision.OK { + log.Info(). + Str("reason", decision.Reason). + Bool("forceable", decision.Forceable). + Msg("refused an update the device is not ready for") + return nil, models.ClientErrf("%s", decision.Message) } + releaseBeforeRestart := true + release := decision.Release defer func() { if releaseBeforeRestart { - releaseMediaGate() + release() } }() previousVersion := config.AppVersion - newVersion, err := applyFn(env.Context, updaterOptions(&env)) + opts := updaterOptions(&env, updater.ModeManual) + opts.Progress = updateProgressFn(&env) + // The download can outlast a charger being unplugged, so the power reading + // is taken again at the last moment the install can still be called off. + opts.PreQuiesce = func(context.Context) error { + powered := updater.PowerReady(deps, updater.ModeManual, params.Force) + return powered.Err() + } + + newVersion, err := applyFn(env.Context, opts) if errors.Is(err, updater.ErrDevelopmentVersion) { return nil, models.ClientErrf("cannot apply updates on development builds") } @@ -173,12 +305,16 @@ func HandleUpdateApply( // The error already says what to do instead. return nil, models.ClientErrf("%s", err.Error()) } + var gateErr *updater.GateError + if errors.As(err, &gateErr) { + return nil, models.ClientErrf("%s", gateErr.Message) + } if err != nil { return nil, fmt.Errorf("update apply failed: %w", err) } restartGuard := newUpdateRestartGuard( - updateAfterWriteFallbackDelay, previousVersion, newVersion, restartFn, releaseMediaGate, + updateAfterWriteFallbackDelay, previousVersion, newVersion, restartFn, release, ) releaseBeforeRestart = false return models.ResponseWithCallback{ diff --git a/pkg/api/methods/update_test.go b/pkg/api/methods/update_test.go index a2e228af7..59609c43d 100644 --- a/pkg/api/methods/update_test.go +++ b/pkg/api/methods/update_test.go @@ -21,6 +21,7 @@ package methods import ( "context" + "encoding/json" "errors" "fmt" "path/filepath" @@ -30,10 +31,14 @@ import ( "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models" "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models/requests" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/permissions" "github.com/ZaparooProject/zaparoo-core/v2/pkg/config" "github.com/ZaparooProject/zaparoo-core/v2/pkg/database" "github.com/ZaparooProject/zaparoo-core/v2/pkg/database/mediadb" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/power" "github.com/ZaparooProject/zaparoo-core/v2/pkg/platforms" + backupcoordinator "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/backup/coordinator" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/playlists" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/state" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/updater" "github.com/ZaparooProject/zaparoo-core/v2/pkg/testing/helpers" @@ -58,6 +63,7 @@ func TestHandleUpdateCheck_DevelopmentVersion(t *testing.T) { Context: t.Context(), Platform: mockPlatform, Config: &config.Instance{}, + IsLocal: true, } result, err := HandleUpdateCheck(env, updater.Check) @@ -82,6 +88,7 @@ func TestHandleUpdateCheck_UpdateAvailable(t *testing.T) { Context: t.Context(), Platform: mockPlatform, Config: &config.Instance{}, + IsLocal: true, } checkFn := func(_ context.Context, _ updater.Options) (*updater.Result, error) { @@ -114,6 +121,7 @@ func TestHandleUpdateCheck_BetaChannel(t *testing.T) { mockPlatform := mocks.NewMockPlatform() mockPlatform.On("ID").Return("mock-platform") mockPlatform.On("Settings").Return(platforms.Settings{DataDir: dataDir}) + mockPlatform.On("ManagedByPackageManager").Return(false) cfg := &config.Instance{} cfg.SetUpdateChannel(config.UpdateChannelBeta) @@ -122,6 +130,7 @@ func TestHandleUpdateCheck_BetaChannel(t *testing.T) { Context: t.Context(), Platform: mockPlatform, Config: cfg, + IsLocal: true, } var received updater.Options @@ -152,6 +161,7 @@ func TestHandleUpdateCheck_NoUpdateAvailable(t *testing.T) { Context: t.Context(), Platform: mockPlatform, Config: &config.Instance{}, + IsLocal: true, } checkFn := func(_ context.Context, _ updater.Options) (*updater.Result, error) { @@ -181,6 +191,7 @@ func TestHandleUpdateCheck_Error(t *testing.T) { Context: t.Context(), Platform: mockPlatform, Config: &config.Instance{}, + IsLocal: true, } checkFn := func(_ context.Context, _ updater.Options) (*updater.Result, error) { @@ -194,6 +205,129 @@ func TestHandleUpdateCheck_Error(t *testing.T) { assert.Nil(t, result) } +// A check is not a privileged action — a household member may reasonably want +// to know an update exists — but it makes the device fetch signed metadata and +// write it to disk, so an unpaired remote client cannot have it either. +func TestHandleUpdateCheck_Authorization(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + clientRole string + isLocal bool + wantAllowed bool + }{ + {name: "unpaired remote is refused"}, + {name: "local unpaired is allowed", isLocal: true, wantAllowed: true}, + {name: "paired member is allowed", clientRole: string(permissions.RoleMember), wantAllowed: true}, + {name: "paired admin is allowed", clientRole: string(permissions.RoleAdmin), wantAllowed: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + mockPlatform := mocks.NewMockPlatform() + mockPlatform.SetupBasicMock() + + checked := false + checkFn := func(_ context.Context, _ updater.Options) (*updater.Result, error) { + checked = true + return &updater.Result{CurrentVersion: "2.9.0"}, nil + } + + env := requests.RequestEnv{ + Context: t.Context(), + Platform: mockPlatform, + Config: &config.Instance{}, + ClientRole: tt.clientRole, + IsLocal: tt.isLocal, + } + + _, err := HandleUpdateCheck(env, checkFn) + if !tt.wantAllowed { + require.ErrorIs(t, err, ErrForbidden) + assert.False(t, checked, "a refused request must not reach the release server") + return + } + require.NoError(t, err) + assert.True(t, checked) + }) + } +} + +func TestUpdateProgressFn_ForwardsEveryField(t *testing.T) { + t.Parallel() + + mockPlatform := mocks.NewMockPlatform() + appState, ns := state.NewState(mockPlatform, "test-boot") + t.Cleanup(appState.StopService) + t.Cleanup(func() { drainCh(ns) }) + + progressFn := updateProgressFn(&requests.RequestEnv{State: appState}) + require.NotNil(t, progressFn) + progressFn(updater.Progress{ + Stage: updater.ProgressDownloading, + Version: "2.10.0", + Trigger: "manual", + Error: "test detail", + BytesDownloaded: 1234, + BytesTotal: 5678, + }) + + select { + case notification := <-ns: + assert.Equal(t, models.NotificationUpdateState, notification.Method) + var payload models.UpdateStateNotification + require.NoError(t, json.Unmarshal(notification.Params, &payload)) + assert.Equal(t, string(updater.ProgressDownloading), payload.Stage) + assert.Equal(t, "2.10.0", payload.Version) + assert.Equal(t, "manual", payload.Trigger) + assert.Equal(t, "test detail", payload.Error) + assert.Equal(t, int64(1234), payload.BytesDownloaded) + assert.Equal(t, int64(5678), payload.BytesTotal) + case <-time.After(time.Second): + t.Fatal("timed out waiting for update progress notification") + } +} + +func TestUpdateGateDeps_ReportsStateSignals(t *testing.T) { + t.Parallel() + + mockPlatform := mocks.NewMockPlatform() + mockPlatform.SetupBasicMock() + appState, ns := state.NewState(mockPlatform, "test-boot") + t.Cleanup(appState.StopService) + t.Cleanup(func() { drainCh(ns) }) + + deps := updateGateDeps(&requests.RequestEnv{Platform: mockPlatform, State: appState}) + require.NotNil(t, deps.BackupActive) + require.NotNil(t, deps.BackgroundMedia) + require.NotNil(t, deps.ActivePlaylist) + assert.False(t, deps.BackupActive()) + assert.False(t, deps.BackgroundMedia()) + assert.False(t, deps.ActivePlaylist()) + + lease, err := appState.BackupCoordinator().Begin( + t.Context(), backupcoordinator.OperationLocalCreate, backupcoordinator.OperationRead, + ) + require.NoError(t, err) + appState.SetBackgroundMedia(models.NewActiveMedia( + "Audio", "Audio", "song.mp3", "Song", platforms.NativeAudioLauncherID, + )) + appState.SetActivePlaylist(&playlists.Playlist{ID: "playlist"}) + assert.True(t, deps.BackupActive()) + assert.True(t, deps.BackgroundMedia()) + assert.True(t, deps.ActivePlaylist()) + + lease.Release() + appState.SetBackgroundMedia(nil) + appState.SetActivePlaylist(nil) + assert.False(t, deps.BackupActive()) + assert.False(t, deps.BackgroundMedia()) + assert.False(t, deps.ActivePlaylist()) +} + func TestUpdateRestartGuard_AfterWriteSupersedesFallback(t *testing.T) { t.Parallel() @@ -242,6 +376,7 @@ func TestHandleUpdateApply_DevelopmentVersion(t *testing.T) { Context: t.Context(), Platform: mockPlatform, Config: &config.Instance{}, + IsLocal: true, } result, err := HandleUpdateApply(env, updater.Apply, func() {}) @@ -252,6 +387,67 @@ func TestHandleUpdateApply_DevelopmentVersion(t *testing.T) { } } +// Replacing the binary decides what code the device runs from then on, so +// update.apply needs the capability and a request from the device itself or +// from a paired client. An unpaired remote request resolves to admin, so it +// has to be refused here in its own right. +func TestHandleUpdateApply_Authorization(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + clientRole string + isLocal bool + wantAllowed bool + }{ + {name: "paired member is refused", clientRole: string(permissions.RoleMember)}, + {name: "unknown role degrades to member and is refused", clientRole: "superuser"}, + {name: "unpaired remote is refused", wantAllowed: false}, + {name: "paired admin is allowed", clientRole: string(permissions.RoleAdmin), wantAllowed: true}, + {name: "local member is allowed", clientRole: string(permissions.RoleMember), isLocal: true, wantAllowed: true}, + {name: "local unpaired is allowed", isLocal: true, wantAllowed: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + mockPlatform := mocks.NewMockPlatform() + mockPlatform.SetupBasicMock() + + applied := false + applyFn := func(_ context.Context, _ updater.Options) (string, error) { + applied = true + return "2.0.0", nil + } + + env := requests.RequestEnv{ + Context: t.Context(), + Platform: mockPlatform, + Config: &config.Instance{}, + ClientRole: tt.clientRole, + IsLocal: tt.isLocal, + } + + result, err := HandleUpdateApply(env, applyFn, func() {}) + if !tt.wantAllowed { + require.ErrorIs(t, err, ErrForbidden) + assert.Nil(t, result) + assert.False(t, applied, "a refused request must not reach the updater") + return + } + require.NoError(t, err) + assert.True(t, applied) + + // Run the callback so the restart guard's fallback timer does not + // outlive the test. + callback, ok := result.(models.ResponseWithCallback) + require.True(t, ok) + callback.AfterWrite() + }) + } +} + func TestHandleUpdateApply_Error(t *testing.T) { t.Parallel() @@ -262,6 +458,7 @@ func TestHandleUpdateApply_Error(t *testing.T) { Context: t.Context(), Platform: mockPlatform, Config: &config.Instance{}, + IsLocal: true, } applyFn := func(_ context.Context, _ updater.Options) (string, error) { @@ -275,6 +472,36 @@ func TestHandleUpdateApply_Error(t *testing.T) { assert.Nil(t, result) } +func TestHandleUpdateApply_ErrorReleasesAcquiredGates(t *testing.T) { + t.Parallel() + + mockPlatform := mocks.NewMockPlatform() + mockPlatform.SetupBasicMock() + appState, ns := state.NewState(mockPlatform, "test-boot") + t.Cleanup(appState.StopService) + t.Cleanup(func() { drainCh(ns) }) + + result, err := HandleUpdateApply(requests.RequestEnv{ + Context: t.Context(), + Platform: mockPlatform, + Config: &config.Instance{}, + State: appState, + IsLocal: true, + }, func(context.Context, updater.Options) (string, error) { + return "", errors.New("download failed") + }, func() {}) + require.Error(t, err) + assert.Nil(t, result) + + finishRestore, err := appState.BeginRestoreGate() + require.NoError(t, err, "failed apply retained restore access") + finishRestore(false) + + releaseMedia, err := appState.AcquireUpdateMediaGate(t.Context()) + require.NoError(t, err, "failed apply retained media gate") + releaseMedia() +} + func TestHandleUpdateApply_UpdateInProgress(t *testing.T) { t.Parallel() @@ -284,6 +511,7 @@ func TestHandleUpdateApply_UpdateInProgress(t *testing.T) { Context: t.Context(), Platform: mockPlatform, Config: &config.Instance{}, + IsLocal: true, } applyFn := func(context.Context, updater.Options) (string, error) { return "", updater.ErrUpdateInProgress @@ -306,6 +534,7 @@ func TestHandleUpdateApply_InsufficientSpace(t *testing.T) { Context: t.Context(), Platform: mockPlatform, Config: &config.Instance{}, + IsLocal: true, } applyFn := func(context.Context, updater.Options) (string, error) { return "", fmt.Errorf("%w: /media/fat has 12 MB free, need at least 90 MB", @@ -328,6 +557,7 @@ func TestHandleUpdateApply_PlatformUnsupported(t *testing.T) { Context: t.Context(), Platform: mockPlatform, Config: &config.Instance{}, + IsLocal: true, } applyFn := func(context.Context, updater.Options) (string, error) { return "", fmt.Errorf("%w: use the Windows installer instead", updater.ErrPlatformUnsupported) @@ -356,6 +586,7 @@ func TestHandleUpdateApply_ActiveMedia(t *testing.T) { Platform: mockPlatform, Config: &config.Instance{}, State: st, + IsLocal: true, } applyFn := func(context.Context, updater.Options) (string, error) { t.Fatal("applyFn should not be called while media is active") @@ -364,7 +595,131 @@ func TestHandleUpdateApply_ActiveMedia(t *testing.T) { result, err := HandleUpdateApply(env, applyFn, func() {}) require.Error(t, err) - assert.Contains(t, err.Error(), "media is active") + assert.Contains(t, err.Error(), "media is playing") + assert.Nil(t, result) +} + +// A token write is recorded against the reader doing it, so the gate has to +// ask about every reader rather than about a reader ID no writer ever uses. +// Restarting part-way through an NDEF write leaves a half-written token. +func TestHandleUpdateApply_ReaderWriting(t *testing.T) { + t.Parallel() + + mockPlatform := mocks.NewMockPlatform() + mockPlatform.SetupBasicMock() + st, _ := state.NewState(mockPlatform, "test-boot") + t.Cleanup(st.StopService) + st.SetReaderWriteActive(true, "pn532_uart:/dev/ttyUSB0") + + applyFn := func(context.Context, updater.Options) (string, error) { + t.Fatal("applyFn should not be called while a reader is writing") + return "", nil + } + + // Writing a token is data the user is part-way through, so force does not + // get past it either. + for _, params := range []json.RawMessage{nil, json.RawMessage(`{"force":true}`)} { + env := requests.RequestEnv{ + Context: t.Context(), + Platform: mockPlatform, + Config: &config.Instance{}, + State: st, + Params: params, + IsLocal: true, + } + + result, err := HandleUpdateApply(env, applyFn, func() {}) + require.Error(t, err) + assert.Contains(t, err.Error(), "a token is being written") + assert.Nil(t, result) + } +} + +// Media playing is the user's session, not their data, so a client that has +// asked them about it can go ahead. +func TestHandleUpdateApply_ForcePastActiveMedia(t *testing.T) { + t.Parallel() + + mockPlatform := mocks.NewMockPlatform() + mockPlatform.SetupBasicMock() + st, _ := state.NewState(mockPlatform, "test-boot") + t.Cleanup(st.StopService) + st.SetActiveMedia(models.NewActiveMedia( + "SNES", "Super Nintendo", filepath.Join("roms", "game.sfc"), "Game", "test-launcher", + )) + + env := requests.RequestEnv{ + Context: t.Context(), + Platform: mockPlatform, + Config: &config.Instance{}, + State: st, + Params: json.RawMessage(`{"force":true}`), + IsLocal: true, + } + applyFn := func(context.Context, updater.Options) (string, error) { + return "2.10.0", nil + } + + result, err := HandleUpdateApply(env, applyFn, func() {}) + require.NoError(t, err) + callback, ok := result.(models.ResponseWithCallback) + require.True(t, ok) + callback.AfterWrite() +} + +// Force is a person accepting the loss of what is on screen. It is not a +// person able to make a flat battery last, so it does not get past one. +func TestHandleUpdateApply_ForceDoesNotBypassLowBattery(t *testing.T) { + t.Parallel() + + mockPlatform := mocks.NewMockPlatform() + mockPlatform.SetupBasicMock() + mockPlatform.SetPowerStatus(power.Status{Source: power.SourceBattery, Percent: 5}) + + env := requests.RequestEnv{ + Context: t.Context(), + Platform: mockPlatform, + Config: &config.Instance{}, + Params: json.RawMessage(`{"force":true}`), + IsLocal: true, + } + applyFn := func(context.Context, updater.Options) (string, error) { + t.Fatal("applyFn should not be called on a flat battery") + return "", nil + } + + result, err := HandleUpdateApply(env, applyFn, func() {}) + require.Error(t, err) + assert.Contains(t, err.Error(), "the battery is at 5%") + assert.Nil(t, result) +} + +// The download can run for minutes, which is long enough for someone to +// unplug the device, so the reading is taken again before anything is +// replaced. +func TestHandleUpdateApply_PreQuiesceRechecksPower(t *testing.T) { + t.Parallel() + + mockPlatform := mocks.NewMockPlatform() + mockPlatform.SetupBasicMock() + + env := requests.RequestEnv{ + Context: t.Context(), + Platform: mockPlatform, + Config: &config.Instance{}, + IsLocal: true, + } + applyFn := func(ctx context.Context, opts updater.Options) (string, error) { + require.NotNil(t, opts.PreQuiesce) + require.NoError(t, opts.PreQuiesce(ctx)) + // The charger comes out part-way through the download. + mockPlatform.SetPowerStatus(power.Status{Source: power.SourceBattery, Percent: 3}) + return "", opts.PreQuiesce(ctx) + } + + result, err := HandleUpdateApply(env, applyFn, func() {}) + require.Error(t, err) + assert.Contains(t, err.Error(), "the battery is at 3%") assert.Nil(t, result) } @@ -380,6 +735,7 @@ func TestHandleUpdateApply_HoldsMediaGateUntilRestart(t *testing.T) { Platform: mockPlatform, Config: &config.Instance{}, State: st, + IsLocal: true, } applyFn := func(context.Context, updater.Options) (string, error) { return "2.10.0", nil @@ -392,9 +748,9 @@ func TestHandleUpdateApply_HoldsMediaGateUntilRestart(t *testing.T) { launchErr := make(chan error, 1) go func() { - release, acquireErr := st.AcquireMediaLaunch() - if release != nil { - release() + access, acquireErr := st.AcquireMediaLaunch() + if access.Release != nil { + access.Release() } launchErr <- acquireErr }() @@ -437,6 +793,7 @@ func TestHandleUpdateApply_IndexingInProgress(t *testing.T) { Platform: mockPlatform, Config: &config.Instance{}, Database: &database.Database{MediaDB: mockMediaDB}, + IsLocal: true, } applyFn := func(_ context.Context, _ updater.Options) (string, error) { @@ -446,7 +803,7 @@ func TestHandleUpdateApply_IndexingInProgress(t *testing.T) { result, err := HandleUpdateApply(env, applyFn, func() {}) require.Error(t, err) - assert.Contains(t, err.Error(), "media indexing is in progress") + assert.Contains(t, err.Error(), "the media database is being generated") assert.Nil(t, result) mockMediaDB.AssertExpectations(t) @@ -462,6 +819,8 @@ func TestHandleUpdateApply_IndexingCompleted(t *testing.T) { mockMediaDB := helpers.NewMockMediaDBI() mockMediaDB.On("GetIndexingStatus").Return(mediadb.IndexingStatusCompleted, nil) + mockMediaDB.On("GetOptimizationStatus").Return(mediadb.IndexingStatusCompleted, nil) + mockMediaDB.On("GetScrapingStatus").Return(mediadb.IndexingStatusCompleted, nil) mockUserDB := helpers.NewMockUserDBI() env := requests.RequestEnv{ @@ -469,6 +828,7 @@ func TestHandleUpdateApply_IndexingCompleted(t *testing.T) { Platform: mockPlatform, Config: &config.Instance{}, Database: &database.Database{UserDB: mockUserDB, MediaDB: mockMediaDB}, + IsLocal: true, } applyFn := func(_ context.Context, opts updater.Options) (string, error) { @@ -495,3 +855,279 @@ func TestHandleUpdateApply_IndexingCompleted(t *testing.T) { mockMediaDB.AssertExpectations(t) } + +func TestHandleUpdateCheck_ReportsEverythingAClientNeeds(t *testing.T) { + t.Parallel() + + mockPlatform := mocks.NewMockPlatform() + mockPlatform.SetupBasicMock() + + cfg := &config.Instance{} + cfg.SetUpdateCheck(true) + cfg.SetUpdateInstall(true) + + env := requests.RequestEnv{ + Context: t.Context(), + Platform: mockPlatform, + Config: cfg, + IsLocal: true, + } + + checkedAt := time.Date(2026, 8, 18, 9, 30, 0, 0, time.UTC) + deferredSince := time.Date(2026, 8, 17, 21, 0, 0, 0, time.UTC) + finishedAt := time.Date(2026, 8, 10, 4, 0, 0, 0, time.UTC) + + checkFn := func(_ context.Context, _ updater.Options) (*updater.Result, error) { + return &updater.Result{ + CheckedAt: checkedAt, + CurrentVersion: "2.9.0", + LatestVersion: "2.10.0", + UpdateAvailable: true, + ReleaseNotes: "New features", + Channel: "stable", + Eligibility: updater.EligibilityEligible, + RolloutHeld: true, + DeferredReason: updater.ReasonActiveMedia, + DeferredSince: deferredSince, + BlockedReason: updater.ReasonActiveMedia, + BlockedMessage: "media is playing", + BlockedForceable: true, + LastResult: &updater.OutcomeReport{ + At: finishedAt, + Outcome: "rolledBack", + FromVersion: "2.8.0", + ToVersion: "2.9.0", + Detail: "the new build would not start", + }, + }, nil + } + + result, err := HandleUpdateCheck(env, checkFn) + require.NoError(t, err) + + resp, ok := result.(models.UpdateCheckResponse) + require.True(t, ok) + assert.Equal(t, "stable", resp.Channel) + assert.Equal(t, updater.EligibilityEligible, resp.Eligibility) + assert.True(t, resp.RolloutHeld) + assert.True(t, resp.AutoInstall) + assert.Equal(t, updater.ReasonActiveMedia, resp.DeferredReason) + require.NotNil(t, resp.CheckedAt) + assert.True(t, checkedAt.Equal(*resp.CheckedAt)) + require.NotNil(t, resp.DeferredSince) + assert.True(t, deferredSince.Equal(*resp.DeferredSince)) + + // blockedBy is what a client reads to decide between hiding the update + // button and offering to go ahead anyway. + require.NotNil(t, resp.BlockedBy) + assert.Equal(t, updater.ReasonActiveMedia, resp.BlockedBy.Reason) + assert.Equal(t, "media is playing", resp.BlockedBy.Message) + assert.True(t, resp.BlockedBy.Forceable) + + require.NotNil(t, resp.LastResult) + assert.True(t, finishedAt.Equal(resp.LastResult.At)) + assert.Equal(t, "rolledBack", resp.LastResult.Outcome) + assert.Equal(t, "2.8.0", resp.LastResult.FromVersion) + assert.Equal(t, "2.9.0", resp.LastResult.ToVersion) + assert.Equal(t, "the new build would not start", resp.LastResult.Detail) +} + +func TestHandleUpdateCheck_OmitsWhatIsNotHappening(t *testing.T) { + t.Parallel() + + mockPlatform := mocks.NewMockPlatform() + mockPlatform.SetupBasicMock() + + env := requests.RequestEnv{ + Context: t.Context(), + Platform: mockPlatform, + Config: &config.Instance{}, + IsLocal: true, + } + + checkFn := func(_ context.Context, _ updater.Options) (*updater.Result, error) { + return &updater.Result{ + CurrentVersion: "2.10.0", + LatestVersion: "2.10.0", + UpdateAvailable: false, + }, nil + } + + result, err := HandleUpdateCheck(env, checkFn) + require.NoError(t, err) + resp, ok := result.(models.UpdateCheckResponse) + require.True(t, ok) + assert.Nil(t, resp.BlockedBy) + assert.Nil(t, resp.LastResult) + assert.Nil(t, resp.CheckedAt) + assert.Nil(t, resp.DeferredSince) + assert.False(t, resp.AutoInstall) + + // A zero time must not reach a client as 0001-01-01, and an absent block + // must not reach it as an empty object it has to special-case. + encoded, err := json.Marshal(resp) + require.NoError(t, err) + assert.NotContains(t, string(encoded), "blockedBy") + assert.NotContains(t, string(encoded), "checkedAt") + assert.NotContains(t, string(encoded), "lastResult") + assert.NotContains(t, string(encoded), "deferredSince") +} + +// TestHandleUpdateCheck_ReadsTheGate proves a check reports what the device is +// busy with, so a client can say why the update button is not there. +func TestHandleUpdateCheck_ReadsTheGate(t *testing.T) { + t.Parallel() + + mockPlatform := mocks.NewMockPlatform() + mockPlatform.SetupBasicMock() + + appState, ns := state.NewState(mockPlatform, "test-boot-uuid") + t.Cleanup(appState.StopService) + t.Cleanup(func() { drainCh(ns) }) + appState.SetActiveMedia(&models.ActiveMedia{SystemID: "SNES", Name: "Test Game"}) + + env := requests.RequestEnv{ + Context: t.Context(), + Platform: mockPlatform, + Config: &config.Instance{}, + State: appState, + IsLocal: true, + } + + var received updater.Options + checkFn := func(_ context.Context, opts updater.Options) (*updater.Result, error) { + received = opts + return &updater.Result{ + CurrentVersion: "2.9.0", + LatestVersion: "2.10.0", + UpdateAvailable: true, + }, nil + } + + _, err := HandleUpdateCheck(env, checkFn) + require.NoError(t, err) + + require.NotNil(t, received.Gate) + require.NotNil(t, received.Gate.ActiveMedia) + assert.True(t, received.Gate.ActiveMedia()) + require.NotNil(t, received.Gate.Power) + assert.NotEmpty(t, received.Gate.Power().Source) +} + +func TestHandleUpdateCheck_DevelopmentVersionReportsEligibility(t *testing.T) { + original := config.AppVersion + config.AppVersion = "DEVELOPMENT" + t.Cleanup(func() { config.AppVersion = original }) + + mockPlatform := mocks.NewMockPlatform() + mockPlatform.SetupBasicMock() + + cfg := &config.Instance{} + cfg.SetUpdateChannel(config.UpdateChannelBeta) + + env := requests.RequestEnv{ + Context: t.Context(), + Platform: mockPlatform, + Config: cfg, + IsLocal: true, + } + + checkFn := func(_ context.Context, _ updater.Options) (*updater.Result, error) { + return nil, updater.ErrDevelopmentVersion + } + + result, err := HandleUpdateCheck(env, checkFn) + require.NoError(t, err) + resp, ok := result.(models.UpdateCheckResponse) + require.True(t, ok) + assert.Equal(t, updater.EligibilityDevelopment, resp.Eligibility) + assert.Equal(t, "beta", resp.Channel) + assert.False(t, resp.UpdateAvailable) +} + +// TestHandleUpdateApply_ForceDoesNotElevate proves force is a way past what a +// person can be asked about, not a way past who they are. +func TestHandleUpdateApply_ForceDoesNotElevate(t *testing.T) { + t.Parallel() + + mockPlatform := mocks.NewMockPlatform() + mockPlatform.SetupBasicMock() + + applied := false + applyFn := func(_ context.Context, _ updater.Options) (string, error) { + applied = true + return "2.0.0", nil + } + + env := requests.RequestEnv{ + Context: t.Context(), + Platform: mockPlatform, + Config: &config.Instance{}, + ClientRole: string(permissions.RoleMember), + Params: json.RawMessage(`{"force":true}`), + } + + result, err := HandleUpdateApply(env, applyFn, func() {}) + require.ErrorIs(t, err, ErrForbidden) + assert.Nil(t, result) + assert.False(t, applied) +} + +func TestHandleUpdateApply_InvalidParams(t *testing.T) { + t.Parallel() + + mockPlatform := mocks.NewMockPlatform() + mockPlatform.SetupBasicMock() + + applied := false + applyFn := func(_ context.Context, _ updater.Options) (string, error) { + applied = true + return "2.0.0", nil + } + + env := requests.RequestEnv{ + Context: t.Context(), + Platform: mockPlatform, + Config: &config.Instance{}, + IsLocal: true, + Params: json.RawMessage(`{"force":"yes"}`), + } + + result, err := HandleUpdateApply(env, applyFn, func() {}) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid params") + assert.Nil(t, result) + assert.False(t, applied) +} + +func TestHandleUpdateApply_NoParamsIsNotForced(t *testing.T) { + t.Parallel() + + mockPlatform := mocks.NewMockPlatform() + mockPlatform.SetupBasicMock() + + appState, ns := state.NewState(mockPlatform, "test-boot-uuid") + t.Cleanup(appState.StopService) + t.Cleanup(func() { drainCh(ns) }) + appState.SetActiveMedia(&models.ActiveMedia{SystemID: "SNES", Name: "Test Game"}) + + applied := false + applyFn := func(_ context.Context, _ updater.Options) (string, error) { + applied = true + return "2.0.0", nil + } + + env := requests.RequestEnv{ + Context: t.Context(), + Platform: mockPlatform, + Config: &config.Instance{}, + State: appState, + IsLocal: true, + } + + result, err := HandleUpdateApply(env, applyFn, func() {}) + require.Error(t, err) + assert.Contains(t, err.Error(), "media is playing") + assert.Nil(t, result) + assert.False(t, applied) +} diff --git a/pkg/api/models/models.go b/pkg/api/models/models.go index 347b3b1cd..5d49c7146 100644 --- a/pkg/api/models/models.go +++ b/pkg/api/models/models.go @@ -45,6 +45,7 @@ const ( NotificationUIChanged = "ui.changed" NotificationAuthLinkStatus = "auth.link.status" NotificationBackupState = "backup.state" + NotificationUpdateState = "update.state" ) // Profile data swap statuses reported by the profiles.data notification. @@ -188,12 +189,18 @@ const ( // caller cancellation, shutdown, and per-transfer timeouts instead; both the // server request context and the local client wait consult this list so the // two cannot disagree. +// +// update.apply belongs here for the same reason: it downloads an archive over +// whatever connection the device has, and the request context cancelling +// mid-install would abort the one operation that must either finish or unwind +// cleanly. Its own stall guard and probe timeouts bound the stages instead. func MethodHasUnboundedRuntime(method string) bool { switch strings.ToLower(method) { case MethodSettingsBackup, MethodSettingsBackupRestore, MethodSettingsBackupRemoteRun, - MethodSettingsBackupRemoteRestore: + MethodSettingsBackupRemoteRestore, + MethodUpdateApply: return true default: return false diff --git a/pkg/api/models/params.go b/pkg/api/models/params.go index d13ac1d9f..97a84f531 100644 --- a/pkg/api/models/params.go +++ b/pkg/api/models/params.go @@ -184,6 +184,8 @@ type UpdateSettingsParams struct { LaunchGuardRequireConfirm *bool `json:"launchGuardRequireConfirm"` ProfilesRequireForLaunch *bool `json:"profilesRequireForLaunch"` ProfilesSwapData *bool `json:"profilesSwapData"` + UpdateCheck *bool `json:"updateCheck"` + UpdateInstall *bool `json:"updateInstall"` } type UpdatePlaytimeLimitsParams struct { @@ -380,3 +382,10 @@ type MediaTitleParseParams struct { SystemID string `json:"systemId" validate:"required,min=1"` Path string `json:"path" validate:"required,min=1"` } + +// UpdateApplyParams are the optional arguments to update.apply. Force lets a +// person go ahead while something is playing that a restart will close; it does +// not get past anything that would risk their data. +type UpdateApplyParams struct { + Force bool `json:"force"` +} diff --git a/pkg/api/models/responses.go b/pkg/api/models/responses.go index 5814d52bb..921f11d23 100644 --- a/pkg/api/models/responses.go +++ b/pkg/api/models/responses.go @@ -161,6 +161,8 @@ type SettingsResponse struct { LaunchGuardRequireConfirm bool `json:"launchGuardRequireConfirm"` ProfilesRequireForLaunch bool `json:"profilesRequireForLaunch"` ProfilesSwapData bool `json:"profilesSwapData"` + UpdateCheck bool `json:"updateCheck"` + UpdateInstall bool `json:"updateInstall"` } type PlaytimeLimitsResponse struct { @@ -852,10 +854,51 @@ type BackupStateNotification struct { } type UpdateCheckResponse struct { - CurrentVersion string `json:"currentVersion"` - LatestVersion string `json:"latestVersion,omitempty"` - ReleaseNotes string `json:"releaseNotes,omitempty"` - UpdateAvailable bool `json:"updateAvailable"` + DeferredSince *time.Time `json:"deferredSince,omitempty"` + LastResult *UpdateLastResult `json:"lastResult,omitempty"` + BlockedBy *UpdateBlockedBy `json:"blockedBy,omitempty"` + CheckedAt *time.Time `json:"checkedAt,omitempty"` + CurrentVersion string `json:"currentVersion"` + ReleaseNotes string `json:"releaseNotes,omitempty"` + Channel string `json:"channel,omitempty"` + Eligibility string `json:"eligibility,omitempty"` + DeferredReason string `json:"deferredReason,omitempty"` + LatestVersion string `json:"latestVersion,omitempty"` + UpdateAvailable bool `json:"updateAvailable"` + RolloutHeld bool `json:"rolloutHeld,omitempty"` + AutoInstall bool `json:"autoInstall"` +} + +// UpdateBlockedBy is what the device is busy with that stops an update being +// installed. Forceable means a person may go ahead anyway by passing force to +// update.apply, which is true for things that only cost them their session and +// false for anything that risks their data. +type UpdateBlockedBy struct { + Reason string `json:"reason"` + Message string `json:"message"` + Forceable bool `json:"forceable"` +} + +// UpdateLastResult is how the previous update finished. +type UpdateLastResult struct { + At time.Time `json:"at"` + Outcome string `json:"outcome"` + FromVersion string `json:"fromVersion,omitempty"` + ToVersion string `json:"toVersion,omitempty"` + Detail string `json:"detail,omitempty"` +} + +// UpdateStateNotification is the payload for the update.state notification, +// sent while an update is being applied. The stages after the restart — +// confirming, succeeded and rolledBack — happen before any client is back, so +// they are reported by update.check's lastResult rather than here. +type UpdateStateNotification struct { + Stage string `json:"stage"` + Version string `json:"version,omitempty"` + Trigger string `json:"trigger,omitempty"` + Error string `json:"error,omitempty"` + BytesDownloaded int64 `json:"bytesDownloaded,omitempty"` + BytesTotal int64 `json:"bytesTotal,omitempty"` } type UpdateApplyResponse struct { diff --git a/pkg/api/notifications/notifications.go b/pkg/api/notifications/notifications.go index 55a6d0c6f..031d26872 100644 --- a/pkg/api/notifications/notifications.go +++ b/pkg/api/notifications/notifications.go @@ -167,3 +167,10 @@ func AuthLinkStatus(ns chan<- models.Notification, payload *models.AuthLinkStatu func BackupState(ns chan<- models.Notification, payload models.BackupStateNotification) { sendNotification(ns, models.NotificationBackupState, payload) } + +// UpdateState reports how far an update being applied has got. +// +//nolint:gocritic // notification payload is copied before async send +func UpdateState(ns chan<- models.Notification, payload models.UpdateStateNotification) { + sendNotification(ns, models.NotificationUpdateState, payload) +} diff --git a/pkg/api/notifications/notifications_test.go b/pkg/api/notifications/notifications_test.go index 063fbe677..770e7fd46 100644 --- a/pkg/api/notifications/notifications_test.go +++ b/pkg/api/notifications/notifications_test.go @@ -358,3 +358,32 @@ func TestUIChanged_Payload(t *testing.T) { require.Len(t, received.Events, 1) assert.Equal(t, "event-1", received.Events[0].ID) } + +func TestUpdateState_Payload(t *testing.T) { + t.Parallel() + + ns := make(chan models.Notification, 1) + + UpdateState(ns, models.UpdateStateNotification{ + Stage: "downloading", + Version: "2.10.0", + Trigger: "manual", + BytesDownloaded: 1024, + BytesTotal: 8192, + }) + + notification := <-ns + assert.Equal(t, models.NotificationUpdateState, notification.Method) + require.NotNil(t, notification.Params) + + var payload models.UpdateStateNotification + require.NoError(t, json.Unmarshal(notification.Params, &payload)) + assert.Equal(t, "downloading", payload.Stage) + assert.Equal(t, "2.10.0", payload.Version) + assert.Equal(t, "manual", payload.Trigger) + assert.Equal(t, int64(1024), payload.BytesDownloaded) + assert.Equal(t, int64(8192), payload.BytesTotal) + // An update that is going fine must not carry an empty error field a + // client could read as a failure. + assert.NotContains(t, string(notification.Params), `"error"`) +} diff --git a/pkg/api/pairing_test.go b/pkg/api/pairing_test.go index 248b6203b..a3b08eedd 100644 --- a/pkg/api/pairing_test.go +++ b/pkg/api/pairing_test.go @@ -657,7 +657,7 @@ func TestHTTPHandlers_FullFlow(t *testing.T) { // // Not t.Parallel — mutates the global zerolog logger to capture output. func TestHandlePairFinish_AuditLogsHMACMismatch(t *testing.T) { - var buf bytes.Buffer + var buf logCapture originalLogger := log.Logger log.Logger = zerolog.New(&buf).Level(zerolog.WarnLevel) t.Cleanup(func() { log.Logger = originalLogger }) @@ -735,7 +735,7 @@ func TestHandlePairFinish_AuditLogsHMACMismatch(t *testing.T) { // // Not t.Parallel — mutates the global zerolog logger. func TestHandlePairFinish_AuditLogsExhaustion(t *testing.T) { - var buf bytes.Buffer + var buf logCapture originalLogger := log.Logger log.Logger = zerolog.New(&buf).Level(zerolog.WarnLevel) t.Cleanup(func() { log.Logger = originalLogger }) diff --git a/pkg/api/permissions/permissions.go b/pkg/api/permissions/permissions.go index c419639e4..fa1edb2e2 100644 --- a/pkg/api/permissions/permissions.go +++ b/pkg/api/permissions/permissions.go @@ -40,6 +40,10 @@ // permission system and restricting it would break unpaired clients. // Setting service.encryption = true requires every remote client to be // paired, which is what makes member restrictions enforceable. +// +// Capabilities listed in authenticatedCapabilities are the exception: +// they need a request from the device itself or from a paired client, +// whatever role the request resolves to. package permissions import "slices" @@ -74,8 +78,26 @@ const ( // CapSettingsWrite covers device settings changes, which include // disabling playtime limits and the require-profile launch gate. CapSettingsWrite Capability = "settings.write" + // CapUpdateApply covers replacing the running binary and restarting + // the service. It is the one capability that is not about weakening + // someone's limits: an update decides what code the device runs from + // then on, and it stops whatever is playing to do it. Checking for an + // update needs no capability. It is also in + // authenticatedCapabilities, so an unpaired remote client cannot use + // it even though such a request resolves to admin. + CapUpdateApply Capability = "update.apply" ) +// authenticatedCapabilities lists the capabilities that need a request from +// the device itself or from a paired client. An unpaired remote request +// resolves to admin, and replacing the running binary is too much to hand to +// a client on the network that has not paired. +// +//nolint:gochecknoglobals // immutable capability table +var authenticatedCapabilities = map[Capability]bool{ + CapUpdateApply: true, +} + // roleCapabilities maps each role to its granted capabilities. // //nolint:gochecknoglobals // immutable capability table @@ -83,6 +105,7 @@ var roleCapabilities = map[Role]map[Capability]bool{ RoleAdmin: { CapProfilesManage: true, CapSettingsWrite: true, + CapUpdateApply: true, }, RoleMember: {}, } @@ -119,8 +142,18 @@ func (g Grant) EffectiveRole() Role { return role } +// Authenticated reports whether the request came from the device itself or +// from a paired client. This is not the same question as EffectiveRole: an +// unpaired remote request resolves to admin but is not authenticated. +func (g Grant) Authenticated() bool { + return g.IsLocal || g.Role != "" +} + // Has reports whether the request may perform the given capability. func (g Grant) Has(capability Capability) bool { + if authenticatedCapabilities[capability] && !g.Authenticated() { + return false + } return roleCapabilities[g.EffectiveRole()][capability] } @@ -130,7 +163,7 @@ func (g Grant) Capabilities() []Capability { roleGrants := roleCapabilities[g.EffectiveRole()] capabilities := make([]Capability, 0, len(roleGrants)) for capability, enabled := range roleGrants { - if enabled { + if enabled && g.Has(capability) { capabilities = append(capabilities, capability) } } diff --git a/pkg/api/permissions/permissions_test.go b/pkg/api/permissions/permissions_test.go index 911740e42..29c3f8987 100644 --- a/pkg/api/permissions/permissions_test.go +++ b/pkg/api/permissions/permissions_test.go @@ -23,6 +23,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGrant_EffectiveRole(t *testing.T) { @@ -66,8 +67,40 @@ func TestGrant_Has(t *testing.T) { assert.True(t, admin.Has(CapProfilesManage)) assert.True(t, admin.Has(CapSettingsWrite)) + assert.True(t, admin.Has(CapUpdateApply)) assert.False(t, member.Has(CapProfilesManage)) assert.False(t, member.Has(CapSettingsWrite)) + assert.False(t, member.Has(CapUpdateApply)) +} + +// An unpaired remote request resolves to admin, so without +// authenticatedCapabilities it would be able to replace the binary. Its other +// capabilities are unaffected. +func TestGrant_UnpairedRemoteCannotApplyUpdates(t *testing.T) { + t.Parallel() + + unpaired := Grant{} + require.Equal(t, RoleAdmin, unpaired.EffectiveRole()) + assert.False(t, unpaired.Authenticated()) + assert.False(t, unpaired.Has(CapUpdateApply)) + assert.True(t, unpaired.Has(CapProfilesManage)) + assert.True(t, unpaired.Has(CapSettingsWrite)) + + for _, grant := range []Grant{ + {IsLocal: true}, + {IsLocal: true, Role: RoleMember}, + {Role: RoleAdmin}, + } { + assert.True(t, grant.Authenticated()) + } + + // Being on the device and being paired are each enough on their own. + assert.True(t, Grant{IsLocal: true}.Has(CapUpdateApply)) + assert.True(t, Grant{Role: RoleAdmin}.Has(CapUpdateApply)) + // The role still has to allow it: a paired member is refused, and a + // voluntary session downgrade still wins. + assert.False(t, Grant{Role: RoleMember}.Has(CapUpdateApply)) + assert.False(t, Grant{IsLocal: true, SessionRole: RoleMember}.Has(CapUpdateApply)) } func TestGrant_Capabilities(t *testing.T) { @@ -81,7 +114,7 @@ func TestGrant_Capabilities(t *testing.T) { { name: "paired admin is sorted", grant: Grant{Role: RoleAdmin}, - want: []Capability{CapProfilesManage, CapSettingsWrite}, + want: []Capability{CapProfilesManage, CapSettingsWrite, CapUpdateApply}, }, { name: "paired member is empty", @@ -89,14 +122,14 @@ func TestGrant_Capabilities(t *testing.T) { want: []Capability{}, }, { - name: "unpaired remote keeps legacy capabilities", + name: "unpaired remote has no update.apply", grant: Grant{}, want: []Capability{CapProfilesManage, CapSettingsWrite}, }, { name: "local member gets local capabilities", grant: Grant{Role: RoleMember, IsLocal: true}, - want: []Capability{CapProfilesManage, CapSettingsWrite}, + want: []Capability{CapProfilesManage, CapSettingsWrite, CapUpdateApply}, }, { name: "unknown role degrades to member", diff --git a/pkg/api/request_priority.go b/pkg/api/request_priority.go index 9af7e606b..e10a36f55 100644 --- a/pkg/api/request_priority.go +++ b/pkg/api/request_priority.go @@ -107,7 +107,8 @@ func classifyAPIMethod(method string) apiRequestPriority { models.MethodMediaScrapeCancel, models.MethodMediaScrapeResume, models.MethodMediaCleanOrphans, - models.MethodSettingsLogsDownload: + models.MethodSettingsLogsDownload, + models.MethodUpdateApply: return apiPriorityLow default: if strings.HasPrefix(method, "media.scrape") || strings.HasPrefix(method, "media.generate") { diff --git a/pkg/api/request_priority_test.go b/pkg/api/request_priority_test.go index 8eb85ad03..ec3d2fc4a 100644 --- a/pkg/api/request_priority_test.go +++ b/pkg/api/request_priority_test.go @@ -42,6 +42,7 @@ func TestRequestTimeoutForAPIMethod(t *testing.T) { {"backup restore", models.MethodSettingsBackupRestore, 0}, {"remote backup", models.MethodSettingsBackupRemoteRun, 0}, {"remote restore", models.MethodSettingsBackupRemoteRestore, 0}, + {"update apply", models.MethodUpdateApply, 0}, {"case insensitive", "SETTINGS.BACKUP", 0}, {"backup list", models.MethodSettingsBackupList, config.APIRequestTimeout}, {"unknown", "custom.method", config.APIRequestTimeout}, @@ -84,6 +85,8 @@ func TestClassifyAPIMethod(t *testing.T) { {"input gamepad", models.MethodInputGamepad, apiPriorityInput}, {"low media generate", models.MethodMediaGenerate, apiPriorityLow}, {"low media image", models.MethodMediaImage, apiPriorityLow}, + {"low update apply", models.MethodUpdateApply, apiPriorityLow}, + {"update check stays normal", models.MethodUpdateCheck, apiPriorityNormal}, {"low scrape prefix", "media.scrape.queue", apiPriorityLow}, {"low generate prefix", "media.generate.extra", apiPriorityLow}, {"unknown normal", "custom.method", apiPriorityNormal}, diff --git a/pkg/api/server_logging_test.go b/pkg/api/server_logging_test.go index ca2bb55db..fb372cb1d 100644 --- a/pkg/api/server_logging_test.go +++ b/pkg/api/server_logging_test.go @@ -22,10 +22,12 @@ package api import ( "bytes" "errors" + "fmt" "strings" "testing" "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/syncutil" "github.com/google/uuid" "github.com/olahol/melody" "github.com/rs/zerolog" @@ -42,6 +44,30 @@ const ( authLinkVerificationURL = "https://online.zaparoo.com/link" ) +// logCapture collects log output for assertions. Tests that capture the global +// logger run alongside parallel tests in this package that are still logging +// into it, so the buffer has to be safe for concurrent writes. +type logCapture struct { + buf bytes.Buffer + mu syncutil.Mutex +} + +func (c *logCapture) Write(p []byte) (int, error) { + c.mu.Lock() + defer c.mu.Unlock() + n, err := c.buf.Write(p) + if err != nil { + return n, fmt.Errorf("write log capture: %w", err) + } + return n, nil +} + +func (c *logCapture) String() string { + c.mu.Lock() + defer c.mu.Unlock() + return c.buf.String() +} + func TestLogSafeResponse(t *testing.T) { tests := []struct { result any @@ -78,7 +104,7 @@ func TestLogSafeResponse(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // Capture log output - var buf bytes.Buffer + var buf logCapture originalLogger := log.Logger log.Logger = zerolog.New(&buf).Level(zerolog.DebugLevel) @@ -145,7 +171,7 @@ func TestLogSafeResponse_BatchRedaction(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var buf bytes.Buffer + var buf logCapture originalLogger := log.Logger log.Logger = zerolog.New(&buf).Level(zerolog.DebugLevel) defer func() { log.Logger = originalLogger }() @@ -170,7 +196,7 @@ func TestLogSafeResponse_DefaultOmitsBody(t *testing.T) { Blob string `json:"blob"` } - var buf bytes.Buffer + var buf logCapture originalLogger := log.Logger log.Logger = zerolog.New(&buf).Level(zerolog.DebugLevel) defer func() { log.Logger = originalLogger }() @@ -252,7 +278,7 @@ func TestHandleResponse(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var buf bytes.Buffer + var buf logCapture originalLogger := log.Logger log.Logger = zerolog.New(&buf).Level(zerolog.DebugLevel) defer func() { log.Logger = originalLogger }() @@ -303,7 +329,7 @@ func TestLogSafeRequest(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // Capture log output - var buf bytes.Buffer + var buf logCapture originalLogger := log.Logger log.Logger = zerolog.New(&buf).Level(zerolog.DebugLevel) @@ -324,7 +350,7 @@ func TestLogSafeRequest(t *testing.T) { } func TestLogSafeRequest_AuthClaimParamsRedacted(t *testing.T) { - var buf bytes.Buffer + var buf logCapture originalLogger := log.Logger log.Logger = zerolog.New(&buf).Level(zerolog.DebugLevel) defer func() { log.Logger = originalLogger }() @@ -344,7 +370,7 @@ func TestLogSafeRequest_AuthClaimParamsRedacted(t *testing.T) { } func TestLogSafeResponse_AuthLinkCodesRedacted(t *testing.T) { - var buf bytes.Buffer + var buf logCapture originalLogger := log.Logger log.Logger = zerolog.New(&buf).Level(zerolog.DebugLevel) defer func() { log.Logger = originalLogger }() @@ -393,7 +419,7 @@ func TestLogWSWriteError(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var buf bytes.Buffer + var buf logCapture originalLogger := log.Logger log.Logger = zerolog.New(&buf) diff --git a/pkg/api/transport_timing_test.go b/pkg/api/transport_timing_test.go index b3b3ca74e..c0e19256d 100644 --- a/pkg/api/transport_timing_test.go +++ b/pkg/api/transport_timing_test.go @@ -20,7 +20,6 @@ package api import ( - "bytes" "context" "encoding/json" "errors" @@ -71,7 +70,7 @@ func TestLogWebSocketTransportTimingFields(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var buf bytes.Buffer + var buf logCapture originalLogger := log.Logger log.Logger = zerolog.New(&buf).Level(zerolog.DebugLevel) defer func() { log.Logger = originalLogger }() @@ -110,7 +109,7 @@ func TestHTTPResponseTransportTimingFields(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { handler, _, _ := createTestPostHandler(t) - var buf bytes.Buffer + var buf logCapture originalLogger := log.Logger log.Logger = zerolog.New(&buf).Level(zerolog.DebugLevel) defer func() { log.Logger = originalLogger }() @@ -172,7 +171,7 @@ func TestWebSocketDispatcherQueueMetadata(t *testing.T) { } func TestWebSocketDispatcherQueueTimingLogs(t *testing.T) { - var buf bytes.Buffer + var buf logCapture originalLogger := log.Logger log.Logger = zerolog.New(&buf).Level(zerolog.DebugLevel) defer func() { log.Logger = originalLogger }() diff --git a/pkg/config/config.go b/pkg/config/config.go index 70a30f9e9..12a98932a 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -92,8 +92,7 @@ func PreserveRestoreOverrides(data []byte, deviceID string, encryption bool) ([] type Values struct { Groovy Groovy `toml:"groovy,omitempty"` Input Input `toml:"input,omitempty"` - AutoUpdate *bool `toml:"auto_update,omitempty"` - UpdateChannel *string `toml:"update_channel,omitempty"` + Updates Updates `toml:"updates,omitempty"` Audio Audio `toml:"audio"` Backup Backup `toml:"backup,omitempty"` Service Service `toml:"service,omitempty"` @@ -111,6 +110,14 @@ type Values struct { ErrorReporting bool `toml:"error_reporting"` } +// Updates controls how the device handles new releases. Every field is a +// pointer so an unset key keeps its default rather than reading as false. +type Updates struct { + Channel *string `toml:"channel,omitempty"` + Check *bool `toml:"check,omitempty"` + Install *bool `toml:"install,omitempty"` +} + type Audio struct { SuccessSound *string `toml:"success_sound,omitempty"` FailSound *string `toml:"fail_sound,omitempty"` @@ -173,6 +180,7 @@ type Instance struct { mappingsExternal []MappingsEntry vals Values defaults Values + updateMu syncutil.Mutex mu syncutil.RWMutex } @@ -186,6 +194,12 @@ func (c *Instance) getFs() afero.Fs { return afero.NewOsFs() } +// AcquireUpdateLock serializes one config load-modify-save transaction. +func (c *Instance) AcquireUpdateLock() func() { + c.updateMu.Lock() + return c.updateMu.Unlock +} + var ( authCfg atomic.Value apiKeys atomic.Value @@ -234,6 +248,7 @@ func NewConfigWithFs(configDir string, defaults Values, fs afero.Fs) (*Instance, cfg := Instance{ fs: fs, + updateMu: syncutil.Mutex{}, mu: syncutil.RWMutex{}, appPath: os.Getenv(AppEnv), cfgPath: cfgPath, @@ -971,24 +986,42 @@ func (c *Instance) SetErrorReporting(enabled bool) { c.vals.ErrorReporting = enabled } -// AutoUpdate returns whether automatic update checking is enabled. -// The defaultEnabled parameter allows platforms to specify their own default -// (e.g. package-managed installs default to false). -// An explicit user setting always takes precedence. -func (c *Instance) AutoUpdate(defaultEnabled bool) bool { +// UpdateCheck returns whether the device looks for new releases. +// +// It is on unless it has been turned off, on every platform. A check reads a +// signed metadata file and sends nothing that identifies the device, so there +// is no reason for a package-managed install to skip it: knowing a newer +// release exists is useful even when the package manager is the thing that +// installs it. +func (c *Instance) UpdateCheck() bool { c.mu.RLock() defer c.mu.RUnlock() - if c.vals.AutoUpdate == nil { - return defaultEnabled - } - return *c.vals.AutoUpdate + return c.vals.Updates.Check == nil || *c.vals.Updates.Check +} + +// SetUpdateCheck sets whether the device looks for new releases. +func (c *Instance) SetUpdateCheck(enabled bool) { + c.mu.Lock() + defer c.mu.Unlock() + c.vals.Updates.Check = &enabled +} + +// UpdateInstall returns whether the device may download and install updates on +// its own. It is off unless it has been turned on, and it is off whenever +// checking is off: a device that is not allowed to look for updates cannot be +// installing them. +func (c *Instance) UpdateInstall() bool { + c.mu.RLock() + defer c.mu.RUnlock() + checking := c.vals.Updates.Check == nil || *c.vals.Updates.Check + return checking && c.vals.Updates.Install != nil && *c.vals.Updates.Install } -// SetAutoUpdate sets whether automatic update checking is enabled. -func (c *Instance) SetAutoUpdate(enabled bool) { +// SetUpdateInstall sets whether the device may install updates on its own. +func (c *Instance) SetUpdateInstall(enabled bool) { c.mu.Lock() defer c.mu.Unlock() - c.vals.AutoUpdate = &enabled + c.vals.Updates.Install = &enabled } // UpdateChannel returns the configured update channel. @@ -996,15 +1029,15 @@ func (c *Instance) SetAutoUpdate(enabled bool) { func (c *Instance) UpdateChannel() string { c.mu.RLock() defer c.mu.RUnlock() - if c.vals.UpdateChannel == nil { + if c.vals.Updates.Channel == nil { return UpdateChannelStable } - return *c.vals.UpdateChannel + return *c.vals.Updates.Channel } // SetUpdateChannel sets the update channel. Valid values are "stable" and "beta". func (c *Instance) SetUpdateChannel(channel string) { c.mu.Lock() defer c.mu.Unlock() - c.vals.UpdateChannel = &channel + c.vals.Updates.Channel = &channel } diff --git a/pkg/config/config_autoupdate_test.go b/pkg/config/config_autoupdate_test.go deleted file mode 100644 index 5acfddac5..000000000 --- a/pkg/config/config_autoupdate_test.go +++ /dev/null @@ -1,121 +0,0 @@ -// Zaparoo Core -// Copyright (c) 2026 The Zaparoo Project Contributors. -// SPDX-License-Identifier: GPL-3.0-or-later -// -// This file is part of Zaparoo Core. -// -// Zaparoo Core is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// Zaparoo Core is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with Zaparoo Core. If not, see . - -package config - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestAutoUpdate(t *testing.T) { - t.Parallel() - - trueVal := true - falseVal := false - - tests := []struct { - autoUpdate *bool - name string - defaultEnabled bool - expected bool - }{ - { - name: "nil default enabled returns true", - autoUpdate: nil, - defaultEnabled: true, - expected: true, - }, - { - name: "nil default disabled returns false", - autoUpdate: nil, - defaultEnabled: false, - expected: false, - }, - { - name: "explicit true overrides default disabled", - autoUpdate: &trueVal, - defaultEnabled: false, - expected: true, - }, - { - name: "explicit false overrides default enabled", - autoUpdate: &falseVal, - defaultEnabled: true, - expected: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - cfg := &Instance{ - vals: Values{ - AutoUpdate: tt.autoUpdate, - }, - } - - result := cfg.AutoUpdate(tt.defaultEnabled) - assert.Equal(t, tt.expected, result) - }) - } -} - -func TestSetAutoUpdate(t *testing.T) { - t.Parallel() - - cfg := &Instance{ - vals: Values{}, - } - - assert.Nil(t, cfg.vals.AutoUpdate) - - cfg.SetAutoUpdate(false) - assert.NotNil(t, cfg.vals.AutoUpdate) - assert.False(t, cfg.AutoUpdate(true)) - - cfg.SetAutoUpdate(true) - assert.True(t, cfg.AutoUpdate(true)) -} - -func TestIsDevelopmentVersion(t *testing.T) { - tests := []struct { - name string - version string - expected bool - }{ - {"literal DEVELOPMENT", "DEVELOPMENT", true}, - {"hash-dev suffix", "abc1234-dev", true}, - {"release version", "2.9.1", false}, - {"prerelease version", "2.10.0-rc1", false}, - {"nightly version", "2.10.0-nightly.20260228", false}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - original := AppVersion - AppVersion = tt.version - t.Cleanup(func() { AppVersion = original }) - - assert.Equal(t, tt.expected, IsDevelopmentVersion()) - }) - } -} diff --git a/pkg/config/config_updatechannel_test.go b/pkg/config/config_updatechannel_test.go index be79f991c..0cf72ef33 100644 --- a/pkg/config/config_updatechannel_test.go +++ b/pkg/config/config_updatechannel_test.go @@ -59,7 +59,7 @@ func TestUpdateChannel(t *testing.T) { cfg := &Instance{ vals: Values{ - UpdateChannel: tt.channel, + Updates: Updates{Channel: tt.channel}, }, } @@ -76,10 +76,10 @@ func TestSetUpdateChannel(t *testing.T) { vals: Values{}, } - assert.Nil(t, cfg.vals.UpdateChannel) + assert.Nil(t, cfg.vals.Updates.Channel) cfg.SetUpdateChannel("beta") - assert.NotNil(t, cfg.vals.UpdateChannel) + assert.NotNil(t, cfg.vals.Updates.Channel) assert.Equal(t, "beta", cfg.UpdateChannel()) cfg.SetUpdateChannel("stable") diff --git a/pkg/config/config_updates_test.go b/pkg/config/config_updates_test.go new file mode 100644 index 000000000..102b2e152 --- /dev/null +++ b/pkg/config/config_updates_test.go @@ -0,0 +1,150 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package config + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestUpdateCheck(t *testing.T) { + t.Parallel() + + trueVal := true + falseVal := false + + tests := []struct { + check *bool + name string + expected bool + }{ + {name: "unset checks", check: nil, expected: true}, + {name: "explicit true checks", check: &trueVal, expected: true}, + {name: "explicit false does not check", check: &falseVal, expected: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + cfg := &Instance{vals: Values{Updates: Updates{Check: tt.check}}} + assert.Equal(t, tt.expected, cfg.UpdateCheck()) + }) + } +} + +func TestSetUpdateCheck(t *testing.T) { + t.Parallel() + + cfg := &Instance{vals: Values{}} + assert.Nil(t, cfg.vals.Updates.Check) + + cfg.SetUpdateCheck(false) + assert.NotNil(t, cfg.vals.Updates.Check) + assert.False(t, cfg.UpdateCheck()) + + cfg.SetUpdateCheck(true) + assert.True(t, cfg.UpdateCheck()) +} + +func TestUpdateInstall(t *testing.T) { + t.Parallel() + + trueVal := true + falseVal := false + + tests := []struct { + check *bool + install *bool + name string + expected bool + }{ + {name: "unset does not install", check: nil, install: nil, expected: false}, + {name: "turned on installs", check: nil, install: &trueVal, expected: true}, + {name: "turned off does not install", check: nil, install: &falseVal, expected: false}, + { + // Installing without checking is not a state the device can be in. + name: "checking off wins", check: &falseVal, install: &trueVal, expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + cfg := &Instance{vals: Values{Updates: Updates{Check: tt.check, Install: tt.install}}} + assert.Equal(t, tt.expected, cfg.UpdateInstall()) + }) + } +} + +func TestSetUpdateInstall(t *testing.T) { + t.Parallel() + + cfg := &Instance{vals: Values{}} + assert.Nil(t, cfg.vals.Updates.Install) + + cfg.SetUpdateInstall(true) + assert.True(t, cfg.UpdateInstall()) + + cfg.SetUpdateInstall(false) + assert.False(t, cfg.UpdateInstall()) +} + +// A config written by an older release still has the keys that were replaced. +// They are ignored, and the defaults they used to carry are what the device +// falls back to. +func TestUpdates_LegacyKeysAreIgnored(t *testing.T) { + t.Parallel() + + cfg := &Instance{vals: Values{}} + legacy := "auto_update = false\nauto_update_install = true\nupdate_channel = 'beta'\n" + require.NoError(t, cfg.applyTOML(legacy)) + + assert.True(t, cfg.UpdateCheck()) + assert.False(t, cfg.UpdateInstall()) + assert.Equal(t, UpdateChannelStable, cfg.UpdateChannel()) +} + +func TestIsDevelopmentVersion(t *testing.T) { + tests := []struct { + name string + version string + expected bool + }{ + {"literal DEVELOPMENT", "DEVELOPMENT", true}, + {"hash-dev suffix", "abc1234-dev", true}, + {"release version", "2.9.1", false}, + {"prerelease version", "2.10.0-rc1", false}, + {"nightly version", "2.10.0-nightly.20260228", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + original := AppVersion + AppVersion = tt.version + t.Cleanup(func() { AppVersion = original }) + + assert.Equal(t, tt.expected, IsDevelopmentVersion()) + }) + } +} diff --git a/pkg/helpers/power/pmset.go b/pkg/helpers/power/pmset.go new file mode 100644 index 000000000..aaa45386d --- /dev/null +++ b/pkg/helpers/power/pmset.go @@ -0,0 +1,121 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package power + +import ( + "regexp" + "strconv" + "strings" +) + +// pmsetPercentRe matches the complete charge field pmset prints on a battery +// line, as in "-InternalBattery-0 (id=4653155)\t62%; discharging; 3:32 remaining present: true". +var pmsetPercentRe = regexp.MustCompile(`(^|[ \t])(\d{1,3})%($|[; \t])`) + +// parsePmsetBatt resolves the four states the updater distinguishes from the +// output of `pmset -g batt`. +// +// It lives outside the darwin build tag so it can be tested on the machines +// that actually run the test suite. Only the command that produces its input +// is macOS-specific. +// +// A Mac with no battery line is desktop hardware such as a Mac mini, which is +// the common case and always safe to install on. A battery whose charge cannot +// be read is reported as unknown rather than assumed full, because the cost of +// being wrong is a laptop that dies mid-install. +func parsePmsetBatt(output string) Status { + var ( + sawSource bool + external bool + unreadable bool + batteries int + lowest = -1 + ) + + for line := range strings.SplitSeq(output, "\n") { + line = strings.TrimSpace(line) + switch { + case line == "": + continue + case strings.HasPrefix(line, "Now drawing from"): + // pmset names the supply in quotes: 'AC Power' or 'Battery Power'. + sawSource = true + if strings.Contains(line, "'AC Power'") { + external = true + } + case strings.Contains(line, "present: true"): + // A battery pmset knows about but reports as absent belongs to a + // removed or unseated pack, and must not count as a supply the + // device is running on. + batteries++ + // A battery charging or already charged is on external power even + // when the source line says otherwise, which is what a Mac reports + // in the moment a charger is plugged in. + if strings.Contains(line, "; charging") || strings.Contains(line, "; charged") { + external = true + } + percent, ok := parsePmsetPercent(line) + if !ok { + unreadable = true + continue + } + if lowest < 0 || percent < lowest { + lowest = percent + } + } + } + + if !sawSource && batteries == 0 { + // Not pmset output at all. Saying "no battery" here would hand the + // updater a green light it has no reading to support. + return Status{Source: SourceUnknown} + } + if batteries == 0 { + return Status{Source: SourceNoBattery} + } + if external { + return Status{Source: SourceExternal} + } + if unreadable { + return Status{Source: SourceUnknown} + } + if lowest < 0 { + return Status{Source: SourceUnknown} + } + return Status{Source: SourceBattery, Percent: lowest} +} + +// parsePmsetPercent reads the charge from one pmset battery line. +func parsePmsetPercent(line string) (int, bool) { + match := pmsetPercentRe.FindStringSubmatch(line) + if match == nil { + return 0, false + } + percent, err := strconv.Atoi(match[2]) + if err != nil { + return 0, false + } + // pmset is supposed to report 0-100, and junk must not read as a full + // battery. + if percent < 0 || percent > 100 { + return 0, false + } + return percent, true +} diff --git a/pkg/helpers/power/pmset_test.go b/pkg/helpers/power/pmset_test.go new file mode 100644 index 000000000..96001054e --- /dev/null +++ b/pkg/helpers/power/pmset_test.go @@ -0,0 +1,131 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package power + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// The samples below are real `pmset -g batt` output shapes. The updater +// refuses an install it cannot prove is safe, so the case that matters most is +// a desktop Mac reading as mains-powered rather than as an unreadable battery. +func TestParsePmsetBatt(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + output string + want Status + }{ + { + name: "desktop Mac has no battery line", + output: "Now drawing from 'AC Power'\n", + want: Status{Source: SourceNoBattery}, + }, + { + name: "laptop on a charger is external", + output: "Now drawing from 'AC Power'\n" + + " -InternalBattery-0 (id=4653155)\t45%; charging; 1:12 remaining present: true\n", + want: Status{Source: SourceExternal}, + }, + { + name: "laptop charged and plugged in is external", + output: "Now drawing from 'AC Power'\n" + + " -InternalBattery-0 (id=4653155)\t100%; charged; 0:00 remaining present: true\n", + want: Status{Source: SourceExternal}, + }, + { + name: "laptop discharging reports its charge", + output: "Now drawing from 'Battery Power'\n" + + " -InternalBattery-0 (id=4653155)\t62%; discharging; 3:32 remaining present: true\n", + want: Status{Source: SourceBattery, Percent: 62}, + }, + { + name: "a charging battery is external even when the source line disagrees", + output: "Now drawing from 'Battery Power'\n" + + " -InternalBattery-0 (id=4653155)\t45%; charging; 1:12 remaining present: true\n", + want: Status{Source: SourceExternal}, + }, + { + name: "an absent battery does not count as a supply", + output: "Now drawing from 'AC Power'\n" + + " -InternalBattery-0 (id=4653155)\t0%; charged; 0:00 remaining present: false\n", + want: Status{Source: SourceNoBattery}, + }, + { + name: "the lowest of several batteries decides", + output: "Now drawing from 'Battery Power'\n" + + " -InternalBattery-0 (id=4653155)\t62%; discharging; 3:32 remaining present: true\n" + + " -InternalBattery-1 (id=4653156)\t18%; discharging; 0:51 remaining present: true\n", + want: Status{Source: SourceBattery, Percent: 18}, + }, + { + name: "a present battery with no readable charge is unknown", + output: "Now drawing from 'Battery Power'\n" + + " -InternalBattery-0 (id=4653155)\t(no estimate); discharging; present: true\n", + want: Status{Source: SourceUnknown}, + }, + { + name: "one unreadable present battery makes the combined reading unknown", + output: "Now drawing from 'Battery Power'\n" + + " -InternalBattery-0 (id=4653155)\t62%; discharging; 3:32 remaining present: true\n" + + " -InternalBattery-1 (id=4653156)\t(no estimate); discharging; present: true\n", + want: Status{Source: SourceUnknown}, + }, + { + name: "empty output is unknown, not a green light", + output: "", + want: Status{Source: SourceUnknown}, + }, + { + name: "output that is not pmset at all is unknown", + output: "command not found\n", + want: Status{Source: SourceUnknown}, + }, + { + name: "a nonsense charge does not read as a full battery", + output: "Now drawing from 'Battery Power'\n" + + " -InternalBattery-0 (id=4653155)\t999%; discharging; 3:32 remaining present: true\n", + want: Status{Source: SourceUnknown}, + }, + { + name: "a signed charge is not partially parsed", + output: "Now drawing from 'Battery Power'\n" + + " -InternalBattery-0 (id=4653155)\t-1%; discharging; 3:32 remaining present: true\n", + want: Status{Source: SourceUnknown}, + }, + { + name: "a four-digit charge is not partially parsed", + output: "Now drawing from 'Battery Power'\n" + + " -InternalBattery-0 (id=4653155)\t1000%; discharging; 3:32 remaining present: true\n", + want: Status{Source: SourceUnknown}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + assert.Equal(t, tt.want, parsePmsetBatt(tt.output)) + }) + } +} diff --git a/pkg/helpers/power/power.go b/pkg/helpers/power/power.go new file mode 100644 index 000000000..6be8174f7 --- /dev/null +++ b/pkg/helpers/power/power.go @@ -0,0 +1,50 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +// Package power reports whether the device is running on mains power or a +// battery, and how much of that battery is left. The updater uses it to +// refuse an install that could lose power part-way through. +package power + +// Source is where the device is drawing power from. +type Source string + +const ( + // SourceNoBattery means the hardware has no battery at all, so it is + // running on whatever mains supply it always runs on. + SourceNoBattery Source = "noBattery" + // SourceExternal means a charger or dock is supplying power. The battery + // percentage does not matter while this is true. + SourceExternal Source = "external" + // SourceBattery means the device is discharging and Percent is its + // remaining charge. + SourceBattery Source = "battery" + // SourceUnknown means the hardware may have a battery but its state could + // not be read. Callers must treat this as "could lose power at any + // moment", not as "probably fine". + SourceUnknown Source = "unknown" +) + +// Status is a snapshot of where the device's power is coming from. +type Status struct { + Source Source + // Percent is the remaining charge, 0-100. It is only meaningful when + // Source is SourceBattery. + Percent int +} diff --git a/pkg/helpers/power/power_darwin.go b/pkg/helpers/power/power_darwin.go new file mode 100644 index 000000000..d98a51034 --- /dev/null +++ b/pkg/helpers/power/power_darwin.go @@ -0,0 +1,58 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +//go:build darwin + +package power + +import ( + "context" + "fmt" + "path/filepath" + "time" + + "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/command" +) + +// pmsetPath is the power management tool macOS ships. It is addressed by +// absolute path so the reading does not depend on the PATH the service +// inherited. +var pmsetPath = filepath.Join(string(filepath.Separator), "usr", "bin", "pmset") + +// pmsetTimeout bounds the reading. pmset answers immediately in normal +// operation, and the caller is the update gate, on the path of an install the +// user is waiting on, so a wedged call must not stall it. +const pmsetTimeout = 2 * time.Second + +// Read reports the device's power state from pmset, which is how macOS +// exposes the battery without linking IOKit through cgo. +func Read() (Status, error) { + ctx, cancel := context.WithTimeout(context.Background(), pmsetTimeout) + defer cancel() + + return readDarwin(ctx, &command.RealExecutor{}) +} + +func readDarwin(ctx context.Context, executor command.Executor) (Status, error) { + output, err := executor.Output(ctx, pmsetPath, "-g", "batt") + if err != nil { + return Status{Source: SourceUnknown}, fmt.Errorf("reading power status from pmset: %w", err) + } + return parsePmsetBatt(string(output)), nil +} diff --git a/pkg/helpers/power/power_darwin_export_test.go b/pkg/helpers/power/power_darwin_export_test.go new file mode 100644 index 000000000..9156fdba9 --- /dev/null +++ b/pkg/helpers/power/power_darwin_export_test.go @@ -0,0 +1,36 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +//go:build darwin + +package power + +import ( + "context" + + "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/command" +) + +func ReadDarwinWithExecutorForTest(ctx context.Context, executor command.Executor) (Status, error) { + return readDarwin(ctx, executor) +} + +func PmsetPathForTest() string { + return pmsetPath +} diff --git a/pkg/helpers/power/power_darwin_test.go b/pkg/helpers/power/power_darwin_test.go new file mode 100644 index 000000000..9c3eb1ca9 --- /dev/null +++ b/pkg/helpers/power/power_darwin_test.go @@ -0,0 +1,88 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +//go:build darwin + +package power_test + +import ( + "context" + "errors" + "testing" + + "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/power" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/testing/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestReadDarwin_Success(t *testing.T) { + t.Parallel() + + executor := &mocks.MockCommandExecutor{} + executor.On("Output", mock.Anything, power.PmsetPathForTest(), []string{"-g", "batt"}).Return([]byte( + "Now drawing from 'Battery Power'\n"+ + " -InternalBattery-0 (id=4653155)\t62%; discharging; 3:32 remaining present: true\n", + ), nil).Once() + + status, err := power.ReadDarwinWithExecutorForTest(t.Context(), executor) + + require.NoError(t, err) + assert.Equal(t, power.Status{Source: power.SourceBattery, Percent: 62}, status) + executor.AssertExpectations(t) +} + +func TestReadDarwin_CommandFailure(t *testing.T) { + t.Parallel() + + commandErr := errors.New("pmset failed") + executor := &mocks.MockCommandExecutor{} + executor.On( + "Output", mock.Anything, power.PmsetPathForTest(), []string{"-g", "batt"}, + ).Return(nil, commandErr).Once() + + status, err := power.ReadDarwinWithExecutorForTest(t.Context(), executor) + + require.ErrorIs(t, err, commandErr) + assert.Equal(t, power.Status{Source: power.SourceUnknown}, status) + executor.AssertExpectations(t) +} + +func TestReadDarwin_ContextCanceled(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithCancel(t.Context()) + cancel() + executor := &mocks.MockCommandExecutor{} + executor.On( + "Output", + mock.MatchedBy(func(callCtx context.Context) bool { + return errors.Is(callCtx.Err(), context.Canceled) + }), + power.PmsetPathForTest(), + []string{"-g", "batt"}, + ).Return(nil, context.Canceled).Once() + + status, err := power.ReadDarwinWithExecutorForTest(ctx, executor) + + require.ErrorIs(t, err, context.Canceled) + assert.Equal(t, power.Status{Source: power.SourceUnknown}, status) + executor.AssertExpectations(t) +} diff --git a/pkg/helpers/power/power_linux.go b/pkg/helpers/power/power_linux.go new file mode 100644 index 000000000..229509b26 --- /dev/null +++ b/pkg/helpers/power/power_linux.go @@ -0,0 +1,158 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package power + +import ( + "errors" + "os" + "path/filepath" + "strconv" + "strings" + + "github.com/spf13/afero" +) + +// sysfsRoot is where the kernel exposes every power supply it knows about, +// one directory per supply. +var sysfsRoot = filepath.Join(string(filepath.Separator), "sys", "class", "power_supply") + +// Read reports the device's power state from the kernel's power-supply +// directory. +func Read() (Status, error) { + return statusFrom(afero.NewOsFs(), sysfsRoot) +} + +// statusFrom resolves the four states the updater distinguishes from a +// power-supply tree. +// +// A device with no battery directory is mains-powered hardware such as a +// MiSTer, which is the common case and always safe to install on. Everything +// else needs a real reading: a battery whose charge cannot be read is reported +// as unknown rather than assumed full, because the cost of being wrong is a +// device that loses power mid-install. +func statusFrom(fs afero.Fs, root string) (Status, error) { + entries, err := afero.ReadDir(fs, root) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + // No power-supply class at all. Kernels built without it are on + // hardware that has nothing to report. + return Status{Source: SourceNoBattery}, nil + } + return Status{Source: SourceUnknown}, err //nolint:wrapcheck // caller logs the sysfs error as-is + } + + var ( + batteries []string + externalOn bool + unreadable bool + ) + for _, entry := range entries { + name := entry.Name() + dir := filepath.Join(root, name) + supplyType, ok := readSupplyField(fs, dir, "type") + if !ok || supplyType == "" { + unreadable = true + continue + } + switch supplyType { + case "Battery": + // A wireless mouse or controller is a battery the kernel knows + // about and the device does not run on. The kernel marks those + // "Device"; a battery the whole machine runs on is "System" or + // says nothing at all. + scope, _ := readSupplyField(fs, dir, "scope") + if scope == "Device" { + continue + } + batteries = append(batteries, dir) + case "Mains", "USB", "USB_PD", "USB_PD_DRP", "BrickID", "Wireless": + online, _ := readSupplyField(fs, dir, "online") + if online == "1" { + externalOn = true + } + } + } + + if len(batteries) == 0 { + if unreadable && !externalOn { + return Status{Source: SourceUnknown}, nil + } + return Status{Source: SourceNoBattery}, nil + } + if externalOn { + return Status{Source: SourceExternal}, nil + } + + // A battery reporting Charging or Full is on external power even when no + // mains supply announced itself, which is how some handhelds wire USB-C. + for _, dir := range batteries { + status, _ := readSupplyField(fs, dir, "status") + switch status { + case "Charging", "Full": + return Status{Source: SourceExternal}, nil + } + } + + // Several batteries means several readings. The lowest is the one that + // decides when the device dies. + lowest := -1 + for _, dir := range batteries { + percent, ok := readCapacity(fs, dir) + if !ok { + unreadable = true + continue + } + if lowest < 0 || percent < lowest { + lowest = percent + } + } + if unreadable { + return Status{Source: SourceUnknown}, nil + } + if lowest < 0 { + return Status{Source: SourceUnknown}, nil + } + return Status{Source: SourceBattery, Percent: lowest}, nil +} + +func readSupplyField(fs afero.Fs, dir, name string) (string, bool) { + data, err := afero.ReadFile(fs, filepath.Join(dir, name)) + if err != nil { + return "", false + } + return strings.TrimSpace(string(data)), true +} + +func readCapacity(fs afero.Fs, dir string) (int, bool) { + raw, ok := readSupplyField(fs, dir, "capacity") + if !ok || raw == "" { + return 0, false + } + percent, err := strconv.Atoi(raw) + if err != nil { + return 0, false + } + // The kernel is supposed to report 0-100 but some drivers report junk on + // a battery they cannot talk to, and junk must not read as a full battery. + if percent < 0 || percent > 100 { + return 0, false + } + return percent, true +} diff --git a/pkg/helpers/power/power_linux_test.go b/pkg/helpers/power/power_linux_test.go new file mode 100644 index 000000000..d9fcab01a --- /dev/null +++ b/pkg/helpers/power/power_linux_test.go @@ -0,0 +1,202 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +//go:build linux + +package power + +import ( + "path/filepath" + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// supply is one directory under the kernel's power-supply class. +type supply struct { + fields map[string]string + name string +} + +func writeSupplies(t *testing.T, root string, supplies []supply) afero.Fs { + t.Helper() + fs := afero.NewMemMapFs() + require.NoError(t, fs.MkdirAll(root, 0o755)) + for _, s := range supplies { + dir := filepath.Join(root, s.name) + require.NoError(t, fs.MkdirAll(dir, 0o755)) + for name, value := range s.fields { + require.NoError(t, afero.WriteFile(fs, filepath.Join(dir, name), []byte(value+"\n"), 0o644)) + } + } + return fs +} + +func TestStatusFrom(t *testing.T) { + t.Parallel() + + root := filepath.Join(string(filepath.Separator), "sys", "class", "power_supply") + + tests := []struct { + name string + supplies []supply + want Status + }{ + { + name: "a machine with nothing to report has no battery", + supplies: []supply{}, + want: Status{Source: SourceNoBattery}, + }, + { + name: "mains only is no battery", + supplies: []supply{ + {name: "AC", fields: map[string]string{"type": "Mains", "online": "1"}}, + }, + want: Status{Source: SourceNoBattery}, + }, + { + name: "a wireless mouse is not the machine's battery", + supplies: []supply{ + {name: "hidpp_battery_0", fields: map[string]string{ + "type": "Battery", "scope": "Device", "status": "Discharging", "capacity": "4", + }}, + }, + want: Status{Source: SourceNoBattery}, + }, + { + name: "a plugged-in handheld is on external power", + supplies: []supply{ + {name: "AC", fields: map[string]string{"type": "Mains", "online": "1"}}, + {name: "BAT0", fields: map[string]string{ + "type": "Battery", "status": "Discharging", "capacity": "12", + }}, + }, + want: Status{Source: SourceExternal}, + }, + { + name: "a charging battery is external power even with no mains supply", + supplies: []supply{ + {name: "BAT0", fields: map[string]string{ + "type": "Battery", "status": "Charging", "capacity": "12", + }}, + }, + want: Status{Source: SourceExternal}, + }, + { + name: "a full battery is external power", + supplies: []supply{ + {name: "BAT0", fields: map[string]string{"type": "Battery", "status": "Full"}}, + }, + want: Status{Source: SourceExternal}, + }, + { + name: "an unplugged handheld reports its charge", + supplies: []supply{ + {name: "AC", fields: map[string]string{"type": "Mains", "online": "0"}}, + {name: "BAT0", fields: map[string]string{ + "type": "Battery", "status": "Discharging", "capacity": "63", + }}, + }, + want: Status{Source: SourceBattery, Percent: 63}, + }, + { + name: "the lowest of several batteries decides", + supplies: []supply{ + {name: "BAT0", fields: map[string]string{ + "type": "Battery", "status": "Discharging", "capacity": "80", + }}, + {name: "BAT1", fields: map[string]string{ + "type": "Battery", "status": "Discharging", "capacity": "22", + }}, + }, + want: Status{Source: SourceBattery, Percent: 22}, + }, + { + name: "one unreadable system battery makes the combined reading unknown", + supplies: []supply{ + {name: "BAT0", fields: map[string]string{ + "type": "Battery", "status": "Discharging", "capacity": "80", + }}, + {name: "BAT1", fields: map[string]string{ + "type": "Battery", "status": "Discharging", + }}, + }, + want: Status{Source: SourceUnknown}, + }, + { + name: "a system battery ranks above a peripheral one", + supplies: []supply{ + {name: "BAT0", fields: map[string]string{ + "type": "Battery", "scope": "System", "status": "Discharging", "capacity": "70", + }}, + {name: "hidpp_battery_0", fields: map[string]string{ + "type": "Battery", "scope": "Device", "status": "Discharging", "capacity": "3", + }}, + }, + want: Status{Source: SourceBattery, Percent: 70}, + }, + { + name: "a battery with no readable charge is unknown, not full", + supplies: []supply{ + {name: "BAT0", fields: map[string]string{"type": "Battery", "status": "Discharging"}}, + }, + want: Status{Source: SourceUnknown}, + }, + { + name: "a listed supply with no readable type is unknown", + supplies: []supply{ + {name: "mystery", fields: map[string]string{}}, + }, + want: Status{Source: SourceUnknown}, + }, + { + name: "a charge outside 0-100 is junk and reads as unknown", + supplies: []supply{ + {name: "BAT0", fields: map[string]string{ + "type": "Battery", "status": "Discharging", "capacity": "6553", + }}, + }, + want: Status{Source: SourceUnknown}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + fs := writeSupplies(t, root, tt.supplies) + got, err := statusFrom(fs, root) + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + +// A kernel built without the power-supply class has nothing to report, which +// is hardware that runs on mains rather than a device hiding a flat battery. +func TestStatusFromMissingClass(t *testing.T) { + t.Parallel() + + root := filepath.Join(string(filepath.Separator), "sys", "class", "power_supply") + got, err := statusFrom(afero.NewMemMapFs(), root) + require.NoError(t, err) + assert.Equal(t, Status{Source: SourceNoBattery}, got) +} diff --git a/pkg/helpers/power/power_other.go b/pkg/helpers/power/power_other.go new file mode 100644 index 000000000..bc3f54c1a --- /dev/null +++ b/pkg/helpers/power/power_other.go @@ -0,0 +1,35 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +//go:build !linux && !windows && !darwin + +package power + +// Read reports an unknown power state. Core ships builds for Linux, Windows +// and macOS, and each has its own reader; a build for anything else has no way +// to ask the hardware. +// +// Unknown is the fail-safe answer rather than the convenient one. The updater +// treats it as "could lose power at any moment" and refuses an automatic +// install, which a person can still force past once they have been told the +// charge is unreadable. Reporting no battery instead would hand every such +// build a green light no reading supports. +func Read() (Status, error) { + return Status{Source: SourceUnknown}, nil +} diff --git a/pkg/helpers/power/power_windows.go b/pkg/helpers/power/power_windows.go new file mode 100644 index 000000000..42589e52a --- /dev/null +++ b/pkg/helpers/power/power_windows.go @@ -0,0 +1,79 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package power + +import ( + "fmt" + "unsafe" + + "golang.org/x/sys/windows" +) + +// systemPowerStatus mirrors the SYSTEM_POWER_STATUS structure filled in by +// GetSystemPowerStatus. +type systemPowerStatus struct { + ACLineStatus byte + BatteryFlag byte + BatteryLifePercent byte + SystemStatusFlag byte + BatteryLifeTime uint32 + BatteryFullLifeTime uint32 +} + +const ( + acLineOnline = 1 + + // batteryFlagNoBattery is the bit Windows sets when the machine has no + // system battery. + batteryFlagNoBattery = 128 + // batteryFlagUnknown is the whole byte Windows returns when it cannot + // determine the battery state. + batteryFlagUnknown = 255 + // batteryPercentUnknown is what BatteryLifePercent holds when the charge + // is not known. + batteryPercentUnknown = 255 +) + +// Read reports the device's power state through the Windows power API. +func Read() (Status, error) { + var raw systemPowerStatus + proc := windows.NewLazySystemDLL("kernel32.dll").NewProc("GetSystemPowerStatus") + //nolint:gosec // G103: the pointer is to a local struct the syscall fills in + ret, _, err := proc.Call(uintptr(unsafe.Pointer(&raw))) + if ret == 0 { + return Status{Source: SourceUnknown}, fmt.Errorf("reading system power status: %w", err) + } + return statusFrom(&raw), nil +} + +// statusFrom resolves the four states the updater distinguishes from one +// SYSTEM_POWER_STATUS reading. +func statusFrom(raw *systemPowerStatus) Status { + if raw.BatteryFlag != batteryFlagUnknown && raw.BatteryFlag&batteryFlagNoBattery != 0 { + return Status{Source: SourceNoBattery} + } + if raw.ACLineStatus == acLineOnline { + return Status{Source: SourceExternal} + } + if raw.BatteryLifePercent <= 100 && raw.BatteryLifePercent != batteryPercentUnknown { + return Status{Source: SourceBattery, Percent: int(raw.BatteryLifePercent)} + } + return Status{Source: SourceUnknown} +} diff --git a/pkg/helpers/power/power_windows_test.go b/pkg/helpers/power/power_windows_test.go new file mode 100644 index 000000000..47dbb762a --- /dev/null +++ b/pkg/helpers/power/power_windows_test.go @@ -0,0 +1,92 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +//go:build windows + +package power + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestStatusFrom(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + want Status + raw systemPowerStatus + }{ + { + name: "a desktop has no battery", + raw: systemPowerStatus{ + ACLineStatus: acLineOnline, + BatteryFlag: batteryFlagNoBattery, + BatteryLifePercent: batteryPercentUnknown, + }, + want: Status{Source: SourceNoBattery}, + }, + { + name: "a plugged-in laptop is on external power", + raw: systemPowerStatus{ + ACLineStatus: acLineOnline, + BatteryFlag: 1, + BatteryLifePercent: 90, + }, + want: Status{Source: SourceExternal}, + }, + { + name: "an unplugged laptop reports its charge", + raw: systemPowerStatus{ + ACLineStatus: 0, + BatteryFlag: 2, + BatteryLifePercent: 37, + }, + want: Status{Source: SourceBattery, Percent: 37}, + }, + { + name: "an unreadable charge is unknown, not full", + raw: systemPowerStatus{ + ACLineStatus: 0, + BatteryFlag: batteryFlagUnknown, + BatteryLifePercent: batteryPercentUnknown, + }, + want: Status{Source: SourceUnknown}, + }, + { + name: "an unknown battery flag does not read as no battery", + raw: systemPowerStatus{ + ACLineStatus: 0, + BatteryFlag: batteryFlagUnknown, + BatteryLifePercent: 15, + }, + want: Status{Source: SourceBattery, Percent: 15}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + assert.Equal(t, tt.want, statusFrom(&tt.raw)) + }) + } +} diff --git a/pkg/platforms/launch.go b/pkg/platforms/launch.go index d7375db28..83e02a37d 100644 --- a/pkg/platforms/launch.go +++ b/pkg/platforms/launch.go @@ -123,6 +123,10 @@ func DoLaunch(params *LaunchParams, getDisplayName func(string) string) error { if params.Options == nil { params.Options = &LaunchOptions{} } + publishActiveMedia := params.SetActiveMedia + if params.Options.ActiveMediaPublisher != nil { + publishActiveMedia = params.Options.ActiveMediaPublisher + } if params.Options.Action == "" { params.Options.Action = action } @@ -269,7 +273,7 @@ func DoLaunch(params *LaunchParams, getDisplayName func(string) string) error { activeMedia.SystemID, activeMedia.SystemName, activeMedia.Path, activeMedia.Name, activeMedia.LauncherID, ) - params.SetActiveMedia(activeMedia) + publishActiveMedia(activeMedia) return nil } diff --git a/pkg/platforms/launch_test.go b/pkg/platforms/launch_test.go index 977e643eb..2216884e8 100644 --- a/pkg/platforms/launch_test.go +++ b/pkg/platforms/launch_test.go @@ -441,6 +441,41 @@ func TestDoLaunch_ExternalLifecycleDefersActiveMedia(t *testing.T) { mockPlatform.AssertExpectations(t) } +func TestDoLaunch_UsesLaunchScopedActiveMediaPublisher(t *testing.T) { + t.Parallel() + + mockPlatform := mocks.NewMockPlatform() + mockPlatform.On("StopActiveLauncher", platforms.StopForPreemption).Return(nil).Once() + launcher := &platforms.Launcher{ + ID: "test-launcher", + SystemID: "SNES", + Lifecycle: platforms.LifecycleFireAndForget, + Launch: func(*config.Instance, string, *platforms.LaunchOptions) (*os.Process, error) { + var noProcess *os.Process + return noProcess, nil + }, + } + + var published *models.ActiveMedia + externalCalled := false + params := &platforms.LaunchParams{ + Platform: mockPlatform, + Config: &config.Instance{}, + SetActiveMedia: func(*models.ActiveMedia) { externalCalled = true }, + Launcher: launcher, + Path: "game.sfc", + Options: &platforms.LaunchOptions{ + ActiveMediaPublisher: func(media *models.ActiveMedia) { published = media }, + }, + } + + require.NoError(t, platforms.DoLaunch(params, func(_ string) string { return "Game" })) + require.NotNil(t, published) + assert.Equal(t, "SNES", published.SystemID) + assert.False(t, externalCalled) + mockPlatform.AssertExpectations(t) +} + func TestDoLaunch_DetailsActionSkipsActiveMedia(t *testing.T) { t.Parallel() diff --git a/pkg/platforms/platforms.go b/pkg/platforms/platforms.go index 1617a7f9b..33b853618 100644 --- a/pkg/platforms/platforms.go +++ b/pkg/platforms/platforms.go @@ -160,6 +160,13 @@ type Control struct { Script string // ZapScript string executed via RunControlScript } +// MediaLaunchAccess is the state publication capability held for one launch. +// Release must be called after LaunchMedia returns. +type MediaLaunchAccess struct { + SetActiveMedia func(*models.ActiveMedia) + Release func() +} + // CmdEnv is the local state of a scanned token, as it processes each ZapScript // command. Every command run has access to and can modify it. type CmdEnv struct { @@ -169,7 +176,7 @@ type CmdEnv struct { // for work tied to service lifetime rather than the current launcher lifetime. ServiceCtx context.Context WaitForMediaReady func(context.Context) error - AcquireMediaLaunch func() (func(), error) + AcquireMediaLaunch func() (MediaLaunchAccess, error) PlaybackManager audio.PlaybackManager UI *uievents.Service Playlist playlists.PlaylistController @@ -240,6 +247,9 @@ type ScanResult struct { // LaunchOptions contains optional parameters that can be passed to launchers. type LaunchOptions struct { + // ActiveMediaPublisher is the launch-scoped publication callback supplied + // by Core. Launchers must not retain or invoke it directly. + ActiveMediaPublisher func(*models.ActiveMedia) // RenderScale is the preferred internal rendering size as a percentage of // available output dimensions. It does not change physical display mode. RenderScale *int diff --git a/pkg/platforms/power.go b/pkg/platforms/power.go new file mode 100644 index 000000000..ed0bc1a11 --- /dev/null +++ b/pkg/platforms/power.go @@ -0,0 +1,110 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package platforms + +import ( + "time" + + "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/power" + "github.com/rs/zerolog/log" +) + +const powerStatusReadTimeout = 2 * time.Second + +// powerReadSlot caps an uncooperative synchronous reader at one in-flight +// goroutine. Later callers still time out instead of starting more blocked +// reads. +var powerReadSlot = make(chan struct{}, 1) + +type powerReadResult struct { + err error + status power.Status +} + +// PowerStatusProvider is optionally implemented by platforms whose power state +// cannot be read the ordinary way. Handheld hardware with an out-of-tree +// battery driver is the case this exists for; platforms that leave it +// unimplemented are read through the kernel or OS power API instead. +type PowerStatusProvider interface { + PowerStatus() (power.Status, error) +} + +// PowerStatus reports where pl is drawing power from, preferring the +// platform's own reading when it has one. +// +// A reading that fails is reported as unknown rather than as an error the +// caller has to interpret: whether the battery is unreadable or the call +// itself broke, what the caller can do about it is the same. +func PowerStatus(pl Platform) power.Status { + return resolvePowerStatus(pl, power.Read, powerStatusReadTimeout) +} + +func resolvePowerStatus( + pl Platform, + fallback func() (power.Status, error), + timeout time.Duration, +) power.Status { + read := fallback + if provider, ok := pl.(PowerStatusProvider); ok { + read = provider.PowerStatus + } + + status, timedOut, err := readPowerStatus(read, timeout) + if timedOut { + log.Debug().Dur("timeout", timeout).Msg("timed out reading device power status") + return power.Status{Source: power.SourceUnknown} + } + if err != nil { + log.Debug().Err(err).Msg("could not read device power status") + return power.Status{Source: power.SourceUnknown} + } + if status.Source == "" { + return power.Status{Source: power.SourceUnknown} + } + return status +} + +func readPowerStatus( + read func() (power.Status, error), + timeout time.Duration, +) (power.Status, bool, error) { + timer := time.NewTimer(timeout) + defer timer.Stop() + + select { + case powerReadSlot <- struct{}{}: + case <-timer.C: + return power.Status{}, true, nil + } + + resultCh := make(chan powerReadResult, 1) + go func() { + status, err := read() + <-powerReadSlot + resultCh <- powerReadResult{status: status, err: err} + }() + + select { + case result := <-resultCh: + return result.status, false, result.err + case <-timer.C: + return power.Status{}, true, nil + } +} diff --git a/pkg/platforms/power_test.go b/pkg/platforms/power_test.go new file mode 100644 index 000000000..c1fe5cefb --- /dev/null +++ b/pkg/platforms/power_test.go @@ -0,0 +1,137 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package platforms + +import ( + "errors" + "testing" + "time" + + "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/power" + "github.com/stretchr/testify/assert" +) + +// plainPlatform implements nothing: the embedded interface is nil and the test +// only ever asks whether it provides a power reading, which it does not. +type plainPlatform struct { + Platform +} + +type poweredPlatform struct { + Platform + read func() (power.Status, error) + err error + status power.Status +} + +func (p *poweredPlatform) PowerStatus() (power.Status, error) { + if p.read != nil { + return p.read() + } + return p.status, p.err +} + +func TestPowerStatus_PlatformReadingWins(t *testing.T) { + pl := &poweredPlatform{status: power.Status{Source: power.SourceBattery, Percent: 42}} + status := PowerStatus(pl) + + assert.Equal(t, power.SourceBattery, status.Source) + assert.Equal(t, 42, status.Percent) +} + +func TestPowerStatus_PlatformErrorReadsAsUnknown(t *testing.T) { + pl := &poweredPlatform{ + status: power.Status{Source: power.SourceBattery, Percent: 90}, + err: errors.New("battery driver not responding"), + } + status := PowerStatus(pl) + + // A percentage that came back alongside an error is not trustworthy, and + // treating it as a full battery is the mistake that costs a device. + assert.Equal(t, power.SourceUnknown, status.Source) + assert.Zero(t, status.Percent) +} + +func TestPowerStatus_UnsetSourceReadsAsUnknown(t *testing.T) { + status := PowerStatus(&poweredPlatform{}) + + assert.Equal(t, power.SourceUnknown, status.Source) +} + +func TestPowerStatus_FallsBackToTheOSReading(t *testing.T) { + called := false + status := resolvePowerStatus(&plainPlatform{}, func() (power.Status, error) { + called = true + return power.Status{Source: power.SourceExternal}, nil + }, time.Second) + + assert.True(t, called) + assert.Equal(t, power.SourceExternal, status.Source) +} + +func TestPowerStatus_TimesOutEveryReaderPath(t *testing.T) { + tests := []struct { + name string + provider bool + }{ + {name: "OS fallback"}, + {name: "platform provider", provider: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + started := make(chan struct{}) + unblock := make(chan struct{}) + finished := make(chan struct{}) + read := func() (power.Status, error) { + close(started) + <-unblock + close(finished) + return power.Status{Source: power.SourceExternal}, nil + } + + var pl Platform = &plainPlatform{} + fallback := read + fallbackCalled := false + if tt.provider { + pl = &poweredPlatform{read: read} + fallback = func() (power.Status, error) { + fallbackCalled = true + return power.Status{Source: power.SourceExternal}, nil + } + } + + status := resolvePowerStatus(pl, fallback, 20*time.Millisecond) + assert.Equal(t, power.SourceUnknown, status.Source) + assert.False(t, fallbackCalled) + select { + case <-started: + case <-time.After(time.Second): + t.Fatal("power reader did not start") + } + close(unblock) + select { + case <-finished: + case <-time.After(time.Second): + t.Fatal("power reader did not finish after release") + } + }) + } +} diff --git a/pkg/service/mediadb_schema_reset_test.go b/pkg/service/mediadb_schema_reset_test.go index a66897281..032447bdd 100644 --- a/pkg/service/mediadb_schema_reset_test.go +++ b/pkg/service/mediadb_schema_reset_test.go @@ -270,7 +270,7 @@ func TestStart_SchemaAheadPostsInboxMessage(t *testing.T) { cfg, err := testhelpers.NewTestConfigWithListenAndPort(nil, testRoot, "127.0.0.1", tcpAddr.Port) require.NoError(t, err) - cfg.SetAutoUpdate(false) + cfg.SetUpdateCheck(false) mockPlatform := testmocks.NewMockPlatform() mockPlatform.On("ID").Return("mock-platform") diff --git a/pkg/service/next_actions_test.go b/pkg/service/next_actions_test.go index 0d1b6bc3b..851bd2128 100644 --- a/pkg/service/next_actions_test.go +++ b/pkg/service/next_actions_test.go @@ -160,7 +160,9 @@ func TestRunTokenZapScript_AppliesPendingLaunchOverride(t *testing.T) { return l != nil && l.ID == "3do-dualram" }), svc.DB, - (*platforms.LaunchOptions)(nil)).Return(nil).Once() + mock.MatchedBy(func(opts *platforms.LaunchOptions) bool { + return opts != nil && opts.ActiveMediaPublisher != nil + })).Return(nil).Once() svc.State.SetPendingLaunchOverride(&state.PendingLaunchOverride{ LauncherID: "3do-dualram", @@ -186,7 +188,9 @@ func TestRunTokenZapScript_PlaylistDoesNotConsumePendingOverride(t *testing.T) { mockPlatform.On("LaunchMedia", cfg, path, (*platforms.Launcher)(nil), svc.DB, - (*platforms.LaunchOptions)(nil)).Return(nil).Once() + mock.MatchedBy(func(opts *platforms.LaunchOptions) bool { + return opts != nil && opts.ActiveMediaPublisher != nil + })).Return(nil).Once() svc.State.SetPendingLaunchOverride(&state.PendingLaunchOverride{ LauncherID: "3do-dualram", Source: tokens.Token{UID: "source"}, diff --git a/pkg/service/scan_behavior_test.go b/pkg/service/scan_behavior_test.go index 87c01b69b..ba8dda1f7 100644 --- a/pkg/service/scan_behavior_test.go +++ b/pkg/service/scan_behavior_test.go @@ -129,11 +129,17 @@ mode = "unrestricted"`)) mock.Anything, ).Return(nil).Run(func(args mock.Arguments) { path := args.String(1) - st.SetActiveMedia(&models.ActiveMedia{ + media := &models.ActiveMedia{ SystemID: "mock", Path: path, Name: path, - }) + } + opts, ok := args.Get(4).(*platforms.LaunchOptions) + if ok && opts != nil && opts.ActiveMediaPublisher != nil { + opts.ActiveMediaPublisher(media) + } else { + st.SetActiveMedia(media) + } launchCh <- path }).Maybe() diff --git a/pkg/service/service_test.go b/pkg/service/service_test.go index 3dd2cb9d4..a0b6d181b 100644 --- a/pkg/service/service_test.go +++ b/pkg/service/service_test.go @@ -205,7 +205,7 @@ func TestStartReturnsErrorWhenAPIPortIsOccupied(t *testing.T) { cfg, err := testhelpers.NewTestConfigWithListenAndPort(nil, testRoot, "127.0.0.1", tcpAddr.Port) require.NoError(t, err) - cfg.SetAutoUpdate(false) + cfg.SetUpdateCheck(false) mockPlatform := mocks.NewMockPlatform() mockPlatform.On("ID").Return("mock-platform") diff --git a/pkg/service/state/state.go b/pkg/service/state/state.go index 46ca20b9d..34efe3cf8 100644 --- a/pkg/service/state/state.go +++ b/pkg/service/state/state.go @@ -91,7 +91,6 @@ type State struct { lastScanned tokens.Token activeToken tokens.Token activeMediaReadyGen uint64 - mediaLaunchAccesses int mediaRestoreMu syncutil.RWMutex mu syncutil.RWMutex mediaLaunchMu syncutil.RWMutex @@ -435,6 +434,25 @@ func (s *State) ReaderWriteActive(readerIDs ...string) bool { return writeState != nil && writeState.activeWrites > 0 } +// AnyReaderWriteActive reports whether any reader is part-way through writing +// a token. +// +// ReaderWriteActive answers for one reader and defaults to the empty ID, which +// no production caller records under: writes are tracked per reader by +// Reader.ID(). A caller that wants to know whether the device is mid-write at +// all — the updater, before it restarts the service — has to ask about every +// reader, not about a reader that does not exist. +func (s *State) AnyReaderWriteActive() bool { + s.mu.RLock() + defer s.mu.RUnlock() + for _, writeState := range s.readerWrites { + if writeState != nil && writeState.activeWrites > 0 { + return true + } + } + return false +} + func (s *State) MarkWrittenTagRemoved(readerIDs ...string) { readerID := "" if len(readerIDs) > 0 { @@ -604,10 +622,10 @@ func (s *State) AcquireRestoreAccess() (func(), error) { return s.restoreAccessAfterLock() } -func (s *State) AcquireMediaLaunch() (func(), error) { +func (s *State) AcquireMediaLaunch() (platforms.MediaLaunchAccess, error) { releaseRestore, err := s.TryAcquireRestoreAccess() if err != nil { - return nil, err + return platforms.MediaLaunchAccess{}, err } // Stop operations take the exclusive side of this gate. Holding the read @@ -617,17 +635,32 @@ func (s *State) AcquireMediaLaunch() (func(), error) { if err := s.ctx.Err(); err != nil { s.mediaLaunchMu.RUnlock() releaseRestore() - return nil, err + return platforms.MediaLaunchAccess{}, err } - s.mu.Lock() - s.mediaLaunchAccesses++ - s.mu.Unlock() - return func() { - s.mu.Lock() - s.mediaLaunchAccesses-- - s.mu.Unlock() - s.mediaLaunchMu.RUnlock() - releaseRestore() + + var accessMu syncutil.Mutex + held := true + return platforms.MediaLaunchAccess{ + SetActiveMedia: func(media *models.ActiveMedia) { + accessMu.Lock() + if !held { + accessMu.Unlock() + s.SetActiveMedia(media) + return + } + s.publishActiveMedia(media, true) + accessMu.Unlock() + }, + Release: func() { + accessMu.Lock() + defer accessMu.Unlock() + if !held { + return + } + held = false + s.mediaLaunchMu.RUnlock() + releaseRestore() + }, }, nil } @@ -774,7 +807,33 @@ func (s *State) MarkActiveMediaReady(gen uint64) { } func (s *State) SetActiveMedia(media *models.ActiveMedia) { + s.publishActiveMedia(media, false) +} + +func (s *State) publishActiveMedia(media *models.ActiveMedia, restoreAccessHeld bool) { if media != nil { + if err := s.ctx.Err(); err != nil { + log.Debug().Err(err).Msg("active media update rejected while service is stopping") + return + } + + // Restore access comes first. The service takes these three locks in + // one order everywhere — restore, then launch, then publish — and + // taking them in any other order closes a cycle between this, a media + // launch, and the updater's gate. + if !restoreAccessHeld { + release, err := s.TryAcquireRestoreAccess() + if errors.Is(err, ErrRestoreInProgress) { + s.backupCoordinator.CancelRestore() + release, err = s.AcquireRestoreAccess() + } + if err != nil { + log.Warn().Err(err).Msg("active media update rejected during backup restore") + return + } + defer release() + } + // This gate is separate from mediaLaunchMu because normal launch code // already holds that lock when it publishes ActiveMedia. External // lifecycle trackers do not, so every publication takes this read side. @@ -784,24 +843,6 @@ func (s *State) SetActiveMedia(media *models.ActiveMedia) { log.Debug().Err(err).Msg("active media update rejected while service is stopping") return } - - s.mu.RLock() - launchAccessHeld := s.mediaLaunchAccesses > 0 - s.mu.RUnlock() - if launchAccessHeld { - s.updateActiveMediaState(media) - return - } - release, err := s.TryAcquireRestoreAccess() - if errors.Is(err, ErrRestoreInProgress) { - s.backupCoordinator.CancelRestore() - release, err = s.AcquireRestoreAccess() - } - if err != nil { - log.Warn().Err(err).Msg("active media update rejected during backup restore") - return - } - defer release() } s.updateActiveMediaState(media) diff --git a/pkg/service/state/state_media_test.go b/pkg/service/state/state_media_test.go index 494f1286f..a4239d3e5 100644 --- a/pkg/service/state/state_media_test.go +++ b/pkg/service/state/state_media_test.go @@ -51,7 +51,7 @@ func TestMediaRestoreGateMutualExclusion(t *testing.T) { st, _ := NewState(nil, "test-boot") defer st.StopService() - releaseLaunch, err := st.AcquireMediaLaunch() + launchAccess, err := st.AcquireMediaLaunch() require.NoError(t, err) restoreErr := make(chan error, 1) go func() { @@ -62,24 +62,24 @@ func TestMediaRestoreGateMutualExclusion(t *testing.T) { restoreErr <- beginErr }() require.ErrorIs(t, <-restoreErr, ErrMediaLaunchInProgress) - releaseLaunch() + launchAccess.Release() finishRestore, err := st.BeginRestoreGate() require.NoError(t, err) launchErr := make(chan error, 1) go func() { - release, acquireErr := st.AcquireMediaLaunch() - if release != nil { - release() + access, acquireErr := st.AcquireMediaLaunch() + if access.Release != nil { + access.Release() } launchErr <- acquireErr }() require.ErrorIs(t, <-launchErr, ErrRestoreInProgress) finishRestore(false) - releaseLaunch, err = st.AcquireMediaLaunch() + launchAccess, err = st.AcquireMediaLaunch() require.NoError(t, err) - releaseLaunch() + launchAccess.Release() } func TestMediaStopGateWaitsForLaunchAndBlocksReplacement(t *testing.T) { @@ -87,7 +87,7 @@ func TestMediaStopGateWaitsForLaunchAndBlocksReplacement(t *testing.T) { st, _ := NewState(nil, "test-boot") defer st.StopService() - releaseLaunch, err := st.AcquireMediaLaunch() + launchAccess, err := st.AcquireMediaLaunch() require.NoError(t, err) stopRelease := make(chan func(), 1) @@ -109,7 +109,7 @@ func TestMediaStopGateWaitsForLaunchAndBlocksReplacement(t *testing.T) { case <-time.After(50 * time.Millisecond): } - releaseLaunch() + launchAccess.Release() var releaseStop func() select { case releaseStop = <-stopRelease: @@ -124,11 +124,11 @@ func TestMediaStopGateWaitsForLaunchAndBlocksReplacement(t *testing.T) { err error }, 1) go func() { - release, acquireErr := st.AcquireMediaLaunch() + access, acquireErr := st.AcquireMediaLaunch() replacementResult <- struct { release func() err error - }{release: release, err: acquireErr} + }{release: access.Release, err: acquireErr} }() select { @@ -217,11 +217,11 @@ func TestMediaStopGateHonorsContextCancellation(t *testing.T) { st, _ := NewState(nil, "test-boot") defer st.StopService() - releaseLaunch, err := st.AcquireMediaLaunch() + launchAccess, err := st.AcquireMediaLaunch() require.NoError(t, err) defer func() { - if releaseLaunch != nil { - releaseLaunch() + if launchAccess.Release != nil { + launchAccess.Release() } }() @@ -236,11 +236,11 @@ func TestMediaStopGateHonorsContextCancellation(t *testing.T) { err error }, 1) go func() { - release, acquireErr := st.AcquireMediaLaunch() + access, acquireErr := st.AcquireMediaLaunch() replacementResult <- struct { release func() err error - }{release: release, err: acquireErr} + }{release: access.Release, err: acquireErr} }() select { @@ -252,8 +252,8 @@ func TestMediaStopGateHonorsContextCancellation(t *testing.T) { t.Fatal("canceled media stop kept a replacement launch blocked") } - releaseLaunch() - releaseLaunch = nil + launchAccess.Release() + launchAccess.Release = nil cleanupCtx, cleanupCancel := context.WithTimeout(context.Background(), time.Second) defer cleanupCancel() releaseStop, err = st.AcquireMediaStop(cleanupCtx) @@ -297,6 +297,48 @@ func TestExternalActiveMediaCancelsRestoreBeforeUpdatingState(t *testing.T) { assert.NotNil(t, st.ActiveMedia()) } +func TestReleasedMediaLaunchPublisherUsesExternalRestoreAccess(t *testing.T) { + t.Parallel() + st, _ := NewState(nil, "test-boot") + defer st.StopService() + + launchAccess, err := st.AcquireMediaLaunch() + require.NoError(t, err) + launchAccess.Release() + + lease, err := st.BackupCoordinator().Begin( + context.Background(), backupcoordinator.OperationLocalRestore, backupcoordinator.OperationWrite, + ) + require.NoError(t, err) + defer lease.Release() + finishRestore, err := st.BeginRestoreGate() + require.NoError(t, err) + + updated := make(chan struct{}) + go func() { + launchAccess.SetActiveMedia(&models.ActiveMedia{SystemID: "SNES", Path: "game.sfc", Name: "Game"}) + close(updated) + }() + + select { + case <-lease.Context().Done(): + case <-time.After(time.Second): + t.Fatal("released launch publisher did not use external restore handling") + } + select { + case <-updated: + t.Fatal("released launch publisher changed media before restore gate released") + case <-time.After(50 * time.Millisecond): + } + finishRestore(false) + select { + case <-updated: + case <-time.After(time.Second): + t.Fatal("released launch publisher did not resume after restore gate released") + } + assert.NotNil(t, st.ActiveMedia()) +} + func TestBlockingRestoreAccessWaitsForRollback(t *testing.T) { t.Parallel() st, _ := NewState(nil, "test-boot") @@ -340,8 +382,8 @@ func TestAcquireMediaLaunch_RejectsStoppedService(t *testing.T) { st, _ := NewState(nil, "test-boot") st.StopService() - release, err := st.AcquireMediaLaunch() - assert.Nil(t, release) + access, err := st.AcquireMediaLaunch() + assert.Nil(t, access.Release) require.ErrorIs(t, err, context.Canceled) } diff --git a/pkg/service/state/state_test.go b/pkg/service/state/state_test.go index 250367098..777c9a1ef 100644 --- a/pkg/service/state/state_test.go +++ b/pkg/service/state/state_test.go @@ -149,6 +149,29 @@ func TestReaderWriteActiveTracksOverlappingWrites(t *testing.T) { assert.False(t, state.ReaderWriteActive("reader-1")) } +// The updater asks whether the device is mid-write at all, and writes are +// recorded against the reader doing them, so an answer that only covers one +// reader ID is no answer. +func TestAnyReaderWriteActiveCoversEveryReader(t *testing.T) { + t.Parallel() + mockPlatform := mocks.NewMockPlatform() + state, _ := NewState(mockPlatform, "test-boot-uuid") + + assert.False(t, state.AnyReaderWriteActive()) + + state.SetReaderWriteActive(true, "reader-1") + assert.True(t, state.AnyReaderWriteActive()) + // The empty ID is the bucket no production writer records under. + assert.False(t, state.ReaderWriteActive()) + + state.SetReaderWriteActive(true, "reader-2") + state.SetReaderWriteActive(false, "reader-1") + assert.True(t, state.AnyReaderWriteActive()) + + state.SetReaderWriteActive(false, "reader-2") + assert.False(t, state.AnyReaderWriteActive()) +} + func TestWrittenTagRemovalDuringWriteClearsCompletedToken(t *testing.T) { t.Parallel() mockPlatform := mocks.NewMockPlatform() @@ -409,3 +432,46 @@ func TestBackupCoordinatorReleasePublishesFinishedNotification(t *testing.T) { t.Fatal("expected backup.state finished notification") } } + +// TestLockOrder_RestoreThenLaunchThenPublish walks every path that takes more +// than one of the three media gates. Built with -tags=deadlock, go-deadlock +// records the order each path takes them in and panics on the first path that +// disagrees, so exercising them once each is enough to catch an inversion. +func TestLockOrder_RestoreThenLaunchThenPublish(t *testing.T) { + t.Parallel() + mockPlatform := mocks.NewMockPlatform() + state, ns := NewState(mockPlatform, "test-boot-uuid") + t.Cleanup(state.StopService) + // Publishing media notifies, and nothing here reads those. + t.Cleanup(func() { + for { + select { + case <-ns: + default: + return + } + } + }) + + // A launch takes restore access and then the launch gate, and its + // publication capability takes the publish gate under those holds. + launchAccess, err := state.AcquireMediaLaunch() + require.NoError(t, err) + launchAccess.SetActiveMedia(&models.ActiveMedia{SystemID: "test", Name: "launch"}) + launchAccess.Release() + state.SetActiveMedia(nil) + + // A publication from outside a launch takes restore access and then the + // publish gate. + state.SetActiveMedia(&models.ActiveMedia{SystemID: "test", Name: "test"}) + state.SetActiveMedia(nil) + + // The updater takes the launch gate and then the publish gate, under the + // restore access its caller is already holding. + releaseRestore, err := state.TryAcquireRestoreAccess() + require.NoError(t, err) + releaseGate, err := state.AcquireUpdateMediaGate(t.Context()) + require.NoError(t, err) + releaseGate() + releaseRestore() +} diff --git a/pkg/service/updater/gate.go b/pkg/service/updater/gate.go new file mode 100644 index 000000000..d4be76c90 --- /dev/null +++ b/pkg/service/updater/gate.go @@ -0,0 +1,415 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package updater + +import ( + "context" + "fmt" + "time" + + "github.com/ZaparooProject/zaparoo-core/v2/pkg/database/mediadb" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/power" +) + +// Mode is who asked for the update. +type Mode string + +const ( + // ModeManual is a person pressing update in a client. They are at the + // device, they can see what it is doing, and they can be asked about + // anything short of a real risk to their data. + ModeManual Mode = "manual" + // ModeAuto is the device deciding for itself. Nobody is watching, so + // anything that would surprise a user is a reason to wait. + ModeAuto Mode = "auto" +) + +// autoInstallDeadline is how long a version may sit deferred by a soft signal +// before an automatic install goes ahead regardless. A cabinet that plays +// something every waking hour would otherwise never be idle and never get a +// security fix. +const autoInstallDeadline = 24 * time.Hour + +// Battery charge an install needs, as a percentage. Automatic installs ask for +// more because nobody is there to plug the device in when it gets close. +const ( + manualBatteryFloor = 20 + autoBatteryFloor = 40 +) + +// Reasons an update cannot be applied right now. These are the machine-readable +// half of a refusal; clients match on them to decide what to say. +const ( + ReasonMediaIndexing = "mediaIndexing" + ReasonMediaOptimizing = "mediaOptimizing" + ReasonMediaScraping = "mediaScraping" + ReasonBackupActive = "backupActive" + ReasonReaderWriting = "readerWriting" + ReasonRestoreActive = "restoreActive" + ReasonActiveMedia = "activeMedia" + ReasonBackgroundMedia = "backgroundMedia" + ReasonActivePlaylist = "activePlaylist" + ReasonPowerLow = "powerLow" + ReasonPowerUnknown = "powerUnknown" + ReasonAPIBusy = "apiBusy" +) + +// GateDeps is everything the gate needs to look at, as plain functions so the +// gate can be tested without a running service. A nil function means the caller +// has nothing to report for that signal and it is skipped. +type GateDeps struct { + // IndexingStatus, OptimizationStatus and ScrapingStatus each return a + // mediadb status string. An error is treated as "not running": a database + // that cannot answer is a problem for the caller to notice elsewhere, and + // refusing every update because of it would leave the device unfixable. + IndexingStatus func() (string, error) + OptimizationStatus func() (string, error) + ScrapingStatus func() (string, error) + + // BackupActive reports whether a backup, restore or upload is running. + BackupActive func() bool + // ReaderWriteActive reports whether a reader is part-way through writing + // a token. + ReaderWriteActive func() bool + // AcquireRestore takes the restore gate, which the install then holds so + // a restore cannot start underneath it. The release function it returns + // is carried on the decision. + AcquireRestore func() (func(), error) + // AcquireMediaGate stops anything new launching and waits for what is + // already launching to settle, so the install's own look at what is + // playing cannot be overtaken by a launch that starts a moment later. The + // install holds it until the restart. + // + // The gate takes this after AcquireRestore, which is the order the rest of + // the service takes those two locks in. Taking them the other way round is + // a lock inversion, which is why neither is the caller's to take. + AcquireMediaGate func(context.Context) (func(), error) + + // ActiveMedia, BackgroundMedia and ActivePlaylist report what the user + // would lose if the service restarted now. + ActiveMedia func() bool + BackgroundMedia func() bool + ActivePlaylist func() bool + + // Power reports where the device's power is coming from. + Power func() power.Status + + // WaitForIdle blocks until the API has been quiet for a while. Only + // automatic installs wait for it; a person pressing update is the request + // that would otherwise stop it ever being idle. + WaitForIdle func(context.Context) error + + // DeferredSince is when this version was first put off by a soft signal, + // or the zero time if it has not been. Once that is more than + // autoInstallDeadline ago the soft signals stop counting. + DeferredSince func() time.Time + + // Now reads the clock. Tests replace it. + Now func() time.Time +} + +// GateDecision is the gate's answer. +type GateDecision struct { + // Release gives back whatever the gate took. It is never nil, so callers can + // defer it without checking, and it does nothing when the gate was never + // taken. + Release func() + // Reason is the machine-readable refusal, empty when OK. + Reason string + // Message says the same thing in words, for a client with nothing better + // to show. + Message string + // Forceable means a person may go ahead anyway. It is never true for + // automatic installs, and never true for anything that risks data rather + // than the user's session. + Forceable bool + // Expires means this is a soft signal that an automatic install may + // ignore once the version has waited out autoInstallDeadline. + Expires bool + // OK means the update may go ahead. + OK bool +} + +// GateError is a refusal from the gate, as an error, so a check made deep +// inside an install can be recognised again by the caller that started it. +type GateError struct { + Reason string + Message string + Forceable bool +} + +func (e *GateError) Error() string { + return e.Message +} + +// blocked builds a refusal that has not taken the restore gate. +func blocked(reason, message string, forceable, expires bool) GateDecision { + return GateDecision{ + Release: func() {}, + Reason: reason, + Message: message, + Forceable: forceable, + Expires: expires, + } +} + +// CanApplyUpdate reports whether an update may be installed right now. +// +// A decision that is OK carries the gates the install needs held, so the caller +// must call Release once the install has finished or failed. force lets a +// person past the signals that are only about their own session; it never gets +// past a signal that risks their data, and mode auto ignores it entirely. +// +// The error is separate from the decision on purpose: a refusal is an answer, +// but a cancelled request is not an answer at all. +func CanApplyUpdate(ctx context.Context, deps *GateDeps, mode Mode, force bool) (GateDecision, error) { + auto := mode == ModeAuto + if auto { + force = false + } + expired := auto && softSignalsExpired(deps) + + if decision, blocking := checkDataSignals(deps); blocking { + return decision, nil + } + if decision, blocking := checkPower(deps, mode, force); blocking { + return decision, nil + } + // The idle wait comes before the gates are held, because waiting for the + // API to go quiet while blocking every launch is how an automatic install + // makes the device look broken. + if decision, blocking := checkIdle(ctx, deps, auto, expired); blocking { + return decision, nil + } + + release, decision, err := acquireHolds(ctx, deps) + if err != nil || !decision.OK { + return decision, err + } + + // Read after the media gate is held, so a launch cannot start between the + // answer and the install that would close it. + if blocking, isBlocking := checkSessionSignals(deps, auto, force, expired); isBlocking { + release() + return blocking, nil + } + return GateDecision{OK: true, Release: release}, nil +} + +// acquireHolds takes the restore gate and then the media gate, in that order, +// and hands back one release function for both. +func acquireHolds(ctx context.Context, deps *GateDeps) (func(), GateDecision, error) { + releaseRestore := func() {} + if deps.AcquireRestore != nil { + acquired, err := deps.AcquireRestore() + if err != nil { + //nolint:nilerr // a restore already running is a refusal, not a failure + return nil, blocked( + ReasonRestoreActive, + "a backup restore is in progress", + false, false, + ), nil + } + releaseRestore = acquired + } + + releaseMedia := func() {} + if deps.AcquireMediaGate != nil { + acquired, err := deps.AcquireMediaGate(ctx) + if err != nil { + releaseRestore() + return nil, GateDecision{Release: func() {}}, + fmt.Errorf("waiting for media activity to settle: %w", err) + } + releaseMedia = acquired + } + + return func() { + releaseMedia() + releaseRestore() + }, GateDecision{OK: true, Release: func() {}}, nil +} + +// checkDataSignals covers the work that would lose or corrupt something if the +// service went away mid-write. None of it can be forced and none of it expires. +func checkDataSignals(deps *GateDeps) (GateDecision, bool) { + statuses := []struct { + read func() (string, error) + reason string + message string + }{ + {deps.IndexingStatus, ReasonMediaIndexing, "the media database is being generated"}, + {deps.OptimizationStatus, ReasonMediaOptimizing, "the media database is being optimized"}, + {deps.ScrapingStatus, ReasonMediaScraping, "media metadata is being downloaded"}, + } + for _, status := range statuses { + if statusBusy(status.read) { + return blocked(status.reason, status.message, false, false), true + } + } + + if deps.BackupActive != nil && deps.BackupActive() { + return blocked(ReasonBackupActive, "a backup or restore is in progress", false, false), true + } + if deps.ReaderWriteActive != nil && deps.ReaderWriteActive() { + return blocked(ReasonReaderWriting, "a token is being written", false, false), true + } + return GateDecision{}, false +} + +// checkIdle makes an automatic install wait for the API to go quiet. A person +// pressing update is the request that would otherwise stop it ever being idle, +// so it does not apply to them. +func checkIdle(ctx context.Context, deps *GateDeps, auto, expired bool) (GateDecision, bool) { + if !auto || expired || deps.WaitForIdle == nil { + return GateDecision{}, false + } + if err := deps.WaitForIdle(ctx); err != nil { + return blocked(ReasonAPIBusy, "the device is still handling requests", false, true), true + } + return GateDecision{}, false +} + +// checkSessionSignals covers what the user would lose if the service restarted +// now. A person can decide that for themselves; an automatic install waits, +// until the version has waited long enough. +func checkSessionSignals(deps *GateDeps, auto, force, expired bool) (GateDecision, bool) { + if expired { + return GateDecision{}, false + } + + media := []struct { + active func() bool + reason string + message string + }{ + {deps.ActiveMedia, ReasonActiveMedia, "media is playing"}, + {deps.BackgroundMedia, ReasonBackgroundMedia, "media is playing in the background"}, + {deps.ActivePlaylist, ReasonActivePlaylist, "a playlist is running"}, + } + for _, signal := range media { + if signal.active == nil || !signal.active() { + continue + } + if force { + continue + } + return blocked(signal.reason, signal.message, !auto, true), true + } + return GateDecision{}, false +} + +// softSignalsExpired reports whether this version has been put off for longer +// than an automatic install is willing to wait. +func softSignalsExpired(deps *GateDeps) bool { + if deps.DeferredSince == nil { + return false + } + since := deps.DeferredSince() + if since.IsZero() { + return false + } + now := time.Now + if deps.Now != nil { + now = deps.Now + } + return now().Sub(since) >= autoInstallDeadline +} + +// checkPower refuses an install the device may not have the charge to finish. +// A known charge below the floor is a hard block: force is a person saying they +// accept losing what is playing, not a person able to make the battery last. +func checkPower(deps *GateDeps, mode Mode, force bool) (GateDecision, bool) { + if deps.Power == nil { + return GateDecision{}, false + } + + floor := manualBatteryFloor + if mode == ModeAuto { + floor = autoBatteryFloor + } + + status := deps.Power() + switch status.Source { + case power.SourceNoBattery, power.SourceExternal: + return GateDecision{}, false + case power.SourceBattery: + if status.Percent >= floor { + return GateDecision{}, false + } + return blocked( + ReasonPowerLow, + fmt.Sprintf( + "the battery is at %d%%, and an update needs at least %d%% or a charger", + status.Percent, floor, + ), + false, false, + ), true + case power.SourceUnknown: + // The device may be on battery and there is no way to tell. An + // automatic install waits for a reading; a person can decide to go + // ahead once they have been told the charge is unknown. + if mode == ModeManual && force { + return GateDecision{}, false + } + return blocked( + ReasonPowerUnknown, + "the battery level could not be read", + mode == ModeManual, false, + ), true + default: + return GateDecision{}, false + } +} + +// statusBusy reports whether a mediadb status function says work is running or +// queued to run. +func statusBusy(read func() (string, error)) bool { + if read == nil { + return false + } + status, err := read() + if err != nil { + return false + } + return status == mediadb.IndexingStatusRunning || status == mediadb.IndexingStatusPending +} + +// Err turns a refusal into an error. It returns nil for a decision that is OK. +func (d *GateDecision) Err() error { + if d.OK { + return nil + } + return &GateError{ + Reason: d.Reason, + Message: d.Message, + Forceable: d.Forceable, + } +} + +// PowerReady re-runs only the power part of the gate. The install calls it once +// the download is finished, because a download long enough to matter is also +// long enough to outlive a charger being unplugged. +func PowerReady(deps *GateDeps, mode Mode, force bool) GateDecision { + if decision, blocking := checkPower(deps, mode, force); blocking { + return decision + } + return GateDecision{OK: true, Release: func() {}} +} diff --git a/pkg/service/updater/gate_test.go b/pkg/service/updater/gate_test.go new file mode 100644 index 000000000..b54e3eae5 --- /dev/null +++ b/pkg/service/updater/gate_test.go @@ -0,0 +1,569 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package updater + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/ZaparooProject/zaparoo-core/v2/pkg/database/mediadb" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/power" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func statusFn(status string) func() (string, error) { + return func() (string, error) { return status, nil } +} + +func alwaysTrue() bool { return true } + +// externalPower is what most devices report, and what the tests that are not +// about power want out of the way. +func externalPower() power.Status { + return power.Status{Source: power.SourceExternal} +} + +// TestCanApplyUpdateSignals walks the whole gate table: what blocks a person +// pressing update, what blocks the device deciding for itself, and which of +// those a person may go ahead through anyway. +func TestCanApplyUpdateSignals(t *testing.T) { + t.Parallel() + + tests := []struct { + mutate func(*GateDeps) + name string + wantReason string + wantForceable bool + wantExpires bool + blocksManual bool + blocksAuto bool + }{ + { + name: "nothing happening", + mutate: func(*GateDeps) {}, + blocksManual: false, + blocksAuto: false, + }, + { + name: "media indexing", + mutate: func(d *GateDeps) { + d.IndexingStatus = statusFn(mediadb.IndexingStatusRunning) + }, + blocksManual: true, blocksAuto: true, + wantReason: ReasonMediaIndexing, + }, + { + name: "media indexing queued", + mutate: func(d *GateDeps) { + d.IndexingStatus = statusFn(mediadb.IndexingStatusPending) + }, + blocksManual: true, blocksAuto: true, + wantReason: ReasonMediaIndexing, + }, + { + name: "database optimization", + mutate: func(d *GateDeps) { + d.OptimizationStatus = statusFn(mediadb.IndexingStatusRunning) + }, + blocksManual: true, blocksAuto: true, + wantReason: ReasonMediaOptimizing, + }, + { + name: "metadata scraping", + mutate: func(d *GateDeps) { + d.ScrapingStatus = statusFn(mediadb.IndexingStatusRunning) + }, + blocksManual: true, blocksAuto: true, + wantReason: ReasonMediaScraping, + }, + { + name: "backup running", + mutate: func(d *GateDeps) { d.BackupActive = alwaysTrue }, + blocksManual: true, blocksAuto: true, + wantReason: ReasonBackupActive, + }, + { + name: "token being written", + mutate: func(d *GateDeps) { d.ReaderWriteActive = alwaysTrue }, + blocksManual: true, blocksAuto: true, + wantReason: ReasonReaderWriting, + }, + { + name: "media playing", + mutate: func(d *GateDeps) { d.ActiveMedia = alwaysTrue }, + blocksManual: true, blocksAuto: true, + wantReason: ReasonActiveMedia, wantForceable: true, wantExpires: true, + }, + { + name: "media playing in the background", + mutate: func(d *GateDeps) { d.BackgroundMedia = alwaysTrue }, + blocksManual: true, blocksAuto: true, + wantReason: ReasonBackgroundMedia, wantForceable: true, wantExpires: true, + }, + { + name: "playlist running", + mutate: func(d *GateDeps) { d.ActivePlaylist = alwaysTrue }, + blocksManual: true, blocksAuto: true, + wantReason: ReasonActivePlaylist, wantForceable: true, wantExpires: true, + }, + { + name: "api still busy", + mutate: func(d *GateDeps) { + d.WaitForIdle = func(context.Context) error { return errors.New("still busy") } + }, + blocksManual: false, blocksAuto: true, + wantReason: ReasonAPIBusy, wantExpires: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + for _, mode := range []Mode{ModeManual, ModeAuto} { + deps := &GateDeps{Power: externalPower} + tt.mutate(deps) + + decision, err := CanApplyUpdate(t.Context(), deps, mode, false) + require.NoError(t, err) + decision.Release() + + wantBlocked := tt.blocksManual + if mode == ModeAuto { + wantBlocked = tt.blocksAuto + } + if !wantBlocked { + assert.True(t, decision.OK, "%s should not block %s", tt.name, mode) + continue + } + require.False(t, decision.OK, "%s should block %s", tt.name, mode) + assert.Equal(t, tt.wantReason, decision.Reason) + assert.NotEmpty(t, decision.Message) + assert.Equal(t, tt.wantExpires, decision.Expires) + if mode == ModeAuto { + assert.False(t, decision.Forceable, "an automatic install never forces") + continue + } + assert.Equal(t, tt.wantForceable, decision.Forceable) + } + }) + } +} + +// Force is a person accepting that their session ends. It is not a way past +// anything that would cost them data. +func TestCanApplyUpdateForce(t *testing.T) { + t.Parallel() + + tests := []struct { + mutate func(*GateDeps) + name string + wantForce bool + }{ + {name: "media playing", mutate: func(d *GateDeps) { d.ActiveMedia = alwaysTrue }, wantForce: true}, + {name: "background media", mutate: func(d *GateDeps) { d.BackgroundMedia = alwaysTrue }, wantForce: true}, + {name: "playlist running", mutate: func(d *GateDeps) { d.ActivePlaylist = alwaysTrue }, wantForce: true}, + { + name: "media indexing", + mutate: func(d *GateDeps) { d.IndexingStatus = statusFn(mediadb.IndexingStatusRunning) }, + }, + {name: "backup running", mutate: func(d *GateDeps) { d.BackupActive = alwaysTrue }}, + {name: "token being written", mutate: func(d *GateDeps) { d.ReaderWriteActive = alwaysTrue }}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + deps := &GateDeps{Power: externalPower} + tt.mutate(deps) + + decision, err := CanApplyUpdate(t.Context(), deps, ModeManual, true) + require.NoError(t, err) + decision.Release() + assert.Equal(t, tt.wantForce, decision.OK) + }) + } +} + +// An automatic install ignores force outright, so a caller that passes it by +// mistake cannot restart a device mid-game with nobody watching. +func TestCanApplyUpdateAutoIgnoresForce(t *testing.T) { + t.Parallel() + + deps := &GateDeps{Power: externalPower, ActiveMedia: alwaysTrue} + decision, err := CanApplyUpdate(t.Context(), deps, ModeAuto, true) + require.NoError(t, err) + decision.Release() + + require.False(t, decision.OK) + assert.Equal(t, ReasonActiveMedia, decision.Reason) + assert.False(t, decision.Forceable) +} + +// A cabinet that plays something every waking hour would defer forever, so the +// soft signals stop counting once a version has waited long enough. The hard +// ones never do. +func TestCanApplyUpdateSoftSignalDeadline(t *testing.T) { + t.Parallel() + + now := time.Date(2026, 8, 19, 12, 0, 0, 0, time.UTC) + + tests := []struct { + since time.Time + mutate func(*GateDeps) + name string + wantErr string + wantOK bool + }{ + { + name: "not deferred yet", + since: time.Time{}, + mutate: func(d *GateDeps) { d.ActiveMedia = alwaysTrue }, + }, + { + name: "deferred, still inside the deadline", + since: now.Add(-autoInstallDeadline + time.Minute), + mutate: func(d *GateDeps) { d.ActiveMedia = alwaysTrue }, + }, + { + name: "deferred past the deadline", + since: now.Add(-autoInstallDeadline - time.Minute), + mutate: func(d *GateDeps) { d.ActiveMedia = alwaysTrue }, + wantOK: true, + }, + { + name: "a busy api also gives up waiting", + since: now.Add(-autoInstallDeadline - time.Minute), + mutate: func(d *GateDeps) { d.WaitForIdle = func(context.Context) error { return errors.New("busy") } }, + wantOK: true, + }, + { + name: "indexing never expires", + since: now.Add(-100 * autoInstallDeadline), + mutate: func(d *GateDeps) { + d.IndexingStatus = statusFn(mediadb.IndexingStatusRunning) + }, + wantErr: ReasonMediaIndexing, + }, + { + name: "a flat battery never expires", + since: now.Add(-100 * autoInstallDeadline), + mutate: func(d *GateDeps) { d.Power = func() power.Status { return battery(5) } }, + wantErr: ReasonPowerLow, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + since := tt.since + deps := &GateDeps{ + Power: externalPower, + Now: func() time.Time { return now }, + DeferredSince: func() time.Time { return since }, + } + tt.mutate(deps) + + decision, err := CanApplyUpdate(t.Context(), deps, ModeAuto, false) + require.NoError(t, err) + decision.Release() + assert.Equal(t, tt.wantOK, decision.OK) + if tt.wantErr != "" { + assert.Equal(t, tt.wantErr, decision.Reason) + } + }) + } +} + +func battery(percent int) power.Status { + return power.Status{Source: power.SourceBattery, Percent: percent} +} + +// The power policy is the one part of the gate that is different for the two +// modes, because nobody is there to plug in a device that installs on its own. +func TestCanApplyUpdatePower(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + wantReason string + status power.Status + manualOK bool + autoOK bool + forcedOK bool + forceableOn bool + }{ + { + name: "no battery", + status: power.Status{Source: power.SourceNoBattery}, + manualOK: true, autoOK: true, forcedOK: true, + }, + { + name: "on a charger", + status: power.Status{Source: power.SourceExternal}, + manualOK: true, autoOK: true, forcedOK: true, + }, + { + name: "full battery", + status: battery(95), + manualOK: true, autoOK: true, forcedOK: true, + }, + { + name: "half battery clears both floors", + status: battery(40), + manualOK: true, autoOK: true, forcedOK: true, + }, + { + name: "just under the automatic floor", + status: battery(39), + manualOK: true, autoOK: false, forcedOK: true, + wantReason: ReasonPowerLow, + }, + { + name: "at the manual floor", + status: battery(20), + manualOK: true, autoOK: false, forcedOK: true, + wantReason: ReasonPowerLow, + }, + { + name: "under the manual floor", + status: battery(19), + manualOK: false, autoOK: false, forcedOK: false, + wantReason: ReasonPowerLow, + }, + { + name: "battery level cannot be read", + status: power.Status{Source: power.SourceUnknown}, + manualOK: false, autoOK: false, forcedOK: true, + wantReason: ReasonPowerUnknown, forceableOn: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + status := tt.status + newDeps := func() *GateDeps { + return &GateDeps{Power: func() power.Status { return status }} + } + + manual, err := CanApplyUpdate(t.Context(), newDeps(), ModeManual, false) + require.NoError(t, err) + manual.Release() + assert.Equal(t, tt.manualOK, manual.OK, "manual") + + auto, err := CanApplyUpdate(t.Context(), newDeps(), ModeAuto, false) + require.NoError(t, err) + auto.Release() + assert.Equal(t, tt.autoOK, auto.OK, "auto") + + forced, err := CanApplyUpdate(t.Context(), newDeps(), ModeManual, true) + require.NoError(t, err) + forced.Release() + assert.Equal(t, tt.forcedOK, forced.OK, "forced") + + if !tt.manualOK { + assert.Equal(t, tt.wantReason, manual.Reason) + assert.Equal(t, tt.forceableOn, manual.Forceable) + assert.False(t, manual.Expires, "power never waits out the deadline") + } + }) + } +} + +// A device with no way to read its power is not a device on a flat battery, so +// a platform that reports nothing is left alone. +func TestCanApplyUpdateWithoutPowerReading(t *testing.T) { + t.Parallel() + + decision, err := CanApplyUpdate(t.Context(), &GateDeps{}, ModeAuto, false) + require.NoError(t, err) + decision.Release() + assert.True(t, decision.OK) +} + +// The restore gate is held, not polled, so nothing can start a restore between +// the check and the install. +func TestCanApplyUpdateHoldsRestoreGate(t *testing.T) { + t.Parallel() + + released := false + deps := &GateDeps{ + Power: externalPower, + AcquireRestore: func() (func(), error) { + return func() { released = true }, nil + }, + } + + decision, err := CanApplyUpdate(t.Context(), deps, ModeManual, false) + require.NoError(t, err) + require.True(t, decision.OK) + assert.False(t, released, "the gate must stay held until the caller lets go") + decision.Release() + assert.True(t, released) +} + +func TestCanApplyUpdateRestoreInProgress(t *testing.T) { + t.Parallel() + + deps := &GateDeps{ + Power: externalPower, + AcquireRestore: func() (func(), error) { return nil, errors.New("restore in progress") }, + } + + decision, err := CanApplyUpdate(t.Context(), deps, ModeManual, false) + require.NoError(t, err) + decision.Release() + require.False(t, decision.OK) + assert.Equal(t, ReasonRestoreActive, decision.Reason) + assert.False(t, decision.Forceable) +} + +// The two gates are taken in the order the rest of the service takes them, and +// held together until the caller lets go. +func TestCanApplyUpdateHoldsBothGatesInOrder(t *testing.T) { + t.Parallel() + + var order []string + deps := &GateDeps{ + Power: externalPower, + AcquireRestore: func() (func(), error) { + order = append(order, "restore") + return func() { order = append(order, "release restore") }, nil + }, + AcquireMediaGate: func(context.Context) (func(), error) { + order = append(order, "media") + return func() { order = append(order, "release media") }, nil + }, + } + + decision, err := CanApplyUpdate(t.Context(), deps, ModeManual, false) + require.NoError(t, err) + require.True(t, decision.OK) + assert.Equal(t, []string{"restore", "media"}, order) + decision.Release() + assert.Equal(t, []string{"restore", "media", "release media", "release restore"}, order) +} + +// The media gate has to be held before the gate reads what is playing, or a +// launch that starts in between is one the install would kill. +func TestCanApplyUpdateReadsMediaBehindTheGate(t *testing.T) { + t.Parallel() + + gateHeld := false + playing := false + deps := &GateDeps{ + Power: externalPower, + AcquireMediaGate: func(context.Context) (func(), error) { + gateHeld = true + // Whatever was launching settles while the gate is being taken. + playing = true + return func() { gateHeld = false }, nil + }, + ActiveMedia: func() bool { return playing }, + } + + decision, err := CanApplyUpdate(t.Context(), deps, ModeManual, false) + require.NoError(t, err) + require.False(t, decision.OK) + assert.Equal(t, ReasonActiveMedia, decision.Reason) + assert.False(t, gateHeld, "a refused update must not keep launches blocked") +} + +// A cancelled request is not an answer, so it comes back as an error rather +// than as a reason a client would show someone. +func TestCanApplyUpdateMediaGateCancelled(t *testing.T) { + t.Parallel() + + restoreReleased := false + deps := &GateDeps{ + Power: externalPower, + AcquireRestore: func() (func(), error) { + return func() { restoreReleased = true }, nil + }, + AcquireMediaGate: func(context.Context) (func(), error) { + return nil, context.Canceled + }, + } + + decision, err := CanApplyUpdate(t.Context(), deps, ModeManual, false) + require.ErrorIs(t, err, context.Canceled) + assert.False(t, decision.OK) + assert.Empty(t, decision.Reason) + assert.True(t, restoreReleased) +} + +// A database that cannot answer is a problem to notice elsewhere. Refusing +// every update over it would leave the device with no way to be fixed. +func TestCanApplyUpdateStatusError(t *testing.T) { + t.Parallel() + + deps := &GateDeps{ + Power: externalPower, + IndexingStatus: func() (string, error) { + return "", errors.New("database is closed") + }, + } + + decision, err := CanApplyUpdate(t.Context(), deps, ModeManual, false) + require.NoError(t, err) + decision.Release() + assert.True(t, decision.OK) +} + +func TestPowerReady(t *testing.T) { + t.Parallel() + + flat := &GateDeps{Power: func() power.Status { return battery(5) }} + blocked := PowerReady(flat, ModeManual, true) + require.False(t, blocked.OK) + + var gateErr *GateError + require.ErrorAs(t, blocked.Err(), &gateErr) + assert.Equal(t, ReasonPowerLow, gateErr.Reason) + assert.Contains(t, gateErr.Error(), "5%") + + charged := &GateDeps{Power: externalPower} + ready := PowerReady(charged, ModeAuto, false) + assert.True(t, ready.OK) + require.NoError(t, ready.Err()) +} + +// PowerReady is the second check, run once the download is done. Everything +// else has already been decided by then and must not be asked again: a game +// launched during the download does not undo an install that is nearly +// finished. +func TestPowerReadyIgnoresOtherSignals(t *testing.T) { + t.Parallel() + + deps := &GateDeps{ + Power: externalPower, + ActiveMedia: alwaysTrue, + IndexingStatus: statusFn(mediadb.IndexingStatusRunning), + } + + assert.True(t, PowerReady(deps, ModeManual, false).OK) +} diff --git a/pkg/service/updater/install.go b/pkg/service/updater/install.go index f49bdeda7..8747e8c9d 100644 --- a/pkg/service/updater/install.go +++ b/pkg/service/updater/install.go @@ -46,8 +46,14 @@ type UpdateBackupper interface { } type installOptions struct { - UserDB UpdateBackupper + UserDB UpdateBackupper + // PreQuiesce runs once the candidate binary is in place and before the + // user database is closed for its snapshot. That is the last moment an + // install can still be called off with nothing to unwind, so it is where + // the second power check goes. + PreQuiesce func(context.Context) error Staged *StagedUpdate + progress *progressReporter TargetPath string DataDir string PreviousVersion string @@ -111,6 +117,21 @@ func installStaged(ctx context.Context, opts *installOptions) (retErr error) { return candidateErr } + // Everything up to here can be abandoned by deleting two files. From the + // snapshot on, the device is committed to either finishing or unwinding, so + // this is where a caller gets its last say. + if opts.PreQuiesce != nil { + if err := opts.PreQuiesce(ctx); err != nil { + _ = os.Remove(candidatePath) + if cleanupErr := removeStagingDir(ctx, opts.Staged.Dir); cleanupErr != nil { + log.Warn().Err(cleanupErr).Str("dir", opts.Staged.Dir). + Msg("could not remove staging after the update was called off") + } + return err + } + } + + opts.progress.stage(ProgressInstalling) snapshot, resumeUserDB, err := opts.UserDB.BackupForUpdate(opts.Staged.Version) if err != nil { _ = os.Remove(candidatePath) diff --git a/pkg/service/updater/progress.go b/pkg/service/updater/progress.go new file mode 100644 index 000000000..c97cdab77 --- /dev/null +++ b/pkg/service/updater/progress.go @@ -0,0 +1,172 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package updater + +import ( + "io" + "time" + + "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/syncutil" +) + +// ProgressStage is where an update has got to. +type ProgressStage string + +const ( + ProgressIdle ProgressStage = "idle" + ProgressChecking ProgressStage = "checking" + ProgressDownloading ProgressStage = "downloading" + ProgressVerifying ProgressStage = "verifying" + ProgressProbing ProgressStage = "probing" + ProgressInstalling ProgressStage = "installing" + ProgressRestarting ProgressStage = "restarting" + // ProgressConfirming through ProgressRolledBack happen on the boot after + // the restart, before any client has reconnected, so they reach clients + // through the last result an update check reports rather than live. + ProgressConfirming ProgressStage = "confirming" + ProgressSucceeded ProgressStage = "succeeded" + ProgressRolledBack ProgressStage = "rolledBack" + ProgressFailed ProgressStage = "failed" +) + +// progressInterval is how often a download reports its byte count. Anything +// faster is wasted on a progress bar and costs a notification round trip per +// update on hardware that has better things to do. +const progressInterval = 500 * time.Millisecond + +// Progress is one update on how an update is going. +type Progress struct { + Stage ProgressStage `json:"stage"` + Version string `json:"version,omitempty"` + Trigger string `json:"trigger,omitempty"` + Error string `json:"error,omitempty"` + BytesDownloaded int64 `json:"bytesDownloaded,omitempty"` + BytesTotal int64 `json:"bytesTotal,omitempty"` +} + +// ProgressFn receives progress updates. It is called from whichever goroutine +// is doing the work, so it must not block. +type ProgressFn func(Progress) + +// progressReporter turns stage changes and downloaded bytes into Progress +// values, filling in the version and trigger every one of them carries. A nil +// reporter, or one with no function to call, silently does nothing, so callers +// never have to check. +type progressReporter struct { + lastAt time.Time + emit ProgressFn + now func() time.Time + version string + trigger string + mu syncutil.Mutex +} + +func newProgressReporter(emit ProgressFn, trigger updateTrigger) *progressReporter { + if emit == nil { + return nil + } + return &progressReporter{emit: emit, trigger: string(trigger), now: time.Now} +} + +// setVersion records the version every later update carries. It is only known +// once the release has been selected. +func (r *progressReporter) setVersion(version string) { + if r == nil { + return + } + r.mu.Lock() + defer r.mu.Unlock() + r.version = version +} + +// stage reports a move to a new stage. +func (r *progressReporter) stage(stage ProgressStage) { + if r == nil { + return + } + r.mu.Lock() + progress := Progress{Stage: stage, Version: r.version, Trigger: r.trigger} + r.lastAt = time.Time{} + r.mu.Unlock() + r.emit(progress) +} + +// failed reports that the update stopped here. +func (r *progressReporter) failed(err error) { + if r == nil { + return + } + message := "" + if err != nil { + message = err.Error() + } + r.mu.Lock() + progress := Progress{ + Stage: ProgressFailed, + Version: r.version, + Trigger: r.trigger, + Error: message, + } + r.mu.Unlock() + r.emit(progress) +} + +// downloaded reports how far a download has got. Updates are rate limited +// except for the one that completes the transfer, which always goes out so a +// progress bar never stops short of the end. +func (r *progressReporter) downloaded(done, total int64) { + if r == nil { + return + } + r.mu.Lock() + now := r.now() + final := total > 0 && done >= total + if !final && !r.lastAt.IsZero() && now.Sub(r.lastAt) < progressInterval { + r.mu.Unlock() + return + } + r.lastAt = now + progress := Progress{ + Stage: ProgressDownloading, + Version: r.version, + Trigger: r.trigger, + BytesDownloaded: done, + BytesTotal: total, + } + r.mu.Unlock() + r.emit(progress) +} + +// progressWriter counts bytes on their way past and reports the running total. +// It sits in the download's writer chain so the count is of bytes that reached +// the file, not bytes the transport claims to have read. +type progressWriter struct { + report *progressReporter + total int64 + done int64 +} + +func (w *progressWriter) Write(p []byte) (int, error) { + w.done += int64(len(p)) + w.report.downloaded(w.done, w.total) + return len(p), nil +} + +var _ io.Writer = (*progressWriter)(nil) diff --git a/pkg/service/updater/progress_test.go b/pkg/service/updater/progress_test.go new file mode 100644 index 000000000..99f499721 --- /dev/null +++ b/pkg/service/updater/progress_test.go @@ -0,0 +1,162 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package updater + +import ( + "errors" + "testing" + "time" + + "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/syncutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// progressRecorder collects what a reporter emitted. +type progressRecorder struct { + got []Progress + mu syncutil.Mutex +} + +func (c *progressRecorder) fn() ProgressFn { + return func(progress Progress) { + c.mu.Lock() + defer c.mu.Unlock() + c.got = append(c.got, progress) + } +} + +func (c *progressRecorder) all() []Progress { + c.mu.Lock() + defer c.mu.Unlock() + return append([]Progress(nil), c.got...) +} + +// A caller that wants no progress passes no function, and every reporter call +// has to survive that without a nil check at the call site. +func TestProgressReporterWithoutFn(t *testing.T) { + t.Parallel() + + report := newProgressReporter(nil, triggerManual) + require.Nil(t, report) + + assert.NotPanics(t, func() { + report.setVersion("2.10.0") + report.stage(ProgressDownloading) + report.downloaded(1, 2) + report.failed(errors.New("boom")) + }) +} + +func TestProgressReporterStages(t *testing.T) { + t.Parallel() + + recorder := &progressRecorder{} + report := newProgressReporter(recorder.fn(), triggerManual) + + report.stage(ProgressChecking) + report.setVersion("2.10.0") + report.stage(ProgressDownloading) + report.failed(errors.New("archive digest does not match")) + + got := recorder.all() + require.Len(t, got, 3) + + assert.Equal(t, ProgressChecking, got[0].Stage) + assert.Empty(t, got[0].Version, "the version is not known until the release is picked") + assert.Equal(t, string(triggerManual), got[0].Trigger) + + assert.Equal(t, ProgressDownloading, got[1].Stage) + assert.Equal(t, "2.10.0", got[1].Version) + + assert.Equal(t, ProgressFailed, got[2].Stage) + assert.Equal(t, "2.10.0", got[2].Version) + assert.Equal(t, "archive digest does not match", got[2].Error) +} + +// A progress bar wants a steady trickle, not one message per network read, but +// it does need the one that says the transfer finished. +func TestProgressReporterThrottlesDownload(t *testing.T) { + t.Parallel() + + now := time.Date(2026, 8, 19, 12, 0, 0, 0, time.UTC) + recorder := &progressRecorder{} + report := newProgressReporter(recorder.fn(), triggerAuto) + report.now = func() time.Time { return now } + + report.downloaded(10, 100) + report.downloaded(20, 100) + report.downloaded(30, 100) + now = now.Add(progressInterval) + report.downloaded(40, 100) + now = now.Add(time.Millisecond) + report.downloaded(100, 100) + + got := recorder.all() + require.Len(t, got, 3) + assert.Equal(t, int64(10), got[0].BytesDownloaded) + assert.Equal(t, int64(40), got[1].BytesDownloaded) + assert.Equal(t, int64(100), got[2].BytesDownloaded, "the last byte always reports") + assert.Equal(t, int64(100), got[2].BytesTotal) + assert.Equal(t, string(triggerAuto), got[2].Trigger) +} + +// A stage change resets the throttle so the first bytes of a download are +// reported straight away rather than half a second in. +func TestProgressReporterStageResetsThrottle(t *testing.T) { + t.Parallel() + + now := time.Date(2026, 8, 19, 12, 0, 0, 0, time.UTC) + recorder := &progressRecorder{} + report := newProgressReporter(recorder.fn(), triggerManual) + report.now = func() time.Time { return now } + + report.downloaded(10, 100) + report.stage(ProgressVerifying) + report.downloaded(20, 100) + + got := recorder.all() + require.Len(t, got, 3) + assert.Equal(t, int64(20), got[2].BytesDownloaded) +} + +func TestProgressWriterCountsBytes(t *testing.T) { + t.Parallel() + + now := time.Date(2026, 8, 19, 12, 0, 0, 0, time.UTC) + recorder := &progressRecorder{} + report := newProgressReporter(recorder.fn(), triggerManual) + report.now = func() time.Time { return now } + + writer := &progressWriter{report: report, total: 6} + written, err := writer.Write([]byte("abc")) + require.NoError(t, err) + assert.Equal(t, 3, written) + + written, err = writer.Write([]byte("def")) + require.NoError(t, err) + assert.Equal(t, 3, written) + + got := recorder.all() + require.Len(t, got, 2) + assert.Equal(t, int64(3), got[0].BytesDownloaded) + assert.Equal(t, int64(6), got[1].BytesDownloaded) + assert.Equal(t, int64(6), got[1].BytesTotal) +} diff --git a/pkg/service/updater/rollout.go b/pkg/service/updater/rollout.go new file mode 100644 index 000000000..6754027cd --- /dev/null +++ b/pkg/service/updater/rollout.go @@ -0,0 +1,52 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package updater + +import ( + "crypto/sha256" + "encoding/binary" +) + +// rolloutBucket places a device in one of a hundred buckets for a release. +// +// The release tag is part of the hash so that widening a rollout from 10% to +// 25% keeps everyone who already has it, while a different release draws an +// unrelated set. Without that, the same unlucky devices would be first every +// single time. +func rolloutBucket(deviceID, releaseTag string) int { + sum := sha256.Sum256([]byte(deviceID + "\x00" + releaseTag)) + return int(binary.BigEndian.Uint32(sum[:4]) % 100) +} + +// RolloutEligible reports whether this device is inside a release's staged +// rollout yet. +// +// A device with no ID only takes releases that have gone out to everyone: an +// unidentified device has no stable bucket, and treating it as bucket 0 would +// quietly make every such device part of the first wave. +func RolloutEligible(deviceID, releaseTag string, rollout int) bool { + if rollout >= 100 { + return true + } + if rollout <= 0 || deviceID == "" { + return false + } + return rolloutBucket(deviceID, releaseTag) < rollout +} diff --git a/pkg/service/updater/rollout_test.go b/pkg/service/updater/rollout_test.go new file mode 100644 index 000000000..620b64640 --- /dev/null +++ b/pkg/service/updater/rollout_test.go @@ -0,0 +1,121 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package updater + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRolloutEligible(t *testing.T) { + t.Parallel() + + const id = "6f1c9a4e-4f1a-4a6e-9f1d-2b3c4d5e6f70" + + tests := []struct { + name string + deviceID string + rollout int + want bool + }{ + {name: "fully released", deviceID: id, rollout: 100, want: true}, + {name: "over 100 is still released", deviceID: id, rollout: 150, want: true}, + {name: "not started", deviceID: id, rollout: 0, want: false}, + {name: "negative is not started", deviceID: id, rollout: -5, want: false}, + { + name: "an unidentified device only takes a full release", + deviceID: "", rollout: 99, want: false, + }, + { + name: "an unidentified device still takes a full release", + deviceID: "", rollout: 100, want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, RolloutEligible(tt.deviceID, "v2.10.0", tt.rollout)) + }) + } +} + +// A rollout being widened must keep the devices it already reached, or the +// first cohort would be swapped for a fresh untested one every time. +func TestRolloutWideningKeepsEarlierDevices(t *testing.T) { + t.Parallel() + + const tag = "v2.10.0" + for i := range 500 { + id := fmt.Sprintf("device-%d", i) + if !RolloutEligible(id, tag, 10) { + continue + } + assert.True(t, RolloutEligible(id, tag, 25), + "%s was in the 10%% cohort and must stay in the 25%% one", id) + } +} + +// The release is part of the hash so no device is permanently the one that +// gets every update first. +func TestRolloutBucketVariesByRelease(t *testing.T) { + t.Parallel() + + const id = "6f1c9a4e-4f1a-4a6e-9f1d-2b3c4d5e6f70" + first := rolloutBucket(id, "v2.10.0") + second := rolloutBucket(id, "v2.11.0") + assert.NotEqual(t, first, second) +} + +func TestRolloutBucketIsStable(t *testing.T) { + t.Parallel() + + const id = "6f1c9a4e-4f1a-4a6e-9f1d-2b3c4d5e6f70" + bucket := rolloutBucket(id, "v2.10.0") + assert.Equal(t, bucket, rolloutBucket(id, "v2.10.0")) + assert.GreaterOrEqual(t, bucket, 0) + assert.Less(t, bucket, 100) +} + +// A percentage is only worth publishing if it means roughly that share of the +// fleet, so check the spread over a realistic number of devices. +func TestRolloutSpread(t *testing.T) { + t.Parallel() + + const ( + devices = 5000 + tag = "v2.10.0" + rollout = 25 + ) + + eligible := 0 + for i := range devices { + if RolloutEligible(fmt.Sprintf("device-%d", i), tag, rollout) { + eligible++ + } + } + + share := float64(eligible) / float64(devices) * 100 + require.InDelta(t, float64(rollout), share, 3.0, + "a %d%% rollout reached %.1f%% of devices", rollout, share) +} diff --git a/pkg/service/updater/stage.go b/pkg/service/updater/stage.go index b2ec6af70..1e47b29c8 100644 --- a/pkg/service/updater/stage.go +++ b/pkg/service/updater/stage.go @@ -165,24 +165,13 @@ var ( // StageOptions describes one staging attempt. type StageOptions struct { - // Release comes from a manifest whose signature has already been checked. - // The archive and the version are re-derived from it here rather than - // trusting anything passed alongside it. - Release *otameta.Release - // PlatformID is the platform half of the archive name. - PlatformID string - // Arch and OS default to this build's. They are settable so the selection - // and the archive member rules can be tested for platforms other than the - // one running the test. - Arch string - OS string - // TargetPath is the binary that will eventually be replaced. Only its base - // name is read here: it names the archive member to pull out, and the name - // the staged copy is written under. - TargetPath string - // StagingRoot holds one directory per staged version. - StagingRoot string - // CurrentVersion is the version running now, which the release has to beat. + Release *otameta.Release + progress *progressReporter + PlatformID string + Arch string + OS string + TargetPath string + StagingRoot string CurrentVersion string } @@ -234,6 +223,7 @@ func (b *cappedBuilder) String() string { type stager struct { fetch assetFetcher release *otameta.Release + progress *progressReporter chmod func(string, os.FileMode) error runProbe probeFn goos string @@ -326,6 +316,7 @@ func newStager(opts *StageOptions, fetch assetFetcher) (*stager, error) { s := &stager{ fetch: fetch, release: opts.Release, + progress: opts.progress, platformID: opts.PlatformID, goos: opts.OS, goarch: opts.Arch, @@ -354,6 +345,7 @@ func (s *stager) run(ctx context.Context) (*StagedUpdate, error) { if err != nil { return nil, err } + s.progress.setVersion(version) // The version has already been through semver parsing, which admits only // digits, dots, hyphens and alphanumerics, so it cannot name anything but a @@ -469,6 +461,7 @@ func (s *stager) stageInto( // rather than the one the manifest gives it, so no metadata string reaches // the filesystem even though selection has already constrained it. archivePath := filepath.Join(dir, otameta.ArchiveBaseName(s.platformID, s.goarch, version)+ext) + s.progress.stage(ProgressDownloading) if err := s.downloadArchive(ctx, asset, archivePath); err != nil { return nil, err } @@ -480,6 +473,7 @@ func (s *stager) stageInto( } binaryPath := filepath.Join(payloadDir, s.binaryName) + s.progress.stage(ProgressVerifying) if err := s.extractBinary(ctx, archivePath, ext, wantDigest, binaryPath); err != nil { return nil, err } @@ -511,6 +505,7 @@ func (s *stager) stageInto( Msg("could not set the exec bit on the staged binary; leaving it to the probe") } + s.progress.stage(ProgressProbing) if err := s.probeBinary(ctx, binaryPath, version); err != nil { return nil, err } @@ -609,7 +604,11 @@ func (s *stager) downloadArchive(ctx context.Context, asset *otameta.Asset, dest // accepted and a longer one is detected rather than silently truncated into // something that would then fail the digest for the wrong reason. digest := sha256.New() - written, copyErr := io.Copy(io.MultiWriter(f, digest), guard.reader(io.LimitReader(body, asset.Size+1))) + counter := &progressWriter{report: s.progress, total: asset.Size} + written, copyErr := io.Copy( + io.MultiWriter(f, digest, counter), + guard.reader(io.LimitReader(body, asset.Size+1)), + ) syncErr := f.Sync() closeErr := f.Close() diff --git a/pkg/service/updater/state.go b/pkg/service/updater/state.go index 877f30206..37b57090f 100644 --- a/pkg/service/updater/state.go +++ b/pkg/service/updater/state.go @@ -53,12 +53,23 @@ var stateMu syncutil.Mutex // would roll backwards whenever an old backup was restored, which is a // self-inflicted downgrade window. type updaterState struct { - ManifestSeenAt time.Time `json:"manifestSeenAt"` - LastResult *updateResult `json:"lastResult,omitempty"` - ManifestETag string `json:"manifestETag"` - ManifestLastModified string `json:"manifestLastModified"` - ManifestGeneration int64 `json:"manifestGeneration"` - StateVersion int `json:"stateVersion"` + ManifestSeenAt time.Time `json:"manifestSeenAt"` + LastResult *updateResult `json:"lastResult,omitempty"` + Deferral *updateDeferral `json:"deferral,omitempty"` + ManifestETag string `json:"manifestETag"` + ManifestLastModified string `json:"manifestLastModified"` + ManifestGeneration int64 `json:"manifestGeneration"` + StateVersion int `json:"stateVersion"` +} + +// updateDeferral records that an automatic install has been putting a version +// off, and since when. It is what lets a check say the device is waiting for a +// quiet moment instead of leaving it looking stalled, and it is what the +// 24-hour deadline is measured from. +type updateDeferral struct { + Since time.Time `json:"since"` + Version string `json:"version"` + Reason string `json:"reason"` } // updateResult records the terminal outcome of an update for the boot that @@ -301,3 +312,74 @@ func sameUpdateResult(a, b *updateResult) bool { a.ToVersion == b.ToVersion && a.Detail == b.Detail } + +// recordDeferral notes that an automatic install of version was put off for +// reason. The start time survives repeated deferrals of the same version, since +// that is what the deadline is measured from; a different version starts the +// clock again. +func recordDeferral(dir, version, reason string) error { + stateMu.Lock() + defer stateMu.Unlock() + + st, err := loadStateWithError(dir) + if err != nil { + return fmt.Errorf("loading updater state before recording a deferral: %w", err) + } + if st.Deferral != nil && st.Deferral.Version == version && st.Deferral.Reason == reason { + return nil + } + + since := time.Now().UTC() + if st.Deferral != nil && st.Deferral.Version == version && !st.Deferral.Since.IsZero() { + since = st.Deferral.Since + } + st.Deferral = &updateDeferral{Since: since, Version: version, Reason: reason} + if err := saveState(dir, &st); err != nil { + return fmt.Errorf("recording the update deferral: %w", err) + } + return nil +} + +// clearDeferral forgets a deferral when expectedVersion is empty or still +// matches the stored version. The match prevents stale checks from clearing a +// newer deferral recorded concurrently. +func clearDeferral(dir, expectedVersion string) error { + stateMu.Lock() + defer stateMu.Unlock() + + st, err := loadStateWithError(dir) + if err != nil { + return fmt.Errorf("loading updater state before clearing a deferral: %w", err) + } + if st.Deferral == nil || (expectedVersion != "" && st.Deferral.Version != expectedVersion) { + return nil + } + st.Deferral = nil + if err := saveState(dir, &st); err != nil { + return fmt.Errorf("clearing the update deferral: %w", err) + } + return nil +} + +// peekDeferralState returns any recorded deferral without changing it. +func peekDeferralState(dir string) *updateDeferral { + stateMu.Lock() + defer stateMu.Unlock() + + st := loadState(dir) + if st.Deferral == nil { + return nil + } + deferral := *st.Deferral + return &deferral +} + +// peekDeferral returns the recorded deferral for version, or nil when the +// device is not waiting on that version. +func peekDeferral(dir, version string) *updateDeferral { + deferral := peekDeferralState(dir) + if deferral == nil || deferral.Version != version { + return nil + } + return deferral +} diff --git a/pkg/service/updater/state_test.go b/pkg/service/updater/state_test.go index 5278b357b..f74d963dd 100644 --- a/pkg/service/updater/state_test.go +++ b/pkg/service/updater/state_test.go @@ -286,3 +286,127 @@ func TestUpdateResult_NilIsNotRecorded(t *testing.T) { assert.Nil(t, peekUpdateResult(dir)) assert.NoFileExists(t, filepath.Join(dir, stateFileName)) } + +func TestRecordDeferral_KeepsSinceAcrossReasons(t *testing.T) { + t.Parallel() + + dir := filepath.Join(t.TempDir(), "updater") + require.NoError(t, recordDeferral(dir, "v2.5.0", ReasonActiveMedia)) + + first := peekDeferral(dir, "v2.5.0") + require.NotNil(t, first) + assert.Equal(t, ReasonActiveMedia, first.Reason) + assert.False(t, first.Since.IsZero()) + assert.Equal(t, time.UTC, first.Since.Location()) + + // The clock an automatic install runs down starts at the first deferral of + // a version, not at the most recent one, so someone who keeps the device + // busy cannot push it back forever. + require.NoError(t, recordDeferral(dir, "v2.5.0", ReasonAPIBusy)) + second := peekDeferral(dir, "v2.5.0") + require.NotNil(t, second) + assert.Equal(t, ReasonAPIBusy, second.Reason) + assert.Equal(t, first.Since, second.Since) +} + +func TestRecordDeferral_ReusesExistingNonZeroTimestamp(t *testing.T) { + t.Parallel() + + dir := filepath.Join(t.TempDir(), "updater") + prior := time.Date(2026, 8, 1, 12, 30, 0, 0, time.FixedZone("prior", 9*60*60)) + require.NoError(t, saveState(dir, &updaterState{Deferral: &updateDeferral{ + Since: prior, Version: "v2.5.0", Reason: ReasonActiveMedia, + }})) + + require.NoError(t, recordDeferral(dir, "v2.5.0", ReasonAPIBusy)) + deferral := peekDeferral(dir, "v2.5.0") + require.NotNil(t, deferral) + assert.True(t, prior.Equal(deferral.Since)) + assert.Equal(t, ReasonAPIBusy, deferral.Reason) +} + +func TestRecordDeferral_SameReasonDoesNotRewrite(t *testing.T) { + t.Parallel() + + dir := filepath.Join(t.TempDir(), "updater") + require.NoError(t, recordDeferral(dir, "v2.5.0", ReasonActiveMedia)) + first := peekDeferral(dir, "v2.5.0") + require.NotNil(t, first) + + require.NoError(t, recordDeferral(dir, "v2.5.0", ReasonActiveMedia)) + second := peekDeferral(dir, "v2.5.0") + require.NotNil(t, second) + assert.Equal(t, first.Since, second.Since) + assert.Equal(t, first.Reason, second.Reason) +} + +func TestRecordDeferral_NewVersionRestartsTheClock(t *testing.T) { + t.Parallel() + + dir := filepath.Join(t.TempDir(), "updater") + require.NoError(t, recordDeferral(dir, "v2.5.0", ReasonActiveMedia)) + old := peekDeferral(dir, "v2.5.0") + require.NotNil(t, old) + + // Backdate the deferral so a fresh timestamp is distinguishable without + // waiting for the wall clock to move. + stale := old.Since.Add(-48 * time.Hour) + st := loadState(dir) + st.Deferral.Since = stale + require.NoError(t, saveState(dir, &st)) + + require.NoError(t, recordDeferral(dir, "v2.6.0", ReasonActiveMedia)) + assert.Nil(t, peekDeferral(dir, "v2.5.0")) + fresh := peekDeferral(dir, "v2.6.0") + require.NotNil(t, fresh) + assert.True(t, fresh.Since.After(stale), "a different version waits from scratch") +} + +func TestPeekDeferral_OtherVersionOrNothingRecorded(t *testing.T) { + t.Parallel() + + dir := filepath.Join(t.TempDir(), "updater") + assert.Nil(t, peekDeferral(dir, "v2.5.0")) + + require.NoError(t, recordDeferral(dir, "v2.5.0", ReasonActiveMedia)) + assert.Nil(t, peekDeferral(dir, "v2.6.0")) +} + +func TestClearDeferral(t *testing.T) { + t.Parallel() + + dir := filepath.Join(t.TempDir(), "updater") + require.NoError(t, recordDeferral(dir, "v2.5.0", ReasonActiveMedia)) + require.NotNil(t, peekDeferral(dir, "v2.5.0")) + + require.NoError(t, clearDeferral(dir, "")) + assert.Nil(t, peekDeferral(dir, "v2.5.0")) + + // Clearing again is not an error, because an update that was never held up + // still clears the deferral when it installs. + require.NoError(t, clearDeferral(dir, "")) + + require.NoError(t, recordDeferral(dir, "v2.6.0", ReasonActiveMedia)) + require.NoError(t, clearDeferral(dir, "v2.5.0")) + require.NotNil(t, peekDeferral(dir, "v2.6.0"), "stale cleanup must not clear a replacement deferral") +} + +func TestRecordDeferral_KeepsOtherState(t *testing.T) { + t.Parallel() + + dir := filepath.Join(t.TempDir(), "updater") + seen := time.Date(2026, 8, 17, 2, 0, 0, 0, time.UTC) + require.NoError(t, saveState(dir, &updaterState{ + ManifestSeenAt: seen, + ManifestETag: "etag-1", + ManifestGeneration: 7, + })) + + require.NoError(t, recordDeferral(dir, "v2.5.0", ReasonActiveMedia)) + require.NoError(t, clearDeferral(dir, "")) + + st := loadState(dir) + assert.Equal(t, "etag-1", st.ManifestETag) + assert.Equal(t, int64(7), st.ManifestGeneration) + assert.True(t, seen.Equal(st.ManifestSeenAt)) +} diff --git a/pkg/service/updater/updater.go b/pkg/service/updater/updater.go index bc7d5dd30..33f4daaaa 100644 --- a/pkg/service/updater/updater.go +++ b/pkg/service/updater/updater.go @@ -27,10 +27,12 @@ import ( "regexp" "runtime" "sync/atomic" + "time" "github.com/ZaparooProject/zaparoo-core/v2/pkg/config" "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/syncutil" "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/tlsroots" + platformids "github.com/ZaparooProject/zaparoo-core/v2/pkg/platforms/ids" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/inbox" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/restart" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/updater/otameta" @@ -47,19 +49,76 @@ var ( applyInProgress atomic.Bool ) +// Eligibility says whether this device can take an OTA update at all, ahead of +// any question about whether one is available. +const ( + // EligibilityEligible means OTA updates work here. + EligibilityEligible = "eligible" + // EligibilityDevelopment means this is a build from source, which has no + // release to compare itself against. + EligibilityDevelopment = "development" + // EligibilityManaged means a package manager owns this install and should + // be the one updating it. + EligibilityManaged = "managed" + // EligibilityUnsupported means OTA updates are not available on this + // operating system yet. + EligibilityUnsupported = "unsupported" +) + // Options describes the device an update is being resolved for. type Options struct { - UserDB UpdateBackupper + UserDB UpdateBackupper + // Progress is called as the update moves through its stages, when the + // caller wants to follow along. Nil reports nothing. + Progress ProgressFn + // PreQuiesce runs at the last moment an install can still be called off. + // The second power check goes here. + PreQuiesce func(context.Context) error + // Gate is what the device is busy with. A check uses it to report what + // would stop an update going ahead right now; nil reports nothing. + Gate *GateDeps PlatformID string Channel string DataDir string + // DeviceID is this device's identifier, used to work out whether a staged + // rollout has reached it yet. Empty means only fully released versions + // count as rolled out. + DeviceID string + // Mode is who asked. It decides how the install is recorded and, for + // automatic installs, that nothing may be forced. + Mode Mode + // Managed says a package manager owns this install, which the check + // reports as the reason OTA updates do not apply here. + Managed bool +} + +// OutcomeReport is how the last update finished, for a client that was not +// connected when it happened. The confirm and rollback stages run on the boot +// after the restart, before any client is back, so this is the only way they +// are ever seen. +type OutcomeReport struct { + At time.Time `json:"at"` + Outcome string `json:"outcome"` + FromVersion string `json:"fromVersion,omitempty"` + ToVersion string `json:"toVersion,omitempty"` + Detail string `json:"detail,omitempty"` } type Result struct { - CurrentVersion string - LatestVersion string - ReleaseNotes string - UpdateAvailable bool + CheckedAt time.Time + DeferredSince time.Time + LastResult *OutcomeReport + Eligibility string + ReleaseNotes string + Channel string + LatestVersion string + DeferredReason string + BlockedReason string + BlockedMessage string + CurrentVersion string + UpdateAvailable bool + RolloutHeld bool + BlockedForceable bool } // session is one update operation's updater and the transport backing it. @@ -74,7 +133,7 @@ type session struct { repo selfupdate.Repository } -func makeUpdater(opts Options) (*session, error) { +func makeUpdater(opts Options) (*session, error) { //nolint:gocritic // hugeParam // tlsroots hands back a transport this updater owns outright, so setting the // header timeout here does not affect anything else in the process. transport := tlsroots.Transport(nil) @@ -119,7 +178,7 @@ func assetFilter(platformID, goarch string) string { return fmt.Sprintf("^zaparoo-%s_%s-", regexp.QuoteMeta(platformID), regexp.QuoteMeta(goarch)) } -func Check(ctx context.Context, opts Options) (*Result, error) { +func Check(ctx context.Context, opts Options) (*Result, error) { //nolint:gocritic // hugeParam if config.IsDevelopmentVersion() { return nil, ErrDevelopmentVersion } @@ -135,20 +194,147 @@ func Check(ctx context.Context, opts Options) (*Result, error) { return nil, fmt.Errorf("detecting latest release: %w", err) } + stateDir := stateDirFor(opts.DataDir) result := &Result{ CurrentVersion: config.AppVersion, + Channel: opts.Channel, + Eligibility: eligibilityFor(&opts), + CheckedAt: time.Now().UTC(), + LastResult: lastOutcome(stateDir), } if found { result.LatestVersion = release.Version() + if err := clearDeferralForRelease(stateDir, result.LatestVersion); err != nil { + log.Warn().Err(err).Msg("could not clear deferral for a superseded update") + } result.UpdateAvailable = release.GreaterThan(config.AppVersion) result.ReleaseNotes = release.ReleaseNotes } + if result.UpdateAvailable { + result.RolloutHeld = rolloutHeld(s.source, opts.DeviceID, release.Version()) + noteGate(ctx, &opts, result, stateDir, release.Version()) + if deferral := peekDeferral(stateDir, release.Version()); deferral != nil { + result.DeferredReason = deferral.Reason + result.DeferredSince = deferral.Since + } + } return result, nil } -func Apply(ctx context.Context, opts Options) (string, error) { +func clearDeferralForRelease(stateDir, version string) error { + deferral := peekDeferralState(stateDir) + if deferral == nil || deferral.Version == version { + return nil + } + return clearDeferral(stateDir, deferral.Version) +} + +// noteGate records what is currently in the way of installing a version. A +// signal that will pass on its own — someone playing a game, a busy API — also +// starts the clock an automatic install eventually runs out of patience with, +// so it is written to disk; the rest is only reported. +// +// This only ever reports, so the gate is asked with nothing to acquire: a check +// must not stop the user launching something while it answers. +func noteGate(ctx context.Context, opts *Options, result *Result, stateDir, version string) { + if opts.Gate == nil { + return + } + reporting := *opts.Gate + reporting.AcquireRestore = nil + reporting.AcquireMediaGate = nil + decision, err := CanApplyUpdate(ctx, &reporting, ModeManual, false) + if err != nil { + log.Warn().Err(err).Msg("could not read what is in the way of an update") + return + } + decision.Release() + if decision.OK { + return + } + result.BlockedReason = decision.Reason + result.BlockedMessage = decision.Message + result.BlockedForceable = decision.Forceable + if !decision.Expires { + return + } + if err := recordDeferral(stateDir, version, decision.Reason); err != nil { + log.Warn().Err(err).Msg("could not record why an update is waiting") + } +} + +// installAdvice says how this device gets a new release. A package manager +// reconciles the files it owns against its own index, so an update installed +// behind its back is undone the next time it runs — pointing someone at an +// update button there would waste their time and confuse them when the version +// went backwards. +func installAdvice(platformID string, managed bool) string { + if !managed { + return "Use the App or TUI to update." + } + switch platformID { + case platformids.Mister: + return "Run update_all to install it." + case platformids.Batocera: + return "Install it through the Batocera package manager." + default: + return "Your package manager installs updates on this device." + } +} + +// eligibilityFor says whether OTA updates apply to this install at all. +func eligibilityFor(opts *Options) string { + switch { + case config.IsDevelopmentVersion(): + return EligibilityDevelopment + case preflightPlatform(runtime.GOOS) != nil: + // Checked before Managed because this one is a refusal Apply enforces, + // while Managed only says the package manager should be doing it. + return EligibilityUnsupported + case opts.Managed: + return EligibilityManaged + default: + return EligibilityEligible + } +} + +// rolloutHeld reports whether a staged rollout has not reached this device yet. +// A release the manifest cannot be re-read for is treated as reached: the +// rollout decides when to offer an update automatically, and failing closed +// there would silently strand devices on a manifest quirk. +func rolloutHeld(source *verifiedSource, deviceID, version string) bool { + release, err := source.releaseForVersion(version) + if err != nil { + log.Debug().Err(err).Str("version", version). + Msg("could not read the rollout for a release") + return false + } + return !RolloutEligible(deviceID, release.TagName, release.Rollout) +} + +// lastOutcome reports how the previous update finished, whether or not it has +// already been shown. A client asking now was not necessarily the client that +// saw it the first time. +func lastOutcome(dir string) *OutcomeReport { + stateMu.Lock() + defer stateMu.Unlock() + + st := loadState(dir) + if st.LastResult == nil { + return nil + } + return &OutcomeReport{ + At: st.LastResult.At, + Outcome: string(st.LastResult.Outcome), + FromVersion: st.LastResult.FromVersion, + ToVersion: st.LastResult.ToVersion, + Detail: st.LastResult.Detail, + } +} + +func Apply(ctx context.Context, opts Options) (string, error) { //nolint:gocritic // hugeParam if config.IsDevelopmentVersion() { return "", ErrDevelopmentVersion } @@ -167,43 +353,57 @@ func Apply(ctx context.Context, opts Options) (string, error) { applyMu.Lock() defer applyMu.Unlock() - if err := ensureNoPendingUpdate(opts.DataDir); err != nil { + trigger := triggerManual + if opts.Mode == ModeAuto { + trigger = triggerAuto + } + report := newProgressReporter(opts.Progress, trigger) + // Anything that stops the update from here on is worth telling the client + // about, whatever stage it happened at. + fail := func(err error) (string, error) { + report.failed(err) return "", err } + if err := ensureNoPendingUpdate(opts.DataDir); err != nil { + return fail(err) + } + s, err := makeUpdater(opts) if err != nil { - return "", err + return fail(err) } defer s.close() + report.stage(ProgressChecking) release, found, err := s.updater.DetectLatest(ctx, s.repo) if err != nil { - return "", fmt.Errorf("detecting the release to apply: %w", err) + return fail(fmt.Errorf("detecting the release to apply: %w", err)) } if !found || !release.GreaterThan(config.AppVersion) { - return "", fmt.Errorf("%w: running %s", ErrNotAnUpgrade, config.AppVersion) + return fail(fmt.Errorf("%w: running %s", ErrNotAnUpgrade, config.AppVersion)) } + report.setVersion(release.Version()) // Checked here rather than at the top of Apply so that a device already on // the newest version is told that, instead of being told its platform is // unsupported. Everything below this point costs the user something the // install can never spend well. if platformErr := preflightPlatform(runtime.GOOS); platformErr != nil { - return "", platformErr + return fail(platformErr) } manifestRelease, err := s.source.releaseForVersion(release.Version()) if err != nil { - return "", err + return fail(err) } targetPath, err := restart.BinaryPath() if err != nil { - return "", fmt.Errorf("resolving the binary to update: %w", err) + return fail(fmt.Errorf("resolving the binary to update: %w", err)) } stagingRoot := stagingRootFor(opts.DataDir) if spaceErr := preflightSpace(&opts, manifestRelease, targetPath, stagingRoot); spaceErr != nil { - return "", spaceErr + return fail(spaceErr) } staged, err := Stage(ctx, &StageOptions{ Release: manifestRelease, @@ -213,9 +413,10 @@ func Apply(ctx context.Context, opts Options) (string, error) { TargetPath: targetPath, StagingRoot: stagingRoot, CurrentVersion: config.AppVersion, + progress: report, }) if err != nil { - return "", fmt.Errorf("staging update: %w", err) + return fail(fmt.Errorf("staging update: %w", err)) } if err := installStaged(ctx, &installOptions{ @@ -226,11 +427,20 @@ func Apply(ctx context.Context, opts Options) (string, error) { PreviousVersion: config.AppVersion, PlatformID: opts.PlatformID, ManifestGeneration: s.source.manifestGeneration(), - Trigger: triggerManual, + Trigger: trigger, + PreQuiesce: opts.PreQuiesce, + progress: report, }); err != nil { - return "", fmt.Errorf("installing update: %w", err) + return fail(fmt.Errorf("installing update: %w", err)) } + // The version on offer has been taken, so nothing is waiting on it any + // more. A failure to say so is not worth failing an installed update over. + if err := clearDeferral(stateDirFor(opts.DataDir), staged.Version); err != nil { + log.Warn().Err(err).Msg("could not clear the recorded update deferral") + } + + report.stage(ProgressRestarting) return staged.Version, nil } @@ -273,14 +483,14 @@ type CheckFn func(ctx context.Context, opts Options) (*Result, error) func CheckAndNotify( ctx context.Context, cfg *config.Instance, - opts Options, + opts Options, //nolint:gocritic // hugeParam inboxSvc *inbox.Service, waitFn func(context.Context, int) bool, checkFn CheckFn, managedInstall bool, ) { - if !cfg.AutoUpdate(!managedInstall) { - log.Debug().Msg("auto-update disabled, skipping update check") + if !cfg.UpdateCheck() { + log.Debug().Msg("update checking is off, skipping update check") return } @@ -293,6 +503,8 @@ func CheckAndNotify( } opts.Channel = cfg.UpdateChannel() + opts.DeviceID = cfg.DeviceID() + opts.Managed = managedInstall result, err := checkFn(ctx, opts) if errors.Is(err, ErrDevelopmentVersion) { log.Debug().Msg("development version, skipping update check") @@ -313,6 +525,16 @@ func CheckAndNotify( if ctx.Err() != nil { return } + // A release still rolling out has not reached this device. Announcing it + // anyway would have everyone install on the first day, which is the one + // thing a staged rollout exists to prevent. Asking for it by hand still + // works, and update.check says why it is being held back. + if result.RolloutHeld { + log.Debug(). + Str("latest", result.LatestVersion). + Msg("update is not rolled out to this device yet") + return + } log.Info(). Str("current", result.CurrentVersion). @@ -321,8 +543,9 @@ func CheckAndNotify( title := fmt.Sprintf("Zaparoo %s is available", result.LatestVersion) body := fmt.Sprintf( - "Currently on %s. Use the App or TUI to update.", + "Currently on %s. %s", result.CurrentVersion, + installAdvice(opts.PlatformID, managedInstall), ) if err := inboxSvc.Add( diff --git a/pkg/service/updater/updater_test.go b/pkg/service/updater/updater_test.go index 736a8416a..01b57f493 100644 --- a/pkg/service/updater/updater_test.go +++ b/pkg/service/updater/updater_test.go @@ -25,12 +25,17 @@ import ( "io" "net/http" "net/http/httptest" + "path/filepath" "regexp" + "strconv" + "strings" "testing" "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models" "github.com/ZaparooProject/zaparoo-core/v2/pkg/config" "github.com/ZaparooProject/zaparoo-core/v2/pkg/database" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/database/mediadb" + platformids "github.com/ZaparooProject/zaparoo-core/v2/pkg/platforms/ids" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/inbox" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/updater/otameta" "github.com/ZaparooProject/zaparoo-core/v2/pkg/testing/helpers" @@ -78,25 +83,56 @@ func TestApply_DevelopmentVersion(t *testing.T) { func alwaysOnline(_ context.Context, _ int) bool { return true } -func TestCheckAndNotify_ManagedInstallDefaultsOff(t *testing.T) { +// A package manager owning the install is a reason not to install, not a +// reason not to look. Someone whose package manager is lagging behind has no +// other way to find out. +func TestCheckAndNotify_ManagedInstallStillChecks(t *testing.T) { t.Parallel() - cfg := &config.Instance{} // AutoUpdate is nil + cfg := &config.Instance{} // Updates.Check is nil waitCalled := false CheckAndNotify(t.Context(), cfg, linuxOptions(), nil, func(_ context.Context, _ int) bool { waitCalled = true - return true + return false }, Check, true) - assert.False(t, waitCalled) + assert.True(t, waitCalled) +} + +func TestInstallAdvice(t *testing.T) { + t.Parallel() + + tests := []struct { + platformID string + want string + managed bool + }{ + {platformID: platformids.Mister, managed: false, want: "Use the App or TUI to update."}, + {platformID: platformids.Mister, managed: true, want: "Run update_all to install it."}, + { + platformID: platformids.Batocera, managed: true, + want: "Install it through the Batocera package manager.", + }, + { + platformID: platformids.Linux, managed: true, + want: "Your package manager installs updates on this device.", + }, + } + + for _, tt := range tests { + t.Run(tt.platformID+"/"+strconv.FormatBool(tt.managed), func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, installAdvice(tt.platformID, tt.managed)) + }) + } } func TestCheckAndNotify_DisabledConfig(t *testing.T) { t.Parallel() cfg := &config.Instance{} - cfg.SetAutoUpdate(false) + cfg.SetUpdateCheck(false) waitCalled := false CheckAndNotify(t.Context(), cfg, linuxOptions(), nil, func(_ context.Context, _ int) bool { @@ -113,7 +149,7 @@ func TestCheckAndNotify_DevelopmentVersion(t *testing.T) { t.Cleanup(func() { config.AppVersion = original }) cfg := &config.Instance{} - cfg.SetAutoUpdate(true) + cfg.SetUpdateCheck(true) CheckAndNotify(t.Context(), cfg, linuxOptions(), nil, alwaysOnline, Check, false) } @@ -122,7 +158,7 @@ func TestCheckAndNotify_NoInternet(t *testing.T) { t.Parallel() cfg := &config.Instance{} - cfg.SetAutoUpdate(true) + cfg.SetUpdateCheck(true) CheckAndNotify(t.Context(), cfg, linuxOptions(), nil, func(_ context.Context, _ int) bool { return false @@ -133,7 +169,7 @@ func TestCheckAndNotify_UpdateAvailable(t *testing.T) { t.Parallel() cfg := &config.Instance{} - cfg.SetAutoUpdate(true) + cfg.SetUpdateCheck(true) mockUserDB := helpers.NewMockUserDBI() mockUserDB.On("AddInboxMessage", mock.MatchedBy(func(msg *database.InboxMessage) bool { @@ -158,11 +194,41 @@ func TestCheckAndNotify_UpdateAvailable(t *testing.T) { mockUserDB.AssertExpectations(t) } +// A package-managed device still gets told a release exists, so the message has +// to point at the thing that actually installs it there. +func TestCheckAndNotify_ManagedInstallBodyNamesThePackageManager(t *testing.T) { + t.Parallel() + + cfg := &config.Instance{} + cfg.SetUpdateCheck(true) + + mockUserDB := helpers.NewMockUserDBI() + mockUserDB.On("AddInboxMessage", mock.MatchedBy(func(msg *database.InboxMessage) bool { + return strings.Contains(msg.Body, "Run update_all to install it.") + })).Return(&database.InboxMessage{DBID: 1}, nil) + + ns := make(chan models.Notification, 10) + inboxSvc := inbox.NewService(mockUserDB, ns) + + checkFn := func(_ context.Context, _ Options) (*Result, error) { + return &Result{ + CurrentVersion: "2.9.0", + LatestVersion: "2.10.0", + UpdateAvailable: true, + }, nil + } + + opts := Options{PlatformID: platformids.Mister, Channel: config.UpdateChannelStable} + CheckAndNotify(t.Context(), cfg, opts, inboxSvc, alwaysOnline, checkFn, true) + + mockUserDB.AssertExpectations(t) +} + func TestCheckAndNotify_BetaChannel(t *testing.T) { t.Parallel() cfg := &config.Instance{} - cfg.SetAutoUpdate(true) + cfg.SetUpdateCheck(true) cfg.SetUpdateChannel(config.UpdateChannelBeta) var receivedChannel string @@ -186,7 +252,7 @@ func TestCheckAndNotify_NoUpdateAvailable(t *testing.T) { t.Parallel() cfg := &config.Instance{} - cfg.SetAutoUpdate(true) + cfg.SetUpdateCheck(true) checkFn := func(_ context.Context, _ Options) (*Result, error) { return &Result{ @@ -204,7 +270,7 @@ func TestCheckAndNotify_CheckError(t *testing.T) { t.Parallel() cfg := &config.Instance{} - cfg.SetAutoUpdate(true) + cfg.SetUpdateCheck(true) checkFn := func(_ context.Context, _ Options) (*Result, error) { return nil, errors.New("network timeout") @@ -471,3 +537,133 @@ func testValidationChainRelease(serverURL string) *selfupdate.Release { }, } } + +func TestClearDeferralForRelease(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + recordedVersion string + releaseVersion string + wantDeferral bool + }{ + { + name: "matching release keeps deferral", + recordedVersion: "v2.5.0", + releaseVersion: "v2.5.0", + wantDeferral: true, + }, + { + name: "different release clears deferral", + recordedVersion: "v2.5.0", + releaseVersion: "v2.6.0", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + dir := filepath.Join(t.TempDir(), "updater") + require.NoError(t, recordDeferral(dir, tt.recordedVersion, ReasonActiveMedia)) + before := peekDeferralState(dir) + require.NotNil(t, before) + + require.NoError(t, clearDeferralForRelease(dir, tt.releaseVersion)) + after := peekDeferralState(dir) + if !tt.wantDeferral { + assert.Nil(t, after) + return + } + require.NotNil(t, after) + assert.Equal(t, before.Version, after.Version) + assert.Equal(t, before.Reason, after.Reason) + assert.Equal(t, before.Since, after.Since) + }) + } +} + +func TestNoteGate_NoGateConfigured(t *testing.T) { + t.Parallel() + + result := &Result{} + noteGate(t.Context(), &Options{}, result, filepath.Join(t.TempDir(), "updater"), "v2.5.0") + + assert.Empty(t, result.BlockedReason) + assert.Empty(t, result.BlockedMessage) +} + +func TestNoteGate_NothingInTheWay(t *testing.T) { + t.Parallel() + + dir := filepath.Join(t.TempDir(), "updater") + result := &Result{} + opts := &Options{Gate: &GateDeps{Power: externalPower}} + noteGate(t.Context(), opts, result, dir, "v2.5.0") + + assert.Empty(t, result.BlockedReason) + assert.Nil(t, peekDeferral(dir, "v2.5.0")) +} + +func TestNoteGate_SoftSignalStartsTheClock(t *testing.T) { + t.Parallel() + + dir := filepath.Join(t.TempDir(), "updater") + result := &Result{} + opts := &Options{Gate: &GateDeps{ + Power: externalPower, + ActiveMedia: alwaysTrue, + }} + noteGate(t.Context(), opts, result, dir, "v2.5.0") + + assert.Equal(t, ReasonActiveMedia, result.BlockedReason) + assert.NotEmpty(t, result.BlockedMessage) + assert.True(t, result.BlockedForceable, "a person may go ahead through their own game") + + // Something that will pass on its own is what the automatic install's + // patience is measured against, so the check writes it down. + deferral := peekDeferral(dir, "v2.5.0") + require.NotNil(t, deferral) + assert.Equal(t, ReasonActiveMedia, deferral.Reason) +} + +func TestNoteGate_HardSignalIsReportedButNotDeferred(t *testing.T) { + t.Parallel() + + dir := filepath.Join(t.TempDir(), "updater") + result := &Result{} + opts := &Options{Gate: &GateDeps{ + Power: externalPower, + IndexingStatus: statusFn(mediadb.IndexingStatusRunning), + }} + noteGate(t.Context(), opts, result, dir, "v2.5.0") + + assert.Equal(t, ReasonMediaIndexing, result.BlockedReason) + assert.False(t, result.BlockedForceable) + // Indexing never times out into being safe, so there is no clock to start. + assert.Nil(t, peekDeferral(dir, "v2.5.0")) +} + +func TestNoteGate_ReportsWithoutTakingAnyGate(t *testing.T) { + t.Parallel() + + dir := filepath.Join(t.TempDir(), "updater") + restoreTaken, mediaTaken := false, false + opts := &Options{Gate: &GateDeps{ + Power: externalPower, + AcquireRestore: func() (func(), error) { + restoreTaken = true + return func() {}, nil + }, + AcquireMediaGate: func(context.Context) (func(), error) { + mediaTaken = true + return func() {}, nil + }, + }} + noteGate(t.Context(), opts, &Result{}, dir, "v2.5.0") + + // A check only reports. Taking either gate would block backups and + // launches every time a client asked whether an update was available. + assert.False(t, restoreTaken) + assert.False(t, mediaTaken) +} diff --git a/pkg/testing/mocks/platform.go b/pkg/testing/mocks/platform.go index 606e56ab8..49e192dbb 100644 --- a/pkg/testing/mocks/platform.go +++ b/pkg/testing/mocks/platform.go @@ -27,6 +27,7 @@ import ( "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models" "github.com/ZaparooProject/zaparoo-core/v2/pkg/config" "github.com/ZaparooProject/zaparoo-core/v2/pkg/database" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/power" "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/syncutil" "github.com/ZaparooProject/zaparoo-core/v2/pkg/platforms" "github.com/ZaparooProject/zaparoo-core/v2/pkg/readers" @@ -38,6 +39,7 @@ import ( // MockPlatform is a mock implementation of the Platform interface using testify/mock type MockPlatform struct { mock.Mock + powerStatus *power.Status launchedMedia []string launchedSystems []string keyboardPresses []string @@ -45,6 +47,26 @@ type MockPlatform struct { mu syncutil.Mutex } +// PowerStatus reports the device's power state. It is deliberately not a +// testify expectation: every test that reaches the updater would otherwise +// have to declare one, and the answer a test does not care about should not +// depend on whether the machine running it has a wireless mouse. +func (m *MockPlatform) PowerStatus() (power.Status, error) { + m.mu.Lock() + defer m.mu.Unlock() + if m.powerStatus == nil { + return power.Status{Source: power.SourceNoBattery}, nil + } + return *m.powerStatus, nil +} + +// SetPowerStatus fixes what PowerStatus reports. +func (m *MockPlatform) SetPowerStatus(status power.Status) { + m.mu.Lock() + defer m.mu.Unlock() + m.powerStatus = &status +} + // ID returns the unique ID of this platform func (m *MockPlatform) ID() string { args := m.Called() diff --git a/pkg/zapscript/commands.go b/pkg/zapscript/commands.go index 5d45a1045..0375868e4 100644 --- a/pkg/zapscript/commands.go +++ b/pkg/zapscript/commands.go @@ -50,7 +50,7 @@ import ( // RunCommandOptions groups optional services used by specific command types. type RunCommandOptions struct { WaitForMediaReady func(context.Context) error - AcquireMediaLaunch func() (func(), error) + AcquireMediaLaunch func() (platforms.MediaLaunchAccess, error) PlaybackManager audio.PlaybackManager UI *uievents.Service LauncherManager *state.LauncherManager diff --git a/pkg/zapscript/launch.go b/pkg/zapscript/launch.go index 1d385dcaa..439a4d8a9 100644 --- a/pkg/zapscript/launch.go +++ b/pkg/zapscript/launch.go @@ -788,14 +788,20 @@ func getLaunchClosure( return errors.New("file not allowed: " + target.path) } - releaseLaunch := func() {} + launchAccess := platforms.MediaLaunchAccess{Release: func() {}} if env.AcquireMediaLaunch != nil { - releaseLaunch, err = env.AcquireMediaLaunch() + launchAccess, err = env.AcquireMediaLaunch() if err != nil { return fmt.Errorf("acquiring media launch gate: %w", err) } } - defer releaseLaunch() + defer launchAccess.Release() + if launchAccess.SetActiveMedia != nil { + if opts == nil { + opts = &platforms.LaunchOptions{} + } + opts.ActiveMediaPublisher = launchAccess.SetActiveMedia + } return pl.LaunchMedia(env.Cfg, target.path, launcher, env.Database, opts) } } diff --git a/pkg/zapscript/launch_test.go b/pkg/zapscript/launch_test.go index 44f2b5eff..1119fe28e 100644 --- a/pkg/zapscript/launch_test.go +++ b/pkg/zapscript/launch_test.go @@ -31,6 +31,7 @@ import ( "time" "github.com/ZaparooProject/go-zapscript" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models" "github.com/ZaparooProject/zaparoo-core/v2/pkg/config" "github.com/ZaparooProject/zaparoo-core/v2/pkg/database" "github.com/ZaparooProject/zaparoo-core/v2/pkg/database/systemdefs" @@ -676,23 +677,33 @@ func TestLaunchClosureHoldsMediaLaunchGate(t *testing.T) { path := filepath.Join("games", "game.sfc") mockPlatform := mocks.NewMockPlatform() gateHeld := false + published := false mockPlatform.On( "LaunchMedia", cfg, path, (*platforms.Launcher)(nil), - (*database.Database)(nil), (*platforms.LaunchOptions)(nil), - ).Run(func(mock.Arguments) { + (*database.Database)(nil), mock.MatchedBy(func(opts *platforms.LaunchOptions) bool { + return opts != nil && opts.ActiveMediaPublisher != nil + }), + ).Run(func(args mock.Arguments) { assert.True(t, gateHeld) + opts, ok := args.Get(4).(*platforms.LaunchOptions) + require.True(t, ok) + opts.ActiveMediaPublisher(&models.ActiveMedia{SystemID: "SNES", Name: "Game"}) }).Return(nil).Once() env := platforms.CmdEnv{ Cfg: cfg, Cmd: zapscript.Command{AdvArgs: zapscript.NewAdvArgs(nil)}, - AcquireMediaLaunch: func() (func(), error) { + AcquireMediaLaunch: func() (platforms.MediaLaunchAccess, error) { gateHeld = true - return func() { gateHeld = false }, nil + return platforms.MediaLaunchAccess{ + SetActiveMedia: func(*models.ActiveMedia) { published = true }, + Release: func() { gateHeld = false }, + }, nil }, } launch := getLaunchClosure(mockPlatform, &env, true) require.NoError(t, launch(launchTarget{path: path, systemID: "SNES"})) + assert.True(t, published) assert.False(t, gateHeld) mockPlatform.AssertExpectations(t) } diff --git a/scripts/tasks/cross-lint.yml b/scripts/tasks/cross-lint.yml index c08950764..ad2bb98b0 100644 --- a/scripts/tasks/cross-lint.yml +++ b/scripts/tasks/cross-lint.yml @@ -1,5 +1,9 @@ version: "3" +vars: + # Pin to avoid current latest's nilness recover panic: dominikh/go-tools#1725. + GOLANGCI_LINT_VERSION: v2.12.2 + tasks: setup: internal: true @@ -22,7 +26,7 @@ tasks: CC: "zig cc -w --target=x86_64-windows-gnu" CXX: "zig c++ -w --target=x86_64-windows-gnu" EXEC: >- - bash -c 'curl -sSfL https://golangci-lint.run/install.sh | sh -s -- -b /home/build/bin 2>&1 | tail -1 + bash -c 'curl -sSfL https://golangci-lint.run/install.sh | sh -s -- -b /home/build/bin {{.GOLANGCI_LINT_VERSION}} 2>&1 | tail -1 && PATH="/home/build/bin:$PATH" golangci-lint run --fix --timeout=5m' EXTRA_DOCKER_ARGS: >- -e GOOS=windows @@ -51,7 +55,7 @@ tasks: -L/opt/macosx-sdk/usr/lib -F/opt/macosx-sdk/System/Library/Frameworks EXEC: >- - bash -c 'curl -sSfL https://golangci-lint.run/install.sh | sh -s -- -b /home/build/bin 2>&1 | tail -1 + bash -c 'curl -sSfL https://golangci-lint.run/install.sh | sh -s -- -b /home/build/bin {{.GOLANGCI_LINT_VERSION}} 2>&1 | tail -1 && PATH="/home/build/bin:$PATH" golangci-lint run --fix --timeout=5m' EXTRA_DOCKER_ARGS: >- -e GOOS=darwin