Skip to content

Commit 41cbd7d

Browse files
authored
feat(tools): transactional ShellExecutor with snapshot+rollback (#2414) (#2473)
Add opt-in snapshot+rollback to ShellExecutor for atomic file operations. Before executing write commands, capture filesystem snapshots of affected paths; restore on failure if auto_rollback is enabled. New config fields on ShellConfig: - transactional: bool (default false) - transaction_scope: Vec<String> glob patterns (empty = all paths) - auto_rollback: bool (default false, triggers on exit code >= 2) - auto_rollback_exit_codes: Vec<i32> explicit exit code list - snapshot_required: bool (default false, abort execution on snapshot failure) New module: zeph-tools/src/shell/transaction.rs - TransactionSnapshot using tempfile::TempDir for automatic cleanup - extract_redirection_targets() parses >, >>, 2>, 2>>, &>, &>> operators - affected_paths() combines path extraction + glob scope filtering - capture() detects and skips symlinks to prevent traversal - rollback() restores all files, collecting errors without early return Integration: - execute_block() snapshots before execution, rollbacks on condition - AuditResult::Rollback and ToolEvent::Rollback variants for audit/TUI - TUI bridge handles Rollback as AgentEvent::Status - --init wizard prompts for transactional/auto_rollback settings - --migrate-config Step 6 acknowledges new optional shell fields 903 tests pass (18 new).
1 parent d196337 commit 41cbd7d

File tree

14 files changed

+1027
-5
lines changed

14 files changed

+1027
-5
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
88

99
### Added
1010

11+
- feat(tools): transactional `ShellExecutor` — opt-in snapshot+rollback for shell commands; file-level snapshot is captured before write commands (detected via `WRITE_INDICATORS` heuristic + redirection target extraction); rollback restores originals on configurable exit codes; new `ShellConfig` fields: `transactional`, `transaction_scope` (glob-filtered paths), `auto_rollback`, `auto_rollback_exit_codes`, `snapshot_required`; new `ToolError::SnapshotFailed`, `AuditResult::Rollback`, `ToolEvent::Rollback` variants; backed by `tempfile::TempDir` for automatic cleanup on success (closes #2414)
1112
- feat(core): `/new` slash command — resets conversation context (messages, compaction state, tool caches, focus/sidequest, pending plans) while preserving memory, MCP connections, providers, and skills; creates a new `ConversationId` in SQLite for audit trail; generates a session digest for the outgoing conversation fire-and-forget unless `--no-digest` is passed; active sub-agents and background compression tasks are cancelled; `--keep-plan` preserves a pending plan graph; available in all channels (CLI, TUI, Telegram) via the unified `handle_builtin_command` path (closes #2451)
1213
- feat(memory): Kumiho AGM-inspired belief revision for graph edges — new `BeliefRevisionConfig` with `similarity_threshold`; `find_superseded_edges()` uses contradiction heuristic (same relation domain + high cosine similarity = supersession); `superseded_by` column added to `graph_edges` for audit trail; `invalidate_edge_with_supersession()` in `GraphStore`; `resolve_edge_typed` accepts optional `BeliefRevisionConfig`; controlled by `[memory.graph.belief_revision] enabled = false` (migration 056, closes #2441)
1314
- feat(memory): D-MEM RPE-based tiered graph extraction routing — `RpeRouter` computes heuristic surprise score from context similarity and entity novelty; low-RPE turns skip the MAGMA LLM extraction pipeline; `consecutive_skips` safety valve forces extraction after `max_skip_turns` consecutive skips; `extract_candidate_entities()` helper for cheap regex+keyword entity detection; controlled by `[memory.graph.rpe] enabled = false, threshold = 0.3, max_skip_turns = 5` (closes #2442)

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ serde = "1.0"
7878
serde_json = "1.0"
7979
serde_norway = "0.9.42"
8080
serial_test = "3.4"
81+
globset = "0.4"
8182
similar = "2.7"
8283
sqlx = { version = "0.8", default-features = false }
8384
subtle = "2.6"

crates/zeph-config/src/migrate.rs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,6 +1422,62 @@ pub fn migrate_database_url(toml_src: &str) -> Result<MigrationResult, MigrateEr
14221422
})
14231423
}
14241424

1425+
/// No-op migration for `[tools.shell]` transactional fields added in #2414.
1426+
///
1427+
/// All 5 new fields have `#[serde(default)]` so existing configs parse without changes.
1428+
/// This step adds them as commented-out hints in `[tools.shell]` if not already present.
1429+
///
1430+
/// # Errors
1431+
///
1432+
/// Returns `MigrateError` if the TOML cannot be parsed or `[tools.shell]` is malformed.
1433+
pub fn migrate_shell_transactional(toml_src: &str) -> Result<MigrationResult, MigrateError> {
1434+
let mut doc = toml_src.parse::<toml_edit::DocumentMut>()?;
1435+
1436+
let tools_shell_exists = doc
1437+
.get("tools")
1438+
.and_then(toml_edit::Item::as_table)
1439+
.is_some_and(|t| t.contains_key("shell"));
1440+
if !tools_shell_exists {
1441+
// No [tools.shell] section — nothing to annotate; new configs will get defaults.
1442+
return Ok(MigrationResult {
1443+
output: toml_src.to_owned(),
1444+
added_count: 0,
1445+
sections_added: Vec::new(),
1446+
});
1447+
}
1448+
1449+
let shell = doc
1450+
.get_mut("tools")
1451+
.and_then(toml_edit::Item::as_table_mut)
1452+
.and_then(|t| t.get_mut("shell"))
1453+
.and_then(toml_edit::Item::as_table_mut)
1454+
.ok_or(MigrateError::InvalidStructure(
1455+
"[tools.shell] is not a table",
1456+
))?;
1457+
1458+
if shell.contains_key("transactional") {
1459+
return Ok(MigrationResult {
1460+
output: toml_src.to_owned(),
1461+
added_count: 0,
1462+
sections_added: Vec::new(),
1463+
});
1464+
}
1465+
1466+
let comment = "# Transactional shell: snapshot files before write commands, rollback on failure.\n\
1467+
# transactional = false\n\
1468+
# transaction_scope = [] # glob patterns; empty = all extracted paths\n\
1469+
# auto_rollback = false # rollback when exit code >= 2\n\
1470+
# auto_rollback_exit_codes = [] # explicit exit codes; overrides >= 2 heuristic\n\
1471+
# snapshot_required = false # abort if snapshot fails (default: warn and proceed)\n";
1472+
append_comment_to_table_suffix(shell, comment);
1473+
1474+
Ok(MigrationResult {
1475+
output: doc.to_string(),
1476+
added_count: 1,
1477+
sections_added: vec!["tools.shell.transactional".to_owned()],
1478+
})
1479+
}
1480+
14251481
// Helper to create a formatted value (used in tests).
14261482
#[cfg(test)]
14271483
fn make_formatted_str(s: &str) -> Value {

crates/zeph-tools/Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@ readme = "README.md"
1616
[dependencies]
1717
dirs.workspace = true
1818
glob.workspace = true
19+
globset.workspace = true
1920
regex.workspace = true
2021
reqwest = { workspace = true, features = ["rustls"] }
2122
schemars.workspace = true
2223
scrape-core.workspace = true
2324
serde = { workspace = true, features = ["derive"] }
2425
serde_json.workspace = true
26+
tempfile.workspace = true
2527
thiserror.workspace = true
2628
toml.workspace = true
2729
tokio = { workspace = true, features = ["fs", "io-util", "macros", "process", "rt", "sync", "time"] }
@@ -45,7 +47,6 @@ zeph-common = { workspace = true, features = ["treesitter"] }
4547
[dev-dependencies]
4648
insta.workspace = true
4749
proptest.workspace = true
48-
tempfile.workspace = true
4950
tokio = { workspace = true, features = ["macros", "rt-multi-thread"] }
5051
toml.workspace = true
5152
wiremock.workspace = true

crates/zeph-tools/src/audit.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ pub enum AuditResult {
6868
Error { message: String },
6969
#[serde(rename = "timeout")]
7070
Timeout,
71+
#[serde(rename = "rollback")]
72+
Rollback { restored: usize, deleted: usize },
7173
}
7274

7375
impl AuditLogger {

crates/zeph-tools/src/config.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,7 @@ impl ToolsConfig {
480480

481481
/// Shell-specific configuration: timeout, command blocklist, and allowlist overrides.
482482
#[derive(Debug, Deserialize, Serialize)]
483+
#[allow(clippy::struct_excessive_bools)]
483484
pub struct ShellConfig {
484485
#[serde(default = "default_timeout")]
485486
pub timeout: u64,
@@ -498,6 +499,27 @@ pub struct ShellConfig {
498499
/// spawning shell commands. Default covers common credential naming conventions.
499500
#[serde(default = "ShellConfig::default_env_blocklist")]
500501
pub env_blocklist: Vec<String>,
502+
/// Enable transactional mode: snapshot files before write commands, rollback on failure.
503+
#[serde(default)]
504+
pub transactional: bool,
505+
/// Glob patterns defining which paths are eligible for snapshotting.
506+
/// Only files matching these patterns (relative to cwd) are captured.
507+
/// Empty = snapshot all files referenced in the command.
508+
#[serde(default)]
509+
pub transaction_scope: Vec<String>,
510+
/// Automatically rollback when exit code >= 2. Default: false.
511+
/// Exit code 1 is excluded because many tools (grep, diff, test) use it for
512+
/// non-error conditions.
513+
#[serde(default)]
514+
pub auto_rollback: bool,
515+
/// Exit codes that trigger auto-rollback. Default: empty (uses >= 2 heuristic).
516+
/// When non-empty, only these exact exit codes trigger rollback.
517+
#[serde(default)]
518+
pub auto_rollback_exit_codes: Vec<i32>,
519+
/// When true, snapshot failure aborts execution with an error.
520+
/// When false (default), snapshot failure emits a warning and execution proceeds.
521+
#[serde(default)]
522+
pub snapshot_required: bool,
501523
}
502524

503525
impl ShellConfig {
@@ -561,6 +583,11 @@ impl Default for ShellConfig {
561583
allow_network: true,
562584
confirm_patterns: default_confirm_patterns(),
563585
env_blocklist: Self::default_env_blocklist(),
586+
transactional: false,
587+
transaction_scope: Vec::new(),
588+
auto_rollback: false,
589+
auto_rollback_exit_codes: Vec::new(),
590+
snapshot_required: false,
564591
}
565592
}
566593
}

crates/zeph-tools/src/executor.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,12 @@ pub enum ToolEvent {
168168
filter_stats: Option<FilterStats>,
169169
diff: Option<DiffData>,
170170
},
171+
Rollback {
172+
tool_name: String,
173+
command: String,
174+
restored_count: usize,
175+
deleted_count: usize,
176+
},
171177
}
172178

173179
pub type ToolEventTx = tokio::sync::mpsc::UnboundedSender<ToolEvent>;
@@ -233,6 +239,9 @@ pub enum ToolError {
233239
category: crate::error_taxonomy::ToolErrorCategory,
234240
message: String,
235241
},
242+
243+
#[error("snapshot failed: {reason}")]
244+
SnapshotFailed { reason: String },
236245
}
237246

238247
impl ToolError {
@@ -254,6 +263,7 @@ impl ToolError {
254263
Self::Http { status, .. } => classify_http_status(*status),
255264
Self::Execution(io_err) => classify_io_error(io_err),
256265
Self::Shell { category, .. } => *category,
266+
Self::SnapshotFailed { .. } => ToolErrorCategory::PermanentFailure,
257267
}
258268
}
259269

crates/zeph-tools/src/shell/mod.rs

Lines changed: 96 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ use crate::executor::{
2020
use crate::filter::{OutputFilterRegistry, sanitize_output};
2121
use crate::permissions::{PermissionAction, PermissionPolicy};
2222

23+
mod transaction;
24+
use transaction::{TransactionSnapshot, affected_paths, build_scope_matchers, is_write_command};
25+
2326
const DEFAULT_BLOCKED: &[&str] = &[
2427
"rm -rf /", "sudo", "mkfs", "dd if=", "curl", "wget", "nc ", "ncat", "netcat", "shutdown",
2528
"reboot", "halt",
@@ -105,6 +108,11 @@ pub struct ShellExecutor {
105108
output_filter_registry: Option<OutputFilterRegistry>,
106109
cancel_token: Option<CancellationToken>,
107110
skill_env: std::sync::RwLock<Option<std::collections::HashMap<String, String>>>,
111+
transactional: bool,
112+
auto_rollback: bool,
113+
auto_rollback_exit_codes: Vec<i32>,
114+
snapshot_required: bool,
115+
transaction_scope_matchers: Vec<globset::GlobMatcher>,
108116
}
109117

110118
impl ShellExecutor {
@@ -153,6 +161,11 @@ impl ShellExecutor {
153161
output_filter_registry: None,
154162
cancel_token: None,
155163
skill_env: std::sync::RwLock::new(None),
164+
transactional: config.transactional,
165+
auto_rollback: config.auto_rollback,
166+
auto_rollback_exit_codes: config.auto_rollback_exit_codes.clone(),
167+
snapshot_required: config.snapshot_required,
168+
transaction_scope_matchers: build_scope_matchers(&config.transaction_scope),
156169
}
157170
}
158171

@@ -256,6 +269,7 @@ impl ShellExecutor {
256269
}))
257270
}
258271

272+
#[allow(clippy::too_many_lines)]
259273
async fn execute_block(
260274
&self,
261275
block: &str,
@@ -264,6 +278,39 @@ impl ShellExecutor {
264278
self.check_permissions(block, skip_confirm).await?;
265279
self.validate_sandbox(block)?;
266280

281+
// Take a transactional snapshot before executing write commands.
282+
let mut snapshot_warning: Option<String> = None;
283+
let snapshot = if self.transactional && is_write_command(block) {
284+
let paths = affected_paths(block, &self.transaction_scope_matchers);
285+
if paths.is_empty() {
286+
None
287+
} else {
288+
match TransactionSnapshot::capture(&paths) {
289+
Ok(snap) => {
290+
tracing::debug!(
291+
files = snap.file_count(),
292+
bytes = snap.total_bytes(),
293+
"transaction snapshot captured"
294+
);
295+
Some(snap)
296+
}
297+
Err(e) if self.snapshot_required => {
298+
return Err(ToolError::SnapshotFailed {
299+
reason: e.to_string(),
300+
});
301+
}
302+
Err(e) => {
303+
tracing::warn!(err = %e, "transaction snapshot failed, proceeding without rollback");
304+
snapshot_warning =
305+
Some(format!("[warn] snapshot failed: {e}; rollback unavailable"));
306+
None
307+
}
308+
}
309+
}
310+
} else {
311+
None
312+
};
313+
267314
if let Some(ref tx) = self.tool_event_tx {
268315
let _ = tx.send(ToolEvent::Started {
269316
tool_name: "bash".to_owned(),
@@ -294,6 +341,49 @@ impl ShellExecutor {
294341
#[allow(clippy::cast_possible_truncation)]
295342
let duration_ms = start.elapsed().as_millis() as u64;
296343

344+
// Perform auto-rollback if configured and the exit code qualifies.
345+
if let Some(snap) = snapshot {
346+
let should_rollback = self.auto_rollback
347+
&& if self.auto_rollback_exit_codes.is_empty() {
348+
exit_code >= 2
349+
} else {
350+
self.auto_rollback_exit_codes.contains(&exit_code)
351+
};
352+
if should_rollback {
353+
match snap.rollback() {
354+
Ok(report) => {
355+
tracing::info!(
356+
restored = report.restored_count,
357+
deleted = report.deleted_count,
358+
"transaction rollback completed"
359+
);
360+
self.log_audit(
361+
block,
362+
AuditResult::Rollback {
363+
restored: report.restored_count,
364+
deleted: report.deleted_count,
365+
},
366+
duration_ms,
367+
None,
368+
)
369+
.await;
370+
if let Some(ref tx) = self.tool_event_tx {
371+
let _ = tx.send(ToolEvent::Rollback {
372+
tool_name: "bash".to_owned(),
373+
command: block.to_owned(),
374+
restored_count: report.restored_count,
375+
deleted_count: report.deleted_count,
376+
});
377+
}
378+
}
379+
Err(e) => {
380+
tracing::error!(err = %e, "transaction rollback failed");
381+
}
382+
}
383+
}
384+
// On success (no rollback): snapshot dropped here; TempDir auto-cleans.
385+
}
386+
297387
let is_timeout = out.contains("[error] command timed out");
298388
let audit_result = if is_timeout {
299389
AuditResult::Timeout
@@ -358,7 +448,12 @@ impl ShellExecutor {
358448
per_block_stats.clone(),
359449
);
360450

361-
Ok((format!("$ {block}\n{filtered}"), per_block_stats))
451+
let output_line = if let Some(warn) = snapshot_warning {
452+
format!("{warn}\n$ {block}\n{filtered}")
453+
} else {
454+
format!("$ {block}\n{filtered}")
455+
};
456+
Ok((output_line, per_block_stats))
362457
}
363458

364459
fn emit_completed(

0 commit comments

Comments
 (0)