diff --git a/app/src-tauri/src/commands/project_commands.rs b/app/src-tauri/src/commands/project_commands.rs index 0a79fe0..971d5b0 100644 --- a/app/src-tauri/src/commands/project_commands.rs +++ b/app/src-tauri/src/commands/project_commands.rs @@ -1494,7 +1494,7 @@ pub async fn stop_project_container( wip: Option, app_handle: tauri::AppHandle, state: State<'_, AppState>, -) -> Result<(), String> { +) -> Result, String> { // Stop had **no** exclusion at all, which is the one gap CLAUDE.md's "every // command that stops, removes or recreates the container consults // `is_migrating`" rule already named and this function did not honour. A @@ -1508,25 +1508,37 @@ pub async fn stop_project_container( .get(&project_id) .ok_or_else(|| format!("Project {} not found", project_id))?; + // The stop guard asked, and meanwhile the workspace stopped by itself + // (idle stop, another window): what the user wanted has happened. + if project.is_aws() && wip.is_some() && project.status == ProjectStatus::Stopped { + return Ok(None); + } + state.projects_store.update_status(&project_id, ProjectStatus::Stopping)?; + // The stop guard's choice, before anything else is torn down: a push that + // fails aborts the stop with the project as it was (auth bridge included). + let mut note = None; + if project.is_aws() { + if wip == Some(WipChoice::Push) { + emit_progress(&app_handle, &project_id, "Pushing unsaved work..."); + } + let target = ContainerTarget::for_project(&project); + note = aws_guard_before_stop( + &state.projects_store, + &project_id, + project.status.clone(), + target.as_ref(), + wip, + super::stop_guard::PUSH_TIMEOUT, + ) + .await?; + } + // Drop host listeners first: they only make sense while the container runs. state.auth_bridge.stop(&project_id).await; if project.is_aws() { - // The stop guard's choice: a push that fails aborts the stop, and - // the project keeps its status (the push output says why). - if wip == Some(WipChoice::Push) { - emit_progress(&app_handle, &project_id, "Pushing unsaved work..."); - let target = ContainerTarget::for_project(&project); - if let Err(e) = - super::stop_guard::before_stop(target.as_ref(), wip, super::stop_guard::PUSH_TIMEOUT).await - { - log::warn!("Project {}: WIP push before stop failed: {}", project_id, e); - let _ = state.projects_store.update_status(&project_id, project.status.clone()); - return Err(e); - } - } // A failed StopTask leaves a billed task running, so unlike Docker it // is not papered over: the project keeps its status and the user // sees why (expired SSO, typically), and can retry. @@ -1560,7 +1572,27 @@ pub async fn stop_project_container( } state.projects_store.update_status(&project_id, ProjectStatus::Stopped)?; - Ok(()) + Ok(note) +} + +/// The AWS stop guard's step before StopTask ([`super::stop_guard::prepare_stop`]). +/// A failed push puts `previous` back, so the project is as it was. +async fn aws_guard_before_stop( + store: &crate::storage::projects_store::ProjectsStore, + project_id: &str, + previous: ProjectStatus, + runner: Option<&R>, + wip: Option, + push_timeout: std::time::Duration, +) -> Result, String> { + match super::stop_guard::prepare_stop(runner, wip, push_timeout).await { + Ok(note) => Ok(note), + Err(e) => { + log::warn!("Project {}: WIP push before stop failed: {}", project_id, e); + let _ = store.update_status(project_id, previous); + Err(e) + } + } } /// The stop guard's question: what would stopping this AWS workspace lose? @@ -1735,15 +1767,19 @@ pub async fn rebuild_project_container( /// whose container was removed mid-migration reports Stopped. /// Removing an AWS project whose connection was deleted can't reach AWS: /// refused unless the user confirmed leaving its resources behind. +/// Stable prefix of that refusal, so the UI can offer the second confirm +/// without matching prose. Mirrored by `AWS_CONNECTION_MISSING` in `lib/types.ts`. +pub const AWS_CONNECTION_MISSING: &str = "AWS_CONNECTION_MISSING: "; + fn missing_connection_removal(forget_aws_resources: bool) -> Result<(), String> { if forget_aws_resources { return Ok(()); } - Err("This project's AWS connection no longer exists, so its task (which may still be \ + Err(AWS_CONNECTION_MISSING.to_string() + + "This project's AWS connection no longer exists, so its task (which may still be \ running and billed), secret and data can't be removed from here. Add the connection \ again in Settings → Hosts and remove the project, or confirm removing it while leaving \ - those resources in AWS." - .into()) + those resources in AWS.") } /// After a failed start: an AWS project whose task ARN is still stored has @@ -2612,7 +2648,54 @@ mod tests { fn removing_without_the_connection_needs_explicit_confirmation() { let e = missing_connection_removal(false).unwrap_err(); assert!(e.contains("still be running") && e.contains("Settings → Hosts"), "{e}"); + assert!(e.starts_with(AWS_CONNECTION_MISSING)); assert!(missing_connection_removal(true).is_ok()); + // The frontend keys its second confirm on the same prefix. + let ts = include_str!("../../../src/lib/types.ts"); + assert!( + ts.contains(&format!("export const AWS_CONNECTION_MISSING = {:?};", AWS_CONNECTION_MISSING)), + "lib/types.ts must mirror AWS_CONNECTION_MISSING" + ); + } + + fn stop_store(status: ProjectStatus) -> (tempfile::TempDir, crate::storage::projects_store::ProjectsStore, String) { + let mut p = aws_project(); + p.status = status; + let dir = tempfile::tempdir().unwrap(); + let store = crate::storage::projects_store::ProjectsStore::over_file_for_test( + dir.path().join("projects.json"), + vec![p.clone()], + ); + (dir, store, p.id) + } + + #[tokio::test] + async fn a_failed_push_before_stop_puts_the_status_back_and_records_nothing() { + use crate::commands::stop_guard::tests::Fake; + let (_d, store, id) = stop_store(ProjectStatus::Running); + store.update_status(&id, ProjectStatus::Stopping).unwrap(); + let fake = Fake::new(Ok(("push to origin failed: denied".into(), 1))); + let e = aws_guard_before_stop(&store, &id, ProjectStatus::Running, Some(&fake), Some(WipChoice::Push), std::time::Duration::from_secs(5)) + .await + .unwrap_err(); + assert!(e.contains("denied"), "{e}"); + assert_eq!(store.get(&id).unwrap().status, ProjectStatus::Running); + assert!(fake.markers.lock().unwrap().is_empty()); + } + + #[tokio::test] + async fn a_successful_push_before_stop_records_it_and_leaves_the_stop_going() { + use crate::commands::stop_guard::tests::Fake; + let (_d, store, id) = stop_store(ProjectStatus::Running); + store.update_status(&id, ProjectStatus::Stopping).unwrap(); + let fake = Fake::new(Ok(("pushed".into(), 0))); + let note = aws_guard_before_stop(&store, &id, ProjectStatus::Running, Some(&fake), Some(WipChoice::Push), std::time::Duration::from_secs(5)) + .await + .unwrap(); + assert_eq!(note, None); + assert_eq!(store.get(&id).unwrap().status, ProjectStatus::Stopping, "untouched: the stop goes on"); + assert_eq!(*fake.markers.lock().unwrap(), vec!["pushed".to_string()]); + assert_eq!(fake.calls.lock().unwrap().len(), 1); } #[test] diff --git a/app/src-tauri/src/commands/stop_guard.rs b/app/src-tauri/src/commands/stop_guard.rs index 14725e6..6d22c1e 100644 --- a/app/src-tauri/src/commands/stop_guard.rs +++ b/app/src-tauri/src/commands/stop_guard.rs @@ -20,9 +20,20 @@ pub const PUSH_TIMEOUT: Duration = Duration::from_secs(300); /// Cap on the push output carried into an error. const MAX_OUTPUT: usize = 4000; -/// Runs the WIP script in the workspace: `(combined output, exit code)`. +/// Where the entrypoint's stop hook looks for the app's decision (Task 7's +/// file, which the idle watcher also writes `idle` to). +pub const STOP_REASON_FILE: &str = "/run/triple-c/stop-reason"; +/// Recorded after the guard's push succeeded: the hook does not push again. +pub const MARKER_PUSHED: &str = "pushed"; +/// Recorded for "Stop without pushing": the hook does not push. +pub const MARKER_DISCARD: &str = "discard"; + +/// The workspace, as the stop guard uses it. pub trait WipRunner: Sync { + /// Run the WIP script as `claude`: `(combined output, exit code)`. fn run(&self, args: Vec) -> impl Future> + Send; + /// Record `marker` in [`STOP_REASON_FILE`] (as root: the directory is root's). + fn write_stop_reason(&self, marker: &str) -> impl Future> + Send; } impl WipRunner for ContainerTarget { @@ -31,6 +42,31 @@ impl WipRunner for ContainerTarget { cmd.extend(args); self.exec_oneshot_as("claude", cmd, Vec::new()).await } + + async fn write_stop_reason(&self, marker: &str) -> Result<(), String> { + let (out, code) = self + .exec_oneshot_as("root", stop_reason_command(marker), Vec::new()) + .await?; + if code == 0 { + Ok(()) + } else { + Err(format!("exit {code}: {}", cap(&out))) + } + } +} + +/// The root command that records `marker`; the value travels as `$1`, never +/// inside the script. +pub fn stop_reason_command(marker: &str) -> Vec { + vec![ + "sh".into(), + "-c".into(), + format!( + "umask 077 && mkdir -p /run/triple-c && chmod 700 /run/triple-c && printf '%s\\n' \"$1\" > {STOP_REASON_FILE}" + ), + "sh".into(), + marker.into(), + ] } /// The repository lines of a dry run. Lines that are not JSON objects (a @@ -47,13 +83,6 @@ pub fn parse_dry_run(output: &str) -> Result, String> { .collect() } -/// Whether anything in `work` would be lost by a stop (or could not be checked). -/// The frontend applies the same rule (`stopGuardNeeded`). -#[cfg(test)] -pub fn needs_guard(work: &UnsavedWork) -> bool { - !work.checked || work.repos.iter().any(|r| r.dirty || r.unpushed > 0) -} - fn unchecked(reason: String) -> UnsavedWork { UnsavedWork { checked: false, repos: Vec::new(), error: Some(reason) } } @@ -125,20 +154,61 @@ pub async fn before_stop( } } +/// The note a stop returns when the hook's own push may still run. +pub const MARKER_NOT_RECORDED: &str = "Triple-C could not tell the workspace what you chose, so its own \ + push of work in progress on shutdown may still run (if it is turned on)."; + +/// Everything the guard does before StopTask: `Push` pushes (a failure is an +/// `Err` and the stop must not happen), then records `pushed`; `Discard` +/// records `discard`; no choice does nothing. A marker that can't be +/// written does not stop the stop: `Ok(Some(note))` says the hook may push. +pub async fn prepare_stop( + runner: Option<&R>, + choice: Option, + push_timeout: Duration, +) -> Result, String> { + let marker = match choice { + None => return Ok(None), + Some(WipChoice::Push) => { + before_stop(runner, choice, push_timeout).await?; + MARKER_PUSHED + } + Some(WipChoice::Discard) => MARKER_DISCARD, + }; + let written = match runner { + Some(r) => tokio::time::timeout(DRY_RUN_TIMEOUT, r.write_stop_reason(marker)) + .await + .unwrap_or_else(|_| Err("timed out".into())), + None => Err("not connected to the workspace".into()), + }; + match written { + Ok(()) => Ok(None), + Err(e) => { + log::warn!("Could not record the stop guard's choice ({marker}): {e}"); + Ok(Some(MARKER_NOT_RECORDED.to_string())) + } + } +} + #[cfg(test)] -mod tests { +pub(crate) mod tests { use super::*; use std::sync::Mutex; - struct Fake { - reply: Result<(String, i64), String>, - calls: Mutex>>, - hang: bool, + pub(crate) struct Fake { + pub reply: Result<(String, i64), String>, + pub calls: Mutex>>, + pub hang: bool, + pub markers: Mutex>, + pub marker_fails: bool, } impl Fake { - fn new(reply: Result<(String, i64), String>) -> Self { - Fake { reply, calls: Mutex::new(Vec::new()), hang: false } + pub fn new(reply: Result<(String, i64), String>) -> Self { + Fake { reply, calls: Mutex::new(Vec::new()), hang: false, markers: Mutex::new(Vec::new()), marker_fails: false } + } + fn markers(&self) -> Vec { + self.markers.lock().unwrap().clone() } fn calls(&self) -> Vec> { self.calls.lock().unwrap().clone() @@ -153,6 +223,13 @@ mod tests { } self.reply.clone() } + async fn write_stop_reason(&self, marker: &str) -> Result<(), String> { + if self.marker_fails { + return Err("agent unreachable".into()); + } + self.markers.lock().unwrap().push(marker.to_string()); + Ok(()) + } } const T: Duration = Duration::from_secs(5); @@ -175,7 +252,6 @@ mod tests { let w = unsaved_work(Some(&fake), T).await; assert!(w.checked && w.error.is_none()); assert_eq!(w.repos.len(), 1); - assert!(!needs_guard(&w)); assert_eq!(fake.calls(), vec![vec!["--dry-run".to_string()]]); } @@ -184,7 +260,6 @@ mod tests { let w = unsaved_work::(None, T).await; assert!(!w.checked); assert!(w.error.as_deref().unwrap().contains("not connected")); - assert!(needs_guard(&w)); let failing = Fake::new(Err("connection refused".into())); let w = unsaved_work(Some(&failing), T).await; @@ -203,15 +278,6 @@ mod tests { assert!(!w.checked && w.error.unwrap().contains("in time")); } - #[test] - fn dirty_or_unpushed_work_needs_the_guard() { - let repo = |dirty, unpushed| RepoState { folder: "x".into(), dirty, unpushed, branch: "b".into() }; - let w = |r| UnsavedWork { checked: true, repos: vec![r], error: None }; - assert!(needs_guard(&w(repo(true, 0)))); - assert!(needs_guard(&w(repo(false, 1)))); - assert!(!needs_guard(&w(repo(false, 0)))); - } - #[tokio::test] async fn push_runs_the_script_with_reason_stop() { let fake = Fake::new(Ok(("pushed".into(), 0))); @@ -246,4 +312,47 @@ mod tests { let c = cap(&s); assert!(c.ends_with("END") && c.len() < 4100); } + + #[tokio::test] + async fn a_successful_push_records_pushed_and_stop_without_pushing_records_discard() { + let fake = Fake::new(Ok(("pushed".into(), 0))); + assert_eq!(prepare_stop(Some(&fake), Some(WipChoice::Push), T).await.unwrap(), None); + assert_eq!(fake.markers(), vec![MARKER_PUSHED]); + + let fake = Fake::new(Err("must not run".into())); + assert_eq!(prepare_stop(Some(&fake), Some(WipChoice::Discard), T).await.unwrap(), None); + assert!(fake.calls().is_empty()); + assert_eq!(fake.markers(), vec![MARKER_DISCARD]); + + assert_eq!(prepare_stop(Some(&fake), None, T).await.unwrap(), None); + assert_eq!(fake.markers().len(), 1, "no choice records nothing"); + } + + #[tokio::test] + async fn a_failed_push_records_nothing() { + let fake = Fake::new(Ok(("denied".into(), 1))); + assert!(prepare_stop(Some(&fake), Some(WipChoice::Push), T).await.is_err()); + assert!(fake.markers().is_empty()); + } + + #[tokio::test] + async fn an_unwritable_marker_still_stops_and_says_the_hook_may_push() { + let mut fake = Fake::new(Ok((String::new(), 0))); + fake.marker_fails = true; + let note = prepare_stop(Some(&fake), Some(WipChoice::Discard), T).await.unwrap(); + assert!(note.unwrap().contains("may still run")); + let note = prepare_stop::(None, Some(WipChoice::Discard), T).await.unwrap(); + assert!(note.is_some()); + } + + #[test] + fn the_marker_is_written_as_an_argument_to_the_file_the_hook_reads() { + let cmd = stop_reason_command("pushed"); + assert_eq!(&cmd[..2], &["sh".to_string(), "-c".to_string()]); + assert!(cmd[2].contains("> /run/triple-c/stop-reason") && cmd[2].contains("\"$1\"")); + assert_eq!(cmd.last().unwrap(), "pushed"); + // The entrypoint matches exactly these values. + let ep = include_str!("../../../../container/entrypoint.sh"); + assert!(ep.contains("pushed|discard)") && ep.contains(STOP_REASON_FILE)); + } } diff --git a/app/src-tauri/src/docker/aws_mode_tests.rs b/app/src-tauri/src/docker/aws_mode_tests.rs index 5a79efa..de6238e 100644 --- a/app/src-tauri/src/docker/aws_mode_tests.rs +++ b/app/src-tauri/src/docker/aws_mode_tests.rs @@ -875,6 +875,34 @@ fn aws_stop_runs_the_wip_push_with_the_recorded_reason_once() { assert!(!log.exists(), "WIP push off"); } +#[test] +fn aws_stop_skips_its_push_when_the_app_already_pushed_or_the_user_declined() { + let tmp = tempfile::tempdir().unwrap(); + let log = tmp.path().join("calls"); + let fake = write_script( + tmp.path(), + "triple-c-wip-push", + &format!("#!/bin/bash\necho \"$*\" >> '{}'\n", log.display()), + ); + let reason = tmp.path().join("reason"); + let blocks = aws_blocks(&["aws_env", "aws_stop"]) + .replace("/usr/local/bin/triple-c-wip-push", fake.to_str().unwrap()) + .replace("/run/triple-c/stop-reason", reason.to_str().unwrap()); + let script = format!("{blocks}\naws_before_stop\n"); + for marker in ["pushed", "discard"] { + fs::write(&reason, format!("{marker}\n")).unwrap(); + let out = bash(&script, &[], &[("TRIPLEC_HOST", "aws"), ("TRIPLEC_WIP_PUSH", "1")], &[]); + assert!(out.status.success(), "{}", text(&out)); + assert!(!log.exists(), "{marker}: no second push"); + assert!(text(&out).contains(&format!("already handled by Triple-C ({marker})")), "{}", text(&out)); + } + // Anything else is not a marker the app writes: the push still runs. + fs::write(&reason, "something-else\n").unwrap(); + let out = bash(&script, &[], &[("TRIPLEC_HOST", "aws"), ("TRIPLEC_WIP_PUSH", "1")], &[]); + assert!(out.status.success(), "{}", text(&out)); + assert_eq!(fs::read_to_string(&log).unwrap(), "--reason sigterm\n"); +} + #[test] fn docker_mode_never_reaches_the_aws_blocks() { // Every new step sits behind TRIPLEC_HOST=aws; the scripts are only diff --git a/app/src-tauri/src/models/project.rs b/app/src-tauri/src/models/project.rs index 0891f3e..a1f015a 100644 --- a/app/src-tauri/src/models/project.rs +++ b/app/src-tauri/src/models/project.rs @@ -557,18 +557,20 @@ impl Default for AwsWorkspaceConfig { } } -/// The valid Fargate memory range (min, max, step) in MiB for a vCPU size. -fn fargate_memory_range(cpu: u32) -> Option<(u32, u32, u32)> { - match cpu { - 256 => Some((512, 2048, 512)), - 512 => Some((1024, 4096, 1024)), - 1024 => Some((2048, 8192, 1024)), - 2048 => Some((4096, 16384, 1024)), - 4096 => Some((8192, 30720, 1024)), - 8192 => Some((16384, 61440, 4096)), - 16384 => Some((32768, 122880, 8192)), - _ => None, - } +/// The valid Fargate memory sizes (MiB) for a CPU size, smallest first. +/// 256 CPU is the irregular one: 512, 1024 or 2048 only. +fn fargate_memory_sizes(cpu: u32) -> Option> { + let (min, max, step) = match cpu { + 256 => return Some(vec![512, 1024, 2048]), + 512 => (1024, 4096, 1024), + 1024 => (2048, 8192, 1024), + 2048 => (4096, 16384, 1024), + 4096 => (8192, 30720, 1024), + 8192 => (16384, 61440, 4096), + 16384 => (32768, 122880, 8192), + _ => return None, + }; + Some((min..=max).step_by(step as usize).collect()) } fn validate_repo_url(url: &str) -> Result<(), String> { @@ -643,17 +645,29 @@ fn validate_repo_folder(folder: &str) -> Result<(), String> { } } +/// "512, 1024 or 2048 MiB", or "2048 to 8192 MiB in steps of 1024". +fn describe_sizes(sizes: &[u32]) -> String { + let (first, last) = (sizes[0], sizes[sizes.len() - 1]); + if sizes.len() <= 3 { + let head: Vec = sizes[..sizes.len() - 1].iter().map(u32::to_string).collect(); + return format!("{} or {} MiB", head.join(", "), last); + } + format!("{} to {} MiB in steps of {}", first, last, sizes[1] - sizes[0]) +} + pub fn validate_aws_workspace(cfg: &AwsWorkspaceConfig) -> Result<(), String> { - let (min, max, step) = fargate_memory_range(cfg.cpu).ok_or_else(|| { + let sizes = fargate_memory_sizes(cfg.cpu).ok_or_else(|| { format!( "{} is not a Fargate CPU size (256, 512, 1024, 2048, 4096, 8192 or 16384)", cfg.cpu ) })?; - if cfg.memory_mib < min || cfg.memory_mib > max || (cfg.memory_mib - min) % step != 0 { + if !sizes.contains(&cfg.memory_mib) { return Err(format!( - "{} MiB of memory is not valid with {} CPU units: use {} to {} MiB in steps of {}", - cfg.memory_mib, cfg.cpu, min, max, step + "{} MiB of memory is not valid with {} CPU units: use {}", + cfg.memory_mib, + cfg.cpu, + describe_sizes(&sizes) )); } if !(21..=200).contains(&cfg.ephemeral_gib) { @@ -1323,12 +1337,27 @@ mod tests { #[test] fn cpu_and_memory_must_be_a_fargate_pair() { - for (c, m) in [(256, 512), (256, 2048), (1024, 2048), (1024, 8192), (2048, 4096), (4096, 30720), (8192, 61440), (16384, 32768), (16384, 122880)] { + // Shared with the frontend's copy (`lib/awsWorkspace.test.ts`). + let fixture: serde_json::Value = + serde_json::from_str(include_str!("../../../src/test/fixtures/fargate-pairs.json")).unwrap(); + let pairs = |k: &str| -> Vec<(u32, u32)> { + fixture[k] + .as_array() + .unwrap() + .iter() + .map(|p| (p[0].as_u64().unwrap() as u32, p[1].as_u64().unwrap() as u32)) + .collect() + }; + for (c, m) in pairs("valid") { assert!(validate_aws_workspace(&cfg(c, m)).is_ok(), "{c}/{m} should be valid"); } - for (c, m) in [(256, 4096), (1024, 1024), (1024, 9216), (1024, 2500), (300, 1024), (16384, 114688 + 1), (8192, 18432)] { + for (c, m) in pairs("invalid") { assert!(validate_aws_workspace(&cfg(c, m)).is_err(), "{c}/{m} should be rejected"); } + let e = validate_aws_workspace(&cfg(256, 1536)).unwrap_err(); + assert!(e.ends_with("use 512, 1024 or 2048 MiB"), "{e}"); + let e = validate_aws_workspace(&cfg(1024, 2500)).unwrap_err(); + assert!(e.ends_with("use 2048 to 8192 MiB in steps of 1024"), "{e}"); } #[test] diff --git a/app/src/components/projects/AddProjectDialog.test.tsx b/app/src/components/projects/AddProjectDialog.test.tsx index b9ba61a..f449cff 100644 --- a/app/src/components/projects/AddProjectDialog.test.tsx +++ b/app/src/components/projects/AddProjectDialog.test.tsx @@ -214,4 +214,23 @@ describe("AddProjectDialog on AWS", () => { fireEvent.click(submitButton()); expect(await screen.findByRole("alert")).toHaveTextContent(/use https:\/\//); }); + + it("forgets the environment when the region changes until that region's scan is done", async () => { + detectAwsEnvironments.mockResolvedValueOnce(scan([environment("dev", "ready", true)])); + let finish!: (v: EnvironmentScan) => void; + detectAwsEnvironments.mockReturnValueOnce(new Promise((r) => (finish = r))); + render(); + chooseAws(); + await waitFor(() => expect(screen.getByLabelText("Environment")).toHaveValue("dev")); + fireEvent.change(screen.getByLabelText("Project name"), { target: { value: "api" } }); + const region = screen.getByLabelText("Region"); + fireEvent.change(region, { target: { value: "us-east-1" } }); + fireEvent.blur(region); + expect(await screen.findByText(/Looking for environments in us-east-1/)).toBeInTheDocument(); + fireEvent.click(submitButton()); + expect(await screen.findByRole("alert")).toHaveTextContent(/Choose an environment/); + expect(add).not.toHaveBeenCalled(); + await act(async () => finish(scan([environment("prod", "ready", true)]))); + expect(screen.getByLabelText("Environment")).toHaveValue("prod"); + }); }); diff --git a/app/src/components/projects/AddProjectDialog.tsx b/app/src/components/projects/AddProjectDialog.tsx index a306ac2..665ba17 100644 --- a/app/src/components/projects/AddProjectDialog.tsx +++ b/app/src/components/projects/AddProjectDialog.tsx @@ -66,6 +66,8 @@ export default function AddProjectDialog({ onClose }: Props) { return; } let cancelled = false; + // An environment chosen in another region is not one in this region. + setEnvironment(""); setScan({ status: "loading" }); detectAwsEnvironments(connection.id, scanRegion).then( (found) => { @@ -126,7 +128,13 @@ export default function AddProjectDialog({ onClose }: Props) { return; } if (connection) { - if (!environment) { + // Only an environment from a finished scan of the region shown counts. + if (region.trim() !== scanRegion) { + setScanRegion(region.trim()); + setError("Choose an environment in the new region first"); + return; + } + if (scan.status !== "done" || !environment) { setError("Choose an environment to run the project in"); return; } diff --git a/app/src/components/projects/ConfirmRemoveModal.test.tsx b/app/src/components/projects/ConfirmRemoveModal.test.tsx index 4a4d7cb..6baf53b 100644 --- a/app/src/components/projects/ConfirmRemoveModal.test.tsx +++ b/app/src/components/projects/ConfirmRemoveModal.test.tsx @@ -5,7 +5,7 @@ import ConfirmRemoveModal, { ConfirmForgetAwsModal } from "./ConfirmRemoveModal" describe("ConfirmRemoveModal", () => { it("keeps an AWS workspace's saved files unless asked", () => { const onConfirm = vi.fn(); - render(); + render(); const box = screen.getByRole("checkbox", { name: /saved files \(EFS\)/ }); expect(box).not.toBeChecked(); fireEvent.click(screen.getByRole("button", { name: "Remove" })); @@ -14,6 +14,14 @@ describe("ConfirmRemoveModal", () => { expect(onConfirm.mock.calls).toEqual([[{ discardData: false }], [{ discardData: true }]]); }); + it("offers no EFS choice for an Ephemeral AWS workspace", () => { + const onConfirm = vi.fn(); + render(); + expect(screen.queryByRole("checkbox")).toBeNull(); + fireEvent.click(screen.getByRole("button", { name: "Remove" })); + expect(onConfirm).toHaveBeenCalledWith({ discardData: false }); + }); + it("offers no EFS choice for a local project", () => { render(); expect(screen.queryByRole("checkbox")).toBeNull(); diff --git a/app/src/components/projects/ConfirmRemoveModal.tsx b/app/src/components/projects/ConfirmRemoveModal.tsx index f081606..b8fe247 100644 --- a/app/src/components/projects/ConfirmRemoveModal.tsx +++ b/app/src/components/projects/ConfirmRemoveModal.tsx @@ -6,6 +6,8 @@ interface Props { projectName: string; /** An AWS project: its task, task definitions and secret go instead of Docker state. */ aws?: boolean; + /** AWS: the workspace may have saved files on EFS (offers deleting them). */ + savedFiles?: boolean; onConfirm: (opts: { discardData: boolean }) => void; onCancel: () => void; } @@ -13,7 +15,13 @@ interface Props { const REMOVE_CLASS = "bg-[var(--error-emphasis)] text-white border border-transparent hover:opacity-90"; -export default function ConfirmRemoveModal({ projectName, aws = false, onConfirm, onCancel }: Props) { +export default function ConfirmRemoveModal({ + projectName, + aws = false, + savedFiles = false, + onConfirm, + onCancel, +}: Props) { const [discardData, setDiscardData] = useState(false); return ( Cancel - @@ -40,21 +52,23 @@ export default function ConfirmRemoveModal({ projectName, aws = false, onConfirm deleted after AWS’s 7-day recovery window). Work that was not pushed to a remote is lost.

- + + )} ) : ( <> diff --git a/app/src/components/projects/ProjectRow.tsx b/app/src/components/projects/ProjectRow.tsx index f6eef6e..7fc7a11 100644 --- a/app/src/components/projects/ProjectRow.tsx +++ b/app/src/components/projects/ProjectRow.tsx @@ -162,7 +162,12 @@ export default function ProjectRow({ project }: Props) { {terminal.reasonNode} {stopGuard && ( - + )} ); diff --git a/app/src/components/projects/RepoListEditor.tsx b/app/src/components/projects/RepoListEditor.tsx index 285feb5..6e4fe1f 100644 --- a/app/src/components/projects/RepoListEditor.tsx +++ b/app/src/components/projects/RepoListEditor.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import type { RepoSource } from "../../lib/types"; import { cleanRepos, repoFolderFromUrl } from "../../lib/awsWorkspace"; import Button from "../ui/Button"; @@ -70,8 +70,8 @@ export function RepoRows({ rows, onRowsChange, onCommit, disabled = false }: Rep aria-label={`Remove repository ${i + 1}`} onClick={() => { onRowsChange(rows.filter((_, j) => j !== i)); - // A removal is a finished edit. The commit is applied after - // the rows update renders, so it sees the shorter list. + // A removal is a finished edit; `onRowsChange` has already + // handed the shorter list over. onCommit?.(); }} > @@ -98,6 +98,9 @@ function sameRepos(a: RepoSource[], b: RepoSource[]): boolean { * The Config tab's save-on-blur repository list. Saves when every row that * has anything in it has a URL; the backend's validation message (bad URL, * folder used twice…) comes back through the save's toast. + * + * Rows in progress survive a refresh of the project list: they are replaced + * from `repos` only when the stored value actually changed. */ export default function RepoListEditor({ repos, @@ -109,25 +112,28 @@ export default function RepoListEditor({ disabled?: boolean; }) { const [rows, setRows] = useState(repos); - useEffect(() => setRows(repos), [repos]); + // The latest rows, so a commit right after an edit (a removal) sees them. + const rowsRef = useRef(rows); + const storedRef = useRef(repos); - // A commit bumps this; the effect then runs against the rendered rows. - const [pending, setPending] = useState(0); useEffect(() => { - if (pending === 0) return; - const cleaned = cleanRepos(rows); - if (cleaned.some((r) => !r.url)) return; - if (sameRepos(cleaned, repos)) return; - void onSave(cleaned); - // Only a commit triggers this; `rows` changing alone does not save. - }, [pending]); + if (sameRepos(repos, storedRef.current)) return; + storedRef.current = repos; + rowsRef.current = repos; + setRows(repos); + }, [repos]); - return ( - setPending((n) => n + 1)} - disabled={disabled} - /> - ); + const change = (next: RepoSource[]) => { + rowsRef.current = next; + setRows(next); + }; + + const commit = () => { + const cleaned = cleanRepos(rowsRef.current); + if (cleaned.some((r) => !r.url)) return; + if (sameRepos(cleaned, storedRef.current)) return; + void onSave(cleaned); + }; + + return ; } diff --git a/app/src/components/projects/StopGuardModal.test.tsx b/app/src/components/projects/StopGuardModal.test.tsx index 9994328..1a638df 100644 --- a/app/src/components/projects/StopGuardModal.test.tsx +++ b/app/src/components/projects/StopGuardModal.test.tsx @@ -25,10 +25,10 @@ describe("StopGuardModal", () => { expect(onChoose).toHaveBeenCalledWith("push"); }); - it("offers Stop anyway and Cancel", () => { + it("offers Stop without pushing and Cancel", () => { const onChoose = vi.fn(); render(); - fireEvent.click(screen.getByRole("button", { name: "Stop anyway" })); + fireEvent.click(screen.getByRole("button", { name: "Stop without pushing" })); fireEvent.click(screen.getByRole("button", { name: "Cancel" })); expect(onChoose.mock.calls).toEqual([["discard"], [null]]); }); @@ -39,10 +39,12 @@ describe("StopGuardModal", () => { , ); expect(screen.queryByRole("button", { name: /Push/ })).toBeNull(); + expect(screen.getByText(/own push of work in progress still runs/)).toBeInTheDocument(); expect(screen.getByText("Triple-C is not connected to the workspace")).toBeInTheDocument(); expect(screen.getByText(/could not check/)).toBeInTheDocument(); const stop = screen.getByRole("button", { name: "Stop anyway" }); diff --git a/app/src/components/projects/StopGuardModal.tsx b/app/src/components/projects/StopGuardModal.tsx index 73dc7ea..bcb3826 100644 --- a/app/src/components/projects/StopGuardModal.tsx +++ b/app/src/components/projects/StopGuardModal.tsx @@ -6,6 +6,8 @@ import Button from "../ui/Button"; interface Props { projectName: string; work: UnsavedWork; + /** The project's "push work in progress when the workspace stops" setting. */ + wipPushOnStop?: boolean; /** `null` = Cancel. */ onChoose: (choice: WipChoice | null) => void; } @@ -23,7 +25,7 @@ function describeRepo(dirty: boolean, unpushed: number): string { * the default (it has focus); it is not offered when the check could not * run, since the push goes the same way the check did. */ -export default function StopGuardModal({ projectName, work, onChoose }: Props) { +export default function StopGuardModal({ projectName, work, wipPushOnStop = false, onChoose }: Props) { const pushRef = useRef(null); const stopRef = useRef(null); const atRisk = work.repos.filter((r) => r.dirty || r.unpushed > 0); @@ -45,7 +47,7 @@ export default function StopGuardModal({ projectName, work, onChoose }: Props) { variant={work.checked ? "danger" : "primary"} onClick={() => onChoose("discard")} > - Stop anyway + {work.checked ? "Stop without pushing" : "Stop anyway"} {work.checked && (