Build a component this server has never built

"Not built here" was the one state nothing could clear. With no recorded
commit a component was not stale, so Update built nothing, so no build
ever wrote a record -- the card said it, and Pull, Update and the
project's own button all did nothing about it, for ever. Worst for a
server component, which the output check beside this one skips by
design, having no APK to look for: that is why ai-app's backend sat
there saying it after an Update that otherwise worked.

Measured on a two-component fixture rather than argued. Before:
/prepare answers `components: []` and the freshness is unchanged
afterwards. After: the same call reports `step: "building"`, the card
reads `current`, and `builtFrom` is on disk.

So `nothing_built` asks the record as well as the output. The cost is
one rebuild per component on a machine whose records were lost, which
was the objection when this was written the other way round -- and is
the right price now that the state is visible on the card. It is
self-clearing: the first build writes the record and the question is
never asked again.

That leaves NotBuiltHere meaning exactly one thing, a component with no
build command, where nothing was ever going to measure anything and no
button would change it. Quiet and unflagged, as against NeverBuilt,
which is flagged and has a Build beside it. One word had been covering
both.

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

server/src/build_state.rs

@@ -67,9 +67,14 @@ pub enum Freshness {
     /// 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.
+    /// Cannot tell, and nothing here can change that: this component has
+    /// no build command, so nothing on this machine was ever going to
+    /// produce a commit to compare against. The ordinary state for a
+    /// project somebody builds by hand, and deliberately *not* actionable
+    /// -- there is no button on the row that would clear it, which is the
+    /// whole difference between it and `NeverBuilt`. A component that
+    /// does have a command and no record is `NeverBuilt` instead, because
+    /// building it is exactly what fixes it.
     NotBuiltHere,
     /// Cannot tell: no checkout under this project, or git would not say.
     NoCheckout,
@@ -627,17 +632,20 @@ impl BuildState {
         // one function, because the two answering it differently is what
         // produced that: the staleness rules knew there was no build and
         // the card did not.
-        if self.nothing_built(component) {
+        // One read of the record, handed to both questions below. Taken
+        // as a whole statement rather than inline, because a temporary
+        // guard in a larger expression stays locked for that expression
+        // -- which is how a helper that passed `inner.lock()...clone()`
+        // straight into `component_is_stale` deadlocked the moment
+        // anything under it wanted the same lock.
+        let built_from = self.inner.lock().unwrap().built_from.clone();
+        if self.nothing_built(component, &built_from) {
             return Freshness::NeverBuilt;
         }
-        let built = self
-            .inner
-            .lock()
-            .unwrap()
-            .built_from
-            .get(component.name())
-            .cloned();
-        let Some(built) = built else {
+        // Only reachable for a component with no build command:
+        // `nothing_built` above has already answered for every component
+        // that has one, and a missing record is `NeverBuilt` there.
+        let Some(built) = built_from.get(component.name()) else {
             return Freshness::NotBuiltHere;
         };
         match crate::git::subtree_dirty(&self.project_path, &component.watched_paths()) {
@@ -646,7 +654,7 @@ impl BuildState {
             None => return Freshness::NoCheckout,
         }
         match crate::git::subtree_head(&self.project_path, &component.watched_paths()) {
-            Some(current) if current == built => Freshness::Current,
+            Some(current) if &current == built => Freshness::Current,
             Some(_) if parked => Freshness::Parked,
             Some(_) => Freshness::Behind,
             None => Freshness::NoCheckout,
@@ -679,9 +687,32 @@ impl BuildState {
     /// this server builds: with no command there is nothing to press, so
     /// saying it out loud would be a nag about a project somebody builds
     /// by hand.
-    fn nothing_built(&self, component: &Component) -> bool {
-        !component.build().is_empty()
-            && matches!(component, Component::Apk { .. })
+    fn nothing_built(&self, component: &Component, built_from: &HashMap<String, String>) -> bool {
+        if component.build().is_empty() {
+            return false;
+        }
+        // No commit recorded means no build of this component has ever
+        // finished here, whatever is or is not sitting on disk. It is the
+        // only way to ask a `Server`, which has no output to look for --
+        // and it used to be asked of nothing at all, which made it the
+        // one state Update could not clear: with no record the component
+        // was not stale, so no build ran, so no record was written. The
+        // card said "not built here" and every button that should have
+        // fixed it did nothing, for ever. Measured on a two-component
+        // fixture: `/prepare` answered with an empty component list and
+        // the freshness was unchanged afterwards.
+        //
+        // Self-clearing, which is what makes it safe to act on: the first
+        // build writes the record and the question is never asked again.
+        // The cost is one rebuild per component on a machine whose
+        // records were lost, which is the price of every later reading
+        // being something this server measured rather than assumed.
+        if !built_from.contains_key(component.name()) {
+            return true;
+        }
+        // And for an APK, the output itself: a build recorded a commit and
+        // somebody deleted what it produced.
+        matches!(component, Component::Apk { .. })
             && crate::discover::find_apks(&component.dir(&self.project_path)).is_empty()
     }
 
@@ -709,7 +740,7 @@ impl BuildState {
         // never been built reports not-stale under it and would leave the
         // first build impossible to trigger -- which a project can be
         // added before having done.
-        if self.nothing_built(component) {
+        if self.nothing_built(component, built_from) {
             return true;
         }
         // Behind the checkout: this component was built from a commit
@@ -1991,7 +2022,11 @@ mod tests {
             .iter()
             .find(|component| component.name() == name)
             .expect("component");
-        state.component_is_stale(component, &state.inner.lock().unwrap().built_from.clone())
+        // Bound first: a `lock()` temporary inside the call's argument
+        // list stays held for the whole call, which deadlocks anything
+        // under it that wants the same lock.
+        let built_from = state.inner.lock().unwrap().built_from.clone();
+        state.component_is_stale(component, &built_from)
     }
 
     /// The trap a per-component staleness check walks straight into once
@@ -2129,8 +2164,10 @@ mod tests {
 
         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",
+            Freshness::NeverBuilt,
+            "this server has built nothing for it, which is a first build to make rather than a \
+             comparison to draw -- `NotBuiltHere` is now only for a component with no build \
+             command at all, where there is nothing to press",
         );
 
         state
@@ -2147,6 +2184,26 @@ mod tests {
             Freshness::Uncommitted,
             "somebody is editing in there, so the commit comparison says nothing",
         );
+
+        // The other kind of not knowing, and the one that stays quiet:
+        // nothing here builds this component, so there is no button that
+        // would ever change the answer and nothing to flag.
+        let by_hand = Component::Apk {
+            name: "by-hand".to_string(),
+            modes: Vec::new(),
+            build: crate::config::ByMode::default(),
+            cwd: Some(PathBuf::from("app")),
+            stale_when: None,
+            also_watch: Vec::new(),
+            strip: false,
+            enroll: crate::config::Command::default(),
+            strip_here: None,
+            mode: None,
+            package: None,
+            built_from: None,
+            built_mode: None,
+        };
+        assert_eq!(state.freshness(&by_hand, false), Freshness::NotBuiltHere);
     }
 
     /// Moving the checkout runs git and nothing else.
@@ -2427,11 +2484,25 @@ mod tests {
         );
     }
 
-    /// Nothing recorded is "we have never built this", not "this is out of
-    /// date" -- otherwise every project that existed before this feature
-    /// would demand a rebuild the moment it arrived.
+    /// Nothing recorded means this server has never built the component,
+    /// and that is something to build rather than something to sit on.
+    ///
+    /// It was the other way round, on the grounds that every project
+    /// predating the record would demand one rebuild when the feature
+    /// arrived. The cost of that reading only showed up once freshness
+    /// started saying which kind of unknown it was: with no record a
+    /// component is not stale, so Update builds nothing, so no record is
+    /// ever written -- a card reading "not built here" with every button
+    /// that should fix it doing nothing, for ever. A `Server` cannot even
+    /// be rescued by the output check beside this one, having no output
+    /// to look for.
+    ///
+    /// So the one-time rebuild is the price, and it buys the thing that
+    /// makes every later reading worth anything: a commit this server
+    /// measured rather than assumed. It is self-clearing -- the first
+    /// build writes the record and the question is never asked again.
     #[test]
-    fn a_component_this_server_has_never_built_is_not_called_stale() {
+    fn a_component_this_server_has_never_built_is_built_once() {
         let dir = tempfile::tempdir().expect("tempdir");
         let root = dir.path();
         let components = two_component_checkout(root);
@@ -2441,8 +2512,20 @@ mod tests {
             state.inner.lock().unwrap().built_from.is_empty(),
             "nothing recorded"
         );
-        assert!(!stale(&state, "backend"));
-        assert!(!stale(&state, "app"));
+        assert!(stale(&state, "backend"), "a server has no output to check");
+        assert!(stale(&state, "app"));
+
+        let head = crate::git::subtree_head(root, &[PathBuf::from("server")]).expect("a commit");
+        state
+            .inner
+            .lock()
+            .unwrap()
+            .built_from
+            .insert("backend".to_string(), head);
+        assert!(
+            !stale(&state, "backend"),
+            "and one build settles it, or the rebuild would repeat for ever",
+        );
     }
 
     /// The same must hold where there is no checkout to compare against: