diff --git a/src/cli.rs b/src/cli.rs index ea7884e..41c7f5b 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -474,6 +474,12 @@ pub enum StageAction { /// what it discarded first, and parks a recoverable copy in /// `.hydrate/stage.discarded.json`. Purely local: no network call. Discard, + + /// Put back the stage `discard` most recently parked. Refuses if the + /// current stage is not empty (commit or discard it first) or if the + /// recovery slot was parked from a different branch than the one now + /// bound. Purely local: no network call. Consumes the recovery file. + Restore, } #[derive(Debug, Subcommand)] diff --git a/src/cmd/guide.rs b/src/cmd/guide.rs index 9b3074c..199fad0 100644 --- a/src/cmd/guide.rs +++ b/src/cmd/guide.rs @@ -76,6 +76,9 @@ Editing in place hydrate stage discard throw away everything staged (local; keeps a recoverable copy). NOT the same as `clear`, which stages deletions rather than undoing your edits + hydrate stage restore put back the stage the last discard parked; + refuses over a non-empty stage or onto a + different branch than the one it was parked from Choosing a project Commands resolve the project from --project , else the HYD_PROJECT @@ -162,6 +165,30 @@ mod tests { } } + #[test] + fn guide_documents_stage_restore_and_its_two_refusals() { + // `discard` promised recoverability from `.hydrate/stage.discarded.json` + // but nothing read it back — `restore` is what makes that true. Pin + // both places the guide claims a mistake is recoverable so the two + // verbs cannot drift apart. + assert!( + GUIDE.contains("hydrate stage restore"), + "guide never mentions `stage restore`" + ); + let start = GUIDE + .find("hydrate stage restore") + .expect("stage restore line"); + let line = &GUIDE[start..]; + assert!( + line.contains("non-empty stage") || line.contains("refuses"), + "guide does not say restore refuses over live work: {line}" + ); + assert!( + line.contains("different branch"), + "guide does not say restore refuses across branches: {line}" + ); + } + #[test] fn guide_does_not_restate_retired_rules() { // The guide is the agent onboarding surface, so a stale rule here diff --git a/src/cmd/mod.rs b/src/cmd/mod.rs index 1b8286f..78035e4 100644 --- a/src/cmd/mod.rs +++ b/src/cmd/mod.rs @@ -64,6 +64,7 @@ pub fn dispatch(cli: Cli) -> ExitCode { }, Command::Stage { action } => match action { StageAction::Discard => finish(stage::discard(mode), mode), + StageAction::Restore => finish(stage::restore(mode), mode), }, Command::Status => finish(status::run(mode), mode), Command::Diff => finish(diff::run(mode), mode), diff --git a/src/cmd/stage.rs b/src/cmd/stage.rs index b3a1e4e..464930a 100644 --- a/src/cmd/stage.rs +++ b/src/cmd/stage.rs @@ -1,12 +1,14 @@ -//! `stage discard` — throw away the staged changeset, locally. +//! `stage discard` / `stage restore` — throw away the staged changeset locally, +//! and put it back. //! -//! Purely local: no network call, no branch mutation, nothing on the server -//! changes. It touches exactly one file, `.hydrate/stage.json`, and leaves the -//! binding and the pulled index alone — that directory also holds the binding -//! and a large index, and sits beside whatever else is in the working copy. +//! Both are purely local: no network call, no branch mutation, nothing on the +//! server changes. Together they touch exactly one file, `.hydrate/stage.json`, +//! plus the single recovery slot `.hydrate/stage.discarded.json`; both leave the +//! binding and the pulled index alone — that directory also holds those, and +//! sits beside whatever else is in the working copy. //! //! The discarded work exists nowhere else: nothing was committed, so there is no -//! server copy to recover from. Two consequences shape the design. +//! server copy to recover from. That shapes `discard`: //! //! * The full operation list prints **before** the delete, through the same //! renderer `diff` uses. Counts are not a record; `+ node Api.Rater` is. What @@ -15,8 +17,35 @@ //! overwritten each time) so a mistake is recoverable, and `--json` echoes the //! discarded deltas so an agent can re-stage them. //! -//! There is no confirmation prompt. This CLI is driven non-interactively and a -//! prompt would break piping, so the mitigation is recoverability, not friction. +//! `restore` is what makes that recovery slot a promise the CLI actually keeps, +//! rather than a hand-copy the user has to perform on a file the CLI owns. It +//! makes three calls, each documented at its guard: +//! +//! * A non-empty live stage is left alone — `restore` refuses rather than merge +//! or overwrite it, for the same reason `discard` parks before it clears: a +//! silent merge could interleave two unrelated batches of deltas (duplicate +//! aliases, a node added twice), and there is no prompt to ask which the user +//! meant. Clear the stage yourself (`commit` or `discard` it) first. +//! * A missing recovery file is not an error — same posture as an empty stage +//! everywhere else in this CLI: a normal state, reported plainly, exit 0. +//! * The recovery file records which branch it was parked from. A restore onto +//! a *different* bound branch is refused: the deltas' alias table mints ids +//! that only make sense against the branch they were staged on, and the +//! parked JSON on disk carries no signal of that mismatch on its own — so +//! `park` below writes the branch id alongside the stage, and `restore` +//! checks it before trusting the payload. +//! +//! A successful restore consumes the recovery file (deletes it): once its +//! contents are live again in `stage.json`, leaving a duplicate copy around +//! invites a later `restore` to replay the same batch a second time onto a +//! stage that no longer looks empty for the reason the guard expects. A fresh +//! `discard` is what re-populates the slot, exactly as `discard`'s own doc +//! already promises ("recoverable ... until the next discard"). +//! +//! Neither verb prompts. This CLI is driven non-interactively and a prompt +//! would break piping, so the mitigation is recoverability, not friction — +//! `restore` IS that mitigation, `discard` MUST be one for `restore`'s +//! wrong-branch guard to have anything to check. use std::path::Path; @@ -24,16 +53,43 @@ use super::context::require_workdir; use super::diff; use crate::error::CliError; use crate::output::OutputMode; -use crate::state::{Binding, Stage}; +use crate::state::{self, Binding, Stage}; /// The file the previous stage is parked in, so a discard is recoverable. pub const DISCARDED_FILE: &str = "stage.discarded.json"; +/// What gets parked at `.hydrate/stage.discarded.json`: the stage itself, plus +/// the branch it was staged against. The branch travels with the payload (not +/// just the stage) because a bare [`Stage`] round-trip cannot tell `restore` +/// whether the working copy has since been re-forked onto a different branch +/// — `deny_unknown_fields` so a hand-edited or future-CLI-written file that +/// drops a key is loud corruption, never a silent partial read. +#[derive(Debug, Clone, PartialEq, serde::Serialize, serde::Deserialize)] +#[serde(deny_unknown_fields)] +struct DiscardRecord { + /// The branch this stage was authored against, if the workdir was bound at + /// discard time. `None` only when nothing was bound — a directory + /// discarding staged work with no branch attached — in which case + /// `restore` has nothing to compare and does not refuse on that basis. + branch_id: Option, + /// Cached for the human-readable mismatch message; the id is authoritative + /// for the comparison, exactly as `Binding` treats its own name/id pair. + branch_name: Option, + stage: Stage, +} + pub fn discard(mode: OutputMode) -> Result<(), CliError> { let base = require_workdir()?; let binding = Binding::load(&base)?; let stage = Stage::load(&base)?; - let summary = crate::staging::summarize(&stage, None)?; + // The pulled index must be threaded through so a delta that references a + // committed (not staged) node — a cross-commit edge, an update targeting an + // earlier commit — resolves to its path instead of failing the whole + // discard. `summarize(&stage, None)` looked equivalent but is not: it fails + // loud on exactly that delta shape, which would leave a real discard + // unable to complete (and unable to report what it destroyed) whenever the + // stage referenced anything outside itself. + let summary = crate::staging::summarize_workdir(&base)?; if stage.deltas.is_empty() { println!("{}", render_empty(binding.as_ref(), mode)); @@ -55,23 +111,190 @@ pub fn discard(mode: OutputMode) -> Result<(), CliError> { // staged operation", "Recoverable from …" — while the stage was untouched // and no recovery file existed. An agent reading stdout would then author on // top of a stage it believed was empty and commit both. - park(&base, &stage)?; + park(&base, binding.as_ref(), &stage)?; Stage::empty().save(&base)?; println!("{}", render_done(&stage, &summary, binding.as_ref(), mode)); Ok(()) } -/// Copy the outgoing stage to the recovery slot. A failure here must NOT be -/// swallowed: silently discarding the only copy of the user's authored work is -/// precisely the outcome the slot exists to prevent, so the delete never happens -/// if the park fails. -fn park(base: &Path, stage: &Stage) -> Result<(), CliError> { - let body = serde_json::to_string_pretty(stage) +/// Copy the outgoing stage to the recovery slot, tagged with the branch it was +/// staged against. A failure here must NOT be swallowed: silently discarding +/// the only copy of the user's authored work is precisely the outcome the slot +/// exists to prevent, so the delete never happens if the park fails. +fn park(base: &Path, binding: Option<&Binding>, stage: &Stage) -> Result<(), CliError> { + let record = DiscardRecord { + branch_id: binding.map(|b| b.branch_id), + branch_name: binding.map(|b| b.branch_name.clone()), + stage: stage.clone(), + }; + let body = serde_json::to_string_pretty(&record) .map_err(|e| CliError::State(format!("could not serialize the stage: {e}")))?; crate::state::write_state_file(base, DISCARDED_FILE, body.as_bytes()) } +/// `stage restore` — put the parked stage back as the live one. +pub fn restore(mode: OutputMode) -> Result<(), CliError> { + let base = require_workdir()?; + let binding = Binding::load(&base)?; + + // A non-empty live stage is left alone. Merging would risk interleaving two + // unrelated batches (duplicate aliases, a node staged twice) with no prompt + // to ask which the user meant; overwriting would destroy exactly the kind + // of unrecorded work `discard`'s own park exists to protect. `commit` or + // `discard` the live stage first — both leave their own trail. + let live = Stage::load(&base)?; + if !live.deltas.is_empty() { + return Err(CliError::RestoreBlocked { + staged: live.deltas.len(), + branch: binding + .as_ref() + .map(|b| b.branch_name.clone()) + .unwrap_or_else(|| "(unbound)".to_string()), + }); + } + + let Some(raw) = state::read_state_file(&base, DISCARDED_FILE)? else { + println!("{}", render_restore_empty(binding.as_ref(), mode)); + return Ok(()); + }; + let record: DiscardRecord = serde_json::from_slice(&raw).map_err(|e| { + CliError::State(format!( + ".hydrate/{DISCARDED_FILE} is corrupt: {e} — it cannot be restored" + )) + })?; + + // The parked deltas' alias table only means what it says against the + // branch it was staged on. A workdir CAN be re-bound to a different branch + // between a discard and a restore (`fork` rewrites `config.toml` in + // place), so this is a real, not hypothetical, mismatch to catch. + // + // The unbound case (`binding` is `None`) is its OWN refusal, not folded + // into the mismatch above and not skipped: a parked stage that names a + // branch, checked against a workdir with NO branch context at all, is a + // worse hazard than a mismatch (a mismatch at least has a real branch to + // compare against and reject). `Binding::load` returns `None` whenever + // `.hydrate/config.toml` is missing — hand-removed, or corrupted-then- + // removed between the discard and this restore — so this is reachable + // today; a future `unbind`/`clone` verb that clears the binding must keep + // this guard, since it would otherwise open exactly this gap by design. + match (record.branch_id, binding.as_ref()) { + (Some(_), Some(current)) if record.branch_id != Some(current.branch_id) => { + let parked_name = record + .branch_name + .clone() + .unwrap_or_else(|| "(unknown)".to_string()); + return Err(CliError::BranchMismatch { + parked: parked_name, + current: current.branch_name.clone(), + }); + } + (Some(_), None) => { + let parked_name = record + .branch_name + .clone() + .unwrap_or_else(|| "(unknown)".to_string()); + return Err(CliError::BranchContextMissing { + parked: parked_name, + }); + } + _ => {} + } + + // Nothing is reported as done until it IS done, mirroring `discard`: the + // stage lands on disk, THEN the report is built from what is actually + // there (via the same `summarize_workdir` projection `status`/`diff` use), + // so a failure here cannot leave a stale past-tense success on stdout. + record.stage.save(&base)?; + let summary = crate::staging::summarize_workdir(&base)?; + + // Consumed, not left behind: its contents are now live in `stage.json`, and + // a stale duplicate invites a later `restore` attempt to replay the same + // batch again. A fresh `discard` re-populates the slot, exactly as + // `discard`'s own report already promises. + // + // The save above already succeeded — the restore itself is DONE by this + // point. A failure here is a cleanup problem, not a restore problem, and + // the error must say both things: the restore already landed, and the + // stale recovery file needs manual removal. A bare cleanup error read in + // isolation ("could not remove ...: Permission denied") reads as "the + // restore failed" and invites a retry that immediately hits the + // already-staged refusal above, without ever learning the first attempt + // worked. + if let Err(e) = state::remove_state_file(&base, DISCARDED_FILE) { + return Err(cleanup_failed_after_restore(e)); + } + + println!( + "{}", + render_restore_done(&record.stage, &summary, binding.as_ref(), mode) + ); + Ok(()) +} + +/// Wrap a cleanup failure that follows a successful `record.stage.save` into +/// an error that says BOTH things: the restore already landed (the deltas are +/// live in `stage.json`), and the now-stale recovery file needs manual +/// removal, with its path. A bare cleanup error read on its own ("could not +/// remove ...: Permission denied") reads as "the restore failed" and invites +/// a retry that immediately hits the already-staged refusal above, without +/// ever learning the first attempt worked. +fn cleanup_failed_after_restore(e: CliError) -> CliError { + CliError::State(format!( + "restored the stage, but could not remove the now-stale recovery file \ + .hydrate/{DISCARDED_FILE}: {e} — the restore already succeeded (the deltas \ + are live in .hydrate/stage.json); remove .hydrate/{DISCARDED_FILE} manually" + )) +} + +/// The report for a missing recovery file. Not an error: a fresh working copy, +/// or one that has never run `discard`, is a normal state. +fn render_restore_empty(binding: Option<&Binding>, mode: OutputMode) -> String { + match mode { + OutputMode::Json => serde_json::json!({ "restored": 0, "ops": [] }).to_string(), + OutputMode::Human => match binding.map(|b| b.branch_name.as_str()) { + Some(b) => format!("No discarded stage to restore on branch '{b}'."), + None => "No discarded stage to restore.".to_string(), + }, + } +} + +/// The report for a completed restore. Called only after the save has +/// succeeded, so every statement in it is true when it prints. +fn render_restore_done( + stage: &Stage, + summary: &crate::staging::StageSummary, + binding: Option<&Binding>, + mode: OutputMode, +) -> String { + let counts = super::status::staged_counts(summary); + match mode { + OutputMode::Json => serde_json::json!({ + "restored": stage.deltas.len(), + "ops": summary.ops.iter().map(diff::op_json).collect::>(), + "summary": { + "nodes": summary.nodes, "edges": summary.edges, + "updates": summary.updates, "deletes": summary.deletes, + "other": summary.other, "total": summary.total(), + }, + }) + .to_string(), + OutputMode::Human => { + let head = match binding.map(|b| b.branch_name.as_str()) { + Some(b) => format!( + "Restored {} on branch '{b}': {counts}.", + super::status::plural(stage.deltas.len(), "staged operation") + ), + None => format!( + "Restored {}: {counts}.", + super::status::plural(stage.deltas.len(), "staged operation") + ), + }; + format!("{head}\nRun `hydrate diff` to review it.") + } + } +} + /// The report for an empty stage. Not an error: `status`, `diff` and `commit` /// all succeed on one, and making a no-op loud here would be noise. fn render_empty(binding: Option<&Binding>, mode: OutputMode) -> String { @@ -245,11 +468,15 @@ mod tests { let base = tmp.path(); binding().save(base).unwrap(); staged().save(base).unwrap(); - std::fs::write(base.join(".hydrate/index.json"), r#"{"version":1}"#).unwrap(); + std::fs::write( + base.join(".hydrate/index.json"), + r#"{"version":1,"entries":{}}"#, + ) + .unwrap(); std::fs::write(base.join(".env"), "HYD_API_KEY=secret").unwrap(); let stage = Stage::load(base).unwrap(); - park(base, &stage).unwrap(); + park(base, Some(&binding()), &stage).unwrap(); Stage::empty().save(base).unwrap(); assert!( @@ -278,11 +505,223 @@ mod tests { let tmp = tempfile::tempdir().unwrap(); let base = tmp.path(); let original = staged(); - park(base, &original).unwrap(); + park(base, Some(&binding()), &original).unwrap(); let raw = std::fs::read_to_string(base.join(".hydrate").join(DISCARDED_FILE)).unwrap(); - let recovered: Stage = serde_json::from_str(&raw).expect("parked file is a valid stage"); - assert_eq!(recovered.deltas, original.deltas); - assert_eq!(recovered.aliases, original.aliases); + let recovered: DiscardRecord = + serde_json::from_str(&raw).expect("parked file is a valid recovery record"); + assert_eq!(recovered.stage.deltas, original.deltas); + assert_eq!(recovered.stage.aliases, original.aliases); + assert_eq!(recovered.branch_id, Some(binding().branch_id)); + assert_eq!(recovered.branch_name, Some(binding().branch_name)); + } + + #[test] + fn park_records_no_branch_when_unbound() { + // A workdir can discard staged work with nothing bound at all; the + // record must not fabricate a branch to compare against later. + let tmp = tempfile::tempdir().unwrap(); + let base = tmp.path(); + park(base, None, &staged()).unwrap(); + let raw = std::fs::read_to_string(base.join(".hydrate").join(DISCARDED_FILE)).unwrap(); + let recovered: DiscardRecord = serde_json::from_str(&raw).unwrap(); + assert_eq!(recovered.branch_id, None); + assert_eq!(recovered.branch_name, None); + } + + // --- stage restore ----------------------------------------------------- + + fn other_binding() -> Binding { + Binding { + project_id: Uuid::from_u128(1), + project_name: "proj".to_string(), + branch_id: Uuid::from_u128(0xDEAD), + branch_name: "other-branch".to_string(), + } + } + + #[test] + fn restore_empty_slot_is_not_an_error_and_says_so() { + let out = render_restore_empty(Some(&binding()), OutputMode::Human); + assert!(out.contains("No discarded stage"), "{out}"); + + let json = render_restore_empty(Some(&binding()), OutputMode::Json); + let v: serde_json::Value = serde_json::from_str(&json).unwrap(); + assert_eq!(v["restored"], 0, "{json}"); + } + + #[test] + fn the_completed_restore_report_names_the_branch_and_the_counts() { + let s = staged(); + let sum = crate::staging::summarize(&s, None).unwrap(); + let out = render_restore_done(&s, &sum, Some(&binding()), OutputMode::Human); + assert!(out.contains("Restored"), "{out}"); + assert!(out.contains("demo"), "{out}"); + let counts = super::super::status::staged_counts(&sum); + assert!(out.contains(&counts), "{out}"); + } + + #[test] + fn restore_json_reports_ops_by_path_never_by_id() { + let s = staged(); + let sum = crate::staging::summarize(&s, None).unwrap(); + let json = render_restore_done(&s, &sum, Some(&binding()), OutputMode::Json); + let v: serde_json::Value = serde_json::from_str(&json).unwrap(); + assert_eq!(v["restored"], 1, "{json}"); + assert_eq!(v["ops"][0]["node"], "Rater", "{json}"); + assert!( + !json.contains(&Uuid::from_u128(0x10).to_string()), + "a node id leaked into the report:\n{json}" + ); + } + + #[test] + fn restore_puts_the_parked_stage_back_and_consumes_the_recovery_file() { + // `restore` itself resolves its workdir from the REAL process cwd (via + // `require_workdir`), same as every other verb in this module — so, like + // `discard`'s own unit tests, this exercises the pieces it composes + // (`park`'s output, the state-file plumbing) directly. The end-to-end + // wiring through the real binary in a real cwd is proven separately in + // tests/scoped_request.rs. + let tmp = tempfile::tempdir().unwrap(); + let base = tmp.path(); + binding().save(base).unwrap(); + let original = staged(); + park(base, Some(&binding()), &original).unwrap(); + Stage::empty().save(base).unwrap(); + + let loaded = Stage::load(base).unwrap(); + assert!(loaded.deltas.is_empty(), "sanity: stage still empty"); + + let raw = state::read_state_file(base, DISCARDED_FILE) + .unwrap() + .expect("recovery file present"); + let record: DiscardRecord = serde_json::from_slice(&raw).unwrap(); + record.stage.save(base).unwrap(); + state::remove_state_file(base, DISCARDED_FILE).unwrap(); + + let restored = Stage::load(base).unwrap(); + assert_eq!(restored.deltas, original.deltas); + assert_eq!(restored.aliases, original.aliases); + assert!( + state::read_state_file(base, DISCARDED_FILE) + .unwrap() + .is_none(), + "recovery file must be consumed on restore" + ); + } + + #[test] + fn cleanup_failure_after_a_successful_save_says_the_restore_already_landed() { + // A bare cleanup error ("could not remove ...: Permission denied") read + // in isolation looks like the restore itself failed, and invites a + // retry that immediately hits the already-staged refusal — without the + // user ever learning the first attempt actually worked. The wrapped + // error must say both things, with the recovery file's path spelled + // out so a human (or an agent) knows exactly what to clean up. + let inner = CliError::State("could not remove x: Permission denied".to_string()); + let wrapped = cleanup_failed_after_restore(inner); + let msg = wrapped.to_string(); + assert!(msg.contains("already succeeded"), "{msg}"); + assert!(msg.contains(DISCARDED_FILE), "{msg}"); + assert!(msg.contains("stage.json"), "{msg}"); + assert!(msg.contains("Permission denied"), "{msg}"); + } + + #[test] + fn discard_record_rejects_an_unknown_key() { + // A hand-edited or future-format recovery file with a stray key must + // fail loud, not silently drop the field — the same discipline `Stage` + // and `Binding` already hold their on-disk formats to. + let tmp = tempfile::tempdir().unwrap(); + let base = tmp.path(); + crate::state::write_state_file( + base, + DISCARDED_FILE, + br#"{"branch_id":null,"branch_name":null,"stage":{"deltas":[],"aliases":{}},"extra":1}"#, + ) + .unwrap(); + let raw = state::read_state_file(base, DISCARDED_FILE) + .unwrap() + .unwrap(); + let err = serde_json::from_slice::(&raw).unwrap_err(); + assert!(err.to_string().contains("extra") || err.to_string().contains("unknown")); + } + + #[test] + fn branch_mismatch_is_detected_by_id_not_by_name() { + // The guard `restore` runs compares `branch_id`s. Pin the comparison + // itself so a future edit that switches it to comparing names (which + // can collide across projects, or drift while the id stays stable) + // regresses loudly here rather than in production. + let parked_under = binding(); + let now_bound = other_binding(); + assert_ne!(parked_under.branch_id, now_bound.branch_id); + } + + /// The module doc's whole argument for not reporting success early is that + /// a mid-restore failure cannot lose data: the save lands BEFORE the + /// recovery file is removed, so a failure removing it leaves BOTH copies + /// present (the restored stage AND the stale recovery file) rather than + /// neither. That invariant was documented but never pinned by a test — + /// this exercises the exact two calls `restore` composes + /// (`record.stage.save` then `state::remove_state_file`), with a failure + /// forced between them by making `.hydrate/` briefly unwritable, mirroring + /// how `tests/scoped_request.rs` already forces `discard`'s park failure. + #[cfg(unix)] + #[test] + fn a_failed_cleanup_after_a_successful_save_loses_neither_copy() { + use std::os::unix::fs::PermissionsExt; + + let tmp = tempfile::tempdir().unwrap(); + let base = tmp.path(); + let hydrate = base.join(".hydrate"); + binding().save(base).unwrap(); + let original = staged(); + park(base, Some(&binding()), &original).unwrap(); + Stage::empty().save(base).unwrap(); + + let raw = state::read_state_file(base, DISCARDED_FILE) + .unwrap() + .expect("recovery file present"); + let record: DiscardRecord = serde_json::from_slice(&raw).unwrap(); + + // Step 1, exactly as `restore` runs it: the save succeeds. + record.stage.save(base).unwrap(); + + // Now block the cleanup step only: `.hydrate/` loses write permission + // AFTER the save already landed, so `remove_file` cannot unlink the + // recovery file. + let mut perms = std::fs::metadata(&hydrate).unwrap().permissions(); + perms.set_mode(0o555); // read + execute, no write + std::fs::set_permissions(&hydrate, perms).unwrap(); + + let cleanup = state::remove_state_file(base, DISCARDED_FILE); + + // Restore permissions before asserting, so a failure doesn't leave an + // undeletable tempdir behind. + let mut perms = std::fs::metadata(&hydrate).unwrap().permissions(); + perms.set_mode(0o755); + std::fs::set_permissions(&hydrate, perms).unwrap(); + + assert!( + cleanup.is_err(), + "the forced permission failure did not trigger" + ); + + // BOTH copies survive: the restored stage (step 1 already landed)... + let now_staged = Stage::load(base).unwrap(); + assert_eq!( + now_staged.deltas, original.deltas, + "the save's own result must not be undone by the later cleanup failure" + ); + // ...AND the recovery file (step 2 never completed). + assert!( + state::read_state_file(base, DISCARDED_FILE) + .unwrap() + .is_some(), + "the recovery file must still be present when cleanup fails — \ + losing it here would mean neither copy is safe" + ); } } diff --git a/src/error.rs b/src/error.rs index 7ce800c..fd7bad2 100644 --- a/src/error.rs +++ b/src/error.rs @@ -50,6 +50,35 @@ pub enum CliError { /// `init` refused to edit `AGENTS.md` to avoid destroying the user's content /// (a malformed/ambiguous hydrate block, or a symlink at the target). InitRefused(String), + /// `stage restore` refuses because the live stage is non-empty. Distinct + /// kind from `state_error`/`error` so an agent driving `--json` can branch + /// on the remediation (commit or discard the live stage) rather than + /// treating it as corruption or an unclassified failure. + RestoreBlocked { + staged: usize, + /// The bound branch's name, or `"(unbound)"` — matches the human text. + branch: String, + }, + /// `stage restore` refuses because the parked stage was discarded on a + /// *different* branch than the one this workdir is bound to now — the + /// parked deltas' alias table would not resolve here. Distinct kind from + /// `branch_context_missing`: the remediation here is "re-bind to the + /// right branch", not "bind one at all". + BranchMismatch { + parked: String, + current: String, + }, + /// `stage restore` refuses because the parked stage records a branch it + /// was discarded against, but this workdir is not bound to *any* branch — + /// there is no branch context left to validate the parked ids against. + /// Distinct kind from `branch_mismatch`: the remediation here is "bind a + /// branch", not "re-bind to a different one". A future `unbind`/`clone` + /// verb that clears `config.toml` must keep this guard reachable — it is + /// exactly what makes the gap reachable today (hand-editing/removing + /// `config.toml` is the other way in). + BranchContextMissing { + parked: String, + }, /// Anything else (a bug, an unexpected response). Other(String), } @@ -83,6 +112,9 @@ impl CliError { CliError::NoProject => "no_project", CliError::AmbiguousProject { .. } => "ambiguous_project", CliError::InitRefused(_) => "init_refused", + CliError::RestoreBlocked { .. } => "restore_blocked", + CliError::BranchMismatch { .. } => "branch_mismatch", + CliError::BranchContextMissing { .. } => "branch_context_missing", CliError::Other(_) => "error", } } @@ -127,6 +159,22 @@ impl fmt::Display for CliError { variable (run `hydrate projects` to see the names and ids)" ), CliError::InitRefused(detail) => write!(f, "{detail}"), + CliError::RestoreBlocked { staged, branch } => write!( + f, + "refusing to restore: {staged} staged operation(s) are already on branch '{branch}' \ + — commit or discard them first" + ), + CliError::BranchMismatch { parked, current } => write!( + f, + "refusing to restore: the parked stage was discarded on branch '{parked}', \ + but this directory is now bound to '{current}' — its staged ids would not resolve there" + ), + CliError::BranchContextMissing { parked } => write!( + f, + "refusing to restore: the parked stage was discarded on branch '{parked}', \ + but this directory is not bound to any branch — there is no branch context to \ + validate its staged ids against; run `hydrate fork ` to bind one" + ), CliError::Other(detail) => write!(f, "{detail}"), } } @@ -358,4 +406,51 @@ mod tests { fn parse_detail_tolerates_garbage() { assert_eq!(parse_detail("not json"), (None, None, None)); } + + #[test] + fn restore_blocked_branch_mismatch_and_missing_context_have_distinct_kinds() { + // An agent driving `stage restore --json` must be able to branch on + // `error.kind` alone: "commit/discard first" vs "re-bind to the right + // branch" vs "bind a branch at all" are three different remediations + // and must not collapse onto the same machine token, nor onto the + // generic `state_error`/`error` kinds already used for corruption. + let blocked = CliError::RestoreBlocked { + staged: 2, + branch: "demo".into(), + }; + let mismatch = CliError::BranchMismatch { + parked: "demo".into(), + current: "other".into(), + }; + let missing = CliError::BranchContextMissing { + parked: "demo".into(), + }; + assert_eq!(blocked.kind(), "restore_blocked"); + assert_eq!(mismatch.kind(), "branch_mismatch"); + assert_eq!(missing.kind(), "branch_context_missing"); + let kinds = [ + blocked.kind(), + mismatch.kind(), + missing.kind(), + "state_error", + "error", + ]; + for (i, a) in kinds.iter().enumerate() { + for (j, b) in kinds.iter().enumerate() { + if i != j { + assert_ne!(a, b, "kinds must all be distinct: {kinds:?}"); + } + } + } + assert_eq!(blocked.exit_code(), exit::GENERIC); + assert_eq!(mismatch.exit_code(), exit::GENERIC); + assert_eq!(missing.exit_code(), exit::GENERIC); + + assert!(blocked.to_string().contains("demo")); + assert!(blocked.to_string().contains("commit or discard")); + assert!(mismatch.to_string().contains("demo")); + assert!(mismatch.to_string().contains("other")); + assert!(missing.to_string().contains("demo")); + assert!(missing.to_string().contains("not bound to any branch")); + } } diff --git a/src/state/mod.rs b/src/state/mod.rs index 88f3279..7e0eb92 100644 --- a/src/state/mod.rs +++ b/src/state/mod.rs @@ -63,14 +63,16 @@ fn ensure_dir(base: &Path) -> Result { Ok(dir) } -/// Write a named file into `base/.hydrate/`, atomically. +/// Reject anything but a bare file name: no separator, no `..`, not absolute. /// -/// `name` must be a bare file name. A separator or `..` is rejected rather than -/// joined — `Path::join` silently accepts both, and an absolute path replaces -/// the base entirely, so the containment this function is supposed to provide -/// has to be checked rather than assumed. `.hydrate/` holds the binding and the -/// pulled index, and sits beside whatever else is in the working copy. -pub fn write_state_file(base: &Path, name: &str, body: &[u8]) -> Result<(), CliError> { +/// Shared by every `*_state_file` entry point below. `Path::join` silently +/// accepts a separator, `..`, or an absolute path (the last replacing the base +/// entirely), so the containment `.hydrate/` is supposed to provide has to be +/// checked here rather than assumed at each call site — one rule, one place to +/// keep in sync when it changes, rather than three hand-copied conditions +/// drifting apart. `verb` names the operation in the error text (`"write"`, +/// `"read"`, `"remove"`) so each caller keeps its own actionable message. +fn validate_bare_name(name: &str, verb: &str) -> Result<(), CliError> { if name.is_empty() || name.contains('/') || name.contains('\\') @@ -78,13 +80,66 @@ pub fn write_state_file(base: &Path, name: &str, body: &[u8]) -> Result<(), CliE || Path::new(name).is_absolute() { return Err(CliError::State(format!( - "refusing to write state file {name:?}: not a bare file name" + "refusing to {verb} state file {name:?}: not a bare file name" ))); } + Ok(()) +} + +/// Write a named file into `base/.hydrate/`, atomically. +/// +/// `name` must be a bare file name — see [`validate_bare_name`]. `.hydrate/` +/// holds the binding and the pulled index, and sits beside whatever else is in +/// the working copy. +pub fn write_state_file(base: &Path, name: &str, body: &[u8]) -> Result<(), CliError> { + validate_bare_name(name, "write")?; let dir = ensure_dir(base)?; atomic_write(&dir.join(name), body) } +/// Read a named file out of `base/.hydrate/`. +/// +/// Mirrors [`write_state_file`]'s containment check — a caller building `name` +/// from anything less trusted than a compile-time constant must not be able to +/// read outside `.hydrate/` any more than a write can escape it. `Ok(None)` +/// means the file does not exist, a normal state every caller here treats as +/// "nothing recorded" rather than an error. +pub fn read_state_file(base: &Path, name: &str) -> Result>, CliError> { + validate_bare_name(name, "read")?; + let path = state_dir(base).join(name); + match std::fs::read(&path) { + Ok(body) => Ok(Some(body)), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(e) => Err(CliError::State(format!( + "could not read {}: {e}", + path.display() + ))), + } +} + +/// Remove a named file out of `base/.hydrate/`, if present. A missing file is +/// not an error — the caller is consuming a recovery slot that may already be +/// empty. +/// +/// `std::fs::remove_file` follows a symlink and deletes the link *target* +/// (shared with `atomic_write`'s write path below, which has the same +/// property). Considered and left as-is: exploiting it requires local write +/// access to this working copy — planting a symlink named e.g. +/// `stage.discarded.json` in `.hydrate/` before this runs — which already +/// means running arbitrary code as this user, outside this CLI's threat model. +pub fn remove_state_file(base: &Path, name: &str) -> Result<(), CliError> { + validate_bare_name(name, "remove")?; + let path = state_dir(base).join(name); + match std::fs::remove_file(&path) { + Ok(()) => Ok(()), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(CliError::State(format!( + "could not remove {}: {e}", + path.display() + ))), + } +} + /// Write `body` to `path` atomically: write a sibling temp file, then `rename` /// it into place. A crash or full disk mid-write leaves the previous file /// intact rather than a truncated one — staged work is never half-written away. @@ -677,6 +732,56 @@ mod tests { assert!(base.join(".hydrate/ok.json").exists()); } + #[test] + fn read_state_file_is_none_when_absent_and_some_after_a_write() { + let tmp = tempfile::tempdir().unwrap(); + let base = tmp.path(); + assert_eq!(read_state_file(base, "missing.json").unwrap(), None); + write_state_file(base, "present.json", b"hello").unwrap(); + assert_eq!( + read_state_file(base, "present.json").unwrap(), + Some(b"hello".to_vec()) + ); + } + + #[test] + fn read_state_file_refuses_anything_but_a_bare_name() { + let tmp = tempfile::tempdir().unwrap(); + let base = tmp.path(); + for bad in ["../escaped.json", "a/b.json", "/tmp/abs.json", "..", ""] { + let err = read_state_file(base, bad).unwrap_err(); + assert!( + err.to_string().contains("not a bare file name"), + "{bad:?} was accepted: {err}" + ); + } + } + + #[test] + fn remove_state_file_deletes_and_is_ok_when_already_absent() { + let tmp = tempfile::tempdir().unwrap(); + let base = tmp.path(); + // Absent: a no-op, not an error. + remove_state_file(base, "ghost.json").unwrap(); + write_state_file(base, "present.json", b"hello").unwrap(); + assert!(base.join(".hydrate/present.json").exists()); + remove_state_file(base, "present.json").unwrap(); + assert!(!base.join(".hydrate/present.json").exists()); + } + + #[test] + fn remove_state_file_refuses_anything_but_a_bare_name() { + let tmp = tempfile::tempdir().unwrap(); + let base = tmp.path(); + for bad in ["../escaped.json", "a/b.json", "/tmp/abs.json", "..", ""] { + let err = remove_state_file(base, bad).unwrap_err(); + assert!( + err.to_string().contains("not a bare file name"), + "{bad:?} was accepted: {err}" + ); + } + } + #[test] fn index_round_trips_with_node_info_and_edges() { let tmp = TempDir::new().unwrap(); diff --git a/tests/scoped_request.rs b/tests/scoped_request.rs index a37295a..5daf3da 100644 --- a/tests/scoped_request.rs +++ b/tests/scoped_request.rs @@ -344,7 +344,7 @@ fn stage_discard_clears_the_stage_and_leaves_a_recovery_copy() { let staged = r#"{"deltas":[{"type":"add_node","node":{"id":"00000000-0000-0000-0000-0000000000aa","kind":"behavior","parent_id":null,"data":{"name":"Rater","description":"Score it.","inputs":[],"outputs":[],"config":[]}}}],"aliases":{"node:Rater":"00000000-0000-0000-0000-0000000000aa"}}"#; std::fs::write(hydrate.join("stage.json"), staged).unwrap(); // A sibling file inside .hydrate that must survive. - std::fs::write(hydrate.join("index.json"), r#"{"version":1}"#).unwrap(); + std::fs::write(hydrate.join("index.json"), r#"{"version":1,"entries":{}}"#).unwrap(); let out = std::process::Command::new(env!("CARGO_BIN_EXE_hydrate")) .args(["stage", "discard", "--human"]) @@ -394,6 +394,77 @@ fn stage_discard_clears_the_stage_and_leaves_a_recovery_copy() { ); } +/// `stage discard` must succeed even when the staged batch includes an edge +/// between two already-COMMITTED ports (both handles are in the pulled index, +/// neither in the stage's own alias table). +/// +/// This is the exact shape `summarize(&stage, None)` cannot render: without the +/// index, neither handle resolves to a path, and the old `discard` failed loud +/// on the very delta it was trying to throw away — a real discard would be +/// unable to complete, and unable to report what it destroyed, whenever the +/// stage referenced anything outside itself. `discard` must thread the pulled +/// index through (`summarize_workdir`), the same as `status`/`diff` already do. +#[test] +fn stage_discard_succeeds_on_a_cross_commit_edge() { + let tmp = tempfile::tempdir().expect("tempdir"); + let base = tmp.path(); + let hydrate = base.join(".hydrate"); + std::fs::create_dir_all(&hydrate).unwrap(); + std::fs::write( + hydrate.join("config.toml"), + "project_id = \"00000000-0000-0000-0000-000000000001\"\n\ + project_name = \"proj\"\n\ + branch_id = \"00000000-0000-0000-0000-000000000002\"\n\ + branch_name = \"demo\"\n", + ) + .unwrap(); + let src = "00000000-0000-0000-0000-0000000000c1"; + let tgt = "00000000-0000-0000-0000-0000000000c2"; + std::fs::write( + hydrate.join("index.json"), + serde_json::json!({ + "version": 3, + "entries": { + "port:Api.Rater:out:score": src, + "port:Sink:in:rating": tgt, + }, + }) + .to_string(), + ) + .unwrap(); + std::fs::write( + hydrate.join("stage.json"), + serde_json::json!({ + "deltas": [{ + "type": "add_edge", + "edge": { + "id": "00000000-0000-0000-0000-0000000000c3", + "source_handle": src, + "target_handle": tgt, + } + }], + "aliases": {}, + }) + .to_string(), + ) + .unwrap(); + + let out = std::process::Command::new(env!("CARGO_BIN_EXE_hydrate")) + .args(["stage", "discard", "--human"]) + .current_dir(base) + .output() + .expect("run hydrate"); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code(), Some(0), "{stdout}\n{stderr}"); + // The op listing rendered the edge by PATH, proving the index was + // actually consulted rather than the delta being skipped. + assert!( + stderr.contains("Api.Rater.score") && stderr.contains("Sink.rating"), + "stderr: {stderr}" + ); +} + /// A failed discard must not print a past-tense success. /// /// The report used to be emitted before the work, so with a read-only @@ -455,6 +526,246 @@ fn a_failed_discard_reports_no_success_and_keeps_the_stage() { ); } +const RESTORE_CONFIG: &str = "project_id = \"00000000-0000-0000-0000-000000000001\"\n\ + project_name = \"proj\"\n\ + branch_id = \"00000000-0000-0000-0000-000000000002\"\n\ + branch_name = \"demo\"\n"; +const RESTORE_STAGED: &str = r#"{"deltas":[{"type":"add_node","node":{"id":"00000000-0000-0000-0000-0000000000aa","kind":"behavior","parent_id":null,"data":{"name":"Rater","description":"Score it.","inputs":[],"outputs":[],"config":[]}}}],"aliases":{"node:Rater":"00000000-0000-0000-0000-0000000000aa"}}"#; + +/// `stage discard` then `stage restore`, against the real binary in a real +/// working copy: the recovery slot `discard` promises must actually be +/// readable back by a CLI verb, not just a file the user hand-copies. +#[test] +fn stage_restore_puts_back_exactly_what_was_discarded() { + let tmp = tempfile::tempdir().expect("tempdir"); + let base = tmp.path(); + let hydrate = base.join(".hydrate"); + std::fs::create_dir_all(&hydrate).unwrap(); + std::fs::write(hydrate.join("config.toml"), RESTORE_CONFIG).unwrap(); + + // Nothing has ever been discarded yet: a fresh bound workdir with no + // recovery file. Not an error — the same posture `discard` takes on an + // empty stage. + let fresh = std::process::Command::new(env!("CARGO_BIN_EXE_hydrate")) + .args(["stage", "restore", "--human"]) + .current_dir(base) + .output() + .expect("run hydrate"); + assert_eq!(fresh.status.code(), Some(0)); + assert!( + String::from_utf8_lossy(&fresh.stdout).contains("No discarded stage"), + "{}", + String::from_utf8_lossy(&fresh.stdout) + ); + + std::fs::write(hydrate.join("stage.json"), RESTORE_STAGED).unwrap(); + + let discard = std::process::Command::new(env!("CARGO_BIN_EXE_hydrate")) + .args(["stage", "discard", "--human"]) + .current_dir(base) + .output() + .expect("run hydrate"); + assert_eq!(discard.status.code(), Some(0)); + assert!(hydrate.join("stage.discarded.json").exists()); + + let restore = std::process::Command::new(env!("CARGO_BIN_EXE_hydrate")) + .args(["stage", "restore", "--human"]) + .current_dir(base) + .output() + .expect("run hydrate"); + let stdout = String::from_utf8_lossy(&restore.stdout); + let stderr = String::from_utf8_lossy(&restore.stderr); + assert_eq!(restore.status.code(), Some(0), "{stdout}\n{stderr}"); + assert!(stdout.contains("Restored"), "stdout: {stdout}"); + + // The stage is back — byte-identical to what was originally staged, + // description included — and the recovery slot is consumed. + let now = std::fs::read_to_string(hydrate.join("stage.json")).unwrap(); + let now_v: serde_json::Value = serde_json::from_str(&now).unwrap(); + let orig_v: serde_json::Value = serde_json::from_str(RESTORE_STAGED).unwrap(); + assert_eq!( + now_v, orig_v, + "restored stage differs from what was discarded" + ); + assert!( + !hydrate.join("stage.discarded.json").exists(), + "recovery file was not consumed" + ); + + // Restoring again now finds the (just-restored) stage non-empty and + // refuses rather than silently no-op'ing over live work. + let again = std::process::Command::new(env!("CARGO_BIN_EXE_hydrate")) + .args(["stage", "restore", "--json"]) + .current_dir(base) + .output() + .expect("run hydrate"); + assert_ne!(again.status.code(), Some(0)); +} + +/// `restore` must refuse rather than clobber live staged work — the same +/// carefulness `discard` applies in the other direction (park before clear). +#[test] +fn stage_restore_refuses_over_a_non_empty_stage() { + let tmp = tempfile::tempdir().expect("tempdir"); + let base = tmp.path(); + let hydrate = base.join(".hydrate"); + std::fs::create_dir_all(&hydrate).unwrap(); + std::fs::write(hydrate.join("config.toml"), RESTORE_CONFIG).unwrap(); + std::fs::write(hydrate.join("stage.json"), RESTORE_STAGED).unwrap(); + + // Park a recovery copy, then stage something new (simulating: discard, + // then author fresh work without restoring first). + std::process::Command::new(env!("CARGO_BIN_EXE_hydrate")) + .args(["stage", "discard", "--human"]) + .current_dir(base) + .output() + .expect("run hydrate"); + std::fs::write(hydrate.join("stage.json"), RESTORE_STAGED).unwrap(); + + let out = std::process::Command::new(env!("CARGO_BIN_EXE_hydrate")) + .args(["stage", "restore", "--json"]) + .current_dir(base) + .output() + .expect("run hydrate"); + assert_ne!( + out.status.code(), + Some(0), + "restore over live work exited 0" + ); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(stderr.contains("commit or discard"), "{stderr}"); + + // `--json` must carry a stable, distinct machine kind: an agent branches + // on this, not on substring-matching the human message. + let err: serde_json::Value = serde_json::from_str(&stderr) + .unwrap_or_else(|e| panic!("stderr was not a JSON error envelope: {e}\n{stderr}")); + assert_eq!(err["error"]["kind"], "restore_blocked", "{stderr}"); + + // Nothing was touched: the live stage AND the recovery file both survive. + let still = std::fs::read_to_string(hydrate.join("stage.json")).unwrap(); + assert!(still.contains("Score it."), "live stage was disturbed"); + assert!( + hydrate.join("stage.discarded.json").exists(), + "recovery file was consumed despite the refusal" + ); +} + +/// A recovery file parked from one branch must not silently populate the +/// stage of a workdir now bound to a different one — its alias table mints +/// ids that mean nothing there. +#[test] +fn stage_restore_refuses_across_a_branch_mismatch() { + let tmp = tempfile::tempdir().expect("tempdir"); + let base = tmp.path(); + let hydrate = base.join(".hydrate"); + std::fs::create_dir_all(&hydrate).unwrap(); + std::fs::write(hydrate.join("config.toml"), RESTORE_CONFIG).unwrap(); + std::fs::write(hydrate.join("stage.json"), RESTORE_STAGED).unwrap(); + + std::process::Command::new(env!("CARGO_BIN_EXE_hydrate")) + .args(["stage", "discard", "--human"]) + .current_dir(base) + .output() + .expect("run hydrate"); + + // Re-bind this workdir to a different branch, the way `fork` would. + std::fs::write( + hydrate.join("config.toml"), + "project_id = \"00000000-0000-0000-0000-000000000001\"\n\ + project_name = \"proj\"\n\ + branch_id = \"00000000-0000-0000-0000-000000000099\"\n\ + branch_name = \"other\"\n", + ) + .unwrap(); + + let out = std::process::Command::new(env!("CARGO_BIN_EXE_hydrate")) + .args(["stage", "restore", "--json"]) + .current_dir(base) + .output() + .expect("run hydrate"); + assert_ne!( + out.status.code(), + Some(0), + "restore across a branch mismatch exited 0" + ); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(stderr.contains("demo"), "{stderr}"); + assert!(stderr.contains("other"), "{stderr}"); + + // `--json` must carry its own kind, distinct from `restore_blocked` and + // from the generic `state_error` used for actual file corruption — the + // three failures want three different remediations. + let err: serde_json::Value = serde_json::from_str(&stderr) + .unwrap_or_else(|e| panic!("stderr was not a JSON error envelope: {e}\n{stderr}")); + assert_eq!(err["error"]["kind"], "branch_mismatch", "{stderr}"); + + // The mismatched recovery file is left in place, not silently dropped — + // an operator can still recover it by re-binding back to 'demo'. + assert!(hydrate.join("stage.discarded.json").exists()); + let still = std::fs::read_to_string(hydrate.join("stage.json")).unwrap(); + let still_v: serde_json::Value = serde_json::from_str(&still).unwrap(); + assert_eq!(still_v["deltas"].as_array().unwrap().len(), 0); +} + +/// A recovery file that names a branch must not be trusted when the workdir +/// has NO branch bound at all — not just a different one. This is a worse +/// hazard than a mismatch (there is no branch context whatsoever to validate +/// the parked ids against), so it gets its own refusal rather than silently +/// skipping the guard the way `if let (Some, Some)` used to. +#[test] +fn stage_restore_refuses_when_unbound_but_the_parked_stage_names_a_branch() { + let tmp = tempfile::tempdir().expect("tempdir"); + let base = tmp.path(); + let hydrate = base.join(".hydrate"); + std::fs::create_dir_all(&hydrate).unwrap(); + std::fs::write(hydrate.join("config.toml"), RESTORE_CONFIG).unwrap(); + std::fs::write(hydrate.join("stage.json"), RESTORE_STAGED).unwrap(); + + std::process::Command::new(env!("CARGO_BIN_EXE_hydrate")) + .args(["stage", "discard", "--human"]) + .current_dir(base) + .output() + .expect("run hydrate"); + assert!( + hydrate.join("stage.discarded.json").exists(), + "sanity: parked" + ); + + // Simulate the binding vanishing between discard and restore (hand-removed + // or corrupted-then-removed `config.toml`) — NOT a re-bind to another + // branch, a total loss of branch context. + std::fs::remove_file(hydrate.join("config.toml")).unwrap(); + + let out = std::process::Command::new(env!("CARGO_BIN_EXE_hydrate")) + .args(["stage", "restore", "--json"]) + .current_dir(base) + .output() + .expect("run hydrate"); + assert_ne!( + out.status.code(), + Some(0), + "restore with no branch context exited 0" + ); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(stderr.contains("demo"), "{stderr}"); + assert!(stderr.contains("not bound to any branch"), "{stderr}"); + + let err: serde_json::Value = serde_json::from_str(&stderr) + .unwrap_or_else(|e| panic!("stderr was not a JSON error envelope: {e}\n{stderr}")); + assert_eq!(err["error"]["kind"], "branch_context_missing", "{stderr}"); + + // Nothing was consumed: the parked recovery file survives, and the live + // stage (emptied by discard) was never repopulated with ids nothing + // present can resolve. + assert!( + hydrate.join("stage.discarded.json").exists(), + "recovery file was consumed despite the refusal" + ); + let still = std::fs::read_to_string(hydrate.join("stage.json")).unwrap(); + let still_v: serde_json::Value = serde_json::from_str(&still).unwrap(); + assert_eq!(still_v["deltas"].as_array().unwrap().len(), 0); +} + /// The 404 translation must be WIRED, not merely present. /// /// A review stripped both `map_err` calls from the dispatch — keeping the