diff --git a/AGENTS.md b/AGENTS.md index 2859793..2c90db0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -441,7 +441,7 @@ mutable at runtime from the phone. answer, and `claim` writes that entry synchronously under the same lock the route answers from -- so nothing can read a component the request just claimed as idle, which the phone would take for "the build is - over". `RunningBuild::building` stays, but it means "anything at all is + over". `BuildStatus::building` stays, but it means "anything at all is happening here" and is only for the controls that act on the whole checkout; anything about one component reads that component's `step`. The app mirrors the split exactly: `ProjectState` for the pull and the @@ -473,6 +473,14 @@ mutable at runtime from the phone. card that starts a selection on long-press fights the gestures it already has. Iris asked for exactly that line on 2026-09-01: "not the 'failed' but the command output for build errors and stuff". + `OutputText` also renders the ANSI escapes rather than printing them, + through the same `ansiAnnotated` the log dialog uses -- a compiler marks + its own errors in colour and the tail of a failed build is that output + verbatim, so raw it arrived as `[1;31merror` with punctuation welded + onto the one line somebody was trying to read. Selection copies + `AnnotatedString.text`, which is the message with every escape already + gone, so what lands on the clipboard is what was on screen rather than + what was on the wire. - **A finished component shows nothing, and its button goes back to normal.** Iris's call, 2026-09-01: "you shouldn't see the time it took once it finishes, it should just go back to its normal enabled button diff --git a/app/androidApp/src/main/kotlin/com/example/devupdater/Theme.kt b/app/androidApp/src/main/kotlin/com/example/devupdater/Theme.kt index 183a917..ce4244a 100644 --- a/app/androidApp/src/main/kotlin/com/example/devupdater/Theme.kt +++ b/app/androidApp/src/main/kotlin/com/example/devupdater/Theme.kt @@ -10,6 +10,7 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.material3.darkColorScheme import androidx.compose.runtime.Composable +import androidx.compose.runtime.remember import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import androidx.compose.ui.text.TextStyle @@ -206,7 +207,23 @@ fun OutputText( color: Color = MaterialTheme.colorScheme.error, style: TextStyle = LocalTextStyle.current, ) { - SelectionContainer { Text(text, style = style, color = color) } + // The same rendering the log dialog gets, and for the same reason: a + // compiler marks its own errors in colour, and the tail of a failed + // build is that output verbatim. Printed raw it read as `[1;31merror` + // -- which is not merely ugly, it is the one line of the message a + // person is trying to read with punctuation welded onto it. The + // sequences with no meaning on a phone are consumed rather than + // printed (see `ansiAnnotated`), so a cursor movement cannot arrive + // looking like corruption either. + // + // [color] stays the colour of everything the escapes did not claim, + // which is what keeps a message with no escapes in it -- git's stderr, + // this app's own fallbacks -- looking exactly as it did. + val rendered = remember(text, color) { ansiAnnotated(text, color) } + // Selection copies `AnnotatedString.text`, which is the message with + // every escape already gone -- so what lands on the clipboard is what + // was on screen rather than what was on the wire. + SelectionContainer { Text(rendered, style = style) } } /** diff --git a/server/src/build_state.rs b/server/src/build_state.rs index 6a0b4df..7e59c6a 100644 --- a/server/src/build_state.rs +++ b/server/src/build_state.rs @@ -209,32 +209,20 @@ pub struct BuildState { inner: Mutex, } +/// What `/status` answers: whether anything needs building, and what is +/// being done about it. +/// +/// One struct rather than a nested one. It was split so a card on +/// `/manifest` could carry the run half without paying for `stale`, which +/// walks every component's directory -- but nothing on the phone ever read +/// that field, so the split was an indirection with one user and the +/// second half of it has gone. #[derive(Serialize)] #[serde(rename_all = "camelCase")] pub struct BuildStatus { + /// The expensive one: answering it walks every component's directory, + /// which is why this is on `/status` and not on the manifest path. pub stale: bool, - /// Flattened, so `/status` answers the one flat object it always has - /// while a card can carry [`RunningBuild`] on its own. They are split - /// because `stale` is the expensive half: it walks every component's - /// directory, and `describe` is on the manifest path. - #[serde(flatten)] - pub run: RunningBuild, -} - -/// What a build is doing, with nothing in it that has to be measured off -/// disk to answer. -/// -/// This is the whole of what the phone needs to pick a build back up. It -/// is reported on the card as well as from `/status`, because the app's -/// record of a run lives only in the composition: leaving the app tears -/// down the polling loop and the card state with it, and without this the -/// list it comes back to cannot say that a build is still going. The -/// server never lost anything -- the run owns its own `Arc` -/// and outlives every request -- so the fix is for the manifest to say so -/// rather than for anything here to be re-attached to. -#[derive(Serialize, Clone)] -#[serde(rename_all = "camelCase")] -pub struct RunningBuild { /// Anything at all is happening for this project -- a pull, or any /// component being built. Deliberately the *project's* question, for /// the controls that act on the whole checkout; a caller asking about @@ -1204,32 +1192,12 @@ impl BuildState { .any(|run| run.name == component && run.error.is_some()) } - /// What this project is doing, or `None` when nothing is. - /// - /// Deliberately not [`Self::status`], which also answers `stale` and - /// so walks every component's directory. This is reached from - /// `describe`, which builds every card on `/manifest` -- fetched on - /// every open, resume and Refresh -- so it is a lock and some clones - /// and nothing else. - pub fn running(&self) -> Option { - let inner = self.inner.lock().unwrap(); - inner.anything_running().then(|| Self::snapshot(&inner)) - } - pub fn status(&self) -> BuildStatus { // Before the lock, not inside it -- see `is_stale`. let stale = self.is_stale(None); let inner = self.inner.lock().unwrap(); BuildStatus { stale, - run: Self::snapshot(&inner), - } - } - - /// The one place the run is read out of the lock, so `/status` and a - /// card cannot come to describe the same build differently. - fn snapshot(inner: &Inner) -> RunningBuild { - RunningBuild { building: inner.anything_running(), error: inner.error.clone(), unrelated_histories: inner.unrelated_histories, @@ -1441,7 +1409,7 @@ mod tests { move || crate::config::project_config(&project).matches_accepted(&accepted, None), Arc::new(|_: &str, _: String| {}), ); - while state.status().run.building { + while state.status().building { tokio::time::sleep(std::time::Duration::from_millis(10)).await; } @@ -1490,7 +1458,7 @@ mod tests { state.build_now(Some("app"), Arc::new(|_: &str, _: String| {})); let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); - while state.status().run.building { + while state.status().building { assert!( std::time::Instant::now() < deadline, "build did not finish in time" @@ -1510,7 +1478,6 @@ mod tests { let status = state.status(); assert_eq!( status - .run .components .iter() .map(|c| c.name.as_str()) @@ -1571,7 +1538,7 @@ mod tests { std::time::Instant::now() < deadline, "quick never finished, so it was waiting on slow", ); - let status = state.status().run; + let status = state.status(); let quick = status.components.iter().find(|c| c.name == "quick"); if quick.is_some_and(|quick| quick.step.is_none()) { assert!( @@ -1591,7 +1558,7 @@ mod tests { "slow is still going, which is the point", ); - while state.status().run.building { + while state.status().building { assert!( std::time::Instant::now() < deadline, "slow did not finish in time" @@ -1634,7 +1601,7 @@ mod tests { state.build_now(None, Arc::new(|_: &str, _: String| {})); let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); - while state.status().run.building { + while state.status().building { assert!( std::time::Instant::now() < deadline, "build did not finish in time" @@ -1642,7 +1609,7 @@ mod tests { tokio::time::sleep(std::time::Duration::from_millis(10)).await; } - let status = state.status().run; + let status = state.status(); assert!( status.error.is_none(), "the project's error slot is for pulls, not for a component's build", @@ -1723,7 +1690,7 @@ mod tests { state.trigger_if_needed(Some("b"), Arc::new(|_: &str, _: String| {})); let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); - while state.status().run.building { + while state.status().building { assert!( std::time::Instant::now() < deadline, "build did not finish in time" diff --git a/server/src/routes.rs b/server/src/routes.rs index bebf039..38f993b 100644 --- a/server/src/routes.rs +++ b/server/src/routes.rs @@ -493,23 +493,6 @@ struct ManifestApp { /// one flag for the list, so a card says whether *it* is the one still /// being worked out. check_pending: bool, - /// The build running for this project right now, if one is, in the - /// same shape `/status` answers. - /// - /// Here so that a build survives leaving the app. The run itself never - /// stops -- it owns its own `Arc` and outlives the request - /// that started it -- but the phone's record of it lives only in the - /// composition, so backgrounding tears down the polling loop and the - /// card state together. Without this the list it comes back to cannot - /// say a build is still going, and the card reads as one that was - /// killed: it offers Update again, and pressing it does nothing, - /// because there is already a run in this project's one build slot. - /// - /// Read with [`crate::build_state::BuildState::running`] rather than - /// `status`, which also answers `stale` and walks every component's - /// directory to do it -- this is on the manifest path. - #[serde(skip_serializing_if = "Option::is_none")] - build: Option, /// What this project produces, in the order it is built. One entry is /// the ordinary case and the card shows it inline; more than one is /// what the phone draws as a nested list. @@ -673,7 +656,6 @@ async fn describe(state: &Arc, entry: &AppEntry) -> Result