From 158cc655c205d7dffc7cccdb02261edb21187b01 Mon Sep 17 00:00:00 2001 From: Skolte <52416805+freezscholte@users.noreply.github.com> Date: Mon, 6 Jul 2026 14:53:40 +0200 Subject: [PATCH 1/2] =?UTF-8?q?fix(store):=20NER-382=20workspace=20drift?= =?UTF-8?q?=20guard=20=E2=80=94=20payload=20honesty,=20WORKSPACE=5FDRIFT?= =?UTF-8?q?=20refusal,=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit attempt attach silently discarded pre-attach edits inside .forge/worktrees//. This change (1) qualifies workspace_path as a materialization target in start/attempt-start payloads (additive workspace_role field + help text), (2) makes attach refuse with typed WORKSPACE_DRIFT (drifted paths in details, secret-filtered) unless --discard-workspace-changes is passed, via a read-only per-file equality check against the recorded materialized_content_ref, and (3) adds integration tests incl. the original silent-loss repro now failing loudly. Produced in the ccx contract pilot (experiment/ccx-spikes, experiments/ccx/RESULTS.md), Arm A stack; gates re-verified independently. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01GTeDdEu9Q4a96DdXK4tfju --- crates/forge-cli/src/args.rs | 17 ++ crates/forge-cli/src/commands/core.rs | 15 +- crates/forge-cli/src/schema.rs | 4 +- crates/forge-cli/tests/forge_attempts.rs | 271 +++++++++++++++++++++++ crates/forge-store/src/attempts.rs | 235 ++++++++++++++++++++ crates/forge-store/src/error.rs | 61 ++++- crates/forge-store/src/lib.rs | 5 +- 7 files changed, 601 insertions(+), 7 deletions(-) diff --git a/crates/forge-cli/src/args.rs b/crates/forge-cli/src/args.rs index 9dfac52..2fcbca9 100644 --- a/crates/forge-cli/src/args.rs +++ b/crates/forge-cli/src/args.rs @@ -217,13 +217,30 @@ pub(crate) struct AttemptArgs { #[derive(Debug, Subcommand)] pub(crate) enum AttemptCommand { + /// Start a new attempt for an existing intent. + /// + /// The reported workspace_path (.forge/worktrees/) is + /// materialized state, not an editing surface; the repo root worktree + /// (after `attempt attach`) is where edits belong. Start(AttemptStartArgs), List, Show { attempt_id: String, }, + /// Attach the repo root worktree to an attempt and materialize its tree there. + /// + /// The attempt's .forge/worktrees/ workspace dir is + /// materialized state, not an editing surface; the repo root worktree + /// (after attach) is where edits belong. Attach { attempt_id: String, + /// Proceed even when the attempt's workspace dir (.forge/worktrees/) + /// has drifted from its recorded materialized content, discarding the drifted + /// workspace edits (NER-382). Without this flag such an attach refuses with + /// WORKSPACE_DRIFT. Discards workspace-dir drift ONLY — it never bypasses the + /// repo-root DIRTY_WORKTREE or ATTEMPT_WORKTREE_MISMATCH protections. + #[arg(long)] + discard_workspace_changes: bool, }, /// Compare competing attempts (per intent) on verified evidence + rank them. Compare(CompareArgs), diff --git a/crates/forge-cli/src/commands/core.rs b/crates/forge-cli/src/commands/core.rs index 1b6568f..9dab360 100644 --- a/crates/forge-cli/src/commands/core.rs +++ b/crates/forge-cli/src/commands/core.rs @@ -166,7 +166,10 @@ pub(crate) fn attempt_response(request_id: Option, args: AttemptArgs) -> )) }) } - AttemptCommand::Attach { attempt_id } => { + AttemptCommand::Attach { + attempt_id, + discard_workspace_changes, + } => { command_result("attempt attach", request_id, |cwd, request_id| { // NER-134: worktree/base materialization goes through `ContentBackend`, // not `forge_content_git::` directly, so git-worktree semantics stay out @@ -191,6 +194,16 @@ pub(crate) fn attempt_response(request_id: Option, args: AttemptArgs) -> } .into()); } + // NER-382: refuse (WORKSPACE_DRIFT) BEFORE any materialization write when + // the target attempt's workspace dir no longer equals its recorded + // materialized content — re-materializing below would silently discard + // those workspace edits. Checked AFTER the switching-baseline dirty-check + // above, and only skipped by the explicit flag, so + // --discard-workspace-changes discards workspace-dir drift ONLY and can + // never bypass DIRTY_WORKTREE. + if !discard_workspace_changes { + forge_store::verify_attempt_workspace_undrifted(&cwd, &attempt_id)?; + } let content_ref = match forge_store::attempt_materialization_ref(&cwd, &attempt_id)? { Some(content_ref) => content_ref, diff --git a/crates/forge-cli/src/schema.rs b/crates/forge-cli/src/schema.rs index 796b025..c5f6126 100644 --- a/crates/forge-cli/src/schema.rs +++ b/crates/forge-cli/src/schema.rs @@ -55,8 +55,8 @@ fn envelope_shape() -> Value { fn command_shapes() -> Value { let commands = [ ("init", "Initializes a .forge repository; data carries root_path and the genesis operation."), - ("start", "Starts an intent + its first attempt; accepts repeatable --require gates and --require-tests-pass structured gates (which also require zero parsed failures) persisted on the intent, and an optional --actor; data carries the started attempt + operation_id."), - ("attempt start", "Starts a new attempt for an existing intent; data carries the attempt + operation_id."), + ("start", "Starts an intent + its first attempt; accepts repeatable --require gates and --require-tests-pass structured gates (which also require zero parsed failures) persisted on the intent, and an optional --actor; data carries the started attempt + operation_id, with workspace_path qualified by workspace_role=\"materialization_target\" (the workspace dir is materialized state, not an editing surface; edits belong in the repo root worktree)."), + ("attempt start", "Starts a new attempt for an existing intent; data carries the attempt + operation_id, with workspace_path qualified by workspace_role=\"materialization_target\" (the workspace dir is materialized state, not an editing surface; edits belong in the repo root worktree after `attempt attach`)."), ("attempt list", "Lists attempts; data carries { attempts: [...] }."), ("attempt show", "Shows one attempt; data carries the attempt detail."), ("attempt attach", "Attaches the active view to an attempt; data carries attempt_id, content_ref, current_view_id."), diff --git a/crates/forge-cli/tests/forge_attempts.rs b/crates/forge-cli/tests/forge_attempts.rs index 92c810f..8bd7b03 100644 --- a/crates/forge-cli/tests/forge_attempts.rs +++ b/crates/forge-cli/tests/forge_attempts.rs @@ -273,6 +273,10 @@ fn native_attempts_surface_and_materialize_workspace_paths() { ); let workspace_path = started["data"]["workspace_path"].as_str().unwrap(); assert!(workspace_path.starts_with(".forge/worktrees/")); + assert_eq!( + started["data"]["workspace_role"], "materialization_target", + "start must qualify workspace_path as a materialization target (NER-382)" + ); let workspace = repo.path().join(workspace_path); assert!(workspace .join(forge_content::WORKSPACE_MARKER_FILE) @@ -313,6 +317,61 @@ fn native_attempts_surface_and_materialize_workspace_paths() { ); } +/// NER-382: `attempt start` must qualify `workspace_path` with +/// `workspace_role: "materialization_target"` in both the fresh payload and the +/// idempotent `--request-id` replay (which is rebuilt from the stored +/// `replay_data`, a separate emission site from the fresh `StartAttempt`). +#[test] +fn attempt_start_payload_and_replay_carry_workspace_role() { + let repo = TestRepo::new_git(); + repo.forge().args(["--json", "init"]).assert().success(); + let started = json_output( + repo.forge() + .args(["--json", "start", "roles"]) + .assert() + .success(), + ); + let intent_id = started["data"]["intent_id"].as_str().unwrap(); + + let second = json_output( + repo.forge() + .args([ + "--json", + "--request-id", + "attempt-start-once", + "attempt", + "start", + "--intent", + intent_id, + ]) + .assert() + .success(), + ); + assert_eq!(second["data"]["workspace_role"], "materialization_target"); + + let replay = json_output( + repo.forge() + .args([ + "--json", + "--request-id", + "attempt-start-once", + "attempt", + "start", + "--intent", + intent_id, + ]) + .assert() + .success(), + ); + assert_eq!(replay["data"]["idempotent_replay"], true); + assert_eq!(replay["data"]["attempt_id"], second["data"]["attempt_id"]); + assert_eq!( + replay["data"]["workspace_path"], + second["data"]["workspace_path"] + ); + assert_eq!(replay["data"]["workspace_role"], "materialization_target"); +} + #[test] fn native_attempt_workspaces_are_isolated_and_bind_saves() { let repo = TestRepo::new_git(); @@ -894,3 +953,215 @@ fn restore_rejects_cross_attempt_snapshot() { .assert() .success(); } + +/// NER-382 drift-guard fixture: a native repo with a competing second attempt whose +/// workspace dir was materialized (so `attempt_workspaces.materialized_content_ref` +/// records a `forge-tree:` ref) but which is not attached yet. Returns the repo, the +/// second attempt's id, and its workspace dir under `.forge/worktrees/`. +fn native_repo_with_competing_attempt() -> (TestRepo, String, std::path::PathBuf) { + let repo = TestRepo::new_git(); + repo.forge() + .args(["--json", "init", "--content-backend", "native"]) + .assert() + .success(); + let first = json_output( + repo.forge() + .args(["--json", "start", "drift guard"]) + .assert() + .success(), + ); + let intent_id = first["data"]["intent_id"].as_str().unwrap(); + let second = json_output( + repo.forge() + .args(["--json", "attempt", "start", "--intent", intent_id]) + .assert() + .success(), + ); + let second_attempt = second["data"]["attempt_id"].as_str().unwrap().to_string(); + let workspace = repo + .path() + .join(second["data"]["workspace_path"].as_str().unwrap()); + (repo, second_attempt, workspace) +} + +#[test] +fn attach_refuses_drifted_attempt_workspace() { + // NER-382 silent-loss repro: edits made inside .forge/worktrees// before + // `attempt attach` used to be clobbered without a trace by re-materialization. + // Attach must now fail loudly with the typed WORKSPACE_DRIFT error listing the + // drifted paths, and the refusal must leave both the workspace dir and the repo + // root untouched (refusal happens BEFORE any materialization write). + let (repo, second_attempt, workspace) = native_repo_with_competing_attempt(); + std::fs::write(workspace.join("README.md"), "drifted edit\n").expect("drift edit"); + std::fs::write(workspace.join("EXTRA.md"), "added in workspace\n").expect("drift add"); + + let drift = json_output( + repo.forge() + .args(["--json", "attempt", "attach", &second_attempt]) + .assert() + .failure(), + ); + assert_eq!(drift["errors"][0]["code"], "WORKSPACE_DRIFT"); + assert_eq!(drift["retry"]["retryable"], false); + let paths: Vec<&str> = drift["errors"][0]["details"]["paths"] + .as_array() + .expect("drift details carry a paths list") + .iter() + .map(|path| path.as_str().unwrap()) + .collect(); + assert!( + paths.contains(&"README.md"), + "modified workspace file must be listed as drifted: {paths:?}" + ); + assert!( + paths.contains(&"EXTRA.md"), + "file added to the workspace must be listed as drifted: {paths:?}" + ); + + assert_eq!( + std::fs::read_to_string(workspace.join("README.md")).unwrap(), + "drifted edit\n", + "a refused attach must not touch the drifted workspace" + ); + assert!(workspace.join("EXTRA.md").exists()); + assert_eq!( + std::fs::read_to_string(repo.path().join("README.md")).unwrap(), + "hello\n", + "a refused attach must not touch the repo-root worktree" + ); +} + +#[test] +fn attach_discard_workspace_changes_discards_drift() { + // NER-382: the documented escape hatch. On the same drifted state, + // `attempt attach --discard-workspace-changes` succeeds and the drifted + // workspace content is re-materialized away. + let (repo, second_attempt, workspace) = native_repo_with_competing_attempt(); + std::fs::write(workspace.join("README.md"), "drifted edit\n").expect("drift edit"); + std::fs::write(workspace.join("EXTRA.md"), "added in workspace\n").expect("drift add"); + + let attached = json_output( + repo.forge() + .args([ + "--json", + "attempt", + "attach", + &second_attempt, + "--discard-workspace-changes", + ]) + .assert() + .success(), + ); + assert_eq!(attached["data"]["attempt_id"], second_attempt.as_str()); + + assert_eq!( + std::fs::read_to_string(workspace.join("README.md")).unwrap(), + "hello\n", + "the drifted edit is discarded by re-materialization" + ); + assert!( + !workspace.join("EXTRA.md").exists(), + "a file added to the workspace is discarded by re-materialization" + ); +} + +#[test] +fn attach_without_workspace_drift_behaves_as_before() { + // NER-382 invariant: attach with NO drift is unchanged — success, no new + // warnings, and the target's base materialized into the repo root as always. + let (repo, second_attempt, workspace) = native_repo_with_competing_attempt(); + + let attached = json_output( + repo.forge() + .args(["--json", "attempt", "attach", &second_attempt]) + .assert() + .success(), + ); + assert_eq!(attached["data"]["attempt_id"], second_attempt.as_str()); + assert!( + attached["warnings"].as_array().unwrap().is_empty(), + "a drift-free attach must not emit new warnings: {}", + attached["warnings"] + ); + assert_eq!( + std::fs::read_to_string(workspace.join("README.md")).unwrap(), + "hello\n" + ); + assert_eq!( + std::fs::read_to_string(repo.path().join("README.md")).unwrap(), + "hello\n" + ); +} + +#[test] +fn attach_skips_drift_check_when_workspace_never_materialized() { + // NER-382: with a NULL/absent materialized_content_ref there is no baseline to + // drift from, so attach proceeds without the check. The git backend is the + // natural such state: its workspace dirs hold only the marker file and never + // record a materialized ref, so even stray content inside the dir is not drift. + let repo = TestRepo::new_git(); + repo.forge().args(["--json", "init"]).assert().success(); + let first = json_output( + repo.forge() + .args(["--json", "start", "no baseline"]) + .assert() + .success(), + ); + let intent_id = first["data"]["intent_id"].as_str().unwrap(); + let second = json_output( + repo.forge() + .args(["--json", "attempt", "start", "--intent", intent_id]) + .assert() + .success(), + ); + let second_attempt = second["data"]["attempt_id"].as_str().unwrap(); + let workspace = repo + .path() + .join(second["data"]["workspace_path"].as_str().unwrap()); + assert!( + workspace + .join(forge_content::WORKSPACE_MARKER_FILE) + .exists(), + "the workspace dir exists (marker only) but was never materialized" + ); + std::fs::write(workspace.join("STRAY.md"), "not a baseline\n").expect("write stray"); + + repo.forge() + .args(["--json", "attempt", "attach", second_attempt]) + .assert() + .success(); +} + +/// NER-382: both payload emission sites qualify `workspace_path` with +/// `workspace_role: "materialization_target"` — `start` (auto-attach) and +/// `attempt start`. (The idempotent `--request-id` replay site is pinned by +/// `attempt_start_payload_and_replay_carry_workspace_role` above.) +#[test] +fn start_and_attempt_start_payloads_carry_workspace_role() { + let repo = TestRepo::new_git(); + repo.forge().args(["--json", "init"]).assert().success(); + + let started = json_output( + repo.forge() + .args(["--json", "start", "role qualifier"]) + .assert() + .success(), + ); + assert!(started["data"]["workspace_path"].is_string()); + assert_eq!(started["data"]["workspace_role"], "materialization_target"); + + let second = json_output( + repo.forge() + .args([ + "--json", + "attempt", + "start", + "--intent", + started["data"]["intent_id"].as_str().unwrap(), + ]) + .assert() + .success(), + ); + assert!(second["data"]["workspace_path"].is_string()); + assert_eq!(second["data"]["workspace_role"], "materialization_target"); +} diff --git a/crates/forge-store/src/attempts.rs b/crates/forge-store/src/attempts.rs index 396b5d2..67f091f 100644 --- a/crates/forge-store/src/attempts.rs +++ b/crates/forge-store/src/attempts.rs @@ -8,6 +8,11 @@ pub(crate) struct WorkspaceMarker { pub(crate) attempt_id: String, } +/// Role qualifier emitted next to `workspace_path` (NER-382): the workspace dir +/// is materialized state, not an editing surface. Enum-ready, but this is the +/// only value today. +pub const WORKSPACE_ROLE_MATERIALIZATION_TARGET: &str = "materialization_target"; + #[derive(Debug, Clone, Serialize)] pub struct StartAttempt { pub intent_id: String, @@ -15,6 +20,7 @@ pub struct StartAttempt { pub base_head: String, pub attached: bool, pub workspace_path: String, + pub workspace_role: &'static str, pub operation_id: String, pub current_view_id: String, } @@ -209,6 +215,7 @@ fn create_attempt( "base_head": base_head, "attached": attach, "workspace_path": workspace_rel_path_for_attempt(&attempt_id), + "workspace_role": WORKSPACE_ROLE_MATERIALIZATION_TARGET, } }), }, @@ -225,6 +232,7 @@ fn create_attempt( Ok(StartAttempt { intent_id, workspace_path: workspace_rel_path_for_attempt(&attempt_id), + workspace_role: WORKSPACE_ROLE_MATERIALIZATION_TARGET, attempt_id, base_head, attached: attach, @@ -561,3 +569,230 @@ pub fn attempt_base_head(cwd: &Path, attempt_id: &str) -> Result { })? .base_head) } + +/// NER-382 workspace drift guard: refuse `attempt attach` when the target attempt's +/// workspace dir no longer holds the content the store recorded materializing into it +/// (`attempt_workspaces.materialized_content_ref`) — re-materializing would silently +/// discard those workspace edits. This is an EQUALITY check, not a diff: the recorded +/// tree is walked via the native store's read primitives and each workspace file is +/// hash/byte-compared against it. Deliberately NOT `diff_working_vs_tree`, which +/// writes a status cache into the scanned root — this check reads the workspace dir +/// only and never mutates it, so a refusal leaves the workspace untouched. +/// +/// Skipped (Ok) when nothing was ever materialized: a NULL/absent +/// `materialized_content_ref` (including every git-backend workspace, which never +/// records one) has no baseline to drift from. +pub fn verify_attempt_workspace_undrifted(cwd: &Path, attempt_id: &str) -> Result<()> { + let context = open_repository(cwd)?; + attempt_by_id(&context, attempt_id)?.ok_or_else(|| ForgeError::UnknownAttempt { + selector: attempt_id.to_string(), + })?; + let connection = open_connection(&context.database_path)?; + let recorded: Option = connection + .query_row( + "SELECT materialized_content_ref FROM attempt_workspaces + WHERE repo_id = ?1 AND attempt_id = ?2", + params![context.repo_id, attempt_id], + |row| row.get::<_, Option>(0), + ) + .optional()? + .flatten(); + let Some(recorded) = recorded else { + return Ok(()); + }; + // Only native `forge-tree:` refs are ever recorded (the git arm of workspace + // materialization records nothing); stay permissive on any other family. + let Some(tree_id) = recorded.strip_prefix(forge_content::FORGE_TREE_PREFIX) else { + return Ok(()); + }; + let workspace = context + .root_path + .join(attempt_workspace_rel_path(&context, attempt_id)?); + let drifted = workspace_drift_paths(&context.root_path, &workspace, tree_id)?; + if drifted.is_empty() { + return Ok(()); + } + Err(ForgeError::WorkspaceDrift { paths: drifted }.into()) +} + +/// The tree-entry mode for a symlink (git's `120000`, mirroring the native backend's +/// symlink representation): the blob holds the link *target* bytes, so equality +/// compares the target string — the link is never followed. +const WORKSPACE_SYMLINK_MODE: u32 = 0o120000; + +/// Plain per-file equality comparison of a workspace dir against a recorded native +/// tree. Expected side: the recorded tree walked via `NativeObjectStore::read_object`, +/// skipping `is_ignored_by_policy` entries exactly like materialization does (those +/// were never written into the workspace, so they cannot have drifted). Actual side: +/// a read-only filesystem walk of the workspace dir under the same policy filter +/// (which also excludes the workspace marker and secret-risk names on both sides). +/// A path drifts when it is missing, added, of the wrong type, or its bytes hash to +/// a different blob id. Bytes/paths only — an executable-bit-only change is not +/// reported (the recorded tree pins content identity, not permissions). +fn workspace_drift_paths(repo_root: &Path, workspace: &Path, tree_id: &str) -> Result> { + let store = forge_content_native::NativeObjectStore::new(repo_root); + let root = forge_content_native::ObjectId::parse(tree_id)?; + let mut expected = std::collections::BTreeMap::new(); + collect_expected_tree_files(&store, &root, "", &mut expected)?; + let mut actual = std::collections::BTreeSet::new(); + if workspace.is_dir() { + collect_workspace_paths(workspace, workspace, &mut actual)?; + } + let mut drifted = std::collections::BTreeSet::new(); + for path in &actual { + if !expected.contains_key(path) { + drifted.insert(path.clone()); + } + } + for (path, (mode, object)) in &expected { + // Short-circuit: a recorded path absent from the walk is a deletion — no + // per-file compare needed. + let matches = actual.contains(path) + && workspace_file_matches_blob(&store, &workspace.join(path), *mode, object)?; + if !matches { + drifted.insert(path.clone()); + } + } + Ok(drifted.into_iter().collect()) +} + +/// Recursively collect `rel path -> (mode, blob object id)` for every file entry of a +/// recorded native tree, via the store's read primitives. The tree payload is the +/// content-addressed native tree object (`{ schema_version, entries: [{ name, kind, +/// mode, object }] }`); `read_object` has already verified its hash, so a malformed +/// shape here is store corruption, surfaced path-free. +fn collect_expected_tree_files( + store: &forge_content_native::NativeObjectStore, + tree_id: &forge_content_native::ObjectId, + prefix: &str, + out: &mut std::collections::BTreeMap, +) -> Result<()> { + let payload = store.read_object(tree_id)?; + let tree: Value = serde_json::from_slice(&payload)?; + let entries = tree + .get("entries") + .and_then(Value::as_array) + .ok_or_else(|| anyhow!("malformed native tree object"))?; + for entry in entries { + let name = entry + .get("name") + .and_then(Value::as_str) + .ok_or_else(|| anyhow!("malformed native tree entry"))?; + let kind = entry + .get("kind") + .and_then(Value::as_str) + .ok_or_else(|| anyhow!("malformed native tree entry"))?; + let mode = entry + .get("mode") + .and_then(Value::as_u64) + .ok_or_else(|| anyhow!("malformed native tree entry"))?; + let object = entry + .get("object") + .and_then(Value::as_str) + .ok_or_else(|| anyhow!("malformed native tree entry"))?; + let rel = if prefix.is_empty() { + name.to_string() + } else { + format!("{prefix}/{name}") + }; + if forge_content::is_ignored_by_policy(&rel) { + continue; + } + match kind { + "file" => { + out.insert(rel, (mode as u32, object.to_string())); + } + "dir" => collect_expected_tree_files( + store, + &forge_content_native::ObjectId::parse(object)?, + &rel, + out, + )?, + _ => bail!("malformed native tree entry kind"), + } + } + Ok(()) +} + +/// Read-only recursive walk of the workspace dir: collects the forward-slash relative +/// path of every regular file and symlink, skipping `is_ignored_by_policy` paths +/// (`.forge/`, `.git/`, the workspace marker, restore temps, secret-risk names) and +/// never descending through symlinked directories. A path that vanishes mid-walk is +/// skipped as benign (mirrors the native scanner). +fn collect_workspace_paths( + root: &Path, + dir: &Path, + out: &mut std::collections::BTreeSet, +) -> Result<()> { + let entries = + fs::read_dir(dir).map_err(|error| anyhow!("read workspace dir: {}", error.kind()))?; + for entry in entries { + let entry = entry.map_err(|error| anyhow!("read workspace dir: {}", error.kind()))?; + let full = entry.path(); + let Some(rel) = workspace_rel(root, &full) else { + continue; + }; + if forge_content::is_ignored_by_policy(&rel) { + continue; + } + let metadata = match fs::symlink_metadata(&full) { + Ok(metadata) => metadata, + Err(_) => continue, + }; + if metadata.is_dir() { + collect_workspace_paths(root, &full, out)?; + } else { + out.insert(rel); + } + } + Ok(()) +} + +/// Workspace-relative, forward-slash path (only `Normal` components; lossy UTF-8) — +/// matching the native scanner's path normalization so the two sides key identically. +fn workspace_rel(root: &Path, full: &Path) -> Option { + let rel = full.strip_prefix(root).ok()?; + let parts: Vec = rel + .components() + .filter_map(|component| match component { + Component::Normal(os) => Some(os.to_string_lossy().into_owned()), + _ => None, + }) + .collect(); + if parts.is_empty() { + return None; + } + Some(parts.join("/")) +} + +/// Per-file equality against one recorded tree entry. A symlink entry compares the +/// link target bytes to the recorded blob payload (never following the link); a +/// regular file hashes its bytes into a blob `ObjectId` and compares ids — no file +/// content ever leaves this function, and the workspace is only read. +fn workspace_file_matches_blob( + store: &forge_content_native::NativeObjectStore, + full: &Path, + mode: u32, + object: &str, +) -> Result { + let metadata = match fs::symlink_metadata(full) { + Ok(metadata) => metadata, + Err(_) => return Ok(false), // vanished since the walk: content differs + }; + if mode == WORKSPACE_SYMLINK_MODE { + if !metadata.file_type().is_symlink() { + return Ok(false); + } + let target = fs::read_link(full) + .map_err(|error| anyhow!("read workspace symlink: {}", error.kind()))?; + let expected = store.read_object(&forge_content_native::ObjectId::parse(object)?)?; + return Ok(target.to_string_lossy().as_bytes() == expected.as_slice()); + } + if !metadata.is_file() { + return Ok(false); // dir or symlink where a regular file was recorded + } + let bytes = fs::read(full).map_err(|error| anyhow!("read workspace file: {}", error.kind()))?; + let actual = + forge_content_native::ObjectId::new(forge_content_native::ObjectKind::Blob, &bytes); + Ok(actual.to_string() == object) +} diff --git a/crates/forge-store/src/error.rs b/crates/forge-store/src/error.rs index 21f1b5a..ad900df 100644 --- a/crates/forge-store/src/error.rs +++ b/crates/forge-store/src/error.rs @@ -300,6 +300,16 @@ pub enum ForgeError { principal_id: String, reason: String, }, + /// `attempt attach` found the target attempt's workspace dir no longer equal to + /// its recorded `attempt_workspaces.materialized_content_ref` (NER-382): + /// re-materializing would silently discard those workspace edits. `paths` lists + /// the drifted workspace-relative paths, secret-risk redacted in + /// [`ForgeError::details`] exactly like `DirtyWorktree`. Deterministic — save the + /// drifted content elsewhere, or re-run `attempt attach` with + /// `--discard-workspace-changes` to explicitly discard it. The override discards + /// workspace-dir drift ONLY; it never bypasses `DIRTY_WORKTREE` or + /// `ATTEMPT_WORKTREE_MISMATCH`. + WorkspaceDrift { paths: Vec }, } impl ForgeError { @@ -350,6 +360,7 @@ impl ForgeError { ForgeError::PrivateDecryptAuthorityMissing { .. } => { "PRIVATE_DECRYPT_AUTHORITY_MISSING" } + ForgeError::WorkspaceDrift { .. } => "WORKSPACE_DRIFT", } } @@ -391,7 +402,9 @@ impl ForgeError { "forge accept" ] }), - ForgeError::DirtyWorktree { paths } => redact_paths(paths), + ForgeError::DirtyWorktree { paths } | ForgeError::WorkspaceDrift { paths } => { + redact_paths(paths) + } ForgeError::AmbiguousAttempt { candidate_ids } | ForgeError::AmbiguousProposal { candidate_ids } => { json!({ "candidate_ids": candidate_ids }) @@ -801,6 +814,10 @@ impl std::fmt::Display for ForgeError { f, "principal {principal_id} lacks private decrypt authority: {reason}" ), + ForgeError::WorkspaceDrift { .. } => write!( + f, + "attempt workspace dir has drifted from its recorded materialized content; save the drifted edits elsewhere, or re-run `attempt attach` with --discard-workspace-changes to discard them" + ), } } } @@ -1096,6 +1113,12 @@ pub fn error_registry() -> &'static [ErrorCodeSpec] { after_ms: None, details_keys: &["principal_id", "reason"], }, + ErrorCodeSpec { + code: "WORKSPACE_DRIFT", + retryable: false, + after_ms: None, + details_keys: &["paths", "redacted_count"], + }, ] } @@ -1650,6 +1673,38 @@ mod tests { assert_eq!(details["redacted_count"], 2); } + /// `WorkspaceDrift` shares `redact_paths` with `DirtyWorktree` (NER-382): the + /// drifted-path list is a machine-visible egress, so secret-risk names must be + /// replaced with the placeholder while the redaction count stays observable. + #[test] + fn workspace_drift_details_redact_secret_paths() { + let error = ForgeError::WorkspaceDrift { + paths: vec![ + "src/main.rs".into(), + ".env".into(), + "server/private.pem".into(), + ], + }; + assert_eq!(error.code(), "WORKSPACE_DRIFT"); + let details = error.details(); + let paths = details["paths"].as_array().expect("paths array"); + let serialized = Value::Array(paths.clone()).to_string(); + assert!(serialized.contains("src/main.rs")); + assert!( + !serialized.contains(".env"), + "secret filename must not appear in details" + ); + assert!( + !serialized.contains("private.pem"), + "secret filename must not appear in details" + ); + assert_eq!(details["redacted_count"], 2); + // The human-readable message must name the override flag and never a path. + let message = error.to_string(); + assert!(message.contains("--discard-workspace-changes")); + assert!(!message.contains("src/main.rs")); + } + #[test] fn round_trips_through_anyhow() { let error: anyhow::Error = ForgeError::NoSnapshot.into(); @@ -1803,6 +1858,7 @@ mod tests { principal_id: "actor_x".into(), reason: "missing_active_encryption_key".into(), }, + ForgeError::WorkspaceDrift { paths: vec![] }, ]; // Exhaustiveness check: if a variant is added, this match fails to compile @@ -1849,7 +1905,8 @@ mod tests { | ForgeError::OrgAlreadyEnabled { .. } | ForgeError::OrgAuthorityRequired { .. } | ForgeError::PrivateContentInvalid { .. } - | ForgeError::PrivateDecryptAuthorityMissing { .. } => {} + | ForgeError::PrivateDecryptAuthorityMissing { .. } + | ForgeError::WorkspaceDrift { .. } => {} } } diff --git a/crates/forge-store/src/lib.rs b/crates/forge-store/src/lib.rs index 81833ad..0eb9983 100644 --- a/crates/forge-store/src/lib.rs +++ b/crates/forge-store/src/lib.rs @@ -42,8 +42,9 @@ mod visibility; pub use attempts::{ attach_attempt, attempt_base_head, attempt_materialization_ref, attempt_workspace_path, ensure_attempt_workspace_marker, list_attempts, record_attempt_workspace_materialized, - resolve_attempt, show_attempt, start_attempt, start_attempt_for_intent, verify_save_target, - AttemptRecord, AttemptShowRecord, AttemptSummary, ResolvedAttempt, StartAttempt, + resolve_attempt, show_attempt, start_attempt, start_attempt_for_intent, + verify_attempt_workspace_undrifted, verify_save_target, AttemptRecord, AttemptShowRecord, + AttemptSummary, ResolvedAttempt, StartAttempt, }; pub(crate) use attempts::{ attempt_by_id, resolve_attempt_in_context, verify_worktree_binding, WorkspaceMarker, From bc2ea57c23fe9f59d4d19878788495343a568635 Mon Sep 17 00:00:00 2001 From: Skolte <52416805+freezscholte@users.noreply.github.com> Date: Mon, 6 Jul 2026 15:47:17 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix(store):=20drift-guard=20review=20fixes?= =?UTF-8?q?=20=E2=80=94=20shared=20ignore-honoring=20equality=20primitive,?= =?UTF-8?q?=20private-label=20exclusion,=20machine-surface=20polish,=20tes?= =?UTF-8?q?t=20hardening?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Multi-persona review (run 20260706-145749-963e80e5) found two validated P1s sharing one root cause: the drift walk used weaker exclusion semantics than the scanner that built the recorded tree (gitignored build artifacts became permanent unclearable WORKSPACE_DRIFT — live reproduced), and the store hand-parsed native tree JSON. Both fixed by a new read-only forge-content-native workspace_equality primitive built on walk_worktree + tree_fingerprints; the store-side walk is deleted. Private-labeled paths are excluded from both sides of the equality (the advertised --discard flag could previously delete private files). Additive: attach schema entry, override_flag/recovery_hint details keys, workspace_role on list/show, replay injection for pre-upgrade rows. Seven new integration tests incl. gitignore-parity and override-cannot-bypass-DIRTY_WORKTREE. Design decisions deferred to NER-383. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01GTeDdEu9Q4a96DdXK4tfju --- crates/forge-cli/src/commands/core.rs | 12 + crates/forge-cli/src/schema.rs | 2 +- crates/forge-cli/tests/forge_attempts.rs | 324 ++++++++++++++++++ crates/forge-content-native/src/lib.rs | 12 +- .../src/workspace_equality.rs | 229 +++++++++++++ crates/forge-store/src/attempts.rs | 219 ++---------- crates/forge-store/src/error.rs | 25 +- crates/forge-store/src/lib.rs | 2 +- 8 files changed, 626 insertions(+), 199 deletions(-) create mode 100644 crates/forge-content-native/src/workspace_equality.rs diff --git a/crates/forge-cli/src/commands/core.rs b/crates/forge-cli/src/commands/core.rs index 9dab360..a6dbe21 100644 --- a/crates/forge-cli/src/commands/core.rs +++ b/crates/forge-cli/src/commands/core.rs @@ -1557,6 +1557,18 @@ pub(crate) fn replay_response( for (key, value) in replay_data { object.insert(key.clone(), value.clone()); } + // NER-382 replay gap: `attempt_started` rows recorded before the + // workspace_role upgrade carry workspace_path in replay_data but no + // workspace_role. Inject the constant so a replayed payload matches + // the schema promise (workspace_path is always role-qualified). + if replay_data.contains_key("workspace_path") + && !replay_data.contains_key("workspace_role") + { + object.insert( + "workspace_role".to_string(), + json!(forge_store::WORKSPACE_ROLE_MATERIALIZATION_TARGET), + ); + } object.insert( "operation_id".to_string(), json!(existing.operation_id.clone()), diff --git a/crates/forge-cli/src/schema.rs b/crates/forge-cli/src/schema.rs index c5f6126..1ad8e03 100644 --- a/crates/forge-cli/src/schema.rs +++ b/crates/forge-cli/src/schema.rs @@ -59,7 +59,7 @@ fn command_shapes() -> Value { ("attempt start", "Starts a new attempt for an existing intent; data carries the attempt + operation_id, with workspace_path qualified by workspace_role=\"materialization_target\" (the workspace dir is materialized state, not an editing surface; edits belong in the repo root worktree after `attempt attach`)."), ("attempt list", "Lists attempts; data carries { attempts: [...] }."), ("attempt show", "Shows one attempt; data carries the attempt detail."), - ("attempt attach", "Attaches the active view to an attempt; data carries attempt_id, content_ref, current_view_id."), + ("attempt attach", "Attaches the active view to an attempt; refuses with WORKSPACE_DRIFT when the attempt's workspace dir no longer equals its recorded materialized content (pass --discard-workspace-changes to explicitly discard workspace-dir drift; the flag never bypasses DIRTY_WORKTREE); data carries attempt_id, content_ref, current_view_id."), ("intent list", "Lists intents; data carries { intents: [...] } with id, title, derived status (accepted if any linked attempt was accepted, else open), the declared gate spec ({ program, args, structured } per gate), and linked attempt ids."), ("intent show", "Shows one intent; data carries intent_id, title/text, derived status, the declared gate spec ({ program, args, structured } per gate), and linked attempt ids. Unknown id -> UNKNOWN_INTENT."), ("save", "Snapshots the worktree; data carries the saved snapshot + operation_id. Native repos with local private path labels exclude those exact paths from the public forge-tree and record encrypted private overlay payload metadata."), diff --git a/crates/forge-cli/tests/forge_attempts.rs b/crates/forge-cli/tests/forge_attempts.rs index 8bd7b03..3973d74 100644 --- a/crates/forge-cli/tests/forge_attempts.rs +++ b/crates/forge-cli/tests/forge_attempts.rs @@ -87,6 +87,11 @@ fn attempt_start_lists_and_shows_competing_attempts() { .success(), ); assert_eq!(listed["data"]["attempts"].as_array().unwrap().len(), 2); + // NER-382: list/show carry the workspace_role qualifier next to workspace_path. + assert_eq!( + listed["data"]["attempts"][0]["workspace_role"], + "materialization_target" + ); let shown = json_output( repo.forge() @@ -100,6 +105,10 @@ fn attempt_start_lists_and_shows_competing_attempts() { .success(), ); assert_eq!(shown["data"]["attempt"]["intent_id"], intent_id); + assert_eq!( + shown["data"]["attempt"]["workspace_role"], + "materialization_target" + ); assert!(shown["data"]["proposals"].as_array().unwrap().is_empty()); } @@ -372,6 +381,68 @@ fn attempt_start_payload_and_replay_carry_workspace_role() { assert_eq!(replay["data"]["workspace_role"], "materialization_target"); } +/// NER-382 replay gap: an `attempt_started` op recorded BEFORE the workspace_role +/// upgrade carries `workspace_path` in its stored `replay_data` but no +/// `workspace_role`. The replay rebuild must inject the constant so the replayed +/// payload still matches the schema promise. Simulated by stripping the key from +/// the stored view state, exactly the shape a pre-upgrade row has. +#[test] +fn attempt_start_replay_injects_workspace_role_for_pre_upgrade_rows() { + let repo = TestRepo::new_git(); + repo.forge().args(["--json", "init"]).assert().success(); + let started = json_output( + repo.forge() + .args(["--json", "start", "pre-upgrade replay"]) + .assert() + .success(), + ); + let intent_id = started["data"]["intent_id"].as_str().unwrap(); + let second = json_output( + repo.forge() + .args([ + "--json", + "--request-id", + "pre-upgrade-once", + "attempt", + "start", + "--intent", + intent_id, + ]) + .assert() + .success(), + ); + + let connection = Connection::open(repo.path().join(".forge/forge.db")).expect("open db"); + let stripped = connection + .execute( + "UPDATE views + SET state_json = json_remove(state_json, '$.replay_data.workspace_role') + WHERE json_extract(state_json, '$.replay_data.workspace_role') IS NOT NULL", + [], + ) + .expect("strip workspace_role from stored replay_data"); + assert!(stripped > 0, "fixture must have stripped at least one row"); + + let replay = json_output( + repo.forge() + .args([ + "--json", + "--request-id", + "pre-upgrade-once", + "attempt", + "start", + "--intent", + intent_id, + ]) + .assert() + .success(), + ); + assert_eq!(replay["data"]["idempotent_replay"], true); + assert_eq!(replay["data"]["attempt_id"], second["data"]["attempt_id"]); + assert!(replay["data"]["workspace_path"].is_string()); + assert_eq!(replay["data"]["workspace_role"], "materialization_target"); +} + #[test] fn native_attempt_workspaces_are_isolated_and_bind_saves() { let repo = TestRepo::new_git(); @@ -959,11 +1030,21 @@ fn restore_rejects_cross_attempt_snapshot() { /// records a `forge-tree:` ref) but which is not attached yet. Returns the repo, the /// second attempt's id, and its workspace dir under `.forge/worktrees/`. fn native_repo_with_competing_attempt() -> (TestRepo, String, std::path::PathBuf) { + native_repo_with_competing_attempt_prepared(|_| {}) +} + +/// Same fixture with a `prepare` hook run after `init` but before `start`, so a test +/// can add repo-root files (a `.gitignore`, a symlink, …) that the genesis snapshot — +/// and therefore the materialized workspace tree — will contain. +fn native_repo_with_competing_attempt_prepared( + prepare: impl FnOnce(&TestRepo), +) -> (TestRepo, String, std::path::PathBuf) { let repo = TestRepo::new_git(); repo.forge() .args(["--json", "init", "--content-backend", "native"]) .assert() .success(); + prepare(&repo); let first = json_output( repo.forge() .args(["--json", "start", "drift guard"]) @@ -1132,6 +1213,249 @@ fn attach_skips_drift_check_when_workspace_never_materialized() { .success(); } +#[test] +fn attach_discard_workspace_changes_never_bypasses_dirty_worktree() { + // NER-382 review: --discard-workspace-changes discards workspace-DIR drift + // ONLY. With a dirty repo-root worktree, the attach must still refuse with + // DIRTY_WORKTREE — not WORKSPACE_DRIFT, and never success — even when the + // workspace dir has also drifted and the override flag is passed. + let (repo, second_attempt, workspace) = native_repo_with_competing_attempt(); + std::fs::write(workspace.join("README.md"), "drifted edit\n").expect("drift edit"); + std::fs::write(repo.path().join("README.md"), "dirtied root\n").expect("dirty root"); + + let dirty = json_output( + repo.forge() + .args([ + "--json", + "attempt", + "attach", + &second_attempt, + "--discard-workspace-changes", + ]) + .assert() + .failure(), + ); + assert_eq!(dirty["errors"][0]["code"], "DIRTY_WORKTREE"); + assert_eq!( + std::fs::read_to_string(repo.path().join("README.md")).unwrap(), + "dirtied root\n", + "the refused attach must not touch the dirty repo-root worktree" + ); + assert_eq!( + std::fs::read_to_string(workspace.join("README.md")).unwrap(), + "drifted edit\n", + "the refused attach must not discard the workspace drift either" + ); +} + +#[test] +fn attach_reports_deleted_workspace_file_as_drift() { + // NER-382: a deletion inside the workspace dir is drift too — re-materializing + // would silently resurrect the file, masking the (possibly intentional) removal. + let (repo, second_attempt, workspace) = native_repo_with_competing_attempt(); + std::fs::remove_file(workspace.join("README.md")).expect("delete materialized file"); + + let drift = json_output( + repo.forge() + .args(["--json", "attempt", "attach", &second_attempt]) + .assert() + .failure(), + ); + assert_eq!(drift["errors"][0]["code"], "WORKSPACE_DRIFT"); + let paths: Vec<&str> = drift["errors"][0]["details"]["paths"] + .as_array() + .expect("drift details carry a paths list") + .iter() + .map(|path| path.as_str().unwrap()) + .collect(); + assert!( + paths.contains(&"README.md"), + "deleted workspace file must be listed as drifted: {paths:?}" + ); + // The structured recovery keys ride along (additive forge.cli.v0 details). + assert_eq!( + drift["errors"][0]["details"]["override_flag"], + "--discard-workspace-changes" + ); +} + +#[cfg(unix)] +#[test] +fn attach_reports_symlink_drift() { + // NER-382 bytes/paths semantics: replacing a materialized regular file with a + // symlink is drift (type change), and re-pointing a materialized symlink at a + // different target is drift (the recorded blob pins the target bytes). + let (repo, second_attempt, workspace) = native_repo_with_competing_attempt_prepared(|repo| { + std::os::unix::fs::symlink("README.md", repo.path().join("link.txt")) + .expect("create repo-root symlink"); + }); + // Type change: README.md becomes a symlink (to identical bytes, even). + std::fs::remove_file(workspace.join("README.md")).expect("remove regular file"); + std::os::unix::fs::symlink("link.txt", workspace.join("README.md")) + .expect("replace with symlink"); + // Target change: the materialized symlink now points somewhere else. + std::fs::remove_file(workspace.join("link.txt")).expect("remove materialized symlink"); + std::os::unix::fs::symlink("ELSEWHERE.md", workspace.join("link.txt")) + .expect("re-point symlink"); + + let drift = json_output( + repo.forge() + .args(["--json", "attempt", "attach", &second_attempt]) + .assert() + .failure(), + ); + assert_eq!(drift["errors"][0]["code"], "WORKSPACE_DRIFT"); + let paths: Vec<&str> = drift["errors"][0]["details"]["paths"] + .as_array() + .expect("drift details carry a paths list") + .iter() + .map(|path| path.as_str().unwrap()) + .collect(); + assert!( + paths.contains(&"README.md"), + "regular file replaced by a symlink must drift: {paths:?}" + ); + assert!( + paths.contains(&"link.txt"), + "symlink with a changed target must drift: {paths:?}" + ); +} + +#[cfg(unix)] +#[test] +fn attach_ignores_chmod_only_workspace_change() { + // NER-382 documented semantics: the drift equality is bytes/paths only — an + // executable-bit-only change in the workspace dir is NOT drift (the recorded + // tree pins content identity, not permissions). + use std::os::unix::fs::PermissionsExt; + + let (repo, second_attempt, workspace) = native_repo_with_competing_attempt(); + std::fs::set_permissions( + workspace.join("README.md"), + std::fs::Permissions::from_mode(0o755), + ) + .expect("chmod workspace file"); + + repo.forge() + .args(["--json", "attempt", "attach", &second_attempt]) + .assert() + .success(); +} + +#[test] +fn attach_drift_check_honors_workspace_gitignore() { + // NER-382 fix 1 (gitignore parity): the workspace walk honors the .gitignore + // materialized INSIDE the workspace dir, exactly like the scanner that built + // the recorded tree. Without it, ignored build artifacts would be permanent + // false drift that --discard-workspace-changes can never clear (the deletion + // pass honors the same ignore semantics and never removes them). + let (repo, second_attempt, workspace) = native_repo_with_competing_attempt_prepared(|repo| { + std::fs::write(repo.path().join(".gitignore"), "target/\n") + .expect("write repo-root gitignore"); + }); + assert_eq!( + std::fs::read_to_string(workspace.join(".gitignore")).unwrap(), + "target/\n", + "the .gitignore is part of the materialized workspace tree" + ); + std::fs::create_dir_all(workspace.join("target")).expect("create ignored dir"); + std::fs::write(workspace.join("target/artifact.o"), "build output\n") + .expect("write ignored artifact"); + // A NON-ignored stray file still drifts — and the ignored artifact is not listed. + std::fs::write(workspace.join("STRAY.txt"), "stray\n").expect("write stray"); + let drift = json_output( + repo.forge() + .args(["--json", "attempt", "attach", &second_attempt]) + .assert() + .failure(), + ); + assert_eq!(drift["errors"][0]["code"], "WORKSPACE_DRIFT"); + let paths: Vec<&str> = drift["errors"][0]["details"]["paths"] + .as_array() + .expect("drift details carry a paths list") + .iter() + .map(|path| path.as_str().unwrap()) + .collect(); + assert!( + paths.contains(&"STRAY.txt"), + "non-ignored stray file must drift: {paths:?}" + ); + assert!( + !paths.iter().any(|path| path.starts_with("target/")), + "gitignored artifact must not be reported as drift: {paths:?}" + ); + + // With only the gitignored artifact present, attach succeeds (no false drift) — + // and re-materialization leaves the ignored artifact in place. + std::fs::remove_file(workspace.join("STRAY.txt")).expect("remove stray"); + repo.forge() + .args(["--json", "attempt", "attach", &second_attempt]) + .assert() + .success(); + assert!( + workspace.join("target/artifact.o").exists(), + "re-materialization must not delete gitignored workspace artifacts" + ); +} + +#[test] +fn attach_drift_check_excludes_private_labeled_paths() { + // NER-382 fix 2 (private path labels): an in-workspace save records a + // private-EXCLUDED public tree while the private file stays on disk, so the + // drift equality must drop the attempt's private-labeled paths from both + // sides — otherwise the guard reports false drift NAMING private paths and + // steers the agent toward --discard-workspace-changes, which deletes them. + let (repo, second_attempt, workspace) = native_repo_with_competing_attempt(); + repo.forge() + .args([ + "--json", + "visibility", + "path", + "set", + "--kind", + "attempt", + "--id", + &second_attempt, + "--path", + "NOTES.md", + ]) + .assert() + .success(); + std::fs::write(workspace.join("NOTES.md"), "private notes\n").expect("write private file"); + + // With a genuinely drifted file alongside, the refusal must not name the + // private path. + std::fs::write(workspace.join("STRAY.txt"), "stray\n").expect("write stray"); + let drift = json_output( + repo.forge() + .args(["--json", "attempt", "attach", &second_attempt]) + .assert() + .failure(), + ); + assert_eq!(drift["errors"][0]["code"], "WORKSPACE_DRIFT"); + let paths: Vec<&str> = drift["errors"][0]["details"]["paths"] + .as_array() + .expect("drift details carry a paths list") + .iter() + .map(|path| path.as_str().unwrap()) + .collect(); + assert!( + paths.contains(&"STRAY.txt"), + "real drift still listed: {paths:?}" + ); + assert!( + !paths.contains(&"NOTES.md"), + "private-labeled path must not be reported as drift: {paths:?}" + ); + + // With only the private-labeled file present, attach succeeds — no false drift. + std::fs::remove_file(workspace.join("STRAY.txt")).expect("remove stray"); + repo.forge() + .args(["--json", "attempt", "attach", &second_attempt]) + .assert() + .success(); +} + /// NER-382: both payload emission sites qualify `workspace_path` with /// `workspace_role: "materialization_target"` — `start` (auto-attach) and /// `attempt start`. (The idempotent `--request-id` replay site is pinned by diff --git a/crates/forge-content-native/src/lib.rs b/crates/forge-content-native/src/lib.rs index 841ead6..226a554 100644 --- a/crates/forge-content-native/src/lib.rs +++ b/crates/forge-content-native/src/lib.rs @@ -1,11 +1,9 @@ //! Native content object store, tree materialization, diff, and merge engine. //! -//! ADR-0001's 3,000-line ceiling allows justified exceptions when cohesion beats -//! size. This file is intentionally capped as one native-content engine because -//! the object framing, tree walking/materialization, diff fingerprinting, and -//! three-way merge code share private invariants. Splitting it mechanically -//! would widen those invariants before there is a behavior change to validate. -//! New native-content domains should still land in sibling modules. +//! ADR-0001's 3,000-line ceiling allows justified exceptions when cohesion beats size: +//! this file stays one native-content engine because object framing, tree walking and +//! materialization, diff fingerprinting, and three-way merge share private invariants +//! that mechanical splitting would widen. New domains land in sibling modules. use anyhow::{anyhow, bail, Context, Result}; use forge_content::{ @@ -29,6 +27,8 @@ use std::process::Command; mod pack; mod status_cache; +mod workspace_equality; +pub use workspace_equality::tree_equality_drift; const SCHEMA_VERSION: u32 = 1; const HUNK_LIMIT: usize = 4096; diff --git a/crates/forge-content-native/src/workspace_equality.rs b/crates/forge-content-native/src/workspace_equality.rs new file mode 100644 index 0000000..5ab2ff5 --- /dev/null +++ b/crates/forge-content-native/src/workspace_equality.rs @@ -0,0 +1,229 @@ +//! Read-only equality of an on-disk directory against a recorded native tree +//! (NER-382 workspace drift guard). +//! +//! This is an EQUALITY primitive, not a diff: it never writes anything (no +//! status cache — deliberately not `diff_working_vs_tree`), so a caller can +//! probe a workspace dir and refuse without leaving a trace in it. +//! +//! The actual side is enumerated with the SAME exclusion contract as the +//! native snapshot scanner (`walk_worktree`): `.gitignore` (nested, with +//! negation) and `.forgeignore` are honored **rooted at `scan_root`** — so a +//! `.gitignore` materialized inside the workspace dir applies to workspace +//! files exactly as it did when the recorded tree was built — with the +//! non-negatable `is_ignored_by_policy` backstop on top. Without this parity, +//! ignored build artifacts in the workspace would surface as permanent false +//! drift that re-materialization can never clear (they are never deleted by +//! the re-materialization pass, which honors the same ignore semantics). + +use std::collections::{BTreeMap, BTreeSet}; +use std::fs; +use std::path::Path; + +use anyhow::{anyhow, Result}; +use forge_content::is_ignored_by_policy; + +use crate::{walk_worktree, NativeObjectStore, ObjectId, ObjectKind, SYMLINK_MODE}; + +/// Compare the directory at `scan_root` against the recorded native `tree` (read +/// from the store under `repo_root`), returning the sorted list of drifted +/// relative paths — empty means equal. `excluded_paths` (relative, `/`-separated, +/// e.g. an attempt's local private path labels) are dropped from BOTH sides. +/// +/// Per-file semantics (bytes/paths only, mode excluded — the recorded tree pins +/// content identity, not permissions): +/// - a recorded symlink compares its link *target* bytes (never followed); +/// - a recorded regular file hashes its bytes to a blob [`ObjectId`] and +/// compares ids — no file content ever leaves this function; +/// - a path is drifted when it is missing, added, of the wrong type, or its +/// bytes hash to a different blob id. +/// +/// Expected side: the recorded tree walked via the store's validating read +/// primitives ([`NativeObjectStore::tree_fingerprints`], which enforces the +/// tree schema and entry-name safety), filtered by `is_ignored_by_policy` +/// exactly like materialization (those entries were never written into the +/// workspace, so they cannot have drifted). Read-only and cache-free: the +/// filesystem under `scan_root` is only ever read. +pub fn tree_equality_drift( + repo_root: &Path, + scan_root: &Path, + tree: &ObjectId, + excluded_paths: &BTreeSet, +) -> Result> { + let store = NativeObjectStore::new(repo_root); + let expected: BTreeMap = store + .tree_fingerprints(tree)? + .into_iter() + .filter(|(path, _)| !is_ignored_by_policy(path) && !excluded_paths.contains(path)) + .collect(); + let mut actual = BTreeSet::new(); + if scan_root.is_dir() { + for path in walk_worktree(scan_root)? { + if is_ignored_by_policy(&path) || excluded_paths.contains(&path) { + continue; + } + actual.insert(path); + } + } + let mut drifted = BTreeSet::new(); + for path in &actual { + if !expected.contains_key(path) { + drifted.insert(path.clone()); + } + } + for (path, (object, mode)) in &expected { + // Short-circuit: a recorded path absent from the walk is a deletion — no + // per-file compare needed. + let matches = + actual.contains(path) && file_matches_blob(&scan_root.join(path), object, *mode)?; + if !matches { + drifted.insert(path.clone()); + } + } + Ok(drifted.into_iter().collect()) +} + +/// Per-file equality against one recorded tree entry. A symlink entry (mode +/// `120000`) compares the link target bytes (never following the link); a +/// regular-file entry hashes the file bytes into a blob [`ObjectId`] and +/// compares ids. A path that vanished since the walk, or is of the wrong type, +/// simply differs (`Ok(false)`) — benign under a concurrent agent fleet. +fn file_matches_blob(full: &Path, object: &str, mode: u32) -> Result { + let metadata = match fs::symlink_metadata(full) { + Ok(metadata) => metadata, + Err(_) => return Ok(false), + }; + let bytes = if mode == SYMLINK_MODE { + if !metadata.file_type().is_symlink() { + return Ok(false); + } + fs::read_link(full) + .map_err(|error| anyhow!("read workspace symlink: {}", error.kind()))? + .to_string_lossy() + .into_owned() + .into_bytes() + } else { + if !metadata.is_file() { + return Ok(false); // dir or symlink where a regular file was recorded + } + fs::read(full).map_err(|error| anyhow!("read workspace file: {}", error.kind()))? + }; + Ok(ObjectId::new(ObjectKind::Blob, &bytes).to_string() == object) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::collections::BTreeMap; + use tempfile::TempDir; + + /// Build a native store under `repo_root/.forge` holding a tree with the given + /// `path -> bytes` files, returning the tree id. Materializes the files into a + /// scratch dir and snapshots it through the production scanner + /// (`snapshot_worktree_into_store`) so the recorded tree matches real trees. + fn store_tree(repo_root: &Path, files: &BTreeMap<&str, &[u8]>) -> ObjectId { + let scratch = TempDir::new().unwrap(); + for (path, bytes) in files { + let full = scratch.path().join(path); + fs::create_dir_all(full.parent().unwrap()).unwrap(); + fs::write(full, bytes).unwrap(); + } + let content = crate::snapshot_worktree_into_store(repo_root, scratch.path()).unwrap(); + let tree = content + .content_ref + .strip_prefix(forge_content::FORGE_TREE_PREFIX) + .unwrap() + .to_string(); + ObjectId::parse(&tree).unwrap() + } + + #[test] + fn equal_dir_reports_no_drift_and_gitignored_strays_are_not_drift() { + let repo = TempDir::new().unwrap(); + let files: BTreeMap<&str, &[u8]> = BTreeMap::from([ + ("README.md", b"hello\n" as &[u8]), + (".gitignore", b"target/\n"), + ]); + let tree = store_tree(repo.path(), &files); + + let scan = TempDir::new().unwrap(); + fs::write(scan.path().join("README.md"), b"hello\n").unwrap(); + fs::write(scan.path().join(".gitignore"), b"target/\n").unwrap(); + // A gitignored build artifact inside the scanned dir must not drift: the + // ignore semantics are rooted at scan_root, mirroring the scanner that + // built the recorded tree (NER-382 fix 1). + fs::create_dir_all(scan.path().join("target")).unwrap(); + fs::write(scan.path().join("target/artifact.o"), b"obj").unwrap(); + + let drifted = + tree_equality_drift(repo.path(), scan.path(), &tree, &BTreeSet::new()).unwrap(); + assert!(drifted.is_empty(), "unexpected drift: {drifted:?}"); + + // A NON-ignored stray file still drifts. + fs::write(scan.path().join("stray.txt"), b"stray").unwrap(); + let drifted = + tree_equality_drift(repo.path(), scan.path(), &tree, &BTreeSet::new()).unwrap(); + assert_eq!(drifted, vec!["stray.txt".to_string()]); + } + + #[test] + fn modified_deleted_and_excluded_paths() { + let repo = TempDir::new().unwrap(); + let files: BTreeMap<&str, &[u8]> = BTreeMap::from([ + ("README.md", b"hello\n" as &[u8]), + ("src/lib.rs", b"pub fn f() {}\n"), + ]); + let tree = store_tree(repo.path(), &files); + + let scan = TempDir::new().unwrap(); + fs::write(scan.path().join("README.md"), b"changed\n").unwrap(); + // src/lib.rs deleted; NOTES.md added but excluded (e.g. a private path label). + fs::write(scan.path().join("NOTES.md"), b"private\n").unwrap(); + + let excluded = BTreeSet::from(["NOTES.md".to_string()]); + let drifted = tree_equality_drift(repo.path(), scan.path(), &tree, &excluded).unwrap(); + assert_eq!( + drifted, + vec!["README.md".to_string(), "src/lib.rs".to_string()], + "modified + deleted drift, sorted; the excluded added path does not" + ); + + // Excluding a recorded path drops it from the expected side too. + let excluded = BTreeSet::from(["NOTES.md".to_string(), "src/lib.rs".to_string()]); + let drifted = tree_equality_drift(repo.path(), scan.path(), &tree, &excluded).unwrap(); + assert_eq!(drifted, vec!["README.md".to_string()]); + } + + #[cfg(unix)] + #[test] + fn chmod_only_change_is_not_drift_but_type_change_is() { + use std::os::unix::fs::PermissionsExt; + + let repo = TempDir::new().unwrap(); + let files: BTreeMap<&str, &[u8]> = + BTreeMap::from([("run.sh", b"#!/bin/sh\n" as &[u8]), ("data.txt", b"d\n")]); + let tree = store_tree(repo.path(), &files); + + let scan = TempDir::new().unwrap(); + fs::write(scan.path().join("run.sh"), b"#!/bin/sh\n").unwrap(); + fs::write(scan.path().join("data.txt"), b"d\n").unwrap(); + fs::set_permissions( + scan.path().join("run.sh"), + fs::Permissions::from_mode(0o755), + ) + .unwrap(); + let drifted = + tree_equality_drift(repo.path(), scan.path(), &tree, &BTreeSet::new()).unwrap(); + assert!( + drifted.is_empty(), + "bytes-only semantics: chmod must not drift: {drifted:?}" + ); + + // Replacing a recorded regular file with a symlink (even to identical + // bytes) IS drift: the recorded entry pins the regular-file type. + fs::remove_file(scan.path().join("data.txt")).unwrap(); + std::os::unix::fs::symlink("run.sh", scan.path().join("data.txt")).unwrap(); + let drifted = + tree_equality_drift(repo.path(), scan.path(), &tree, &BTreeSet::new()).unwrap(); + assert_eq!(drifted, vec!["data.txt".to_string()]); + } +} diff --git a/crates/forge-store/src/attempts.rs b/crates/forge-store/src/attempts.rs index 67f091f..c6832be 100644 --- a/crates/forge-store/src/attempts.rs +++ b/crates/forge-store/src/attempts.rs @@ -43,6 +43,9 @@ pub struct AttemptSummary { pub status: String, pub attached: bool, pub workspace_path: String, + /// Additive NER-382 qualifier mirroring [`StartAttempt::workspace_role`]: + /// `attempt list`/`attempt show` carry the same role next to `workspace_path`. + pub workspace_role: &'static str, } #[derive(Debug, Clone, Serialize)] @@ -476,6 +479,7 @@ pub fn list_attempts(cwd: &Path) -> Result> { base_head: row.get(3)?, status: row.get(4)?, workspace_path: row.get(5)?, + workspace_role: WORKSPACE_ROLE_MATERIALIZATION_TARGET, }) })?; rows.collect::, _>>() @@ -500,6 +504,7 @@ pub fn show_attempt(cwd: &Path, attempt_id: &str) -> Result { base_head: attempt.base_head.clone(), status: attempt.status.clone(), workspace_path: attempt_workspace_rel_path(&context, &attempt.attempt_id)?, + workspace_role: WORKSPACE_ROLE_MATERIALIZATION_TARGET, }, latest_snapshot: latest_snapshot_for_attempt(&context, &attempt.attempt_id)?, latest_evidence: latest_evidence_for_attempt(&context, &attempt.attempt_id)?, @@ -573,11 +578,17 @@ pub fn attempt_base_head(cwd: &Path, attempt_id: &str) -> Result { /// NER-382 workspace drift guard: refuse `attempt attach` when the target attempt's /// workspace dir no longer holds the content the store recorded materializing into it /// (`attempt_workspaces.materialized_content_ref`) — re-materializing would silently -/// discard those workspace edits. This is an EQUALITY check, not a diff: the recorded -/// tree is walked via the native store's read primitives and each workspace file is -/// hash/byte-compared against it. Deliberately NOT `diff_working_vs_tree`, which -/// writes a status cache into the scanned root — this check reads the workspace dir -/// only and never mutates it, so a refusal leaves the workspace untouched. +/// discard those workspace edits. This is an EQUALITY check, not a diff: it delegates +/// to [`forge_content_native::tree_equality_drift`], the read-only, cache-free +/// primitive that enumerates the workspace dir under the SAME exclusion contract as +/// the native snapshot scanner (policy filter + `.forgeignore` + `.gitignore` rooted +/// at the workspace dir) and hash/byte-compares each file against the recorded tree. +/// A refusal leaves the workspace untouched. +/// +/// The attempt's local private path labels are excluded from BOTH sides: an +/// in-workspace `save` records a private-EXCLUDED public tree while the private +/// files stay on disk, so counting them would be permanent false drift naming +/// private paths (and `--discard-workspace-changes` would then delete them). /// /// Skipped (Ok) when nothing was ever materialized: a NULL/absent /// `materialized_content_ref` (including every git-backend workspace, which never @@ -605,194 +616,26 @@ pub fn verify_attempt_workspace_undrifted(cwd: &Path, attempt_id: &str) -> Resul let Some(tree_id) = recorded.strip_prefix(forge_content::FORGE_TREE_PREFIX) else { return Ok(()); }; + // Same labels helper family `save` uses (`local_private_path_exclusions`), minus + // its file-existence check: this guard only needs the label *paths*, and the + // labeled files live in the workspace dir, not necessarily under `cwd`. + let excluded: std::collections::BTreeSet = + local_private_path_labels(cwd, "attempt", attempt_id)? + .into_iter() + .map(|label| label.path) + .collect(); let workspace = context .root_path .join(attempt_workspace_rel_path(&context, attempt_id)?); - let drifted = workspace_drift_paths(&context.root_path, &workspace, tree_id)?; + let tree = forge_content_native::ObjectId::parse(tree_id)?; + let drifted = forge_content_native::tree_equality_drift( + &context.root_path, + &workspace, + &tree, + &excluded, + )?; if drifted.is_empty() { return Ok(()); } Err(ForgeError::WorkspaceDrift { paths: drifted }.into()) } - -/// The tree-entry mode for a symlink (git's `120000`, mirroring the native backend's -/// symlink representation): the blob holds the link *target* bytes, so equality -/// compares the target string — the link is never followed. -const WORKSPACE_SYMLINK_MODE: u32 = 0o120000; - -/// Plain per-file equality comparison of a workspace dir against a recorded native -/// tree. Expected side: the recorded tree walked via `NativeObjectStore::read_object`, -/// skipping `is_ignored_by_policy` entries exactly like materialization does (those -/// were never written into the workspace, so they cannot have drifted). Actual side: -/// a read-only filesystem walk of the workspace dir under the same policy filter -/// (which also excludes the workspace marker and secret-risk names on both sides). -/// A path drifts when it is missing, added, of the wrong type, or its bytes hash to -/// a different blob id. Bytes/paths only — an executable-bit-only change is not -/// reported (the recorded tree pins content identity, not permissions). -fn workspace_drift_paths(repo_root: &Path, workspace: &Path, tree_id: &str) -> Result> { - let store = forge_content_native::NativeObjectStore::new(repo_root); - let root = forge_content_native::ObjectId::parse(tree_id)?; - let mut expected = std::collections::BTreeMap::new(); - collect_expected_tree_files(&store, &root, "", &mut expected)?; - let mut actual = std::collections::BTreeSet::new(); - if workspace.is_dir() { - collect_workspace_paths(workspace, workspace, &mut actual)?; - } - let mut drifted = std::collections::BTreeSet::new(); - for path in &actual { - if !expected.contains_key(path) { - drifted.insert(path.clone()); - } - } - for (path, (mode, object)) in &expected { - // Short-circuit: a recorded path absent from the walk is a deletion — no - // per-file compare needed. - let matches = actual.contains(path) - && workspace_file_matches_blob(&store, &workspace.join(path), *mode, object)?; - if !matches { - drifted.insert(path.clone()); - } - } - Ok(drifted.into_iter().collect()) -} - -/// Recursively collect `rel path -> (mode, blob object id)` for every file entry of a -/// recorded native tree, via the store's read primitives. The tree payload is the -/// content-addressed native tree object (`{ schema_version, entries: [{ name, kind, -/// mode, object }] }`); `read_object` has already verified its hash, so a malformed -/// shape here is store corruption, surfaced path-free. -fn collect_expected_tree_files( - store: &forge_content_native::NativeObjectStore, - tree_id: &forge_content_native::ObjectId, - prefix: &str, - out: &mut std::collections::BTreeMap, -) -> Result<()> { - let payload = store.read_object(tree_id)?; - let tree: Value = serde_json::from_slice(&payload)?; - let entries = tree - .get("entries") - .and_then(Value::as_array) - .ok_or_else(|| anyhow!("malformed native tree object"))?; - for entry in entries { - let name = entry - .get("name") - .and_then(Value::as_str) - .ok_or_else(|| anyhow!("malformed native tree entry"))?; - let kind = entry - .get("kind") - .and_then(Value::as_str) - .ok_or_else(|| anyhow!("malformed native tree entry"))?; - let mode = entry - .get("mode") - .and_then(Value::as_u64) - .ok_or_else(|| anyhow!("malformed native tree entry"))?; - let object = entry - .get("object") - .and_then(Value::as_str) - .ok_or_else(|| anyhow!("malformed native tree entry"))?; - let rel = if prefix.is_empty() { - name.to_string() - } else { - format!("{prefix}/{name}") - }; - if forge_content::is_ignored_by_policy(&rel) { - continue; - } - match kind { - "file" => { - out.insert(rel, (mode as u32, object.to_string())); - } - "dir" => collect_expected_tree_files( - store, - &forge_content_native::ObjectId::parse(object)?, - &rel, - out, - )?, - _ => bail!("malformed native tree entry kind"), - } - } - Ok(()) -} - -/// Read-only recursive walk of the workspace dir: collects the forward-slash relative -/// path of every regular file and symlink, skipping `is_ignored_by_policy` paths -/// (`.forge/`, `.git/`, the workspace marker, restore temps, secret-risk names) and -/// never descending through symlinked directories. A path that vanishes mid-walk is -/// skipped as benign (mirrors the native scanner). -fn collect_workspace_paths( - root: &Path, - dir: &Path, - out: &mut std::collections::BTreeSet, -) -> Result<()> { - let entries = - fs::read_dir(dir).map_err(|error| anyhow!("read workspace dir: {}", error.kind()))?; - for entry in entries { - let entry = entry.map_err(|error| anyhow!("read workspace dir: {}", error.kind()))?; - let full = entry.path(); - let Some(rel) = workspace_rel(root, &full) else { - continue; - }; - if forge_content::is_ignored_by_policy(&rel) { - continue; - } - let metadata = match fs::symlink_metadata(&full) { - Ok(metadata) => metadata, - Err(_) => continue, - }; - if metadata.is_dir() { - collect_workspace_paths(root, &full, out)?; - } else { - out.insert(rel); - } - } - Ok(()) -} - -/// Workspace-relative, forward-slash path (only `Normal` components; lossy UTF-8) — -/// matching the native scanner's path normalization so the two sides key identically. -fn workspace_rel(root: &Path, full: &Path) -> Option { - let rel = full.strip_prefix(root).ok()?; - let parts: Vec = rel - .components() - .filter_map(|component| match component { - Component::Normal(os) => Some(os.to_string_lossy().into_owned()), - _ => None, - }) - .collect(); - if parts.is_empty() { - return None; - } - Some(parts.join("/")) -} - -/// Per-file equality against one recorded tree entry. A symlink entry compares the -/// link target bytes to the recorded blob payload (never following the link); a -/// regular file hashes its bytes into a blob `ObjectId` and compares ids — no file -/// content ever leaves this function, and the workspace is only read. -fn workspace_file_matches_blob( - store: &forge_content_native::NativeObjectStore, - full: &Path, - mode: u32, - object: &str, -) -> Result { - let metadata = match fs::symlink_metadata(full) { - Ok(metadata) => metadata, - Err(_) => return Ok(false), // vanished since the walk: content differs - }; - if mode == WORKSPACE_SYMLINK_MODE { - if !metadata.file_type().is_symlink() { - return Ok(false); - } - let target = fs::read_link(full) - .map_err(|error| anyhow!("read workspace symlink: {}", error.kind()))?; - let expected = store.read_object(&forge_content_native::ObjectId::parse(object)?)?; - return Ok(target.to_string_lossy().as_bytes() == expected.as_slice()); - } - if !metadata.is_file() { - return Ok(false); // dir or symlink where a regular file was recorded - } - let bytes = fs::read(full).map_err(|error| anyhow!("read workspace file: {}", error.kind()))?; - let actual = - forge_content_native::ObjectId::new(forge_content_native::ObjectKind::Blob, &bytes); - Ok(actual.to_string() == object) -} diff --git a/crates/forge-store/src/error.rs b/crates/forge-store/src/error.rs index ad900df..b1c8c05 100644 --- a/crates/forge-store/src/error.rs +++ b/crates/forge-store/src/error.rs @@ -402,8 +402,17 @@ impl ForgeError { "forge accept" ] }), - ForgeError::DirtyWorktree { paths } | ForgeError::WorkspaceDrift { paths } => { - redact_paths(paths) + ForgeError::DirtyWorktree { paths } => redact_paths(paths), + ForgeError::WorkspaceDrift { paths } => { + // Same secret-redacted paths payload as DirtyWorktree, plus additive + // recovery keys (NER-382 review): name the explicit override flag so + // an agent can act on the refusal without parsing the human message. + let mut details = redact_paths(paths); + details["override_flag"] = json!("--discard-workspace-changes"); + details["recovery_hint"] = json!( + "Save the drifted workspace edits elsewhere, then re-run `attempt attach` with --discard-workspace-changes to explicitly discard them." + ); + details } ForgeError::AmbiguousAttempt { candidate_ids } | ForgeError::AmbiguousProposal { candidate_ids } => { @@ -1117,7 +1126,7 @@ pub fn error_registry() -> &'static [ErrorCodeSpec] { code: "WORKSPACE_DRIFT", retryable: false, after_ms: None, - details_keys: &["paths", "redacted_count"], + details_keys: &["paths", "redacted_count", "override_flag", "recovery_hint"], }, ] } @@ -1699,6 +1708,16 @@ mod tests { "secret filename must not appear in details" ); assert_eq!(details["redacted_count"], 2); + // Additive recovery keys (NER-382 review): the override flag is structured, + // not only prose, and matches the registry's details_keys. + assert_eq!(details["override_flag"], "--discard-workspace-changes"); + assert!( + details["recovery_hint"] + .as_str() + .expect("recovery_hint string") + .contains("--discard-workspace-changes"), + "recovery_hint must name the override flag" + ); // The human-readable message must name the override flag and never a path. let message = error.to_string(); assert!(message.contains("--discard-workspace-changes")); diff --git a/crates/forge-store/src/lib.rs b/crates/forge-store/src/lib.rs index 0eb9983..7e0703e 100644 --- a/crates/forge-store/src/lib.rs +++ b/crates/forge-store/src/lib.rs @@ -44,7 +44,7 @@ pub use attempts::{ ensure_attempt_workspace_marker, list_attempts, record_attempt_workspace_materialized, resolve_attempt, show_attempt, start_attempt, start_attempt_for_intent, verify_attempt_workspace_undrifted, verify_save_target, AttemptRecord, AttemptShowRecord, - AttemptSummary, ResolvedAttempt, StartAttempt, + AttemptSummary, ResolvedAttempt, StartAttempt, WORKSPACE_ROLE_MATERIALIZATION_TARGET, }; pub(crate) use attempts::{ attempt_by_id, resolve_attempt_in_context, verify_worktree_binding, WorkspaceMarker,