Say a build is signed with the wrong key before the installer does

Three things.

A download whose signing certificate does not match the installed
copy is stopped with a dialog naming both digests and offering the
one thing that gets past it: removing the old app. Android's own
answer is "App not installed" with no cause, which reads as the
download having failed. Where either certificate cannot be read the
answer is "don't know" and the install goes ahead as before. The
download is carried on the component's state so that the removal is
followed by the install it was for rather than by a second download.

Pressing that revealed that ACTION_DELETE now needs
REQUEST_DELETE_PACKAGES, and fails invisibly without it -- so the
"Remove the old app" offer for a renamed package had presumably
never worked either.

The build mode and the installed variant are one dropdown, not two.
They answer to the same words, so two pickers offering debug and
release read as one choice asked twice. Where a component declares
modes the mode is the whole answer, and the server serves the build
named after it rather than the newest. Every dropdown now hangs from
one outlined pill with a chevron.

And a checkout parked on a chosen commit is no longer called out of
date, with HEAD in the commit picker as the way back to following
the branch. The commit list comes from that branch rather than from
HEAD, so parking no longer hides the commits after it -- the same
one-way door the tracked-only dirty check closed, in a place that
check did not reach.

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

server/src/build_state.rs

@@ -47,8 +47,8 @@ pub enum Freshness {
     Current,
     /// Its directory has moved past the commit it was built from.
     Behind,
-    /// Never built here, no checkout to compare against, or uncommitted
-    /// work in its directory.
+    /// Never built here, no checkout to compare against, uncommitted work
+    /// in its directory, or a checkout parked on a commit somebody chose.
     Unknown,
 }
 
@@ -555,7 +555,25 @@ impl BuildState {
     /// notice the new output rather than to produce one. Rebuilding
     /// uncommitted work and sending it to a phone would also be a
     /// surprising thing to do with work its author has not committed.
-    pub fn freshness(&self, component: &Component) -> Freshness {
+    ///
+    /// `parked` says the checkout is sitting on a commit somebody chose
+    /// rather than following a branch, which is the one way a phone can
+    /// leave it: picking a commit in the settings sheet detaches HEAD.
+    /// Saying "out of date" about a checkout somebody deliberately moved
+    /// backwards is nagging about a decision already made -- and it is
+    /// nothing to act on either, because every way this reads as behind
+    /// while parked is a build that failed, a declaration waiting to be
+    /// accepted, or a component with no build step, each of which is
+    /// already said on the same card beside the button for it. So while
+    /// parked a differing commit is reported as unknown: withheld, rather
+    /// than claimed current, since nothing here measured the output to be
+    /// what somebody wanted.
+    ///
+    /// Passed in rather than read here. The caller has already asked git
+    /// for this project's status, and asking again would be one more
+    /// process per component on a path fetched on every open, resume and
+    /// Refresh.
+    pub fn freshness(&self, component: &Component, parked: bool) -> Freshness {
         // Behind for a reason no commit can express: what is on disk was
         // built some other way than this component is set to build now.
         // Reported before the checkout is consulted at all, because it is
@@ -581,6 +599,7 @@ impl BuildState {
         }
         match crate::git::subtree_head(&self.project_path, &component.watched_paths()) {
             Some(current) if current == built => Freshness::Current,
+            Some(_) if parked => Freshness::Unknown,
             Some(_) => Freshness::Behind,
             None => Freshness::Unknown,
         }
@@ -1933,10 +1952,62 @@ mod tests {
             "a debug build must not satisfy a component set to build release",
         );
         assert_eq!(
-            switched.freshness(&switched.components[0]),
+            switched.freshness(&switched.components[0], false),
             Freshness::Behind,
             "and the card has to say so rather than reading as current",
         );
+        assert_eq!(
+            switched.freshness(&switched.components[0], true),
+            Freshness::Behind,
+            "a parked checkout says nothing about which mode was built, so this one still \
+             reports -- what the parking silences is the commit comparison alone",
+        );
+    }
+
+    /// Picking a commit is a decision, and a card that answers it with
+    /// "out of date" is nagging about one already made. The comparison
+    /// itself is unchanged -- what changes is that a *parked* checkout
+    /// withholds it rather than reporting the difference as being behind.
+    ///
+    /// Checked against the ordinary case in the same test, because that is
+    /// the half a change like this can break without looking broken: a
+    /// checkout following its branch must still say when its build is
+    /// older than what is checked out.
+    #[test]
+    fn a_checkout_parked_on_a_chosen_commit_is_not_reported_as_out_of_date() {
+        let dir = tempfile::tempdir().expect("tempdir");
+        let root = dir.path();
+        let components = two_component_checkout(root);
+        let first = crate::git::subtree_head(root, &[PathBuf::from("app")]).expect("a commit");
+
+        std::fs::write(root.join("app/main.kt"), "two").expect("write");
+        run(root, &["git", "commit", "-qam", "two"]);
+
+        let state = state_for(root, components);
+        // Built at the first commit, and the checkout has since moved on:
+        // the ordinary way to be behind, and the one that must survive.
+        state
+            .inner
+            .lock()
+            .unwrap()
+            .built_from
+            .insert("app".to_string(), first);
+        let app = state
+            .components
+            .iter()
+            .find(|component| component.name() == "app")
+            .expect("the app component");
+
+        assert_eq!(
+            state.freshness(app, false),
+            Freshness::Behind,
+            "a checkout following its branch still reports a build older than it",
+        );
+        assert_eq!(
+            state.freshness(app, true),
+            Freshness::Unknown,
+            "and one parked on a chosen commit withholds it rather than nagging",
+        );
     }
 
     /// The shape of bug a unit test that only calls `component_is_stale`

server/src/git.rs

@@ -354,7 +354,7 @@ pub fn branches(project: &Path) -> Vec<Branch> {
     let head = status(project).map(|status| status.branch);
     // Detached HEAD reports the literal `HEAD`, which is not a branch
     // name, so nothing matches it and every branch reads as not current.
-    let current = head.filter(|branch| branch != "HEAD");
+    let current = head.filter(|branch| branch != DETACHED);
 
     let mut branches: Vec<Branch> = Vec::new();
     // `for-each-ref refs/heads` rather than `git branch`, which adds a
@@ -416,7 +416,68 @@ pub fn branches(project: &Path) -> Vec<Branch> {
     branches
 }
 
-/// The most recent `limit` commits reachable from HEAD, newest first.
+/// The branch this checkout is following, or the one it would return to.
+///
+/// `Some(name)` for an ordinary checkout. A checkout parked on a commit is
+/// on no branch at all, and the way back to the tip is the branch that
+/// commit sits on -- asked of git rather than remembered here, because
+/// nothing this server writes down survives somebody moving the checkout
+/// from the build machine's own terminal.
+///
+/// Only when exactly one local branch contains it: with several there is
+/// no such thing as *the* branch, and picking one would be this server
+/// guessing which history somebody meant. The branch picker is what
+/// answers that, and it lists all of them.
+///
+/// `refs/heads` through `for-each-ref` for the same reason [`branches`]
+/// uses it: `git branch --contains` adds the `(HEAD detached at abc123)`
+/// pseudo-entry, which is not a branch and cannot be checked out.
+pub fn head_branch(project: &Path) -> Option<String> {
+    let branch = git(project, &["rev-parse", "--abbrev-ref", "HEAD"]).ok()?;
+    if branch != DETACHED {
+        return Some(branch);
+    }
+    let listed = git(
+        project,
+        &[
+            "for-each-ref",
+            "--format=%(refname:short)",
+            "--contains",
+            "HEAD",
+            "refs/heads",
+        ],
+    )
+    .ok()?;
+    let mut names = listed
+        .lines()
+        .map(str::trim)
+        .filter(|name| !name.is_empty());
+    let only = names.next()?;
+    names.next().is_none().then(|| only.to_string())
+}
+
+/// What git calls the branch when no branch is checked out.
+///
+/// A real branch may not be named this (git refuses it), so comparing
+/// against the string is safe -- but it means "on no branch" wherever it
+/// appears, and spelling it out is what stops that being read as a name.
+pub const DETACHED: &str = "HEAD";
+
+/// The most recent `limit` commits on the branch this checkout is
+/// following, newest first, plus where HEAD actually is.
+///
+/// Listed from the *branch* rather than from HEAD, which are the same
+/// thing until somebody parks the checkout on an older commit. Listed
+/// from HEAD then, the commits after it disappear from the list -- so the
+/// picker that moved the checkout back could only ever move it further
+/// back, and the way forward was a terminal on the build machine. Which
+/// is the one-way door this whole sheet exists to avoid.
+///
+/// A checkout parked further back than `limit` is not in the window at
+/// all, so its own commit is appended: it is older than everything above
+/// it, which is where it belongs by date, and without it the picker has
+/// nothing to show for where the checkout *is* -- which reads as not
+/// knowing rather than as being a long way back.
 ///
 /// Local history only. What the remote has and this checkout has not
 /// fetched is not offered: Pull is what brings those in, and the card
@@ -424,17 +485,28 @@ pub fn branches(project: &Path) -> Vec<Branch> {
 /// round trip, and opening the sheet cannot fail the way a remote check
 /// can.
 pub fn recent_commits(project: &Path, limit: usize) -> Vec<CommitSummary> {
-    // A record separator that cannot occur in a subject, so a message
-    // containing anything at all still parses. `%x1f` is the ASCII unit
-    // separator; git writes it literally.
-    let format = "--format=%H%x1f%h%x1f%at%x1f%s";
     let head = git(project, &["rev-parse", "HEAD"])
         .ok()
         .map(|sha| sha.trim().to_string());
-    let Ok(out) = git(
-        project,
-        &["log", &format!("-n{limit}"), format, "--no-color"],
-    ) else {
+    let from = head_branch(project).unwrap_or_else(|| DETACHED.to_string());
+    let mut commits = log(project, &[&format!("-n{limit}"), &from], head.as_deref());
+    if !commits.iter().any(|commit| commit.current) {
+        commits.extend(log(project, &["-n1", DETACHED], head.as_deref()));
+    }
+    commits
+}
+
+/// One `git log` read, parsed. Shared by the two calls
+/// [`recent_commits`] makes so that the format string and the parsing of
+/// it are written once -- they only work as a pair, and a second copy of
+/// either is a second thing to keep in step.
+fn log(project: &Path, args: &[&str], head: Option<&str>) -> Vec<CommitSummary> {
+    // A record separator that cannot occur in a subject, so a message
+    // containing anything at all still parses. `%x1f` is the ASCII unit
+    // separator; git writes it literally.
+    let mut command = vec!["log", "--format=%H%x1f%h%x1f%at%x1f%s", "--no-color"];
+    command.extend_from_slice(args);
+    let Ok(out) = git(project, &command) else {
         return Vec::new();
     };
     out.lines()
@@ -447,7 +519,7 @@ pub fn recent_commits(project: &Path, limit: usize) -> Vec<CommitSummary> {
             // is allowed to be missing rather than dropping the commit.
             let subject = parts.next().unwrap_or_default().to_string();
             Some(CommitSummary {
-                current: head.as_deref() == Some(sha.as_str()),
+                current: head == Some(sha.as_str()),
                 sha,
                 short,
                 at,
@@ -924,6 +996,76 @@ mod tests {
         );
     }
 
+    /// The other half of the one-way door, and the one a dirty-tree fix
+    /// does nothing about: listed from HEAD, a checkout parked on an old
+    /// commit loses every commit after it from the picker, so the control
+    /// that moved it back cannot move it forward. The list comes from the
+    /// branch instead, and `head_branch` is what the "latest" entry moves
+    /// to.
+    #[test]
+    fn a_parked_checkout_can_still_see_the_way_forward() {
+        let dir = tempfile::tempdir().expect("tempdir");
+        let (_origin, clone) = origin_and_clone(dir.path());
+        for subject in ["two", "three"] {
+            std::fs::write(clone.join("file"), subject).expect("write");
+            run(&clone, &["git", "commit", "-qam", subject]);
+        }
+        assert_eq!(head_branch(&clone).as_deref(), Some("main"));
+
+        let first = recent_commits(&clone, 10)
+            .last()
+            .expect("the first commit")
+            .sha
+            .clone();
+        checkout(&clone, &first).expect("moving back");
+
+        assert_eq!(
+            head_branch(&clone).as_deref(),
+            Some("main"),
+            "the branch containing the parked commit is the way back to the tip",
+        );
+        let parked = recent_commits(&clone, 10);
+        let subjects: Vec<&str> = parked.iter().map(|c| c.subject.as_str()).collect();
+        assert_eq!(
+            subjects,
+            vec!["three", "two", "one"],
+            "the commits after the parked one are still offered",
+        );
+        assert!(
+            parked.iter().filter(|c| c.current).count() == 1
+                && parked.last().expect("the parked commit").current,
+            "and where the checkout actually is, is marked exactly once",
+        );
+
+        // Parked further back than the window: the commit the checkout is
+        // on is not in it, and appending it is what keeps the picker from
+        // having nothing to show for where it is.
+        let short = recent_commits(&clone, 1);
+        assert_eq!(short.len(), 2, "{short:?}");
+        assert_eq!(short[0].subject, "three");
+        assert!(short[1].current && short[1].subject == "one", "{short:?}");
+    }
+
+    /// With two branches over the parked commit there is no such thing as
+    /// *the* branch to return to, and choosing one would be this server
+    /// deciding which history somebody meant.
+    #[test]
+    fn no_way_back_is_offered_when_several_branches_could_be_meant() {
+        let dir = tempfile::tempdir().expect("tempdir");
+        let (_origin, clone) = origin_and_clone(dir.path());
+        std::fs::write(clone.join("file"), "two").expect("write");
+        run(&clone, &["git", "commit", "-qam", "two"]);
+        run(&clone, &["git", "branch", "other"]);
+
+        let first = recent_commits(&clone, 10)
+            .last()
+            .expect("the first commit")
+            .sha
+            .clone();
+        checkout(&clone, &first).expect("moving back");
+        assert_eq!(head_branch(&clone), None);
+    }
+
     /// An origin repo with one commit, and a clone of it.
     fn origin_and_clone(root: &Path) -> (PathBuf, PathBuf) {
         let origin = root.join("origin");

server/src/registry.rs

@@ -124,7 +124,17 @@ impl AppEntry {
     }
 
     /// The APK to serve: `requested` if it is still one of this project's
-    /// builds, else the newest one found under it.
+    /// builds, else the build of the mode this component is set to, else
+    /// the newest one found under it.
+    ///
+    /// The mode is consulted before the mtime because it is an answer
+    /// somebody gave: a component set to build `release` has said which of
+    /// its outputs is the one that counts, and "whichever was written most
+    /// recently" can disagree with that -- a debug build made by hand
+    /// afterwards is newer, and serving it would put a debug app on the
+    /// phone from a card saying release. Only ever a build that is already
+    /// there; a mode nothing has been built in yet falls through to the
+    /// newest, exactly as before.
     ///
     /// Which variant a phone wants is that phone's preference, so it
     /// arrives with the request rather than being stored here -- two
@@ -150,6 +160,11 @@ impl AppEntry {
         {
             return Some(found.clone());
         }
+        if let Some(mode) = component.effective_mode()
+            && let Some(found) = variants.iter().find(|candidate| candidate.variant == mode)
+        {
+            return Some(found.clone());
+        }
         variants.into_iter().next()
     }
 

server/src/routes.rs

@@ -404,6 +404,11 @@ impl ManifestComponent {
         // a declaration is waiting to be accepted, which is the project's
         // answer rather than this component's.
         may_build: bool,
+        // Whether the checkout is parked on a commit somebody picked,
+        // which is the project's state rather than this component's --
+        // read once for the card and handed down, because asking git per
+        // component would be a process each on the manifest path.
+        parked: bool,
     ) -> Result<Self, ApiError> {
         let name = component.name().to_string();
         let is_server = matches!(component, crate::config::Component::Server { .. });
@@ -446,7 +451,7 @@ impl ManifestComponent {
             freshness: entry
                 .build
                 .as_ref()
-                .map(|build| build.freshness(component))
+                .map(|build| build.freshness(component, parked))
                 .unwrap_or(crate::build_state::Freshness::Unknown),
             data_path: state_paths
                 .as_ref()
@@ -718,14 +723,28 @@ async fn describe(state: &Arc<AppState>, entry: &AppEntry) -> Result<ManifestApp
     // declaration is exactly the command that must not run.
     let declaration = entry.declaration_state();
     let pending = declaration.pending;
+    // Read before the components rather than after: each of them reports
+    // its freshness against this checkout, and a checkout parked on a
+    // chosen commit is not one to report as out of date.
+    let git = crate::git::status(&entry.project_path);
+    let parked = git
+        .as_ref()
+        .is_some_and(|git| git.branch == crate::git::DETACHED);
     let mut components = Vec::with_capacity(entry.components.len());
     for component in &entry.components {
         components.push(
-            ManifestComponent::read(state, &entry.key, entry, component, pending.is_none()).await?,
+            ManifestComponent::read(
+                state,
+                &entry.key,
+                entry,
+                component,
+                pending.is_none(),
+                parked,
+            )
+            .await?,
         );
     }
 
-    let git = crate::git::status(&entry.project_path);
     // An upstream is part of it: a branch that tracks nothing has nothing
     // to be pulled from, so `pull` refuses and asking a remote about it is
     // asking about a remote it doesn't have. Without this the card said so
@@ -1556,6 +1575,16 @@ async fn build_pull(
 struct RefsResponse {
     branches: Vec<crate::git::Branch>,
     commits: Vec<crate::git::CommitSummary>,
+    /// The branch the commit picker's "latest" entry moves to, so that
+    /// picking a commit is something a phone can undo.
+    ///
+    /// Absent when there is no single branch to return to -- a checkout
+    /// parked on a commit that several branches contain, or none. The
+    /// picker says so rather than choosing one; which history somebody
+    /// meant is not something this server can work out, and the branch
+    /// list above is the honest answer to it.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    head: Option<String>,
 }
 
 /// How much history the commit picker offers.
@@ -1581,6 +1610,7 @@ async fn checkout_refs(
     let refs = tokio::task::spawn_blocking(move || RefsResponse {
         branches: crate::git::branches(&project),
         commits: crate::git::recent_commits(&project, COMMIT_HISTORY),
+        head: crate::git::head_branch(&project),
     })
     .await
     .context("reading the checkout's refs panicked")?;