diff --git a/AGENTS.md b/AGENTS.md index 35cc2b3..3adee99 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -587,6 +587,36 @@ mutable at runtime from the phone. differ in nothing but the label and the old one said "Pulling and building" for all three. +- **Freshness says *which* answer it is, and only three of them read as + out of date.** `Freshness` was `current | behind | unknown`, and the + card drew unknown as nothing at all -- so a component this server had + never built looked exactly like one whose build was current, and the + only difference lived in a config file on the other machine. That + silence cost two rounds of "why isn't it flagging anything" that no + amount of looking at the phone could answer. It now names the state: + `behind`, `neverBuilt` and `otherMode` are measured and have something + to do about them; `uncommitted`, `notBuiltHere`, `noCheckout` and + `parked` are the ways of not knowing, and each says so in the row in + the ordinary text colour -- words, not a colour, because "we could not + check" is a difference in kind from "there is something here". + `ProjectComponent.isStale` is the one place the division is written + down, and it decides all three of: whether the row says "out of date", + whether the card sits above the "Up to date" heading, and whether + Update is pressable. Iris's rule for it, 2026-09-02: "it should not do + that until things actually can be updated." + +- **What a build recorded is carried across the self row being + re-derived, like the settings beside it.** `reconcile_self` rebuilds + this server's own row from its declaration at every startup, and + `built_from`/`built_mode` were not among the things carried -- so the + commit written down by the build that produced the new binary was + forgotten by the process that build started, and restarting is how this + server takes an update. Its own card then had no freshness ever again: + never behind, never current, and its Update with nothing to do, since + both readings need a commit to compare against. `built_records` / + `restore_built` carry it, keyed by name *and* directory like every + other carry-across here. + - **One predicate decides both the "Up to date" heading and whether Update can be pressed** (`hasWorkWaiting` in `UpdaterScreen.kt`): they are the same question, since Update is the button that clears diff --git a/app/androidApp/src/main/kotlin/com/example/devupdater/UpdateManifest.kt b/app/androidApp/src/main/kotlin/com/example/devupdater/UpdateManifest.kt index 11395e5..affc535 100644 --- a/app/androidApp/src/main/kotlin/com/example/devupdater/UpdateManifest.kt +++ b/app/androidApp/src/main/kotlin/com/example/devupdater/UpdateManifest.kt @@ -84,6 +84,15 @@ data class GitStatus( val root: String, ) +/** + * What a busy component's freshness is replaced with while it is being worked on. + * + * A word of the app's own rather than one of the server's, because it is not a measurement: it + * means "withheld until this run is over", and every reading of freshness treats it as saying + * nothing. The server's own words all describe something it actually looked at. + */ +const val FRESHNESS_WITHHELD = "withheld" + /** * What a build button says for the components it covers. * @@ -180,11 +189,45 @@ data class ProjectComponent( // for a server, which builds nothing this phone installs. val apk: ComponentApk?, ) { - // Only "behind" is worth saying. "Current" is what a card already - // implies, and "unknown" said out loud would be on most rows most of - // the time, which is how a mark stops meaning anything. - val isBehind: Boolean - get() = freshness == "behind" + /** + * Whether there is something measured to act on: the build is behind the checkout, there is no + * build at all, or what is there was built in another mode. + * + * The one place that division is written down, because it decides three things that must not + * disagree -- whether the row says "out of date", whether the card sits above the "Up to date" + * heading, and whether Update can be pressed. Everything else is either current or a state + * nobody could measure, and calling those out of date is a nag with nothing behind it: Iris + * asked for exactly that on 2026-09-02, "it should not do that until things actually can be + * updated". + */ + val isStale: Boolean + get() = freshness in setOf("behind", "neverBuilt", "otherMode") + + /** + * What the row says about this component's build, or null where it says nothing. + * + * The three ways of not knowing get words of their own rather than the silence they used to + * share with "current". That silence is what made "why isn't it flagging anything" impossible + * to answer from the phone: a component this server has never built looked exactly like one + * whose build was up to date, and the only difference was in a config file on the other + * machine. They are deliberately *not* coloured like "out of date" -- they are facts about what + * could not be measured, not a button to press. + */ + val freshnessNote: String? + get() = + when (freshness) { + "behind" -> "out of date" + "neverBuilt" -> "never built" + "otherMode" -> "built in another mode" + "uncommitted" -> "uncommitted changes" + "notBuiltHere" -> "not built here" + // "current", "parked", "noCheckout": nothing to say. The + // first is what a card already implies, and the other two + // are said by the branch line above -- a parked checkout + // shows "no branch", and a project outside git shows no + // branch line at all. + else -> null + } val isServer: Boolean get() = kind == "server" diff --git a/app/androidApp/src/main/kotlin/com/example/devupdater/UpdaterScreen.kt b/app/androidApp/src/main/kotlin/com/example/devupdater/UpdaterScreen.kt index c0f82cb..6421cc2 100644 --- a/app/androidApp/src/main/kotlin/com/example/devupdater/UpdaterScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/devupdater/UpdaterScreen.kt @@ -2016,10 +2016,10 @@ private fun AppCard( // was pressed and is read again only once the run is // over. So "out of date" beside the bar that is making // it current is not a measurement, it is the answer - // from before the press -- and unknown is what this - // card already says when it has no reading, so both - // readers of it (the row's own note, and the sibling - // warning) go quiet without either having to know why. + // from before the press -- so it is replaced by a word + // that says nothing, and both readers of it (the row's + // own note, and the sibling warning) go quiet without + // either having to know why. // Deliberately the same pair of conditions that // disables the Update button: what cannot be acted on // is exactly what cannot be measured just now. @@ -2028,7 +2028,7 @@ private fun AppCard( .sortedBy { it.isServer } .map { component -> if (projectState.busy || componentStates[component.name].busy) - component.copy(freshness = "unknown") + component.copy(freshness = FRESHNESS_WITHHELD) else component } components.forEach { component -> @@ -3213,7 +3213,7 @@ private fun ComponentCard( // Said in words, not by colour alone: "behind the // checkout" is a difference in kind from "running", and a // reader has no way to learn a colour that means it. - val behind = component.isBehind + val freshnessNote = component.freshnessNote val status = when { !component.isServer -> null @@ -3246,14 +3246,19 @@ private fun ComponentCard( overflow = TextOverflow.Ellipsis, ) } - if (behind) { + freshnessNote?.let { Separator() Text( - "out of date", + it, style = MaterialTheme.typography.bodyMedium, - // The colour Pull & Build wears, because that is - // the button this is telling you to press. - color = ActionTone.Primary.color, + // Update's colour for the states with something + // to do, because that is the button this is + // telling you to press -- and the ordinary text + // colour for the ones that only say what could + // not be measured, which is not a call to act. + color = + if (component.isStale) ActionTone.Primary.color + else MaterialTheme.colorScheme.onSurfaceVariant, maxLines = 1, overflow = TextOverflow.Ellipsis, ) @@ -4201,7 +4206,7 @@ private fun hasWorkWaiting( !entry.built || entry.newCommits || entry.components.any { component -> - component.isBehind || + component.isStale || component.apk?.let { !isUpToDate(it, installedTimes[component.name], chosenVariants[component.name]) } == true diff --git a/server/src/build_state.rs b/server/src/build_state.rs index 03d1c17..a2efdf1 100644 --- a/server/src/build_state.rs +++ b/server/src/build_state.rs @@ -33,13 +33,23 @@ pub struct Progress { /// anything -- only that the second is the whole of it. const PROGRESS_MARKER: &str = "@@progress "; -/// What is known about whether a component's build is current. +/// What is known about whether a component's build is current, and why. /// -/// Three states rather than a boolean, because "we cannot tell" must not -/// share a value with either answer. A project this server has never -/// built, one outside git, and one with uncommitted work in its directory -/// are all *unknown* -- and an unknown drawn as `Current` would be the -/// exact failure this was added to stop. +/// More than "yes, no, don't know" because the card has to *say* which, +/// and because the three ways of not knowing are three different things +/// to do next. It was three states for a while and the silence cost two +/// rounds of "why isn't it flagging anything": a component this server +/// has never built and one whose build is genuinely current looked +/// identical on the card, which is the failure the unknown state exists +/// to prevent, one level further in. +/// +/// The states divide into three groups, and the phone is where that +/// division is written down (`ProjectComponent.isStale`, one place): +/// something to do about it (`Behind`, `NeverBuilt`, `OtherMode`), +/// nothing to do because it is current, and *cannot tell* (the rest). +/// Only the first group makes a card read as out of date -- saying that +/// about something nobody measured, or about a commit somebody +/// deliberately parked on, is a nag with nothing behind it. #[derive(Clone, Copy, PartialEq, Eq, Debug, Serialize)] #[serde(rename_all = "camelCase")] pub enum Freshness { @@ -47,9 +57,25 @@ pub enum Freshness { Current, /// Its directory has moved past the commit it was built from. Behind, - /// Never built here, no checkout to compare against, uncommitted work - /// in its directory, or a checkout parked on a commit somebody chose. - Unknown, + /// Nothing where this component's builds land, so there is a first + /// build to make rather than a comparison to draw. + NeverBuilt, + /// Built, but in a mode other than the one now chosen -- which no + /// commit can express, since the checkout has not moved. + OtherMode, + /// Cannot tell: uncommitted work in this component's directory makes + /// the comparison unreliable. Somebody is editing there and building + /// it themselves. + Uncommitted, + /// Cannot tell: this server has never built it, so there is no commit + /// to compare the build on disk against. An ordinary state for a + /// project somebody builds by hand. + NotBuiltHere, + /// Cannot tell: no checkout under this project, or git would not say. + NoCheckout, + /// Withheld: the checkout is parked on a commit somebody chose, so a + /// differing commit is a decision rather than something behind. + Parked, } /// How a finished component reports the commit it was built from. @@ -590,7 +616,7 @@ impl BuildState { // commit the debug build was made from still does not make that // build a release one. if component.built_in_another_mode() { - return Freshness::Behind; + return Freshness::OtherMode; } // Nothing built at all, which no commit can express either: the // comparison below is between two commits, so a component whose @@ -602,7 +628,7 @@ impl BuildState { // produced that: the staleness rules knew there was no build and // the card did not. if self.nothing_built(component) { - return Freshness::Behind; + return Freshness::NeverBuilt; } let built = self .inner @@ -612,17 +638,18 @@ impl BuildState { .get(component.name()) .cloned(); let Some(built) = built else { - return Freshness::Unknown; + return Freshness::NotBuiltHere; }; - if crate::git::subtree_dirty(&self.project_path, &component.watched_paths()) != Some(false) - { - return Freshness::Unknown; + match crate::git::subtree_dirty(&self.project_path, &component.watched_paths()) { + Some(false) => {} + Some(true) => return Freshness::Uncommitted, + None => return Freshness::NoCheckout, } match crate::git::subtree_head(&self.project_path, &component.watched_paths()) { Some(current) if current == built => Freshness::Current, - Some(_) if parked => Freshness::Unknown, + Some(_) if parked => Freshness::Parked, Some(_) => Freshness::Behind, - None => Freshness::Unknown, + None => Freshness::NoCheckout, } } @@ -2021,12 +2048,12 @@ mod tests { ); assert_eq!( switched.freshness(&switched.components[0], false), - Freshness::Behind, + Freshness::OtherMode, "and the card has to say so rather than reading as current", ); assert_eq!( switched.freshness(&switched.components[0], true), - Freshness::Behind, + Freshness::OtherMode, "a parked checkout says nothing about which mode was built, so this one still \ reports -- what the parking silences is the commit comparison alone", ); @@ -2073,11 +2100,55 @@ mod tests { ); assert_eq!( state.freshness(app, true), - Freshness::Unknown, + Freshness::Parked, "and one parked on a chosen commit withholds it rather than nagging", ); } + /// The three ways of not knowing are three different answers, and the + /// card has to be able to say which. + /// + /// They were one `Unknown` that the card drew as nothing at all, and + /// the silence cost two rounds of "why isn't it flagging anything" -- + /// a component this server has never built looked exactly like one + /// whose build was current. Neither of these is *stale*: there is + /// nothing measured to act on, and saying "out of date" about them + /// would be a nag with nothing behind it. + #[test] + fn not_knowing_says_which_kind_it_is() { + let dir = tempfile::tempdir().expect("tempdir"); + let root = dir.path(); + let components = two_component_checkout(root); + let head = crate::git::subtree_head(root, &[PathBuf::from("app")]).expect("a commit"); + let state = state_for(root, components); + let app = state + .components + .iter() + .find(|component| component.name() == "app") + .expect("the app component"); + + assert_eq!( + state.freshness(app, false), + Freshness::NotBuiltHere, + "no build of ours to compare, which is an ordinary state for a project built by hand", + ); + + state + .inner + .lock() + .unwrap() + .built_from + .insert("app".to_string(), head); + assert_eq!(state.freshness(app, false), Freshness::Current); + + std::fs::write(root.join("app/main.kt"), "edited, not committed").expect("write"); + assert_eq!( + state.freshness(app, false), + Freshness::Uncommitted, + "somebody is editing in there, so the commit comparison says nothing", + ); + } + /// Moving the checkout runs git and nothing else. /// /// It used to build whatever the move left behind, which made looking @@ -2226,7 +2297,7 @@ mod tests { std::fs::remove_file(root.join("app/build/outputs/apk/debug/a.apk")).expect("rm"); assert_eq!( state.freshness(app, false), - Freshness::Behind, + Freshness::NeverBuilt, "the commits still match, and there is nothing built to match them", ); assert!( diff --git a/server/src/config.rs b/server/src/config.rs index 42adbf6..1c5d1e0 100644 --- a/server/src/config.rs +++ b/server/src/config.rs @@ -843,6 +843,40 @@ impl Component { *package = Some(measured); } } + + /// What a build here recorded about this component: the commit it was + /// built from and the mode it was built in. + /// + /// Read and put back together, because they are one fact + /// (`built_in_another_mode` compares them) and half of it is worse + /// than neither: a commit with no mode reads as a build made before + /// modes existed and stays quiet, which is right, while a mode with no + /// commit is a claim about a build nothing recorded. + pub fn built(&self) -> (Option, Option) { + ( + self.built_from().map(str::to_string), + self.built_mode().map(str::to_string), + ) + } + + /// Puts back what [`Self::built`] took. + pub fn set_built(&mut self, (from, mode): (Option, Option)) { + match self { + Self::Apk { + built_from, + built_mode, + .. + } + | Self::Server { + built_from, + built_mode, + .. + } => { + *built_from = from; + *built_mode = mode; + } + } + } } /// Whether any of these has something to run, as opposed to a checkout diff --git a/server/src/registry.rs b/server/src/registry.rs index 015c0e1..749185c 100644 --- a/server/src/registry.rs +++ b/server/src/registry.rs @@ -352,6 +352,30 @@ fn restore_chosen_settings( } } +/// What a build recorded about each component, keyed the way every other +/// carry-across here is: by name *and* directory, so a reused name for a +/// component that now builds somewhere else does not inherit the old one's +/// commit. +fn built_records( + components: &[Component], +) -> HashMap, Option)> { + components + .iter() + .map(|component| (component_id(component), component.built())) + .collect() +} + +fn restore_built( + components: &mut [Component], + recorded: &HashMap, Option)>, +) { + for component in components { + if let Some(built) = recorded.get(&component_id(component)) { + component.set_built(built.clone()); + } + } +} + fn measured_packages(components: &[Component]) -> HashMap { components .iter() @@ -908,6 +932,16 @@ fn reconcile_self(config: &mut Config, self_project: &Path) -> bool { // would not survive being acted on. if let Some(existing) = existing.as_ref() { restore_chosen_settings(&mut components, &chosen_settings(&existing.components)); + // And what a build here measured, for the same reason and with + // more force: this row is re-derived at *every* startup, and + // restarting is how this server takes its own update -- so a + // commit recorded by the build that produced the new binary was + // forgotten by the process it started. Its own card then had no + // freshness for ever after: never behind, never current, and its + // Update with nothing to do, because both readings need a commit + // to compare against. Carried across rather than re-derived, + // because nothing but a build can know it. + restore_built(&mut components, &built_records(&existing.components)); } let derived = ProjectConfig { key: SELF_KEY.to_string(), @@ -1578,6 +1612,38 @@ mod tests { assert!(entries[0].git_ipv4); } + /// What a build recorded survives the row being re-derived too, and + /// this is the row where it matters most: restarting is how this + /// server takes its own update, so the commit written down by the + /// build that produced the new binary is read by the process it + /// started. Dropped, this server's own card had no freshness for ever + /// after -- never behind, never current, and its Update with nothing + /// to do, because both readings need a commit to compare against. + #[test] + fn what_a_build_recorded_survives_the_built_in_row_being_re_derived() { + let dir = tempfile::tempdir().expect("tempdir"); + let app = self_checkout(dir.path()); + let mut config = Config::default(); + reconcile_self(&mut config, &app); + + let name = config.projects[0].components[0].name().to_string(); + config.projects[0].components[0] + .set_built((Some("c0ffee".to_string()), Some("release".to_string()))); + + reconcile_self(&mut config, &app); + let component = config.projects[0] + .components + .iter() + .find(|component| component.name() == name) + .expect("the component is still there"); + assert_eq!( + component.built_from(), + Some("c0ffee"), + "the commit a build recorded is a measurement, not something the declaration derives", + ); + assert_eq!(component.built_mode(), Some("release")); + } + /// What is derived stays derived. The symptom this prevents is the one /// the path being compiled in used to cause: a row naming a directory /// nobody pulls, on a card that otherwise works. diff --git a/server/src/routes.rs b/server/src/routes.rs index 072e934..2069f9a 100644 --- a/server/src/routes.rs +++ b/server/src/routes.rs @@ -456,7 +456,9 @@ impl ManifestComponent { .build .as_ref() .map(|build| build.freshness(component, parked)) - .unwrap_or(crate::build_state::Freshness::Unknown), + // No build machinery at all: nothing could have recorded + // a commit, and that is not the same as being current. + .unwrap_or(crate::build_state::Freshness::NotBuiltHere), data_path: state_paths .as_ref() .and_then(|paths| paths.data.as_deref())