Keep what a build recorded when a declaration is accepted

Accepting a project's declaration replaced its components with the
declared ones and carried across two of the three things this machine
knows about them -- the package read out of an APK and the mode somebody
picked here -- while dropping `builtFrom`. So every card of that project
went back to saying it had never been built here: no freshness, nothing
flagged, and Update with nothing to do, which is exactly what "I switched
branches on ai-app and nothing was flagged" looks like from the phone.

Silent, too, because "never built here" is a true sentence about plenty
of projects. The same field went missing from the built-in row's startup
reconciliation last week; two carry-across pairs remembered separately is
what produced both, so there is now one `CarriedOver` for all three and
both callers take it whole.

The built-in card's own update no longer goes through the self-update
dialog. Its Update pulls, builds and installs this app like any other
card's, downloading over the frozen /self/apk route -- the build that
produced the APK rebuilt this server too, so the one download that has to
survive the server changing underneath it takes the route nothing can
rename. It waits for the server to be answering again both before
fetching and before handing anything to the installer, since the restart
lands somewhere in that window either way.

The dialog stays for what it was written for: it is offered when the
built-in card is missing from the list, or when there is no list at all
because this app is too old to read what the server now sends. That is
the one case where nothing on screen can offer the update itself.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

app/androidApp/src/main/kotlin/com/example/devupdater/ApkInstaller.kt

@@ -36,6 +36,18 @@ fun downloadApk(
     // second phone must not have its download changed by it. The
     // server checks the path against that component's builds, so a stale
     // one falls back to its newest rather than naming a file.
+    // This app's own APK is the one download that has to survive the
+    // server changing underneath it: the build that produced it rebuilt
+    // the server too, and the process answering afterwards is a newer one
+    // than the app asking. So the built-in project goes over the frozen
+    // rescue route rather than the manifest's, which is the same route
+    // and the same file the self-update check offers -- the difference is
+    // only that nothing about it can be renamed. It carries no component
+    // or variant because it cannot: the built-in project has one APK, and
+    // which build of it to serve is the mode set on the build machine.
+    if (entry.builtIn) {
+        return downloadFromRoute(context, SELF_APK_ROUTE, "self", onProgress)
+    }
     val query = StringBuilder("?component=").append(encode(component))
     chosenVariant(context, entry.key, component)?.let {
         query.append("&variant=").append(encode(it))

app/androidApp/src/main/kotlin/com/example/devupdater/UpdaterScreen.kt

@@ -161,6 +161,17 @@ private const val REFRESH_ATTEMPTS_AFTER_PULL = 4
 private const val WAKE_RETRY_MS = 600L
 private const val RESTART_WAIT_MS = 1000L
 
+/**
+ * How many of those to wait through when the built-in project's own update is what is happening.
+ *
+ * Longer than [REFRESH_ATTEMPTS_AFTER_PULL] because this is not a retry of a read that might
+ * succeed anyway — it is waiting out a restart that is definitely happening, and one going through
+ * a service manager is a stop and a start rather than an exec. Bounded all the same: the APK is
+ * already downloaded by the second of the two waits, and never installing it would be the worse
+ * failure.
+ */
+private const val RESTART_ATTEMPTS = 15
+
 private sealed class ManifestState {
     data object Loading : ManifestState()
 
@@ -311,12 +322,20 @@ fun UpdaterScreen(settingsVersion: Int) {
     // not -- which is exactly when replacing this app matters most.
     var selfUpdate by remember { mutableStateOf<SelfBuild?>(null) }
     val selfUpdateContext = LocalContext.current
-    // On arrival, and again whenever this app's own project has just been
-    // built: the second is the moment the newer copy comes into existence,
-    // and the moment the server it has to keep talking to has just
-    // changed.
-    var selfUpdateTick by remember { mutableStateOf(0) }
-    LaunchedEffect(selfUpdateTick) { selfUpdate = selfUpdateAvailable(selfUpdateContext) }
+    // Offered when, and only when, the list cannot offer it: the built-in
+    // card is not there, or there is no list at all. That card's own
+    // Update now pulls, builds and installs this app like any other, and
+    // a dialog appearing over the card that is already doing it says the
+    // same thing twice -- worse, it says it in the one place a person
+    // cannot see what it is about. What is left is the case this screen
+    // was written for: a server that has changed what the manifest says
+    // into something this app is too old to read, which is exactly when
+    // replacing this app matters most and exactly when nothing on the
+    // list can say so.
+    var selfCardMissing by remember { mutableStateOf(false) }
+    LaunchedEffect(selfCardMissing) {
+        selfUpdate = if (selfCardMissing) selfUpdateAvailable(selfUpdateContext) else null
+    }
     // Keys of apps added on the Add screen, waiting to be taken into the
     // list one at a time.
     var added by remember { mutableStateOf<List<String>>(emptyList()) }
@@ -331,7 +350,7 @@ fun UpdaterScreen(settingsVersion: Int) {
             added = added,
             onAddedApplied = { added = emptyList() },
             onAdd = { adding = true },
-            onOwnProjectBuilt = { selfUpdateTick++ },
+            onSelfCardMissing = { selfCardMissing = it },
         )
         selfUpdate?.let { build ->
             // Over the list for the same reason the Add screen is, and
@@ -500,12 +519,26 @@ private fun AppListScreen(
     added: List<String>,
     onAddedApplied: () -> Unit,
     onAdd: () -> Unit,
-    onOwnProjectBuilt: () -> Unit,
+    /** Whether the built-in project's card is unavailable to offer this app's own update. */
+    onSelfCardMissing: (Boolean) -> Unit,
 ) {
     val context = LocalContext.current
     val scope = rememberCoroutineScope()
 
     var manifestState by remember { mutableStateOf<ManifestState>(ManifestState.Loading) }
+    // Whether the built-in card is unavailable to offer this app's own
+    // update, which is the whole of what the screen above needs from this
+    // one. Derived here rather than asked for, so the rule sits beside the
+    // state it reads. Loading counts as present: not having found out yet
+    // is not the same as an answer, and a dialog that flashed up during
+    // every load would be exactly the noise it is there to avoid.
+    val selfCardMissing =
+        when (val state = manifestState) {
+            is ManifestState.Loaded -> state.manifest.entries.none { it.builtIn }
+            is ManifestState.Error -> true
+            ManifestState.Loading -> false
+        }
+    LaunchedEffect(selfCardMissing) { onSelfCardMissing(selfCardMissing) }
     // What each project is doing, and separately what each of its
     // components is: two levels, like installedTimes below and for the
     // same reason. A project can build two clients, and one of them being
@@ -823,6 +856,31 @@ private fun AppListScreen(
         }
     }
 
+    /**
+     * Waits until the server is answering again, for a bounded time, and says whether it is.
+     *
+     * Only the built-in project needs it, and it needs it twice. Building this project rebuilds
+     * this server's binary and restarts it a moment later, so the two things that come after that
+     * build — fetching the new APK, and handing it to the system installer — both happen while the
+     * process on the other end may be exec-ing into its replacement. A download that dies mid
+     * transfer reads as the update having failed at the moment it was working, and an install
+     * offered during the restart replaces this app while the server it must talk to is down.
+     *
+     * Asked over `/self`, the same frozen route the rescue check uses: it is two numbers and no
+     * nesting, so it answers as soon as the new process is listening whatever else changed.
+     *
+     * Gives up rather than waiting for ever, and the caller carries on anyway: the APK is already
+     * on the phone by then, and refusing to install it because the server is down would strand
+     * somebody at the one moment a newer copy might be the fix.
+     */
+    suspend fun awaitServerBack(): Boolean {
+        repeat(RESTART_ATTEMPTS) {
+            if (withContext(Dispatchers.IO) { runCatching { selfBuild() }.isSuccess }) return true
+            delay(RESTART_WAIT_MS)
+        }
+        return false
+    }
+
     fun install(file: File) {
         if (!canRequestInstall(context)) {
             context.startActivity(requestInstallPermissionIntent(context))
@@ -851,13 +909,6 @@ private fun AppListScreen(
             try {
                 applyOne(entry.key)
                 settle()
-                // Building this app's own project is what produces the
-                // newer copy of it, and restarts the server it has to
-                // keep talking to. So this is the moment to offer it,
-                // rather than at some later launch.
-                if (entry.builtIn) {
-                    onOwnProjectBuilt()
-                }
                 return
             } catch (e: DownloadServerException) {
                 if (attempt == REFRESH_ATTEMPTS_AFTER_PULL - 1) {
@@ -986,6 +1037,17 @@ private fun AppListScreen(
                 }
             }
 
+            // The build that just ran rebuilt this server, so what is
+            // about to be asked for the APK is a process that may be
+            // replacing itself. Waited for here rather than retried after
+            // the fact: a download that dies partway is reported as a
+            // failure, and this is the update where that failure reads as
+            // the update itself having broken.
+            if (entry.builtIn) {
+                setComponent(entry.key, component, ComponentState.Busy("Waiting for the server"))
+                awaitServerBack()
+            }
+
             // Not "downloading" until something is actually coming down:
             // the server may still be producing what it is about to send.
             setComponent(entry.key, component, ComponentState.Fetching)
@@ -1026,6 +1088,16 @@ private fun AppListScreen(
                 setComponent(entry.key, component, ComponentState.ReadyToInstall(file))
                 return@run false
             }
+            // The other half of the wait above: the download itself holds
+            // the restart off while it runs -- the server counts what it
+            // is sending -- so the exec lands, if it lands at all, in the
+            // moment between the last byte and this install. Replacing
+            // this app then leaves the copy that starts next unable to
+            // reach anything, which reads as the update having broken it.
+            if (entry.builtIn) {
+                setComponent(entry.key, component, ComponentState.Busy("Waiting for the server"))
+                awaitServerBack()
+            }
             setComponent(entry.key, component, null)
             install(file)
             true

server/src/registry.rs

@@ -319,58 +319,58 @@ fn component_id(component: &Component) -> ComponentId {
     )
 }
 
-/// What this machine chose about each component, for carrying across a
-/// rewrite of the components list.
+/// Everything about a component that this machine either measured or was
+/// told here, for carrying across a rewrite of the components list.
 ///
-/// The mirror of [`measured_packages`], and here for the same reason:
-/// both acceptance and the self entry's startup reconciliation replace
-/// `components` wholesale with what the checkout declares, which is right
-/// for everything the project asked for and wrong for everything somebody
-/// chose here. Without it, choosing to build this server in `debug`
-/// would last exactly until the next restart -- and restarting is how
-/// this server is updated, so it would never last at all.
+/// There are two places that rewrite one -- accepting a declaration, and
+/// the built-in row's reconciliation at every startup -- and both replace
+/// `components` wholesale with what the checkout declares. That is right
+/// for everything the project asked for and wrong for everything this
+/// side knows: the package read out of an APK, the mode somebody picked,
+/// the commit a build recorded. One type for all three rather than a
+/// collect-and-restore pair each, because the pairs were remembered
+/// individually and one of them was not -- acceptance carried the package
+/// and the chosen mode and dropped `builtFrom`, so accepting a project's
+/// declaration told every one of its cards it had never been built here.
+/// Silent, because that is a true sentence about plenty of projects and
+/// so reads as an answer rather than as a loss. Adding a field to a
+/// component now means adding it here once, and both callers get it.
 ///
-/// Keyed the same way for the same reason: once a component's directory
-/// decides which builds are its own, a reused name is a different
-/// component, and handing it the old one's mode would build something
-/// nobody asked for.
-fn chosen_settings(components: &[Component]) -> HashMap<ComponentId, crate::config::Choices> {
-    components
-        .iter()
-        .map(|component| (component_id(component), component.choices()))
-        .collect()
+/// Keyed by name *and* directory like everything else here: once a
+/// component's directory decides which builds are its own, a reused name
+/// is a different component, and handing it the old one's package or
+/// commit would be wrong until that component happened to be built.
+#[derive(Default)]
+struct CarriedOver {
+    packages: HashMap<ComponentId, String>,
+    chosen: HashMap<ComponentId, crate::config::Choices>,
+    built: HashMap<ComponentId, (Option<String>, Option<String>)>,
 }
 
-fn restore_chosen_settings(
-    components: &mut [Component],
-    chosen: &HashMap<ComponentId, crate::config::Choices>,
-) {
-    for component in components {
-        if let Some(choices) = chosen.get(&component_id(component)) {
-            component.restore(choices.clone());
-        }
+fn carried_over(components: &[Component]) -> CarriedOver {
+    CarriedOver {
+        packages: measured_packages(components),
+        chosen: components
+            .iter()
+            .map(|component| (component_id(component), component.choices()))
+            .collect(),
+        built: components
+            .iter()
+            .map(|component| (component_id(component), component.built()))
+            .collect(),
     }
 }
 
-/// 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<ComponentId, (Option<String>, Option<String>)> {
-    components
-        .iter()
-        .map(|component| (component_id(component), component.built()))
-        .collect()
-}
-
-fn restore_built(
-    components: &mut [Component],
-    recorded: &HashMap<ComponentId, (Option<String>, Option<String>)>,
-) {
+fn restore_carried_over(components: &mut [Component], carried: &CarriedOver) {
     for component in components {
-        if let Some(built) = recorded.get(&component_id(component)) {
+        let id = component_id(component);
+        if let Some(package) = carried.packages.get(&id) {
+            component.set_package(package.clone());
+        }
+        if let Some(choices) = carried.chosen.get(&id) {
+            component.restore(choices.clone());
+        }
+        if let Some(built) = carried.built.get(&id) {
             component.set_built(built.clone());
         }
     }
@@ -793,11 +793,14 @@ impl AppState {
     /// thing that rewrites that file is a pull the same person just
     /// performed.
     ///
-    /// Measured values are carried across rather than reset. A declaration
-    /// says nothing about the package a build produces -- that is read from
-    /// the APK -- so accepting one must not throw away what was already
-    /// read, or every acceptance would blank the installed-version check
-    /// until the next build.
+    /// What this machine measured or was told is carried across rather
+    /// than reset, through [`CarriedOver`]. A declaration says nothing
+    /// about the package a build produces, the mode somebody picked here,
+    /// or the commit a build was made from -- so accepting one must not
+    /// throw any of them away. Dropping the package would blank the
+    /// installed-version check until the next build; dropping the commit
+    /// did worse, because a component with nothing recorded reads as one
+    /// this server has never built.
     pub fn approve_declaration(&self, key: &str) -> Result<()> {
         let entry = self
             .entry(key)
@@ -816,8 +819,7 @@ impl AppState {
                 .iter_mut()
                 .find(|project| project.key == key)
                 .with_context(|| format!("{key} is built in and has nothing to accept"))?;
-            let measured = measured_packages(&project.components);
-            let chosen = chosen_settings(&project.components);
+            let carried = carried_over(&project.components);
             project.git_pull = declared.git_pull;
             // Everything `matches_accepted` compares has to be written
             // here, or accepting cannot clear the gate. `resources` joined
@@ -828,17 +830,12 @@ impl AppState {
             // nothing. Add a field to the gate and add it here.
             project.resources = declared.resources;
             project.components = declared.components;
-            for component in &mut project.components {
-                if let Some(package) = measured.get(&component_id(component)) {
-                    component.set_package(package.clone());
-                }
-            }
             // Accepting is about what the project asks to have run. It is
             // not somebody withdrawing the mode they picked or the strip
-            // they turned off, so those survive it -- exactly as the
-            // measured package does, and for the same reason: neither is
-            // part of what was being agreed to.
-            restore_chosen_settings(&mut project.components, &chosen);
+            // they turned off, and it is not this machine forgetting what
+            // it built -- none of those is part of what was being agreed
+            // to.
+            restore_carried_over(&mut project.components, &carried);
             Ok(())
         })
     }
@@ -920,29 +917,28 @@ fn reconcile_self(config: &mut Config, self_project: &Path) -> bool {
             "app".to_string()
         }
     };
-    set_measured_package(&mut components, &apk_name, SELF_PACKAGE.to_string());
     let existing = config
         .projects
         .iter_mut()
         .find(|project| project.key == SELF_KEY);
     // Carried across the row being rebuilt, like `gitIpv4` below: what is
-    // *derived* here is what the checkout declares, and a mode chosen for
-    // this server's own component is not that. Restarting is how this
-    // server takes an update, so a choice that did not survive a restart
-    // would not survive being acted on.
+    // *derived* here is what the checkout declares, and neither a mode
+    // chosen for this server's own component nor the commit its last
+    // build was made from is that. It matters more here than at an
+    // acceptance, because this row is re-derived at *every* startup and
+    // restarting is how this server takes its own update -- so a choice
+    // that did not survive a restart would not survive being acted on,
+    // and a commit that did not left this server's own card with no
+    // freshness for ever after: never behind, never current, and its
+    // Update with nothing to do, since both readings need a commit to
+    // compare against.
     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));
+        restore_carried_over(&mut components, &carried_over(&existing.components));
     }
+    // After the carry-across, so what is derived wins: this is the one
+    // project whose package this server states rather than reads, and an
+    // older row carrying something else must not override it.
+    set_measured_package(&mut components, &apk_name, SELF_PACKAGE.to_string());
     let derived = ProjectConfig {
         key: SELF_KEY.to_string(),
         label: declared.label.unwrap_or_else(|| SELF_LABEL.to_string()),
@@ -1379,6 +1375,36 @@ mod tests {
         assert_eq!(component.build().to_line(), "d");
     }
 
+    /// The commit a build recorded is a measurement of this machine, not
+    /// something the project's declaration has an opinion about -- so it
+    /// survives that declaration being accepted again, exactly as the
+    /// chosen mode and the measured package do. It did not, and the cost
+    /// was invisible: with nothing recorded a component reads as one this
+    /// server has never built, which is a true sentence about plenty of
+    /// projects and so looks like an answer rather than a loss. Every
+    /// card of a project whose declaration had ever been accepted said it.
+    #[test]
+    fn what_a_build_recorded_survives_the_declaration_being_accepted_again() {
+        let dir = tempfile::tempdir().expect("tempdir");
+        write_moded_request(dir.path());
+        let (_home, state) = state_with(dir.path());
+        state.approve_declaration("demo").expect("approve");
+        state.record_built("demo", "app", "c0ffee".to_string());
+
+        state.approve_declaration("demo").expect("approve again");
+        let component = &state.entry("demo").expect("entry").components[0];
+        assert_eq!(
+            component.built_from(),
+            Some("c0ffee"),
+            "accepting must not tell the card this component was never built here",
+        );
+        assert_eq!(
+            component.built_mode(),
+            Some("release"),
+            "and the mode goes with the commit, or the record describes a build nobody made",
+        );
+    }
+
     /// A mode this component does not declare is refused rather than
     /// stored: `ByMode::get` falls back to the first, so storing it would
     /// leave the card naming a mode nothing is built in -- which reads