diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 7051ddf..0b4c82c 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -23,8 +23,13 @@ pub async fn release_plan(launcher: State<'_, Launcher>) -> CmdResult, password: String, with_api: bool) -> CmdResult<()> { - launcher.release_run(password, with_api).await +pub async fn release_run( + launcher: State<'_, Launcher>, + password: String, + with_api: bool, + with_mcp: bool, +) -> CmdResult<()> { + launcher.release_run(password, with_api, with_mcp).await } #[tauri::command] diff --git a/src-tauri/src/launcher.rs b/src-tauri/src/launcher.rs index cabfc34..d8880db 100644 --- a/src-tauri/src/launcher.rs +++ b/src-tauri/src/launcher.rs @@ -2322,7 +2322,7 @@ impl Launcher { } /// Start a release in its own console. Returns once it has started, not when it is done. - pub async fn release_run(&self, password: String, with_api: bool) -> Result<(), String> { + pub async fn release_run(&self, password: String, with_api: bool, with_mcp: bool) -> Result<(), String> { let plan = self.release_plan().await; if let Some(why) = plan.blocked { return Err(why); @@ -2354,6 +2354,7 @@ impl Launcher { &pack_next, launcher_next.as_deref(), with_api && plan.docker, + with_mcp && plan.docker, &password, ) .map(|_| ()) diff --git a/src-tauri/src/release.rs b/src-tauri/src/release.rs index 315150c..69df9de 100644 --- a/src-tauri/src/release.rs +++ b/src-tauri/src/release.rs @@ -73,10 +73,21 @@ fn head(repo: &Path) -> Option { git(repo, &["rev-parse", "HEAD"]) } -fn dirty(repo: &Path) -> bool { - git(repo, &["status", "--porcelain"]).is_some_and(|s| !s.trim().is_empty()) +/// `shipped` limits the question to the paths an artifact is actually built from. It matters +/// for the MCP: that repo also holds these release scripts, so an unscoped check let editing +/// a script lock out every other step - the pack and the launcher included. +fn dirty(repo: &Path, shipped: Option<&[&str]>) -> bool { + let mut args = vec!["status", "--porcelain"]; + if let Some(paths) = shipped { + args.push("--"); + args.extend_from_slice(paths); + } + git(repo, &args).is_some_and(|s| !s.trim().is_empty()) } +/// What lands in the MCP image: `Dockerfile` copies only these. +const MCP_SHIPPED: &[&str] = &["Dockerfile", "requirements.txt", "src"]; + /// One artifact's row: is this repo past what is published? fn item( key: &'static str, @@ -84,10 +95,11 @@ fn item( repo: PathBuf, live: Option, published_commit: Option, + shipped: Option<&[&str]>, ) -> ReleaseItem { let exists = repo.is_dir(); let commit = exists.then(|| head(&repo)).flatten(); - let dirty = exists && dirty(&repo); + let dirty = exists && dirty(&repo, shipped); let behind = match (&commit, &published_commit) { (Some(now), Some(was)) => Some(now != was), _ => None, @@ -118,18 +130,30 @@ pub fn plan( let scripts = mcp_repo.join("scripts").join("release_pack.ps1").is_file() && mcp_repo.join("scripts").join("package_launcher.ps1").is_file(); let items = vec![ - item("pack", "ODIAN", dev_root.join("ODIAN_Engine"), pack_live, pack_commit), - item("launcher", "Launcher", dev_root.join("ODIAN_Launcher"), launcher_live, launcher_commit), + item("pack", "ODIAN", dev_root.join("ODIAN_Engine"), pack_live, pack_commit, None), + item("launcher", "Launcher", dev_root.join("ODIAN_Launcher"), launcher_live, launcher_commit, None), // The container is built from the same Engine checkout as the pack; only its pipeline // differs, and only it needs Docker. - item("api", "Engine API", dev_root.join("ODIAN_Engine"), api_live, None), + item("api", "Engine API", dev_root.join("ODIAN_Engine"), api_live, None, None), + item("mcp", "MCP server", dev_root.join("ODIAN_MCP_Server"), None, None, Some(MCP_SHIPPED)), ]; let blocked = if !online { Some("The DHP is not answering: nothing can be published.".to_string()) } else if !scripts { Some(format!("The release scripts are not in {}.", mcp_repo.display())) } else if items.iter().any(|i| i.dirty) { - Some("Commit what is open first: a release built from uncommitted work cannot be rebuilt.".to_string()) + // Naming them matters: "pack" and "api" are the same checkout, so a bare message sent + // you hunting through three repos for one open file. + let mut who: Vec<&str> = Vec::new(); + for i in items.iter().filter(|i| i.dirty) { + if !who.contains(&i.title) { + who.push(i.title); + } + } + Some(format!( + "Commit what is open first ({}): a release built from uncommitted work cannot be rebuilt.", + who.join(", ") + )) } else { None }; @@ -152,6 +176,7 @@ pub fn run_release( pack_version: &str, launcher_version: Option<&str>, with_api: bool, + with_mcp: bool, sudo: &str, ) -> Result { let scripts = mcp_repo.join("scripts"); @@ -189,6 +214,19 @@ pub fn run_release( )); } + if with_mcp { + // LAST, always. The MCP serves /download/* and proxies /dhp, so rolling it interrupts + // the very road the steps above publish over; anything still in flight would fail. + body.push_str(&format!( + "Write-Host '=== MCP server ===' -ForegroundColor Cyan + & '{}' + if ($LASTEXITCODE -eq 2) {{ Write-Host 'MCP: nothing to deploy' -ForegroundColor Yellow }} + elseif ($LASTEXITCODE -ne 0) {{ throw 'the MCP step failed' }} +", + scripts.join("deploy_mcp.ps1").display() + )); + } + // Everything inside try/finally, and everything recorded. A step that throws used to // take the window with it - the release simply vanished, with nothing to read. let ps = format!( @@ -279,17 +317,50 @@ mod tests { fn a_repo_that_matches_what_is_published_is_not_behind() { // The comparison is COMMIT against COMMIT. Versions cannot answer it: a version says // what was cut, not from what - which is how a release gets cut twice from one tree. - let same = item("pack", "ODIAN", PathBuf::from("."), Some("0.71.0".into()), head(Path::new("."))); + let same = item("pack", "ODIAN", PathBuf::from("."), Some("0.71.0".into()), head(Path::new(".")), None); if same.commit.is_some() { assert_eq!(same.behind, Some(false), "same commit: nothing to release"); } // Nothing published says where it came from: say so rather than guess. - let unknown = item("pack", "ODIAN", PathBuf::from("."), Some("0.71.0".into()), None); + let unknown = item("pack", "ODIAN", PathBuf::from("."), Some("0.71.0".into()), None, None); assert_eq!(unknown.behind, None); assert!(unknown.note.contains("does not say") || unknown.dirty || unknown.commit.is_none()); // A folder that is not there is named, not silently "up to date". - let missing = item("pack", "ODIAN", PathBuf::from(r"Z:\nothing\here"), None, None); + let missing = item("pack", "ODIAN", PathBuf::from(r"Z:\nothing\here"), None, None, None); assert_eq!(missing.behind, None); assert!(missing.note.contains("not in the checkout folder")); } + + /// The regression that adding the MCP step introduced: that repo also holds the release + /// scripts, so an unscoped `git status` made editing a script look like uncommitted + /// release content and locked out the pack and the launcher too. + #[test] + fn a_dirty_file_outside_the_shipped_paths_does_not_block() { + let dir = std::env::temp_dir().join(format!("odian-dirty-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(dir.join("src")).unwrap(); + std::fs::create_dir_all(dir.join("scripts")).unwrap(); + let ok = |args: &[&str]| git(&dir, args).is_some(); + if !ok(&["init"]) { + return; // no git here; nothing to prove + } + let _ = git(&dir, &["config", "user.email", "t@t"]); + let _ = git(&dir, &["config", "user.name", "t"]); + std::fs::write(dir.join("src").join("app.py"), "x").unwrap(); + std::fs::write(dir.join("requirements.txt"), "y").unwrap(); + std::fs::write(dir.join("Dockerfile"), "FROM x").unwrap(); + assert!(ok(&["add", "-A"]) && ok(&["commit", "-m", "base"])); + assert!(!dirty(&dir, Some(MCP_SHIPPED)), "clean tree"); + + // An untracked release script: tooling, never in the image. + std::fs::write(dir.join("scripts").join("deploy_mcp.ps1"), "# tool").unwrap(); + assert!(dirty(&dir, None), "the repo really is dirty"); + assert!(!dirty(&dir, Some(MCP_SHIPPED)), "but nothing that ships changed"); + + // Something that does ship: this must still block. + std::fs::write(dir.join("src").join("app.py"), "changed").unwrap(); + assert!(dirty(&dir, Some(MCP_SHIPPED)), "shipped source changed: block"); + + let _ = std::fs::remove_dir_all(&dir); + } } diff --git a/src/components/ReleasePanel.svelte b/src/components/ReleasePanel.svelte index d1f53d5..22e192f 100644 --- a/src/components/ReleasePanel.svelte +++ b/src/components/ReleasePanel.svelte @@ -12,9 +12,12 @@ let password = $state(""); let withApi = $state(true); + // Off by default, deliberately: it serves /download/* and proxies /dhp, so it is the one + // step that interrupts the road everything else publishes over. + let withMcp = $state(false); async function release() { - await launcher.releaseRun(password, withApi); + await launcher.releaseRun(password, withApi, withMcp); password = ""; // held for the one run, never kept } @@ -79,6 +82,13 @@ — every operator is logged out +
subscribe("status", handler), onInstallProgress: (handler) => subscribe("install-progress", handler), releasePlan: () => invoke("release_plan"), - releaseRun: (password, withApi) => invoke("release_run", { password, withApi }), + releaseRun: (password, withApi, withMcp) => invoke("release_run", { password, withApi, withMcp }), onTileSyncProgress: (handler) => subscribe("tile-sync-progress", handler), onRemoveProgress: (handler) => subscribe("remove-progress", handler), onStack: (handler) => subscribe("stack", handler), diff --git a/src/lib/launcher.svelte.ts b/src/lib/launcher.svelte.ts index 8440e97..d56d618 100644 --- a/src/lib/launcher.svelte.ts +++ b/src/lib/launcher.svelte.ts @@ -414,9 +414,9 @@ class Launcher { }); } - releaseRun(password: string, withApi: boolean) { + releaseRun(password: string, withApi: boolean, withMcp: boolean) { return this.run("release", async () => { - await backend.releaseRun(password, withApi); + await backend.releaseRun(password, withApi, withMcp); this.releaseOpen = false; }); } diff --git a/src/lib/mock.ts b/src/lib/mock.ts index b04fbd4..5240fc1 100644 --- a/src/lib/mock.ts +++ b/src/lib/mock.ts @@ -777,14 +777,16 @@ class MockBackend implements Backend { dirty: false, note: "up to date" }, { key: "api", title: "Engine API", live: null, commit: "a1b2c3d", behind: null, dirty: false, note: "what is published does not say which commit it came from" }, + { key: "mcp", title: "MCP server", live: null, commit: "c7d8e9f", behind: null, + dirty: false, note: "the script asks the node whether it is already running this commit" }, ], }; } - async releaseRun(password: string, withApi: boolean): Promise { + async releaseRun(password: string, withApi: boolean, withMcp: boolean): Promise { await this.wait(400); if (!password.trim()) throw "The node's sudo password is needed to cut a pack and to roll the API."; - this.log("info", `Release started in its own window (browser mock: nothing runs). API: ${withApi}`); + this.log("info", `Release started in its own window (browser mock: nothing runs). API ${withApi}, MCP ${withMcp}`); } async syncNow(): Promise { diff --git a/src/lib/types.ts b/src/lib/types.ts index 0df6239..2f472ff 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -374,7 +374,7 @@ export interface Backend { /** Take the tiles the DHP baked while this PC was away. */ syncTiles(): Promise; releasePlan(): Promise; - releaseRun(password: string, withApi: boolean): Promise; + releaseRun(password: string, withApi: boolean, withMcp: boolean): Promise; /** What the operator has to decide, oldest first. */ getSyncConflicts(): Promise; /** Records one decision; resolves with what is left to decide. */