From 2118e950e5d3eaf911db0479c4fe77a3fdb6b44f Mon Sep 17 00:00:00 2001 From: Toni Bergholm Date: Wed, 21 Oct 2026 11:24:19 +0300 Subject: [PATCH 1/2] fix(repo,cli): ws harvest defaults to the author recorded at fork (#286) ws_harvest took a resolved author from the CLI and ignored the one stored in the session manifest, contradicting the fork --help. It now takes Option<&str>: an explicit harvest --author wins, otherwise the fork author is used. ws refresh already read the stored author; sc work has no stored value. --- .claude/skills/sc-cli-reference/SKILL.md | 4 +- crates/cli/src/main.rs | 6 +- crates/repo/src/file_dir_collision.rs | 4 +- crates/repo/src/partial_sessions.rs | 18 ++-- crates/repo/src/partial_size.rs | 4 +- crates/repo/src/ws.rs | 103 ++++++++++++++++------- crates/repo/src/ws_refresh.rs | 22 ++--- crates/repo/tests/exec_bit.rs | 2 +- 8 files changed, 104 insertions(+), 59 deletions(-) diff --git a/.claude/skills/sc-cli-reference/SKILL.md b/.claude/skills/sc-cli-reference/SKILL.md index 56dc1753..998d11fe 100644 --- a/.claude/skills/sc-cli-reference/SKILL.md +++ b/.claude/skills/sc-cli-reference/SKILL.md @@ -167,8 +167,8 @@ cargo run --bin sc -- work --agents 3 [--name ] [--budget-mb ] -- bash demo/run_work_demo.sh # parallel-agents round-trip proof cargo run --bin sc -- ws fork --agents N [--name ] [--identity ] [--author ] # durable multi-invocation # (--agents default 2; --author is stored in the - # manifest, but harvest takes its own --author, - # then $SC_AUTHOR, then the OS user) + # manifest and is the harvest default; an explicit + # harvest --author wins) # session: materializes N checkouts under # .sc/ws// and persists a manifest (P20). # --name opens a further named session in diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index 8f84dd5e..48b26201 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -832,8 +832,8 @@ enum WsOp { /// on both sides (ciphertext-id fast paths need no identity at all). #[arg(long)] identity: Option, - /// Author recorded on any landing commit (default $SC_AUTHOR, then - /// the OS username). + /// Author recorded on any landing commit (default: the author given to + /// `sc ws fork` for this session). #[arg(long)] author: Option, /// Attach this transcript file to each workspace's landed snapshot @@ -3620,7 +3620,7 @@ fn run_ws(op: WsOp) -> Result<()> { let outcomes = repo.ws_harvest( session.as_deref(), into.as_deref(), - &resolve_author(author), + author.as_deref(), sk.as_ref(), )?; diff --git a/crates/repo/src/file_dir_collision.rs b/crates/repo/src/file_dir_collision.rs index 25de1931..d7943f39 100644 --- a/crates/repo/src/file_dir_collision.rs +++ b/crates/repo/src/file_dir_collision.rs @@ -172,7 +172,7 @@ fn ws_harvest_refuses_a_directory_over_an_absent_protected_file() { let dir = s.workspaces[0].dir.clone(); assert!(!dir.join("secret").exists()); put(&dir, "secret/agent.txt", "agent\n"); - let err = repo.ws_harvest(None, None, "t", None).unwrap_err(); + let err = repo.ws_harvest(None, None, Some("t"), None).unwrap_err(); assert_collision(&err, "secret", "secret/agent.txt"); assert_eq!(repo.head_tip().unwrap(), Some(c1), "no ref moved"); assert!( @@ -184,7 +184,7 @@ fn ws_harvest_refuses_a_directory_over_an_absent_protected_file() { // The session is still open: removing the directory lets it harvest. std::fs::remove_dir_all(dir.join("secret")).unwrap(); put(&dir, "other.txt", "ok\n"); - let out = repo.ws_harvest(None, None, "t", None).unwrap(); + let out = repo.ws_harvest(None, None, Some("t"), None).unwrap(); assert_eq!(out.len(), 1); cleanup(repo, &root); } diff --git a/crates/repo/src/partial_sessions.rs b/crates/repo/src/partial_sessions.rs index 412d5097..8e5081d9 100644 --- a/crates/repo/src/partial_sessions.rs +++ b/crates/repo/src/partial_sessions.rs @@ -48,7 +48,7 @@ fn harvest_lands_a_clean_merge_and_carries_the_out_of_view_subtrees() { std::fs::write(fx.clone_root.join("src/a.txt"), SRC_LAST).unwrap(); let host_tip = clone.commit("t", "host edit").unwrap(); - let outcomes = clone.ws_harvest(None, None, "t", None).unwrap(); + let outcomes = clone.ws_harvest(None, None, Some("t"), None).unwrap(); let merged = match outcomes.as_slice() { [WsHarvestOutcome::Landed { merged_tip, .. }] => *merged_tip, other => panic!("expected a clean landing, got {other:?}"), @@ -82,7 +82,7 @@ fn harvest_conflict_falls_back_to_a_branch() { std::fs::write(fx.clone_root.join("src/a.txt"), b"uno\ntwo\nthree\n").unwrap(); let host_tip = clone.commit("t", "host edit").unwrap(); - let outcomes = clone.ws_harvest(None, None, "t", None).unwrap(); + let outcomes = clone.ws_harvest(None, None, Some("t"), None).unwrap(); match outcomes.as_slice() { [WsHarvestOutcome::FallbackBranch { branch, .. }] => { assert_eq!(branch, "work-1"); @@ -125,7 +125,7 @@ fn two_named_sessions_land_independently_under_the_view() { std::fs::write(b.workspaces[0].dir.join("src/a.txt"), SRC_LAST).unwrap(); let first = match clone - .ws_harvest(Some("a"), None, "t", None) + .ws_harvest(Some("a"), None, Some("t"), None) .unwrap() .as_slice() { @@ -137,7 +137,7 @@ fn two_named_sessions_land_independently_under_the_view() { "b is still open" ); let merged = match clone - .ws_harvest(Some("b"), None, "t", None) + .ws_harvest(Some("b"), None, Some("t"), None) .unwrap() .as_slice() { @@ -162,7 +162,7 @@ fn harvest_of_an_untouched_workspace_is_unchanged() { let fx = Fx::origin("ws-unchanged", |_| {}); let clone = fx.clone(false); clone.ws_fork(None, 1, "t", None).unwrap(); - let outcomes = clone.ws_harvest(None, None, "t", None).unwrap(); + let outcomes = clone.ws_harvest(None, None, Some("t"), None).unwrap(); assert!(matches!( outcomes.as_slice(), [WsHarvestOutcome::Unchanged { .. }] @@ -290,7 +290,7 @@ fn a_failing_probe_leaves_no_candidate_branch() { Some(b1) ); - let err = clone.ws_harvest(None, None, "t", None).unwrap_err(); + let err = clone.ws_harvest(None, None, Some("t"), None).unwrap_err(); eprintln!("probe error: {err:?}"); assert!( refs::read_branch_tip(clone.layout(), "work-1") @@ -349,7 +349,11 @@ fn refresh_moves_a_workspace_without_reading_the_rest() { assert!(!dir.join("docs").exists(), "docs is outside the view"); assert!(!has(&clone, &new_docs), "and was never read or fetched"); - let merged = match clone.ws_harvest(None, None, "t", None).unwrap().as_slice() { + let merged = match clone + .ws_harvest(None, None, Some("t"), None) + .unwrap() + .as_slice() + { [WsHarvestOutcome::Landed { merged_tip, .. }] => *merged_tip, other => panic!("expected a landing, got {other:?}"), }; diff --git a/crates/repo/src/partial_size.rs b/crates/repo/src/partial_size.rs index 8027744e..9a1059d2 100644 --- a/crates/repo/src/partial_size.rs +++ b/crates/repo/src/partial_size.rs @@ -908,7 +908,7 @@ fn workspace_session_on_a_size_limited_clone_lands() { ) .unwrap(); clone.commit("t", "host edit").unwrap(); - let outcomes = clone.ws_harvest(name, None, "t", None).unwrap(); + let outcomes = clone.ws_harvest(name, None, Some("t"), None).unwrap(); assert!( matches!(outcomes.as_slice(), [WsHarvestOutcome::Landed { .. }]), "got {outcomes:?}" @@ -1097,7 +1097,7 @@ fn named_sessions_on_a_size_limited_clone() { ) .unwrap(); for name in ["a", "b"] { - let out = clone.ws_harvest(Some(name), None, "t", None).unwrap(); + let out = clone.ws_harvest(Some(name), None, Some("t"), None).unwrap(); assert!( matches!(out.as_slice(), [WsHarvestOutcome::Landed { .. }]), "{name}: {out:?}" diff --git a/crates/repo/src/ws.rs b/crates/repo/src/ws.rs index b9899359..b2d6ba7f 100644 --- a/crates/repo/src/ws.rs +++ b/crates/repo/src/ws.rs @@ -922,7 +922,7 @@ impl Repo { &self, name: Option<&str>, into: Option<&str>, - author: &str, + author: Option<&str>, identity: Option<&scl_crypto::SecretKey>, ) -> Result> { if crate::merge_state::in_progress(&self.layout) { @@ -943,6 +943,9 @@ impl Repo { ))); } + // An explicit author wins; otherwise the one recorded at fork. + let author = author.unwrap_or(&session.author).to_string(); + let author = author.as_str(); let landing = into.unwrap_or(&session.base_branch).to_string(); let current = refs::current_branch(self.layout())?; if current != landing { @@ -1381,7 +1384,7 @@ mod tests { // Harvest carries the untouched out-of-sparse subtree verbatim. std::fs::write(entry.dir.join("src/a.txt"), b"a v2").unwrap(); - let outcomes = repo.ws_harvest(None, None, "t", None).unwrap(); + let outcomes = repo.ws_harvest(None, None, Some("t"), None).unwrap(); assert_eq!(outcomes.len(), 1); match &outcomes[0] { WsHarvestOutcome::Landed { index: 1, .. } => {} @@ -1454,7 +1457,7 @@ mod tests { // 4. Harvest. The workspace's absent docs/x.txt must be carried // (never materialized, not a user deletion), not committed as a // deletion. - let outcomes = repo.ws_harvest(None, None, "t", None).unwrap(); + let outcomes = repo.ws_harvest(None, None, Some("t"), None).unwrap(); assert_eq!(outcomes.len(), 1); match &outcomes[0] { WsHarvestOutcome::Landed { index: 1, .. } => {} @@ -1540,7 +1543,7 @@ mod tests { // Harvesting the untouched workspaces lands nothing and leaves the // base branch tip unchanged. let base_before = repo.head_tip().unwrap().unwrap(); - let outcomes = repo.ws_harvest(None, None, "t", None).unwrap(); + let outcomes = repo.ws_harvest(None, None, Some("t"), None).unwrap(); assert!(outcomes .iter() .all(|o| matches!(o, WsHarvestOutcome::Unchanged { .. }))); @@ -1688,7 +1691,7 @@ mod tests { std::fs::write(dir1.join("p1.txt"), "one edited\n").unwrap(); std::fs::write(dir2.join("p2.txt"), "two edited\n").unwrap(); - let outcomes = repo.ws_harvest(None, None, "t", Some(&sk)).unwrap(); + let outcomes = repo.ws_harvest(None, None, Some("t"), Some(&sk)).unwrap(); assert_eq!(outcomes.len(), 2); for o in &outcomes { let merged_tip = match o { @@ -1739,7 +1742,7 @@ mod tests { std::fs::remove_file(dir.join("secret/gone")).unwrap(); // An AWS-style key id trips the P5 scanner, so the workspace stays live. std::fs::write(dir.join("leak.txt"), "AKIAIOSFODNN7EXAMPLE\n").unwrap(); - let outcomes = repo.ws_harvest(None, None, "t", Some(&sk)).unwrap(); + let outcomes = repo.ws_harvest(None, None, Some("t"), Some(&sk)).unwrap(); assert!( matches!(outcomes[0], WsHarvestOutcome::Rejected { index: 1, .. }), "got {outcomes:?}" @@ -1974,7 +1977,7 @@ mod tests { std::fs::write(session.workspaces[1].dir.join("b.txt"), "new-b\n").unwrap(); let oplog_before = repo.oplog().unwrap().len(); - let outcomes = repo.ws_harvest(None, None, "t", None).unwrap(); + let outcomes = repo.ws_harvest(None, None, Some("t"), None).unwrap(); assert_eq!(outcomes.len(), 2); match &outcomes[0] { WsHarvestOutcome::Landed { index: 1, .. } => {} @@ -2040,7 +2043,7 @@ mod tests { std::fs::create_dir_all(session.workspaces[1].dir.join("n")).unwrap(); std::fs::write(session.workspaces[1].dir.join("n/child"), "nested\n").unwrap(); - let outcomes = repo.ws_harvest(None, None, "t", None).unwrap(); + let outcomes = repo.ws_harvest(None, None, Some("t"), None).unwrap(); assert_eq!(outcomes.len(), 2); assert!( matches!(&outcomes[0], WsHarvestOutcome::Landed { index: 1, .. }), @@ -2072,7 +2075,7 @@ mod tests { std::fs::write(root.join("x.txt"), "main-x\n").unwrap(); repo.commit("t", "conflict x on main").unwrap(); - let outcomes = repo.ws_harvest(None, None, "t", None).unwrap(); + let outcomes = repo.ws_harvest(None, None, Some("t"), None).unwrap(); assert_eq!(outcomes.len(), 2); match &outcomes[0] { WsHarvestOutcome::Landed { index: 1, .. } => {} @@ -2130,7 +2133,7 @@ mod tests { let session = repo.ws_fork(None, 1, "t", None).unwrap(); repo.switch("other").unwrap(); - let err = repo.ws_harvest(None, None, "t", None).unwrap_err(); + let err = repo.ws_harvest(None, None, Some("t"), None).unwrap_err(); match err { Error::InvalidArgument(msg) => { assert!( @@ -2174,7 +2177,9 @@ mod tests { repo.branch("work-2").unwrap(); // --into with the checked-out branch name behaves as default. - let outcomes = repo.ws_harvest(None, Some("main"), "t", None).unwrap(); + let outcomes = repo + .ws_harvest(None, Some("main"), Some("t"), None) + .unwrap(); match &outcomes[1] { WsHarvestOutcome::FallbackBranch { index: 2, branch } => { assert_eq!(branch, "work-2-2"); @@ -2185,7 +2190,7 @@ mod tests { // --into a non-checked-out branch errors. let _session2 = repo.ws_fork(None, 1, "t", None).unwrap(); let err = repo - .ws_harvest(None, Some("nonexistent-branch"), "t", None) + .ws_harvest(None, Some("nonexistent-branch"), Some("t"), None) .unwrap_err(); assert!(matches!(err, Error::InvalidArgument(_))); @@ -2202,14 +2207,14 @@ mod tests { // (a) merge/pick/rebase in progress -> typed errors. crate::merge_state::write(repo.layout(), &ObjectId::of(b"theirs"), &[], None).unwrap(); assert!(matches!( - repo.ws_harvest(None, None, "t", None), + repo.ws_harvest(None, None, Some("t"), None), Err(Error::MergeInProgress) )); crate::merge_state::clear(repo.layout()).unwrap(); crate::pick_state::write(repo.layout(), &ObjectId::of(b"picked"), &[], None, None).unwrap(); assert!(matches!( - repo.ws_harvest(None, None, "t", None), + repo.ws_harvest(None, None, Some("t"), None), Err(Error::PickInProgress) )); crate::pick_state::clear(repo.layout()).unwrap(); @@ -2229,7 +2234,7 @@ mod tests { }; crate::rebase_state::write(repo.layout(), &st).unwrap(); assert!(matches!( - repo.ws_harvest(None, None, "t", None), + repo.ws_harvest(None, None, Some("t"), None), Err(Error::RebaseInProgress) )); crate::rebase_state::clear(repo.layout()).unwrap(); @@ -2240,7 +2245,7 @@ mod tests { // Session intact. std::fs::write(session.workspaces[0].dir.join("a.txt"), "ws-edit\n").unwrap(); std::fs::write(root.join("a.txt"), "dirty-uncommitted\n").unwrap(); - let err = repo.ws_harvest(None, None, "t", None).unwrap_err(); + let err = repo.ws_harvest(None, None, Some("t"), None).unwrap_err(); assert!(matches!(err, Error::InvalidArgument(_))); let after = repo @@ -2278,7 +2283,7 @@ mod tests { ) .unwrap(); - let outcomes = repo.ws_harvest(None, None, "t", None).unwrap(); + let outcomes = repo.ws_harvest(None, None, Some("t"), None).unwrap(); assert_eq!(outcomes.len(), 2); match &outcomes[0] { WsHarvestOutcome::Landed { index: 1, .. } => {} @@ -2331,7 +2336,7 @@ mod tests { // Fixing the file then re-harvesting completes and ends the session. std::fs::remove_file(e2.dir.join("leak.txt")).unwrap(); - let outcomes2 = repo.ws_harvest(None, None, "t", None).unwrap(); + let outcomes2 = repo.ws_harvest(None, None, Some("t"), None).unwrap(); assert_eq!(outcomes2.len(), 1); match &outcomes2[0] { WsHarvestOutcome::Unchanged { index: 2 } => {} @@ -2356,7 +2361,7 @@ mod tests { // the re-minted candidate id matches regardless of load. crate::repo::TEST_NOW.with(|c| c.set(Some(1_700_000_000))); - let outcomes = repo.ws_harvest(None, None, "t", None).unwrap(); + let outcomes = repo.ws_harvest(None, None, Some("t"), None).unwrap(); let landed_tip = match &outcomes[0] { WsHarvestOutcome::Landed { index: 1, @@ -2394,7 +2399,7 @@ mod tests { // Re-harvest: no error, resolves as a no-op Landed at the existing // tip, and leaves no stray branch behind. - let outcomes2 = repo.ws_harvest(None, None, "t", None).unwrap(); + let outcomes2 = repo.ws_harvest(None, None, Some("t"), None).unwrap(); assert_eq!(outcomes2.len(), 1); match &outcomes2[0] { WsHarvestOutcome::Landed { @@ -2516,7 +2521,7 @@ mod tests { repo.ws_fork(Some("a"), 1, "t", None).unwrap(); // No unnamed session: the default does not silently pick a named one. for err in [ - repo.ws_harvest(None, None, "t", None).unwrap_err(), + repo.ws_harvest(None, None, Some("t"), None).unwrap_err(), repo.ws_abandon(None, None).unwrap_err(), repo.ws_run(None, 1, &["true".into()], false, None) .unwrap_err(), @@ -2526,7 +2531,10 @@ mod tests { }; assert!(m.contains("open: a") && m.contains("--session"), "{m}"); } - match repo.ws_harvest(Some("nope"), None, "t", None).unwrap_err() { + match repo + .ws_harvest(Some("nope"), None, Some("t"), None) + .unwrap_err() + { Error::InvalidArgument(m) => assert!(m.contains("named 'nope'"), "{m}"), other => panic!("{other:?}"), } @@ -2546,12 +2554,14 @@ mod tests { fork_and_edit(&repo, Some("b"), "a.txt", "from b\n"); let base = repo.head_tip().unwrap().unwrap(); - let out = repo.ws_harvest(Some(first), None, "t", None).unwrap(); + let out = repo.ws_harvest(Some(first), None, Some("t"), None).unwrap(); assert!(matches!( out[..], [WsHarvestOutcome::Landed { index: 1, .. }] )); - let out = repo.ws_harvest(Some(second), None, "t", None).unwrap(); + let out = repo + .ws_harvest(Some(second), None, Some("t"), None) + .unwrap(); match &out[..] { [WsHarvestOutcome::FallbackBranch { index: 1, branch }] => { assert_eq!(*branch, format!("{second}-1")); @@ -2588,12 +2598,14 @@ mod tests { fork_and_edit(&repo, Some("a"), "file-a.txt", "A\n"); fork_and_edit(&repo, Some("b"), "file-b.txt", "B\n"); - repo.ws_harvest(Some(first), None, "t", None).unwrap(); + repo.ws_harvest(Some(first), None, Some("t"), None).unwrap(); let one = repo.head_tip().unwrap().unwrap(); // The other session is untouched by the first landing. assert!(repo.ws_session(Some(second)).unwrap().unwrap().workspaces[0].live); - let out = repo.ws_harvest(Some(second), None, "t", None).unwrap(); + let out = repo + .ws_harvest(Some(second), None, Some("t"), None) + .unwrap(); let tip = match out[..] { [WsHarvestOutcome::Landed { merged_tip, .. }] => merged_tip, ref other => panic!("expected a landing, got {other:?}"), @@ -2647,7 +2659,7 @@ mod tests { .is_err()); // ...and still harvests. - let out = repo.ws_harvest(Some("b"), None, "t", None).unwrap(); + let out = repo.ws_harvest(Some("b"), None, Some("t"), None).unwrap(); assert!(matches!( out[..], [WsHarvestOutcome::Landed { index: 1, .. }] @@ -2660,6 +2672,30 @@ mod tests { std::fs::remove_dir_all(&root).unwrap(); } + #[test] + fn harvest_defaults_to_the_fork_author_and_an_explicit_one_wins() { + // #286: the author recorded at fork is the harvest default, for the + // unnamed and for a named session alike; `--author` at harvest wins. + for name in [None, Some("feat")] { + for (harvest_author, want) in [(None, "alice"), (Some("bob"), "bob")] { + let root = tmp_root(&format!("author-{name:?}-{harvest_author:?}")); + let repo = init(&root); + let s = repo.ws_fork(name, 1, "alice", None).unwrap(); + assert_eq!(s.author, "alice"); + std::fs::write(s.workspaces[0].dir.join("a.txt"), "edited\n").unwrap(); + let out = repo.ws_harvest(name, None, harvest_author, None).unwrap(); + assert!(matches!( + out[..], + [WsHarvestOutcome::Landed { index: 1, .. }] + )); + let tip = repo.head_tip().unwrap().unwrap(); + assert_eq!(repo.snapshot(&tip).unwrap().author, want); + drop(repo); + std::fs::remove_dir_all(&root).unwrap(); + } + } + } + #[test] fn abandoning_one_workspace_of_a_named_session_keeps_its_siblings() { let root = tmp_root("named-abandon-one"); @@ -2699,7 +2735,9 @@ mod tests { std::fs::write(root.join("a.txt"), "host\n").unwrap(); repo.commit("t", "host edit").unwrap(); repo.branch("feat-1").unwrap(); - let out = repo.ws_harvest(Some("feat"), None, "t", None).unwrap(); + let out = repo + .ws_harvest(Some("feat"), None, Some("t"), None) + .unwrap(); match &out[..] { [WsHarvestOutcome::FallbackBranch { branch, .. }] => assert_eq!(branch, "feat-1-2"), other => panic!("{other:?}"), @@ -2769,19 +2807,22 @@ mod tests { Error::InvalidArgument(m) => assert!(m.contains("still running"), "{m}"), other => panic!("{other:?}"), }; - busy(repo.ws_harvest(Some("a"), None, "t", None).unwrap_err()); + busy( + repo.ws_harvest(Some("a"), None, Some("t"), None) + .unwrap_err(), + ); busy(repo.ws_abandon(Some("a"), None).unwrap_err()); busy(repo.ws_abandon(Some("a"), Some(1)).unwrap_err()); // A second run in the same workspace is refused too. busy(repo.ws_run_plan(Some("a"), 1, false, None).err().unwrap()); // Nothing was touched, and the other session is unaffected. assert!(repo.ws_session(Some("a")).unwrap().unwrap().workspaces[0].live); - let out = repo.ws_harvest(Some("b"), None, "t", None).unwrap(); + let out = repo.ws_harvest(Some("b"), None, Some("t"), None).unwrap(); assert!(matches!(out[..], [WsHarvestOutcome::Landed { .. }])); // Once the command is over, the workspace harvests normally. assert_eq!(plan.run(&["true".into()]).unwrap(), 0); - let out = repo.ws_harvest(Some("a"), None, "t", None).unwrap(); + let out = repo.ws_harvest(Some("a"), None, Some("t"), None).unwrap(); assert!(matches!(out[..], [WsHarvestOutcome::Landed { .. }])); drop(repo); diff --git a/crates/repo/src/ws_refresh.rs b/crates/repo/src/ws_refresh.rs index cbd9bbe2..e5c3c588 100644 --- a/crates/repo/src/ws_refresh.rs +++ b/crates/repo/src/ws_refresh.rs @@ -1122,7 +1122,7 @@ mod tests { // The listing now shows the new base. let s = repo.ws_session(None).unwrap().unwrap(); assert_eq!((s.base_branch.as_str(), s.base_snapshot), ("main", tip)); - let out = repo.ws_harvest(None, None, "t", None).unwrap(); + let out = repo.ws_harvest(None, None, Some("t"), None).unwrap(); assert!(out .iter() .all(|o| matches!(o, WsHarvestOutcome::Unchanged { .. }))); @@ -1170,7 +1170,7 @@ mod tests { "the edits are still edits" ); - let out = repo.ws_harvest(None, None, "t", None).unwrap(); + let out = repo.ws_harvest(None, None, Some("t"), None).unwrap(); let merged = match out.as_slice() { [WsHarvestOutcome::Landed { merged_tip, .. }] => *merged_tip, other => panic!("expected a landing, got {other:?}"), @@ -1206,7 +1206,7 @@ mod tests { "the workspace's deletion stays" ); assert!(dir.join("stay.txt").exists()); - let out = repo.ws_harvest(None, None, "t", None).unwrap(); + let out = repo.ws_harvest(None, None, Some("t"), None).unwrap(); let [WsHarvestOutcome::Landed { merged_tip, .. }] = out.as_slice() else { panic!("expected a landing, got {out:?}") }; @@ -1246,7 +1246,7 @@ mod tests { assert!(!d1.join("b.txt").exists() && !d2.join("b.txt").exists()); assert_eq!(read(&d2, "other.txt"), "two\n"); // Harvest still works exactly as before: one falls back, one lands. - let out = repo.ws_harvest(None, None, "t", None).unwrap(); + let out = repo.ws_harvest(None, None, Some("t"), None).unwrap(); assert!( matches!(out[0], WsHarvestOutcome::FallbackBranch { .. }), "{out:?}" @@ -1411,7 +1411,7 @@ mod tests { // Harvest would read the refreshed b.txt as the agent's own change, and // re-apply it even after the host reverts it: it refuses instead. let err = repo - .ws_harvest(None, None, "t", None) + .ws_harvest(None, None, Some("t"), None) .unwrap_err() .to_string(); assert!( @@ -1432,7 +1432,7 @@ mod tests { !dirs[0].join("b.txt").exists(), "the host's revert is honoured" ); - let out = repo.ws_harvest(None, None, "t", None).unwrap(); + let out = repo.ws_harvest(None, None, Some("t"), None).unwrap(); assert!( out.iter() .all(|o| matches!(o, WsHarvestOutcome::Landed { .. })), @@ -1561,7 +1561,7 @@ mod tests { for d in &dirs { assert_eq!(read(d, "b.txt"), "b\n"); } - let out = repo.ws_harvest(None, None, "t", None).unwrap(); + let out = repo.ws_harvest(None, None, Some("t"), None).unwrap(); assert!(matches!(out[0], WsHarvestOutcome::Landed { .. }), "{out:?}"); let head = repo.head_tip().unwrap().unwrap(); assert_eq!( @@ -1584,7 +1584,7 @@ mod tests { repo.ws_refresh(None, None, None).unwrap(); assert_eq!(read(&dirs[0], "a.txt"), "one\ntwo\nthree\nFOUR\nfive\n"); assert!(!dirs[0].join("b.txt").exists()); - let out = repo.ws_harvest(None, None, "t", None).unwrap(); + let out = repo.ws_harvest(None, None, Some("t"), None).unwrap(); assert!(matches!(out[0], WsHarvestOutcome::Landed { .. }), "{out:?}"); let head = repo.head_tip().unwrap().unwrap(); assert_eq!( @@ -2213,7 +2213,7 @@ mod tests { // logs/ is tracked on the tip, so a harvest still reads it. repo.ws_refresh(None, None, None).unwrap(); assert_eq!(read(&dir, "logs/a.log"), "agent log edit\n"); - let out = repo.ws_harvest(None, None, "t", None).unwrap(); + let out = repo.ws_harvest(None, None, Some("t"), None).unwrap(); let [WsHarvestOutcome::Landed { merged_tip, .. }] = out.as_slice() else { panic!("expected a landing, got {out:?}") }; @@ -2483,7 +2483,7 @@ mod tests { // untouched workspace is not spuriously changed; the protected edit is. assert!(!repo.ws_changed(None, &session.workspaces[0]).unwrap()); assert!(repo.ws_changed(None, &session.workspaces[1]).unwrap()); - let out = repo.ws_harvest(None, None, "t", Some(&sk)).unwrap(); + let out = repo.ws_harvest(None, None, Some("t"), Some(&sk)).unwrap(); assert!( matches!(out[0], WsHarvestOutcome::Unchanged { .. }), "{out:?}" @@ -2516,7 +2516,7 @@ mod tests { repo.ws_refresh(None, None, None).unwrap(); assert_eq!(read(&dir, "src/b.txt"), "b\n"); assert!(!dir.join("docs").exists(), "outside the session's view"); - let out = repo.ws_harvest(None, None, "t", None).unwrap(); + let out = repo.ws_harvest(None, None, Some("t"), None).unwrap(); let [WsHarvestOutcome::Landed { merged_tip, .. }] = out.as_slice() else { panic!("expected a landing, got {out:?}") }; diff --git a/crates/repo/tests/exec_bit.rs b/crates/repo/tests/exec_bit.rs index 87eb06e3..c4c8f962 100644 --- a/crates/repo/tests/exec_bit.rs +++ b/crates/repo/tests/exec_bit.rs @@ -217,7 +217,7 @@ fn ws_checkout_keeps_exec_bit() { assert!(!on_disk_exec(dir, "plain.txt")); // Harvesting an edit of another file must not demote the script. std::fs::write(dir.join("plain.txt"), "edited").unwrap(); - repo.ws_harvest(None, None, "t", None).unwrap(); + repo.ws_harvest(None, None, Some("t"), None).unwrap(); assert_eq!(committed(&repo, "run.sh"), FileMode::EXEC); assert_eq!(committed(&repo, "plain.txt"), FileMode::FILE); } From 1e4ea9290ff6c61fdbedf0f1b801f1d4e0d3c1e1 Mon Sep 17 00:00:00 2001 From: Toni Bergholm Date: Wed, 21 Oct 2026 11:39:12 +0300 Subject: [PATCH 2/2] test(repo): windows-safe temp dir name and residue assertion in ws author test (#286) --- crates/repo/src/ws.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/repo/src/ws.rs b/crates/repo/src/ws.rs index b2d6ba7f..3cfe3537 100644 --- a/crates/repo/src/ws.rs +++ b/crates/repo/src/ws.rs @@ -2678,7 +2678,11 @@ mod tests { // unnamed and for a named session alike; `--author` at harvest wins. for name in [None, Some("feat")] { for (harvest_author, want) in [(None, "alice"), (Some("bob"), "bob")] { - let root = tmp_root(&format!("author-{name:?}-{harvest_author:?}")); + let root = tmp_root(&format!( + "author-{}-{}", + name.unwrap_or("unnamed"), + harvest_author.unwrap_or("default") + )); let repo = init(&root); let s = repo.ws_fork(name, 1, "alice", None).unwrap(); assert_eq!(s.author, "alice"); @@ -2692,6 +2696,7 @@ mod tests { assert_eq!(repo.snapshot(&tip).unwrap().author, want); drop(repo); std::fs::remove_dir_all(&root).unwrap(); + assert!(!root.exists(), "test must leave no residue"); } } }