From ddc7beb8616d0138ed48a264c24931cd272d5254 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sat, 31 Oct 2026 20:55:46 -0400 Subject: [PATCH] Try again picks up where a build stopped instead of starting over A run with no message was read as a first build, so Try again studied the original again and gave the builder the first brief, even with its session saved. A run now carries on when the builder has already worked, and only runs the check when the builder had finished (awaiting_check in craft.json). A check that returns an error couldn't look at the build at all, so it gets a second go, and then the run says the build is finished but couldn't be checked. Co-Authored-By: Claude Opus 5.5 --- crates/ai/src/rebuild/mod.rs | 86 ++++++++++----- crates/ai/src/rebuild/tests.rs | 184 +++++++++++++++++++++++++++++++++ crates/engine/src/workspace.rs | 11 ++ crates/ui-egui/src/demo.rs | 1 + docs/ROADMAP.md | 10 ++ docs/pitfalls.md | 6 ++ docs/schemas/craft.schema.json | 4 + 7 files changed, 275 insertions(+), 27 deletions(-) diff --git a/crates/ai/src/rebuild/mod.rs b/crates/ai/src/rebuild/mod.rs index 06989d0a..87cb279a 100644 --- a/crates/ai/src/rebuild/mod.rs +++ b/crates/ai/src/rebuild/mod.rs @@ -125,7 +125,8 @@ pub struct RunOptions { pub shown: Option, /// Pick up a build whose last run stopped part-way (paused, the app closed, the computer /// stopped): Study isn't repeated when it was done, and an agent with a saved session is - /// told to carry on rather than given the brief again. + /// told to carry on rather than given the brief again. A run with no message on a project + /// whose builder has already worked does the same, so "Try again" never starts over. pub continuing: bool, /// Refinement rounds after the first build (each fixes what the last check found). pub max_rounds: u32, @@ -579,7 +580,8 @@ impl Worker { self.self_check = Some((kind.clone(), opts.good_enough)); let kind = kind.as_ref(); self.editor = self.find_editor(&ws.manifest.stack)?; - let first = (opts.message.is_none() && !opts.continuing) || !ws.capture_dir().exists(); + let carrying_on = opts.continuing || (opts.message.is_none() && ws.builder_started()); + let first = (opts.message.is_none() && !carrying_on) || !ws.capture_dir().exists(); if first { self.study(ws, kind)?; if let Some((name, ex)) = extractor { @@ -610,7 +612,7 @@ impl Worker { // from the brief with the request added. Some(m) if !first && resumable => follow_up(m, kind) + &now(), // Carrying on after a stop: the same, told to finish what it was doing. - None if opts.continuing && !first && has_history => continue_prompt(kind) + &now(), + None if carrying_on && !first && has_history => continue_prompt(kind) + &now(), _ => { let b = brief(kind, &cx, opts.message.as_deref()); if api_loop { format!("{API_BUILDER}\n\n{b}") } else { b } @@ -625,35 +627,47 @@ impl Worker { // Deep mode on a program: decompiling is required, so a build turn without any gets one reminder. let must_decompile = !self.engine.session().clean_room() && matches!(ws.manifest.target, TargetRef::Local { .. }); let mut reminded = false; + // The builder finished and only the check didn't: the build is there to check. + let mut only_check = carrying_on && !first && opts.message.is_none() && ws.manifest.awaiting_check; loop { - self.step(ws, Step::Build, Status::Building, if round == 0 { "Building the first version…" } else { "Improving the build…" })?; - let reply = self.turn(ws, agent.as_mut(), &prompt)?; - if !reply.trim().is_empty() { - ws.manifest.last_reply = reply.trim().to_string(); - } - // The stack is recorded as the one the build uses, once it shows: left to the builder, - // or another than the default because the request asked for it. A game engine stays - // (it says the project is a game, and its editor is found for it). - if game::Builder::from_stack(&ws.manifest.stack).is_none() - && let Some(s) = stacks::used_in(&ws.app_dir()) - { - ws.manifest.stack = s.name.clone(); - } - ws.save()?; - // What the person added while the agent worked goes in before anything else. - if let Some(p) = self.added(ws) { - prompt = p; - continue; - } - if must_decompile && !self.decompiled && !reminded { - reminded = true; - prompt = DECOMPILE_FIRST.to_string(); - continue; + if !std::mem::take(&mut only_check) { + let doing = match (round, carrying_on) { + (0, false) => "Building the first version…", + (0, true) => "Carrying on with the build…", + _ => "Improving the build…", + }; + self.step(ws, Step::Build, Status::Building, doing)?; + ws.manifest.awaiting_check = false; + let reply = self.turn(ws, agent.as_mut(), &prompt)?; + if !reply.trim().is_empty() { + ws.manifest.last_reply = reply.trim().to_string(); + } + // The stack is recorded as the one the build uses, once it shows: left to the builder, + // or another than the default because the request asked for it. A game engine stays + // (it says the project is a game, and its editor is found for it). + if game::Builder::from_stack(&ws.manifest.stack).is_none() + && let Some(s) = stacks::used_in(&ws.app_dir()) + { + ws.manifest.stack = s.name.clone(); + } + ws.manifest.awaiting_check = true; + ws.save()?; + // What the person added while the agent worked goes in before anything else. + if let Some(p) = self.added(ws) { + prompt = p; + continue; + } + if must_decompile && !self.decompiled && !reminded { + reminded = true; + prompt = DECOMPILE_FIRST.to_string(); + continue; + } } round += 1; self.step(ws, Step::Check, Status::Checking, "Checking the build against the original…")?; - let checks = self.run_checks(ws, kind, round)?; + let checks = self.checks_with_a_retry(ws, kind, round)?; ws.manifest.checks.extend(checks.iter().cloned()); + ws.manifest.awaiting_check = false; ws.save()?; for c in &checks { self.emit(RunEvent::Checked(c.clone())); @@ -723,6 +737,24 @@ impl Worker { } } + /// The round's checks. A check that fails says what's wrong with the build; one that returns + /// an error couldn't look at all (a browser that went away), which says nothing about the + /// build, so it gets a second go before the run stops, and the run says what is really wrong. + fn checks_with_a_retry(&self, ws: &Workspace, kind: &dyn kinds::TargetKind, round: u32) -> Result, ToolError> { + let e = match self.run_checks(ws, kind, round) { + Err(e) if e.code != ErrorCode::Cancelled => e, + done => return done, + }; + self.emit(RunEvent::Agent(AgentEvent::Status { text: format!("The check couldn't finish ({}). Trying it once more…", e.message) })); + self.run_checks(ws, kind, round).map_err(|e| { + if e.code == ErrorCode::Cancelled { + return e; + } + ToolError::new(e.code, format!("The build is finished, but it couldn't be checked: {}", e.message)) + .with_remediation("Press Try again to check it again. Nothing needs building again.") + }) + } + fn turn(&mut self, ws: &mut Workspace, agent: &mut dyn Conversation, prompt: &str) -> Result { let mut prompt = prompt.to_string(); let mut tries = 0u64; diff --git a/crates/ai/src/rebuild/tests.rs b/crates/ai/src/rebuild/tests.rs index 4e16c1de..748a01b8 100644 --- a/crates/ai/src/rebuild/tests.rs +++ b/crates/ai/src/rebuild/tests.rs @@ -751,6 +751,190 @@ fn clean_room_studies_nothing_inside_and_deep_mode_looks() { } /// A Worker for checks, on a session with Deep mode on or off. +/// A kind whose check can't run while `app/check-fails` still counts down, and a stand-in coding +/// agent that notes each prompt it gets in `prompts.txt` beside the project. +struct FlakyKind; +struct FlakyCheck; + +impl Check for FlakyCheck { + fn id(&self) -> &'static str { + "flaky" + } + fn run(&self, cx: &mut CheckCx) -> Result { + 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 { + std::fs::write(&file, (left - 1).to_string()).unwrap(); + return Err(ToolError::new(ErrorCode::EngineMissing, "the WebSocket is closed")); + } + Ok(CheckResult::new(self.id(), cx.round, true, "Fine.")) + } +} + +impl TargetKind for FlakyKind { + fn id(&self) -> &'static str { + "flaky" + } + fn words(&self) -> KindWords { + KindWords { button: "Open", icon: "box", ready: "Ready", change_hint: "Ask" } + } + fn detect(&self, _cx: &DetectCx) -> Option { + Some(1000) + } + fn default_stack(&self, _cx: &DetectCx) -> &'static str { + "Anything" + } + fn goal(&self) -> &'static str { + "a thing" + } + fn checks(&self, _ws: &Workspace) -> Vec> { + vec![Box::new(FlakyCheck)] + } +} + +#[cfg(unix)] +mod picking_up_a_build { + use super::*; + use std::os::unix::fs::PermissionsExt; + + struct Bench { + _dir: tempfile::TempDir, + hub: AiHub, + projects: PathBuf, + prompts: PathBuf, + } + + fn bench() -> Bench { + let dir = tempfile::tempdir().unwrap(); + let prompts = dir.path().join("prompts.txt"); + let agent = dir.path().join("agent.sh"); + std::fs::write(&agent, format!("#!/bin/sh\nfor last; do :; done\nprintf '%s\\n===\\n' \"$last\" >> '{}'\necho Done.\n", prompts.display())).unwrap(); + std::fs::set_permissions(&agent, std::fs::Permissions::from_mode(0o755)).unwrap(); + let engine = Arc::new(Engine::new(craft_anything_engine::testing::session())); + let hub = AiHub::with_stores(engine, dir.path().join("settings.toml"), crate::KeyStore::file_only(dir.path().join("keys.toml"))); + let mut cfg = hub.config(); + cfg.upsert(crate::BackendEntry { + config: crate::BackendConfig { + name: "Stand-in".into(), + kind: BackendKind::AgentCli, + model: "stand-in".into(), + base_url: None, + ssh_host: None, + enabled: true, + }, + options: crate::BackendOptions { command: Some(agent.display().to_string()), ..Default::default() }, + }); + hub.engine.session().settings.ai = cfg.to_table().unwrap(); + let projects = dir.path().join("projects"); + std::fs::create_dir_all(&projects).unwrap(); + Bench { _dir: dir, hub, projects, prompts } + } + + 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(); + let began = Instant::now(); + let mut ok = None; + while ok.is_none() { + assert!(began.elapsed() < Duration::from_secs(30), "the run never finished"); + match run.rx.recv_timeout(Duration::from_secs(30)) { + Ok(RunEvent::Finished { ok: o, .. }) => ok = Some(o), + Ok(_) => {} + Err(e) => panic!("the run went away: {e}"), + } + } + (ok.unwrap(), feed::load(&Workspace::open(&ws.root).unwrap())) + } + + fn prompts_given(b: &Bench) -> usize { + std::fs::read_to_string(&b.prompts).unwrap_or_default().matches("===").count() + } + + fn studied(feed: &[feed::FeedItem]) -> bool { + feed.iter().any(|i| matches!(i, feed::FeedItem::Step { step: Step::Study, .. })) + } + + /// A project whose builder has worked: its session and its study are there. + fn worked_on(b: &Bench) -> Workspace { + let mut ws = Workspace::create(&b.projects, "x", "A thing", TargetRef::None).unwrap(); + ws.manifest.agent_session = Some("session-1".into()); + ws.manifest.stack = "Anything".into(); + ws.save().unwrap(); + std::fs::create_dir_all(ws.capture_dir()).unwrap(); + std::fs::create_dir_all(ws.app_dir()).unwrap(); + ws + } + + #[test] + fn try_again_after_a_check_that_could_not_run_only_checks() { + let b = bench(); + let mut ws = worked_on(&b); + ws.manifest.awaiting_check = true; + ws.save().unwrap(); + let (ok, feed) = try_it(&b, &ws); + assert!(ok, "{feed:?}"); + assert_eq!(prompts_given(&b), 0, "the builder had finished: it isn't asked again"); + assert!(!studied(&feed), "the original isn't studied again"); + let ws = Workspace::open(&ws.root).unwrap(); + assert!(!ws.manifest.awaiting_check); + assert_eq!(ws.manifest.checks.len(), 1); + } + + #[test] + fn try_again_after_the_builder_stopped_carries_on_instead_of_starting_over() { + let b = bench(); + let ws = worked_on(&b); + let (ok, feed) = try_it(&b, &ws); + assert!(ok, "{feed:?}"); + assert!(!studied(&feed), "the original isn't studied again"); + let told = std::fs::read_to_string(&b.prompts).unwrap(); + assert!(told.contains("Pick up where you left off"), "{told}"); + assert!(!told.contains("Their request"), "not the first-build brief again: {told}"); + } + + #[test] + fn a_new_project_still_starts_from_the_top() { + let b = bench(); + let ws = Workspace::create(&b.projects, "x", "A thing", TargetRef::None).unwrap(); + let (ok, feed) = try_it(&b, &ws); + assert!(ok, "{feed:?}"); + let told = std::fs::read_to_string(&b.prompts).unwrap(); + assert!(!told.contains("Pick up where you left off"), "{told}"); + } + + #[test] + fn a_check_that_could_not_run_gets_a_second_go_and_the_build_is_not_blamed() { + let b = bench(); + let mut ws = worked_on(&b); + std::fs::write(ws.app_dir().join("check-fails"), "1").unwrap(); + ws.manifest.awaiting_check = true; + ws.save().unwrap(); + let (ok, _) = try_it(&b, &ws); + assert!(ok, "one lost browser doesn't fail a finished build"); + + // It keeps failing: the run stops, says the build is fine, and Try again only checks. + let mut ws = worked_on(&b); + ws.manifest.name = "y".into(); + ws.manifest.awaiting_check = false; + ws.save().unwrap(); + std::fs::write(ws.app_dir().join("check-fails"), "2").unwrap(); + let (ok, feed) = try_it(&b, &ws); + assert!(!ok); + 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:?}" + ); + let asked = prompts_given(&b); + assert!(Workspace::open(&ws.root).unwrap().manifest.awaiting_check, "the builder's turn is on record"); + let (ok, _) = try_it(&b, &ws); + assert!(ok); + assert_eq!(prompts_given(&b), asked, "and it isn't asked to build again"); + } +} + fn check_worker(ws: &Workspace, deep: bool) -> Worker { let mut s = craft_anything_engine::testing::session(); s.settings.deep_mode = deep; diff --git a/crates/engine/src/workspace.rs b/crates/engine/src/workspace.rs index 446dd771..021e5317 100644 --- a/crates/engine/src/workspace.rs +++ b/crates/engine/src/workspace.rs @@ -390,6 +390,10 @@ pub struct Manifest { /// (`Settings::questions`). The person's, like the budget. #[serde(default, skip_serializing_if = "Option::is_none")] pub questions: Option, + /// The builder finished its turn and the run's checks haven't reported on it yet, so a run + /// that picks this up only needs to check: nothing is left to build. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub awaiting_check: bool, } impl Manifest { @@ -415,6 +419,7 @@ impl Manifest { start_files: Vec::new(), builder: None, questions: None, + awaiting_check: false, } } @@ -817,6 +822,12 @@ impl Workspace { self.root.join("inbox") } /// The API builder's conversation, kept so a follow-up continues it (`ai::context`). + /// A builder has worked on this project and its conversation was kept, so a later run carries + /// it on instead of starting over. + pub fn builder_started(&self) -> bool { + self.manifest.agent_session.is_some() || self.transcript_path().is_file() + } + pub fn transcript_path(&self) -> PathBuf { self.root.join("transcript.json") } diff --git a/crates/ui-egui/src/demo.rs b/crates/ui-egui/src/demo.rs index b19463f6..6aacc1b5 100644 --- a/crates/ui-egui/src/demo.rs +++ b/crates/ui-egui/src/demo.rs @@ -237,6 +237,7 @@ fn workspace(slug: &str, name: &str, request: &str, target: TargetRef, status: S start_files: Vec::new(), builder: None, questions: None, + awaiting_check: false, }, } } diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 4dbc7b0e..0e81374f 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -777,6 +777,16 @@ Gaps found while using the app for real projects. that couldn't start, panics - [ ] Sometimes starting the app shows nothing: find why from `desktop.log` (a GOG Galaxy Vulkan overlay layer loads into the app; try DX12 first on Windows if that's it) +- [x] Try again picks up where a run stopped instead of starting over: a project whose builder + has worked isn't studied again or given the first brief; when the builder had finished and + only the check didn't (`awaiting_check` in craft.json), only the check runs. A check that + returns an error gets a second go, and then the run says the build is finished but couldn't + be checked +- [ ] Find why a site capture lost its browser ("the WebSocket is closed", 419 ms into + `web_site_capture` for a second page, seen once; the same capture passed 41 s earlier) +- [ ] Two engine tests fail on a Mac, on `main` too: `real_binary_end_to_end_if_present` expects + `/bin/ls` to be Linux, and `auto_mode_keeps_going_but_never_does_the_dangerous_things` is + refused `rm -rf` in a temp folder under `/var/folders` ### R2.10 Pinned builds Owner: maintainers · Status: in progress diff --git a/docs/pitfalls.md b/docs/pitfalls.md index c62fcb63..ad6a6397 100644 --- a/docs/pitfalls.md +++ b/docs/pitfalls.md @@ -314,6 +314,12 @@ computer. Say what ran and what it printed, write the rest to a log the person c agent run does: `crate::failure_log`), and never name a next step you don't spell out. Exit 255 from ssh is ssh's own failure: don't blame the remote program. Test a message by causing the failure. +**A retry that took "no message" for "brand new".** A run with no message and no `continuing` +flag was read as a first build, and Try again starts exactly that run. A browser that died during +the final check failed a finished build; Try again then studied the original again and gave the +builder the first brief. A run decides what's left from what the project records, not from which +button started it, and a check that couldn't look says nothing about the build. + **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/craft.schema.json b/docs/schemas/craft.schema.json index 5e525884..63a74a57 100644 --- a/docs/schemas/craft.schema.json +++ b/docs/schemas/craft.schema.json @@ -30,6 +30,10 @@ "format": "double", "default": 5.0 }, + "awaiting_check": { + "description": "The builder finished its turn and the run's checks haven't reported on it yet, so a run\nthat picks this up only needs to check: nothing is left to build.", + "type": "boolean" + }, "builder": { "description": "The AI the person chose to build this project (a backend's name); none: the default\nbuilder. The person's, like the budget.", "type": [