From af159febb7e6b683d60bb8746f41c33a55ffafea Mon Sep 17 00:00:00 2001 From: Thomas Kroll Date: Fri, 13 Nov 2026 18:12:03 +0100 Subject: [PATCH 1/4] feat: detect the API version of the server and what the API key may do Two small queries (GetApiVersion, GetKeyAccess) tell NOVA which Unraid API version the active server runs and which roles and permissions its API key holds. The result is kept in memory as ServerCapabilities and exposed as MainViewModel.capabilities; nothing reads it yet, so there is no visible change. Detection runs when the server, its address or the key changes, on pull-to-refresh, and when the app returns to the foreground. There is no timer, no retry loop and no polling. Unknown is never denied: a failed detection, a refused `me` and a role NOVA does not know leave the access unknown, so controls stay as they are today. Only a successful answer can say "denied". The model, the role table, the classification of server answers and the repository wiring have unit tests built on the real response bodies of an Unraid 7.4.0 server. ADR-0047 records the decision. Refs #315, #247, #317, #316 Co-Authored-By: Claude Sonnet 5.5 --- .../github/nofuturekid/nova/queries.graphql | 23 ++ .../github/nofuturekid/nova/schema.graphqls | 67 +++++ .../nova/data/model/ServerCapabilities.kt | 178 ++++++++++++ .../nova/data/repository/CapabilityProbe.kt | 150 ++++++++++ .../data/repository/CapabilityRepository.kt | 127 ++++++++ .../nova/data/repository/UnraidRepository.kt | 11 + .../nova/ui/screens/main/MainViewModel.kt | 18 +- .../nova/data/model/ServerCapabilitiesTest.kt | 168 +++++++++++ .../data/repository/CapabilityProbeTest.kt | 208 ++++++++++++++ .../repository/CapabilityRepositoryTest.kt | 272 ++++++++++++++++++ docs/adr/0047-server-capabilities.md | 53 ++++ docs/adr/README.md | 1 + 12 files changed, 1275 insertions(+), 1 deletion(-) create mode 100644 app/src/main/kotlin/io/github/nofuturekid/nova/data/model/ServerCapabilities.kt create mode 100644 app/src/main/kotlin/io/github/nofuturekid/nova/data/repository/CapabilityProbe.kt create mode 100644 app/src/main/kotlin/io/github/nofuturekid/nova/data/repository/CapabilityRepository.kt create mode 100644 app/src/test/kotlin/io/github/nofuturekid/nova/data/model/ServerCapabilitiesTest.kt create mode 100644 app/src/test/kotlin/io/github/nofuturekid/nova/data/repository/CapabilityProbeTest.kt create mode 100644 app/src/test/kotlin/io/github/nofuturekid/nova/data/repository/CapabilityRepositoryTest.kt create mode 100644 docs/adr/0047-server-capabilities.md diff --git a/app/src/main/graphql/io/github/nofuturekid/nova/queries.graphql b/app/src/main/graphql/io/github/nofuturekid/nova/queries.graphql index 7c3071a7..b9b792de 100644 --- a/app/src/main/graphql/io/github/nofuturekid/nova/queries.graphql +++ b/app/src/main/graphql/io/github/nofuturekid/nova/queries.graphql @@ -204,3 +204,26 @@ query GetNetworkInterfaces { } } } + +# Capability detection (ADR-0047). Two operations on purpose: `me` is non-null, so for a key without +# ME:READ_ANY a FORBIDDEN on `me` would null the whole response and lose the version as well. +query GetApiVersion { + info { + versions { + core { + unraid + api + } + } + } +} + +query GetKeyAccess { + me { + roles + permissions { + resource + actions + } + } +} diff --git a/app/src/main/graphql/io/github/nofuturekid/nova/schema.graphqls b/app/src/main/graphql/io/github/nofuturekid/nova/schema.graphqls index df858758..1db03463 100644 --- a/app/src/main/graphql/io/github/nofuturekid/nova/schema.graphqls +++ b/app/src/main/graphql/io/github/nofuturekid/nova/schema.graphqls @@ -40,6 +40,7 @@ type Query { plugins: [Plugin!]! pluginInstallOperations: [PluginInstallOperation!]! display: InfoDisplay + me: UserAccount! } # ── Display (global temperature thresholds) ────────────────────────── @@ -265,6 +266,7 @@ type InfoVersions { type CoreVersions { unraid: String + api: String kernel: String } @@ -565,3 +567,68 @@ type DockerContainerStats { netIO: String! blockIO: String! } + +# ── API key access (capability detection, ADR-0047) ────────────────── +# Subset of the live Unraid `me` query. `me` describes the API key, not a +# signed-in user. Enum values are the Unraid 4.37.11 schema; a value a newer +# server adds arrives as an unknown enum value and keeps its raw name. + +type UserAccount { + roles: [Role!]! + permissions: [Permission!] +} + +type Permission { + resource: Resource! + actions: [AuthAction!]! +} + +enum Role { + ADMIN + CONNECT + GUEST + VIEWER +} + +enum AuthAction { + CREATE_ANY + CREATE_OWN + READ_ANY + READ_OWN + UPDATE_ANY + UPDATE_OWN + DELETE_ANY + DELETE_OWN +} + +enum Resource { + ACTIVATION_CODE + API_KEY + ARRAY + CLOUD + CONFIG + CONNECT + CONNECT__REMOTE_ACCESS + CUSTOMIZATIONS + DASHBOARD + DISK + DISPLAY + DOCKER + FLASH + INFO + LOGS + ME + NETWORK + NOTIFICATIONS + ONLINE + OS + OWNER + PERMISSION + REGISTRATION + SERVERS + SERVICES + SHARE + VARS + VMS + WELCOME +} diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/data/model/ServerCapabilities.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/data/model/ServerCapabilities.kt new file mode 100644 index 00000000..2973a587 --- /dev/null +++ b/app/src/main/kotlin/io/github/nofuturekid/nova/data/model/ServerCapabilities.kt @@ -0,0 +1,178 @@ +package io.github.nofuturekid.nova.data.model + +/** + * What NOVA knows about the server it talks to and about what its API key may + * do (ADR-0047). Pure Kotlin on purpose: no Apollo and no Android types, so the + * rules below are unit-tested on the JVM. + * + * One rule runs through all of it: UNKNOWN IS NOT DENIED. A failed detection, a + * key whose `me` is refused and a role NOVA has never heard of all leave the + * answer at [Access.Unknown]. Only a successful detection can say [Access.Denied], + * so a transient error can never take a button away from the user. + */ + +/** The server address and API key that capabilities were detected for. Equality only; never logged. */ +data class CapabilityTarget(val serverId: String, val baseUrl: String, private val apiKey: String) { + override fun toString() = "CapabilityTarget($serverId, $baseUrl, key=***)" +} + +/** An Unraid API version, compared by number only (a build suffix is not part of the order). */ +data class ApiVersion(val major: Int, val minor: Int, val patch: Int) : Comparable { + override fun compareTo(other: ApiVersion) = compareValuesBy(this, other, { it.major }, { it.minor }, { it.patch }) + + override fun toString() = "$major.$minor.$patch" + + companion object { + private val LEADING_NUMBERS = Regex("""^\s*(\d+)\.(\d+)\.(\d+)""") + + /** Leading x.y.z of "4.37.11+8e587fe2", "4.36.1+f3205d97" or "4.37.11-2"; null for anything else. */ + fun parse(raw: String?): ApiVersion? { + val (a, b, c) = raw?.let { LEADING_NUMBERS.find(it) }?.destructured ?: return null + return ApiVersion( + a.toIntOrNull() ?: return null, + b.toIntOrNull() ?: return null, + c.toIntOrNull() ?: return null, + ) + } + } +} + +/** An API capability gated by the server's API version, with the first version that has it. */ +enum class ApiFeature(val since: ApiVersion) { + /** Every screen (#247). */ + Base(ApiVersion(4, 30, 0)), + + /** Documented here; the existing network-speed probe stays. */ + NetworkSpeed(ApiVersion(4, 35, 0)), + + /** `docker.restart` in one step (#317). */ + DockerRestart(ApiVersion(4, 36, 0)), +} + +/** + * A control NOVA offers and the permission the server checks for it. The single source for the + * hide-controls work (#315) and the documentation of key permissions (#316). + * + * [operations] are the names of the GraphQL operations in `mutations.graphql` behind the control. + */ +enum class KeyAction(val resource: String, val action: String, val operations: Set) { + ControlArray( + "ARRAY", "UPDATE_ANY", + setOf("StartArray", "StopArray", "StartParityCheck", "PauseParityCheck", "ResumeParityCheck", "CancelParityCheck"), + ), + ControlDocker( + "DOCKER", "UPDATE_ANY", + setOf( + "StartContainer", "StopContainer", "RestartContainer", "PauseContainer", "UnpauseContainer", + "UpdateContainer", "UpdateAllContainers", + ), + ), + ControlVms( + "VMS", "UPDATE_ANY", + setOf("StartVm", "StopVm", "PauseVm", "ResumeVm", "ForceStopVm", "RebootVm", "ResetVm"), + ), + UpdateNotifications( + "NOTIFICATIONS", "UPDATE_ANY", + setOf("ArchiveNotification", "UnreadNotification", "ArchiveAllNotifications", "RecalculateNotificationOverview"), + ), + DeleteNotifications( + "NOTIFICATIONS", "DELETE_ANY", + setOf("DeleteNotification", "DeleteArchivedNotifications"), + ), + ; + + /** The permission in the server's own spelling, e.g. `DOCKER:UPDATE_ANY`. */ + val permission: String get() = "$resource:$action" + + companion object { + fun forOperation(name: String): KeyAction? = entries.firstOrNull { name in it.operations } + } +} + +/** What the key may do. [Unknown] is not [Denied]: the UI hides a control only for [Denied]. */ +enum class Access { Allowed, Denied, Unknown } + +/** Whether the server's API has a feature. [Unknown] is not [No]. */ +enum class Support { Yes, No, Unknown } + +sealed interface KeyAccess { + data object Unknown : KeyAccess + + /** [roles] and [grants] as the server's raw enum names, e.g. "VIEWER" and "DOCKER:UPDATE_ANY". */ + data class Known(val roles: Set, val grants: Set) : KeyAccess +} + +enum class Detection { + /** Nothing detected yet (no server, or no key). */ + Idle, + + /** Detection for [ServerCapabilities.target] is running and no earlier result exists for it. */ + Detecting, + + /** The server answered; the version or the access (or both) is known. */ + Done, + + /** The server said `UNAUTHENTICATED`. Only ever set from that answer. */ + KeyRejected, + + /** No usable answer (unreachable, server error). Version and access stay unknown. */ + Failed, +} + +data class ServerCapabilities( + val target: CapabilityTarget? = null, + val detection: Detection = Detection.Idle, + val unraidVersion: String? = null, + /** The raw API version, for display ("4.37.11+8e587fe2"). */ + val apiVersionText: String? = null, + val apiVersion: ApiVersion? = null, + /** The version query failed validation, so the API is older than 4.15.0 (the field does not exist there). */ + val versionFieldMissing: Boolean = false, + val access: KeyAccess = KeyAccess.Unknown, +) { + /** + * Whether the key may use [a]: [Access.Allowed] when the key holds the permission explicitly or one of + * its roles grants it; [Access.Unknown] when nothing is known or a role is unknown to NOVA and no known + * role grants it (a future role must never hide a control); otherwise [Access.Denied]. + */ + fun access(a: KeyAction): Access { + val known = access as? KeyAccess.Known ?: return Access.Unknown + if (a.permission in known.grants) return Access.Allowed + val verdicts = known.roles.map { roleGrants(it, a.resource, a.action) } + return when { + verdicts.any { it == true } -> Access.Allowed + verdicts.any { it == null } -> Access.Unknown + else -> Access.Denied + } + } + + fun supports(f: ApiFeature): Support = when { + versionFieldMissing -> Support.No + apiVersion == null -> Support.Unknown + apiVersion >= f.since -> Support.Yes + else -> Support.No + } + + /** True only when the server proved it: the version is below the floor, or the version field does not exist. */ + val apiTooOld: Boolean get() = supports(ApiFeature.Base) == Support.No + + companion object { + val UNKNOWN = ServerCapabilities() + } +} + +/** + * Static mirror of the Unraid API's role policy (`api/src/unraid-api/auth/casbin/policy.ts`; identical from + * v4.30.0 to v4.37.11 and main). `true` = the role grants it, `false` = it does not, `null` = a role NOVA + * does not know. The server compares the action exactly: UPDATE_ANY does not imply READ_ANY and `*_OWN` + * never satisfies an `*_ANY` check. + */ +internal fun roleGrants(role: String, resource: String, action: String): Boolean? = when (role) { + "ADMIN" -> true + "VIEWER" -> action == "READ_ANY" && resource != "API_KEY" + // CONNECT inherits VIEWER and may update its own remote-access setting. + "CONNECT" -> (action == "READ_ANY" && resource != "API_KEY") || + (resource == "CONNECT__REMOTE_ACCESS" && action == "UPDATE_ANY") + "GUEST" -> resource == "ME" && action == "READ_ANY" + else -> null +} diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/data/repository/CapabilityProbe.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/data/repository/CapabilityProbe.kt new file mode 100644 index 00000000..f20ed075 --- /dev/null +++ b/app/src/main/kotlin/io/github/nofuturekid/nova/data/repository/CapabilityProbe.kt @@ -0,0 +1,150 @@ +package io.github.nofuturekid.nova.data.repository + +import com.apollographql.apollo.ApolloClient +import com.apollographql.apollo.api.Error +import io.github.nofuturekid.nova.data.model.ApiVersion +import io.github.nofuturekid.nova.data.model.CapabilityTarget +import io.github.nofuturekid.nova.data.model.Detection +import io.github.nofuturekid.nova.data.model.KeyAccess +import io.github.nofuturekid.nova.data.model.ServerCapabilities +import io.github.nofuturekid.nova.graphql.GetApiVersionQuery +import io.github.nofuturekid.nova.graphql.GetKeyAccessQuery +import kotlinx.coroutines.CancellationException + +/** + * Detection of the server's API version and the API key's access (ADR-0047): the two + * operations `GetApiVersion` and `GetKeyAccess`, and the classification of what the server + * answers. The mapping from answers to capabilities lives here, not in `GraphQlMapper.kt`. + * + * The rule for every branch: only a SUCCESSFUL answer can take something away. A refused or + * failed `me` leaves the access unknown (controls stay visible, as before this feature); + * only `UNAUTHENTICATED` from the server marks the key as rejected, never an inference. + */ + +/** The result of one detection, before it is tied to a target (see [toCapabilities]). */ +data class ProbeOutcome( + val detection: Detection, + val unraidVersion: String? = null, + val apiVersionText: String? = null, + val apiVersion: ApiVersion? = null, + /** The version query failed validation: the API is older than 4.15.0. */ + val versionFieldMissing: Boolean = false, + val access: KeyAccess = KeyAccess.Unknown, +) { + fun toCapabilities(target: CapabilityTarget?) = ServerCapabilities( + target = target, + detection = detection, + unraidVersion = unraidVersion, + apiVersionText = apiVersionText, + apiVersion = apiVersion, + versionFieldMissing = versionFieldMissing, + access = access, + ) +} + +/** What the `errors` of one GraphQL response say, by `extensions.code`. First match wins in this order. */ +internal enum class ErrorClass { KeyRejected, ValidationFailed, Forbidden, Other } + +internal fun classifyErrors(errors: List): ErrorClass = when { + errors.any { it.code() == "UNAUTHENTICATED" } -> ErrorClass.KeyRejected + // Apollo parses Unraid's HTTP 400 `application/graphql-response+json` body into `errors`. + errors.any { it.code() == "GRAPHQL_VALIDATION_FAILED" || it.message.startsWith("Cannot query field") } -> + ErrorClass.ValidationFailed + errors.any { it.code() == "FORBIDDEN" } -> ErrorClass.Forbidden + else -> ErrorClass.Other +} + +private fun Error.code(): Any? = extensions?.get("code") + +private sealed interface VersionStep { + data class Got(val unraid: String?, val apiText: String?) : VersionStep + data object Missing : VersionStep + data object Unknown : VersionStep + data object Rejected : VersionStep + data object Failed : VersionStep +} + +private sealed interface AccessStep { + data class Got(val access: KeyAccess.Known) : AccessStep + data object Unknown : AccessStep + data object Rejected : AccessStep + data object TransportFailed : AccessStep +} + +/** + * Sends `GetApiVersion`, then `GetKeyAccess`, and classifies each answer. Never throws for a + * server or transport problem (an [com.apollographql.apollo.exception.ApolloException] counts as + * a transport failure); only cancellation passes through. + */ +internal suspend fun probeCapabilities(client: ApolloClient): ProbeOutcome { + val version = probeVersion(client) + when (version) { + VersionStep.Rejected -> return ProbeOutcome(Detection.KeyRejected) + VersionStep.Failed -> return ProbeOutcome(Detection.Failed) + else -> Unit + } + val versionKnown = version is VersionStep.Got || version == VersionStep.Missing + val base = when (version) { + is VersionStep.Got -> ProbeOutcome( + Detection.Done, + unraidVersion = version.unraid, + apiVersionText = version.apiText, + apiVersion = ApiVersion.parse(version.apiText), + ) + VersionStep.Missing -> ProbeOutcome(Detection.Done, versionFieldMissing = true) + else -> ProbeOutcome(Detection.Done) + } + return when (val access = probeAccess(client)) { + is AccessStep.Got -> base.copy(access = access.access) + AccessStep.Unknown -> base + AccessStep.Rejected -> ProbeOutcome(Detection.KeyRejected) + AccessStep.TransportFailed -> if (versionKnown) base else ProbeOutcome(Detection.Failed) + } +} + +private suspend fun probeVersion(client: ApolloClient): VersionStep = try { + val resp = client.query(GetApiVersionQuery()).execute() + val core = resp.data?.info?.versions?.core + val errors = resp.errors.orEmpty() + when { + core != null -> VersionStep.Got(core.unraid, core.api) + errors.isNotEmpty() -> when (classifyErrors(errors)) { + ErrorClass.KeyRejected -> VersionStep.Rejected + ErrorClass.ValidationFailed -> VersionStep.Missing + ErrorClass.Forbidden -> VersionStep.Unknown + ErrorClass.Other -> VersionStep.Failed + } + // Transport failure, or a body without data and without errors. + else -> VersionStep.Failed + } +} catch (e: CancellationException) { + throw e +} catch (e: Exception) { + VersionStep.Failed +} + +private suspend fun probeAccess(client: ApolloClient): AccessStep = try { + val resp = client.query(GetKeyAccessQuery()).execute() + val me = resp.data?.me + val errors = resp.errors.orEmpty() + when { + me != null -> AccessStep.Got( + KeyAccess.Known( + roles = me.roles.map { it.rawValue }.toSet(), + grants = me.permissions.orEmpty().flatMap { p -> + p.actions.map { "${p.resource.rawValue}:${it.rawValue}" } + }.toSet(), + ), + ) + errors.isNotEmpty() -> when (classifyErrors(errors)) { + ErrorClass.KeyRejected -> AccessStep.Rejected + // FORBIDDEN (a key without ME:READ_ANY), validation and any other error: fail open. + else -> AccessStep.Unknown + } + else -> AccessStep.TransportFailed + } +} catch (e: CancellationException) { + throw e +} catch (e: Exception) { + AccessStep.TransportFailed +} diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/data/repository/CapabilityRepository.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/data/repository/CapabilityRepository.kt new file mode 100644 index 00000000..8097556e --- /dev/null +++ b/app/src/main/kotlin/io/github/nofuturekid/nova/data/repository/CapabilityRepository.kt @@ -0,0 +1,127 @@ +package io.github.nofuturekid.nova.data.repository + +import io.github.nofuturekid.nova.data.model.CapabilityTarget +import io.github.nofuturekid.nova.data.model.Detection +import io.github.nofuturekid.nova.data.model.ServerCapabilities +import io.github.nofuturekid.nova.di.ApplicationScope +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock +import javax.inject.Inject +import javax.inject.Singleton + +/** + * The capabilities of the active server: its API version and what its API key may do (ADR-0047). + * Built like [io.github.nofuturekid.nova.data.update.UpdateTracker]: a singleton that detects in + * the application scope, so a detection is not lost when a screen goes away, and holds the result + * in memory only (a server upgrade or a changed key would leave a stored result stale). + * + * Detection runs on exactly three occasions and no other: the TARGET changes (server id, resolved + * address or API key; a renamed server or a changed accent is not a change), [refresh] is called + * (pull-to-refresh, return to the foreground), or the app starts. There is no timer, no retry loop + * and no polling, so nothing runs while the app is in the background. + * + * Fail open, by construction: + * - a new target starts at [Detection.Detecting] with unknown access, so one server's + * "denied" never gates another server; + * - a failed detection never replaces a result that was good for the same target, so a + * transient error cannot make controls appear and disappear; + * - a result for a target that is no longer current is dropped. + */ +@Singleton +class CapabilityRepository internal constructor( + private val scope: CoroutineScope, + active: Flow, + private val targetOf: (ActiveServer?) -> CapabilityTarget?, + private val probe: suspend (ActiveServer) -> ProbeOutcome, +) { + @Inject + constructor( + servers: ServerRepository, + unraid: UnraidRepository, + @ApplicationScope scope: CoroutineScope, + ) : this( + scope = scope, + active = servers.activeWithKey, + targetOf = unraid::capabilityTarget, + probe = unraid::probeCapabilities, + ) + + private val state = MutableStateFlow(ServerCapabilities.UNKNOWN) + + /** The capabilities of the active server. Starts, and stays, unknown until a detection has answered. */ + val capabilities: StateFlow = state.asStateFlow() + + private class Running(val target: CapabilityTarget, val job: Job) + + /** Guards [currentActive], [running] and the start of a detection. */ + private val lock = Mutex() + private var currentActive: ActiveServer? = null + private var running: Running? = null + + // Last in the property list: the launched job reads every property above. + init { + scope.launch { active.collect(::onActive) } + } + + /** + * Detects again for the current target: pull-to-refresh, and the app returning to the foreground + * (the user may have widened or narrowed the key in the Unraid web UI, which keeps the same key + * string). Joins a detection that is already running for the same target; never runs two in + * parallel. The last good result stays visible while it runs. + */ + fun refresh() { + scope.launch { + lock.withLock { + val a = currentActive + val target = targetOf(a) + if (a != null && target != null) startUnlessRunning(a, target) + } + } + } + + private suspend fun onActive(a: ActiveServer?) = lock.withLock { + currentActive = a // always: a pin or mode change must reach the next probe even if the target is equal + val target = targetOf(a) + if (target == state.value.target) return@withLock + running?.job?.cancel() + running = null + if (target == null) { + state.value = ServerCapabilities.UNKNOWN + } else { + state.value = ServerCapabilities(target = target, detection = Detection.Detecting) + startUnlessRunning(a!!, target) + } + } + + /** Caller holds [lock]. */ + private fun startUnlessRunning(a: ActiveServer, target: CapabilityTarget) { + if (running?.let { it.target == target && it.job.isActive } == true) return + running = Running(target, scope.launch { detect(a, target) }) + } + + private suspend fun detect(a: ActiveServer, target: CapabilityTarget) { + val outcome = try { + probe(a) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + ProbeOutcome(Detection.Failed) + } + state.update { current -> + when { + current.target != target -> current // the target changed while this ran: drop the answer + outcome.detection == Detection.Failed && current.detection == Detection.Done -> current + else -> outcome.toCapabilities(target) + } + } + } +} diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/data/repository/UnraidRepository.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/data/repository/UnraidRepository.kt index 11701f5c..5f277774 100644 --- a/app/src/main/kotlin/io/github/nofuturekid/nova/data/repository/UnraidRepository.kt +++ b/app/src/main/kotlin/io/github/nofuturekid/nova/data/repository/UnraidRepository.kt @@ -52,6 +52,7 @@ import io.github.nofuturekid.nova.data.api.toServerInfo import io.github.nofuturekid.nova.data.api.toVms import io.github.nofuturekid.nova.data.api.toIfaceSamples import io.github.nofuturekid.nova.data.model.ArrayInfo +import io.github.nofuturekid.nova.data.model.CapabilityTarget import io.github.nofuturekid.nova.data.model.ConnectionMode import io.github.nofuturekid.nova.data.model.Container import io.github.nofuturekid.nova.data.model.ContainerLiveStats @@ -1104,6 +1105,16 @@ class UnraidRepository @Inject constructor( withTimeoutOrNull(REFRESH_MAX_WAIT_MS) { refreshNudge.awaitCompleted(generation) } } + /** + * The server address and key that capabilities are detected for (ADR-0047), or null without a + * usable key. Equal targets mean the same detection result is still valid. + */ + fun capabilityTarget(active: ActiveServer?): CapabilityTarget? = + active?.takeIf { it.apiKey.isNotBlank() }?.let { CapabilityTarget(it.server.id, activeUrl(it), it.apiKey) } + + /** The two detection queries for [active], classified (ADR-0047). Read-only. */ + suspend fun probeCapabilities(active: ActiveServer): ProbeOutcome = probeCapabilities(buildClient(active)) + private fun activeUrl(active: ActiveServer): String = when (active.mode) { ConnectionMode.Local -> active.server.localUrl ConnectionMode.Remote -> active.server.remoteUrl.ifBlank { active.server.localUrl } diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/main/MainViewModel.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/main/MainViewModel.kt index 11799987..68c8fb45 100644 --- a/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/main/MainViewModel.kt +++ b/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/main/MainViewModel.kt @@ -35,11 +35,13 @@ import io.github.nofuturekid.nova.data.model.LiveMetrics import io.github.nofuturekid.nova.data.model.NetworkThroughput import io.github.nofuturekid.nova.data.model.Notifications import io.github.nofuturekid.nova.data.model.Server +import io.github.nofuturekid.nova.data.model.ServerCapabilities import io.github.nofuturekid.nova.data.model.ServerInfo import io.github.nofuturekid.nova.data.model.Temperature import io.github.nofuturekid.nova.data.model.UpdateStatus import io.github.nofuturekid.nova.data.model.Vm import io.github.nofuturekid.nova.data.repository.ActionFailedException +import io.github.nofuturekid.nova.data.repository.CapabilityRepository import io.github.nofuturekid.nova.data.repository.DomainState import io.github.nofuturekid.nova.data.repository.ServerRepository import io.github.nofuturekid.nova.data.repository.SettingsRepository @@ -63,6 +65,7 @@ class MainViewModel @Inject constructor( private val unraid: UnraidRepository, private val settings: SettingsRepository, private val updateTracker: UpdateTracker, + private val caps: CapabilityRepository, ) : ViewModel() { // ── Resilient action seam (triage #19, ADR-0037) ────────────────── @@ -163,7 +166,19 @@ class MainViewModel @Inject constructor( fun setDockerSheetOpen(open: Boolean) { _dockerSheetOpen.value = open } private val _appVisible = MutableStateFlow(true) - fun setAppVisible(visible: Boolean) { _appVisible.value = visible } + fun setAppVisible(visible: Boolean) { + // Returning to the foreground (not the first start, which detects by itself): the user may + // have changed the key's permissions in the Unraid web UI while NOVA was away (ADR-0047). + if (visible && !_appVisible.value) caps.refresh() + _appVisible.value = visible + } + + /** + * The API version of the active server and what its API key may do (ADR-0047). Unknown until + * detected, and unknown is never "denied". Nothing reads it yet; the screens take it up in + * follow-up changes. + */ + val capabilities: StateFlow = caps.capabilities // ── Per-domain polling state (gated, lifecycle-aware) ───────────── // @@ -336,6 +351,7 @@ class MainViewModel @Inject constructor( /** Pull-to-refresh: nudges every active stream to refetch now and * returns when the refreshed data has arrived (or failed). */ suspend fun refresh() { + caps.refresh() unraid.refreshAll() } diff --git a/app/src/test/kotlin/io/github/nofuturekid/nova/data/model/ServerCapabilitiesTest.kt b/app/src/test/kotlin/io/github/nofuturekid/nova/data/model/ServerCapabilitiesTest.kt new file mode 100644 index 00000000..d1d49820 --- /dev/null +++ b/app/src/test/kotlin/io/github/nofuturekid/nova/data/model/ServerCapabilitiesTest.kt @@ -0,0 +1,168 @@ +package io.github.nofuturekid.nova.data.model + +import java.io.File +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * ADR-0047. The model decides which controls a key may be offered, so every test here + * states the user-facing reason, not just the table lookup. + */ +class ServerCapabilitiesTest { + private fun withAccess(roles: Set, grants: Set = emptySet()) = + ServerCapabilities(detection = Detection.Done, access = KeyAccess.Known(roles, grants)) + + private fun ServerCapabilities.everyAction() = KeyAction.entries.associateWith { access(it) } + + // WHY: a read-only key must not be offered buttons the server will refuse (#315). + @Test fun viewerKeyMayReadButNotControl() { + assertTrue(withAccess(setOf("VIEWER")).everyAction().values.all { it == Access.Denied }) + } + + @Test fun adminKeyMayDoEverything() { + assertTrue(withAccess(setOf("ADMIN")).everyAction().values.all { it == Access.Allowed }) + } + + // WHY: the least-privilege key (VIEWER plus a few explicit permissions) must get exactly its buttons. + @Test fun explicitPermissionsAddToRole() { + val caps = withAccess(setOf("VIEWER"), setOf("DOCKER:UPDATE_ANY")) + assertEquals(Access.Allowed, caps.access(KeyAction.ControlDocker)) + assertEquals(Access.Denied, caps.access(KeyAction.ControlVms)) + assertEquals(Access.Denied, caps.access(KeyAction.DeleteNotifications)) + } + + // WHY: a key can hold permissions and no role at all; its grants alone decide. + @Test fun keyWithoutRolesUsesItsPermissions() { + val caps = withAccess(emptySet(), setOf("VMS:UPDATE_ANY")) + assertEquals(Access.Allowed, caps.access(KeyAction.ControlVms)) + assertEquals(Access.Denied, caps.access(KeyAction.ControlDocker)) + } + + // WHY: the server matches UPDATE_ANY exactly; an *_OWN grant never satisfies the guard. + @Test fun ownActionsDoNotCount() { + assertEquals(Access.Denied, withAccess(emptySet(), setOf("DOCKER:UPDATE_OWN")).access(KeyAction.ControlDocker)) + } + + // WHY: UPDATE_ANY does not imply DELETE_ANY on the server, so notifications split in two. + @Test fun updateNotificationsDoesNotAllowDelete() { + val caps = withAccess(emptySet(), setOf("NOTIFICATIONS:UPDATE_ANY")) + assertEquals(Access.Allowed, caps.access(KeyAction.UpdateNotifications)) + assertEquals(Access.Denied, caps.access(KeyAction.DeleteNotifications)) + } + + // WHY: a role a newer Unraid adds must never hide a control NOVA cannot judge. + @Test fun unknownRoleFailsOpen() { + val caps = withAccess(setOf("FUTURE_ROLE")) + assertTrue(caps.everyAction().values.all { it == Access.Unknown }) + } + + // WHY: a known role that grants it still wins over an unknown role next to it. + @Test fun knownGrantBeatsUnknownRole() { + assertEquals(Access.Allowed, withAccess(setOf("FUTURE_ROLE", "ADMIN")).access(KeyAction.ControlArray)) + } + + // WHY: "the permission query failed" is not "the key may not" (#315: the app then behaves as today). + @Test fun unknownAccessIsNeverDenied() { + assertTrue(ServerCapabilities.UNKNOWN.everyAction().values.all { it == Access.Unknown }) + assertTrue(ServerCapabilities(detection = Detection.Failed).everyAction().values.all { it == Access.Unknown }) + } + + // WHY: this pins our copy of the server's role policy (policy.ts) so drift shows up as a failing test. + @Test fun roleTableMirrorsUpstream() { + assertEquals(false, roleGrants("VIEWER", "API_KEY", "READ_ANY")) + assertEquals(true, roleGrants("VIEWER", "DOCKER", "READ_ANY")) + assertEquals(true, roleGrants("VIEWER", "ME", "READ_ANY")) + assertEquals(false, roleGrants("VIEWER", "DOCKER", "UPDATE_ANY")) + assertEquals(true, roleGrants("ADMIN", "API_KEY", "DELETE_ANY")) + assertEquals(true, roleGrants("CONNECT", "CONNECT__REMOTE_ACCESS", "UPDATE_ANY")) + assertEquals(false, roleGrants("CONNECT", "DOCKER", "UPDATE_ANY")) + assertEquals(true, roleGrants("GUEST", "ME", "READ_ANY")) + assertEquals(false, roleGrants("GUEST", "DOCKER", "READ_ANY")) + assertNull(roleGrants("FUTURE_ROLE", "DOCKER", "READ_ANY")) + } + + // WHY: a new control must not bypass hiding and the documented permission table (#315, #316). + @Test fun everyMutationHasAKeyAction() { + val dir = listOf(File("src/main/graphql"), File("app/src/main/graphql")).first { it.isDirectory } + val source = File(dir, "io/github/nofuturekid/nova/mutations.graphql").readLines() + .filterNot { it.trimStart().startsWith("#") }.joinToString("\n") + val names = Regex("""\bmutation\s+(\w+)""").findAll(source).map { it.groupValues[1] }.toList() + assertTrue("no mutations found", names.size > 20) + for (n in names) { + val owners = KeyAction.entries.filter { n in it.operations } + assertEquals("mutation $n must belong to exactly one KeyAction", 1, owners.size) + assertEquals(owners.single(), KeyAction.forOperation(n)) + } + } + + // WHY: one operation behind two permissions would make "hide" and "document" disagree. + @Test fun noOperationBelongsToTwoActions() { + val all = KeyAction.entries.flatMap { it.operations.toList() } + assertEquals(all.toSet().size, all.size) + } + + @Test fun permissionIsSpelledLikeTheServer() { + assertEquals("DOCKER:UPDATE_ANY", KeyAction.ControlDocker.permission) + assertEquals("NOTIFICATIONS:DELETE_ANY", KeyAction.DeleteNotifications.permission) + } + + // WHY: the API string carries a build suffix; the order must ignore it. + @Test fun parsesApiVersionWithBuildSuffix() { + assertEquals(ApiVersion(4, 37, 11), ApiVersion.parse("4.37.11+8e587fe2")) + assertEquals(ApiVersion(4, 36, 1), ApiVersion.parse("4.36.1+f3205d97")) + assertEquals(ApiVersion(4, 37, 11), ApiVersion.parse("4.37.11-2")) + assertNull(ApiVersion.parse("")) + assertNull(ApiVersion.parse(null)) + assertNull(ApiVersion.parse("unknown")) + assertNull(ApiVersion.parse("4.37")) + } + + // WHY: 4.9.0 is older than 4.30.0 although "9" sorts after "30" as text. + @Test fun versionsCompareByNumber() { + assertTrue(ApiVersion(4, 9, 0) < ApiVersion(4, 30, 0)) + assertTrue(ApiVersion(4, 36, 0) > ApiVersion(4, 35, 9)) + assertTrue(ApiVersion(5, 0, 0) > ApiVersion(4, 99, 99)) + } + + private fun caps(version: String?, fieldMissing: Boolean = false) = ServerCapabilities( + detection = Detection.Done, apiVersion = ApiVersion.parse(version), versionFieldMissing = fieldMissing, + ) + + // WHY: sending docker.restart to a 4.35 server fails validation; unknown must not claim support. + @Test fun restartNeeds436() { + assertEquals(Support.No, caps("4.35.1").supports(ApiFeature.DockerRestart)) + assertEquals(Support.Yes, caps("4.36.0").supports(ApiFeature.DockerRestart)) + assertEquals(Support.Yes, caps("4.37.11+8e587fe2").supports(ApiFeature.DockerRestart)) + assertEquals(Support.Unknown, caps(null).supports(ApiFeature.DockerRestart)) + } + + @Test fun floorsAreTheDocumentedOnes() { + assertEquals(ApiVersion(4, 30, 0), ApiFeature.Base.since) + assertEquals(ApiVersion(4, 35, 0), ApiFeature.NetworkSpeed.since) + assertEquals(ApiVersion(4, 36, 0), ApiFeature.DockerRestart.since) + assertEquals(Support.No, caps("4.29.2").supports(ApiFeature.Base)) + assertEquals(Support.Yes, caps("4.30.0").supports(ApiFeature.Base)) + assertEquals(Support.No, caps("4.34.0").supports(ApiFeature.NetworkSpeed)) + assertEquals(Support.Yes, caps("4.35.0").supports(ApiFeature.NetworkSpeed)) + } + + // WHY: "too old" is claimed only when the server proved it; unknown must not show an "API too old" hint. + @Test fun apiTooOldNeedsProof() { + assertTrue(caps("4.29.2").apiTooOld) + assertTrue(caps(null, fieldMissing = true).apiTooOld) + assertFalse(caps(null).apiTooOld) + assertFalse(ServerCapabilities.UNKNOWN.apiTooOld) + assertFalse(caps("4.30.0").apiTooOld) + } + + // WHY: the key must never reach a log line through the target. + @Test fun targetNeverPrintsTheKey() { + val t = CapabilityTarget("s1", "http://tower", "secret-key-123") + assertFalse(t.toString().contains("secret-key-123")) + assertEquals(t, CapabilityTarget("s1", "http://tower", "secret-key-123")) + assertTrue(t != CapabilityTarget("s1", "http://tower", "other-key")) + } +} diff --git a/app/src/test/kotlin/io/github/nofuturekid/nova/data/repository/CapabilityProbeTest.kt b/app/src/test/kotlin/io/github/nofuturekid/nova/data/repository/CapabilityProbeTest.kt new file mode 100644 index 00000000..2dde8aca --- /dev/null +++ b/app/src/test/kotlin/io/github/nofuturekid/nova/data/repository/CapabilityProbeTest.kt @@ -0,0 +1,208 @@ +package io.github.nofuturekid.nova.data.repository + +import com.apollographql.apollo.ApolloClient +import com.apollographql.apollo.api.http.HttpRequest +import com.apollographql.apollo.api.http.HttpResponse +import com.apollographql.apollo.network.http.HttpEngine +import io.github.nofuturekid.nova.data.model.Access +import io.github.nofuturekid.nova.data.model.ApiVersion +import io.github.nofuturekid.nova.data.model.Detection +import io.github.nofuturekid.nova.data.model.KeyAccess +import io.github.nofuturekid.nova.data.model.KeyAction +import io.github.nofuturekid.nova.data.model.Support +import io.github.nofuturekid.nova.data.model.ApiFeature +import kotlinx.coroutines.test.runTest +import okio.Buffer +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import java.io.IOException + +/** + * ADR-0047. Every body below is copied from a real answer of an Unraid 7.4.0 server (API 4.37.11) + * recorded on 2026-11-13, so each test fails if the classification stops matching what the server + * really sends. They run on a scripted HTTP engine, i.e. through the real Apollo response path. + */ +class CapabilityProbeTest { + + private class Engine( + private val version: () -> HttpResponse, + private val access: () -> HttpResponse, + ) : HttpEngine { + val operations = mutableListOf() + override suspend fun execute(request: HttpRequest): HttpResponse { + val body = Buffer().also { request.body?.writeTo(it) }.readUtf8() + return when { + "GetApiVersion" in body -> { operations += "GetApiVersion"; version() } + "GetKeyAccess" in body -> { operations += "GetKeyAccess"; access() } + else -> error("unexpected operation: $body") + } + } + override fun close() {} + } + + private fun reply(body: String, status: Int = 200, type: String = "application/json"): () -> HttpResponse = { + HttpResponse.Builder(status).addHeader("Content-Type", type).body(Buffer().writeUtf8(body)).build() + } + + private val versionOk = reply("""{"data":{"info":{"versions":{"core":{"unraid":"7.4.0","api":"4.37.11+8e587fe2"}}}}}""") + private val viewerMe = reply("""{"data":{"me":{"roles":["VIEWER"],"permissions":[]}}}""") + private val adminMe = reply("""{"data":{"me":{"roles":["ADMIN"],"permissions":[]}}}""") + private val unauthenticated = reply( + """{"errors":[{"message":"API key validation failed","locations":[{"line":1,"column":3}],"path":["me"],"extensions":{"code":"UNAUTHENTICATED","originalError":{"message":"API key validation failed","error":"Unauthorized","statusCode":401}}}],"data":null}""", + ) + private val forbidden = reply( + """{"errors":[{"message":"Forbidden resource","locations":[{"line":1,"column":3}],"path":["me"],"extensions":{"code":"FORBIDDEN","originalError":{"message":"Forbidden resource","error":"Forbidden","statusCode":403}}}],"data":null}""", + ) + private val validation400 = reply( + """{"errors":[{"message":"Cannot query field \"core\" on type \"InfoVersions\".","locations":[{"line":1,"column":12}],"extensions":{"code":"GRAPHQL_VALIDATION_FAILED"}}]}""", + status = 400, + type = "application/graphql-response+json", + ) + private val resolverError = reply( + """{"errors":[{"message":"boom","extensions":{"code":"INTERNAL_SERVER_ERROR"}}],"data":null}""", + ) + private val unreachable: () -> HttpResponse = { throw IOException("connection refused") } + + private fun client(engine: HttpEngine) = + ApolloClient.Builder().serverUrl("http://unraid.test/graphql").httpEngine(engine).build() + + private suspend fun probe(version: () -> HttpResponse, access: () -> HttpResponse): Pair { + val engine = Engine(version, access) + return probeCapabilities(client(engine)) to engine + } + + // WHY: #315 — a read-only key reads everything and must not be offered buttons. + @Test fun viewerKey_isDetected_withVersion() = runTest { + val (out, _) = probe(versionOk, viewerMe) + assertEquals(Detection.Done, out.detection) + assertEquals("7.4.0", out.unraidVersion) + assertEquals("4.37.11+8e587fe2", out.apiVersionText) + assertEquals(ApiVersion(4, 37, 11), out.apiVersion) + assertEquals(KeyAccess.Known(setOf("VIEWER"), emptySet()), out.access) + val caps = out.toCapabilities(null) + assertTrue(KeyAction.entries.all { caps.access(it) == Access.Denied }) + assertEquals(Support.Yes, caps.supports(ApiFeature.DockerRestart)) + } + + @Test fun adminKey_isDetected() = runTest { + val (out, _) = probe(versionOk, adminMe) + assertTrue(KeyAction.entries.all { out.toCapabilities(null).access(it) == Access.Allowed }) + } + + // WHY: explicit permissions are reported as RESOURCE:ACTION, the same spelling KeyAction uses. + @Test fun explicitPermissions_becomeGrants() = runTest { + val me = reply("""{"data":{"me":{"roles":["VIEWER"],"permissions":[{"resource":"DOCKER","actions":["UPDATE_ANY","READ_ANY"]},{"resource":"VMS","actions":["UPDATE_OWN"]}]}}}""") + val (out, _) = probe(versionOk, me) + assertEquals( + KeyAccess.Known(setOf("VIEWER"), setOf("DOCKER:UPDATE_ANY", "DOCKER:READ_ANY", "VMS:UPDATE_OWN")), + out.access, + ) + } + + // WHY: a role a newer Unraid adds must stay "unknown", never "denied". Apollo hands an enum value that + // is not in the client schema over as UNKNOWN__ (the raw name is not kept), which the role table + // does not know either, so the answer is Unknown. + @Test fun futureRole_staysUnknown() = runTest { + val me = reply("""{"data":{"me":{"roles":["FUTURE_ROLE"],"permissions":[]}}}""") + val (out, _) = probe(versionOk, me) + assertEquals(Detection.Done, out.detection) + assertEquals(KeyAccess.Known(setOf("UNKNOWN__"), emptySet()), out.access) + assertTrue(KeyAction.entries.all { out.toCapabilities(null).access(it) == Access.Unknown }) + } + + // WHY: the same, next to a known role that grants nothing: still "unknown", not "denied". + @Test fun futureRoleNextToViewer_staysUnknown() = runTest { + val me = reply("""{"data":{"me":{"roles":["VIEWER","FUTURE_ROLE"],"permissions":[]}}}""") + val (out, _) = probe(versionOk, me) + assertTrue(KeyAction.entries.all { out.toCapabilities(null).access(it) == Access.Unknown }) + } + + // WHY: #315 "If the permission query fails, the app behaves as today": a refused `me` hides nothing. + @Test fun forbiddenMe_failsOpen_andKeepsTheVersion() = runTest { + val (out, _) = probe(versionOk, forbidden) + assertEquals(Detection.Done, out.detection) + assertEquals(KeyAccess.Unknown, out.access) + assertEquals(ApiVersion(4, 37, 11), out.apiVersion) + assertTrue(KeyAction.entries.all { out.toCapabilities(null).access(it) == Access.Unknown }) + } + + @Test fun otherErrorOnMe_failsOpen() = runTest { + val (out, _) = probe(versionOk, resolverError) + assertEquals(Detection.Done, out.detection) + assertEquals(KeyAccess.Unknown, out.access) + } + + // WHY: a wrong key must be reported as rejected, and only because the server said so. + @Test fun unauthenticated_isKeyRejected_andSkipsTheSecondQuery() = runTest { + val (out, engine) = probe(unauthenticated, viewerMe) + assertEquals(Detection.KeyRejected, out.detection) + assertEquals(KeyAccess.Unknown, out.access) + assertEquals(listOf("GetApiVersion"), engine.operations) + } + + @Test fun unauthenticatedOnMe_isKeyRejected() = runTest { + val (out, _) = probe(versionOk, unauthenticated) + assertEquals(Detection.KeyRejected, out.detection) + } + + // WHY: #247 — an unknown field is HTTP 400 with a validation error; it means an API older than 4.15.0. + @Test fun versionValidationError_meansTheApiIsTooOld() = runTest { + val (out, _) = probe(validation400, validation400) + assertEquals(Detection.Done, out.detection) + assertTrue(out.versionFieldMissing) + val caps = out.toCapabilities(null) + assertTrue(caps.apiTooOld) + assertEquals(Support.No, caps.supports(ApiFeature.DockerRestart)) + assertEquals(KeyAccess.Unknown, out.access) + } + + // WHY: a role-less key without INFO is FORBIDDEN on the version; we still learn what we can from `me`. + @Test fun forbiddenVersion_isUnknown_notTooOld() = runTest { + val (out, engine) = probe(forbidden, viewerMe) + assertEquals(Detection.Done, out.detection) + assertEquals(null, out.apiVersion) + assertEquals(false, out.versionFieldMissing) + assertEquals(false, out.toCapabilities(null).apiTooOld) + assertEquals(KeyAccess.Known(setOf("VIEWER"), emptySet()), out.access) + assertEquals(listOf("GetApiVersion", "GetKeyAccess"), engine.operations) + } + + // WHY: an unreachable server is "could not detect", not "key rejected" and not "denied". + @Test fun transportFailure_isFailed_andHidesNothing() = runTest { + val (out, engine) = probe(unreachable, unreachable) + assertEquals(Detection.Failed, out.detection) + assertEquals(listOf("GetApiVersion"), engine.operations) + val caps = out.toCapabilities(null) + assertTrue(KeyAction.entries.all { caps.access(it) == Access.Unknown }) + assertEquals(false, caps.apiTooOld) + } + + @Test fun serverErrorOnVersion_isFailed() = runTest { + assertEquals(Detection.Failed, probe(resolverError, viewerMe).first.detection) + } + + // WHY: the version is worth keeping when only the second query fails. + @Test fun transportFailureOnMe_keepsTheVersion() = runTest { + val (out, _) = probe(versionOk, unreachable) + assertEquals(Detection.Done, out.detection) + assertEquals(ApiVersion(4, 37, 11), out.apiVersion) + assertEquals(KeyAccess.Unknown, out.access) + } + + // WHY: a body that is not GraphQL (a proxy, a captive portal) must not be taken for a verdict. + @Test fun nonGraphqlHttpError_isFailed() = runTest { + val (out, _) = probe(reply("nope", status = 502, type = "text/html"), viewerMe) + assertEquals(Detection.Failed, out.detection) + } + + @Test fun classifyErrors_firstMatchWins() { + fun err(code: String?, msg: String = "x") = com.apollographql.apollo.api.Error.Builder(msg) + .apply { if (code != null) extensions(mapOf("code" to code)) }.build() + assertEquals(ErrorClass.KeyRejected, classifyErrors(listOf(err("FORBIDDEN"), err("UNAUTHENTICATED")))) + assertEquals(ErrorClass.ValidationFailed, classifyErrors(listOf(err(null, "Cannot query field \"a\" on type \"B\".")))) + assertEquals(ErrorClass.ValidationFailed, classifyErrors(listOf(err("GRAPHQL_VALIDATION_FAILED")))) + assertEquals(ErrorClass.Forbidden, classifyErrors(listOf(err("FORBIDDEN")))) + assertEquals(ErrorClass.Other, classifyErrors(listOf(err("INTERNAL_SERVER_ERROR")))) + } +} diff --git a/app/src/test/kotlin/io/github/nofuturekid/nova/data/repository/CapabilityRepositoryTest.kt b/app/src/test/kotlin/io/github/nofuturekid/nova/data/repository/CapabilityRepositoryTest.kt new file mode 100644 index 00000000..79bffac1 --- /dev/null +++ b/app/src/test/kotlin/io/github/nofuturekid/nova/data/repository/CapabilityRepositoryTest.kt @@ -0,0 +1,272 @@ +package io.github.nofuturekid.nova.data.repository + +import io.github.nofuturekid.nova.data.local.ApiKeyResult +import io.github.nofuturekid.nova.data.model.Access +import io.github.nofuturekid.nova.data.model.ApiVersion +import io.github.nofuturekid.nova.data.model.CapabilityTarget +import io.github.nofuturekid.nova.data.model.ConnectionMode +import io.github.nofuturekid.nova.data.model.Detection +import io.github.nofuturekid.nova.data.model.KeyAccess +import io.github.nofuturekid.nova.data.model.KeyAction +import io.github.nofuturekid.nova.data.model.Server +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Everything here runs at one point of virtual time (there are no delays to wait for), so + * `runCurrent()` settles it. `advanceUntilIdle()` would skip the repository's work: it runs in + * `backgroundScope`, which that call deliberately ignores. + * + * ADR-0047. These tests pin WHEN detection runs (target change, refresh, start; never on a timer) + * and that a failure or a stale answer can never change what the user sees. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class CapabilityRepositoryTest { + + private fun active(id: String = "s1", name: String = "Tower", url: String = "http://tower", key: String = "key-1") = + ActiveServer( + server = Server(id = id, name = name, hostname = name, localUrl = url, remoteUrl = ""), + mode = ConnectionMode.Local, + apiKey = key, + keyState = if (key.isBlank()) ApiKeyResult.Absent else ApiKeyResult.Present(key), + ) + + private val targetOf: (ActiveServer?) -> CapabilityTarget? = { a -> + a?.takeIf { it.apiKey.isNotBlank() }?.let { CapabilityTarget(it.server.id, it.server.localUrl, it.apiKey) } + } + + private val viewer = ProbeOutcome( + Detection.Done, "7.4.0", "4.37.11+8e587fe2", ApiVersion(4, 37, 11), + access = KeyAccess.Known(setOf("VIEWER"), emptySet()), + ) + private val admin = viewer.copy(access = KeyAccess.Known(setOf("ADMIN"), emptySet())) + private val failed = ProbeOutcome(Detection.Failed) + + /** Probe that answers from a queue (the last answer repeats) and counts its calls. */ + private class Script(vararg answers: suspend () -> ProbeOutcome) { + private val queue = answers.toList() + var calls = 0 + val seen = mutableListOf() + suspend fun run(a: ActiveServer): ProbeOutcome { + seen += a + val answer = queue[minOf(calls, queue.size - 1)] + calls++ + return answer() + } + } + + private fun answer(o: ProbeOutcome): suspend () -> ProbeOutcome = { o } + + private fun TestScope.repo(source: MutableStateFlow, script: Script) = + CapabilityRepository(backgroundScope, source, targetOf, script::run) + + // WHY: before any detection the answer is unknown, which shows the controls as today. + @Test fun startsUnknown_andWithoutKeyNeverProbes() = runTest { + val script = Script(answer(viewer)) + val r = repo(MutableStateFlow(active(key = "")), script) + runCurrent() + assertEquals(Detection.Idle, r.capabilities.value.detection) + assertNull(r.capabilities.value.target) + assertEquals(0, script.calls) + } + + @Test fun detectsOnceForTheActiveServer() = runTest { + val script = Script(answer(viewer)) + val r = repo(MutableStateFlow(active()), script) + runCurrent() + assertEquals(1, script.calls) + assertEquals(Detection.Done, r.capabilities.value.detection) + assertEquals(Access.Denied, r.capabilities.value.access(KeyAction.ControlDocker)) + } + + // WHY: one server's "denied" must never gate another server while the second one is still unknown. + @Test fun newTargetResetsToUnknownFirst() = runTest { + val gate = CompletableDeferred() + val source = MutableStateFlow(active("a", key = "ka")) + val r = repo(source, Script(answer(viewer), { gate.await() })) + runCurrent() + assertEquals(Access.Denied, r.capabilities.value.access(KeyAction.ControlDocker)) + + source.value = active("b", url = "http://other", key = "kb") + runCurrent() + val now = r.capabilities.value + assertEquals(Detection.Detecting, now.detection) + assertEquals("b", now.target?.serverId) + assertTrue(KeyAction.entries.all { now.access(it) == Access.Unknown }) + + gate.complete(admin) + runCurrent() + assertEquals(Access.Allowed, r.capabilities.value.access(KeyAction.ControlDocker)) + } + + // WHY: only server, address or key start a detection; a rename or accent change is not worth two requests. + @Test fun unrelatedServerChangeDoesNotReprobe() = runTest { + val script = Script(answer(viewer)) + val source = MutableStateFlow(active(name = "Tower")) + repo(source, script) + runCurrent() + source.value = active(name = "Renamed tower") + runCurrent() + assertEquals(1, script.calls) + } + + @Test fun changedKeyOrAddressReprobes() = runTest { + val script = Script(answer(viewer)) + val source = MutableStateFlow(active()) + repo(source, script) + runCurrent() + source.value = active(key = "key-2") + runCurrent() + source.value = active(key = "key-2", url = "http://tower.example") + runCurrent() + assertEquals(3, script.calls) + } + + // WHY: pull-to-refresh and foreground can arrive together; never two detections in parallel. + @Test fun refreshJoinsRunningDetection() = runTest { + val gate = CompletableDeferred() + val script = Script({ gate.await() }) + val r = repo(MutableStateFlow(active()), script) + runCurrent() + r.refresh() + r.refresh() + runCurrent() + assertEquals(1, script.calls) + gate.complete(viewer) + runCurrent() + assertEquals(1, script.calls) + assertEquals(Detection.Done, r.capabilities.value.detection) + } + + // WHY: the user widens the key in the web UI (same key string); refresh must pick that up. + @Test fun refreshAfterDone_detectsAgain_andShowsTheNewAccess() = runTest { + val script = Script(answer(viewer), answer(admin)) + val r = repo(MutableStateFlow(active()), script) + runCurrent() + assertEquals(Access.Denied, r.capabilities.value.access(KeyAction.ControlVms)) + r.refresh() + runCurrent() + assertEquals(2, script.calls) + assertEquals(Access.Allowed, r.capabilities.value.access(KeyAction.ControlVms)) + } + + // WHY: while a re-detection runs, the last good result stays (no flicker of buttons). + @Test fun lastResultStaysVisibleWhileRedetecting() = runTest { + val gate = CompletableDeferred() + val script = Script(answer(viewer), { gate.await() }) + val r = repo(MutableStateFlow(active()), script) + runCurrent() + r.refresh() + runCurrent() + assertEquals(Detection.Done, r.capabilities.value.detection) + assertEquals(Access.Denied, r.capabilities.value.access(KeyAction.ControlDocker)) + gate.complete(viewer) + } + + // WHY: a transient error must not toggle controls. + @Test fun failedRedetectionKeepsLastResult() = runTest { + val script = Script(answer(viewer), answer(failed)) + val r = repo(MutableStateFlow(active()), script) + runCurrent() + val before = r.capabilities.value + r.refresh() + runCurrent() + assertEquals(2, script.calls) + assertEquals(before, r.capabilities.value) + } + + // WHY: a first detection that fails is "unknown", so the UI fails open. + @Test fun failedFirstDetection_isFailed_andUnknown() = runTest { + val r = repo(MutableStateFlow(active()), Script(answer(failed))) + runCurrent() + val caps = r.capabilities.value + assertEquals(Detection.Failed, caps.detection) + assertTrue(KeyAction.entries.all { caps.access(it) == Access.Unknown }) + } + + @Test fun probeThatThrows_isFailed_notACrash() = runTest { + val r = repo(MutableStateFlow(active()), Script({ throw IllegalStateException("boom") })) + runCurrent() + assertEquals(Detection.Failed, r.capabilities.value.detection) + } + + // WHY: the server said UNAUTHENTICATED; the next detection with a fixed key must clear it. + @Test fun keyRejected_thenAccepted() = runTest { + val script = Script(answer(ProbeOutcome(Detection.KeyRejected)), answer(viewer)) + val r = repo(MutableStateFlow(active()), script) + runCurrent() + assertEquals(Detection.KeyRejected, r.capabilities.value.detection) + r.refresh() + runCurrent() + assertEquals(Detection.Done, r.capabilities.value.detection) + } + + // WHY: an answer for a server the user has already left must not land on the new one. + @Test fun staleAnswerIsDropped() = runTest { + val gateA = CompletableDeferred() + val script = Script({ gateA.await() }, answer(viewer)) + val source = MutableStateFlow(active("a", key = "ka")) + val r = repo(source, script) + runCurrent() + source.value = active("b", url = "http://other", key = "kb") + runCurrent() + gateA.complete(admin) // A answers late + runCurrent() + assertEquals("b", r.capabilities.value.target?.serverId) + assertEquals(Access.Denied, r.capabilities.value.access(KeyAction.ControlDocker)) + } + + @Test fun removingTheKeyResetsToUnknown() = runTest { + val source = MutableStateFlow(active()) + val r = repo(source, Script(answer(viewer))) + runCurrent() + source.value = active(key = "") + runCurrent() + assertEquals(Detection.Idle, r.capabilities.value.detection) + assertTrue(KeyAction.entries.all { r.capabilities.value.access(it) == Access.Unknown }) + } + + // WHY: the probe must use the newest server settings (e.g. a certificate pin accepted after a failed + // first detection) although the target is unchanged. + @Test fun refreshUsesTheNewestServerSettings() = runTest { + val script = Script(answer(failed), answer(viewer)) + val source = MutableStateFlow(active()) + val r = repo(source, script) + runCurrent() + source.value = active().copy(localCertPin = "pinned") + runCurrent() + assertEquals(1, script.calls) + r.refresh() + runCurrent() + assertEquals("pinned", script.seen.last().localCertPin) + } + + // WHY: no new background work (the maintainer's rule): nothing re-detects by itself. + @Test fun noProbeWithoutTrigger() = runTest { + val script = Script(answer(viewer)) + repo(MutableStateFlow(active()), script) + runCurrent() + advanceTimeBy(60 * 60 * 1000L) + runCurrent() + assertEquals(1, script.calls) + } + + // WHY: a failed detection must not turn into a retry loop either. + @Test fun noRetryAfterFailure() = runTest { + val script = Script(answer(failed)) + repo(MutableStateFlow(active()), script) + runCurrent() + advanceTimeBy(60 * 60 * 1000L) + runCurrent() + assertEquals(1, script.calls) + } +} diff --git a/docs/adr/0047-server-capabilities.md b/docs/adr/0047-server-capabilities.md new file mode 100644 index 00000000..1d257daa --- /dev/null +++ b/docs/adr/0047-server-capabilities.md @@ -0,0 +1,53 @@ +# ADR-0047: Detect the server's API version and what the API key may do + +- **Status**: Accepted (2026-11-13) +- **Date**: 2026-11-13 +- **Tags**: data, api, architecture + +## Context + +NOVA talks to the Unraid GraphQL API with one API key per server. Two facts about that API were not visible to the app so far: + +- **The API version.** NOVA needs API 4.30.0 or newer (4.35.0 for live network speed, 4.36.0 for `docker.restart`). An older server answers a query with an HTTP 400 validation error, which the user saw as a raw error text (#247). +- **What the key may do.** A key with the `VIEWER` role reads everything NOVA shows but every action is refused with `FORBIDDEN` (#315). The app offered the buttons anyway. + +What was verified against a real server (Unraid 7.4.0, API `4.37.11+8e587fe2`, read-only `VIEWER` key, 2026-11-13) and against the upstream source: + +- `info { versions { core { api } } }` returns the API version with a build suffix (`4.37.11+8e587fe2`); the field exists since API 4.15.0. `me { roles permissions { resource actions } }` returns `{"roles":["VIEWER"],"permissions":[]}`: `me` describes the API key, not a signed-in user, and `permissions` lists only explicit permissions. +- The role policy of the API (`ADMIN` everything, `VIEWER` read on everything except `API_KEY`, `CONNECT`, `GUEST`) is identical from 4.30.0 to 4.37.11. The server compares actions exactly: `UPDATE_ANY` does not imply `READ_ANY`, and `*_OWN` never satisfies an `*_ANY` check. +- `me` is non-null: a `FORBIDDEN` on it nulls the whole response, so it must not share a request with the version. +- A wrong key gets `UNAUTHENTICATED` with HTTP 200 on every query except `__typename`, which is what the existing `Ping` sends. An unknown field gets HTTP 400 with `GRAPHQL_VALIDATION_FAILED`. +- Introspection is blocked on a stock 7.4.0 server (sandbox off), so the schema cannot be asked what it supports. + +## Decision + +Detect two things per server, with two small queries, keep the result in memory, and make it available as state. This change adds the foundation only; no screen reads it yet. + +- **Model** (`data/model/ServerCapabilities.kt`, pure Kotlin): `ServerCapabilities` holds the API version (parsed; a build suffix is ignored when comparing), the key's roles and explicit grants, and a `Detection` state. `access(KeyAction)` answers `Allowed`, `Denied` or `Unknown` from the static role table plus the explicit grants; `supports(ApiFeature)` answers `Yes`, `No` or `Unknown` against the floors 4.30.0, 4.35.0 and 4.36.0. `KeyAction` is the single list of controls NOVA offers and the permission the server checks for each; a unit test requires that every mutation in `mutations.graphql` belongs to exactly one, so a new control cannot slip past hiding or the documentation. +- **Detection** (`GetApiVersion`, `GetKeyAccess` in `queries.graphql`): two operations on purpose, so a refused `me` does not lose the version. `CapabilityProbe.kt` sends them and classifies the answers by `extensions.code`: `UNAUTHENTICATED` is a rejected key, `GRAPHQL_VALIDATION_FAILED` on the version means an API older than 4.15.0, `FORBIDDEN` leaves the access unknown, transport and other errors are a failed detection. The mapping lives in its own file, not in `GraphQlMapper.kt`. +- **Wiring** (`CapabilityRepository`, a singleton built like `UpdateTracker`; `MainViewModel.capabilities`): detection runs when the target (server id, resolved address, API key) changes, on pull-to-refresh, and when the app returns to the foreground. There is no timer, no retry loop and no polling, so nothing runs in the background. +- **Fail open.** Unknown is never denied. A detection that fails, a `me` that is refused and a role NOVA does not know all leave the access `Unknown`; a later UI shows controls unless the access is `Denied`, which only a successful detection can say. A failed re-detection keeps the last good result for the same target; a new target starts unknown, so one server's result never gates another. +- **Memory only.** Not stored in DataStore or in `Server`: a server upgrade or a changed key would leave a stored result stale. + +Domain availability (is Docker or the VM service turned on, does the server have an array) is deliberately not part of this model. It would cost more queries, goes stale when the user switches a service on, and each tab's own query answers it on every poll anyway. + +## Consequences + +- **Positive** — one place that knows the API version and the key's access; later changes (hiding controls a key cannot use, the atomic container restart, the Add server test, per-tab error reasons) read it instead of each probing the server. No visible change in this step. +- **Negative / trade-offs** — two more small requests per server change, pull-to-refresh and return to the foreground. The static role table copies the server's policy and can drift; an unknown role fails open, so drift can only show a button the server then refuses, never hide one the key may use (exception: a role that loses rights it has today). The CONNECT role is mirrored from the upstream policy but NOVA's detection of it is unverified on a live server. +- **Deviation from the first design, on purpose.** The detection pipeline lives in the application scope for as long as the repository exists, instead of being shared only while a screen collects it. The return to the foreground arrives together with the screen's own re-subscription, so a refresh that depended on an active collector could be lost. The foreground refresh fires only when the app was really away (not at the first start, which detects by itself), so it does not double the first detection. +- **Trigger to revisit** — if the Unraid API exposes a supported way to ask for the key's effective permissions without `PERMISSION:READ_ANY`, replace the static role table; if the role policy changes upstream, update the table and the test that pins it; if a feature must be gated that has no version floor, revisit the "no introspection" rule once introspection is allowed by default. + +## Alternatives considered + +- **Introspection of the schema.** Would tell exactly what the server has, but a stock Unraid 7.4.0 blocks it unless the developer sandbox is on, and 4.37.8 and newer also block `__type`. Version floors work everywhere. +- **Probing with a mutation to see whether it is refused.** Rejected: a probe that can change a server is not acceptable, and the server's `FORBIDDEN` for a real action is shown to the user anyway. +- **One query for version and access.** Rejected: `me` is non-null, so a key without `ME:READ_ANY` would lose the version as well. +- **`getPermissionsForRoles` from the API.** Needs `PERMISSION:READ_ANY`, which a read-only key does not have. The client computes the effective rights from the role table instead. +- **Detecting on a timer.** Rejected: nothing in this design runs in the background; the three triggers cover every real change (a key's permissions change only when the user edits them in the web UI, then returns to NOVA). +- **Storing the result.** Rejected, see Decision. + +## References + +- Issues #315 (read-only keys), #247 (older API versions), #317 (container restart), #316 (documentation of key permissions). +- Related: ADR-0035 (API key storage), `UpdateTracker` (same in-memory singleton pattern). diff --git a/docs/adr/README.md b/docs/adr/README.md index 4d60734c..7a658e70 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -81,4 +81,5 @@ ADRs live under `docs/` so the CI docs-only bypass applies — pure-ADR PRs comp | [0044](./0044-container-detail-live-stats-overlay.md) | Surface live CPU/Mem/Net/Disk in the container detail sheet from the docker stats overlay | Accepted | | [0045](./0045-array-disk-display.md) | Array disk display — sleeping disks, per-disk thresholds, errors | Accepted (updated beta10 — 2026-05-30) | | [0046](./0046-update-via-browser-download.md) | Offer updates of the GitHub build as a browser download, without an in-app installer | Accepted | +| [0047](./0047-server-capabilities.md) | Detect the server's API version and what the API key may do | Accepted | | [0049](./0049-google-play-third-distribution-channel.md) | Google Play as a third distribution channel | Accepted | From 6aec2a09d4a3691dce3591489fbf001ddd6d271a Mon Sep 17 00:00:00 2001 From: Thomas Kroll Date: Fri, 13 Nov 2026 18:19:31 +0100 Subject: [PATCH 2/4] address review findings Refresh the capabilities on every ON_START so a fresh MainViewModel after a finished activity re-detects (the repository joins a running detection, so the first start does not probe twice). Correct the schema comment about unknown enum values (UNKNOWN__, no raw name) and the CapabilityProbeTest KDoc about which bodies are recorded and which are constructed. ADR-0047 follows. Co-Authored-By: Claude Sonnet 5.5 --- .../graphql/io/github/nofuturekid/nova/schema.graphqls | 2 +- .../nofuturekid/nova/ui/screens/main/MainViewModel.kt | 7 ++++--- .../nova/data/repository/CapabilityProbeTest.kt | 7 ++++--- docs/adr/0047-server-capabilities.md | 2 +- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/app/src/main/graphql/io/github/nofuturekid/nova/schema.graphqls b/app/src/main/graphql/io/github/nofuturekid/nova/schema.graphqls index 1db03463..82f17da0 100644 --- a/app/src/main/graphql/io/github/nofuturekid/nova/schema.graphqls +++ b/app/src/main/graphql/io/github/nofuturekid/nova/schema.graphqls @@ -571,7 +571,7 @@ type DockerContainerStats { # ── API key access (capability detection, ADR-0047) ────────────────── # Subset of the live Unraid `me` query. `me` describes the API key, not a # signed-in user. Enum values are the Unraid 4.37.11 schema; a value a newer -# server adds arrives as an unknown enum value and keeps its raw name. +# server adds arrives as UNKNOWN__ (Apollo keeps no raw name), which NOVA treats as unknown. type UserAccount { roles: [Role!]! diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/main/MainViewModel.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/main/MainViewModel.kt index 68c8fb45..dd49a1e7 100644 --- a/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/main/MainViewModel.kt +++ b/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/main/MainViewModel.kt @@ -167,9 +167,10 @@ class MainViewModel @Inject constructor( private val _appVisible = MutableStateFlow(true) fun setAppVisible(visible: Boolean) { - // Returning to the foreground (not the first start, which detects by itself): the user may - // have changed the key's permissions in the Unraid web UI while NOVA was away (ADR-0047). - if (visible && !_appVisible.value) caps.refresh() + // Every ON_START, including a fresh MainViewModel after the activity was finished while the + // process lived: the user may have changed the key's permissions in the Unraid web UI (ADR-0047). + // refresh() joins a detection that is already running, so the first start does not probe twice. + if (visible) caps.refresh() _appVisible.value = visible } diff --git a/app/src/test/kotlin/io/github/nofuturekid/nova/data/repository/CapabilityProbeTest.kt b/app/src/test/kotlin/io/github/nofuturekid/nova/data/repository/CapabilityProbeTest.kt index 2dde8aca..87751705 100644 --- a/app/src/test/kotlin/io/github/nofuturekid/nova/data/repository/CapabilityProbeTest.kt +++ b/app/src/test/kotlin/io/github/nofuturekid/nova/data/repository/CapabilityProbeTest.kt @@ -19,9 +19,10 @@ import org.junit.Test import java.io.IOException /** - * ADR-0047. Every body below is copied from a real answer of an Unraid 7.4.0 server (API 4.37.11) - * recorded on 2026-11-13, so each test fails if the classification stops matching what the server - * really sends. They run on a scripted HTTP engine, i.e. through the real Apollo response path. + * ADR-0047. versionOk, viewerMe, adminMe and unauthenticated are copied verbatim from real answers + * of an Unraid 7.4.0 server (API 4.37.11) recorded on 2026-11-13; forbidden and validation400 keep + * the real shape with the path or field adapted to these operations; resolverError and the + * explicit-permission and FUTURE_ROLE bodies are constructed. They run on a scripted HTTP engine, i.e. through the real Apollo response path. */ class CapabilityProbeTest { diff --git a/docs/adr/0047-server-capabilities.md b/docs/adr/0047-server-capabilities.md index 1d257daa..e1ca97f9 100644 --- a/docs/adr/0047-server-capabilities.md +++ b/docs/adr/0047-server-capabilities.md @@ -35,7 +35,7 @@ Domain availability (is Docker or the VM service turned on, does the server have - **Positive** — one place that knows the API version and the key's access; later changes (hiding controls a key cannot use, the atomic container restart, the Add server test, per-tab error reasons) read it instead of each probing the server. No visible change in this step. - **Negative / trade-offs** — two more small requests per server change, pull-to-refresh and return to the foreground. The static role table copies the server's policy and can drift; an unknown role fails open, so drift can only show a button the server then refuses, never hide one the key may use (exception: a role that loses rights it has today). The CONNECT role is mirrored from the upstream policy but NOVA's detection of it is unverified on a live server. -- **Deviation from the first design, on purpose.** The detection pipeline lives in the application scope for as long as the repository exists, instead of being shared only while a screen collects it. The return to the foreground arrives together with the screen's own re-subscription, so a refresh that depended on an active collector could be lost. The foreground refresh fires only when the app was really away (not at the first start, which detects by itself), so it does not double the first detection. +- **Deviation from the first design, on purpose.** The detection pipeline lives in the application scope for as long as the repository exists, instead of being shared only while a screen collects it. The return to the foreground arrives together with the screen's own re-subscription, so a refresh that depended on an active collector could be lost. The foreground refresh fires on every start of the main screen; it joins a detection that is already running, so the first start does not probe twice. - **Trigger to revisit** — if the Unraid API exposes a supported way to ask for the key's effective permissions without `PERMISSION:READ_ANY`, replace the static role table; if the role policy changes upstream, update the table and the test that pins it; if a feature must be gated that has no version floor, revisit the "no introspection" rule once introspection is allowed by default. ## Alternatives considered From 88f918585cdc7eaeb4027b034f4ed136edde3e40 Mon Sep 17 00:00:00 2001 From: Thomas Kroll Date: Fri, 13 Nov 2026 18:10:53 +0100 Subject: [PATCH 3/4] feat: decide which server events raise an alert and remember what was already reported First part of alerts outside the app (#321): the pure decision logic and its stored state. Nothing runs in the background yet. evaluateAlerts takes what one check of one server returned and the remembered state, and returns the alerts to post and the next state. New unread alert and warning notifications are posted once per id; the array conditions "not started" and "degraded" (the rules of attentionItems, wrapped) are posted when they begin and again only after they were gone. The first successful read only records, a failed read changes nothing, the level filters, at most five alerts per server and check get their own notification, and the remembered ids are capped. The state is stored per server in the existing DataStore and removed with the server entry. ServerProbe is the interface for fetching one check once; only a test fake exists. ADR-0048 records the decision and amends ADR-0017. Co-Authored-By: Claude Sonnet 5.5 --- .../nova/data/alerts/AlertRules.kt | 168 ++++++++ .../nova/data/alerts/AlertState.kt | 25 ++ .../nova/data/alerts/AlertStateStore.kt | 31 ++ .../nova/data/alerts/ServerProbe.kt | 36 ++ .../nova/data/local/SettingsStore.kt | 30 ++ .../nova/data/repository/ServerRepository.kt | 1 + .../nova/data/alerts/AlertCheckCycleTest.kt | 130 +++++++ .../nova/data/alerts/AlertFakes.kt | 27 ++ .../nova/data/alerts/AlertRulesTest.kt | 360 ++++++++++++++++++ .../nova/data/alerts/AlertStateStoreTest.kt | 112 ++++++ ...in-split-queries-with-lifecycle-polling.md | 4 + .../0048-opt-in-background-alert-checks.md | 82 ++++ docs/adr/README.md | 3 +- 13 files changed, 1008 insertions(+), 1 deletion(-) create mode 100644 app/src/main/kotlin/io/github/nofuturekid/nova/data/alerts/AlertRules.kt create mode 100644 app/src/main/kotlin/io/github/nofuturekid/nova/data/alerts/AlertState.kt create mode 100644 app/src/main/kotlin/io/github/nofuturekid/nova/data/alerts/AlertStateStore.kt create mode 100644 app/src/main/kotlin/io/github/nofuturekid/nova/data/alerts/ServerProbe.kt create mode 100644 app/src/test/kotlin/io/github/nofuturekid/nova/data/alerts/AlertCheckCycleTest.kt create mode 100644 app/src/test/kotlin/io/github/nofuturekid/nova/data/alerts/AlertFakes.kt create mode 100644 app/src/test/kotlin/io/github/nofuturekid/nova/data/alerts/AlertRulesTest.kt create mode 100644 app/src/test/kotlin/io/github/nofuturekid/nova/data/alerts/AlertStateStoreTest.kt create mode 100644 docs/adr/0048-opt-in-background-alert-checks.md diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/data/alerts/AlertRules.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/data/alerts/AlertRules.kt new file mode 100644 index 00000000..19f1d196 --- /dev/null +++ b/app/src/main/kotlin/io/github/nofuturekid/nova/data/alerts/AlertRules.kt @@ -0,0 +1,168 @@ +package io.github.nofuturekid.nova.data.alerts + +import io.github.nofuturekid.nova.data.model.ArrayInfo +import io.github.nofuturekid.nova.data.model.AttentionSeverity +import io.github.nofuturekid.nova.data.model.AttentionSnapshot +import io.github.nofuturekid.nova.data.model.AttentionTarget +import io.github.nofuturekid.nova.data.model.NotifImportance +import io.github.nofuturekid.nova.data.model.Notifications +import io.github.nofuturekid.nova.data.model.UnraidNotification +import io.github.nofuturekid.nova.data.model.attentionItems + +/** + * The array conditions that become alerts. Unraid itself never notifies about these + * (it stays silent about disks while the array is stopped), so NOVA raises them. + * + * Deliberately not widened to disk temperature, disk errors or parity items: Unraid + * already sends those as notifications, and raising them again would post every + * problem twice. A test fails if this set grows without that decision. + */ +val STATE_ALERT_KEYS: Set = setOf("array-stopped", "array-degraded") + +/** At most this many alerts are posted per server and check; the rest are folded into a summary. */ +const val MAX_ALERTS_PER_CHECK = 5 + +/** What the user wants to hear about. Critical things are always included. */ +enum class AlertLevel { AlertsOnly, AlertsAndWarnings } + +/** The server an evaluation is for: the stable id keys the state, the name is for display. */ +data class ServerRef(val id: String, val name: String) + +/** Where an [Alert] comes from. */ +enum class AlertSource { Notification, ArrayCondition } + +/** + * One alert to post now. [key] is the notification id or the condition key; it is + * stable, so the poster can derive a stable Android notification id from server id + key. + */ +data class Alert( + val key: String, + val source: AlertSource, + val severity: AttentionSeverity, + val title: String, + val text: String, + /** The server's ISO timestamp for a notification; null for a condition. */ + val timestamp: String?, + val target: AttentionTarget, +) + +/** + * Result of one evaluation. [toPost] holds at most [MAX_ALERTS_PER_CHECK] alerts, most + * urgent first; [foldedCount] more qualified but are only worth a summary line + * ([foldedSummaryText]). [nextState] must be stored whatever was posted: ids that were + * folded or filtered out count as reported. + */ +data class AlertEvaluation( + val server: ServerRef, + val toPost: List, + val foldedCount: Int, + val nextState: ServerAlertState, +) + +/** The summary line for alerts that did not get their own notification, e.g. "and 3 more on Tower". */ +fun foldedSummaryText(serverName: String, count: Int): String = "and $count more on $serverName" + +/** + * Decides which alerts to post after one check of one server, and what to remember. + * Pure: no Android, no clock, no I/O. + * + * [notifications] and [array] are what the check returned; null means "not loaded this + * time" (server unreachable, query failed). A null half changes nothing and posts nothing, + * so alerts created during an outage are posted once the server answers again, and nothing + * is posted twice. + * + * Notifications: every unread notification of importance alert or warning whose id is not + * remembered is new. The very first successful read only remembers the ids, so switching + * alerts on does not dump everything already unread on the user. An id is reported once, + * even if it leaves the unread list and returns. A later notification with the same text + * has a new id (Unraid builds it from the time) and is reported, as it should be. + * + * Array conditions: raised when they begin, not while they last, and again only after they + * were gone at an in-between read. The rules are those of [attentionItems], wrapped and + * restricted to [STATE_ALERT_KEYS], so the wording is the Overview card's own. + * + * [level]: [AlertLevel.AlertsOnly] posts only critical items. Items it filters out are still + * remembered, so switching to [AlertLevel.AlertsAndWarnings] later does not post old ones. + */ +fun evaluateAlerts( + server: ServerRef, + previous: ServerAlertState, + notifications: Notifications?, + array: ArrayInfo?, + level: AlertLevel, +): AlertEvaluation { + val (conditionAlerts, afterConditions) = evaluateConditions(previous, array, level) + val (notificationAlerts, next) = evaluateNotifications(afterConditions, notifications, level) + // sortedByDescending is stable: equal severity keeps conditions before notifications. + val ordered = (conditionAlerts + notificationAlerts).sortedByDescending { it.severity } + return AlertEvaluation( + server = server, + toPost = ordered.take(MAX_ALERTS_PER_CHECK), + foldedCount = (ordered.size - MAX_ALERTS_PER_CHECK).coerceAtLeast(0), + nextState = next, + ) +} + +private fun evaluateNotifications( + previous: ServerAlertState, + notifications: Notifications?, + level: AlertLevel, +): Pair, ServerAlertState> { + if (notifications == null) return emptyList() to previous + val current = notifications.items.distinctBy { it.id } + val remembered = previous.seenNotificationIds.toSet() + val alerts = if (!previous.notificationsBaselined) { + emptyList() + } else { + current.filter { it.id !in remembered }.mapNotNull { it.toAlert(level) } + } + // Ids still on the server go last, so only ids that are gone are forgotten first. + val currentIds = current.map { it.id } + val stillListed = currentIds.toSet() + val seen = (previous.seenNotificationIds.filter { it !in stillListed } + currentIds) + .takeLast(MAX_SEEN_NOTIFICATION_IDS) + return alerts to previous.copy(notificationsBaselined = true, seenNotificationIds = seen) +} + +private fun evaluateConditions( + previous: ServerAlertState, + array: ArrayInfo?, + level: AlertLevel, +): Pair, ServerAlertState> { + if (array == null) return emptyList() to previous + val current = attentionItems(AttentionSnapshot(array, notifications = null, thresholds = null)) + .filter { it.key in STATE_ALERT_KEYS } + val alerts = if (!previous.conditionsBaselined) { + emptyList() + } else { + current + .filter { it.key !in previous.openConditions && level.allows(it.severity) } + .map { + Alert(it.key, AlertSource.ArrayCondition, it.severity, it.title, it.detail, timestamp = null, target = it.target) + } + } + return alerts to previous.copy(conditionsBaselined = true, openConditions = current.map { it.key }.toSet()) +} + +private fun UnraidNotification.toAlert(level: AlertLevel): Alert? { + val severity = when (importance) { + NotifImportance.Alert -> AttentionSeverity.Critical + NotifImportance.Warning -> AttentionSeverity.Warning + NotifImportance.Info -> return null + } + if (!level.allows(severity)) return null + return Alert( + key = id, + source = AlertSource.Notification, + severity = severity, + title = subject.ifBlank { title }.ifBlank { "Server notification" }, + text = description, + timestamp = timestamp, + target = AttentionTarget.Notifications, + ) +} + +private fun AlertLevel.allows(severity: AttentionSeverity): Boolean = when (this) { + AlertLevel.AlertsOnly -> severity == AttentionSeverity.Critical + AlertLevel.AlertsAndWarnings -> severity >= AttentionSeverity.Warning +} diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/data/alerts/AlertState.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/data/alerts/AlertState.kt new file mode 100644 index 00000000..dfe0a478 --- /dev/null +++ b/app/src/main/kotlin/io/github/nofuturekid/nova/data/alerts/AlertState.kt @@ -0,0 +1,25 @@ +package io.github.nofuturekid.nova.data.alerts + +import kotlinx.serialization.Serializable + +/** How many notification ids one server remembers. Oldest forgotten first beyond this. */ +const val MAX_SEEN_NOTIFICATION_IDS = 300 + +/** + * What NOVA remembers about one server so that an alert is raised once, not at every check. + * Persisted per server (see [AlertStateStore]); [evaluateAlerts] produces the next value. + * + * The two halves are independent because the two reads can fail independently: a failed + * read leaves its half exactly as it was. + */ +@Serializable +data class ServerAlertState( + /** False until one notification read succeeded: that read only records, it posts nothing. */ + val notificationsBaselined: Boolean = false, + /** Ids already reported or deliberately skipped, oldest first, at most [MAX_SEEN_NOTIFICATION_IDS]. */ + val seenNotificationIds: List = emptyList(), + /** False until one array read succeeded: that read only records, it posts nothing. */ + val conditionsBaselined: Boolean = false, + /** Array conditions that held at the last successful read, a subset of [STATE_ALERT_KEYS]. */ + val openConditions: Set = emptySet(), +) diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/data/alerts/AlertStateStore.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/data/alerts/AlertStateStore.kt new file mode 100644 index 00000000..65d52957 --- /dev/null +++ b/app/src/main/kotlin/io/github/nofuturekid/nova/data/alerts/AlertStateStore.kt @@ -0,0 +1,31 @@ +package io.github.nofuturekid.nova.data.alerts + +import io.github.nofuturekid.nova.data.local.SettingsStore +import javax.inject.Inject +import javax.inject.Singleton + +/** + * The remembered [ServerAlertState] per server, keyed by the server's stable id. + * A server without an entry has the empty state, which means "not baselined yet". + */ +interface AlertStateStore { + suspend fun get(serverId: String): ServerAlertState + suspend fun put(serverId: String, state: ServerAlertState) + + /** Forgets one server, for example when its entry is deleted. */ + suspend fun remove(serverId: String) + + /** Forgets every server, for example when alerts are switched off, so the next start baselines afresh. */ + suspend fun clear() +} + +/** Persists the state in the app's DataStore, so it survives process death. */ +@Singleton +class SettingsAlertStateStore @Inject constructor( + private val store: SettingsStore, +) : AlertStateStore { + override suspend fun get(serverId: String): ServerAlertState = store.alertStateFor(serverId) + override suspend fun put(serverId: String, state: ServerAlertState) = store.setAlertState(serverId, state) + override suspend fun remove(serverId: String) = store.clearAlertState(serverId) + override suspend fun clear() = store.clearAllAlertStates() +} diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/data/alerts/ServerProbe.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/data/alerts/ServerProbe.kt new file mode 100644 index 00000000..b047a895 --- /dev/null +++ b/app/src/main/kotlin/io/github/nofuturekid/nova/data/alerts/ServerProbe.kt @@ -0,0 +1,36 @@ +package io.github.nofuturekid.nova.data.alerts + +import io.github.nofuturekid.nova.data.model.ArrayInfo +import io.github.nofuturekid.nova.data.model.Notifications +import io.github.nofuturekid.nova.data.model.Server + +/** + * The one-shot seam: fetch what one check needs from one server, once, and say how it went. + * The background worker will call it per server; a home-screen widget can reuse it. + * + * Only the interface exists so far. The real implementation (two existing read-only queries, + * fail-closed certificate trust, no key reset, no prompts) belongs to the worker change. + * [pin] is the pinned local certificate fingerprint of [server], if any. + */ +interface ServerProbe { + suspend fun probe(server: Server, pin: String?): ProbeResult +} + +sealed interface ProbeResult { + /** The server answered. Each half is null when its own query failed. */ + data class Ok(val notifications: Notifications?, val array: ArrayInfo?) : ProbeResult + data object Unreachable : ProbeResult + data object CertificateProblem : ProbeResult + data object KeyMissing : ProbeResult + data object KeyUndecryptable : ProbeResult + data class Rejected(val message: String) : ProbeResult +} + +/** + * What [evaluateAlerts] gets for a result. Anything but [ProbeResult.Ok] loaded nothing, + * so it changes no state and posts nothing: failures are never alerts. + */ +val ProbeResult.loadedNotifications: Notifications? get() = (this as? ProbeResult.Ok)?.notifications + +/** See [loadedNotifications]. */ +val ProbeResult.loadedArray: ArrayInfo? get() = (this as? ProbeResult.Ok)?.array diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/data/local/SettingsStore.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/data/local/SettingsStore.kt index 3a9d8450..88c406e5 100644 --- a/app/src/main/kotlin/io/github/nofuturekid/nova/data/local/SettingsStore.kt +++ b/app/src/main/kotlin/io/github/nofuturekid/nova/data/local/SettingsStore.kt @@ -14,6 +14,7 @@ import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map import kotlinx.serialization.encodeToString import kotlinx.serialization.json.Json +import io.github.nofuturekid.nova.data.alerts.ServerAlertState import io.github.nofuturekid.nova.data.model.AppSettings import io.github.nofuturekid.nova.data.model.ConnectionMode import io.github.nofuturekid.nova.data.model.Server @@ -38,6 +39,7 @@ private object Keys { val LastUpdateCheck = longPreferencesKey("last_update_check") val DismissedUpdateTag = stringPreferencesKey("dismissed_update_tag") val CertPins = stringPreferencesKey("local_cert_pins_json") + val AlertStates = stringPreferencesKey("alert_state_json") } /** Shared layout mode for the Docker / VMs / Array list views. Each @@ -216,4 +218,32 @@ class SettingsStore @Inject constructor( else prefs[Keys.DismissedUpdateTag] = tag } } + + // ── Alert state ─────────────────────────────────────────────── + + /** server id → what was already reported for it. Unreadable data counts as empty. */ + private fun decodeAlertStates(prefs: Preferences): Map = + prefs[Keys.AlertStates]?.let { + runCatching { json.decodeFromString>(it) }.getOrDefault(emptyMap()) + } ?: emptyMap() + + suspend fun alertStateFor(serverId: String): ServerAlertState = + decodeAlertStates(ds.data.first())[serverId] ?: ServerAlertState() + + suspend fun setAlertState(serverId: String, state: ServerAlertState) { + ds.edit { prefs -> + prefs[Keys.AlertStates] = json.encodeToString(decodeAlertStates(prefs) + (serverId to state)) + } + } + + suspend fun clearAlertState(serverId: String) { + ds.edit { prefs -> + val next = decodeAlertStates(prefs) - serverId + if (next.isEmpty()) prefs.remove(Keys.AlertStates) else prefs[Keys.AlertStates] = json.encodeToString(next) + } + } + + suspend fun clearAllAlertStates() { + ds.edit { it.remove(Keys.AlertStates) } + } } diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/data/repository/ServerRepository.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/data/repository/ServerRepository.kt index d8f08c3b..c275826f 100644 --- a/app/src/main/kotlin/io/github/nofuturekid/nova/data/repository/ServerRepository.kt +++ b/app/src/main/kotlin/io/github/nofuturekid/nova/data/repository/ServerRepository.kt @@ -69,6 +69,7 @@ class ServerRepository @Inject constructor( store.setServers(current) keys.remove(id) store.clearPin(id) + store.clearAlertState(id) if (store.activeServerId.first() == id) { store.setActiveServer(nextActiveAfterDelete(current)) } diff --git a/app/src/test/kotlin/io/github/nofuturekid/nova/data/alerts/AlertCheckCycleTest.kt b/app/src/test/kotlin/io/github/nofuturekid/nova/data/alerts/AlertCheckCycleTest.kt new file mode 100644 index 00000000..3418fbc5 --- /dev/null +++ b/app/src/test/kotlin/io/github/nofuturekid/nova/data/alerts/AlertCheckCycleTest.kt @@ -0,0 +1,130 @@ +package io.github.nofuturekid.nova.data.alerts + +import io.github.nofuturekid.nova.data.model.ArrayInfo +import io.github.nofuturekid.nova.data.model.ArrayState +import io.github.nofuturekid.nova.data.model.NotifImportance +import io.github.nofuturekid.nova.data.model.Notifications +import io.github.nofuturekid.nova.data.model.Server +import io.github.nofuturekid.nova.data.model.UnraidNotification +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Runs whole checks the way the background worker will: probe, evaluate, store, over several + * rounds and servers, on the in-memory fakes. + * + * WHY: the rules are only right if the remembered state really carries from one check to the + * next and stays per server. These tests would fail if a server's state leaked into another's, + * if a failed probe overwrote the memory, or if switching alerts off did not make the next + * start a silent first check. + */ +class AlertCheckCycleTest { + + private val tower = server("srv-tower", "Tower") + private val backup = server("srv-backup", "Backup") + private val probe = FakeServerProbe() + private val store = InMemoryAlertStateStore() + + /** One check of [server], as the worker will do it. Returns the keys that would be posted. */ + private fun check(server: Server, level: AlertLevel = AlertLevel.AlertsAndWarnings): List = runBlocking { + val result = probe.probe(server, pin = null) + val evaluation = evaluateAlerts( + ServerRef(server.id, server.name), store.get(server.id), + result.loadedNotifications, result.loadedArray, level, + ) + store.put(server.id, evaluation.nextState) + evaluation.toPost.map { it.key } + } + + @Test fun a_notification_is_posted_by_exactly_one_check() { + probe.answer(tower.id, ok(listOf(alert("n1")), ArrayState.Started)) + assertEquals("first check seeds", emptyList(), check(tower)) + + probe.answer(tower.id, ok(listOf(alert("n1"), alert("n2")), ArrayState.Started)) + assertEquals(listOf("n2"), check(tower)) + assertEquals(emptyList(), check(tower)) + assertEquals(emptyList(), check(tower)) + } + + @Test fun state_is_independent_per_server() { + // WHY: the same notification id on two servers is two events; one server's baseline + // must not silence, and one server's memory must not suppress, the other. + probe.answer(tower.id, ok(emptyList(), ArrayState.Started)) + check(tower) + // Backup is added later and has not been baselined yet. + probe.answer(tower.id, ok(listOf(alert("n1")), ArrayState.Started)) + probe.answer(backup.id, ok(listOf(alert("n1")), ArrayState.Stopped)) + + assertEquals("Backup's first check is silent although Tower alerts", emptyList(), check(backup)) + assertEquals(listOf("n1"), check(tower)) + assertEquals(emptyList(), check(tower)) + assertEquals(setOf(tower.id, backup.id), store.serverIds) + + probe.answer(backup.id, ok(listOf(alert("n1"), alert("n2")), ArrayState.Stopped)) + assertEquals(listOf("n2"), check(backup)) + } + + @Test fun an_unreachable_check_keeps_the_memory_so_nothing_repeats_afterwards() { + probe.answer(tower.id, ok(listOf(alert("n1")), ArrayState.Stopped)) + check(tower) + probe.answer(tower.id, ProbeResult.Unreachable) + assertEquals(emptyList(), check(tower)) + probe.answer(tower.id, ok(listOf(alert("n1")), ArrayState.Stopped)) + assertEquals("neither the old alert nor the old array condition returns", emptyList(), check(tower)) + } + + @Test fun removing_a_server_forgets_it_and_a_server_added_again_starts_silent() { + probe.answer(tower.id, ok(listOf(alert("n1")), ArrayState.Started)) + check(tower) + runBlocking { store.remove(tower.id) } + assertEquals(emptySet(), store.serverIds) + + probe.answer(tower.id, ok(listOf(alert("n1"), alert("n2")), ArrayState.Started)) + assertEquals("a re-added server is baselined again, not flooded", emptyList(), check(tower)) + } + + @Test fun switching_alerts_off_clears_everything_so_the_next_start_is_a_silent_first_check() { + probe.answer(tower.id, ok(listOf(alert("n1")), ArrayState.Started)) + probe.answer(backup.id, ok(listOf(alert("n9")), ArrayState.Started)) + check(tower); check(backup) + runBlocking { store.clear() } + assertEquals(emptySet(), store.serverIds) + + probe.answer(tower.id, ok(listOf(alert("n1"), alert("n2")), ArrayState.Started)) + assertEquals(emptyList(), check(tower)) + } + + @Test fun the_stored_state_stays_bounded_over_many_checks() { + // WHY: a server that keeps raising notifications for years must not make the stored + // state grow without end. + probe.answer(tower.id, ok(emptyList(), ArrayState.Started)) + check(tower) + var round = 0 + repeat(40) { + val batch = (1..20).map { alert("n${round++}") } + probe.answer(tower.id, ok(batch, ArrayState.Started)) + check(tower) + } + val seen = runBlocking { store.get(tower.id) }.seenNotificationIds + assertEquals(MAX_SEEN_NOTIFICATION_IDS, seen.size) + assertTrue("the newest id is remembered", "n${round - 1}" in seen) + } + + private fun ok(items: List, state: ArrayState) = ProbeResult.Ok( + Notifications( + unreadWarning = 0, unreadAlert = items.size, unreadTotal = items.size, items = items, + ), + ArrayInfo(state, totalTb = 8.0, usedTb = 2.0, parity = null, disks = emptyList()), + ) + + private fun alert(id: String) = UnraidNotification( + id = id, title = "Title", subject = "Alert $id", description = "Details", importance = NotifImportance.Alert, + type = null, link = null, timestamp = null, formattedTimestamp = null, + ) + + private fun server(id: String, name: String) = Server( + id = id, name = name, hostname = "$id.example", localUrl = "https://$id.example", remoteUrl = "", + ) +} diff --git a/app/src/test/kotlin/io/github/nofuturekid/nova/data/alerts/AlertFakes.kt b/app/src/test/kotlin/io/github/nofuturekid/nova/data/alerts/AlertFakes.kt new file mode 100644 index 00000000..3bff194d --- /dev/null +++ b/app/src/test/kotlin/io/github/nofuturekid/nova/data/alerts/AlertFakes.kt @@ -0,0 +1,27 @@ +package io.github.nofuturekid.nova.data.alerts + +import io.github.nofuturekid.nova.data.model.Server + +/** In-memory [AlertStateStore] for tests of the code that will use the store. */ +class InMemoryAlertStateStore : AlertStateStore { + private val states = mutableMapOf() + + override suspend fun get(serverId: String): ServerAlertState = states[serverId] ?: ServerAlertState() + override suspend fun put(serverId: String, state: ServerAlertState) { states[serverId] = state } + override suspend fun remove(serverId: String) { states.remove(serverId) } + override suspend fun clear() { states.clear() } + + val serverIds: Set get() = states.keys.toSet() +} + +/** [ServerProbe] that answers from a script: the result set for a server id, [ProbeResult.Unreachable] otherwise. */ +class FakeServerProbe(private val results: MutableMap = mutableMapOf()) : ServerProbe { + val probed = mutableListOf>() + + fun answer(serverId: String, result: ProbeResult) { results[serverId] = result } + + override suspend fun probe(server: Server, pin: String?): ProbeResult { + probed += server.id to pin + return results[server.id] ?: ProbeResult.Unreachable + } +} diff --git a/app/src/test/kotlin/io/github/nofuturekid/nova/data/alerts/AlertRulesTest.kt b/app/src/test/kotlin/io/github/nofuturekid/nova/data/alerts/AlertRulesTest.kt new file mode 100644 index 00000000..c269468d --- /dev/null +++ b/app/src/test/kotlin/io/github/nofuturekid/nova/data/alerts/AlertRulesTest.kt @@ -0,0 +1,360 @@ +package io.github.nofuturekid.nova.data.alerts + +import io.github.nofuturekid.nova.data.model.ArrayInfo +import io.github.nofuturekid.nova.data.model.ArrayState +import io.github.nofuturekid.nova.data.model.AttentionSeverity +import io.github.nofuturekid.nova.data.model.AttentionTarget +import io.github.nofuturekid.nova.data.model.Disk +import io.github.nofuturekid.nova.data.model.DiskStatus +import io.github.nofuturekid.nova.data.model.DiskType +import io.github.nofuturekid.nova.data.model.NotifImportance +import io.github.nofuturekid.nova.data.model.Notifications +import io.github.nofuturekid.nova.data.model.UnraidNotification +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Tests for [evaluateAlerts], the decision behind alerts outside the app. + * + * WHY these cases matter: an alert that repeats teaches users to switch alerts off, and + * one that never comes defeats the feature. So: + * - turning alerts on must not dump what is already unread (first check seeds silently); + * - a notification is reported once, however often it is read and even after it left the + * unread list, but a later notification with the same text is a new event; + * - an array that stays stopped is one alert, not one per check, and a new episode after + * it was fine in between is an alert again; + * - a server that did not answer is not news, and must not cause a repeat once it does; + * - the state must not grow without bound, because it lives on the phone for good. + */ +class AlertRulesTest { + + private val tower = ServerRef("srv-1", "Tower") + private val both = AlertLevel.AlertsAndWarnings + + // ── First check ─────────────────────────────────────────────────── + + @Test fun first_check_posts_nothing_and_records_baseline() { + // WHY: switching alerts on must not flood the user with everything already unread. + val result = evaluateAlerts( + tower, ServerAlertState(), + notifications = unread(alert("n1"), warning("n2")), + array = array(ArrayState.Stopped), + level = both, + ) + assertEquals(emptyList(), result.toPost) + assertEquals(0, result.foldedCount) + assertTrue(result.nextState.notificationsBaselined) + assertTrue(result.nextState.conditionsBaselined) + assertEquals(listOf("n1", "n2"), result.nextState.seenNotificationIds) + assertEquals(setOf("array-stopped"), result.nextState.openConditions) + } + + @Test fun after_the_baseline_only_what_is_new_is_posted() { + val first = evaluateAlerts(tower, ServerAlertState(), unread(alert("n1")), array(ArrayState.Started), both) + val second = evaluateAlerts(tower, first.nextState, unread(alert("n1"), alert("n2", subject = "Disk 3 failed")), array(ArrayState.Started), both) + assertEquals(listOf("n2"), second.toPost.map { it.key }) + assertEquals("Disk 3 failed", second.toPost.single().title) + } + + // ── Notifications: once per id ──────────────────────────────────── + + @Test fun new_id_is_posted_once_and_never_again_even_after_leaving_unread() { + // WHY: "never re-alert the same notification", also when it is archived and comes back. + var state = evaluateAlerts(tower, ServerAlertState(), unread(), array(ArrayState.Started), both).nextState + val appears = evaluateAlerts(tower, state, unread(alert("n1")), null, both) + assertEquals(listOf("n1"), appears.toPost.map { it.key }) + state = appears.nextState + + val stillThere = evaluateAlerts(tower, state, unread(alert("n1")), null, both) + assertEquals(emptyList(), stillThere.toPost) + state = stillThere.nextState + + val archived = evaluateAlerts(tower, state, unread(), null, both) + assertEquals(emptyList(), archived.toPost) + state = archived.nextState + + val comesBack = evaluateAlerts(tower, state, unread(alert("n1")), null, both) + assertEquals(emptyList(), comesBack.toPost) + } + + @Test fun same_text_with_a_new_id_is_a_new_event() { + // WHY: Unraid builds the id from the event time, so the same problem a day later is a + // new notification and must be reported again. + val state = baselined(seen = listOf("disk-hot-100")) + val result = evaluateAlerts(tower, state, unread(warning("disk-hot-200", subject = "disk1 is hot")), null, both) + assertEquals(listOf("disk-hot-200"), result.toPost.map { it.key }) + } + + @Test fun info_is_never_posted() { + // WHY: INFO includes "returned to normal" and update notices; they are not alerts. + val state = baselined() + val result = evaluateAlerts(tower, state, unread(info("i1")), null, both) + assertEquals(emptyList(), result.toPost) + // It is remembered all the same, so it cannot come back as news. + assertEquals(listOf("i1"), result.nextState.seenNotificationIds) + } + + @Test fun alert_and_warning_map_to_critical_and_warning_with_their_text() { + val result = evaluateAlerts( + tower, baselined(), + unread(alert("a", subject = "Disk 1 failed", description = "Disk 1 is in error state", timestamp = "2026-11-13T10:00:00Z"), warning("w")), + null, both, + ) + val a = result.toPost.first { it.key == "a" } + assertEquals(AttentionSeverity.Critical, a.severity) + assertEquals("Disk 1 failed", a.title) + assertEquals("Disk 1 is in error state", a.text) + assertEquals("2026-11-13T10:00:00Z", a.timestamp) + assertEquals(AlertSource.Notification, a.source) + assertEquals(AttentionTarget.Notifications, a.target) + assertEquals(AttentionSeverity.Warning, result.toPost.first { it.key == "w" }.severity) + } + + @Test fun a_blank_subject_falls_back_to_the_title() { + val result = evaluateAlerts(tower, baselined(), unread(alert("a", subject = "", title = "Array notice")), null, both) + assertEquals("Array notice", result.toPost.single().title) + } + + // ── Level ───────────────────────────────────────────────────────── + + @Test fun alerts_only_drops_warnings_but_marks_them_seen() { + // WHY: switching from "Alerts only" to "Alerts and warnings" later must not post old warnings. + val state = baselined() + val quiet = evaluateAlerts(tower, state, unread(warning("w1"), alert("a1")), null, AlertLevel.AlertsOnly) + assertEquals(listOf("a1"), quiet.toPost.map { it.key }) + assertTrue("w1" in quiet.nextState.seenNotificationIds) + + val widened = evaluateAlerts(tower, quiet.nextState, unread(warning("w1"), alert("a1")), null, both) + assertEquals(emptyList(), widened.toPost) + } + + @Test fun alerts_and_warnings_posts_both() { + val result = evaluateAlerts(tower, baselined(), unread(warning("w1"), alert("a1")), null, both) + assertEquals(setOf("w1", "a1"), result.toPost.map { it.key }.toSet()) + } + + @Test fun alerts_only_posts_a_degraded_array_but_not_a_stopped_one_and_remembers_both() { + // WHY: a stopped array is a warning-level condition; "Alerts only" means the critical ones. + // Later widening must not post a stopped array that has been stopped all along. + val state = baselined() + val stopped = evaluateAlerts(tower, state, null, array(ArrayState.Stopped), AlertLevel.AlertsOnly) + assertEquals(emptyList(), stopped.toPost) + assertEquals(setOf("array-stopped"), stopped.nextState.openConditions) + assertEquals(emptyList(), evaluateAlerts(tower, stopped.nextState, null, array(ArrayState.Stopped), both).toPost) + + val degraded = evaluateAlerts(tower, state, null, array(ArrayState.Error), AlertLevel.AlertsOnly) + assertEquals(listOf("array-degraded"), degraded.toPost.map { it.key }) + } + + // ── Array conditions ────────────────────────────────────────────── + + @Test fun array_stopped_posted_on_transition_not_while_it_lasts_and_rearmed_after_clearing() { + // WHY: one alert per episode. A server that stays stopped all night is one alert. + var state = evaluateAlerts(tower, ServerAlertState(), null, array(ArrayState.Started), both).nextState + + val begins = evaluateAlerts(tower, state, null, array(ArrayState.Stopped), both) + val alert = begins.toPost.single() + assertEquals("array-stopped", alert.key) + assertEquals(AlertSource.ArrayCondition, alert.source) + assertEquals(AttentionTarget.Array, alert.target) + assertEquals("Array is not started", alert.title) + state = begins.nextState + + repeat(3) { + val lasts = evaluateAlerts(tower, state, null, array(ArrayState.Stopped), both) + assertEquals(emptyList(), lasts.toPost) + state = lasts.nextState + } + + state = evaluateAlerts(tower, state, null, array(ArrayState.Started), both).nextState + assertEquals(emptySet(), state.openConditions) + + val again = evaluateAlerts(tower, state, null, array(ArrayState.Stopped), both) + assertEquals(listOf("array-stopped"), again.toPost.map { it.key }) + } + + @Test fun degraded_array_is_critical_and_raised_once() { + val state = baselined() + val begins = evaluateAlerts(tower, state, null, array(ArrayState.Error), AlertLevel.AlertsOnly) + val alert = begins.toPost.single() + assertEquals("array-degraded", alert.key) + assertEquals(AttentionSeverity.Critical, alert.severity) + assertEquals(emptyList(), evaluateAlerts(tower, begins.nextState, null, array(ArrayState.Error), AlertLevel.AlertsOnly).toPost) + } + + @Test fun disk_temperature_and_disk_error_items_are_not_posted() { + // WHY: Unraid already notifies about hot disks and disk errors. Posting them from the + // array as well would double every alert. This fails if STATE_ALERT_KEYS is widened + // without deciding that. + assertEquals(setOf("array-stopped", "array-degraded"), STATE_ALERT_KEYS) + val sick = array(ArrayState.Started, disks = listOf(disk("disk1", tempC = 70, numErrors = 12))) + val result = evaluateAlerts(tower, baselined(), null, sick, both) + assertEquals(emptyList(), result.toPost) + assertEquals(emptySet(), result.nextState.openConditions) + } + + @Test fun rebuilding_array_is_not_posted() { + val result = evaluateAlerts(tower, baselined(), null, array(ArrayState.Parity), both) + assertEquals(emptyList(), result.toPost) + } + + // ── Failures ────────────────────────────────────────────────────── + + @Test fun unreachable_server_is_silent_and_changes_nothing() { + // WHY: nothing was loaded; "the server did not answer" is not an alert, and it must not + // reset the memory, or the next successful check would repeat everything. + val state = ServerAlertState(true, listOf("n1"), true, setOf("array-stopped")) + val result = evaluateAlerts(tower, state, notifications = null, array = null, level = both) + assertEquals(emptyList(), result.toPost) + assertEquals(0, result.foldedCount) + assertEquals(state, result.nextState) + } + + @Test fun unreachable_before_the_first_success_does_not_baseline() { + // WHY: a failed check must not count as the silent first check, or an alert that + // appeared meanwhile would be swallowed. + val result = evaluateAlerts(tower, ServerAlertState(), null, null, both) + assertEquals(ServerAlertState(), result.nextState) + } + + @Test fun failed_notifications_keep_that_half_and_still_evaluate_the_array() { + val state = ServerAlertState(true, listOf("n1"), true, emptySet()) + val result = evaluateAlerts(tower, state, notifications = null, array = array(ArrayState.Stopped), level = both) + assertEquals(listOf("array-stopped"), result.toPost.map { it.key }) + assertEquals(listOf("n1"), result.nextState.seenNotificationIds) + } + + @Test fun failed_array_keeps_the_condition_half_and_still_evaluates_notifications() { + val state = ServerAlertState(true, listOf("n1"), true, setOf("array-stopped")) + val result = evaluateAlerts(tower, state, unread(alert("n2")), array = null, level = both) + assertEquals(listOf("n2"), result.toPost.map { it.key }) + assertEquals(setOf("array-stopped"), result.nextState.openConditions) + } + + @Test fun items_created_during_an_outage_are_posted_once_when_the_server_answers_again() { + var state = baselined(seen = listOf("n1")) + state = evaluateAlerts(tower, state, null, null, both).nextState + val back = evaluateAlerts(tower, state, unread(alert("n1"), alert("n2")), null, both) + assertEquals(listOf("n2"), back.toPost.map { it.key }) + assertEquals(emptyList(), evaluateAlerts(tower, back.nextState, unread(alert("n1"), alert("n2")), null, both).toPost) + } + + @Test fun every_probe_result_but_ok_loads_nothing() { + // WHY: failures must travel the "nothing loaded" path, whichever way they failed. + val failures = listOf( + ProbeResult.Unreachable, ProbeResult.CertificateProblem, ProbeResult.KeyMissing, + ProbeResult.KeyUndecryptable, ProbeResult.Rejected("HTTP 401"), + ) + for (failure in failures) { + val state = ServerAlertState(true, listOf("n1"), true, setOf("array-degraded")) + val result = evaluateAlerts(tower, state, failure.loadedNotifications, failure.loadedArray, both) + assertEquals("$failure", emptyList(), result.toPost) + assertEquals("$failure", state, result.nextState) + } + val ok = ProbeResult.Ok(unread(alert("n9")), array(ArrayState.Started)) + assertEquals(listOf("n9"), ok.loadedNotifications!!.items.map { it.id }) + assertEquals(ArrayState.Started, ok.loadedArray!!.state) + } + + // ── Flood cap ───────────────────────────────────────────────────── + + @Test fun more_than_five_new_fold_into_a_summary_and_are_all_remembered() { + val state = baselined() + val many = (1..8).map { alert("n$it") } + val result = evaluateAlerts(tower, state, unread(*many.toTypedArray()), null, both) + assertEquals(MAX_ALERTS_PER_CHECK, result.toPost.size) + assertEquals(3, result.foldedCount) + assertEquals("and 3 more on Tower", foldedSummaryText(tower.name, result.foldedCount)) + // WHY: folded ones were announced by the summary; they must not return next time. + assertEquals((1..8).map { "n$it" }, result.nextState.seenNotificationIds) + assertEquals(emptyList(), evaluateAlerts(tower, result.nextState, unread(*many.toTypedArray()), null, both).toPost) + } + + @Test fun the_most_urgent_alerts_get_the_individual_notifications() { + val state = baselined() + val items = (1..6).map { warning("w$it") } + alert("a1") + alert("a2") + val result = evaluateAlerts(tower, state, unread(*items.toTypedArray()), array(ArrayState.Error), both) + assertEquals(5, result.toPost.size) + assertEquals(4, result.foldedCount) + // Critical first: the degraded array and both alerts, then warnings. + assertEquals(setOf("array-degraded", "a1", "a2"), result.toPost.take(3).map { it.key }.toSet()) + assertTrue(result.toPost.drop(3).all { it.severity == AttentionSeverity.Warning }) + } + + @Test fun five_or_fewer_are_not_folded() { + val result = evaluateAlerts(tower, baselined(), unread(*(1..5).map { alert("n$it") }.toTypedArray()), null, both) + assertEquals(5, result.toPost.size) + assertEquals(0, result.foldedCount) + } + + // ── Bounded memory ──────────────────────────────────────────────── + + @Test fun seen_ids_are_capped_and_the_oldest_are_forgotten_first() { + // WHY: the state lives on the phone for good; without a cap it grows with every + // notification the server ever shows. + val old = (1..MAX_SEEN_NOTIFICATION_IDS).map { "old$it" } + val state = baselined(seen = old) + val result = evaluateAlerts(tower, state, unread(alert("new1"), alert("new2")), null, both) + val seen = result.nextState.seenNotificationIds + assertEquals(MAX_SEEN_NOTIFICATION_IDS, seen.size) + assertFalse("old1" in seen) + assertFalse("old2" in seen) + assertEquals("old3", seen.first()) + assertEquals(listOf("new1", "new2"), seen.takeLast(2)) + } + + @Test fun ids_that_are_gone_from_the_server_are_forgotten_before_ids_still_listed() { + // WHY: an id still on the server must never be forgotten, or it would be posted again. + val state = baselined(seen = listOf("still") + (1..MAX_SEEN_NOTIFICATION_IDS - 1).map { "gone$it" }) + val result = evaluateAlerts(tower, state, unread(alert("still"), alert("fresh")), null, both) + val seen = result.nextState.seenNotificationIds + assertEquals(MAX_SEEN_NOTIFICATION_IDS, seen.size) + assertTrue("still" in seen) + assertFalse("gone1" in seen) + assertEquals(emptyList(), evaluateAlerts(tower, result.nextState, unread(alert("still"), alert("fresh")), null, both).toPost) + } + + @Test fun a_repeated_id_in_one_read_is_remembered_once() { + val result = evaluateAlerts(tower, baselined(), unread(alert("n1"), alert("n1")), null, both) + assertEquals(1, result.toPost.size) + assertEquals(listOf("n1"), result.nextState.seenNotificationIds) + } + + // ── Helpers ─────────────────────────────────────────────────────── + + private fun baselined(seen: List = emptyList(), open: Set = emptySet()) = + ServerAlertState(true, seen, true, open) + + private fun unread(vararg items: UnraidNotification) = Notifications( + unreadWarning = items.count { it.importance == NotifImportance.Warning }, + unreadAlert = items.count { it.importance == NotifImportance.Alert }, + unreadInfo = items.count { it.importance == NotifImportance.Info }, + unreadTotal = items.size, + items = items.toList(), + ) + + private fun alert(id: String, subject: String = "Alert $id", title: String = "Title $id", description: String = "Details $id", timestamp: String? = null) = + notification(id, NotifImportance.Alert, subject, title, description, timestamp) + + private fun warning(id: String, subject: String = "Warning $id") = + notification(id, NotifImportance.Warning, subject, "Title $id", "Details $id", null) + + private fun info(id: String) = notification(id, NotifImportance.Info, "Info $id", "Title $id", "Details $id", null) + + private fun notification(id: String, importance: NotifImportance, subject: String, title: String, description: String, timestamp: String?) = + UnraidNotification( + id = id, title = title, subject = subject, description = description, importance = importance, + type = null, link = null, timestamp = timestamp, formattedTimestamp = null, + ) + + private fun array(state: ArrayState, disks: List = emptyList()) = + ArrayInfo(state, totalTb = 8.0, usedTb = 2.0, parity = null, disks = disks) + + private fun disk(name: String, tempC: Int, numErrors: Long = 0L) = Disk( + name = name, device = "sdb", type = DiskType.Data, sizeTb = 4.0, usedTb = 1.0, + tempC = tempC, status = DiskStatus.Ok, model = "", isSpinning = true, rotational = true, + numErrors = numErrors, warningC = 45, criticalC = 55, + ) +} diff --git a/app/src/test/kotlin/io/github/nofuturekid/nova/data/alerts/AlertStateStoreTest.kt b/app/src/test/kotlin/io/github/nofuturekid/nova/data/alerts/AlertStateStoreTest.kt new file mode 100644 index 00000000..8c14043a --- /dev/null +++ b/app/src/test/kotlin/io/github/nofuturekid/nova/data/alerts/AlertStateStoreTest.kt @@ -0,0 +1,112 @@ +package io.github.nofuturekid.nova.data.alerts + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.github.nofuturekid.nova.data.local.ApiKeyStore +import io.github.nofuturekid.nova.data.local.SettingsStore +import io.github.nofuturekid.nova.data.model.Server +import io.github.nofuturekid.nova.data.repository.ServerRepository +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.annotation.Config + +/** + * WHY: the memory of what was already reported is worthless if it is lost or shared. + * Lost at process death, every check after the system killed the app would be a silent + * "first check" and real alerts would vanish, or (if it fell back to "nothing seen") the + * whole backlog would be re-posted. Shared between servers, one server's baseline would + * hide another's alerts. Kept after the server is deleted, it would sit on the phone for + * good. + * + * Runs the real [SettingsAlertStateStore] on the real DataStore file (Robolectric). The + * DataStore file outlives a test inside one JVM, so every test starts by clearing it. A new + * store object over the same file stands for a restarted process: nothing is kept in memory. + */ +@RunWith(AndroidJUnit4::class) +@Config(sdk = [35]) +class AlertStateStoreTest { + private lateinit var ctx: Context + + @Before fun setUp() = runBlocking { + ctx = ApplicationProvider.getApplicationContext() + SettingsAlertStateStore(SettingsStore(ctx)).clear() + } + + private fun freshStore() = SettingsAlertStateStore(SettingsStore(ctx)) + + private val busy = ServerAlertState(true, listOf("n1", "n2"), true, setOf("array-stopped")) + + @Test fun a_server_without_state_is_not_baselined() = runBlocking { + assertEquals(ServerAlertState(), freshStore().get("unknown")) + } + + @Test fun state_survives_a_restart() = runBlocking { + freshStore().put("srv-1", busy) + assertEquals(busy, freshStore().get("srv-1")) + } + + @Test fun servers_do_not_share_state() = runBlocking { + val store = freshStore() + val other = ServerAlertState(true, listOf("x"), false, emptySet()) + store.put("srv-1", busy) + store.put("srv-2", other) + assertEquals(busy, freshStore().get("srv-1")) + assertEquals(other, freshStore().get("srv-2")) + } + + @Test fun put_replaces_a_servers_state_instead_of_adding_to_it() = runBlocking { + val store = freshStore() + store.put("srv-1", busy) + store.put("srv-1", ServerAlertState(true, listOf("n3"), true, emptySet())) + assertEquals(listOf("n3"), freshStore().get("srv-1").seenNotificationIds) + } + + @Test fun remove_forgets_only_that_server() = runBlocking { + val store = freshStore() + store.put("srv-1", busy) + store.put("srv-2", busy) + store.remove("srv-1") + assertEquals(ServerAlertState(), freshStore().get("srv-1")) + assertEquals(busy, freshStore().get("srv-2")) + } + + @Test fun clear_forgets_every_server() = runBlocking { + val store = freshStore() + store.put("srv-1", busy) + store.put("srv-2", busy) + store.clear() + assertEquals(ServerAlertState(), freshStore().get("srv-1")) + assertEquals(ServerAlertState(), freshStore().get("srv-2")) + } + + @Test fun deleting_a_server_entry_removes_its_alert_state() = runBlocking { + // WHY: a deleted server must not leave its list of notification ids on the phone, and a + // server added again must be baselined afresh instead of inheriting old memory. + val settings = SettingsStore(ctx) + val repo = ServerRepository(settings, ApiKeyStore(ctx)) + settings.servers.first().forEach { repo.delete(it.id) } + + // A blank key keeps the Android Keystore out of the test. + val kept = repo.upsert(server("Tower"), apiKey = "") + val deleted = repo.upsert(server("Backup"), apiKey = "") + val store = SettingsAlertStateStore(settings) + store.put(kept.id, busy) + store.put(deleted.id, busy) + + repo.delete(deleted.id) + + assertEquals(ServerAlertState(), freshStore().get(deleted.id)) + assertEquals(busy, freshStore().get(kept.id)) + repo.delete(kept.id) + } + + private fun server(name: String) = Server( + id = "", name = name, hostname = "${name.lowercase()}.example", + localUrl = "https://${name.lowercase()}.example", remoteUrl = "", + ) +} diff --git a/docs/adr/0017-domain-split-queries-with-lifecycle-polling.md b/docs/adr/0017-domain-split-queries-with-lifecycle-polling.md index 08130f22..dc2ef5d1 100644 --- a/docs/adr/0017-domain-split-queries-with-lifecycle-polling.md +++ b/docs/adr/0017-domain-split-queries-with-lifecycle-polling.md @@ -88,6 +88,10 @@ fires one shared `refreshNudge` that every domain stream waits on (previously a fetch started after the request has completed, bounded by `REFRESH_MAX_WAIT_MS`. +## Amendment — 2026-11-13 (opt-in background alert checks) + +"Backgrounded app = zero polls, period" holds for every screen stream and stays in force. The one exception is the optional alert check of [ADR-0048](./0048-opt-in-background-alert-checks.md): only while the user has switched "Alerts from your servers" on, a periodic background job asks each server for its unread alerts and warnings and its array state. It is off by default, and with it off the app does no background work, as before. + ## References - The v0.1.22-beta1 incident: `Cannot query field "updateStatus" on type "DockerContainer"` collapsing the entire app to "Can't reach the server", despite the bad field only affecting Docker data. Fixed in PR #25. diff --git a/docs/adr/0048-opt-in-background-alert-checks.md b/docs/adr/0048-opt-in-background-alert-checks.md new file mode 100644 index 00000000..5c632cb3 --- /dev/null +++ b/docs/adr/0048-opt-in-background-alert-checks.md @@ -0,0 +1,82 @@ +# ADR-0048: Opt-in background alert checks + +- **Status**: Accepted (2026-11-13). Amends ADR-0017. The decision logic and its stored state are built first; the background job, the settings screen and the notifications follow in later changes (#321). +- **Date**: 2026-11-13 +- **Tags**: data, security, performance, notifications + +## Context + +Users want to hear about a problem on their server without opening NOVA (#321). Until now NOVA did no background work at all. ADR-0017 states it as a rule: the app lifecycle gates every poll, "so backgrounded app = zero polls, period". (Release 0.1.42 did not remove background polling; it fixed three screens that kept polling while the app was in the background, which is the same rule.) + +The maintainer decided on 2026-11-13: alerts are opt-in and off by default, and background checks exist only while the switch is on. The default level is alerts and warnings. + +The facts the design rests on: + +- Unraid's own monitor already creates a notification, with an importance of alert, warning or normal, for hot disks, SMART warnings, disks in error, full disks, read errors, a degraded pool, a boot drive that is not writable and parity checks. NOVA already reads the unread alerts and warnings with the query behind the bell (`GetNotifications`). +- Unraid does not notify about an array that is not started, and it stays silent about disk errors while the array is stopped. +- Unraid's notification agents send plain HTTP posts. UnifiedPush requires encrypted messages from the sender (RFC 8291), so it cannot be fed by those agents. +- The API key is stored encrypted with a key in the Android Keystore that does not require an unlocked device or user authentication, so a background check can use it. + +## Decision + +**Add an optional background check for alerts that runs only while the user has switched it on.** With the switch off, NOVA still does nothing in the background, exactly as before. ADR-0017's rule stays in force for every screen stream; only this opt-in job is an exception to it. + +What is decided here, and what this first change implements: + +1. **Events.** A new unread Unraid notification of importance alert or warning, and two array conditions that Unraid never notifies about: the array is not started, and the array is degraded. The array conditions are the rules of `attentionItems` (the Overview card), wrapped and limited to those two keys; nothing is forked. Disk temperature, disk errors, parity and pools are not raised from the array data again, because Unraid already sends them as notifications and every problem would be posted twice. Information-level notifications are never alerts. Containers and an unreachable server are not alerts either: a stopped container is often deliberate, and the phone cannot tell "server down" from "phone away from home". +2. **Once per notification.** Each server remembers the ids of the notifications it has seen. An id is never posted twice, even if the notification leaves the unread list and returns. The id is built from the event time, so a later notification with the same text is a new event and is posted. The list is kept in insertion order and capped at 300; an id that is still on the server is never forgotten before one that is gone, so the stored state cannot grow without bound. +3. **First check is silent.** The first successful read of a server after the switch is turned on, or after the server is added, only records what is already there and posts nothing. Turning the switch off forgets all state, so the next start is a first check again. The same rule applies to the array conditions. +4. **Array conditions once per episode.** A condition is raised when it begins and again only after it was gone at an in-between check. +5. **Level.** "Alerts and warnings" posts everything above; "Alerts only" posts only critical items, that is alert notifications and a degraded array. Items the level filters out are still remembered, so widening the level later does not post old ones. +6. **Failures change nothing.** A check that did not load a half (server unreachable, certificate problem, key missing, request rejected) neither posts nor alters the remembered state of that half, so alerts created during an outage are posted once when the server answers again and nothing is posted twice. A failed check never counts as the silent first check. +7. **Several servers.** Every saved server has its own state, keyed by its stable id, and the state is removed with the server entry. +8. **At most five alerts per server and check** get their own notification, most urgent first; the rest are folded into one summary line. +9. **One-shot seam.** What one check needs from one server (the unread alerts and warnings, the array state, and how the attempt went) is behind a small interface, so the worker, and later a widget, share one code path. This change adds the interface and a test fake; the implementation that talks to the server arrives with the worker. + +Planned for the following changes, recorded here so the picture is complete: a periodic WorkManager job (every 30 minutes by default, 15 minutes to 3 hours selectable) initialised on demand so that WorkManager is never touched while the switch is off; the two existing read-only queries over one connection; fail-closed certificate trust and no prompts in the background; no foreground service, no exact alarms, no battery-optimisation exemption request; `POST_NOTIFICATIONS` asked for only when the user turns the switch on. + +## Consequences + +**Positive** + +- Users learn about a failing disk, an array that is not started or a degraded array without opening NOVA, with no relay, no push service, no Google services and no server plugin. +- The decision logic is pure and fully unit-tested before anything runs in the background, and the Overview card and a later widget share its rules. +- Nothing changes for users who leave the switch off. + +**Negative / trade-offs** + +- Android decides when a periodic job runs. Under Doze and OEM battery managers an alert can come late, sometimes by an hour or more, and on Android 13 and later an app that is not opened for 8 days is limited to about one run a day. +- WorkManager adds three permissions (`WAKE_LOCK`, `RECEIVE_BOOT_COMPLETED`, `ACCESS_NETWORK_STATE`) to every install, even with the switch off; the privacy policy has to name them and say the checks are active only while alerts are on. +- A notification is seen only if Unraid keeps it as unread (browser delivery on, the default). Users who turned that off get no alerts for that importance. +- A scheduled parity check raises one warning when it starts; "Alerts only" removes it but also drops warnings such as SMART or read errors. +- Raising a limit later (more than 300 remembered ids, more than five alerts per check) is a data-format and wording decision, not a free change. + +**Constraints on other work** + +- A biometric app lock (#323) must gate the UI only. If it bound the Keystore key to user authentication or to an unlocked device, background checks could no longer decrypt the key. +- Targeting Android API 37 needs the `ACCESS_LOCAL_NETWORK` permission for connections inside the home network; until then NOVA's target level (36) is unaffected. Such a target bump must treat a blocked local network as "cannot check", never as "server down". + +**Trigger to revisit** + +- Unraid ships a Web Push sender or a generic webhook agent (instant alerts without polling become possible). +- A widget needs a refresh of its own beyond this job. +- Users report that missing alerts for notifications that only reach the archive matter; reading the archive would then be added. + +## Alternatives considered + +**UnifiedPush through the user's own ntfy.** Unraid's agents send plain posts, while UnifiedPush requires RFC 8291 encrypted messages. Unraid has no such sender and the project excludes a relay or server plugin that could add one. Users who already run ntfy can get instant alerts with configuration only (Unraid's ntfy agent to their own server, read by the ntfy app); the documentation will say so. + +**Firebase Cloud Messaging.** Needs Google services and a sender the project would have to run. Excluded by the project's principles. + +**A foreground service holding the notification subscription open.** Needs a permanent notification, and since Android 12 a foreground service cannot be started from the background; the cost is far higher than a short periodic check. + +**Raising disk and pool problems from the array data as well.** Duplicates Unraid's own notifications and doubles every alert. The allow-list of two keys, with a test that fails when it grows, keeps this a deliberate decision. + +**No alerts, documentation only.** Leaves the users who asked without an answer; the opt-in design costs nothing for everyone else. + +## References + +- Issue #321 (alerts outside the app) and the design posted there. +- [ADR-0017](./0017-domain-split-queries-with-lifecycle-polling.md): the foreground-only polling rule this amends. +- [ADR-0035](./0035-apikey-store-decrypt-failure-semantics.md): a background check never resets keys. +- [ADR-0041](./0041-host-ssl-entry-and-self-signed-local-trust.md): background trust fails closed. diff --git a/docs/adr/README.md b/docs/adr/README.md index 7a658e70..78220504 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -51,7 +51,7 @@ ADRs live under `docs/` so the CI docs-only bypass applies — pure-ADR PRs comp | [0014](./0014-single-ci-run-per-pr.md) | Single CI run per PR — release workflow resolves through PR | Accepted | | [0015](./0015-promotion-requires-new-commit.md) | Promotion to a different version-string requires a new commit | Accepted | | [0016](./0016-long-running-apollo-client.md) | Separate Apollo client for long-running mutations | Accepted | -| [0017](./0017-domain-split-queries-with-lifecycle-polling.md) | Domain-split queries with lifecycle-aware polling | Accepted | +| [0017](./0017-domain-split-queries-with-lifecycle-polling.md) | Domain-split queries with lifecycle-aware polling | Accepted (amended 2026-11-13 by ADR-0048) | | [0018](./0018-gradle-cache-warm-on-main.md) | Populate the Gradle build cache on push to main | Reversed 2026-05-17 (no measurable CI speedup in practice) | | [0019](./0019-parallel-debug-release-jobs.md) | Split CI into parallel debug + release jobs | Accepted | | [0020](./0020-migrate-to-ksp2.md) | Migrate from KSP1 to KSP2 | Accepted | @@ -82,4 +82,5 @@ ADRs live under `docs/` so the CI docs-only bypass applies — pure-ADR PRs comp | [0045](./0045-array-disk-display.md) | Array disk display — sleeping disks, per-disk thresholds, errors | Accepted (updated beta10 — 2026-05-30) | | [0046](./0046-update-via-browser-download.md) | Offer updates of the GitHub build as a browser download, without an in-app installer | Accepted | | [0047](./0047-server-capabilities.md) | Detect the server's API version and what the API key may do | Accepted | +| [0048](./0048-opt-in-background-alert-checks.md) | Opt-in background alert checks (amends ADR-0017) | Accepted | | [0049](./0049-google-play-third-distribution-channel.md) | Google Play as a third distribution channel | Accepted | From 816d88d08712978f1be7f6ea6d2b3b2ef79cb980 Mon Sep 17 00:00:00 2001 From: Thomas Kroll Date: Fri, 13 Nov 2026 18:17:27 +0100 Subject: [PATCH 4/4] address review findings - An array with state Offline (not known) now counts as not loaded, so it neither clears the open conditions nor sets the baseline. - "Alerts only" no longer suppresses the two array conditions; Unraid never notifies about them. ADR-0048 item 5 updated. - ADR-0048: only WAKE_LOCK and RECEIVE_BOOT_COMPLETED are new permissions; ACCESS_NETWORK_STATE is already declared, FOREGROUND_SERVICE is stripped. Co-Authored-By: Claude Sonnet 5.5 --- .../nova/data/alerts/AlertRules.kt | 15 ++++++----- .../nova/data/alerts/AlertRulesTest.kt | 26 ++++++++++++------- .../0048-opt-in-background-alert-checks.md | 6 ++--- 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/data/alerts/AlertRules.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/data/alerts/AlertRules.kt index 19f1d196..ddd2a893 100644 --- a/app/src/main/kotlin/io/github/nofuturekid/nova/data/alerts/AlertRules.kt +++ b/app/src/main/kotlin/io/github/nofuturekid/nova/data/alerts/AlertRules.kt @@ -1,6 +1,7 @@ package io.github.nofuturekid.nova.data.alerts import io.github.nofuturekid.nova.data.model.ArrayInfo +import io.github.nofuturekid.nova.data.model.ArrayState import io.github.nofuturekid.nova.data.model.AttentionSeverity import io.github.nofuturekid.nova.data.model.AttentionSnapshot import io.github.nofuturekid.nova.data.model.AttentionTarget @@ -69,7 +70,7 @@ fun foldedSummaryText(serverName: String, count: Int): String = "and $count more * [notifications] and [array] are what the check returned; null means "not loaded this * time" (server unreachable, query failed). A null half changes nothing and posts nothing, * so alerts created during an outage are posted once the server answers again, and nothing - * is posted twice. + * is posted twice. [array] with state Offline (not known) counts as not loaded. * * Notifications: every unread notification of importance alert or warning whose id is not * remembered is new. The very first successful read only remembers the ids, so switching @@ -81,8 +82,9 @@ fun foldedSummaryText(serverName: String, count: Int): String = "and $count more * were gone at an in-between read. The rules are those of [attentionItems], wrapped and * restricted to [STATE_ALERT_KEYS], so the wording is the Overview card's own. * - * [level]: [AlertLevel.AlertsOnly] posts only critical items. Items it filters out are still - * remembered, so switching to [AlertLevel.AlertsAndWarnings] later does not post old ones. + * [level]: [AlertLevel.AlertsOnly] leaves out warning-level notifications; the two array + * conditions are always posted. Notifications it filters out are still remembered, so switching + * to [AlertLevel.AlertsAndWarnings] later does not post old ones. */ fun evaluateAlerts( server: ServerRef, @@ -91,7 +93,7 @@ fun evaluateAlerts( array: ArrayInfo?, level: AlertLevel, ): AlertEvaluation { - val (conditionAlerts, afterConditions) = evaluateConditions(previous, array, level) + val (conditionAlerts, afterConditions) = evaluateConditions(previous, array) val (notificationAlerts, next) = evaluateNotifications(afterConditions, notifications, level) // sortedByDescending is stable: equal severity keeps conditions before notifications. val ordered = (conditionAlerts + notificationAlerts).sortedByDescending { it.severity } @@ -127,16 +129,15 @@ private fun evaluateNotifications( private fun evaluateConditions( previous: ServerAlertState, array: ArrayInfo?, - level: AlertLevel, ): Pair, ServerAlertState> { - if (array == null) return emptyList() to previous + if (array == null || array.state == ArrayState.Offline) return emptyList() to previous val current = attentionItems(AttentionSnapshot(array, notifications = null, thresholds = null)) .filter { it.key in STATE_ALERT_KEYS } val alerts = if (!previous.conditionsBaselined) { emptyList() } else { current - .filter { it.key !in previous.openConditions && level.allows(it.severity) } + .filter { it.key !in previous.openConditions } .map { Alert(it.key, AlertSource.ArrayCondition, it.severity, it.title, it.detail, timestamp = null, target = it.target) } diff --git a/app/src/test/kotlin/io/github/nofuturekid/nova/data/alerts/AlertRulesTest.kt b/app/src/test/kotlin/io/github/nofuturekid/nova/data/alerts/AlertRulesTest.kt index c269468d..0ec6191b 100644 --- a/app/src/test/kotlin/io/github/nofuturekid/nova/data/alerts/AlertRulesTest.kt +++ b/app/src/test/kotlin/io/github/nofuturekid/nova/data/alerts/AlertRulesTest.kt @@ -135,16 +135,12 @@ class AlertRulesTest { assertEquals(setOf("w1", "a1"), result.toPost.map { it.key }.toSet()) } - @Test fun alerts_only_posts_a_degraded_array_but_not_a_stopped_one_and_remembers_both() { - // WHY: a stopped array is a warning-level condition; "Alerts only" means the critical ones. - // Later widening must not post a stopped array that has been stopped all along. - val state = baselined() - val stopped = evaluateAlerts(tower, state, null, array(ArrayState.Stopped), AlertLevel.AlertsOnly) - assertEquals(emptyList(), stopped.toPost) - assertEquals(setOf("array-stopped"), stopped.nextState.openConditions) - assertEquals(emptyList(), evaluateAlerts(tower, stopped.nextState, null, array(ArrayState.Stopped), both).toPost) + @Test fun alerts_only_still_posts_an_array_that_is_not_started() { + // WHY: Unraid never notifies about a stopped array; the level only quiets Unraid's own warnings. + val stopped = evaluateAlerts(tower, baselined(), null, array(ArrayState.Stopped), AlertLevel.AlertsOnly) + assertEquals(listOf("array-stopped"), stopped.toPost.map { it.key }) - val degraded = evaluateAlerts(tower, state, null, array(ArrayState.Error), AlertLevel.AlertsOnly) + val degraded = evaluateAlerts(tower, baselined(), null, array(ArrayState.Error), AlertLevel.AlertsOnly) assertEquals(listOf("array-degraded"), degraded.toPost.map { it.key }) } @@ -322,6 +318,18 @@ class AlertRulesTest { assertEquals(listOf("n1"), result.nextState.seenNotificationIds) } + @Test fun an_unknown_array_state_keeps_the_condition_half() { + // WHY: Offline means "not known" (no array block, unknown enum value), not "condition gone"; + // clearing the open condition would post the same stopped array twice in one episode. + val unknown = evaluateAlerts(tower, baselined(open = setOf("array-stopped")), null, array(ArrayState.Offline), both) + assertEquals(emptyList(), unknown.toPost) + assertEquals(setOf("array-stopped"), unknown.nextState.openConditions) + assertEquals(emptyList(), evaluateAlerts(tower, unknown.nextState, null, array(ArrayState.Stopped), both).toPost) + + // A first read that is unknown must not count as the silent baseline. + assertFalse(evaluateAlerts(tower, ServerAlertState(), null, array(ArrayState.Offline), both).nextState.conditionsBaselined) + } + // ── Helpers ─────────────────────────────────────────────────────── private fun baselined(seen: List = emptyList(), open: Set = emptySet()) = diff --git a/docs/adr/0048-opt-in-background-alert-checks.md b/docs/adr/0048-opt-in-background-alert-checks.md index 5c632cb3..25028418 100644 --- a/docs/adr/0048-opt-in-background-alert-checks.md +++ b/docs/adr/0048-opt-in-background-alert-checks.md @@ -27,13 +27,13 @@ What is decided here, and what this first change implements: 2. **Once per notification.** Each server remembers the ids of the notifications it has seen. An id is never posted twice, even if the notification leaves the unread list and returns. The id is built from the event time, so a later notification with the same text is a new event and is posted. The list is kept in insertion order and capped at 300; an id that is still on the server is never forgotten before one that is gone, so the stored state cannot grow without bound. 3. **First check is silent.** The first successful read of a server after the switch is turned on, or after the server is added, only records what is already there and posts nothing. Turning the switch off forgets all state, so the next start is a first check again. The same rule applies to the array conditions. 4. **Array conditions once per episode.** A condition is raised when it begins and again only after it was gone at an in-between check. -5. **Level.** "Alerts and warnings" posts everything above; "Alerts only" posts only critical items, that is alert notifications and a degraded array. Items the level filters out are still remembered, so widening the level later does not post old ones. +5. **Level.** "Alerts and warnings" posts everything above; "Alerts only" leaves out new warning notifications. The two array conditions are always posted, because Unraid never notifies about them. Notifications the level filters out are still remembered, so widening the level later does not post old ones. 6. **Failures change nothing.** A check that did not load a half (server unreachable, certificate problem, key missing, request rejected) neither posts nor alters the remembered state of that half, so alerts created during an outage are posted once when the server answers again and nothing is posted twice. A failed check never counts as the silent first check. 7. **Several servers.** Every saved server has its own state, keyed by its stable id, and the state is removed with the server entry. 8. **At most five alerts per server and check** get their own notification, most urgent first; the rest are folded into one summary line. 9. **One-shot seam.** What one check needs from one server (the unread alerts and warnings, the array state, and how the attempt went) is behind a small interface, so the worker, and later a widget, share one code path. This change adds the interface and a test fake; the implementation that talks to the server arrives with the worker. -Planned for the following changes, recorded here so the picture is complete: a periodic WorkManager job (every 30 minutes by default, 15 minutes to 3 hours selectable) initialised on demand so that WorkManager is never touched while the switch is off; the two existing read-only queries over one connection; fail-closed certificate trust and no prompts in the background; no foreground service, no exact alarms, no battery-optimisation exemption request; `POST_NOTIFICATIONS` asked for only when the user turns the switch on. +Planned for the following changes, recorded here so the picture is complete: a periodic WorkManager job (every 30 minutes by default, 15 minutes to 3 hours selectable) initialised on demand so that WorkManager is never touched while the switch is off; the two existing read-only queries over one connection; fail-closed certificate trust and no prompts in the background; no foreground service, no exact alarms, no battery-optimisation exemption request; `FOREGROUND_SERVICE` stripped from the merged manifest; `POST_NOTIFICATIONS` asked for only when the user turns the switch on. ## Consequences @@ -46,7 +46,7 @@ Planned for the following changes, recorded here so the picture is complete: a p **Negative / trade-offs** - Android decides when a periodic job runs. Under Doze and OEM battery managers an alert can come late, sometimes by an hour or more, and on Android 13 and later an app that is not opened for 8 days is limited to about one run a day. -- WorkManager adds three permissions (`WAKE_LOCK`, `RECEIVE_BOOT_COMPLETED`, `ACCESS_NETWORK_STATE`) to every install, even with the switch off; the privacy policy has to name them and say the checks are active only while alerts are on. +- WorkManager adds two permissions, `WAKE_LOCK` and `RECEIVE_BOOT_COMPLETED`, to every install, even with the switch off. It also merges `ACCESS_NETWORK_STATE`, which NOVA already declares, and `FOREGROUND_SERVICE`, which the worker change removes with `tools:node="remove"` because NOVA never starts a foreground service. The privacy policy has to name the two new ones and say the checks are active only while alerts are on. - A notification is seen only if Unraid keeps it as unread (browser delivery on, the default). Users who turned that off get no alerts for that importance. - A scheduled parity check raises one warning when it starts; "Alerts only" removes it but also drops warnings such as SMART or read errors. - Raising a limit later (more than 300 remembered ids, more than five alerts per check) is a data-format and wording decision, not a free change.