From 48759fb5fccde45405c2db29d162ac55edbc6562 Mon Sep 17 00:00:00 2001 From: Afloat <132809511+Afloat16@users.noreply.github.com> Date: Tue, 3 Nov 2026 07:31:22 -0500 Subject: [PATCH] Stop unchanged rebuilds after two turns across runs (#557) Refs #44, #110 and #481. A build stops after two builder turns in a row change no app files, counted across runs in a versioned inbox/build-progress.json; a new request or a real edit starts another attempt. File digests are cached by size and modification time, so unchanged files aren't read again. --- crates/ai/src/rebuild/mod.rs | 123 ++- crates/ai/src/rebuild/note_progress_tests.rs | 260 +++++++ crates/ai/src/rebuild/tests.rs | 408 +++++++++- crates/engine/src/workspace.rs | 2 + crates/engine/src/workspace/progress.rs | 747 +++++++++++++++++++ docs/ROADMAP.md | 4 +- docs/STATUS.md | 42 ++ docs/file-formats.md | 7 +- docs/pitfalls.md | 5 + docs/schemas/build-progress.schema.json | 30 + xtask/src/main.rs | 1 + 11 files changed, 1614 insertions(+), 15 deletions(-) create mode 100644 crates/ai/src/rebuild/note_progress_tests.rs create mode 100644 crates/engine/src/workspace/progress.rs create mode 100644 docs/schemas/build-progress.schema.json diff --git a/crates/ai/src/rebuild/mod.rs b/crates/ai/src/rebuild/mod.rs index 45cc1947..955e884d 100644 --- a/crates/ai/src/rebuild/mod.rs +++ b/crates/ai/src/rebuild/mod.rs @@ -21,6 +21,7 @@ use crate::cli_agents::{Builder, CliSpec}; use crate::{AgentEvent, AiHub, BackendKind, Conversation, Role}; use craft_anything_engine::inbox::{Answer, Inbox, InboxDir, Request}; use craft_anything_engine::project::{Project, Purpose}; +use craft_anything_engine::workspace::progress::{AppFingerprintCache, BuildProgress}; use craft_anything_engine::workspace::{CheckResult, ProjectUsage, Status, TargetRef, Workspace}; use craft_anything_engine::{CallOptions, Caller, CancelToken, Engine, ToolOutput}; use craft_anything_engines::game; @@ -249,6 +250,7 @@ impl RebuildRun { decompiled: false, usage: Some(UsageClock { root: root.clone(), since: Instant::now() }), self_check: None, + reset_progress_after_notes: false, }; let r2 = running.clone(); std::thread::Builder::new() @@ -429,6 +431,8 @@ struct Worker { usage: Option, /// The kind and bar the builder's own checks (`rebuild_check`) are judged by, once known. self_check: Option<(Arc, f64)>, + /// A hook consumed a new request; persist its reset only after the builder finishes. + reset_progress_after_notes: bool, } /// The project a run records its usage in, and when it last did. @@ -499,7 +503,7 @@ impl Worker { /// The prompt for notes the person added while the agent worked, if there are any. fn added(&self, ws: &mut Workspace) -> Option { - let notes = std::mem::take(&mut *self.notes.lock().unwrap_or_else(PoisonError::into_inner)); + let mut notes = std::mem::take(&mut *self.notes.lock().unwrap_or_else(PoisonError::into_inner)); if notes.is_empty() { return None; } @@ -508,6 +512,10 @@ impl Worker { for n in ¬es { notes::forget(&dir, n.id); } + notes.retain(|n| !n.text.trim().is_empty()); + if notes.is_empty() { + return None; + } for n in ¬es { self.emit(RunEvent::Note { shown: n.shown.clone(), pictures: n.pictures.clone() }); } @@ -590,6 +598,15 @@ impl Worker { extractor: Option<(String, crate::agent::ApiConversation)>, opts: &RunOptions, ) -> Result { + let mut progress = BuildProgress::load(&ws.root)?; + let mut progress_available = true; + let mut fingerprints = AppFingerprintCache::default(); + // Try again has no message. Only an explicit new request starts another attempt on + // unchanged files; restarting the app or resuming a paused run keeps the saved count. + if opts.message.as_deref().is_some_and(|m| !m.trim().is_empty()) { + progress.reset(); + progress.save(&ws.root)?; + } // Fail before any work when the agent's tools are missing or it can't sign in. let remote = entry.config.ssh_host.as_deref().is_some_and(|h| !h.trim().is_empty()); if !remote && api.is_none() { @@ -686,6 +703,14 @@ impl Worker { let mut only_check = carrying_on && !first && opts.message.is_none() && ws.manifest.awaiting_check; loop { if !std::mem::take(&mut only_check) { + progress = BuildProgress::load(&ws.root)?; + if let Some(fingerprint) = self.progress_fingerprint(ws, &mut progress, &mut progress_available, &mut fingerprints)? { + progress.before_turn(fingerprint); + if progress.stalled() { + return Err(no_build_progress(ws)); + } + progress.save(&ws.root)?; + } let doing = match (round, carrying_on) { (0, false) => "Building the first version…", (0, true) => "Carrying on with the build…", @@ -698,7 +723,24 @@ impl Worker { if let Some(a) = &keeper { self.keep_session(ws, a.as_ref()); } - let reply = reply?; + let reply = match reply { + Ok(reply) => reply, + Err(error) => { + // Consumed instructions start a fresh attempt even if this turn fails + // or is cancelled. The provider retry loop has ended; stop any remaining + // work before touching the record, and retain the original failure. + agent.stop(); + drop(agent); + if self.reset_progress_after_notes + && let Err(reset_error) = self.progress_after_notes(ws) + { + self.emit(RunEvent::Agent(AgentEvent::Status { + text: format!("The new request couldn't reset the unchanged-turn count ({}).", reset_error.message), + })); + } + return Err(error); + } + }; if !reply.trim().is_empty() { ws.manifest.last_reply = reply.trim().to_string(); } @@ -712,8 +754,19 @@ impl Worker { } ws.manifest.awaiting_check = true; ws.save()?; + // Keep the finished turn before scanning: a cancellation during the scan + // must not make Try again repeat work that only needs its checks. + // A consumed note starts another attempt, but its record I/O must not + // interrupt a live builder or enter the provider retry path. + progress = self.progress_after_notes(ws)?; + if let Some(fingerprint) = self.progress_fingerprint(ws, &mut progress, &mut progress_available, &mut fingerprints)? { + progress.after_turn(fingerprint); + progress.save(&ws.root)?; + } // What the person added while the agent worked goes in before anything else. if let Some(p) = self.added(ws) { + progress.reset(); + progress.save(&ws.root)?; prompt = p; continue; } @@ -732,11 +785,24 @@ impl Worker { for c in &checks { self.emit(RunEvent::Checked(c.clone())); } - let not_good = checks.iter().find(|c| !c.good(opts.good_enough)); if let Some(p) = self.added(ws) { + progress.reset(); + progress.save(&ws.root)?; prompt = p; continue; } + let not_good = checks.iter().find(|c| !c.good(opts.good_enough)); + if not_good.is_some() && progress_available && progress.stalled() { + // A check-only resume skipped the builder's before-turn scan. A real edit + // while the project was closed (or during this check) still starts an attempt. + if let Some(fingerprint) = self.progress_fingerprint(ws, &mut progress, &mut progress_available, &mut fingerprints)? { + progress.before_turn(fingerprint); + progress.save(&ws.root)?; + if progress.stalled() { + return Err(no_build_progress(ws)); + } + } + } let Some(check) = not_good else { break }; if refinements >= opts.max_rounds { break; @@ -773,6 +839,47 @@ impl Worker { Ok(summary) } + /// Read progress after the turn has ended. A consumed request resets the allowance on + /// success, failure or cancellation; a failed save never acknowledges the pending reset. + fn progress_after_notes(&mut self, ws: &Workspace) -> Result { + let mut progress = BuildProgress::load(&ws.root)?; + if self.reset_progress_after_notes { + progress.reset(); + progress.save(&ws.root)?; + self.reset_progress_after_notes = false; + } + Ok(progress) + } + + /// File scanning is an optional guard, not a prerequisite for building or checking. Once + /// it is unavailable, skip it for this run and start a new comparison on the next run. + fn progress_fingerprint( + &self, + ws: &Workspace, + progress: &mut BuildProgress, + available: &mut bool, + fingerprints: &mut AppFingerprintCache, + ) -> Result, ToolError> { + self.stop.check()?; + if !*available { + return Ok(None); + } + match fingerprints.scan(ws, &self.stop) { + Ok(fingerprint) => Ok(Some(fingerprint)), + Err(e) if e.code == ErrorCode::Cancelled => Err(e), + Err(e) => { + *available = false; + progress.invalidate(); + // Record errors remain errors: never overwrite a broken or newer record. + progress.save(&ws.root)?; + self.emit(RunEvent::Agent(AgentEvent::Status { + text: format!("Build progress couldn't be checked ({}). Continuing without the unchanged-turn limit for this run.", e.message), + })); + Ok(None) + } + } + } + /// One builder turn. Its session is kept even when the turn fails, so nothing is lost; a turn /// cut short (a time limit, a crash, a dropped connection) is picked up where it stopped, up to /// three times. Problems only the person can fix (sign-in, credit, a missing app) end the run. @@ -966,6 +1073,7 @@ impl Worker { self.emit(RunEvent::Agent(e)); } if done { + self.relay_read_notes(ws); self.stop.check()?; // An agent-level error (sign-in, crash, limits) ends the run: checking a // build that never happened would only mislead. @@ -987,7 +1095,7 @@ impl Worker { /// Notes the builder took at a step of this turn (its hook claimed them): they join the feed /// now and aren't sent again after the turn. - fn relay_read_notes(&self, ws: &Workspace) { + fn relay_read_notes(&mut self, ws: &Workspace) { let dir = notes::notes_dir(&ws.inbox_dir()); let read: Vec = { let mut notes = self.notes.lock().unwrap_or_else(PoisonError::into_inner); @@ -995,10 +1103,12 @@ impl Worker { *notes = waiting; read }; + let new_request = read.iter().any(|n| !n.text.trim().is_empty()); for n in read { notes::forget(&dir, n.id); self.emit(RunEvent::Note { shown: n.shown, pictures: n.pictures }); } + self.reset_progress_after_notes |= new_request; } fn relay_inbox(&mut self) { @@ -1010,6 +1120,11 @@ impl Worker { } } +fn no_build_progress(ws: &Workspace) -> ToolError { + ToolError::new(ErrorCode::OverLimit, format!("The build stopped after two builder turns changed no files in {}.", ws.app_place())) + .with_remediation("Send a new request with a concrete correction, or edit the app's files before trying again.") +} + /// Largest builder session the project keeps a copy of. const MAX_KEPT_SESSION: u64 = 64_000_000; /// How much of a kept session a new one is handed when the old one isn't on this computer. diff --git a/crates/ai/src/rebuild/note_progress_tests.rs b/crates/ai/src/rebuild/note_progress_tests.rs new file mode 100644 index 00000000..b1197eff --- /dev/null +++ b/crates/ai/src/rebuild/note_progress_tests.rs @@ -0,0 +1,260 @@ +use super::*; +use craft_anything_engine::workspace::progress::BUILD_PROGRESS_FILE; + +fn consumed_notes(notes: &Mutex>, dir: &Path) { + for (id, text) in [(71, "Keep the header blue"), (72, "Use larger text")] { + notes.lock().unwrap().push(Note { id, text: text.into(), shown: text.into(), pictures: Vec::new() }); + notes::hand_over(dir, id, text).unwrap(); + } + assert_eq!(notes::claim(dir).len(), 2); +} + +struct NotesDuringPoll { + notes: Arc>>, + notes_dir: PathBuf, + progress: PathBuf, + directory: bool, + polls: usize, + sends: usize, +} + +impl Conversation for NotesDuringPoll { + fn send(&mut self, _: &str) { + self.sends += 1; + } + + fn poll(&mut self) -> Vec { + self.polls += 1; + if self.polls == 1 { + consumed_notes(&self.notes, &self.notes_dir); + if self.directory { + std::fs::create_dir(&self.progress).unwrap(); + } else { + std::fs::write(&self.progress, "{ broken progress").unwrap(); + } + return vec![AgentEvent::TextDelta { text: "Finished the edit".into() }]; + } + vec![AgentEvent::TurnDone { backend: "scripted".into(), input_tokens: 0, output_tokens: 0, cost_usd: None, cost_is_estimate: false }] + } + + fn answer_consent(&mut self, _: &str, _: bool) {} + fn stop(&mut self) {} + fn is_busy(&self) -> bool { + self.polls < 2 + } +} + +#[test] +fn consumed_notes_wait_for_turn_done_before_accessing_progress() { + for directory in [false, true] { + let dir = tempfile::tempdir().unwrap(); + let mut ws = Workspace::create(dir.path(), "Note polling", "Build an app", TargetRef::None).unwrap(); + let mut worker = check_worker(&ws, false); + let (tx, rx) = std::sync::mpsc::channel(); + worker.tx = tx; + let mut agent = NotesDuringPoll { + notes: worker.notes.clone(), + notes_dir: notes::notes_dir(&ws.inbox_dir()), + progress: ws.root.join(BUILD_PROGRESS_FILE), + directory, + polls: 0, + sends: 0, + }; + let result = worker.turn_with_wait(&mut ws, &mut agent, "Apply the notes", None, |_, _| panic!("a local record cannot retry the builder")); + assert_eq!(result.unwrap(), "Finished the edit"); + assert_eq!((agent.sends, agent.polls), (1, 2), "keep polling the existing request until TurnDone"); + assert!(!agent.is_busy()); + let events: Vec<_> = rx.try_iter().collect(); + assert_eq!(events.iter().filter(|event| matches!(event, RunEvent::Note { .. })).count(), 2); + assert_eq!(events.iter().filter(|event| matches!(event, RunEvent::Agent(AgentEvent::TurnDone { .. }))).count(), 1); + assert!(worker.notes.lock().unwrap().is_empty()); + assert!(BuildProgress::load(&ws.root).is_err(), "polling must not overwrite the broken progress record"); + } +} + +#[test] +fn consumed_note_progress_errors_preserve_the_completed_turn() { + use crate::agent::tests::{Scripted, turn}; + use crate::agent::{ApiConversation, EngineHost, LoopConfig, ModelSlot}; + use crate::model::{Block, ChatModel, Delta, Stop, StopReason, TurnRequest, TurnResult}; + + struct NotesInTurn { + script: Scripted, + notes: Arc>>, + notes_dir: PathBuf, + progress: PathBuf, + directory: bool, + } + + impl ChatModel for NotesInTurn { + fn turn(&mut self, request: &TurnRequest<'_>, delta: &mut dyn FnMut(Delta), stop: &Stop) -> Result { + consumed_notes(&self.notes, &self.notes_dir); + if self.directory { + std::fs::remove_file(&self.progress).unwrap(); + std::fs::create_dir(&self.progress).unwrap(); + } else { + std::fs::write(&self.progress, "{ broken progress").unwrap(); + } + self.script.turn(request, delta, stop) + } + + fn model(&self) -> String { + self.script.model() + } + } + + for directory in [false, true] { + let dir = tempfile::tempdir().unwrap(); + let mut ws = Workspace::create(dir.path(), "Note persistence", "Build an app", TargetRef::None).unwrap(); + std::fs::create_dir_all(ws.capture_dir()).unwrap(); + ws.manifest.stack = "Rust (console)".into(); + ws.save().unwrap(); + let mut worker = check_worker(&ws, false); + let seen = Arc::default(); + let model = NotesInTurn { + script: Scripted { + turns: [turn(vec![Block::Text { text: "Finished the requested edit".into() }], StopReason::EndTurn)].into(), + seen: Arc::clone(&seen), + }, + notes: worker.notes.clone(), + notes_dir: notes::notes_dir(&ws.inbox_dir()), + progress: ws.root.join(BUILD_PROGRESS_FILE), + directory, + }; + let slot = ModelSlot { name: "Scripted".into(), model: Box::new(model), local: true, effort: None, max_turns: 10, context_tokens: 0 }; + let api = ApiConversation::start(Arc::new(EngineHost(worker.engine.clone())), vec![slot], LoopConfig::default()).unwrap(); + let entry = crate::BackendEntry { + config: crate::BackendConfig { + name: "Scripted".into(), + kind: BackendKind::OpenaiCompatible, + model: "scripted".into(), + base_url: None, + ssh_host: None, + enabled: true, + }, + options: Default::default(), + }; + let opts = RunOptions { message: Some("Apply the requested edits".into()), ..RunOptions::default() }; + let error = worker.run(&mut ws, entry, Some(api), None, &opts).unwrap_err(); + let saved = Workspace::open(&ws.root).unwrap(); + assert!(saved.manifest.awaiting_check, "the finished turn must be recorded before a progress error: {error:?}"); + assert_eq!(saved.manifest.last_reply, "Finished the requested edit"); + assert_eq!(seen.lock().unwrap().len(), 1, "progress persistence must not send another model request"); + assert!(error.message.contains("build-progress.json"), "retain the actual record error: {error:?}"); + assert!(BuildProgress::load(&ws.root).is_err(), "do not discard or repair an unreadable record implicitly"); + } +} + +#[test] +fn consumed_note_resets_survive_failed_and_cancelled_turns() { + use crate::agent::{ApiConversation, EngineHost, LoopConfig, ModelSlot}; + use crate::model::{ChatModel, Delta, Stop, TurnRequest, TurnResult}; + use std::sync::atomic::AtomicUsize; + + struct InterruptedNote { + notes: Arc>>, + notes_dir: PathBuf, + progress: PathBuf, + text: &'static str, + record: &'static str, + cancelled: bool, + stop: CancelToken, + calls: Arc, + } + + impl ChatModel for InterruptedNote { + fn turn(&mut self, _: &TurnRequest<'_>, _: &mut dyn FnMut(Delta), _: &Stop) -> Result { + self.calls.fetch_add(1, Ordering::SeqCst); + self.notes.lock().unwrap().push(Note { id: 73, text: self.text.into(), shown: self.text.into(), pictures: Vec::new() }); + notes::hand_over(&self.notes_dir, 73, self.text).unwrap(); + assert_eq!(notes::claim(&self.notes_dir), [self.text]); + if !self.record.is_empty() { + std::fs::write(&self.progress, self.record).unwrap(); + } + if self.cancelled { + self.stop.cancel(); + Err(ToolError::cancelled()) + } else { + Err(ToolError::new(ErrorCode::NoPermission, "fixture sign-in required").with_remediation("Sign in to the fixture provider.")) + } + } + + fn model(&self) -> String { + "scripted".into() + } + } + + for cancelled in [false, true] { + for text in ["Keep the header blue", " "] { + for record in ["", "{ broken progress", r#"{"version":2,"fingerprint":null,"unchanged_turns":1}"#] { + let dir = tempfile::tempdir().unwrap(); + let mut ws = Workspace::create(dir.path(), "Interrupted note", "Build an app", TargetRef::None).unwrap(); + std::fs::create_dir_all(ws.capture_dir()).unwrap(); + std::fs::create_dir_all(ws.app_dir()).unwrap(); + std::fs::write(ws.app_dir().join("app.txt"), "previous app files").unwrap(); + ws.manifest.stack = "Rust (console)".into(); + ws.manifest.last_reply = "Previous completed turn".into(); + ws.save().unwrap(); + let mut progress = BuildProgress::default(); + progress.before_turn(ws.app_fingerprint(&CancelToken::default()).unwrap()); + progress.unchanged_turns = 1; + progress.save(&ws.root).unwrap(); + let mut worker = check_worker(&ws, false); + let (tx, rx) = std::sync::mpsc::channel(); + worker.tx = tx; + let calls = Arc::new(AtomicUsize::new(0)); + let model = InterruptedNote { + notes: worker.notes.clone(), + notes_dir: notes::notes_dir(&ws.inbox_dir()), + progress: ws.root.join(BUILD_PROGRESS_FILE), + text, + record, + cancelled, + stop: worker.stop.clone(), + calls: calls.clone(), + }; + let slot = ModelSlot { name: "Scripted".into(), model: Box::new(model), local: true, effort: None, max_turns: 10, context_tokens: 0 }; + let api = ApiConversation::start(Arc::new(EngineHost(worker.engine.clone())), vec![slot], LoopConfig::default()).unwrap(); + let entry = crate::BackendEntry { + config: crate::BackendConfig { + name: "Scripted".into(), + kind: BackendKind::OpenaiCompatible, + model: "scripted".into(), + base_url: None, + ssh_host: None, + enabled: true, + }, + options: Default::default(), + }; + let opts = RunOptions { continuing: true, ..RunOptions::default() }; + let error = worker.run(&mut ws, entry, Some(api), None, &opts).unwrap_err(); + assert_eq!(calls.load(Ordering::SeqCst), 1, "an interrupted request must not be sent again"); + if cancelled { + assert_eq!(error.code, ErrorCode::Cancelled, "retain cancellation even when the progress record is unreadable"); + } else { + assert_eq!(error.code, ErrorCode::ProgramFailed); + assert_eq!(error.message, "Scripted: fixture sign-in required"); + assert_eq!(error.remediation.as_deref(), Some("Sign in to the fixture provider.")); + } + let saved = Workspace::open(&ws.root).unwrap(); + assert!(!saved.manifest.awaiting_check, "an interrupted turn is not a completed build"); + assert_eq!(saved.manifest.last_reply, "Previous completed turn"); + let events: Vec<_> = rx.try_iter().collect(); + assert_eq!(events.iter().filter(|event| matches!(event, RunEvent::Note { .. })).count(), 1); + assert!(worker.notes.lock().unwrap().is_empty()); + assert!(!notes::was_read(¬es::notes_dir(&ws.inbox_dir()), 73)); + if record.is_empty() { + let recovered = BuildProgress::load(&saved.root).unwrap(); + assert_eq!(recovered.fingerprint, progress.fingerprint, "the interrupted turn does not change the comparison baseline"); + assert_eq!(recovered.unchanged_turns, u32::from(text.trim().is_empty()), "a consumed new request survives reopening"); + } else { + assert_eq!(std::fs::read_to_string(ws.root.join(BUILD_PROGRESS_FILE)).unwrap(), record, "never replace a broken or newer record"); + let reported = events + .iter() + .any(|event| matches!(event, RunEvent::Agent(AgentEvent::Status { text }) if text.contains("couldn't reset the unchanged-turn count"))); + assert_eq!(reported, !text.trim().is_empty(), "report a failed reset separately from the original failure"); + } + } + } + } +} diff --git a/crates/ai/src/rebuild/tests.rs b/crates/ai/src/rebuild/tests.rs index f22b23bf..3622e069 100644 --- a/crates/ai/src/rebuild/tests.rs +++ b/crates/ai/src/rebuild/tests.rs @@ -1,6 +1,9 @@ use super::check::build_check; use super::*; +#[path = "note_progress_tests.rs"] +mod note_progress; + // Helpers that take a built-in kind's id, so each test reads as one line. fn kind(id: &str) -> Arc { KindRegistry::built_in().get(id).unwrap() @@ -294,6 +297,7 @@ fn notes_added_while_it_works_reach_the_agent_at_its_next_step() { decompiled: false, usage: None, self_check: None, + reset_progress_after_notes: false, }; assert!(w.added(&mut ws).is_none(), "nothing added, nothing to say"); let note = |id, text: &str| Note { id, text: format!("{text} (pictures: a.png)"), shown: text.into(), pictures: Vec::new() }; @@ -995,6 +999,7 @@ fn an_api_backend_builds_with_the_build_tools() { decompiled: false, usage: None, self_check: None, + reset_progress_after_notes: false, }; w.attach(&ws).unwrap(); // The model writes a file with the Build tools, then answers. @@ -1111,6 +1116,7 @@ fn the_mechanics_sheet_is_required_and_held_against_the_code() { decompiled: false, usage: None, self_check: None, + reset_progress_after_notes: false, }; let check = |round| w.run_checks(&ws, &SheetKind, round).unwrap().remove(0); @@ -1195,6 +1201,10 @@ mod picking_up_a_build { "flaky" } fn run(&self, cx: &mut CheckCx) -> Result { + let rejection = std::fs::read_to_string(cx.ws.root.join("reject-build")).unwrap_or_default(); + if !rejection.is_empty() && (rejection != "once" || cx.ws.manifest.checks.is_empty()) { + return Ok(CheckResult::new(self.id(), cx.round, false, "The app is still missing.")); + } let file = cx.ws.app_dir().join("check-fails"); let left: u32 = std::fs::read_to_string(&file).ok().and_then(|s| s.trim().parse().ok()).unwrap_or(0); if left > 0 { @@ -1260,11 +1270,13 @@ mod picking_up_a_build { } fn try_it(b: &Bench, ws: &Workspace) -> (bool, Vec) { - let mut kinds = KindRegistry::built_in(); - kinds.register(Arc::new(FlakyKind)); - let run = - RebuildRun::start(&b.hub, Workspace::open(&ws.root).unwrap(), RunOptions { kinds, backend: Some("Stand-in".into()), ..RunOptions::default() }) - .unwrap(); + try_with(b, ws, RunOptions::default()) + } + + fn try_with(b: &Bench, ws: &Workspace, mut opts: RunOptions) -> (bool, Vec) { + opts.kinds.register(Arc::new(FlakyKind)); + opts.backend = Some("Stand-in".into()); + let run = RebuildRun::start(&b.hub, Workspace::open(&ws.root).unwrap(), opts).unwrap(); let began = Instant::now(); let mut ok = None; while ok.is_none() { @@ -1365,6 +1377,383 @@ mod picking_up_a_build { assert!(!told.contains("Their request"), "not the first-build brief again: {told}"); } + #[test] + fn unchanged_builder_turns_stop_after_two_and_stay_stopped_when_reopened() { + let b = bench(); + let ws = worked_on(&b); + std::fs::write(ws.root.join("reject-build"), "not ready").unwrap(); + let (ok, feed) = try_it(&b, &ws); + assert!(!ok, "{feed:?}"); + assert_eq!(prompts_given(&b), 2, "a third unchanged turn would spend more without progress"); + assert!( + feed.iter().any(|i| matches!(i, feed::FeedItem::Error { message, .. } if message.contains("two builder turns"))), + "the stop explains what happened: {feed:?}" + ); + let reopened = Workspace::open(&ws.root).unwrap(); + let (ok, _) = try_it(&b, &reopened); + assert!(!ok); + assert_eq!(prompts_given(&b), 2, "Try again must keep the saved progress limit"); + } + + #[test] + fn single_turn_runs_share_the_same_unchanged_turn_limit() { + let b = bench(); + let ws = worked_on(&b); + std::fs::write(ws.root.join("reject-build"), "not ready").unwrap(); + for expected in [1, 2, 2] { + let (ok, _) = try_with(&b, &ws, RunOptions { max_rounds: 0, ..RunOptions::default() }); + assert!(!ok); + assert_eq!(prompts_given(&b), expected); + } + } + + #[test] + fn a_real_edit_or_nonempty_new_request_starts_another_attempt() { + let b = bench(); + let ws = worked_on(&b); + std::fs::write(ws.root.join("reject-build"), "not ready").unwrap(); + assert!(!try_it(&b, &ws).0); + assert_eq!(prompts_given(&b), 2); + std::fs::write(ws.app_dir().join("index.html"), "a new starting point").unwrap(); + assert!(!try_it(&b, &ws).0); + assert_eq!(prompts_given(&b), 4, "a real edit reset the previous limit"); + let opts = RunOptions { message: Some("Use the new index.html and add a working button.".into()), ..RunOptions::default() }; + assert!(!try_with(&b, &ws, opts).0); + assert_eq!(prompts_given(&b), 6, "a new request starts another attempt"); + let empty = RunOptions { message: Some(" ".into()), ..RunOptions::default() }; + assert!(!try_with(&b, &ws, empty).0); + assert_eq!(prompts_given(&b), 6, "an empty message cannot erase the saved count"); + } + + #[test] + fn a_successful_check_can_finish_the_second_unchanged_turn() { + let b = bench(); + let ws = worked_on(&b); + std::fs::write(ws.root.join("reject-build"), "once").unwrap(); + assert!(try_it(&b, &ws).0); + assert_eq!(prompts_given(&b), 2); + assert_eq!(BuildProgress::load(&ws.root).unwrap().unchanged_turns, 2); + } + + #[test] + fn a_saved_check_only_run_is_not_blocked_by_the_progress_limit() { + let b = bench(); + let mut ws = worked_on(&b); + ws.manifest.awaiting_check = true; + ws.save().unwrap(); + let mut progress = BuildProgress::load(&ws.root).unwrap(); + progress.before_turn(ws.app_fingerprint(&CancelToken::default()).unwrap()); + progress.unchanged_turns = 2; + progress.save(&ws.root).unwrap(); + assert!(try_it(&b, &ws).0); + assert_eq!(prompts_given(&b), 0); + } + + #[test] + fn a_failed_check_only_resume_detects_manual_edits_before_refusing_more_build_turns() { + for edited in [false, true] { + let b = bench(); + let mut ws = worked_on(&b); + std::fs::write(ws.root.join("reject-build"), "not ready").unwrap(); + ws.manifest.awaiting_check = true; + ws.save().unwrap(); + let mut progress = BuildProgress::load(&ws.root).unwrap(); + progress.before_turn(ws.app_fingerprint(&CancelToken::default()).unwrap()); + progress.unchanged_turns = 2; + progress.save(&ws.root).unwrap(); + if edited { + std::fs::write(ws.app_dir().join("index.html"), "a manual correction").unwrap(); + } + assert!(!try_it(&b, &ws).0); + assert_eq!(prompts_given(&b), if edited { 2 } else { 0 }, "edited={edited}"); + assert_eq!(BuildProgress::load(&ws.root).unwrap().unchanged_turns, 2); + } + } + + #[test] + fn scan_io_errors_skip_only_the_guard_and_cancellation_is_never_skipped() { + for fail_scan_first in [false, true] { + let b = bench(); + let ws = worked_on(&b); + let worker = check_worker(&ws, false); + let mut progress = BuildProgress::default(); + progress.before_turn(ws.app_fingerprint(&CancelToken::default()).unwrap()); + progress.unchanged_turns = 2; + progress.save(&ws.root).unwrap(); + let mut available = true; + let mut fingerprints = AppFingerprintCache::default(); + if fail_scan_first { + // A deterministic read_dir I/O failure, including when tests run as root. + std::fs::remove_dir(ws.app_dir()).unwrap(); + std::fs::write(ws.app_dir(), "the app directory is temporarily unavailable").unwrap(); + assert!(worker.progress_fingerprint(&ws, &mut progress, &mut available, &mut fingerprints).unwrap().is_none()); + assert!(!available); + assert_eq!(progress, BuildProgress::default()); + assert_eq!(BuildProgress::load(&ws.root).unwrap(), progress); + std::fs::remove_file(ws.app_dir()).unwrap(); + std::fs::create_dir(ws.app_dir()).unwrap(); + assert!( + worker.progress_fingerprint(&ws, &mut progress, &mut available, &mut fingerprints).unwrap().is_none(), + "a recovered directory does not resume comparisons within the same run" + ); + } + let before = progress.clone(); + worker.stop.cancel(); + assert_eq!(worker.progress_fingerprint(&ws, &mut progress, &mut available, &mut fingerprints).unwrap_err().code, ErrorCode::Cancelled); + assert_eq!(progress, before, "cancellation does not erase a known comparison"); + assert_eq!(BuildProgress::load(&ws.root).unwrap(), before); + } + } + + #[test] + fn a_skipped_scan_does_not_hide_the_builders_own_failure() { + let b = bench(); + let ws = worked_on(&b); + let nested = (0..65).fold(ws.app_dir().join("nested"), |p, _| p.join("d")); + std::fs::create_dir_all(nested).unwrap(); + // Authentication is a typed provider failure. A plain exit 1 is transient, regardless + // of the English words it prints, and is covered by the retry-policy tests. + let mut config = b.hub.config(); + let mut entry = config.get("Stand-in").unwrap().clone(); + entry.config.model = "opencode".into(); + config.upsert(entry); + b.hub.engine.session().settings.ai = config.to_table().unwrap(); + let agent = b._dir.path().join("agent.sh"); + let script = std::fs::read_to_string(&agent).unwrap(); + let script = script.replacen("#!/bin/sh\n", "#!/bin/sh\nif [ \"$2\" = '--help' ]; then printf '%s\\n' '--format'; exit 0; fi\n", 1); + let error = json!({"type":"error","error":{"name":"APIError","data":{"message":"fixture sign in needed","statusCode":401,"isRetryable":false}}}); + std::fs::write(&agent, script.replace("echo Done.", &format!("printf '%s\\n' '{error}'\nexit 1"))).unwrap(); + let (ok, feed) = try_it(&b, &ws); + assert!(!ok); + assert_eq!(prompts_given(&b), 1, "the unavailable scan does not block or repeat the builder"); + assert!(feed.iter().any(|i| matches!(i, feed::FeedItem::Error { message, .. } if message.contains("fixture sign in needed"))), "{feed:?}"); + let saved = Workspace::open(&ws.root).unwrap(); + assert!(!saved.manifest.awaiting_check, "a failed turn has not finished its build"); + assert!(saved.manifest.checks.is_empty(), "the failed builder is not described as a successful check"); + } + + #[test] + fn scan_limits_do_not_block_builds_or_keep_a_stale_stall_count() { + for only_check in [false, true] { + let b = bench(); + let mut ws = worked_on(&b); + let mut progress = BuildProgress::default(); + progress.before_turn(ws.app_fingerprint(&CancelToken::default()).unwrap()); + progress.unchanged_turns = 2; + progress.save(&ws.root).unwrap(); + ws.manifest.awaiting_check = only_check; + ws.save().unwrap(); + let nested = (0..65).fold(ws.app_dir().join("nested"), |p, _| p.join("d")); + std::fs::create_dir_all(&nested).unwrap(); + assert_eq!(ws.app_fingerprint(&CancelToken::default()).unwrap_err().code, ErrorCode::OverLimit); + std::fs::write(ws.root.join("reject-build"), "once").unwrap(); + let (ok, feed) = try_it(&b, &ws); + assert!(ok, "a scan limit must not fail the build (only_check={only_check}): {feed:?}"); + assert_eq!(prompts_given(&b), if only_check { 1 } else { 2 }); + assert_eq!(BuildProgress::load(&ws.root).unwrap(), BuildProgress::default(), "an unknown comparison breaks the saved streak"); + let notices = feed + .iter() + .filter_map(|i| match i { + feed::FeedItem::Work { notes, .. } => Some(notes.iter().filter(|n| n.contains("without the unchanged-turn limit")).count()), + _ => None, + }) + .sum::(); + assert_eq!(notices, 1, "one notice explains that this run skipped the unavailable guard"); + assert_eq!(Workspace::open(&ws.root).unwrap().manifest.status, Status::Ready); + std::fs::remove_dir_all(ws.app_dir().join("nested")).unwrap(); + assert!(try_it(&b, &ws).0); + assert_eq!(BuildProgress::load(&ws.root).unwrap().unchanged_turns, 1, "the next readable run starts a new comparison"); + } + } + + #[test] + fn a_post_turn_scan_limit_keeps_the_finished_turn_for_check_only_recovery() { + let b = bench(); + let ws = worked_on(&b); + std::fs::write(ws.app_dir().join("check-fails"), "2").unwrap(); + let nested = (0..65).fold(ws.app_dir().join("nested"), |p, _| p.join("d")); + let agent = b._dir.path().join("agent.sh"); + let script = std::fs::read_to_string(&agent).unwrap(); + std::fs::write(&agent, format!("{script}\nmkdir -p '{}'\n", nested.display())).unwrap(); + let (ok, feed) = try_it(&b, &ws); + assert!(!ok, "the original check failure still stops the run"); + let saved = Workspace::open(&ws.root).unwrap(); + assert!(saved.manifest.awaiting_check, "the successful builder turn is saved before an unavailable scan"); + assert_eq!(saved.manifest.last_reply, "Done."); + assert_eq!(prompts_given(&b), 1); + assert_eq!(BuildProgress::load(&ws.root).unwrap(), BuildProgress::default()); + assert!( + feed.iter().any(|i| matches!(i, feed::FeedItem::Error { message, .. } if message.contains("The build is finished, but it couldn't be checked"))), + "{feed:?}" + ); + assert!(try_it(&b, &saved).0, "Try again can finish the checks while the scan is still unavailable"); + assert_eq!(prompts_given(&b), 1, "the completed builder turn is not repeated"); + assert!(!Workspace::open(&ws.root).unwrap().manifest.awaiting_check); + } + + #[test] + fn an_api_builder_has_the_same_two_unchanged_turn_limit() { + use crate::agent::tests::{Scripted, turn}; + use crate::agent::{ApiConversation, EngineHost, LoopConfig, ModelSlot}; + use crate::model::{Block, StopReason}; + let b = bench(); + let mut ws = worked_on(&b); + std::fs::write(ws.root.join("reject-build"), "not ready").unwrap(); + let seen = Arc::default(); + let script = Scripted { + turns: (0..5).map(|_| turn(vec![Block::Text { text: "No files changed.".into() }], StopReason::EndTurn)).collect(), + seen: Arc::clone(&seen), + }; + let mut worker = check_worker(&ws, false); + let slot = ModelSlot { name: "Scripted".into(), model: Box::new(script), local: true, effort: None, max_turns: 10, context_tokens: 0 }; + let api = ApiConversation::start(Arc::new(EngineHost(worker.engine.clone())), vec![slot], LoopConfig::default()).unwrap(); + let entry = crate::BackendEntry { + config: crate::BackendConfig { + name: "Scripted".into(), + kind: BackendKind::OpenaiCompatible, + model: "scripted".into(), + base_url: None, + ssh_host: None, + enabled: true, + }, + options: Default::default(), + }; + let mut opts = RunOptions::default(); + opts.kinds.register(Arc::new(FlakyKind)); + let error = worker.run(&mut ws, entry, Some(api), None, &opts).unwrap_err(); + assert!(error.message.contains("two builder turns"), "{error:?}"); + assert_eq!(seen.lock().unwrap().len(), 2); + } + + #[test] + fn new_notes_during_a_turn_or_check_get_their_own_attempt_but_empty_notes_do_not() { + use crate::agent::tests::{Scripted, turn}; + use crate::agent::{ApiConversation, EngineHost, LoopConfig, ModelSlot}; + use crate::model::{Block, ChatModel, Delta, Stop, StopReason, TurnRequest, TurnResult}; + type Notes = Arc>>; + fn note(text: &str) -> Note { + Note { id: 42, text: text.into(), shown: text.into(), pictures: Vec::new() } + } + struct NoteModel { + script: Scripted, + notes: Notes, + add: bool, + claim: bool, + dir: std::path::PathBuf, + text: &'static str, + } + impl ChatModel for NoteModel { + fn turn(&mut self, req: &TurnRequest<'_>, delta: &mut dyn FnMut(Delta), stop: &Stop) -> Result { + if self.add && self.script.seen.lock().unwrap().len() == 1 { + self.notes.lock().unwrap().push(note(self.text)); + if self.claim { + notes::hand_over(&self.dir, 42, self.text).unwrap(); + assert_eq!(notes::claim(&self.dir), [self.text]); + } + } + self.script.turn(req, delta, stop) + } + fn model(&self) -> String { + self.script.model() + } + } + struct NoteCheck { + notes: Notes, + add: bool, + text: &'static str, + } + impl Check for NoteCheck { + fn id(&self) -> &'static str { + "note-check" + } + fn run(&self, cx: &mut CheckCx) -> Result { + if self.add && cx.round == 2 { + self.notes.lock().unwrap().push(note(self.text)); + } + Ok(CheckResult::new(self.id(), cx.round, false, "The app is still missing.")) + } + } + struct NoteKind { + notes: Notes, + add: bool, + text: &'static str, + } + impl TargetKind for NoteKind { + fn id(&self) -> &'static str { + "note-kind" + } + fn words(&self) -> KindWords { + FlakyKind.words() + } + fn detect(&self, _: &DetectCx) -> Option { + Some(1001) + } + fn default_stack(&self, _: &DetectCx) -> &'static str { + "Anything" + } + fn goal(&self) -> &'static str { + "a thing" + } + fn checks(&self, _: &Workspace) -> Vec> { + vec![Box::new(NoteCheck { notes: self.notes.clone(), add: self.add, text: self.text })] + } + } + for mode in ["queued-turn", "queued-check", "hook"] { + for text in ["Use the built-in renderer", " "] { + let b = bench(); + let mut ws = worked_on(&b); + let mut worker = check_worker(&ws, false); + let seen = Arc::default(); + let script = Scripted { + turns: (0..5).map(|_| turn(vec![Block::Text { text: "No files changed.".into() }], StopReason::EndTurn)).collect(), + seen: Arc::clone(&seen), + }; + let model = NoteModel { + script, + notes: worker.notes.clone(), + add: mode != "queued-check", + claim: mode == "hook", + dir: notes::notes_dir(&ws.inbox_dir()), + text, + }; + let slot = ModelSlot { name: "Scripted".into(), model: Box::new(model), local: true, effort: None, max_turns: 10, context_tokens: 0 }; + let api = ApiConversation::start(Arc::new(EngineHost(worker.engine.clone())), vec![slot], LoopConfig::default()).unwrap(); + let entry = crate::BackendEntry { + config: crate::BackendConfig { + name: "Scripted".into(), + kind: BackendKind::OpenaiCompatible, + model: "scripted".into(), + base_url: None, + ssh_host: None, + enabled: true, + }, + options: Default::default(), + }; + let mut opts = RunOptions::default(); + opts.kinds.register(Arc::new(NoteKind { notes: worker.notes.clone(), add: mode == "queued-check", text })); + let error = worker.run(&mut ws, entry, Some(api), None, &opts).unwrap_err(); + assert!(error.message.contains("two builder turns"), "{error:?}"); + let requests = seen.lock().unwrap(); + let expected = if text.trim().is_empty() { + 2 + } else if mode == "hook" { + 3 + } else { + 4 + }; + assert_eq!(requests.len(), expected, "mode={mode}, text={text:?}"); + if !text.trim().is_empty() && mode != "hook" { + assert!(requests[2].last().unwrap().text().contains(text), "the queued instruction reaches the builder"); + } + assert!(worker.notes.lock().unwrap().is_empty(), "each instruction is consumed once"); + assert!(!notes::was_read(¬es::notes_dir(&ws.inbox_dir()), 42)); + assert_eq!(BuildProgress::load(&ws.root).unwrap().unchanged_turns, 2); + assert!(!try_it(&b, &ws).0); + assert_eq!(prompts_given(&b), 0, "plain Try again still cannot reset the saved count"); + } + } + } + /// #469: after the builder changed, the session the old one saved isn't handed to the new /// one, which starts from the brief; the same agent still carries on. #[test] @@ -1449,6 +1838,7 @@ fn check_worker(ws: &Workspace, deep: bool) -> Worker { decompiled: false, usage: None, self_check: None, + reset_progress_after_notes: false, } } @@ -1835,6 +2225,7 @@ fn preparing_the_builders_agent_does_not_lock_up() { decompiled: false, usage: None, self_check: None, + reset_progress_after_notes: false, }; let entry = crate::BackendEntry { config: crate::BackendConfig { @@ -1876,6 +2267,7 @@ fn the_run_answers_the_builders_own_check_during_its_turn() { decompiled: false, usage: None, self_check: Some((Arc::new(SheetKind), 90.0)), + reset_progress_after_notes: false, }; let mut builder = ChecksItsWork { root: ws.root.clone(), asked: None, reply: None }; let reply = w.turn(&mut ws, &mut builder, "Build it.", None).unwrap(); @@ -2051,7 +2443,7 @@ fn a_note_the_builder_took_at_a_step_joins_the_feed_and_isnt_sent_again() { let d = tempfile::tempdir().unwrap(); let mut ws = Workspace::create(d.path(), "Site", "Clone it", TargetRef::None).unwrap(); let (tx, rx) = std::sync::mpsc::channel(); - let w = Worker { + let mut w = Worker { wake: Default::default(), engine: Arc::new(Engine::new(craft_anything_engine::testing::session())), tx, @@ -2069,6 +2461,7 @@ fn a_note_the_builder_took_at_a_step_joins_the_feed_and_isnt_sent_again() { decompiled: false, usage: None, self_check: None, + reset_progress_after_notes: false, }; let dir = notes::notes_dir(&ws.inbox_dir()); notes::hand_over(&dir, 1, "make the header blue").unwrap(); @@ -2216,6 +2609,7 @@ fn a_session_left_on_another_computer_starts_a_new_one_from_the_project() { decompiled: false, usage: None, self_check: None, + reset_progress_after_notes: false, }; let first = resumed_elsewhere("THE BRIEF", &ws.manifest.last_reply, None); let reply = w.turn(&mut ws, &mut agent, "The person asks: make it blue", Some(first)).unwrap(); @@ -2270,6 +2664,7 @@ fn a_missing_session_with_nothing_to_start_from_is_said_plainly() { decompiled: false, usage: None, self_check: None, + reset_progress_after_notes: false, }; let e = w.turn_with_wait(&mut ws, &mut agent, "Carry on.", None, |_, _| panic!("a missing session without a fresh start must not be retried")).unwrap_err(); assert!(e.message.contains("isn't on this computer"), "{e:?}"); @@ -2333,6 +2728,7 @@ fn the_builders_conversation_travels_with_the_project() { decompiled: false, usage: None, self_check: None, + reset_progress_after_notes: false, }; let adapter = KeepsSessions(store.path().to_path_buf()); // No session yet, or one this computer doesn't have: nothing to keep, nothing said. diff --git a/crates/engine/src/workspace.rs b/crates/engine/src/workspace.rs index d6efb482..e03bc075 100644 --- a/crates/engine/src/workspace.rs +++ b/crates/engine/src/workspace.rs @@ -26,6 +26,8 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use std::path::{Path, PathBuf}; +pub mod progress; + pub const MANIFEST: &str = "craft.json"; /// Folders a copy of a project leaves out: what a build makes again, which can be gigabytes. diff --git a/crates/engine/src/workspace/progress.rs b/crates/engine/src/workspace/progress.rs new file mode 100644 index 00000000..79ee0db2 --- /dev/null +++ b/crates/engine/src/workspace/progress.rs @@ -0,0 +1,747 @@ +//! File progress kept between builder turns and runs. File contents, rather than timestamps, +//! decide whether the app changed; a successful write of identical bytes is not progress. + +use super::{OWN_NAMES, Workspace, rebuilt}; +use crate::CancelToken; +use craft_anything_evidence::{ErrorCode, ToolError}; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; +use serde_json::Value; +use sha2::{Digest, Sha256}; +use std::collections::HashMap; +use std::io::Read; +use std::path::{Path, PathBuf}; +use std::time::SystemTime; + +/// A builder gets two consecutive turns without changing the app before it needs attention. +pub const UNCHANGED_TURN_LIMIT: u32 = 2; +// Keep state inside an already reserved Craft directory. A project created by an older +// version may have its own app data named build-progress.json at the root. +pub const BUILD_PROGRESS_FILE: &str = "inbox/build-progress.json"; +pub const BUILD_PROGRESS_VERSION: u32 = 1; +const MAX_RECORD_BYTES: u64 = 16 * 1024; +const MAX_ENTRIES: usize = 100_000; +const MAX_DEPTH: usize = 64; + +const STEPS: &[crate::versioned::Step] = &[v0_to_v1]; +const _: () = assert!(STEPS.len() == BUILD_PROGRESS_VERSION as usize); + +fn v0_to_v1(doc: &mut Value) -> Result<(), String> { + if doc.is_object() { Ok(()) } else { Err("the progress record isn't a JSON object".into()) } +} + +/// The app as the builder last left it, and turns since it last changed. Only the rebuild run +/// writes this record. An older project without one has no counted turns. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct BuildProgress { + pub version: u32, + #[serde(default)] + pub fingerprint: Option, + #[serde(default)] + pub unchanged_turns: u32, +} + +impl Default for BuildProgress { + fn default() -> Self { + Self { version: BUILD_PROGRESS_VERSION, fingerprint: None, unchanged_turns: 0 } + } +} + +impl BuildProgress { + /// Read a project's progress. Broken or newer records stop the run rather than quietly + /// giving it another budget. Version 0 has the same shape; version 1 adds the version number. + pub fn load(root: &Path) -> Result { + let path = root.join(BUILD_PROGRESS_FILE); + let file = match std::fs::File::open(&path) { + Ok(f) => f, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(Self::default()), + Err(e) => return Err(ToolError::io(path.display(), e)), + }; + let mut bytes = Vec::new(); + file.take(MAX_RECORD_BYTES + 1).read_to_end(&mut bytes).map_err(|e| ToolError::io(path.display(), e))?; + if bytes.len() as u64 > MAX_RECORD_BYTES { + return Err(ToolError::new(ErrorCode::OverLimit, format!("{BUILD_PROGRESS_FILE} is too large to read."))); + } + let mut doc: Value = serde_json::from_slice(&bytes).map_err(|e| ToolError::invalid(format!("Couldn't read {BUILD_PROGRESS_FILE}: {e}")))?; + let found = match doc.get("version") { + None => 0, + Some(v) => { + v.as_u64().and_then(|v| u32::try_from(v).ok()).ok_or_else(|| ToolError::invalid(format!("{BUILD_PROGRESS_FILE} has an invalid version.")))? + } + }; + if found > BUILD_PROGRESS_VERSION { + return Err(ToolError::unsupported(format!("{BUILD_PROGRESS_FILE} was written by a newer Craft Anything, so this build is left alone.")) + .with_remediation("Update Craft Anything to continue this build.")); + } + let version = crate::versioned::upgrade(&mut doc, found, 0, STEPS) + .map_err(|e| ToolError::invalid(format!("Couldn't bring {BUILD_PROGRESS_FILE} up to date: {e}")))?; + let obj = doc.as_object_mut().ok_or_else(|| ToolError::invalid(format!("{BUILD_PROGRESS_FILE} isn't a JSON object.")))?; + obj.insert("version".into(), Value::from(version)); + serde_json::from_value(doc).map_err(|e| ToolError::invalid(format!("Couldn't read {BUILD_PROGRESS_FILE}: {e}"))) + } + + pub fn save(&self, root: &Path) -> Result<(), ToolError> { + if self.version > BUILD_PROGRESS_VERSION { + return Err(ToolError::unsupported("The newer build progress record can't be overwritten.")); + } + // Recheck on disk: a run must not overwrite a newer record written since it started. + Self::load(root)?; + let mut current = self.clone(); + current.version = BUILD_PROGRESS_VERSION; + let bytes = serde_json::to_vec_pretty(¤t).map_err(|e| ToolError::internal(e.to_string()))?; + crate::project::write_atomic(&root.join(BUILD_PROGRESS_FILE), &bytes) + } + + /// A fresh request starts a new attempt; a plain retry keeps the existing count. + pub fn reset(&mut self) { + self.unchanged_turns = 0; + } + + /// An unavailable scan breaks the comparison streak. A later readable run needs a new + /// baseline, not a comparison with files from before the unobserved turns. + pub fn invalidate(&mut self) { + self.fingerprint = None; + self.reset(); + } + + /// An edit made between turns or while the project was closed is progress too. + pub fn before_turn(&mut self, fingerprint: String) { + if self.fingerprint.as_deref() != Some(fingerprint.as_str()) { + self.reset(); + } + self.fingerprint = Some(fingerprint); + } + + /// Count only completed builder turns. Checks and conversation records never count as a turn. + pub fn after_turn(&mut self, fingerprint: String) { + self.unchanged_turns = if self.fingerprint.as_deref() == Some(fingerprint.as_str()) { self.unchanged_turns.saturating_add(1) } else { 0 }; + self.fingerprint = Some(fingerprint); + } + + pub fn stalled(&self) -> bool { + self.unchanged_turns >= UNCHANGED_TURN_LIMIT + } +} + +/// The published schema of the progress record (`cargo xtask schemas`). +pub fn build_progress_schema() -> String { + let schema = schemars::generate::SchemaSettings::draft2020_12().into_generator().into_root_schema_for::(); + serde_json::to_string_pretty(&schema).map(|s| s + "\n").unwrap_or_default() +} + +impl Workspace { + /// A stable digest of the app's file names and contents. Dependency and build caches follow + /// the same exclusions as project copies; Craft's own records and git metadata are left out. + /// Symlinks are recorded as links, never followed. Reads are streamed and cancellable. + pub fn app_fingerprint(&self, stop: &CancelToken) -> Result { + AppFingerprintCache::default().scan(self, stop) + } +} + +/// Content digests reused within one build run. The first scan reads the files; subsequent +/// scans only reread files whose metadata changed. Nothing here is persisted or added to the +/// fingerprint, so identical bytes still have the same version-1 fingerprint after a rewrite. +#[derive(Default)] +pub struct AppFingerprintCache { + root: Option, + files: HashMap, + #[cfg(test)] + content_bytes: u64, +} + +struct CachedFile { + stamp: FileStamp, + digest: [u8; 32], + seen: bool, +} + +#[derive(Clone, Copy, PartialEq, Eq)] +struct FileStamp { + len: u64, + modified: SystemTime, + executable: u32, + #[cfg(unix)] + identity: (u64, u64, i64, i64), +} + +impl FileStamp { + fn of(metadata: &std::fs::Metadata) -> Option { + #[cfg(unix)] + use std::os::unix::fs::MetadataExt; + Some(Self { + len: metadata.len(), + modified: metadata.modified().ok()?, + executable: executable_mode(metadata), + #[cfg(unix)] + identity: (metadata.dev(), metadata.ino(), metadata.ctime(), metadata.ctime_nsec()), + }) + } +} + +struct Scan { + hash: Sha256, + visited: usize, +} + +impl AppFingerprintCache { + /// Return the same content fingerprint as a fresh scan while avoiding unchanged reads. + /// Root changes, scan errors and cancellation discard cached comparisons. Missing files + /// and paths that become another type are removed after every successful scan. + pub fn scan(&mut self, ws: &Workspace, stop: &CancelToken) -> Result { + let root = ws.app_dir(); + if self.root.as_ref() != Some(&root) { + self.files.clear(); + self.root = Some(root.clone()); + } + for file in self.files.values_mut() { + file.seen = false; + } + let mut scan = Scan { hash: Sha256::new(), visited: 0 }; + scan.hash.update(b"craft-build-files-v1\0"); + if let Err(error) = fingerprint_tree(ws, &root, &root, &mut scan, self, 0, stop) { + self.files.clear(); + self.root = None; + return Err(error); + } + self.files.retain(|_, file| file.seen); + Ok(hex::encode(scan.hash.finalize())) + } + + fn file(&mut self, path: &Path, relative: &Path, stop: &CancelToken) -> Result<[u8; 32], ToolError> { + stop.check()?; + let metadata = std::fs::symlink_metadata(path).map_err(|e| ToolError::io(path.display(), e))?; + if !metadata.is_file() { + return Err(ToolError::new(ErrorCode::FileChanged, format!("{} changed type while checking build progress.", path.display()))); + } + if let Some(stamp) = FileStamp::of(&metadata) + && let Some(file) = self.files.get_mut(relative) + && file.stamp == stamp + { + file.seen = true; + return Ok(file.digest); + } + let read = file_fingerprint(path, stop)?; + #[cfg(test)] + { + self.content_bytes = self.content_bytes.saturating_add(read.content_bytes); + } + // Use the metadata of the file actually opened, only after its entire stable read + // succeeded. Without a usable modification time it is deliberately not cached. + if let Some(stamp) = read.stamp { + self.files.insert(relative.to_path_buf(), CachedFile { stamp, digest: read.digest, seen: true }); + } + Ok(read.digest) + } +} + +fn fingerprint_tree( + ws: &Workspace, + root: &Path, + dir: &Path, + scan: &mut Scan, + cache: &mut AppFingerprintCache, + depth: usize, + stop: &CancelToken, +) -> Result<(), ToolError> { + stop.check()?; + if depth > MAX_DEPTH { + return Err(ToolError::new( + ErrorCode::OverLimit, + format!("Can't check build progress: {} is nested more than {MAX_DEPTH} folders deep.", dir.display()), + )); + } + let mut entries = Vec::new(); + for entry in std::fs::read_dir(dir).map_err(|e| ToolError::io(dir.display(), e))? { + stop.check()?; + let entry = entry.map_err(|e| ToolError::io(dir.display(), e))?; + let path = entry.path(); + let name = entry.file_name(); + if name == ".git" || (ws.manifest.app_in_root && dir == root && OWN_NAMES.iter().any(|n| name == *n)) { + continue; + } + let kind = entry.file_type().map_err(|e| ToolError::io(path.display(), e))?; + if kind.is_dir() && rebuilt(&path) { + continue; + } + scan.visited = scan.visited.saturating_add(1); + if scan.visited > MAX_ENTRIES { + return Err(ToolError::new( + ErrorCode::OverLimit, + format!("Can't check build progress: {} contains more than {MAX_ENTRIES} files and folders.", root.display()), + )); + } + entries.push((path, kind)); + } + entries.sort_by(|a, b| a.0.cmp(&b.0)); + for (path, kind) in entries { + stop.check()?; + if kind.is_dir() { + fingerprint_tree(ws, root, &path, scan, cache, depth + 1, stop)?; + continue; + } + let relative = path.strip_prefix(root).map_err(|_| ToolError::internal("A build file is outside its app folder."))?; + for component in relative.components() { + let name = component.as_os_str().as_encoded_bytes(); + scan.hash.update((name.len() as u64).to_le_bytes()); + scan.hash.update(name); + } + scan.hash.update(0u64.to_le_bytes()); + if kind.is_symlink() { + scan.hash.update(b"link\0"); + let target = std::fs::read_link(&path).map_err(|e| ToolError::io(path.display(), e))?; + let target = target.as_os_str().as_encoded_bytes(); + scan.hash.update((target.len() as u64).to_le_bytes()); + scan.hash.update(target); + } else if kind.is_file() { + scan.hash.update(b"file\0"); + scan.hash.update(cache.file(&path, relative, stop)?); + } else { + // A dev server can leave sockets or pipes beside its source. Their contents are + // not comparable (and opening a FIFO can block); only their name and type count. + scan.hash.update(special_type(kind)); + } + } + Ok(()) +} + +fn special_type(kind: std::fs::FileType) -> &'static [u8] { + #[cfg(unix)] + { + use std::os::unix::fs::FileTypeExt; + if kind.is_socket() { + return b"socket\0"; + } + if kind.is_fifo() { + return b"fifo\0"; + } + if kind.is_char_device() { + return b"character-device\0"; + } + if kind.is_block_device() { + return b"block-device\0"; + } + } + let _ = kind; + b"special\0" +} + +struct FileFingerprint { + digest: [u8; 32], + stamp: Option, + #[cfg(test)] + content_bytes: u64, +} + +fn file_fingerprint(path: &Path, stop: &CancelToken) -> Result { + let mut file = std::fs::File::open(path).map_err(|e| ToolError::io(path.display(), e))?; + let before = file.metadata().map_err(|e| ToolError::io(path.display(), e))?; + // A growing log can't keep the scan running forever. A changed file is checked again on the + // next run instead of being mistaken for an unchanged app. + let mut reader = (&mut file).take(before.len()); + let mut bytes = 0u64; + let mut hash = Sha256::new(); + // Executability changes what can be built or launched. Other permission and timestamp + // changes alone do not count as progress. Non-Unix platforms have no execute mode bits. + hash.update(executable_mode(&before).to_le_bytes()); + let mut buffer = [0u8; 64 * 1024]; + loop { + stop.check()?; + let n = reader.read(&mut buffer).map_err(|e| ToolError::io(path.display(), e))?; + if n == 0 { + break; + } + bytes = bytes.saturating_add(n as u64); + hash.update(buffer.get(..n).ok_or_else(|| ToolError::internal("A file read exceeded its buffer."))?); + } + let after = file.metadata().map_err(|e| ToolError::io(path.display(), e))?; + if bytes != before.len() + || before.len() != after.len() + || before.modified().ok() != after.modified().ok() + || executable_mode(&before) != executable_mode(&after) + || FileStamp::of(&before) != FileStamp::of(&after) + { + return Err(ToolError::new( + ErrorCode::FileChanged, + format!("{} changed while checking build progress. Try again after its write finishes.", path.display()), + )); + } + Ok(FileFingerprint { + digest: hash.finalize().into(), + stamp: FileStamp::of(&before), + #[cfg(test)] + content_bytes: bytes, + }) +} + +#[cfg(unix)] +fn executable_mode(metadata: &std::fs::Metadata) -> u32 { + use std::os::unix::fs::PermissionsExt; + metadata.permissions().mode() & 0o111 +} + +#[cfg(not(unix))] +fn executable_mode(_metadata: &std::fs::Metadata) -> u32 { + 0 +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::workspace::{ExistingFiles, TargetRef}; + + fn fingerprint(ws: &Workspace) -> String { + ws.app_fingerprint(&CancelToken::default()).unwrap() + } + + fn modified(path: &Path, seconds: u64) { + let time = SystemTime::UNIX_EPOCH + std::time::Duration::from_secs(seconds); + std::fs::File::options().write(true).open(path).unwrap().set_times(std::fs::FileTimes::new().set_modified(time)).unwrap(); + } + + #[test] + fn cache_does_not_reread_an_unchanged_large_tree_or_count_identical_rewrites_as_progress() { + let d = tempfile::tempdir().unwrap(); + let ws = Workspace::create(d.path(), "Game", "A game with imported assets", TargetRef::None).unwrap(); + let bytes = vec![b'a'; 2 * 1024 * 1024]; + for name in ["Original/a.bin", "Original/b.bin", "assets/c.bin", "assets/d.bin"] { + let path = ws.app_dir().join(name); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(&path, &bytes).unwrap(); + modified(&path, 1); + } + let mut cache = AppFingerprintCache::default(); + let stop = CancelToken::default(); + let first = cache.scan(&ws, &stop).unwrap(); + assert_eq!(cache.content_bytes, 8 * 1024 * 1024, "the first scan actually read every asset byte"); + let second = cache.scan(&ws, &stop).unwrap(); + assert_eq!(second, first); + assert_eq!(cache.content_bytes, 8 * 1024 * 1024, "the unchanged second scan read zero content bytes"); + let file = ws.app_dir().join("Original/a.bin"); + std::fs::write(&file, &bytes).unwrap(); + modified(&file, 2); + let rewritten = cache.scan(&ws, &stop).unwrap(); + assert_eq!(rewritten, first, "mtime is only a cache key, not build progress"); + assert_eq!(cache.content_bytes, 10 * 1024 * 1024, "only the rewritten file was reread"); + let mut progress = BuildProgress::default(); + progress.before_turn(first.clone()); + progress.after_turn(second); + progress.after_turn(rewritten); + assert!(progress.stalled(), "rewriting identical bytes cannot reset the unchanged-turn limit"); + let mut changed = bytes; + changed[0] = b'b'; + std::fs::write(&file, changed).unwrap(); + modified(&file, 3); + let edited = cache.scan(&ws, &stop).unwrap(); + assert_ne!(edited, first, "a same-length content edit is still progress"); + assert_eq!(cache.content_bytes, 12 * 1024 * 1024); + assert_eq!(edited, fingerprint(&ws), "cached and fresh scans use the same content hash"); + } + + #[test] + fn cache_tracks_added_deleted_and_retyped_paths() { + let d = tempfile::tempdir().unwrap(); + let ws = Workspace::create(d.path(), "Files", "A project", TargetRef::None).unwrap(); + std::fs::create_dir_all(ws.app_dir()).unwrap(); + let path = ws.app_dir().join("item"); + std::fs::write(&path, "a").unwrap(); + let mut cache = AppFingerprintCache::default(); + let stop = CancelToken::default(); + let first = cache.scan(&ws, &stop).unwrap(); + std::fs::remove_file(&path).unwrap(); + std::fs::create_dir(&path).unwrap(); + std::fs::write(path.join("child"), "b").unwrap(); + assert_ne!(cache.scan(&ws, &stop).unwrap(), first); + std::fs::remove_dir_all(&path).unwrap(); + std::fs::write(&path, "a").unwrap(); + assert_eq!(cache.scan(&ws, &stop).unwrap(), first); + assert_eq!(cache.content_bytes, 3, "a type change evicts the old file digest"); + std::fs::remove_file(&path).unwrap(); + let empty = cache.scan(&ws, &stop).unwrap(); + assert_ne!(empty, first); + std::fs::write(&path, "a").unwrap(); + assert_eq!(cache.scan(&ws, &stop).unwrap(), first); + assert_eq!(cache.content_bytes, 4, "a removed path is not retained in the cache"); + std::fs::write(ws.app_dir().join("new"), "c").unwrap(); + assert_ne!(cache.scan(&ws, &stop).unwrap(), first); + assert_eq!(cache.content_bytes, 5, "adding a file leaves unchanged file contents cached"); + } + + #[test] + fn cache_discards_comparisons_after_scan_errors_and_cancellation() { + let d = tempfile::tempdir().unwrap(); + let ws = Workspace::create(d.path(), "Files", "A project", TargetRef::None).unwrap(); + std::fs::create_dir_all(ws.app_dir()).unwrap(); + std::fs::write(ws.app_dir().join("source"), "abc").unwrap(); + let mut cache = AppFingerprintCache::default(); + let stop = CancelToken::default(); + let first = cache.scan(&ws, &stop).unwrap(); + let nested = (0..=MAX_DEPTH).fold(ws.app_dir().join("deep"), |p, _| p.join("d")); + std::fs::create_dir_all(nested).unwrap(); + assert_eq!(cache.scan(&ws, &stop).unwrap_err().code, ErrorCode::OverLimit); + std::fs::remove_dir_all(ws.app_dir().join("deep")).unwrap(); + assert_eq!(cache.scan(&ws, &stop).unwrap(), first); + assert_eq!(cache.content_bytes, 6, "an error discarded the previous successful read too"); + let cancelled = CancelToken::default(); + cancelled.cancel(); + assert_eq!(cache.scan(&ws, &cancelled).unwrap_err().code, ErrorCode::Cancelled); + assert_eq!(cache.scan(&ws, &stop).unwrap(), first); + assert_eq!(cache.content_bytes, 9, "cancellation left no cached partial scan"); + } + + #[test] + fn cache_never_reuses_reads_from_another_project_root() { + let d = tempfile::tempdir().unwrap(); + let mut cache = AppFingerprintCache::default(); + let stop = CancelToken::default(); + for name in ["First", "Second"] { + let ws = Workspace::create(d.path(), name, "A project", TargetRef::None).unwrap(); + std::fs::create_dir_all(ws.app_dir()).unwrap(); + let path = ws.app_dir().join("source"); + std::fs::write(&path, "abc").unwrap(); + modified(&path, 1); + assert_eq!(cache.scan(&ws, &stop).unwrap(), fingerprint(&ws)); + } + assert_eq!(cache.content_bytes, 6, "matching relative paths and metadata in another root still need reading"); + } + + #[test] + fn progress_counts_identical_bytes_and_resets_for_edits() { + let d = tempfile::tempdir().unwrap(); + let ws = Workspace::create(d.path(), "Notes", "A notes app", TargetRef::None).unwrap(); + std::fs::create_dir_all(ws.app_dir()).unwrap(); + let file = ws.app_dir().join("main.txt"); + std::fs::write(&file, "first").unwrap(); + let first = fingerprint(&ws); + let mut progress = BuildProgress::load(&ws.root).unwrap(); + progress.before_turn(first.clone()); + std::fs::write(&file, "first").unwrap(); + progress.after_turn(fingerprint(&ws)); + assert_eq!(progress.unchanged_turns, 1); + progress.save(&ws.root).unwrap(); + let mut progress = BuildProgress::load(&Workspace::open(&ws.root).unwrap().root).unwrap(); + progress.before_turn(fingerprint(&ws)); + progress.after_turn(fingerprint(&ws)); + assert!(progress.stalled()); + std::fs::write(&file, "other").unwrap(); + assert_ne!(first, fingerprint(&ws), "same length, different contents"); + progress.before_turn(fingerprint(&ws)); + assert_eq!(progress.unchanged_turns, 0); + std::fs::remove_file(file).unwrap(); + progress.after_turn(fingerprint(&ws)); + assert_eq!(progress.unchanged_turns, 0, "deleting an app file is a change too"); + } + + #[test] + fn root_apps_ignore_craft_records_and_caches_but_keep_runtime_assets() { + let d = tempfile::tempdir().unwrap(); + let root = d.path().join("mine"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::write(root.join("index.html"), "the app").unwrap(); + let ws = Workspace::create_at(&root, "Mine", "A page", TargetRef::None, Some(ExistingFiles::Edit)).unwrap(); + let before = fingerprint(&ws); + for dir in ["checks", "node_modules", "target", ".git"] { + std::fs::create_dir_all(root.join(dir)).unwrap(); + std::fs::write(root.join(dir).join("changed.txt"), "changed outside the app's source").unwrap(); + } + std::fs::write(root.join("feed.json"), "changed feed").unwrap(); + BuildProgress::default().save(&ws.root).unwrap(); + assert_eq!(before, fingerprint(&ws)); + std::fs::create_dir_all(root.join("assets/obj")).unwrap(); + std::fs::write(root.join("assets/obj/shape.obj"), "runtime model").unwrap(); + assert_ne!(before, fingerprint(&ws), "an obj asset folder is not a managed build cache"); + } + + #[test] + fn progress_records_migrate_and_newer_or_broken_records_are_left_alone() { + let d = tempfile::tempdir().unwrap(); + let path = d.path().join(BUILD_PROGRESS_FILE); + assert_eq!(BuildProgress::load(d.path()).unwrap(), BuildProgress::default()); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(&path, r#"{"fingerprint":"previous","unchanged_turns":2}"#).unwrap(); + let old = BuildProgress::load(d.path()).unwrap(); + assert_eq!(old.version, BUILD_PROGRESS_VERSION); + assert!(old.stalled()); + old.save(d.path()).unwrap(); + assert!(std::fs::read_to_string(&path).unwrap().contains("\"version\": 1")); + for bad in [r#"{"version":99,"future":true}"#, "not json", r#"{"version":-1}"#, r#"{"app_data":"not a Craft progress record"}"#] { + std::fs::write(&path, bad).unwrap(); + assert!(BuildProgress::load(d.path()).is_err()); + assert!(old.save(d.path()).is_err()); + assert_eq!(std::fs::read_to_string(&path).unwrap(), bad); + } + std::fs::write(&path, vec![b' '; MAX_RECORD_BYTES as usize + 1]).unwrap(); + assert_eq!(BuildProgress::load(d.path()).unwrap_err().code, ErrorCode::OverLimit); + } + + #[test] + fn a_duplicate_has_a_new_attempt_and_progress_is_never_app_source() { + let d = tempfile::tempdir().unwrap(); + let root = d.path().join("mine"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::write(root.join("index.html"), "the app").unwrap(); + let ws = Workspace::create_at(&root, "Mine", "A page", TargetRef::None, Some(ExistingFiles::Edit)).unwrap(); + let progress = BuildProgress { fingerprint: Some(fingerprint(&ws)), unchanged_turns: 2, ..BuildProgress::default() }; + progress.save(&ws.root).unwrap(); + let copy = ws.duplicate("Another attempt").unwrap(); + assert_eq!(BuildProgress::load(©.root).unwrap(), BuildProgress::default()); + assert!(!copy.app_dir().join(BUILD_PROGRESS_FILE).exists()); + assert_eq!(fingerprint(&ws), fingerprint(©)); + assert_eq!(BuildProgress::load(&ws.root).unwrap(), progress, "the original keeps its limit"); + } + + #[test] + fn an_existing_root_build_progress_name_remains_the_persons_app_data() { + for (data, directory) in + [(r#"{"player":"saved"}"#, false), ("{}", false), (r#"{"version":1,"fingerprint":null,"unchanged_turns":2}"#, false), ("a runtime asset", true)] + { + let d = tempfile::tempdir().unwrap(); + let root = d.path().join("mine"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::write(root.join("index.html"), "the app").unwrap(); + let ws = Workspace::create_at(&root, "Mine", "A page", TargetRef::None, Some(ExistingFiles::Edit)).unwrap(); + let before = fingerprint(&ws); + // Simulate an existing in-place project from before progress was recorded. Even a + // JSON object matching the new record's shape does not prove Craft owns it. + let relative = if directory { "build-progress.json/state.txt" } else { "build-progress.json" }; + let app_file = root.join(relative); + std::fs::create_dir_all(app_file.parent().unwrap()).unwrap(); + std::fs::write(&app_file, data).unwrap(); + assert!(!ws.is_own_record(&root.join("build-progress.json"))); + let with_data = fingerprint(&ws); + assert_ne!(before, with_data, "the root name belongs to the app"); + let progress = BuildProgress { fingerprint: Some(with_data.clone()), unchanged_turns: 2, ..BuildProgress::default() }; + progress.save(&root).unwrap(); + assert_eq!(std::fs::read_to_string(&app_file).unwrap(), data, "save must not claim or migrate the root file"); + assert_eq!(fingerprint(&ws), with_data, "the inbox record is not app progress"); + std::fs::write(&app_file, format!("{data}\n")).unwrap(); + assert_ne!(fingerprint(&ws), with_data, "changes to that app file count"); + std::fs::write(&app_file, data).unwrap(); + let copy = ws.duplicate("Another attempt").unwrap(); + assert_eq!(std::fs::read_to_string(copy.app_dir().join(relative)).unwrap(), data); + assert_eq!(BuildProgress::load(©.root).unwrap(), BuildProgress::default()); + assert!(!copy.app_dir().join(BUILD_PROGRESS_FILE).exists()); + ws.delete().unwrap(); + assert_eq!(std::fs::read_to_string(&app_file).unwrap(), data, "removing Craft's records must keep the app's data"); + assert!(!root.join(BUILD_PROGRESS_FILE).exists()); + } + } + + #[test] + fn inbox_messages_do_not_consume_the_progress_record() { + use crate::inbox::{Answer, Inbox, RequestKind}; + let d = tempfile::tempdir().unwrap(); + let progress = BuildProgress { unchanged_turns: 2, ..BuildProgress::default() }; + progress.save(d.path()).unwrap(); + let inbox = Inbox::new(d.path().join("inbox")); + assert!(inbox.pending().is_empty()); + let id = inbox.post(RequestKind::Question, "Continue?", "", Vec::new(), true, None).unwrap(); + assert_eq!(inbox.pending().len(), 1); + inbox.answer(&id, &Answer { text: Some("Yes".into()), ..Default::default() }).unwrap(); + let answer = inbox.wait(&id, std::time::Duration::from_millis(100), &|| false).unwrap(); + assert_eq!(answer.text.as_deref(), Some("Yes")); + assert!(inbox.pending().is_empty()); + assert_eq!(BuildProgress::load(d.path()).unwrap(), progress); + } + + #[test] + fn moving_the_project_folder_keeps_its_unchanged_turn_count() { + let d = tempfile::tempdir().unwrap(); + let ws = Workspace::create(d.path(), "Notes", "A notes app", TargetRef::None).unwrap(); + std::fs::create_dir_all(ws.app_dir()).unwrap(); + std::fs::write(ws.app_dir().join("index.html"), "the app").unwrap(); + let progress = BuildProgress { fingerprint: Some(fingerprint(&ws)), unchanged_turns: 2, ..BuildProgress::default() }; + progress.save(&ws.root).unwrap(); + let moved = d.path().join("moved-project"); + std::fs::rename(&ws.root, &moved).unwrap(); + let ws = Workspace::open(&moved).unwrap(); + let mut loaded = BuildProgress::load(&ws.root).unwrap(); + loaded.before_turn(fingerprint(&ws)); + assert_eq!(loaded, progress, "the fingerprint is relative to the app, not its original folder"); + assert!(loaded.stalled()); + } + + #[cfg(unix)] + #[test] + fn cache_keeps_executable_mode_as_progress_but_not_other_permission_bits() { + use std::os::unix::fs::PermissionsExt; + let d = tempfile::tempdir().unwrap(); + let ws = Workspace::create(d.path(), "Script", "A script", TargetRef::None).unwrap(); + std::fs::create_dir_all(ws.app_dir()).unwrap(); + let file = ws.app_dir().join("build.sh"); + std::fs::write(&file, "#!/bin/sh\nexit 0\n").unwrap(); + std::fs::set_permissions(&file, std::fs::Permissions::from_mode(0o644)).unwrap(); + let mut cache = AppFingerprintCache::default(); + let stop = CancelToken::default(); + let before = cache.scan(&ws, &stop).unwrap(); + let mut progress = BuildProgress { fingerprint: Some(before.clone()), unchanged_turns: 2, ..BuildProgress::default() }; + std::fs::set_permissions(&file, std::fs::Permissions::from_mode(0o600)).unwrap(); + progress.before_turn(cache.scan(&ws, &stop).unwrap()); + assert!(progress.stalled(), "read/write mode alone is not source progress"); + std::fs::set_permissions(&file, std::fs::Permissions::from_mode(0o700)).unwrap(); + progress.before_turn(cache.scan(&ws, &stop).unwrap()); + assert_ne!(progress.fingerprint.as_deref(), Some(before.as_str())); + assert_eq!(progress.unchanged_turns, 0, "making the script runnable is progress"); + } + + #[test] + fn deeply_nested_apps_stop_without_recursing_indefinitely() { + let d = tempfile::tempdir().unwrap(); + let ws = Workspace::create(d.path(), "Nested", "A nested app", TargetRef::None).unwrap(); + let mut dir = ws.app_dir(); + for _ in 0..=MAX_DEPTH { + dir.push("d"); + } + std::fs::create_dir_all(dir).unwrap(); + let error = ws.app_fingerprint(&CancelToken::default()).unwrap_err(); + assert_eq!(error.code, ErrorCode::OverLimit); + assert!(error.message.contains("nested")); + } + + #[test] + fn the_progress_schema_is_current() { + let path = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../docs/schemas/build-progress.schema.json"); + let saved = std::fs::read_to_string(path).unwrap_or_default(); + assert_eq!(saved.replace("\r\n", "\n"), build_progress_schema()); + } + + #[cfg(unix)] + #[test] + fn special_files_contribute_their_name_and_type_without_reading_them() { + use std::os::unix::net::UnixListener; + let d = tempfile::tempdir().unwrap(); + let ws = Workspace::create(d.path(), "Socket", "An app with a dev server", TargetRef::None).unwrap(); + std::fs::create_dir_all(ws.app_dir()).unwrap(); + let path = ws.app_dir().join("server.sock"); + let listener = UnixListener::bind(&path).unwrap(); + let socket = fingerprint(&ws); + assert_eq!(fingerprint(&ws), socket, "an idle socket is stable and is not read"); + drop(listener); + std::fs::remove_file(&path).unwrap(); + std::fs::write(&path, "").unwrap(); + assert_ne!(fingerprint(&ws), socket, "a regular file at the same name has a different type"); + std::fs::remove_file(&path).unwrap(); + let _listener = UnixListener::bind(&path).unwrap(); + assert_eq!(fingerprint(&ws), socket, "recreating the same endpoint is not source progress"); + std::fs::write(ws.app_dir().join("main.txt"), "new source").unwrap(); + assert_ne!(fingerprint(&ws), socket, "regular app changes still count beside a socket"); + } + + #[cfg(unix)] + #[test] + fn links_are_not_followed_and_cancel_stops_the_scan() { + let d = tempfile::tempdir().unwrap(); + let ws = Workspace::create(d.path(), "Notes", "A notes app", TargetRef::None).unwrap(); + std::fs::create_dir_all(ws.app_dir()).unwrap(); + let outside = d.path().join("outside"); + std::fs::write(&outside, "one").unwrap(); + std::os::unix::fs::symlink(&outside, ws.app_dir().join("link")).unwrap(); + std::os::unix::fs::symlink(ws.app_dir(), ws.app_dir().join("cycle")).unwrap(); + let before = fingerprint(&ws); + std::fs::write(outside, "two").unwrap(); + assert_eq!(before, fingerprint(&ws)); + let stop = CancelToken::default(); + stop.cancel(); + assert_eq!(ws.app_fingerprint(&stop).unwrap_err().code, ErrorCode::Cancelled); + } +} diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index b010cf2b..e4df6d07 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -165,11 +165,11 @@ only when it's green and a maintainer has fully reviewed it, code as well as wor ### R5.3 A builder that can't make progress stops Owner: open · Status: todo Every round spends the person's money or quota. One that can't do anything shouldn't start. -- [ ] A build stops and says why after two builder turns in a row that changed nothing in `app/`, +- [x] A build stops and says why after two builder turns in a row that changed nothing in `app/`, on every backend (#44, #110, #417, #481). The count is saved in the project, so it holds across runs: a run ends at 7 checks (`max_rounds`) but check numbers carry on, so #44's check 28 and #417's 35+ look like several runs (a guess: Try again is the only restart found, and - Auto mode doesn't start runs) + Auto mode doesn't start runs) — implementation: `2fc2970948315b64d47359543b458509da42ab3f` - [x] A builder whose tools didn't start isn't resumed. `turn` (`rebuild/mod.rs`) tries 3 more times ("Picking up where it left off", #377) because `worth_retrying` doesn't know this error. Give it a typed signal, as `Conversation::session_lost` does, not one more word in that function's diff --git a/docs/STATUS.md b/docs/STATUS.md index f8c98c25..e14bd030 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -238,3 +238,45 @@ was run; other text-call formats remain unsupported. ## Next See [ROADMAP.md](ROADMAP.md) for the ordered plan. + +## Build progress across runs + +When app files can be compared, a build stops after two consecutive completed builder turns leave them unchanged. +The versioned `inbox/build-progress.json` record keeps that count when the project is reopened or +Try again is pressed. File contents and executable permissions decide progress, so rewriting +identical bytes does not reset it. An actual app edit or a new nonempty request starts another +attempt, including notes queued during a turn or check and notes consumed by a builder hook. +Empty notes and plain Try again keep the saved count. Dependency/build caches, +git metadata and Craft's own records, including checks in a project edited in place, do not count. +Symlinks are recorded without following their targets. A successful check may still finish the +second unchanged turn, and a run waiting only for checks does not start the builder again. +Within a run, file digests are reused while size, modification time and executable bits match; +Unix file identity and change time also invalidate a cached read. The first scan reads content, +but unchanged imported assets are not read again before and after every turn. The metadata is +only a cache key: rewriting identical bytes still produces the same content fingerprint. +The cache is isolated by project root, drops deleted or retyped paths, and is discarded after +scan errors or cancellation. It is not persisted, and the version-1 fingerprint/schema is unchanged. +Sockets, pipes and devices contribute their names and types without opening their contents. +If a scan cannot finish because of limits, permissions or changing files, the feed says why the +unchanged-turn limit is skipped for that run. The saved fingerprint and count are cleared so a +later readable run starts a fresh comparison. Checks and the existing round limit still apply. +Cancellation still stops the run; completed builder replies are saved as awaiting checks before +the post-turn scan. Broken or newer progress records and record-write failures remain errors. + +Scripted API turns and executable CLI stand-ins cover the stop, persistence, fresh requests and +check recovery. This change has not been verified with a paid model or a live provider. Copying a +whole project keeps its progress record; Duplicate creates a new attempt with a fresh conversation +and count, as it already does for usage. +An existing root-level file or directory named `build-progress.json` remains app data: it is +fingerprinted, copied by Duplicate and kept when Craft's project records are removed. Its JSON +shape does not make it a Craft record, and it is never migrated into the inbox. + +Notes consumed by builder hooks now only queue a progress reset while the turn is running. +The reset is saved after the successful turn and its `awaiting_check` recovery state are +persisted. Multiple consumed notes coalesce into one reset; blank notes do not reset the count. +A broken or inaccessible progress record still returns its actual error at that safe boundary, +without starting another provider request or losing the finished builder reply. +If the turn fails or is cancelled after consuming a note, its reset is saved before the run ends +so reopening keeps the new request's full allowance. The original failure or cancellation remains +the result; a reset that cannot be saved is reported separately and never overwrites a broken +or newer record. Scripted turns cover both outcomes, blank notes and unreadable records. diff --git a/docs/file-formats.md b/docs/file-formats.md index 4af988d6..fa602944 100644 --- a/docs/file-formats.md +++ b/docs/file-formats.md @@ -8,6 +8,7 @@ version up to the current one, then turn the document into Rust types. The step | File | Owner | Current version | Older files | Newer files | |---|---|---|---|---| | `/craft.json` | `engine::workspace` (`Manifest`) | `MANIFEST_VERSION` = 2 (`version`; missing = 0) | brought forward, saved as current | opened read-only: saving is refused | +| `/inbox/build-progress.json` | `engine::workspace::progress` (`BuildProgress`; written only by the rebuild run) | `BUILD_PROGRESS_VERSION` = 1 (`version`; missing = 0) | brought forward; an absent file starts with no counted turns | the build stops and the record is left unchanged | | `config.toml` (`settings::config_dir()`) | `engine::settings` (`Settings`) | `SETTINGS_VERSION` = 2 (`version`; missing = 0; 2 moved `[engines.ghidra] enabled` to `deep_mode`) | brought forward | loaded, never saved over | | `.cany/project.toml` | `engine::project` (`ProjectMeta`) | `FORMAT_VERSION` = 1 (`format_version`, required) | brought forward | refused, with "update Craft Anything" | | `/transcript.json` | `ai::context` (the API builder's conversation) | `TRANSCRIPT_VERSION` = 1 (`{version, history}`) | read as before | not resumed; a fresh build replaces it | @@ -23,8 +24,8 @@ A saved `User` message with pictures is `{text, pictures: [paths]}`, where the p copies in the project's `attachments/`; one without pictures stays the plain string earlier versions wrote. -Other JSON in a project (`assets.json`, `inbox/`, `checks/`, evidence records) has no version yet; -add one the first time its shape changes. +Other JSON in a project (`assets.json`, `inbox/` except `inbox/build-progress.json`, `checks/`, +evidence records) has no version yet; add one the first time its shape changes. ## Stand-in cached responses @@ -58,5 +59,5 @@ out of the project list. list and the constant disagree. 3. Add a test that reads a literal sample of the old file (keep the existing samples; old files must keep loading forever). -4. For `craft.json`, `mechanics.json` and `self-test.json`, run `cargo xtask schemas` and commit `docs/schemas/`. +4. For `craft.json`, `inbox/build-progress.json`, `mechanics.json` and `self-test.json`, run `cargo xtask schemas` and commit `docs/schemas/`. `cargo xtask schemas --check` (and an engine test) fail when the published schema is stale. diff --git a/docs/pitfalls.md b/docs/pitfalls.md index 839f5dfa..6497e655 100644 --- a/docs/pitfalls.md +++ b/docs/pitfalls.md @@ -401,6 +401,11 @@ tool calls without matching results. The next model request then carried unpaire Every retained call needs a matching result, including an explicit non-execution result. Test the next serialized request as well as whether a tool ran. +**A deferred request reset lost on failure.** Moving consumed-note bookkeeping out of the live +builder loop saved the reset only after a successful turn. A failed or cancelled turn had already +removed the note, so reopening kept the old unchanged-turn count. Persist deferred request state +on terminal errors too, outside provider retries; keep the original outcome if that save also fails. + **A filter written for one kind of text, run on another.** The stand-in's secret scan was written for API answers, then also ran on recorded scripts: `this.token=t` became `[dropped]` and the scripts broke, and "Basic Membership" lost a word. It also tested whole tokens, so a secret inside a URL or diff --git a/docs/schemas/build-progress.schema.json b/docs/schemas/build-progress.schema.json new file mode 100644 index 00000000..003a1d60 --- /dev/null +++ b/docs/schemas/build-progress.schema.json @@ -0,0 +1,30 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "title": "BuildProgress", + "description": "The app as the builder last left it, and turns since it last changed. Only the rebuild run\nwrites this record. An older project without one has no counted turns.", + "type": "object", + "properties": { + "fingerprint": { + "type": [ + "string", + "null" + ], + "default": null + }, + "unchanged_turns": { + "type": "integer", + "format": "uint32", + "default": 0, + "minimum": 0 + }, + "version": { + "type": "integer", + "format": "uint32", + "minimum": 0 + } + }, + "additionalProperties": false, + "required": [ + "version" + ] +} diff --git a/xtask/src/main.rs b/xtask/src/main.rs index b845db95..48449142 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -354,6 +354,7 @@ fn schemas(root: &Path, check: bool) -> bool { let dir = root.join("docs").join("schemas"); let files = [ ("craft.schema.json", craft_anything_engine::workspace::manifest_schema()), + ("build-progress.schema.json", craft_anything_engine::workspace::progress::build_progress_schema()), ("mechanics.schema.json", craft_anything_engine::mechanics::mechanics_schema()), ("self-test.schema.json", craft_anything_engine::self_test::self_test_schema()), ];