diff --git a/crates/core/src/agent.rs b/crates/core/src/agent.rs index 209f7bef..b39c57c0 100644 --- a/crates/core/src/agent.rs +++ b/crates/core/src/agent.rs @@ -57,6 +57,10 @@ use crate::types::{estimate_tokens, AgentEvent, ChatMessage, ToolCall}; const APPROVAL_TIMEOUT: Duration = Duration::from_secs(600); const POLICY_CHANGED_DURING_HOOK: &str = "Execution mode changed during pre-tool checks; this call was not executed. Request it again under the current mode."; +/// A batched call that auto admitted without asking about its content, still +/// waiting for a slot when the mode left auto. Not declined: requested +/// again, it takes the serial path, which raises the card. +const MODE_LEFT_AUTO_BEFORE_START: &str = "Execution mode changed before this call started; this call was not executed. Request it again under the current mode."; /// Stream tokens to the UI in ~25ms batches: keeps redraw work negligible /// with no perceptible latency. const FLUSH_INTERVAL: Duration = Duration::from_millis(25); @@ -111,6 +115,7 @@ fn batchable_call( call: &ToolCall, registry: &Registry, permissions: &TurnPermissions, + mode: ApprovalMode, data_dir: &Path, project_root: &Path, ) -> bool { @@ -130,14 +135,16 @@ fn batchable_call( PermissionDecision::Ask | PermissionDecision::Deny { .. } => return false, PermissionDecision::Allow | PermissionDecision::Default => {} } - // Same principle as Ask: a tool whose content no human has approved needs - // the serial path, which is where the prompt and its actionable event live. - // This check is load-bearing rather than defensive - batching selects for - // external non-mutating tools, which is exactly the population the content - // gate exists to catch, so an unapproved tool called twice in one message - // would otherwise run unattended. Kept last because it is the only check - // that touches disk, and built-ins return before the ledger is read. - unapproved_capability(registry, data_dir, project_root, name).is_none() + // Same principle as Ask: outside auto, a tool whose content no human has + // approved needs the serial path, which is where the prompt and its + // actionable event live. This check is load-bearing rather than + // defensive - batching selects for external non-mutating tools, which is + // exactly the population the content gate exists to catch, so an + // unapproved tool called twice in one message would otherwise run + // unattended. Auto runs it unattended anyway, so there the question is + // never asked and the ledger never read. Kept last because it is the only + // check that touches disk, and built-ins return before the ledger is read. + mode == ApprovalMode::Auto || unapproved_capability(registry, data_dir, project_root, name).is_none() } /// The reply for a tool call whose result was never recorded: the process @@ -254,8 +261,8 @@ fn insert_startup_note( text: String, ) { // The door, not each note. Every turn-start note is one bracketed clause - // by construction and several are built from author-controlled bytes (a - // ledger record's capability path, a provider name read out of + // by construction and several are built from author-controlled bytes (an + // approval record's capability path, a provider name read out of // providers.json, a hook stem and its parse reason). Flattening the // parameter here neutralizes all of them at once, and a note added later // inherits the rule instead of reintroducing the class. @@ -466,6 +473,23 @@ async fn report_schemas_over_budget( } } +/// The approval events a new session counts as already seen: every one on +/// record, so its first turn narrates only what happens after it starts. +/// Auto never asks the chain anything, so a session built there is seeded +/// by its first turn in another mode instead (`narrate_outside_approvals`). +fn seen_approval_events(core: &Core, project_root: &Path) -> Option> { + if core.approval_mode(project_root) == ApprovalMode::Auto { + return None; + } + Some( + crate::ledger::approval_events(&core.data_dir, project_root) + .unwrap_or_default() + .into_iter() + .map(|e| e.id) + .collect(), + ) +} + fn build_session_data(core: &Arc, session_id: &str, project_root: &Path) -> Result { sessions::ensure_owned(core, session_id)?; Ok(if let Some(mut messages) = sessions::load_messages(core, session_id)? { @@ -539,15 +563,9 @@ fn build_session_data(core: &Arc, session_id: &str, project_root: &Path) - deferred_resume_edits: edits, take_seq: 0, schemas_over_budget_reported: false, - ledger_synced: false, - unrecorded_external: None, reported_policy_notices, reported_approval_mode, - seen_ledger_events: crate::ledger::approval_events(&core.data_dir, project_root) - .unwrap_or_default() - .into_iter() - .map(|e| e.id) - .collect(), + seen_ledger_events: seen_approval_events(core, project_root), prompt_ratio: None, }; land_deferred_resume_edits(core, session_id, &mut data, saved); @@ -575,15 +593,9 @@ fn build_session_data(core: &Arc, session_id: &str, project_root: &Path) - deferred_resume_edits: Vec::new(), take_seq: 0, schemas_over_budget_reported: false, - ledger_synced: false, - unrecorded_external: None, reported_policy_notices: Default::default(), reported_approval_mode: None, - seen_ledger_events: crate::ledger::approval_events(&core.data_dir, project_root) - .unwrap_or_default() - .into_iter() - .map(|e| e.id) - .collect(), + seen_ledger_events: seen_approval_events(core, project_root), prompt_ratio: None, } }) @@ -699,6 +711,8 @@ async fn execute_readonly_batch( ) -> bool { let mut parsed: Vec = Vec::with_capacity(calls.len()); let mut blocked: Vec> = Vec::with_capacity(calls.len()); + // Per call: admitted by auto without the content question being asked. + let mut unasked: Vec = Vec::with_capacity(calls.len()); for call in calls { let name = call.function.name.as_str(); // batchable_call already validated the JSON; Null (impossible) would @@ -774,8 +788,9 @@ async fn execute_readonly_batch( // the serial path can prompt; if one still lands here, block rather // than run unapproved host code outside auto. The sites share one // predicate, so the batch path cannot drift away from the gate again. + let auto = ctx.core.approval_mode(ctx.project_root) == ApprovalMode::Auto; let block = block.or_else(|| { - if ctx.core.approval_mode(ctx.project_root) == ApprovalMode::Auto { return None; } + if auto { return None; } unapproved_capability(ctx.registry, &ctx.core.data_dir, ctx.project_root, name).map( |source| tools::ToolOutcome { ok: false, @@ -785,15 +800,18 @@ async fn execute_readonly_batch( }, ) }); + unasked.push(auto && block.is_none()); blocked.push(block); } + let core = ctx.core; let data_dir = &ctx.core.data_dir; let futures: Vec<_> = calls .iter() .zip(parsed.iter()) .zip(blocked.iter()) - .map(|((call, args), block)| { + .zip(unasked.iter().copied()) + .map(|(((call, args), block), unasked)| { let name = call.function.name.clone(); let args = args.clone(); let root = ctx.project_root.to_path_buf(); @@ -803,19 +821,36 @@ async fn execute_readonly_batch( let blocked_outcome = block.clone(); async move { if let Some(outcome) = blocked_outcome { - return outcome; + return (outcome, false); + } + // Calls past the parallelism cap start only as earlier ones + // end, long after admission. Auto admitted this one without + // asking about its content; if the mode has left auto since, + // unapproved content must not start on that admission, or it + // runs under ask with no card. Read live, as the serial path + // reads the mode right before each call. + if unasked + && core.approval_mode(&root) != ApprovalMode::Auto + && unapproved_capability(®istry, data_dir, &root, &name).is_some() + { + let refused = tools::ToolOutcome { + ok: false, + output: MODE_LEFT_AUTO_BEFORE_START.into(), + ..Default::default() + }; + return (refused, false); } - registry.execute(&name, &args, data_dir, &root, caps, cancel).await + (registry.execute(&name, &args, data_dir, &root, caps, cancel).await, true) } }) .collect(); let outcomes = collect_bounded(futures, ctx.parallelism).await; for (i, call) in calls.iter().enumerate() { - let outcome = &outcomes[i]; + let (outcome, ran) = &outcomes[i]; let name = call.function.name.as_str(); let args = &parsed[i]; - if blocked[i].is_none() { + if *ran { count_usage(ctx.usage, ctx.registry, ctx.project_root, name, args, outcome.ok); let failures = ctx .hooks @@ -1493,11 +1528,10 @@ pub async fn reload_session( sessions::attach(core, session_id)?; let root = project_root.to_path_buf(); let dd = core.data_dir.clone(); - let mut snapshot = + let snapshot = tokio::task::spawn_blocking(move || crate::registry::capture_extensions(&dd, &root)) .await .map_err(|e| format!("reload discovery failed: {e}"))?; - let files = std::mem::take(&mut snapshot.files); // Activation reads the memory store, so it stays off the runtime thread // like the capture above. let registry = tokio::task::spawn_blocking(move || Registry::from_snapshot(snapshot)) @@ -1508,29 +1542,21 @@ pub async fn reload_session( ensure_session_hydrated(core, session_id, project_root).await?; let counts = (registry.tools.len(), registry.skills.len()); - { + let changes = { let mut sessions_map = core.sessions.lock().await; let data = sessions_map .get_mut(session_id) .ok_or_else(|| "session state is unavailable; try /new".to_string())?; // A turn that slipped past the running check owns the transcript - // (mem::take leaves it empty); refuse rather than clobber - and - // refuse before touching the ledger, whose state must not move for - // a reload that was never applied. + // (mem::take leaves it empty); refuse rather than clobber. if data.messages.is_empty() { return Err("a turn is in flight; run /reload after it finishes".into()); } + let changes = generation_changes(&data.registry, ®istry, project_root); apply_freeze(core, session_id, data, registry, prompt, breakdown); - } - - // A forced reload observes whatever is on disk now; no turn was running, - // so any delta since the last freeze is external to the session. Settled - // only after the freeze applied, mirroring the turn-start refreeze: what - // is recorded is the snapshot this reload activated, never bytes a racing - // turn is writing. - let (reload_receipt, _) = - settle_ledger(core, session_id, project_root, files, crate::ledger::Actor::External).await; - Ok((counts.0, counts.1, reload_receipt)) + changes + }; + Ok((counts.0, counts.1, changes)) } /// The numbers one forced compaction reports. Built inside the guarded task, @@ -1915,13 +1941,6 @@ fn refreeze_receipt_text( added.approved.join(", ") )); } - if mode == ApprovalMode::Auto { - let names: Vec<&str> = added.unapproved.iter().chain(&added.modified_unapproved) - .map(|(name, _)| name.as_str()).collect(); - if !names.is_empty() { - note.push_str(&format!(" Tools available under auto without content approval: {}. No approval hashes were granted.", names.join(", "))); - } - } if mode == ApprovalMode::Readonly && (!added.unapproved.is_empty() || !added.modified_unapproved.is_empty()) { note.push_str(" Unapproved tool content is disabled in readonly mode."); } @@ -1967,24 +1986,6 @@ fn refreeze_receipt_text( listed.join(", ") )); } - if !added.removed_approved.is_empty() { - note.push_str(&format!( - " Removed approved tools: {} — the approval outlives the file (identical bytes at \ - that path would run without a card); tools never fail closed, so nothing needs \ - forgetting (openmax --forget is for hooks).", - added.removed_approved.join(", ") - )); - } - if !added.removed_approval_survives.is_empty() { - note.push_str(&format!( - " Removed tools with a surviving approval: {} — the manifest's approval is still \ - recorded in the ledger, so if the exact approved bytes are restored they may run \ - without a card (openmax --ledger says whether it still holds those bytes). The \ - code on disk was edited or deleted, so restoring the manifest alone would ask \ - again; nothing needs forgetting (openmax --forget is for hooks).", - added.removed_approval_survives.join(", ") - )); - } if let Some((added_m, updated_m, removed_m)) = &added.memory { let mut parts = Vec::new(); if !added_m.is_empty() { @@ -2021,7 +2022,8 @@ fn refreeze_receipt_text( /// Names the incoming generation adds, split by whether a human has already /// approved the exact bytes: approved names are callable now; unapproved /// names are registered and their first call prompts. Computed against the -/// incoming registry (which carries the tools) and the ledger. +/// incoming registry (which carries the tools) and, outside auto, the +/// ledger. Under auto every added name is callable, and nothing is read. struct AddedTools { approved: Vec, /// (name, project-relative manifest path for the --approve command) @@ -2037,26 +2039,12 @@ struct AddedTools { /// so this is what a refreeze caused by something else (a tool or skill /// change) folds into the rebuilt index. memory: Option<(Vec, Vec, Vec)>, - /// External tools present before and gone now whose CURRENT bytes a human - /// had approved (manifest sha approved AND the code on disk still matches). - /// The approval outlives the file (content-addressed), tools never fail - /// closed, and --forget is for hooks: say all three, or the agent hands - /// the human a chore that does nothing. - removed_approved: Vec, - /// Removed external tools whose MANIFEST sha was approved but whose bound - /// code no longer matches on disk (edited, or deleted alongside the - /// manifest). The approval RECORD survives in the ledger, so omitting it as - /// if nothing survived is wrong; yet the current disk bytes are - /// not approved, so this must NOT claim they run without a card. Whether - /// the original bytes can actually be restored depends on whether the - /// ledger's stored objects are still intact, so the clause says "may" and - /// points at openmax --ledger. A distinct clause. - removed_approval_survives: Vec, } fn classify_added_tools( old_registry: &Registry, new_registry: &Registry, + mode: ApprovalMode, data_dir: &Path, project_root: &Path, ) -> AddedTools { @@ -2088,73 +2076,16 @@ fn classify_added_tools( } _ => None, }; - let mut removed_approved: Vec = Vec::new(); - let mut removed_approval_survives: Vec = Vec::new(); - if !old_registry.tools.is_empty() { - let approvals = crate::ledger::approvals(data_dir, project_root).unwrap_or_default(); - for old_spec in &old_registry.tools { - let crate::registry::ToolKind::External(old_ext) = &old_spec.kind else { continue }; - if new_registry.get(&old_spec.name).is_some() { - continue; - } - // Removal vs present-but-inert is decided from the NEW capture's - // own observations, never a second disk probe: a probe races the - // capture it claims to describe, and it cannot see WHY a name is - // absent, only that a path exists - which is how a directory - // dropped over the manifest was called both "NOT loaded" and - // "removed" in one receipt, and how a broken namesake shadowing a - // deleted approved manifest still drew "identical bytes at that - // path would run", which the withheld name makes false. The - // receipt gets exactly ONE clause per absent name: - // - the capture read bytes at this path (edited broken, renamed, - // withheld by an override, or past the tool cap): the NOT-loaded - // clause, the New-tools clause, or the prompt's cap trailer - // already narrates that file; - // - a broken entry sits at this path (unreadable, or a directory - // over the `.toml`): the NOT-loaded clause names it; - // - a broken file elsewhere occupies this NAME (declared, or stem - // as the fallback - the withhold pass's own key): the name is - // not restorable-by-bytes, and that file's clause explains it; - // - otherwise the capture found no trace: the tool left disk, and - // only then is it a removal. - if new_registry.read_paths.contains(&old_ext.source_path) - || new_registry - .broken_tools - .iter() - .any(|(p, n)| *p == old_ext.source_path || *n == old_spec.name) - { - continue; - } - // Was the manifest ever approved? That signal is disk-independent: - // the ledger keeps the record and the content-addressed objects. - if !approvals.contains(&old_ext.source_sha256) { - continue; - } - // Does the code on disk STILL match what was approved? bound_code - // re-reads it, so a script edited or deleted with the manifest - // fails covers_code. When it matches, restoring the identical - // bytes runs without a card. When it does not, an approval still - // SURVIVES in the ledger (restorable) - reporting it as gone hides - // that - but the current bytes are not approved, so the - // two facts are said in two different clauses. - let code = crate::ledger::bound_code(&old_ext.command, &old_ext.args, project_root); - if approvals.covers_code(&code) { - removed_approved.push(old_spec.name.clone()); - } else { - removed_approval_survives.push(old_spec.name.clone()); - } - } - } - removed_approved.sort(); - removed_approval_survives.sort(); let mut out = AddedTools { approved: Vec::new(), unapproved: Vec::new(), modified_unapproved: Vec::new(), memory, - removed_approved, - removed_approval_survives, }; + if mode == ApprovalMode::Auto { + out.approved = added_tool_names(old_registry, new_registry); + return out; + } for name in added_tool_names(old_registry, new_registry) { match unapproved_capability(new_registry, data_dir, project_root, &name) { Some(source) => out.unapproved.push((name, source.path)), @@ -2189,6 +2120,77 @@ fn added_tool_names(old: &Registry, new: &Registry) -> Vec { .collect() } +/// What the incoming generation changes against the outgoing one, a line per +/// capability file: ` added`, ` modified`, or ` removed`, +/// so the action space never changes without a receipt saying where. Read +/// off the two registries, so it costs no disk access. When both came from a +/// capture, every file either read is compared by the hash of the bytes it +/// read: a SKILL.md body edit, a broken manifest, and a manifest the tool cap +/// keeps out of the prompt are all named. A resumed session's manifest +/// carries those hashes too. One written before they were kept does not, so +/// that one comparison falls back to what it does carry: a loaded tool by +/// its content hash and an indexed skill by the index line the prompt shows +/// for it. A file the new +/// capture read or failed to read is still on disk, so it is never called +/// removed: the receipt's NOT-loaded clause names one it could not read. +fn generation_changes(old: &Registry, new: &Registry, project_root: &Path) -> Vec { + use std::collections::BTreeMap; + #[derive(PartialEq)] + enum Identity<'a> { + /// The hash of the bytes a capture read. + Read(u64), + /// A loaded tool's manifest content hash. + Tool(&'a str), + /// An indexed skill's name and description. + Skill(&'a str, &'a str), + } + fn read(files: &std::collections::HashMap) -> BTreeMap<&Path, Identity<'_>> { + files.iter().map(|(path, hash)| (path.as_path(), Identity::Read(*hash))).collect() + } + fn loaded(registry: &Registry) -> BTreeMap<&Path, Identity<'_>> { + let tools = registry.tools.iter().filter_map(|spec| match &spec.kind { + crate::registry::ToolKind::External(ext) => { + Some((ext.source_path.as_path(), Identity::Tool(&ext.source_sha256))) + } + crate::registry::ToolKind::Builtin => None, + }); + let skills = registry + .skills + .iter() + .map(|skill| (skill.path.as_path(), Identity::Skill(&skill.name, &skill.description))); + tools.chain(skills).collect() + } + let (before, after) = match (&old.read_paths, &new.read_paths) { + (Some(old_read), Some(new_read)) => (read(old_read), read(new_read)), + _ => (loaded(old), loaded(new)), + }; + let mut changes: Vec<(&Path, &str)> = Vec::new(); + for (&path, identity) in &after { + match before.get(path) { + None => changes.push((path, "added")), + Some(previous) if previous != identity => changes.push((path, "modified")), + Some(_) => {} + } + } + for &path in before.keys() { + let still_on_disk = new.read_paths.as_ref().is_some_and(|read| read.contains_key(path)) + || new.broken.iter().any(|(broken, _)| broken == path); + if !after.contains_key(path) && !still_on_disk { + changes.push((path, "removed")); + } + } + changes.sort(); + changes + .into_iter() + .map(|(path, kind)| { + let shown = path.strip_prefix(project_root).unwrap_or(path); + // The path is the agent's own filename and this line leads the + // receipt, so it flattens where it is built. + crate::text::one_line(&format!("{} {kind}", shown.display())) + }) + .collect() +} + /// The self-modification loop closes here: at turn start, capture one immutable /// generation of extension bytes. If its fingerprint no longer matches the /// session's registry, activate that exact generation and rebuild the prompt in @@ -2199,7 +2201,7 @@ async fn refreeze_if_extensions_changed( session_id: &str, project_root: &Path, ) -> Option { - let mut snapshot = { + let snapshot = { let root = project_root.to_path_buf(); let dd = core.data_dir.clone(); match tokio::task::spawn_blocking(move || crate::registry::capture_extensions(&dd, &root)).await @@ -2208,45 +2210,14 @@ async fn refreeze_if_extensions_changed( Err(_) => return None, } }; - let files = std::mem::take(&mut snapshot.files); let disk_fp = snapshot.fingerprint(); - let (stale, unsynced) = { + let stale = { let sessions_map = core.sessions.lock().await; - match sessions_map.get(session_id) { - Some(d) => ( - !d.messages.is_empty() && d.registry.ext_fingerprint != disk_fp, - !d.ledger_synced, - ), - None => (false, false), - } + sessions_map + .get(session_id) + .is_some_and(|d| !d.messages.is_empty() && d.registry.ext_fingerprint != disk_fp) }; if !stale { - if unsynced { - // Nothing to activate, but the freeze read these files straight - // from disk, so the ledger has not necessarily met them: changes - // made while no session was running (a human, git, an installer) - // would stay unrecorded - and the first mid-turn sync would then - // sweep them up as this agent's work. Reconcile before any agent - // attribution is possible; on a project the ledger has never - // seen, this same sync writes the initial baseline. - let (receipt, landed) = settle_ledger( - core, - session_id, - project_root, - files, - crate::ledger::Actor::External, - ) - .await; - if !landed { - // Failed is not settled: the generation stays held with the - // attribution it was owed, the next sync lands it first, and - // silence here is how a backlog ends up recorded as someone - // else's work. - for message in receipt { - core.send_agent(session_id, AgentEvent::Error { message }); - } - } - } return None; } let Ok(registry) = tokio::task::spawn_blocking(move || Registry::from_snapshot(snapshot)).await else { @@ -2256,28 +2227,25 @@ async fn refreeze_if_extensions_changed( let counts = (registry.tools.len(), registry.skills.len()); let broken = registry.broken.clone(); let shadowed = registry.shadowed_skills.clone(); - let applied_added = { + let mode = core.approval_mode(project_root); + let applied = { let mut sessions_map = core.sessions.lock().await; match sessions_map.get_mut(session_id) { // Re-check under the lock: this turn owns `running`, so nothing // else mutates the session, but stay defensive about empty // (taken) state. Some(data) if !data.messages.is_empty() && data.registry.ext_fingerprint != disk_fp => { - let added = classify_added_tools(&data.registry, ®istry, &core.data_dir, project_root); + let added = classify_added_tools(&data.registry, ®istry, mode, &core.data_dir, project_root); + let changes = generation_changes(&data.registry, ®istry, project_root); apply_freeze(core, session_id, data, registry, prompt, breakdown); - Some(added) + Some((added, changes)) } _ => None, } }; - if let Some(added_tools) = applied_added { - // Turn start: the change happened while no turn was running, so it - // is external to this session (a human, git, an installer). - let (changes, _) = - settle_ledger(core, session_id, project_root, files, crate::ledger::Actor::External) - .await; + if let Some((added_tools, changes)) = applied { let receipt = - refreeze_receipt_text(&changes, &added_tools, &broken, &shadowed, project_root, core.approval_mode(project_root)); + refreeze_receipt_text(&changes, &added_tools, &broken, &shadowed, project_root, mode); core.send_agent(session_id, AgentEvent::Refrozen { tools: counts.0, skills: counts.1, @@ -2292,97 +2260,6 @@ async fn refreeze_if_extensions_changed( None } -/// Sync the ledger and describe the outcome for the refreeze receipt. A -/// ledger failure never blocks activation, but it is reported in the receipt -/// rather than swallowed. The flag says whether the sync actually landed: -/// receipt text alone must never count as reconciliation, or a failed sync -/// reads as a settled one and its backlog is later misattributed. -fn ledger_changes( - core: &Arc, - project_root: &Path, - files: &[(std::path::PathBuf, String, Vec)], - actor: crate::ledger::Actor, - session_id: &str, -) -> (Vec, bool) { - match crate::ledger::sync(&core.data_dir, project_root, files, actor, Some(session_id)) { - Ok(changes) => (crate::ledger::describe(&changes, project_root), true), - // The error must not displace the narration: this line fills the - // receipt's what-changed slot, and "ledger error: ..." alone read as - // if the refreeze had failed. Activation never depends on the ledger, - // so the change is live, and the next sync records from the ledger's - // head, so recording is deferred, not lost. Say all three, then the - // error. - Err(e) => ( - vec![format!( - "extension files changed but are not yet recorded in the capability \ - ledger ({e}); the change is still active, and recording is retried at \ - the next sync" - )], - false, - ), - } -} - -/// Record `files` (the generation a freeze just read) under `actor`, after -/// landing any turn-start generation an earlier failure left unrecorded. -/// -/// The one attribution the ledger must never get wrong is a human's edit -/// filed as the agent's. That needs a failed turn-start sync (External) -/// followed by a successful mid-turn sync (Session) in the same process: the -/// mid-turn delta then spans the human's edits too. So an unrecorded -/// turn-start generation is held and landed first, and while it cannot land -/// no Session record is written at all. The hold is in memory only: the next -/// process opens with a turn-start sync, which is External anyway, so the -/// worst a lost hold can do is file agent work as external, never the -/// reverse. Generations observed across one outage collapse to the newest. -/// -/// Returns the receipt lines and whether everything landed. -async fn settle_ledger( - core: &Arc, - session_id: &str, - project_root: &Path, - files: crate::state::ExtensionGeneration, - actor: crate::ledger::Actor, -) -> (Vec, bool) { - use crate::ledger::Actor; - let held = { - let mut map = core.sessions.lock().await; - map.get_mut(session_id).and_then(|d| d.unrecorded_external.take()) - }; - let mut receipt = Vec::new(); - let mut landed_all = true; - let mut hold: Option = None; - // An External generation supersedes a held one: the ledger records the - // delta from its head, so the newer snapshot carries everything the - // older would have, under the same actor. - if let (Some(held), Actor::Session) = (held, actor) { - let (lines, landed) = ledger_changes(core, project_root, &held, Actor::External, session_id); - receipt.extend(lines); - if !landed { - landed_all = false; - hold = Some(held); - } - } - if hold.is_none() { - let (lines, landed) = ledger_changes(core, project_root, &files, actor, session_id); - receipt.extend(lines); - if !landed { - landed_all = false; - // A failed Session sync needs no hold: the next sync from the - // ledger's head records the same delta under the same actor. - if actor == Actor::External { - hold = Some(files); - } - } - } - let mut map = core.sessions.lock().await; - if let Some(d) = map.get_mut(session_id) { - d.unrecorded_external = hold; - d.ledger_synced = landed_all; - } - (receipt, landed_all) -} - /// The mid-turn half of the self-modification loop. The turn-start check /// closes create-then-use across turns; this closes it within one turn: /// called between iterations after a mutating call succeeded, it captures one @@ -2402,7 +2279,7 @@ async fn refreeze_between_iterations( registry: &mut Arc, messages: &mut Vec, ) -> bool { - let mut snapshot = { + let snapshot = { let root = project_root.to_path_buf(); let dd = core.data_dir.clone(); match tokio::task::spawn_blocking(move || crate::registry::capture_extensions(&dd, &root)).await @@ -2411,7 +2288,6 @@ async fn refreeze_between_iterations( Err(_) => return false, } }; - let files = std::mem::take(&mut snapshot.files); if snapshot.fingerprint() == registry.ext_fingerprint { return false; } @@ -2422,7 +2298,9 @@ async fn refreeze_between_iterations( }; let (prompt, breakdown) = system_prompt_with_breakdown(project_root, &new_registry); let counts = (new_registry.tools.len(), new_registry.skills.len()); - let added_tools = classify_added_tools(registry, &new_registry, &core.data_dir, project_root); + let mode = core.approval_mode(project_root); + let added_tools = classify_added_tools(registry, &new_registry, mode, &core.data_dir, project_root); + let changes = generation_changes(registry, &new_registry, project_root); let broken = new_registry.broken.clone(); let shadowed = new_registry.shadowed_skills.clone(); let new_registry = Arc::new(new_registry); @@ -2440,21 +2318,13 @@ async fn refreeze_between_iterations( } } *registry = new_registry; - // Mid-turn: this session's own mutating call produced the change, so - // the current generation records as Session. A turn-start generation a - // broken ledger left unrecorded lands first, as External - this sync - // advances the head, and a head past that generation turns a human's - // pre-session edits into this session's record for good. If nothing can - // land, the receipt says so and activation still proceeds. - let (changes, _) = - settle_ledger(core, session_id, project_root, files, crate::ledger::Actor::Session).await; // The Refrozen event below reaches the UI, not the model, and the eval // showed exactly that gap: an agent authored a valid tool, three // refreezes fired, and it still ran the script by hand because nothing // in its transcript said the tool was callable. Ride the receipt on this // iteration's last tool result, where the model reads next. One line, // only when extension bytes actually changed. - let receipt = refreeze_receipt_text(&changes, &added_tools, &broken, &shadowed, project_root, core.approval_mode(project_root)); + let receipt = refreeze_receipt_text(&changes, &added_tools, &broken, &shadowed, project_root, mode); append_and_emit_note(core, session_id, messages, &receipt); core.send_agent(session_id, AgentEvent::Refrozen { tools: counts.0, @@ -2567,6 +2437,66 @@ fn rehydrate_approval_mode(messages: &[ChatMessage]) -> Option { .find_map(|line| notes.iter().find(|(_, text)| line == text).map(|(mode, _)| *mode)) } +/// Approvals recorded outside this running session (#199): a human at +/// another terminal ran `openmax --approve`, and that reaches the session +/// through no other channel - the refreeze receipt names FILE changes, not +/// approvals, so a tool that asked yesterday just silently stops asking. +/// Name any approval/retire event this session has not yet seen, skipping +/// ones it recorded itself (the model watched those on the card). +/// +/// Auto never asks: its calls run without content approval, so a grant +/// changes nothing it does, and reading the chain every turn was pure cost +/// on the common path. A session that first reads the chain here (one built +/// in auto) takes what is on record as seen and narrates nothing: those +/// grants predate anything this turn could have been told. +/// +/// An unverifiable ledger never reaches this point in a turn: the hook +/// layer fails closed on the same state first and blocks the input entirely +/// ("input blocked: the capability ledger cannot be read...", naming +/// --ledger-repair), and the approvals cache that gate reads is keyed on the +/// log's content hash, so a rewrite cannot serve it a stale pass. The Result +/// keeps the API honest for callers that must not mistake "unreadable" for +/// "no activity". +async fn narrate_outside_approvals( + core: &Arc, + session_id: &str, + project_root: &Path, + messages: &mut Vec, +) { + if core.approval_mode(project_root) == ApprovalMode::Auto { + return; + } + let Ok(events) = crate::ledger::approval_events(&core.data_dir, project_root) else { + return; + }; + let mut fresh: Vec = Vec::new(); + { + let mut map = core.sessions.lock().await; + let Some(data) = map.get_mut(session_id) else { return }; + let Some(seen) = data.seen_ledger_events.as_mut() else { + data.seen_ledger_events = Some(events.iter().map(|e| e.id).collect()); + return; + }; + for e in &events { + if !seen.insert(e.id) { + continue; + } + if e.session_id.as_deref() == Some(session_id) { + continue; // this session's own in-session grant + } + let path = e.path.strip_prefix(project_root).unwrap_or(&e.path); + fresh.push(format!( + "{} {}", + if e.granted { "approved" } else { "approval retired for" }, + path.display() + )); + } + } + if !fresh.is_empty() { + insert_startup_note(core, session_id, messages, outside_approval_note(&fresh)); + } +} + async fn refresh_approval_policy( core: &Arc, session_id: &str, @@ -2679,45 +2609,7 @@ async fn run_loop( insert_startup_note(core, session_id, guard.messages(), receipt); } - // Approvals recorded outside this running session (#199): a human at - // another terminal ran `openmax --approve`, and that reaches the session - // through no other channel - the refreeze receipt names FILE changes, - // not approvals, so a tool that asked yesterday just silently stops - // asking. Name any approval/retire event this session has not yet seen, - // skipping ones it recorded itself (the model watched those on the card). - // An unverifiable ledger never reaches this point in a turn: the hook - // layer fails closed on the same state first and blocks the input - // entirely ("input blocked: the capability ledger cannot be read...", - // naming --ledger-repair), and the approvals cache that gate reads is - // keyed on the log's content hash, so a rewrite cannot serve it a stale - // pass (the old length-keyed cache could). The Result keeps - // the API honest for callers that must not mistake "unreadable" for - // "no activity". - if let Ok(events) = crate::ledger::approval_events(&core.data_dir, project_root) { - let mut fresh: Vec = Vec::new(); - { - let mut map = core.sessions.lock().await; - if let Some(data) = map.get_mut(session_id) { - for e in &events { - if !data.seen_ledger_events.insert(e.id) { - continue; - } - if e.session_id.as_deref() == Some(session_id) { - continue; // this session's own in-session grant - } - let path = e.path.strip_prefix(project_root).unwrap_or(&e.path); - fresh.push(format!( - "{} {}", - if e.granted { "approved" } else { "approval retired for" }, - path.display() - )); - } - } - } - if !fresh.is_empty() { - insert_startup_note(core, session_id, guard.messages(), outside_approval_note(&fresh)); - } - } + narrate_outside_approvals(core, session_id, project_root, guard.messages()).await; // Discovered at turn start and re-discovered after any iteration whose // mutating call succeeded: a deny the agent just wrote must be in force @@ -2758,7 +2650,7 @@ async fn run_loop( // edit would surface turns later as an unrelated-looking resolve error. let mut providers_seen = crate::providers::providers_status(&core.data_dir).content_hash; let mut hooks_seen = crate::hooks::hooks_fingerprint(&core.data_dir, project_root); - let mut approval_seen = unapproved_tool_map(®istry, &core.data_dir, project_root); + let mut approval_seen = approval_state(core, ®istry, project_root); if let Some(note) = settings_drift_note(core) { // Same channel as the refreeze receipt: before the prompt, once per @@ -3090,15 +2982,18 @@ async fn run_loop( // Only the serial path sets this. A batched external tool is host code // that could also write a capability file, so a mid-turn refreeze can // be one iteration late after a pure batch; turn start always catches - // it. Not a gate hole - whatever such a tool writes is unapproved - // content, which asks on its own first call. + // it. Not a gate hole: outside auto, whatever such a tool writes is + // unapproved content, which asks on its own first call, and under + // auto nothing asks. let mut extensions_touched = false; + let batch_mode = core.approval_mode(project_root); let segments = partition_concurrent_runs(&tool_calls, |call| { batchable_call( call, ®istry, &permissions, + batch_mode, &core.data_dir, project_root, ) @@ -3111,7 +3006,11 @@ async fn run_loop( break 'turns; } - if segment.concurrent { + // The segments were formed under `batch_mode`. A group formed in + // auto can hold unapproved tools, which outside auto only the + // serial path can ask about, so a group whose mode has changed + // since runs serially instead of being declined as a batch. + if segment.concurrent && policy_mode == batch_mode { let mut batch_ctx = ReadonlyBatchCtx { core, session_id, @@ -3455,8 +3354,7 @@ async fn run_loop( // fingerprint-gated (a no-op when nothing on disk moved), // so this is safe to set eagerly; gating it on `outcome.ok` // instead left a written-then-failed file inactive until - // the turn ended, where the next turn start then recorded - // it as an out-of-session `Actor::External` edit. + // the next turn start. extensions_touched = true; // Reload here, not at iteration end: one assistant // response can carry the policy write and the call @@ -3582,7 +3480,7 @@ async fn run_loop( // runs it, but moves no fingerprint: no refreeze, no // receipt - and the model learned only when the next // call raised a card. Diff approval state per call. - let approval_now = unapproved_tool_map(®istry, &core.data_dir, project_root); + let approval_now = approval_state(core, ®istry, project_root); let mut revoked: Vec = Vec::new(); for (name, state) in &approval_now { if let (Some(path), Some(None)) = (state, approval_seen.get(name)) { @@ -3595,14 +3493,12 @@ async fn run_loop( approval_seen = approval_now; if !revoked.is_empty() { revoked.sort(); - let note = if core.approval_mode(project_root) == ApprovalMode::Auto { - format!("[tool code changed: {}. Auto continues without content approval; saved approvals were not extended to these bytes.]", revoked.join(", ")) - } else { format!( + let note = format!( "[approval revoked: the code these tools run changed since a human \ approved it, so the next call of each stops for a card: {}. Re-approve \ the exact bytes, or prove them first with openmax --check --run-examples.]", revoked.join(", ") - ) }; + ); append_and_emit_note(core, session_id, guard.messages(), ¬e); } // A hook file written this call: hooks are outside @@ -3690,7 +3586,7 @@ async fn run_loop( schemas_wire = registry.schemas_wire_arc(); // The refreeze receipt already narrated manifest-level approval // changes; resync so the per-call diff does not repeat them. - approval_seen = unapproved_tool_map(®istry, &core.data_dir, project_root); + approval_seen = approval_state(core, ®istry, project_root); } // The system prompt at index 0 changed on refreeze, so the transcript // prefix on disk is stale: rewrite instead of append. @@ -4015,6 +3911,22 @@ fn unapproved_tool_map( .collect() } +/// `unapproved_tool_map` outside auto, and empty under it: auto runs every +/// external tool without content approval, so there is no revocation to +/// announce, and the map costs a read of the chain per external tool on +/// every mutating call. A diff across a mode switch starts from empty and +/// so announces nothing; the next call's card (or its absence) is accurate. +fn approval_state( + core: &Core, + registry: &Registry, + project_root: &Path, +) -> std::collections::HashMap> { + if core.approval_mode(project_root) == ApprovalMode::Auto { + return std::collections::HashMap::new(); + } + unapproved_tool_map(registry, &core.data_dir, project_root) +} + /// True when the harness itself spawned this process: every child it starts /// carries the marker (execution.rs), and the marker is what `--approve`, /// `--trust-project`, `--forget` refuse on. The same wall applies to the @@ -4835,7 +4747,7 @@ mod tests { tool_call("danger", r#"{"key":"b"}"#), ]; let batchable = - || batchable_call(&calls[0], ®istry, &perms, &data, &project); + || batchable_call(&calls[0], ®istry, &perms, ApprovalMode::Ask, &data, &project); let gated = || unapproved_capability(®istry, &data, &project, "danger"); assert!(gated().is_some(), "nothing approved yet"); @@ -4863,7 +4775,7 @@ mod tests { "a swapped payload must not reach the unattended batch path by being called twice" ); let segments = partition_concurrent_runs(&calls, |c| { - batchable_call(c, ®istry, &perms, &data, &project) + batchable_call(c, ®istry, &perms, ApprovalMode::Ask, &data, &project) }); assert_eq!(segments.len(), 2, "each call gets its own serial segment"); assert!(segments.iter().all(|s| !s.concurrent)); @@ -4992,38 +4904,6 @@ mod tests { let _ = std::fs::remove_dir_all(dir); } - /// A ledger failure must not replace the receipt's what-changed slot with - /// a bare "ledger error:": activation never depends on the ledger, so the - /// change the model just made is live either way, and the next sync - /// records it. The receipt must say those facts; leading with the error - /// read as if the refreeze itself had failed. - #[test] - fn a_ledger_failure_reports_deferred_recording_not_a_bare_error() { - let dir = std::env::temp_dir().join(format!("openmax-lf-{}", uuid::Uuid::new_v4())); - let (core, _rx) = Core::new(dir.clone()).unwrap(); - let project = dir.join("project"); - std::fs::create_dir_all(&project).unwrap(); - // A FILE at the ledger root makes every project ledger dir - // un-creatable, failing the sync deterministically (works even when - // tests run as root, unlike a permissions-based denial). - std::fs::write(core.data_dir.join("ledger"), b"not a dir").unwrap(); - let files = - vec![(project.join(".openmax/tools/t.toml"), "sha".to_string(), b"x".to_vec())]; - let (lines, landed) = - ledger_changes(&core, &project, &files, crate::ledger::Actor::Session, "s1"); - assert!(!landed, "a failed sync must report not-landed"); - let joined = lines.join("; "); - assert!( - joined.contains("still active") && joined.contains("retried"), - "the receipt says the change is live and the recording deferred: {joined}" - ); - assert!( - !joined.starts_with("ledger error"), - "the error must not displace the narration: {joined}" - ); - let _ = std::fs::remove_dir_all(dir); - } - /// The note for approval activity the session did not perform must not /// presume a benign third party: the actor may equally be an agent /// process that stepped around the session marker (ADR-0006's friction @@ -5267,11 +5147,13 @@ mod tests { /// Concurrent batching selects for external non-mutating tools - exactly /// the population the content gate exists to catch - and the batch path - /// has no approval UI. So an unapproved tool must never be batchable: two - /// consecutive calls to it have to fall to the serial path that prompts, - /// or the gate would only cover calls the model happens to emit alone. + /// has no approval UI. So outside auto an unapproved tool must never be + /// batchable: two consecutive calls to it have to fall to the serial path + /// that prompts, or the gate would only cover calls the model happens to + /// emit alone. Auto runs it unattended on either path, so there it + /// batches like any read-only tool, without asking the ledger. #[test] - fn an_unapproved_external_tool_is_never_batchable() { + fn an_unapproved_external_tool_batches_only_in_auto() { let dir = std::env::temp_dir().join(format!("openmax-batch-{}", uuid::Uuid::new_v4())); let data_dir = dir.join("data"); let project = dir.join("project"); @@ -5289,24 +5171,32 @@ mod tests { ]; assert!( - !batchable_call(&calls[0], ®istry, &perms, &data_dir, &project), + !batchable_call(&calls[0], ®istry, &perms, ApprovalMode::Ask, &data_dir, &project), "unapproved host code must not be eligible for the unattended batch path" ); let segments = partition_concurrent_runs(&calls, |c| { - batchable_call(c, ®istry, &perms, &data_dir, &project) + batchable_call(c, ®istry, &perms, ApprovalMode::Ask, &data_dir, &project) }); assert_eq!(segments.len(), 2, "each call gets its own serial segment"); assert!(segments.iter().all(|s| !s.concurrent)); + let reads = crate::ledger::log_reads(&data_dir, &project); + let segments = partition_concurrent_runs(&calls, |c| { + batchable_call(c, ®istry, &perms, ApprovalMode::Auto, &data_dir, &project) + }); + assert_eq!(segments.len(), 1); + assert!(segments[0].concurrent, "auto batches unapproved read-only tools"); + assert_eq!(crate::ledger::log_reads(&data_dir, &project), reads, "auto never asks the ledger"); + // Approved content is ordinary read-only work and batches again. let sha = match ®istry.get("peek").unwrap().kind { crate::registry::ToolKind::External(ext) => ext.source_sha256.clone(), crate::registry::ToolKind::Builtin => unreachable!("peek is external"), }; crate::ledger::approve_hash(&data_dir, &project, &sha).unwrap(); - assert!(batchable_call(&calls[0], ®istry, &perms, &data_dir, &project)); + assert!(batchable_call(&calls[0], ®istry, &perms, ApprovalMode::Ask, &data_dir, &project)); let segments = partition_concurrent_runs(&calls, |c| { - batchable_call(c, ®istry, &perms, &data_dir, &project) + batchable_call(c, ®istry, &perms, ApprovalMode::Ask, &data_dir, &project) }); assert_eq!(segments.len(), 1); assert!(segments[0].concurrent, "approved read-only tools still batch"); @@ -5326,7 +5216,7 @@ mod tests { ]; let empty_perms = TurnPermissions::new(Permissions::default()); let segments = partition_concurrent_runs(&calls, |c| { - batchable_call(c, ®istry, &empty_perms, nowhere(), nowhere()) + batchable_call(c, ®istry, &empty_perms, ApprovalMode::Ask, nowhere(), nowhere()) }); assert_eq!(segments.len(), 3); assert!(segments[0].concurrent && segments[0].start == 0 && segments[0].end == 2); @@ -5345,7 +5235,7 @@ mod tests { tool_call("grep", r#"{"pattern":"fn"}"#), ]; let segments = partition_concurrent_runs(&calls, |c| { - batchable_call(c, ®istry, &empty_perms, nowhere(), nowhere()) + batchable_call(c, ®istry, &empty_perms, ApprovalMode::Ask, nowhere(), nowhere()) }); assert_eq!(segments.len(), 1); assert!(segments[0].concurrent); @@ -5354,6 +5244,7 @@ mod tests { &tool_call("write_file", r#"{"path":"x","content":"y"}"#), ®istry, &empty_perms, + ApprovalMode::Ask, nowhere(), nowhere(), )); @@ -5361,6 +5252,7 @@ mod tests { &tool_call("nope", r#"{}"#), ®istry, &empty_perms, + ApprovalMode::Ask, nowhere(), nowhere(), )); @@ -5442,7 +5334,7 @@ mod tests { let calls = vec![tool_call("read_file", r#"{"path":"a.rs"}"#)]; let empty_perms = TurnPermissions::new(Permissions::default()); let segments = partition_concurrent_runs(&calls, |c| { - batchable_call(c, ®istry, &empty_perms, nowhere(), nowhere()) + batchable_call(c, ®istry, &empty_perms, ApprovalMode::Ask, nowhere(), nowhere()) }); assert_eq!(segments.len(), 1); assert!(!segments[0].concurrent); @@ -5463,7 +5355,7 @@ mod tests { ]; let empty_perms = TurnPermissions::new(Permissions::default()); let segments = partition_concurrent_runs(&calls, |c| { - batchable_call(c, ®istry, &empty_perms, nowhere(), nowhere()) + batchable_call(c, ®istry, &empty_perms, ApprovalMode::Ask, nowhere(), nowhere()) }); assert_eq!(segments.len(), 3); assert!(!segments[0].concurrent); @@ -5908,13 +5800,8 @@ mod tests { // its (already re-prefilled) cache. assert!(!refreeze_between_iterations(&core, id, &project, &mut registry, &mut messages).await); - // The ledger recorded the exact generation the freeze used, and the - // wire event carried a receipt naming the file. - let records = crate::ledger::history(&core.data_dir, &project).unwrap(); - assert!( - records.iter().any(|r| r.path.ends_with(".openmax/tools/deploy.toml")), - "the activated tool must be in the ledger" - ); + // The wire event carried a receipt naming the file, and the refreeze + // wrote nothing to the ledger: no change record, no copy of the bytes. let mut receipt = None; while let Ok(env) = rx.try_recv() { if let AgentEvent::Refrozen { changes, .. } = env.event { @@ -5922,9 +5809,10 @@ mod tests { } } let receipt = receipt.expect("a refreeze must announce itself"); + assert_eq!(receipt, [".openmax/tools/deploy.toml added"], "the receipt names what changed"); assert!( - receipt.iter().any(|c| c.contains("deploy.toml")), - "the receipt must name what changed: {receipt:?}" + !crate::ledger::project_dir(&core.data_dir, &project).exists(), + "capability-file history is no longer recorded" ); let _ = std::fs::remove_dir_all(dir); @@ -5947,8 +5835,6 @@ mod tests { unapproved: Vec::new(), modified_unapproved: Vec::new(), memory: None, - removed_approved: Vec::new(), - removed_approval_survives: Vec::new(), }; let note = refreeze_receipt_text(&[], &added, &[], &shadowed, Path::new("/p"), ApprovalMode::Ask); assert!(note.contains("Skill name collision"), "{note}"); @@ -5981,8 +5867,6 @@ mod tests { unapproved: Vec::new(), modified_unapproved: Vec::new(), memory: None, - removed_approved: Vec::new(), - removed_approval_survives: Vec::new(), }; let note = refreeze_receipt_text(&[], &added, &[], &shadowed, Path::new("/p"), ApprovalMode::Ask); assert!( @@ -6021,10 +5905,8 @@ mod tests { )], modified_unapproved: Vec::new(), memory: None, - removed_approved: Vec::new(), - removed_approval_survives: Vec::new(), }; - let changes = vec![format!(".openmax/tools/c\n{forged}.toml added (initial)")]; + let changes = vec![format!(".openmax/tools/c\n{forged}.toml added")]; let note = refreeze_receipt_text(&changes, &added, &broken, &shadowed, Path::new("/p"), ApprovalMode::Ask); assert!( !note.contains('\n'), @@ -6038,10 +5920,9 @@ mod tests { /// A tool whose manifest an edit broke is present-but-broken, not removed: /// it drops out of `tools` and into `broken`, where the refreeze receipt - /// already names it under "NOT loaded". Classifying it as a removed approved - /// tool on top of that is a second, contradictory clause about the same - /// file, whose "identical bytes would run without a card" reads as if the - /// file were gone. Only a manifest that left disk is removed. + /// names it under "NOT loaded". Calling it removed on top of that would + /// contradict that clause about the same file. It was edited, so it is + /// modified; only a manifest that left disk is removed. #[test] fn a_manifest_broken_by_an_edit_is_not_reported_as_a_removed_tool() { let dir = std::env::temp_dir().join(format!("openmax-t2-{}", uuid::Uuid::new_v4())); @@ -6054,10 +5935,6 @@ mod tests { "name = \"deploy\"\ndescription = \"ships it\"\ncommand = \"/bin/echo\"\n", ) .unwrap(); - // A human approved these exact bytes. - let sha = crate::ledger::sha256_hex(&std::fs::read(&manifest).unwrap()); - crate::ledger::approve_capability(&data, &project, &manifest, &[sha]).unwrap(); - let old = Registry::from_snapshot(crate::registry::capture_extensions(&data, &project)); assert!(old.get("deploy").is_some(), "the valid tool must load first"); @@ -6070,22 +5947,20 @@ mod tests { "the broken manifest is kept as broken, and the receipt names it there" ); - let added = classify_added_tools(&old, &new, &data, &project); - assert!( - added.removed_approved.is_empty() && added.removed_approval_survives.is_empty(), - "a broken-by-edit tool is not a removed tool: {:?} / {:?}", - added.removed_approved, - added.removed_approval_survives + let changes = generation_changes(&old, &new, &project); + assert_eq!( + changes, + [".openmax/tools/deploy.toml modified"], + "a broken-by-edit tool is modified, not removed" ); let _ = std::fs::remove_dir_all(dir); } - /// The disk-presence check must not over-suppress: a manifest that truly - /// left disk (not present, not broken, not shadowed) is still a removed - /// approved tool, with its surviving approval named. This is the other side - /// of the broken-not-removed rule, so `exists()` cannot swallow a real - /// removal. + /// The other side of the broken-not-removed rule: a manifest that truly + /// left disk (not present, not broken) is named as removed, and a file + /// that arrived is named as added, so the receipt never leaves the action + /// space changed without saying where. #[test] fn a_deleted_manifest_is_still_reported_as_a_removed_tool() { let dir = std::env::temp_dir().join(format!("openmax-t2d-{}", uuid::Uuid::new_v4())); @@ -6098,14 +5973,14 @@ mod tests { "name = \"deploy\"\ndescription = \"ships it\"\ncommand = \"/bin/echo\"\n", ) .unwrap(); - let sha = crate::ledger::sha256_hex(&std::fs::read(&manifest).unwrap()); - crate::ledger::approve_capability(&data, &project, &manifest, &[sha]).unwrap(); - let old = Registry::from_snapshot(crate::registry::capture_extensions(&data, &project)); assert!(old.get("deploy").is_some()); - // The manifest leaves disk entirely. + // The manifest leaves disk entirely, and a skill arrives. std::fs::remove_file(&manifest).unwrap(); + let skill = project.join(".agents/skills/ship/SKILL.md"); + std::fs::create_dir_all(skill.parent().unwrap()).unwrap(); + std::fs::write(&skill, "---\nname: ship\ndescription: how to ship\n---\nsteps\n").unwrap(); let new = Registry::from_snapshot(crate::registry::capture_extensions(&data, &project)); assert!(new.get("deploy").is_none()); assert!( @@ -6113,131 +5988,128 @@ mod tests { "a deleted file is gone, not broken" ); - let added = classify_added_tools(&old, &new, &data, &project); - assert!( - added.removed_approved == vec!["deploy".to_string()], - "a genuinely removed approved tool is still named: {:?} / {:?}", - added.removed_approved, - added.removed_approval_survives + let changes = generation_changes(&old, &new, &project); + assert_eq!( + changes, + [".agents/skills/ship/SKILL.md added", ".openmax/tools/deploy.toml removed"], + "each file that changed is named, with what happened to it" ); + // A rewrite in place is a modification of the same file. + std::fs::write(&skill, "---\nname: ship\ndescription: how to ship safely\n---\nsteps\n") + .unwrap(); + let newer = Registry::from_snapshot(crate::registry::capture_extensions(&data, &project)); + assert_eq!(generation_changes(&new, &newer, &project), [".agents/skills/ship/SKILL.md modified"]); + let _ = std::fs::remove_dir_all(dir); } - /// A directory dropped at the manifest's `.toml` path lands in `broken` - /// ("unreadable: ..."), and the receipt's NOT-loaded clause names it. It - /// must not ALSO be classified as a removed approved tool: two clauses - /// calling the same path present-but-unreadable and gone contradict each - /// other, and the removal clause's "identical bytes at that path would run" - /// is unactionable while a directory occupies the path. The - /// capture's own observation - a broken entry at this path - wins. + /// A SKILL.md body is what the model reads when it loads the skill, and + /// the fingerprint hashes it, so a body-only edit refreezes. Its index + /// line (name and description) is unchanged, so a receipt that compared + /// index lines named no file and fell back to "extension files changed": + /// a skill rewritten by a git pull would arrive without saying which. The + /// capture's own read of the bytes is what names it. #[test] - fn a_directory_at_the_manifest_path_is_broken_not_doubly_removed() { - let dir = std::env::temp_dir().join(format!("openmax-t2dir-{}", uuid::Uuid::new_v4())); + fn a_skill_body_edit_is_named_as_a_modified_file() { + let dir = std::env::temp_dir().join(format!("openmax-t2body-{}", uuid::Uuid::new_v4())); let data = dir.join("data"); let project = dir.join("project"); - let manifest = project.join(".openmax/tools/deploy.toml"); - std::fs::create_dir_all(manifest.parent().unwrap()).unwrap(); - std::fs::write( - &manifest, - "name = \"deploy\"\ndescription = \"ships it\"\ncommand = \"/bin/echo\"\n", - ) - .unwrap(); - let sha = crate::ledger::sha256_hex(&std::fs::read(&manifest).unwrap()); - crate::ledger::approve_capability(&data, &project, &manifest, &[sha]).unwrap(); + let skill = project.join(".agents/skills/ship/SKILL.md"); + std::fs::create_dir_all(skill.parent().unwrap()).unwrap(); + std::fs::write(&skill, "---\nname: ship\ndescription: how to ship\n---\nrun the tests\n") + .unwrap(); let old = Registry::from_snapshot(crate::registry::capture_extensions(&data, &project)); - assert!(old.get("deploy").is_some()); - // The manifest file is replaced by a directory at the same path. - std::fs::remove_file(&manifest).unwrap(); - std::fs::create_dir(&manifest).unwrap(); - assert!(manifest.exists() && !manifest.is_file(), "a directory now sits at the path"); + std::fs::write(&skill, "---\nname: ship\ndescription: how to ship\n---\nskip the tests\n") + .unwrap(); let new = Registry::from_snapshot(crate::registry::capture_extensions(&data, &project)); - assert!(new.get("deploy").is_none()); - assert!( - new.broken.iter().any(|(p, r)| *p == manifest && r.contains("unreadable")), - "the directory is a broken entry the receipt names: {:?}", - new.broken - ); + assert_ne!(old.ext_fingerprint, new.ext_fingerprint, "the body edit refreezes"); - let added = classify_added_tools(&old, &new, &data, &project); - assert!( - added.removed_approved.is_empty() && added.removed_approval_survives.is_empty(), - "the broken clause speaks alone; no contradictory removal clause: {:?} / {:?}", - added.removed_approved, - added.removed_approval_survives - ); + assert_eq!(generation_changes(&old, &new, &project), [".agents/skills/ship/SKILL.md modified"]); let _ = std::fs::remove_dir_all(dir); } - /// The approved global manifest is deleted while a broken project file - /// declares the same tool name. Restoring the approved bytes would NOT - /// make the tool callable - the broken override withholds the name - so - /// "Removed approved tools: X (identical bytes would run without a card)" - /// is a false promise. The broken file's NOT-loaded clause is - /// the one truthful explanation, and the match is on the DECLARED name - /// (the broken file's stem differs here), the same key the withhold pass - /// judges collisions on. + /// A resumed session's registry is rebuilt from its manifest, not from a + /// capture. Without the per-file hashes its freeze read, the receipt + /// could only compare index lines, so a SKILL.md body edited while the + /// session was closed went live under "extension files changed" and was + /// never named. A manifest written before the hashes were kept still + /// loads and compares index lines. #[test] - fn a_removal_shadowed_by_a_broken_namesake_is_not_reported_removed() { - let dir = std::env::temp_dir().join(format!("openmax-t2sh-{}", uuid::Uuid::new_v4())); + fn a_skill_body_edited_while_the_session_was_closed_is_named_on_resume() { + let dir = std::env::temp_dir().join(format!("openmax-t2resume-{}", uuid::Uuid::new_v4())); let data = dir.join("data"); let project = dir.join("project"); - let global = data.join("tools/deploy.toml"); - std::fs::create_dir_all(global.parent().unwrap()).unwrap(); - // The project directory must exist BEFORE the approval: the ledger - // keys its per-project store on the canonicalized project path, and a - // path that cannot canonicalize yet would record under a different key - // than every later read resolves (macOS /var vs /private/var). - std::fs::create_dir_all(project.join(".openmax/tools")).unwrap(); - std::fs::write( - &global, - "name = \"deploy\"\ndescription = \"ships it\"\ncommand = \"/bin/echo\"\n", - ) - .unwrap(); - let sha = crate::ledger::sha256_hex(&std::fs::read(&global).unwrap()); - crate::ledger::approve_capability(&data, &project, &global, std::slice::from_ref(&sha)) + let skill = project.join(".agents/skills/ship/SKILL.md"); + std::fs::create_dir_all(skill.parent().unwrap()).unwrap(); + std::fs::write(&skill, "---\nname: ship\ndescription: how to ship\n---\nrun the tests\n") .unwrap(); - let old = Registry::from_snapshot(crate::registry::capture_extensions(&data, &project)); - assert!(old.get("deploy").is_some()); - assert!( - crate::ledger::approved_hashes(&data, &project).unwrap().contains(&sha), - "the approval must record the manifest sha, or the classify probe is vacuous" - ); + let frozen = Registry::from_snapshot(crate::registry::capture_extensions(&data, &project)); + let saved = serde_json::to_string(&frozen.to_manifest()).unwrap(); + let resumed = Registry::from_manifest(serde_json::from_str(&saved).unwrap()); - // The approved global manifest leaves disk, and a broken project file - // under a DIFFERENT stem declares the same name (parses as TOML, fails - // the spec, so the declared name is recoverable). - std::fs::remove_file(&global).unwrap(); - let shadow = project.join(".openmax/tools/shipit.toml"); - std::fs::write(&shadow, "name = \"deploy\"\ndescription = \"broken, no command\"\n") + std::fs::write(&skill, "---\nname: ship\ndescription: how to ship\n---\nskip the tests\n") .unwrap(); let new = Registry::from_snapshot(crate::registry::capture_extensions(&data, &project)); - assert!(new.get("deploy").is_none()); - assert!( - new.broken.iter().any(|(p, _)| *p == shadow), - "the broken namesake is recorded: {:?}", - new.broken + assert_ne!(resumed.ext_fingerprint, new.ext_fingerprint, "the body edit refreezes on resume"); + assert_eq!( + generation_changes(&resumed, &new, &project), + [".agents/skills/ship/SKILL.md modified"] ); - let added = classify_added_tools(&old, &new, &data, &project); - assert!( - added.removed_approved.is_empty() && added.removed_approval_survives.is_empty(), - "a name a broken file still occupies is not a removal: {:?} / {:?}", - added.removed_approved, - added.removed_approval_survives - ); + let mut legacy: Value = serde_json::from_str(&saved).unwrap(); + legacy.as_object_mut().unwrap().remove("read_files"); + let legacy = Registry::from_manifest(serde_json::from_value(legacy).unwrap()); + assert!(generation_changes(&legacy, &new, &project).is_empty(), "index lines are unchanged"); + + let _ = std::fs::remove_dir_all(dir); + } + + /// Each change entry leads the model's receipt and goes out on its own + /// in `AgentEvent::Refrozen.changes`, where the headless frontend prints + /// the entries on one stderr line. A tool or skill path carrying a + /// newline must not forge a second line in either place: every entry is + /// one line and still begins with the project-relative path. + #[cfg(unix)] + #[test] + fn a_changed_path_cannot_forge_a_line_in_the_receipt() { + let dir = std::env::temp_dir().join(format!("openmax-t2nl-{}", uuid::Uuid::new_v4())); + let data = dir.join("data"); + let project = dir.join("project"); + std::fs::create_dir_all(&project).unwrap(); + let old = Registry::from_snapshot(crate::registry::capture_extensions(&data, &project)); + + let forged = "and the file was approved"; + let manifest = project.join(format!(".openmax/tools/a\n{forged}.toml")); + std::fs::create_dir_all(manifest.parent().unwrap()).unwrap(); + std::fs::write(&manifest, "name = \"a\"\ndescription = \"x\"\ncommand = \"/bin/echo\"\n") + .unwrap(); + let skill = project.join(format!(".agents/skills/b\n{forged}/SKILL.md")); + std::fs::create_dir_all(skill.parent().unwrap()).unwrap(); + std::fs::write(&skill, "---\nname: b\ndescription: y\n---\nbody\n").unwrap(); + let new = Registry::from_snapshot(crate::registry::capture_extensions(&data, &project)); + + let changes = generation_changes(&old, &new, &project); + assert_eq!(changes.len(), 2, "{changes:?}"); + for line in &changes { + assert!(!line.contains('\n'), "{line:?}"); + } + assert!(changes[0].starts_with(".agents/skills/b "), "{:?}", changes[0]); + assert!(changes[1].starts_with(".openmax/tools/a "), "{:?}", changes[1]); let _ = std::fs::remove_dir_all(dir); } - /// The suppression must not over-reach: a broken file at another path - /// occupying a DIFFERENT name explains nothing about this tool, so a - /// genuinely deleted approved manifest is still reported as removed. + /// A directory dropped at the manifest's `.toml` path lands in `broken` + /// ("unreadable: ..."), and the receipt's NOT-loaded clause names it. It + /// must not ALSO be called removed: two lines calling the same path + /// present-but-unreadable and gone contradict each other. The capture's + /// own observation - a broken entry at this path - wins. #[test] - fn an_unrelated_broken_file_does_not_suppress_a_removal() { - let dir = std::env::temp_dir().join(format!("openmax-t2un-{}", uuid::Uuid::new_v4())); + fn a_directory_at_the_manifest_path_is_broken_not_doubly_removed() { + let dir = std::env::temp_dir().join(format!("openmax-t2dir-{}", uuid::Uuid::new_v4())); let data = dir.join("data"); let project = dir.join("project"); let manifest = project.join(".openmax/tools/deploy.toml"); @@ -6247,35 +6119,31 @@ mod tests { "name = \"deploy\"\ndescription = \"ships it\"\ncommand = \"/bin/echo\"\n", ) .unwrap(); - let sha = crate::ledger::sha256_hex(&std::fs::read(&manifest).unwrap()); - crate::ledger::approve_capability(&data, &project, &manifest, &[sha]).unwrap(); let old = Registry::from_snapshot(crate::registry::capture_extensions(&data, &project)); assert!(old.get("deploy").is_some()); + // The manifest file is replaced by a directory at the same path. std::fs::remove_file(&manifest).unwrap(); - std::fs::write( - project.join(".openmax/tools/other.toml"), - "name = \"other\"\ndescription = \"broken, no command\"\n", - ) - .unwrap(); + std::fs::create_dir(&manifest).unwrap(); + assert!(manifest.exists() && !manifest.is_file(), "a directory now sits at the path"); let new = Registry::from_snapshot(crate::registry::capture_extensions(&data, &project)); - - let added = classify_added_tools(&old, &new, &data, &project); + assert!(new.get("deploy").is_none()); assert!( - added.removed_approved == vec!["deploy".to_string()], - "an unrelated broken file must not swallow the removal: {:?} / {:?}", - added.removed_approved, - added.removed_approval_survives + new.broken.iter().any(|(p, r)| *p == manifest && r.contains("unreadable")), + "the directory is a broken entry the receipt names: {:?}", + new.broken ); + let changes = generation_changes(&old, &new, &project); + assert!(changes.is_empty(), "the broken clause speaks alone: {changes:?}"); + let _ = std::fs::remove_dir_all(dir); } - /// A valid approved manifest pushed past `MAX_EXTERNAL_TOOLS` by a newly - /// added tool is discovered-but-not-loaded (the prompt trailer reports the - /// count). The file never left disk, so it is not a removed tool; the - /// capture read it, and that read - not a fresh disk probe - is what the - /// classifier consults. + /// A valid manifest pushed past `MAX_EXTERNAL_TOOLS` by a newly added tool + /// is discovered-but-not-loaded (the prompt trailer reports the count). + /// The file never left disk, so it is not removed; the capture read it, + /// and that read - not a fresh disk probe - is what the receipt consults. #[test] fn a_tool_pushed_past_the_cap_is_not_a_removed_tool() { let dir = std::env::temp_dir().join(format!("openmax-t2cap-{}", uuid::Uuid::new_v4())); @@ -6298,15 +6166,8 @@ mod tests { "name = \"zz-deploy\"\ndescription = \"ships it\"\ncommand = \"/bin/echo\"\n", ) .unwrap(); - let sha = crate::ledger::sha256_hex(&std::fs::read(&manifest).unwrap()); - crate::ledger::approve_capability(&data, &project, &manifest, std::slice::from_ref(&sha)) - .unwrap(); - assert!( - crate::ledger::approved_hashes(&data, &project).unwrap().contains(&sha), - "the approval must record the manifest sha, or the classify probe is vacuous" - ); let old = Registry::from_snapshot(crate::registry::capture_extensions(&data, &project)); - assert!(old.get("zz-deploy").is_some(), "the approved tool holds the last cap slot"); + assert!(old.get("zz-deploy").is_some(), "the tool holds the last cap slot"); std::fs::write( tools_dir.join("aa-extra.toml"), @@ -6317,23 +6178,33 @@ mod tests { assert!(new.get("zz-deploy").is_none(), "the cap dropped the last-sorted tool"); assert_eq!(new.tools_omitted, 1, "the trailer accounts for the capped tool"); - let added = classify_added_tools(&old, &new, &data, &project); - assert!( - added.removed_approved.is_empty() && added.removed_approval_survives.is_empty(), - "a capped-out tool never left disk and is not a removal: {:?} / {:?}", - added.removed_approved, - added.removed_approval_survives + let changes = generation_changes(&old, &new, &project); + assert_eq!( + changes, + [".openmax/tools/aa-extra.toml added"], + "a capped-out tool never left disk and is not a removal" ); + // A manifest that arrives already past the cap never loads, but it is + // still a new file in the action space's source, and the receipt + // names it: the prompt trailer only counts. + std::fs::write( + tools_dir.join("zzz-late.toml"), + "name = \"zzz-late\"\ndescription = \"late\"\ncommand = \"/bin/echo\"\n", + ) + .unwrap(); + let newer = Registry::from_snapshot(crate::registry::capture_extensions(&data, &project)); + assert!(newer.get("zzz-late").is_none(), "the cap keeps it out"); + assert_eq!(generation_changes(&new, &newer, &project), [".openmax/tools/zzz-late.toml added"]); + let _ = std::fs::remove_dir_all(dir); } /// A manifest that stays a regular file but turns unreadable is /// present-but-broken, not removed. Discovery must record the read failure /// as a broken entry so the receipt's "NOT loaded" clause names it; before - /// that, the unreadable file was skipped entirely, and because the path is - /// still a file on disk the removal clauses were suppressed too, so the - /// approved tool vanished with no explanation anywhere. + /// that, the unreadable file was skipped entirely, and the tool vanished + /// with no explanation anywhere. #[cfg(unix)] #[test] fn an_unreadable_manifest_is_reported_broken_not_silently_vanished() { @@ -6348,8 +6219,6 @@ mod tests { "name = \"deploy\"\ndescription = \"ships it\"\ncommand = \"/bin/echo\"\n", ) .unwrap(); - let sha = crate::ledger::sha256_hex(&std::fs::read(&manifest).unwrap()); - crate::ledger::approve_capability(&data, &project, &manifest, &[sha]).unwrap(); let old = Registry::from_snapshot(crate::registry::capture_extensions(&data, &project)); assert!(old.get("deploy").is_some()); @@ -6371,13 +6240,8 @@ mod tests { new.broken ); - let added = classify_added_tools(&old, &new, &data, &project); - assert!( - added.removed_approved.is_empty() && added.removed_approval_survives.is_empty(), - "an unreadable manifest is present-but-broken, not removed: {:?} / {:?}", - added.removed_approved, - added.removed_approval_survives - ); + let changes = generation_changes(&old, &new, &project); + assert!(changes.is_empty(), "an unreadable manifest is present-but-broken, not removed: {changes:?}"); std::fs::set_permissions(&manifest, std::fs::Permissions::from_mode(0o644)).unwrap(); let _ = std::fs::remove_dir_all(dir); @@ -6388,7 +6252,7 @@ mod tests { /// fails). The extension refreeze runs after any mutating call, not only a /// successful one, for the same reason the permissions reload does; gating /// it on success left the written tool absent from the next step's schemas - /// and, at turn end, recorded as an out-of-session edit. The manifest here + /// until the next turn started. The manifest here /// must reach the very next request even though the call that wrote it /// failed. The tool name is chosen to appear nowhere but its own schema, so /// the assertion cannot pass on the prompt or transcript echo alone. @@ -7082,470 +6946,18 @@ mod tests { let _ = std::fs::remove_dir_all(dir); } - /// Changes made while no session was running (a human, git, an installer) - /// are recorded at the next session's first turn start, as `external`. - /// The freeze reads disk directly, so without this reconciliation the - /// delta would either never be ledgered at all (the new registry's - /// fingerprint already matches disk, so no refreeze ever fires) or be - /// swept into the first mid-turn sync as the agent's own work. + /// Resuming a session in a fresh process must still see extension files + /// written after the manifest was persisted. The session is absent from + /// the in-memory map at that point, so the freeze check has to hydrate it + /// before comparing its registry against disk; otherwise `openmax -c` + /// spends its first turn without the tools the agent already wrote. #[tokio::test] - async fn first_turn_records_changes_made_between_sessions() { + async fn resumed_session_sees_extensions_on_its_first_turn() { use crate::state::Core; let dir = std::env::temp_dir().join(format!("openmax-agent-{}", uuid::Uuid::new_v4())); let (core, _rx) = Core::new(dir.clone()).unwrap(); - let project = dir.join("project"); - std::fs::create_dir_all(project.join(".openmax/tools")).unwrap(); - let manifest = project.join(".openmax/tools/deploy.toml"); - let v1 = "name = \"deploy\"\ndescription = \"ships it\"\ncommand = \"/bin/echo\"\n"; - std::fs::write(&manifest, v1).unwrap(); - - // An earlier session's first turn writes the baseline (Initial, since - // the ledger has never seen this project). - { - let mut data = build_session_data(&core, "earlier", &project).unwrap(); - data.messages.push(ChatMessage::user("hi")); - core.sessions.lock().await.insert("earlier".into(), data); - } - refreeze_if_extensions_changed(&core, "earlier", &project).await; - let baseline = crate::ledger::history(&core.data_dir, &project).unwrap(); - assert!( - baseline.iter().any(|r| r.path.ends_with("deploy.toml")), - "first contact must write the baseline" - ); - - // Between sessions the file changes, with no harness running. - let v2 = "name = \"deploy\"\ndescription = \"ships it twice\"\ncommand = \"/bin/echo\"\n"; - std::fs::write(&manifest, v2).unwrap(); - let v2_sha = crate::ledger::sha256_hex(v2.as_bytes()); - - // A fresh session freezes v2 straight from disk: fingerprints agree, - // so nothing refreezes - but the ledger must still meet v2. - { - let mut data = build_session_data(&core, "later", &project).unwrap(); - data.messages.push(ChatMessage::user("hi")); - core.sessions.lock().await.insert("later".into(), data); - } - refreeze_if_extensions_changed(&core, "later", &project).await; - let records = crate::ledger::history(&core.data_dir, &project).unwrap(); - let change = records - .iter() - .rev() - .find(|r| r.path.ends_with("deploy.toml") && r.sha256.as_deref() == Some(v2_sha.as_str())) - .expect("the between-sessions change must be recorded"); - assert_eq!( - change.actor, - crate::ledger::Actor::External, - "no turn was running, so the change is external, not the agent's" - ); - - // Reconciliation is once per session: the next turn start of the same - // session touches the ledger not at all. - let settled = records.len(); - refreeze_if_extensions_changed(&core, "later", &project).await; - assert_eq!( - crate::ledger::history(&core.data_dir, &project).unwrap().len(), - settled, - "a synced session must not re-record on every turn start" - ); - - let _ = std::fs::remove_dir_all(dir); - } - - /// A first-turn reconciliation that fails must say so and stay unsynced, - /// so the next turn start retries. Marking a failed sync as settled would - /// drop the between-sessions delta forever - or worse, hand it to the - /// next mid-turn sync to record as the agent's own work. - #[tokio::test] - async fn failed_first_turn_reconciliation_reports_and_retries() { - use crate::state::Core; - - let dir = std::env::temp_dir().join(format!("openmax-agent-{}", uuid::Uuid::new_v4())); - let (core, mut rx) = Core::new(dir.clone()).unwrap(); - let project = dir.join("project"); - std::fs::create_dir_all(project.join(".openmax/tools")).unwrap(); - let manifest = project.join(".openmax/tools/deploy.toml"); - let v1 = "name = \"deploy\"\ndescription = \"ships it\"\ncommand = \"/bin/echo\"\n"; - std::fs::write(&manifest, v1).unwrap(); - - { - let mut data = build_session_data(&core, "earlier", &project).unwrap(); - data.messages.push(ChatMessage::user("hi")); - core.sessions.lock().await.insert("earlier".into(), data); - } - refreeze_if_extensions_changed(&core, "earlier", &project).await; - while rx.try_recv().is_ok() {} - - // Between sessions the file changes - and the ledger breaks (a - // partial write): reconciliation must fail loudly, not settle. - let v2 = "name = \"deploy\"\ndescription = \"ships it twice\"\ncommand = \"/bin/echo\"\n"; - std::fs::write(&manifest, v2).unwrap(); - let log = crate::ledger::project_dir(&core.data_dir, &project).join("log.jsonl"); - let intact = std::fs::read_to_string(&log).unwrap(); - std::fs::write(&log, format!("{intact}not json")).unwrap(); - - { - let mut data = build_session_data(&core, "later", &project).unwrap(); - data.messages.push(ChatMessage::user("hi")); - core.sessions.lock().await.insert("later".into(), data); - } - refreeze_if_extensions_changed(&core, "later", &project).await; - let mut reported = false; - while let Ok(env) = rx.try_recv() { - if let AgentEvent::Error { message } = env.event { - assert!(message.contains("ledger"), "{message}"); - reported = true; - } - } - assert!(reported, "a failed reconciliation must be reported, not swallowed"); - - // The ledger is repaired; the next turn start retries and lands the - // delta as external, because the session never marked itself synced. - std::fs::write(&log, &intact).unwrap(); - refreeze_if_extensions_changed(&core, "later", &project).await; - let records = crate::ledger::history(&core.data_dir, &project).unwrap(); - let v2_sha = crate::ledger::sha256_hex(v2.as_bytes()); - let change = records - .iter() - .rev() - .find(|r| r.path.ends_with("deploy.toml") && r.sha256.as_deref() == Some(v2_sha.as_str())) - .expect("the retry must record the between-sessions change"); - assert_eq!(change.actor, crate::ledger::Actor::External); - - let _ = std::fs::remove_dir_all(dir); - } - - /// The laundering path: the first-turn reconciliation fails, and the - /// agent mutates an extension in that same turn. The mid-turn sync must - /// land the held turn-start generation as External before writing any - /// Session record - a head advanced past it would attribute the human's - /// pre-session edits to the agent for good. While the ledger stays - /// broken the Session sync is skipped too (activation still proceeds); - /// once it heals, the held generation lands External, then the agent's - /// own delta lands Session. - #[tokio::test] - async fn midturn_sync_settles_the_external_backlog_first() { - use crate::state::Core; - - let dir = std::env::temp_dir().join(format!("openmax-agent-{}", uuid::Uuid::new_v4())); - let (core, _rx) = Core::new(dir.clone()).unwrap(); - let project = dir.join("project"); - std::fs::create_dir_all(project.join(".openmax/tools")).unwrap(); - let deploy = project.join(".openmax/tools/deploy.toml"); - let v1 = "name = \"deploy\"\ndescription = \"ships it\"\ncommand = \"/bin/echo\"\n"; - std::fs::write(&deploy, v1).unwrap(); - { - let mut data = build_session_data(&core, "earlier", &project).unwrap(); - data.messages.push(ChatMessage::user("hi")); - core.sessions.lock().await.insert("earlier".into(), data); - } - refreeze_if_extensions_changed(&core, "earlier", &project).await; - let baseline = crate::ledger::history(&core.data_dir, &project).unwrap().len(); - - // Between sessions: a human edits the tool, and the ledger breaks. - let v2 = "name = \"deploy\"\ndescription = \"ships it twice\"\ncommand = \"/bin/echo\"\n"; - std::fs::write(&deploy, v2).unwrap(); - let log = crate::ledger::project_dir(&core.data_dir, &project).join("log.jsonl"); - let intact = std::fs::read_to_string(&log).unwrap(); - std::fs::write(&log, format!("{intact}not json")).unwrap(); - - // First turn start fails and holds the External generation. - { - let mut data = build_session_data(&core, "later", &project).unwrap(); - data.messages.push(ChatMessage::user("hi")); - core.sessions.lock().await.insert("later".into(), data); - } - refreeze_if_extensions_changed(&core, "later", &project).await; - let (mut messages, mut registry) = { - let mut map = core.sessions.lock().await; - let data = map.get_mut("later").unwrap(); - let (messages, _seq) = take_messages(data); - (messages, data.registry.clone()) - }; - - // The agent writes a tool while the ledger is still broken: - // activation proceeds, but no Session record may land over the - // unsettled backlog. - std::fs::write( - project.join(".openmax/tools/built.toml"), - "name = \"built\"\ndescription = \"agent-written\"\ncommand = \"/bin/echo\"\n", - ) - .unwrap(); - assert!( - refreeze_between_iterations(&core, "later", &project, &mut registry, &mut messages) - .await, - "activation must not wait on the ledger" - ); - assert!(registry.get("built").is_some(), "the new tool is live"); - std::fs::write(&log, &intact).unwrap(); - assert_eq!( - crate::ledger::history(&core.data_dir, &project).unwrap().len(), - baseline, - "nothing may land while the backlog cannot: a Session record here is the laundering" - ); - - // Healed: the next mid-turn sync settles the backlog as External - // first, then records the agent's own delta as Session. - std::fs::write( - project.join(".openmax/tools/second.toml"), - "name = \"second\"\ndescription = \"agent-written too\"\ncommand = \"/bin/echo\"\n", - ) - .unwrap(); - assert!( - refreeze_between_iterations(&core, "later", &project, &mut registry, &mut messages) - .await - ); - let records = crate::ledger::history(&core.data_dir, &project).unwrap(); - let v2_sha = crate::ledger::sha256_hex(v2.as_bytes()); - let external_at = records - .iter() - .position(|r| { - r.path.ends_with("deploy.toml") && r.sha256.as_deref() == Some(v2_sha.as_str()) - }) - .expect("the human's edit must be recorded"); - assert_eq!(records[external_at].actor, crate::ledger::Actor::External); - for name in ["built.toml", "second.toml"] { - let at = records - .iter() - .position(|r| r.path.ends_with(name)) - .unwrap_or_else(|| panic!("{name} must be recorded")); - assert_eq!( - records[at].actor, - crate::ledger::Actor::Session, - "the agent's own work stays the agent's" - ); - assert!( - external_at < at, - "the External backlog must land before any Session record" - ); - } - - let _ = std::fs::remove_dir_all(dir); - } - - /// A turn-start sync that failed is retried at the next turn start, still - /// as external work. Agent changes made while the ledger was down land - /// external too, and the human's edit is never filed as the session's: - /// attribution errs toward understating the agent, never the human. The - /// generations observed across the outage collapse to what is on disk - /// when the ledger heals; the one in between is not recorded. - #[tokio::test] - async fn a_failed_turn_start_sync_is_retried_external_at_the_next_turn() { - use crate::state::Core; - - let dir = std::env::temp_dir().join(format!("openmax-agent-{}", uuid::Uuid::new_v4())); - let (core, _rx) = Core::new(dir.clone()).unwrap(); - let project = dir.join("project"); - std::fs::create_dir_all(project.join(".openmax/tools")).unwrap(); - let deploy = project.join(".openmax/tools/deploy.toml"); - let v1 = "name = \"deploy\"\ndescription = \"ships it\"\ncommand = \"/bin/echo\"\n"; - std::fs::write(&deploy, v1).unwrap(); - { - let mut data = build_session_data(&core, "earlier", &project).unwrap(); - data.messages.push(ChatMessage::user("hi")); - core.sessions.lock().await.insert("earlier".into(), data); - } - refreeze_if_extensions_changed(&core, "earlier", &project).await; - - // The ledger breaks; a human edits the tool; a new session starts. - let v2 = "name = \"deploy\"\ndescription = \"ships it twice\"\ncommand = \"/bin/echo\"\n"; - std::fs::write(&deploy, v2).unwrap(); - let log = crate::ledger::project_dir(&core.data_dir, &project).join("log.jsonl"); - let intact = std::fs::read_to_string(&log).unwrap(); - std::fs::write(&log, format!("{intact}not json")).unwrap(); - { - let mut data = build_session_data(&core, "later", &project).unwrap(); - data.messages.push(ChatMessage::user("hi")); - core.sessions.lock().await.insert("later".into(), data); - } - // First turn start fails: the External generation is held. - refreeze_if_extensions_changed(&core, "later", &project).await; - // The agent writes a tool mid-turn, still broken: nothing lands. - let (mut messages, mut registry) = { - let mut map = core.sessions.lock().await; - let data = map.get_mut("later").unwrap(); - let (messages, _seq) = take_messages(data); - (messages, data.registry.clone()) - }; - std::fs::write( - project.join(".openmax/tools/built.toml"), - "name = \"built\"\ndescription = \"agent-written\"\ncommand = \"/bin/echo\"\n", - ) - .unwrap(); - assert!( - refreeze_between_iterations(&core, "later", &project, &mut registry, &mut messages) - .await - ); - - // Turn ends (transcript restored); the ledger heals; a human edits - // the tool again while no turn runs. - { - let mut map = core.sessions.lock().await; - map.get_mut("later").unwrap().messages = messages; - } - std::fs::write(&log, &intact).unwrap(); - let v3 = "name = \"deploy\"\ndescription = \"ships it thrice\"\ncommand = \"/bin/echo\"\n"; - std::fs::write(&deploy, v3).unwrap(); - - // The stale turn-start refreeze records what is on disk now. - refreeze_if_extensions_changed(&core, "later", &project).await; - let records = crate::ledger::history(&core.data_dir, &project).unwrap(); - let find = |sha: &str, name: &str| { - records - .iter() - .find(|r| r.path.ends_with(name) && r.sha256.as_deref() == Some(sha)) - }; - let v3_rec = find(&crate::ledger::sha256_hex(v3.as_bytes()), "deploy.toml") - .expect("the human's latest edit is recorded"); - assert_eq!(v3_rec.actor, crate::ledger::Actor::External); - let built_sha = crate::ledger::sha256_hex( - &std::fs::read(project.join(".openmax/tools/built.toml")).unwrap(), - ); - let built_rec = find(&built_sha, "built.toml").expect("the agent's tool is recorded"); - assert_eq!( - built_rec.actor, - crate::ledger::Actor::External, - "work the outage hid lands as external: the direction that understates the agent" - ); - assert!( - find(&crate::ledger::sha256_hex(v2.as_bytes()), "deploy.toml").is_none(), - "the generation between two failures is not recorded" - ); - assert!( - records.iter().all(|r| r.actor != crate::ledger::Actor::Session), - "nothing from this window is filed as the session's" - ); - - let _ = std::fs::remove_dir_all(dir); - } - - /// `/reload` records as external work, so a turn-start generation a - /// failed reconciliation left held is superseded by the reload's own - /// snapshot: the human's edit lands as external and nothing stays held. - #[tokio::test] - async fn reload_lands_an_unrecorded_turn_start_generation() { - use crate::state::Core; - - let dir = std::env::temp_dir().join(format!("openmax-agent-{}", uuid::Uuid::new_v4())); - let (core, _rx) = Core::new(dir.clone()).unwrap(); - let project = dir.join("project"); - std::fs::create_dir_all(project.join(".openmax/tools")).unwrap(); - crate::trust::trust_project(&core.data_dir, &project).unwrap(); - let deploy = project.join(".openmax/tools/deploy.toml"); - let v1 = "name = \"deploy\"\ndescription = \"ships it\"\ncommand = \"/bin/echo\"\n"; - std::fs::write(&deploy, v1).unwrap(); - { - let mut data = build_session_data(&core, "s", &project).unwrap(); - data.messages.push(ChatMessage::user("hi")); - core.sessions.lock().await.insert("s".into(), data); - } - refreeze_if_extensions_changed(&core, "s", &project).await; - - // Break the ledger, edit the tool, fail the first-turn reconcile. - let v2 = "name = \"deploy\"\ndescription = \"ships it twice\"\ncommand = \"/bin/echo\"\n"; - std::fs::write(&deploy, v2).unwrap(); - let log = crate::ledger::project_dir(&core.data_dir, &project).join("log.jsonl"); - let intact = std::fs::read_to_string(&log).unwrap(); - std::fs::write(&log, format!("{intact}not json")).unwrap(); - { - let mut map = core.sessions.lock().await; - map.get_mut("s").unwrap().ledger_synced = false; - } - refreeze_if_extensions_changed(&core, "s", &project).await; - assert!( - core.sessions.lock().await.get("s").unwrap().unrecorded_external.is_some(), - "the failed generation must be held" - ); - - // Heal, then /reload: the human's edit lands as external. - std::fs::write(&log, &intact).unwrap(); - reload_session(&core, "s", &project).await.unwrap(); - let records = crate::ledger::history(&core.data_dir, &project).unwrap(); - let v2_sha = crate::ledger::sha256_hex(v2.as_bytes()); - let change = records - .iter() - .find(|r| r.path.ends_with("deploy.toml") && r.sha256.as_deref() == Some(v2_sha.as_str())) - .expect("reload must land the held generation"); - assert_eq!(change.actor, crate::ledger::Actor::External); - assert!( - core.sessions.lock().await.get("s").unwrap().unrecorded_external.is_none(), - "nothing may stay held after a successful reload" - ); - - let _ = std::fs::remove_dir_all(dir); - } - - /// A reload refused because a turn owns the transcript must refuse - /// before touching the ledger: settling first would land the held - /// generation and mark the session reconciled for a registry generation - /// that was never applied - ledger state moving for a reload that did - /// not happen. - #[tokio::test] - async fn refused_reload_leaves_the_ledger_untouched() { - use crate::state::Core; - - let dir = std::env::temp_dir().join(format!("openmax-agent-{}", uuid::Uuid::new_v4())); - let (core, _rx) = Core::new(dir.clone()).unwrap(); - let project = dir.join("project"); - std::fs::create_dir_all(project.join(".openmax/tools")).unwrap(); - crate::trust::trust_project(&core.data_dir, &project).unwrap(); - let deploy = project.join(".openmax/tools/deploy.toml"); - let v1 = "name = \"deploy\"\ndescription = \"ships it\"\ncommand = \"/bin/echo\"\n"; - std::fs::write(&deploy, v1).unwrap(); - { - let mut data = build_session_data(&core, "s", &project).unwrap(); - data.messages.push(ChatMessage::user("hi")); - core.sessions.lock().await.insert("s".into(), data); - } - refreeze_if_extensions_changed(&core, "s", &project).await; - let baseline = crate::ledger::history(&core.data_dir, &project).unwrap().len(); - - // A failed reconcile leaves a generation held; the ledger then heals. - let v2 = "name = \"deploy\"\ndescription = \"ships it twice\"\ncommand = \"/bin/echo\"\n"; - std::fs::write(&deploy, v2).unwrap(); - let log = crate::ledger::project_dir(&core.data_dir, &project).join("log.jsonl"); - let intact = std::fs::read_to_string(&log).unwrap(); - std::fs::write(&log, format!("{intact}not json")).unwrap(); - { - let mut map = core.sessions.lock().await; - map.get_mut("s").unwrap().ledger_synced = false; - } - refreeze_if_extensions_changed(&core, "s", &project).await; - std::fs::write(&log, &intact).unwrap(); - - // A turn takes the transcript, then a reload races in: it must be - // refused with the queue and history exactly as they were. - { - let mut map = core.sessions.lock().await; - let _ = take_messages(map.get_mut("s").unwrap()); - } - let err = reload_session(&core, "s", &project).await.unwrap_err(); - assert!(err.contains("turn is in flight"), "{err}"); - assert_eq!( - crate::ledger::history(&core.data_dir, &project).unwrap().len(), - baseline, - "a refused reload must not record anything" - ); - assert!( - core.sessions.lock().await.get("s").unwrap().unrecorded_external.is_some(), - "the held generation must survive a refused reload" - ); - - let _ = std::fs::remove_dir_all(dir); - } - - /// Resuming a session in a fresh process must still see extension files - /// written after the manifest was persisted. The session is absent from - /// the in-memory map at that point, so the freeze check has to hydrate it - /// before comparing its registry against disk; otherwise `openmax -c` - /// spends its first turn without the tools the agent already wrote. - #[tokio::test] - async fn resumed_session_sees_extensions_on_its_first_turn() { - use crate::state::Core; - - let dir = std::env::temp_dir().join(format!("openmax-agent-{}", uuid::Uuid::new_v4())); - let (core, _rx) = Core::new(dir.clone()).unwrap(); - let id = &sessions::create(&core, "/tmp/p".into()).unwrap().id; + let id = &sessions::create(&core, "/tmp/p".into()).unwrap().id; let project = dir.join("project"); std::fs::create_dir_all(&project).unwrap(); @@ -8153,6 +7565,344 @@ mod tests { let _ = std::fs::remove_dir_all(dir); } + /// One reply carrying `calls` as (id, tool, arguments), in order. + fn tool_calls_sse(calls: &[(&str, &str, Value)]) -> String { + let calls: Vec = calls + .iter() + .enumerate() + .map(|(index, (id, name, args))| { + serde_json::json!({ + "index": index, + "id": id, + "function": { "name": name, "arguments": args.to_string() }, + }) + }) + .collect(); + format!( + "data: {}\n\ndata: {}\n\ndata: [DONE]\n\n", + serde_json::json!({ "choices": [{ "delta": { "tool_calls": calls }, "finish_reason": null }] }), + serde_json::json!({ "choices": [{ "delta": {}, "finish_reason": "tool_calls" }] }), + ) + } + + /// Auto runs external tools without content approval, so an auto turn + /// has no question to put to the approval chain, and every read of it was + /// pure cost: each turn start re-read and hashed the whole log, each + /// mutating call re-read it once per external tool, and a refreeze copied + /// every capability file into the history. The same unasked question kept + /// unapproved read-only tools out of concurrent batches. Exercised end to + /// end: two calls of an unapproved read-only tool in one reply, then a + /// write that adds a tool mid-turn, then the answer. + #[tokio::test] + async fn an_auto_turn_batches_unapproved_tools_and_never_reads_the_ledger() { + use crate::state::Core; + + let dir = std::env::temp_dir().join(format!("openmax-agent-{}", uuid::Uuid::new_v4())); + let (core, mut rx) = Core::new(dir.clone()).unwrap(); + let project = dir.join("project"); + std::fs::create_dir_all(project.join(".openmax/tools")).unwrap(); + std::fs::write( + project.join(".openmax/tools/peek.toml"), + "name = \"peek\"\ndescription = \"reads\"\ncommand = \"/bin/echo\"\nargs = [\"peeked\"]\n", + ) + .unwrap(); + crate::trust::trust_project(&core.data_dir, &project).unwrap(); + // A chain with a record in it, so a read would have something to read. + crate::ledger::approve_hash(&core.data_dir, &project, &"0".repeat(64)).unwrap(); + + let peek_twice = tool_calls_sse(&[ + ("c1", "peek", serde_json::json!({})), + ("c2", "peek", serde_json::json!({})), + ]); + let add_tool = tool_calls_sse(&[( + "c3", + "write_file", + serde_json::json!({ + "path": ".openmax/tools/later.toml", + "content": "name = \"later\"\ndescription = \"d\"\ncommand = \"/bin/echo\"\n", + }), + )]); + let (base_url, requests) = scripted_endpoint(&[&peek_twice, &add_tool, STOP_SSE]).await; + { + let mut s = core.settings.lock().unwrap(); + s.base_url = base_url; + s.model = "stub".into(); + s.context_tokens = Some(16384); + s.approval_mode = ApprovalMode::Auto; + s.max_agent_iterations = 5; + } + let reads_before = crate::ledger::log_reads(&core.data_dir, &project); + + start_turn(core.clone(), "sess-auto-ledger".into(), project.clone(), "peek twice".into()) + .unwrap(); + let deadline = tokio::time::Instant::now() + Duration::from_secs(30); + let mut order: Vec = Vec::new(); + let mut stop = None; + while tokio::time::Instant::now() < deadline { + if let Ok(Some(env)) = + tokio::time::timeout(Duration::from_millis(200), rx.recv()).await + { + match env.event { + AgentEvent::ToolStart { call_id, .. } => order.push(format!("start {call_id}")), + AgentEvent::ToolEnd { call_id, ok, output } => { + assert!(ok, "{call_id} failed: {output}"); + order.push(format!("end {call_id}")); + } + AgentEvent::Done { stop_reason } => { + stop = Some(stop_reason); + break; + } + _ => {} + } + } + } + assert_eq!(stop.as_deref(), Some("stop"), "{order:?}"); + assert_eq!(*requests.lock().unwrap(), 3); + assert!(project.join(".openmax/tools/later.toml").is_file()); + assert_eq!( + crate::ledger::log_reads(&core.data_dir, &project) - reads_before, + 0, + "an auto turn never reads the approval chain (calls: {order:?})" + ); + // A concurrent batch starts every call before any ends; the serial + // path alternates start and end. + assert_eq!( + order[..4], + ["start c1", "start c2", "end c1", "end c2"], + "two read-only calls of an unapproved tool batch under auto: {order:?}" + ); + + let _ = std::fs::remove_dir_all(dir); + } + + /// A reply is split into batches once, under the mode of that moment, + /// but each batch runs under the mode of its own moment. Auto groups two + /// calls of an unapproved read-only tool; if the user switches to ask + /// while an earlier call in the same reply runs, that group must take the + /// serial path and raise the card. Run as a batch, the content gate + /// declined both calls and the human was never asked. + #[tokio::test] + async fn a_batch_formed_in_auto_asks_when_the_mode_turns_to_ask_before_it_runs() { + use crate::state::Core; + + let dir = std::env::temp_dir().join(format!("openmax-agent-{}", uuid::Uuid::new_v4())); + let (core, mut rx) = Core::new(dir.join("data")).unwrap(); + let project = dir.join("project"); + std::fs::create_dir_all(project.join(".openmax/tools")).unwrap(); + std::fs::write( + project.join(".openmax/tools/peek.toml"), + "name = \"peek\"\ndescription = \"reads\"\ncommand = \"/bin/echo\"\nargs = [\"peeked\"]\n", + ) + .unwrap(); + crate::trust::trust_project(&core.data_dir, &project).unwrap(); + core.set_project_approval_mode(&project, ApprovalMode::Auto).unwrap(); + + // The serial call says it is running, then waits for the test to + // switch the mode, so the switch lands between partition and batch. + let started = project.join("started"); + let go = project.join("go"); + let command = format!( + "touch '{}'; for i in $(seq 1 600); do [ -f '{}' ] && exit 0; sleep 0.05; done; exit 1", + started.display(), + go.display(), + ); + let reply = tool_calls_sse(&[ + ("c0", "bash", serde_json::json!({ "command": command })), + ("c1", "peek", serde_json::json!({})), + ("c2", "peek", serde_json::json!({})), + ]); + let (base_url, _requests) = scripted_endpoint(&[&reply, STOP_SSE]).await; + { + let mut s = core.settings.lock().unwrap(); + s.base_url = base_url; + s.model = "stub".into(); + s.context_tokens = Some(16384); + s.max_agent_iterations = 3; + } + + start_turn(core.clone(), "sess-batch-mode".into(), project.clone(), "peek".into()).unwrap(); + let deadline = tokio::time::Instant::now() + Duration::from_secs(30); + let mut switched = false; + let mut cards = 0; + let mut ends: Vec<(String, bool, String)> = Vec::new(); + let mut stop = None; + while tokio::time::Instant::now() < deadline { + if !switched && started.exists() { + core.set_project_approval_mode(&project, ApprovalMode::Ask).unwrap(); + std::fs::write(&go, "").unwrap(); + switched = true; + } + match tokio::time::timeout(Duration::from_millis(50), rx.recv()).await { + Ok(Some(env)) => match env.event { + AgentEvent::ApprovalRequest { approval_id, .. } => { + cards += 1; + core.respond_approval(&approval_id, true); + } + AgentEvent::ToolEnd { call_id, ok, output } => ends.push((call_id, ok, output)), + AgentEvent::Done { stop_reason } => { + stop = Some(stop_reason); + break; + } + _ => {} + }, + Ok(None) => panic!("event stream closed before Done"), + Err(_) => {} + } + } + assert!(switched, "the serial call never started: {ends:?}"); + assert_eq!(stop.as_deref(), Some("stop"), "{ends:?}"); + assert!( + cards >= 1, + "the unapproved tool must raise a card once the mode is ask: {ends:?}" + ); + assert_eq!(ends.len(), 3, "{ends:?}"); + assert!( + ends.iter().all(|(_, ok, _)| *ok), + "no call may be declined without the human being asked: {ends:?}" + ); + + let _ = std::fs::remove_dir_all(dir); + } + + /// A batch admits every call up front and then starts them behind the + /// parallelism cap, so a call can wait in the queue long after auto let + /// it in. Auto never asked whether the tool's content is approved; if + /// the user leaves auto while the call waits, starting it anyway runs + /// unapproved host code under ask with no card. Here the first call + /// holds the only slot while the mode turns to ask, and the queued + /// second call of the same unapproved tool must not start. + #[cfg(unix)] + #[tokio::test] + async fn a_queued_batch_call_does_not_start_once_the_mode_leaves_auto() { + use crate::state::Core; + + let dir = std::env::temp_dir().join(format!("openmax-agent-{}", uuid::Uuid::new_v4())); + let (core, mut rx) = Core::new(dir.join("data")).unwrap(); + let project = dir.join("project"); + std::fs::create_dir_all(project.join(".openmax/tools")).unwrap(); + // Each run is counted, and a run says it started, then waits for the + // test to switch the mode before it ends. + let runs = project.join("runs"); + let started = project.join("started"); + let go = project.join("go"); + let script = format!( + "echo run >> '{}'; touch '{}'; for i in $(seq 1 600); do [ -f '{}' ] && exit 0; sleep 0.05; done; exit 1", + runs.display(), + started.display(), + go.display(), + ); + std::fs::write( + project.join(".openmax/tools/peek.toml"), + format!( + "name = \"peek\"\ndescription = \"reads\"\ncommand = \"/bin/sh\"\nargs = [\"-c\", {}]\n", + // A JSON string literal is a valid TOML basic string here. + serde_json::Value::String(script) + ), + ) + .unwrap(); + crate::trust::trust_project(&core.data_dir, &project).unwrap(); + core.set_project_approval_mode(&project, ApprovalMode::Auto).unwrap(); + + let reply = tool_calls_sse(&[ + ("c1", "peek", serde_json::json!({})), + ("c2", "peek", serde_json::json!({})), + ]); + let (base_url, _requests) = scripted_endpoint(&[&reply, STOP_SSE]).await; + { + let mut s = core.settings.lock().unwrap(); + s.base_url = base_url; + s.model = "stub".into(); + s.context_tokens = Some(16384); + s.max_agent_iterations = 3; + // One slot: the second call waits until the first ends. + s.max_parallel_tools = 1; + } + + start_turn(core.clone(), "sess-batch-queue".into(), project.clone(), "peek".into()).unwrap(); + let deadline = tokio::time::Instant::now() + Duration::from_secs(30); + let mut switched = false; + let mut cards = 0; + let mut ends: Vec<(String, bool, String)> = Vec::new(); + let mut stop = None; + while tokio::time::Instant::now() < deadline { + if !switched && started.exists() { + core.set_project_approval_mode(&project, ApprovalMode::Ask).unwrap(); + std::fs::write(&go, "").unwrap(); + switched = true; + } + match tokio::time::timeout(Duration::from_millis(50), rx.recv()).await { + Ok(Some(env)) => match env.event { + AgentEvent::ApprovalRequest { approval_id, .. } => { + cards += 1; + core.respond_approval(&approval_id, false); + } + AgentEvent::ToolEnd { call_id, ok, output } => ends.push((call_id, ok, output)), + AgentEvent::Done { stop_reason } => { + stop = Some(stop_reason); + break; + } + _ => {} + }, + Ok(None) => panic!("event stream closed before Done"), + Err(_) => {} + } + } + assert!(switched, "the first call never started: {ends:?}"); + assert_eq!(stop.as_deref(), Some("stop"), "{ends:?}"); + assert_eq!(cards, 0, "a batch raises no card: {ends:?}"); + let ran = std::fs::read_to_string(&runs).unwrap_or_default().lines().count(); + assert_eq!(ran, 1, "the queued call ran under ask without a card: {ends:?}"); + assert_eq!(ends.len(), 2, "{ends:?}"); + assert!(ends[0].0 == "c1" && ends[0].1, "{ends:?}"); + assert_eq!( + (ends[1].0.as_str(), ends[1].1, ends[1].2.as_str()), + ("c2", false, MODE_LEFT_AUTO_BEFORE_START), + "the queued call is refused with a reason to ask again" + ); + + let _ = std::fs::remove_dir_all(dir); + } + + /// A session built in auto never read the approval chain, so its first + /// turn in ask takes what is on record as already seen instead of naming + /// every approval ever granted as activity it did not perform; a grant + /// made after that is still named. + #[tokio::test] + async fn a_session_built_in_auto_starts_approval_narration_from_what_is_on_record() { + use crate::state::Core; + + let dir = std::env::temp_dir().join(format!("openmax-agent-{}", uuid::Uuid::new_v4())); + let (core, _rx) = Core::new(dir.clone()).unwrap(); + let project = dir.join("project"); + std::fs::create_dir_all(&project).unwrap(); + core.settings.lock().unwrap().approval_mode = ApprovalMode::Auto; + crate::ledger::approve_hash(&core.data_dir, &project, &"a".repeat(64)).unwrap(); + let reads = crate::ledger::log_reads(&core.data_dir, &project); + let data = build_session_data(&core, "s", &project).unwrap(); + assert!(data.seen_ledger_events.is_none(), "auto builds without reading the chain"); + assert_eq!(crate::ledger::log_reads(&core.data_dir, &project), reads); + core.sessions.lock().await.insert("s".into(), data); + + let mut messages = vec![ChatMessage::user("hi")]; + narrate_outside_approvals(&core, "s", &project, &mut messages).await; + assert_eq!(messages.len(), 1, "auto narrates nothing and reads nothing"); + assert_eq!(crate::ledger::log_reads(&core.data_dir, &project), reads); + + core.settings.lock().unwrap().approval_mode = ApprovalMode::Ask; + narrate_outside_approvals(&core, "s", &project, &mut messages).await; + assert_eq!(messages.len(), 1, "an approval on record before the first read is not news"); + + crate::ledger::approve_hash(&core.data_dir, &project, &"b".repeat(64)).unwrap(); + narrate_outside_approvals(&core, "s", &project, &mut messages).await; + assert_eq!(messages.len(), 2, "{messages:?}"); + assert!( + messages[0].content.as_deref().is_some_and(|c| c.contains("this session did not perform")), + "{messages:?}" + ); + + let _ = std::fs::remove_dir_all(dir); + } + /// A read-only call repeated with byte-identical results is the shape of /// a model that has finished but keeps "verifying" instead of answering: /// its reasoning says the task is complete, its reply is the same tool diff --git a/crates/core/src/ledger.rs b/crates/core/src/ledger.rs index a40d9e1f..9979c3b9 100644 --- a/crates/core/src/ledger.rs +++ b/crates/core/src/ledger.rs @@ -1,21 +1,19 @@ //! The capability ledger: a core-owned, append-only, hash-chained record of -//! every observed change to the capability files that enter the frozen -//! registry (external tool TOMLs and skill SKILL.mds), plus a -//! content-addressed store of the bytes themselves. +//! the content approvals a human granted for a project's capability files. //! -//! Why the core owns this: the agent can write any file inside the project, -//! so only the process that runs turns can honestly say *when* a capability -//! file changed relative to the session lifecycle. The ledger lives outside -//! the project (like `trust.json`), where the confined file tools never -//! write. Each record embeds the sha256 of the previous record line, so a -//! torn write or an edit that does not recompute the chain is detected. A -//! deliberate rewrite of log and pin together is not: there is no key, and -//! bash can reach the data dir. That is the honest ceiling without an OS -//! sandbox, and the same ceiling trust already lives at. +//! The ledger lives outside the project (like `trust.json`), where the +//! confined file tools never write. Each record embeds the sha256 of the +//! previous record line, so a torn write or an edit that does not recompute +//! the chain is detected. A deliberate rewrite of log and pin together is +//! not: there is no key, and bash can reach the data dir. That is the honest +//! ceiling without an OS sandbox, and the same ceiling trust already lives at. //! -//! Rollback is deliberately a file operation, not a product: `openmax -//! --ledger` prints history with object paths, and restoring is `cp`. The -//! core guarantees the history exists; using it stays ordinary file work. +//! Earlier builds also recorded every change to a tool or skill file here, +//! with a copy of its bytes under `objects/`. That history is no longer +//! recorded: keeping it hashed and copied every capability file at every +//! capture, and no gate ever consulted it. Logs that carry those change +//! records still parse and verify, so an older binary and this one share a +//! log, and stored objects are never deleted. use std::collections::HashMap; use std::io::Write as _; @@ -24,38 +22,29 @@ use std::path::{Path, PathBuf}; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; -/// Who changed a capability file, at the strength the core can prove. +/// Who wrote a record, at the strength the core can prove. #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum Actor { - /// Present when this project's ledger was first populated; prior history - /// is unknowable, so no stronger claim is made. + /// A change record from a project's first recorded freeze. Only earlier + /// builds wrote change records; kept so their logs still parse. Initial, - /// Changed while an agent turn was running in the named session: the - /// mid-turn refreeze after a successful mutating call observed it. + /// An approval granted at a card in the named session (or, in a change + /// record from an earlier build, a change a mid-turn refreeze observed). Session, - /// Changed while no turn was running (a human edit, `git pull`, a - /// third-party install): observed at turn start or `/reload`. + /// An approval granted outside any session (`openmax --approve`), or a + /// retirement (`openmax --forget`). External, } -impl Actor { - pub fn as_str(self) -> &'static str { - match self { - Actor::Initial => "initial", - Actor::Session => "session", - Actor::External => "external", - } - } -} - /// What a record asserts. Omitted from the wire for `Change` and defaulted on /// read, so every line an older harness wrote keeps its exact bytes and its /// place in the chain. #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum Kind { - /// A capability file appeared, changed, or was removed. + /// A capability file appeared, changed, or was removed. Written only by + /// earlier builds; read so their logs still verify, and never authority. #[default] Change, /// A human approved this exact content for unattended execution. @@ -80,11 +69,11 @@ impl Kind { } } -/// One observed change. `sha256` is `None` when the file was removed. -/// Approval records reuse the shape: `sha256` is the manifest's content, -/// `also` carries the rest of the hashes blessed in the same act (the -/// project-local code that manifest runs), and `path` is where the human -/// approved it (empty when the caller only knew a hash). +/// One record. In an approval, `sha256` is the manifest's content, `also` +/// carries the rest of the hashes blessed in the same act (the project-local +/// code that manifest runs), and `path` is where the human approved it (empty +/// when the caller only knew a hash). In a change record an earlier build +/// wrote, `sha256` is `None` when the file was removed. #[derive(Clone, Debug, Serialize, Deserialize)] pub struct Record { pub v: u32, @@ -126,15 +115,6 @@ fn is_false(b: &bool) -> bool { !*b } -/// One entry of a sync's outcome, for the refreeze receipt. -#[derive(Clone, Debug)] -pub struct Change { - pub path: PathBuf, - pub actor: Actor, - /// `added`, `modified`, or `removed`. - pub kind: &'static str, -} - /// Bumped when approvals moved into the chain. Older lines still parse; the /// number only says which build's shape a record carries. const RECORD_VERSION: u32 = 2; @@ -175,6 +155,27 @@ fn log_path(dir: &Path) -> PathBuf { dir.join("log.jsonl") } +/// Reads of each ledger directory's log, so a test can prove a path never +/// consults the chain. Keyed by directory: every test owns its data dir, and +/// tests share this process. +#[cfg(test)] +static LOG_READS: std::sync::OnceLock>> = + std::sync::OnceLock::new(); + +#[cfg(test)] +fn count_log_read(dir: &Path) { + let mut reads = LOG_READS.get_or_init(Default::default).lock().unwrap_or_else(|e| e.into_inner()); + *reads.entry(dir.to_path_buf()).or_default() += 1; +} + +/// How many times this project's log has been read in this process. +#[cfg(test)] +pub(crate) fn log_reads(data_dir: &Path, project_root: &Path) -> usize { + let dir = project_dir(data_dir, project_root); + let reads = LOG_READS.get_or_init(Default::default).lock().unwrap_or_else(|e| e.into_inner()); + reads.get(&dir).copied().unwrap_or(0) +} + fn lock_path(dir: &Path) -> PathBuf { dir.join("ledger.lock") } @@ -230,9 +231,9 @@ struct Verified { } impl Verified { - /// Records past the pin that grant or retire authority. A crashed sync - /// leaves only observations behind; anything stronger in an unpinned - /// tail has to wait for a human. + /// Records past the pin that grant or retire authority. A crashed + /// append of change records (an earlier build's) leaves only observations + /// behind; anything stronger in an unpinned tail has to wait for a human. fn unpinned_authority(&self) -> impl Iterator { self.records[self.pinned..].iter().filter(|r| !r.kind.is_change()) } @@ -252,17 +253,6 @@ fn refuse_unpinned_authority(verified: &Verified) -> Result<(), String> { ))) } -/// The full history plus whether the last append's pin never landed. -pub struct History { - pub records: Vec, - /// True when a crash left records past the pin; the next sync re-pins. - /// Nothing was removed, so this is a repairable state, not tampering. - pub interrupted_write: bool, - /// How many leading records the stored chain head vouches for. Records - /// past this index are the interrupted (unpinned) tail. - pub pinned: usize, -} - fn read_trimmed(path: &Path) -> Option { std::fs::read_to_string(path).ok().map(|s| s.trim().to_string()) } @@ -272,6 +262,8 @@ fn read_trimmed(path: &Path) -> Option { /// error, never silently skipped or displayed as authentic: a ledger that /// cannot be trusted must not be read around. fn read_verified(dir: &Path) -> Result { + #[cfg(test)] + count_log_read(dir); let path = log_path(dir); let (text, log_present) = match std::fs::read_to_string(&path) { Ok(text) => (text, true), @@ -466,44 +458,6 @@ fn approval_event_id(generation: u64, chain_index: usize, r: &Record) -> u64 { h.finish() } -/// History plus the interrupted-write flag, for callers that report state. -pub fn read(data_dir: &Path, project_root: &Path) -> Result { - read_verified(&project_dir(data_dir, project_root)).map(|v| History { - records: v.records, - interrupted_write: v.pin == Pin::Interrupted, - pinned: v.pinned, - }) -} - -/// The last known hash per path (None = removed), from the full history. -/// Only change records describe files; approvals carry a hash, not a state. -fn head(records: &[Record]) -> HashMap> { - let mut map = HashMap::new(); - for r in records.iter().filter(|r| r.kind.is_change()) { - map.insert(r.path.clone(), r.sha256.clone()); - } - map -} - -/// Record the difference between the ledger head and `files` (the exact -/// generation a freeze read: path -> (sha256, bytes)). New and changed files -/// get `actor` (or `Initial` when the ledger is empty), removed paths get a -/// removal record, and changed content lands in `objects/`. Returns -/// what changed, for the refreeze receipt. Serialized by an exclusive flock; -/// callers already hold the turn, so contention is a second harness process. -pub fn sync( - data_dir: &Path, - project_root: &Path, - files: &[(PathBuf, String, Vec)], - actor: Actor, - session_id: Option<&str>, -) -> Result, String> { - let dir = project_dir(data_dir, project_root); - std::fs::create_dir_all(dir.join("objects")) - .map_err(|e| format!("cannot create {}: {e}", dir.display()))?; - with_lock(&dir, || sync_locked(&dir, files, actor, session_id)) -} - /// Run `f` under the ledger's exclusive flock. Never call this from inside /// another `with_lock`: flock is per open file description, so a second lock /// in the same process would wait on itself forever. @@ -564,7 +518,7 @@ fn write_durable(path: &Path, bytes: &[u8]) -> Result<(), String> { /// a record the log does not have; /// * the pin itself needs no barrier: losing it lands in exactly the state /// the pending pin describes, which reads as an interrupted write and -/// re-pins on the next sync. +/// re-pins on the next append (or `openmax --ledger-repair`). fn append_chained(dir: &Path, lines: &str, new_head: &str) -> Result<(), String> { write_durable(&pending_head_path(dir), new_head.as_bytes())?; let mut file = std::fs::OpenOptions::new() @@ -580,101 +534,6 @@ fn append_chained(dir: &Path, lines: &str, new_head: &str) -> Result<(), String> Ok(()) } -fn sync_locked( - dir: &Path, - files: &[(PathBuf, String, Vec)], - actor: Actor, - session_id: Option<&str>, -) -> Result, String> { - let verified = read_verified(dir)?; - // A sync appends and re-pins, which must never quietly bless an unpinned - // authority tail; a change-only tail (a crashed sync) heals below. - refuse_unpinned_authority(&verified)?; - let known = head(&verified.records); - // Keyed on change records, not on the log: a ledger that so far holds - // only approvals has still never seen this project's files. - let effective_actor = if known.is_empty() { Actor::Initial } else { actor }; - let mut prev = verified.head.clone(); - - let ts = unix_now(); - - let mut changes = Vec::new(); - let mut lines = String::new(); - let mut seen: Vec<&PathBuf> = Vec::new(); - for (path, sha, bytes) in files { - seen.push(path); - let kind = match known.get(path) { - Some(Some(existing)) if existing == sha => continue, - Some(Some(_)) => "modified", - Some(None) => "modified", // re-added after removal - None => "added", - }; - // Never trust a pre-existing object blindly: rollback follows these - // bytes, so an object that does not hash to its own name is replaced - // with the authentic content this generation actually read. - let object = dir.join("objects").join(sha); - let object_valid = std::fs::read(&object) - .map(|existing| sha256_hex(&existing) == *sha) - .unwrap_or(false); - if !object_valid { - crate::sessions::write_atomic(&object, bytes)?; - } - let record = Record { - v: RECORD_VERSION, - ts, - path: path.clone(), - sha256: Some(sha.clone()), - actor: effective_actor, - session_id: session_id.map(str::to_string), - kind: Kind::Change, - also: Vec::new(), - event: None, - code: Vec::new(), - blocking: false, - prev: prev.clone(), - }; - let line = serde_json::to_string(&record).map_err(|e| e.to_string())?; - prev = sha256_hex(line.as_bytes()); - lines.push_str(&line); - lines.push('\n'); - changes.push(Change { path: path.clone(), actor: effective_actor, kind }); - } - // Removals: paths the ledger knows as present that this generation lacks. - for (path, last) in &known { - if last.is_some() && !seen.contains(&path) { - let record = Record { - v: RECORD_VERSION, - ts, - path: path.clone(), - sha256: None, - actor: effective_actor, - session_id: session_id.map(str::to_string), - kind: Kind::Change, - also: Vec::new(), - event: None, - code: Vec::new(), - blocking: false, - prev: prev.clone(), - }; - let line = serde_json::to_string(&record).map_err(|e| e.to_string())?; - prev = sha256_hex(line.as_bytes()); - lines.push_str(&line); - lines.push('\n'); - changes.push(Change { path: path.clone(), actor: effective_actor, kind: "removed" }); - } - } - - if !lines.is_empty() { - append_chained(dir, &lines, &prev)?; - } else if verified.pin == Pin::Interrupted { - // Heal a crash-interrupted append even when this generation changed - // nothing: the records are intact and chained, only the pin is stale. - crate::sessions::write_atomic(&chain_head_path(dir), &verified.head)?; - let _ = std::fs::remove_file(pending_head_path(dir)); - } - Ok(changes) -} - // ---------- content-bound approvals ---------- // // Approvals are ledger records, not a file beside the ledger. `approved.json` @@ -684,7 +543,7 @@ fn sync_locked( // hook and have the next session run it. As records they inherit the chain, // the pin, and the audit trail: a forged approval has to forge the chain // (detectable, and the same ceiling trust already lives at), and every real -// one shows up in `openmax --ledger` with its time, actor, and session. +// one carries its time, actor, and session. /// One hook as a human approved it: the shape reconciliation must remember, /// because the file itself can no longer be trusted to describe it. @@ -852,6 +711,8 @@ pub fn approvals(data_dir: &Path, project_root: &Path) -> Result ` restores what - // an approval blessed; until now approvals stored no object - // at all, so an approved manifest deleted before any freeze - // saw it - and EVERY bound script, which no freeze ever - // reads - was unrestorable while --ledger said otherwise - // sending a reader hunting for an object that cannot exist. - // Store the vouched manifest bytes, and each bound - // file whose on-disk bytes hash to a sha the human vouched. - store_object(&dir, vouched, &bytes)?; - if let Ok(text) = std::str::from_utf8(&bytes) { - for code in manifest_code_source(p, text, project_root) { - let (Some(sha), Ok(code_bytes)) = (code.sha256.as_deref(), std::fs::read(&code.path)) else { - continue; - }; - if shas.iter().any(|s| s == sha) && sha256_hex(&code_bytes) == sha { - store_object(&dir, sha, &code_bytes)?; - } - } - } // Every code hash this act records in `also` (that is, - // `shas[1..]`) must now have a stored object, or a restore of - // that hash would fail. A bound file changed, deleted, or made - // unreadable since the card hashed it leaves its vouched sha - // unstored: reject the whole approval rather than record a hash - // with no restorable bytes. Code that was already - // missing AT card time never entered `shas`, so this does not - // fire for it - that tool is simply not covered and asks again. - // The manifest object already written is orphaned (no record - // references it), never a dangling approval. - for code_sha in shas.iter().skip(1) { - // The object must exist AND hash to its own name: a mere - // is_file() check would accept a pre-existing object at - // objects/ that holds unrelated bytes (a changed - // script whose sha slot was pre-populated), so a restore - // would produce bytes the reviewer never approved. - // store_object only writes bytes that hash to - // the sha, so a valid object here means we stored it this - // act or an earlier act stored the identical bytes. - let intact = std::fs::read(dir.join("objects").join(code_sha)) - .map(|b| sha256_hex(&b) == *code_sha) - .unwrap_or(false); - if !intact { - return Err(format!( - "a bound file changed, was removed, or could not be read since it was shown, so its approved bytes ({}) are not restorable; nothing was approved - review the files and approve them again", - &code_sha[..code_sha.len().min(12)] - )); - } + // `shas[1..]`) must still be the bytes of a file these + // vouched manifest bytes name. A bound file changed, deleted, + // or made unreadable since the card hashed it would otherwise + // land a hash on record for bytes that no longer exist, next + // to a manifest the human believes is now covered: reject the + // whole approval instead. Code that was already missing AT + // card time never entered `shas`, so this does not fire for + // it - that tool is simply not covered and asks again. + let bound: Vec = std::str::from_utf8(&bytes) + .map(|text| { + manifest_code_source(p, text, project_root) + .into_iter() + .filter_map(|code| code.sha256) + .collect() + }) + .unwrap_or_default(); + if let Some(code_sha) = shas.iter().skip(1).find(|sha| !bound.contains(sha)) { + return Err(format!( + "a bound file changed, was removed, or could not be read since it was shown, so its bytes no longer hash to the vouched {}; nothing was approved - review the files and approve them again", + &code_sha[..code_sha.len().min(12)] + )); } std::str::from_utf8(&bytes) .ok() @@ -1249,135 +1085,6 @@ fn quarantine_tail_locked(dir: &Path, verified: &Verified) -> Result `, so a rewritten object is a backdoor with a -/// documented delivery route; the write path re-verifies, and this is the -/// read path doing the same. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub enum ObjectState { - Intact, - Missing, - Corrupt, -} - -/// Write `bytes` as `objects/` unless a valid object is already there. -/// Never trusts a pre-existing object blindly: one that does not hash to its -/// name is replaced with the authentic bytes. -fn store_object(dir: &Path, sha: &str, bytes: &[u8]) -> Result<(), String> { - let object = dir.join("objects").join(sha); - let valid = std::fs::read(&object) - .map(|existing| sha256_hex(&existing) == sha) - .unwrap_or(false); - if valid { - return Ok(()); - } - std::fs::create_dir_all(dir.join("objects")) - .map_err(|e| format!("cannot create {}: {e}", dir.join("objects").display()))?; - crate::sessions::write_atomic(&object, bytes) -} - -pub fn object_state(data_dir: &Path, project_root: &Path, sha: &str) -> ObjectState { - let path = project_dir(data_dir, project_root).join("objects").join(sha); - match std::fs::read(&path) { - Ok(bytes) if sha256_hex(&bytes) == sha => ObjectState::Intact, - Ok(_) => ObjectState::Corrupt, - Err(_) => ObjectState::Missing, - } -} - -/// True when an approval's PRIMARY manifest bytes cannot be restored: the -/// record approved a manifest sha but its object is missing or corrupt (never -/// stored, removed, or rewritten since). `--ledger` surfaces this so -/// a row with intact bound objects but an unrestorable manifest does not read -/// as fully restorable. False for non-approvals and path-only -/// approvals, which have no manifest object to restore. -pub fn approval_manifest_missing(data_dir: &Path, project_root: &Path, record: &Record) -> bool { - record.kind == Kind::Approval - && record - .sha256 - .as_deref() - .is_some_and(|sha| !matches!(object_state(data_dir, project_root, sha), ObjectState::Intact)) -} - -/// The `(sha, path)` pairs `--ledger`'s footer may offer as `cp` restore -/// commands for one approval's bound code: each vouched hash in `also`, -/// paired with the project file the approved bytes named for it, intact -/// objects only. -/// -/// The pairing is positional because it was recorded positionally: the -/// approval hashed the manifest's bound files in `manifest_code_source` -/// order and `approve()` wrote them to `also` in that order. A hook record -/// carries the named paths in `code`; a tool record carries none (`code` is -/// filled from the hook shape only), so its paths are re-derived by parsing -/// the STORED manifest object - the bytes the hash pins, never the live -/// file, which may be exactly what needs restoring. A bound file -/// the card could not read never entered `also`, so a hash list shorter than -/// the named-path list is ambiguous: nothing is offered then, because a -/// guessed pairing prints a command that writes approved bytes over the -/// wrong file. -pub fn approval_restore_targets( - data_dir: &Path, - project_root: &Path, - record: &Record, -) -> Vec<(String, PathBuf)> { - if record.kind != Kind::Approval || record.also.is_empty() { - return Vec::new(); - } - let paths: Vec = if !record.code.is_empty() && record.code.len() == record.also.len() { - record.code.iter().map(PathBuf::from).collect() - } else { - let named = approved_manifest_code_paths(data_dir, project_root, record); - if named.len() != record.also.len() { - return Vec::new(); - } - named - }; - record - .also - .iter() - .zip(paths) - .filter(|(sha, _)| matches!(object_state(data_dir, project_root, sha.as_str()), ObjectState::Intact)) - .map(|(sha, path)| (sha.clone(), path)) - .collect() -} - -/// The bound-code paths the approved manifest bytes name. The record's sha is -/// what authenticates the bytes, not where they live, so the stored object -/// and the file at the record's path are both accepted sources when their -/// bytes hash to it - a pruned object with the manifest still installed must -/// not hide the script restore the footer exists to print. Empty -/// when neither source hashes to the vouched sha (an edited manifest revokes, -/// so its named paths are nobody's to offer), when nothing was ever stored -/// (a hash-only approval), or when the record carries no path to parse -/// against. -fn approved_manifest_code_paths( - data_dir: &Path, - project_root: &Path, - record: &Record, -) -> Vec { - if record.path.as_os_str().is_empty() { - return Vec::new(); - } - let Some(sha) = record.sha256.as_deref() else { - return Vec::new(); - }; - let object = project_dir(data_dir, project_root).join("objects").join(sha); - let Some(bytes) = [object, record.path.clone()] - .iter() - .filter_map(|p| std::fs::read(p).ok()) - .find(|b| sha256_hex(b) == sha) - else { - return Vec::new(); - }; - let Ok(text) = std::str::from_utf8(&bytes) else { - return Vec::new(); - }; - manifest_code_source(&record.path, text, project_root) - .into_iter() - .map(|c| c.path) - .collect() -} - /// Unix seconds as `YYYY-MM-DD HH:MM:SSZ`. Raw epoch seconds are unreadable /// in an audit trail, and a date crate is not worth pulling in for one line. pub fn format_ts(secs: u64) -> String { @@ -1422,8 +1129,7 @@ fn hook_record_source(path: &Path, text: &str, project_root: &Path) -> Option Vec Vec Vec { - changes - .iter() - .map(|c| { - let path = c.path.strip_prefix(project_root).unwrap_or(&c.path); - // The path is the agent's own filename and this line is the - // receipt's leading clause, so it flattens where it is built. - crate::text::one_line(&format!( - "{} {} ({})", - path.display(), - c.kind, - c.actor.as_str() - )) - }) - .collect() -} - #[cfg(test)] mod tests { use super::*; @@ -1936,23 +1622,6 @@ mod tests { Ok(records_text.lines().filter(|l| !l.trim().is_empty()).count()) } - /// `describe` builds the receipt's leading "what changed" clause out of the - /// agent's own filename, so a path carrying a line break forged a line - /// ahead of everything the harness had to say about the change. - #[test] - fn a_changed_path_cannot_forge_a_line_in_the_receipt_headline() { - let forged = "and the file was approved"; - let changes = vec![Change { - path: std::path::PathBuf::from(format!("/p/.openmax/tools/a\n{forged}.toml")), - kind: "added", - actor: Actor::Initial, - }]; - let lines = describe(&changes, Path::new("/p")); - assert_eq!(lines.len(), 1, "{lines:?}"); - assert!(!lines[0].contains('\n'), "{:?}", lines[0]); - assert!(lines[0].starts_with(".openmax/tools/a "), "{:?}", lines[0]); - } - /// A same-second approve/retire/re-approve of identical bytes at the same /// path must give the two approvals DISTINCT event ids, or the per-session /// watermark drops the re-approval and the next turn is told the @@ -2005,38 +1674,42 @@ mod tests { dir } - fn entry(root: &Path, rel: &str, content: &str) -> (PathBuf, String, Vec) { - (root.join(rel), sha256_hex(content.as_bytes()), content.as_bytes().to_vec()) + /// Append change records the way earlier builds did on every refreeze, + /// so the chain, pin, and repair tests run against the logs those builds + /// left behind, and prove those logs still verify. A `None` content is a + /// removal. + fn legacy_changes(data: &Path, root: &Path, changes: &[(&str, Option<&str>)]) { + let dir = project_dir(data, root); + with_lock(&dir, || { + let verified = read_verified(&dir)?; + let records = changes + .iter() + .map(|(rel, content)| Record { + v: RECORD_VERSION, + ts: unix_now(), + path: root.join(rel), + sha256: content.map(|c| sha256_hex(c.as_bytes())), + actor: Actor::External, + session_id: None, + kind: Kind::Change, + also: Vec::new(), + event: None, + code: Vec::new(), + blocking: false, + prev: String::new(), + }) + .collect(); + let (lines, head) = chain(records, &verified.head)?; + append_chained(&dir, &lines, &head) + }) + .unwrap(); } - #[test] - fn first_sync_is_initial_then_changes_carry_the_caller_actor() { - let data = temp("data"); - let root = temp("proj"); - let files = vec![entry(&root, ".openmax/tools/a.toml", "v1")]; - let changes = sync(&data, &root, &files, Actor::Session, Some("s1")).unwrap(); - assert_eq!(changes.len(), 1); - assert_eq!(changes[0].actor, Actor::Initial, "an empty ledger seeds as initial"); - - // Unchanged content records nothing. - assert!(sync(&data, &root, &files, Actor::External, None).unwrap().is_empty()); - - let files = vec![entry(&root, ".openmax/tools/a.toml", "v2")]; - let changes = sync(&data, &root, &files, Actor::Session, Some("s1")).unwrap(); - assert_eq!(changes.len(), 1); - assert_eq!(changes[0].actor, Actor::Session); - assert_eq!(changes[0].kind, "modified"); - - // Removal is a record, not silence. - let changes = sync(&data, &root, &[], Actor::External, None).unwrap(); - assert_eq!(changes.len(), 1); - assert_eq!(changes[0].kind, "removed"); - - let records = history(&data, &root).unwrap(); - assert_eq!(records.len(), 3); - assert!(records[2].sha256.is_none()); - let _ = std::fs::remove_dir_all(&data); - let _ = std::fs::remove_dir_all(&root); + /// Three versions of one tool file, as an earlier build recorded them. + fn three_legacy_versions(data: &Path, root: &Path) { + for v in ["v1", "v2", "v3"] { + legacy_changes(data, root, &[(".openmax/tools/a.toml", Some(v))]); + } } /// ledger.lock speaks the same flock(2) protocol as the session locks: @@ -2051,12 +1724,11 @@ mod tests { std::fs::create_dir_all(&dir).unwrap(); let older = crate::sessions::raw_flock(&lock_path(&dir)).expect("ledger.lock is idle"); let (done_tx, done_rx) = std::sync::mpsc::channel(); - let syncer = { + let approver = { let (data, root, done_tx) = (data.clone(), root.clone(), done_tx.clone()); std::thread::spawn(move || { - let files = vec![entry(&root, ".openmax/tools/a.toml", "v1")]; - sync(&data, &root, &files, Actor::External, None).unwrap(); - done_tx.send("sync").unwrap(); + approve_hash(&data, &root, &sha256_hex(b"v1")).unwrap(); + done_tx.send("approve").unwrap(); }) }; let usage = { @@ -2070,7 +1742,7 @@ mod tests { let early = done_rx.recv_timeout(std::time::Duration::from_millis(300)); assert!(early.is_err(), "{early:?} ran while an older binary held ledger.lock"); drop(older); - syncer.join().unwrap(); + approver.join().unwrap(); usage.join().unwrap(); assert_eq!(history(&data, &root).unwrap().len(), 1); assert_eq!(load_usage(&data, &root).unwrap().total_calls, 1); @@ -2078,20 +1750,6 @@ mod tests { let _ = std::fs::remove_dir_all(&root); } - #[test] - fn objects_hold_the_exact_bytes_for_rollback() { - let data = temp("obj-data"); - let root = temp("obj-proj"); - let files = vec![entry(&root, ".openmax/tools/a.toml", "name = \"a\"")]; - sync(&data, &root, &files, Actor::External, None).unwrap(); - let records = history(&data, &root).unwrap(); - let sha = records[0].sha256.clone().unwrap(); - let object = project_dir(&data, &root).join("objects").join(&sha); - assert_eq!(std::fs::read_to_string(object).unwrap(), "name = \"a\""); - let _ = std::fs::remove_dir_all(&data); - let _ = std::fs::remove_dir_all(&root); - } - /// The approvals cache key is the log's CONTENT, not its length: a /// same-length in-place rewrite of a pinned record keeps the pin file /// and the byte count intact while breaking the chain, and the old @@ -2227,12 +1885,11 @@ mod tests { assert_eq!(repair_generation(&dir), 2, "every variant counts toward the generation"); } - /// An approval stores what it blessed: the manifest AND every bound - /// file, so `cp objects/ ` restores exactly what a human - /// approved. Before, approvals stored nothing (only freezes did, and a - /// freeze never reads a bound script), while --ledger promised restore. + /// An approval records hashes, not bytes: capability-file history is no + /// longer kept, so nothing would ever read a copy, and the project's own + /// files are where the approved bytes live. #[test] - fn an_approval_stores_the_manifest_and_bound_code_as_objects() { + fn an_approval_copies_no_bytes_into_the_ledger() { let data = temp("appr-obj-data"); let root = temp("appr-obj-proj"); std::fs::create_dir_all(root.join(".openmax/tools")).unwrap(); @@ -2243,18 +1900,11 @@ mod tests { let manifest_sha = sha256_hex(&std::fs::read(&manifest).unwrap()); let script_sha = sha256_hex(&std::fs::read(&script).unwrap()); approve_capability(&data, &root, &manifest, &[manifest_sha.clone(), script_sha.clone()]).unwrap(); - let objects = project_dir(&data, &root).join("objects"); - assert_eq!( - std::fs::read(objects.join(&manifest_sha)).unwrap(), - std::fs::read(&manifest).unwrap(), - "the approved manifest bytes are restorable" - ); - assert_eq!( - std::fs::read(objects.join(&script_sha)).unwrap(), - std::fs::read(&script).unwrap(), - "the approved bound script bytes are restorable" + assert!(is_approved(&data, &root, &manifest_sha) && is_approved(&data, &root, &script_sha)); + assert!( + !project_dir(&data, &root).join("objects").exists(), + "an approval stores no copy of what it blessed" ); - assert!(matches!(object_state(&data, &root, &script_sha), ObjectState::Intact)); let _ = std::fs::remove_dir_all(&data); let _ = std::fs::remove_dir_all(&root); } @@ -2262,7 +1912,7 @@ mod tests { /// A missing interpreter-script argument (`/bin/sh run.sh`, run.sh gone) /// binds to a None entry, so the empty binding cannot read as covered. /// Otherwise deleting it left an empty `bound_code`, which `covers_code` - /// accepts, and the removed tool was wrongly called cardless-restorable. + /// accepts, so the removed tool ran ungated. #[test] fn a_missing_interpreter_script_arg_binds_to_none() { let root = temp("interp-proj"); @@ -2295,8 +1945,8 @@ mod tests { /// If a bound script changes after the card hashed it but before `approve` /// runs, the approval is REJECTED: recording the vouched sha in `also` - /// while its object cannot be stored would leave an approved hash with no - /// restorable bytes. Nothing is recorded. + /// would put a hash on record for bytes that no longer exist, beside a + /// manifest the human believes is now covered. Nothing is recorded. #[test] fn approve_rejects_a_bound_file_changed_after_the_card() { let data = temp("changed-data"); @@ -2313,7 +1963,7 @@ mod tests { std::fs::write(&script, "echo B\n").unwrap(); let err = approve_capability(&data, &root, &manifest, &[manifest_sha, script_sha]) .expect_err("a changed bound file must be rejected"); - assert!(err.contains("changed") && err.contains("not restorable"), "{err}"); + assert!(err.contains("changed") && err.contains("nothing was approved"), "{err}"); assert!( history(&data, &root).unwrap().is_empty(), "a rejected approval appends no record" @@ -2322,12 +1972,11 @@ mod tests { let _ = std::fs::remove_dir_all(&root); } - /// The object check verifies CONTENT, not just existence: a changed bound - /// script whose sha slot in `objects/` was pre-populated with unrelated - /// bytes must still be rejected, or a restore would produce bytes the - /// reviewer never approved. + /// The bytes on disk decide, never a stored copy: an object an earlier + /// build kept for the vouched hash does not stand in for a bound script + /// that changed since the card. #[test] - fn approve_rejects_a_bound_file_whose_object_slot_is_corrupt() { + fn a_stored_object_does_not_stand_in_for_a_changed_bound_file() { let data = temp("corrupt-data"); let root = temp("corrupt-proj"); std::fs::create_dir_all(root.join(".openmax/tools")).unwrap(); @@ -2337,46 +1986,19 @@ mod tests { std::fs::write(&script, "echo A\n").unwrap(); let manifest_sha = sha256_hex(&std::fs::read(&manifest).unwrap()); let script_sha = sha256_hex(&std::fs::read(&script).unwrap()); - // An unrelated object is planted at objects/. + // The authentic old bytes, as an earlier build stored them. let objects = project_dir(&data, &root).join("objects"); std::fs::create_dir_all(&objects).unwrap(); - std::fs::write(objects.join(&script_sha), b"unrelated corrupt bytes").unwrap(); - // The script changes, so the store loop will not overwrite the slot. + std::fs::write(objects.join(&script_sha), b"echo A\n").unwrap(); std::fs::write(&script, "echo B\n").unwrap(); - let err = approve_capability(&data, &root, &manifest, &[manifest_sha, script_sha]) - .expect_err("a corrupt object slot must not pass as restorable"); - assert!(err.contains("not restorable"), "{err}"); + let err = approve_capability(&data, &root, &manifest, &[manifest_sha, script_sha.clone()]) + .expect_err("a stored copy must not vouch for changed bytes"); + assert!(err.contains("nothing was approved"), "{err}"); assert!(history(&data, &root).unwrap().is_empty()); - let _ = std::fs::remove_dir_all(&data); - let _ = std::fs::remove_dir_all(&root); - } - - /// A hash-only approval stores no manifest object, so --ledger must read - /// its manifest as not restorable; a full (path-form) approval stores the - /// object and reads as restorable. - #[test] - fn a_hash_only_approval_reads_as_manifest_not_restorable() { - let data = temp("mrestore-data"); - let root = temp("mestore-proj"); - let sha = sha256_hex(b"name = \"t\"\n"); - approve_hash(&data, &root, &sha).unwrap(); - let hash_only = history(&data, &root).unwrap(); - assert!( - approval_manifest_missing(&data, &root, &hash_only[0]), - "a hash-only approval stored no manifest object to restore from" - ); - - // A full approval stores the manifest object. - std::fs::create_dir_all(root.join(".openmax/tools")).unwrap(); - let manifest = root.join(".openmax/tools/t.toml"); - std::fs::write(&manifest, "name = \"full\"\ndescription = \"d\"\ncommand = \"/bin/echo\"\n").unwrap(); - let full_sha = sha256_hex(&std::fs::read(&manifest).unwrap()); - approve_capability(&data, &root, &manifest, std::slice::from_ref(&full_sha)).unwrap(); - let recs = history(&data, &root).unwrap(); - let full = recs.iter().find(|r| r.sha256.as_deref() == Some(full_sha.as_str())).unwrap(); - assert!( - !approval_manifest_missing(&data, &root, full), - "a full approval stored the manifest bytes, so it is restorable" + assert_eq!( + std::fs::read(objects.join(&script_sha)).unwrap(), + b"echo A\n", + "a stored object is never deleted or rewritten" ); let _ = std::fs::remove_dir_all(&data); let _ = std::fs::remove_dir_all(&root); @@ -2386,10 +2008,7 @@ mod tests { fn the_chain_links_every_record_and_detects_tampering() { let data = temp("chain-data"); let root = temp("chain-proj"); - for v in ["v1", "v2", "v3"] { - let files = vec![entry(&root, ".openmax/tools/a.toml", v)]; - sync(&data, &root, &files, Actor::External, None).unwrap(); - } + three_legacy_versions(&data, &root); let log = log_path(&project_dir(&data, &root)); let text = std::fs::read_to_string(&log).unwrap(); assert_eq!(verify_chain(&text), Ok(3)); @@ -2408,10 +2027,7 @@ mod tests { fn removing_trailing_records_is_detected() { let data = temp("trunc-data"); let root = temp("trunc-proj"); - for v in ["v1", "v2", "v3"] { - let files = vec![entry(&root, ".openmax/tools/a.toml", v)]; - sync(&data, &root, &files, Actor::External, None).unwrap(); - } + three_legacy_versions(&data, &root); let log = log_path(&project_dir(&data, &root)); let text = std::fs::read_to_string(&log).unwrap(); let prefix: String = text.lines().take(2).map(|l| format!("{l}\n")).collect(); @@ -2422,20 +2038,6 @@ mod tests { let _ = std::fs::remove_dir_all(&root); } - #[test] - fn a_corrupted_object_is_replaced_with_authentic_bytes() { - let data = temp("objfix-data"); - let root = temp("objfix-proj"); - let (path, sha, bytes) = entry(&root, ".openmax/tools/a.toml", "authentic"); - let object = project_dir(&data, &root).join("objects").join(&sha); - std::fs::create_dir_all(object.parent().unwrap()).unwrap(); - std::fs::write(&object, "forged").unwrap(); - sync(&data, &root, &[(path, sha.clone(), bytes)], Actor::External, None).unwrap(); - assert_eq!(std::fs::read_to_string(&object).unwrap(), "authentic"); - let _ = std::fs::remove_dir_all(&data); - let _ = std::fs::remove_dir_all(&root); - } - #[test] fn usage_merges_across_turns() { let data = temp("use-data"); @@ -2606,8 +2208,7 @@ mod tests { let manifest = root.join("gate.toml"); let body = "event = \"pre_tool_use\"\ncommand = \"./gate.sh\"\n"; std::fs::write(&manifest, body).unwrap(); - // gate.sh's bytes are b"script", so its sha is the second vouched hash - // and its object is stored - the approval records only restorable bytes. + // gate.sh's bytes are b"script", so its sha is the second vouched hash. std::fs::write(root.join("gate.sh"), "script").unwrap(); let shas = vec![sha256_hex(body.as_bytes()), sha256_hex(b"script")]; approve_capability(&data, &root, &manifest, &shas).unwrap(); @@ -2664,136 +2265,6 @@ mod tests { let _ = std::fs::remove_dir_all(&root); } - /// An approved external tool records its bound script's hash in `also` - /// but no path list - `code` is filled from the hook shape only - so a - /// footer pairing `also` with `code` offered nothing: the one intact - /// object an operator needs after deleting the script had no cp line. - /// The path comes back from the stored manifest object, the - /// approved bytes, never the live file, which here is already gone. - #[test] - fn an_external_tools_bound_script_gets_a_restore_target() { - let data = temp("tool-restore-data"); - let root = temp("tool-restore-proj"); - let manifest = root.join("deploy.toml"); - let body = "name = \"deploy\"\ndescription = \"d\"\ncommand = \"./deploy.sh\"\n"; - std::fs::write(&manifest, body).unwrap(); - std::fs::write(root.join("deploy.sh"), "script").unwrap(); - let bound = manifest_code(&manifest, &root); - assert_eq!(bound.len(), 1, "the fixture binds exactly the script"); - let script_path = bound[0].path.clone(); - let shas = vec![sha256_hex(body.as_bytes()), sha256_hex(b"script")]; - approve_capability(&data, &root, &manifest, &shas).unwrap(); - std::fs::remove_file(root.join("deploy.sh")).unwrap(); - - let records = history(&data, &root).unwrap(); - assert!(records[0].code.is_empty(), "a tool record carries no path list"); - assert_eq!( - approval_restore_targets(&data, &root, &records[0]), - vec![(sha256_hex(b"script"), script_path)], - "the intact script object is offered at the path the approved bytes name" - ); - let _ = std::fs::remove_dir_all(&data); - let _ = std::fs::remove_dir_all(&root); - } - - /// The record's sha authenticates the manifest bytes wherever they live: - /// with the stored object pruned but the manifest file still hashing to - /// the vouched sha, a tool's intact script object keeps its restore line, - /// while an EDITED manifest file authenticates nothing. - #[test] - fn a_tools_restore_survives_a_pruned_manifest_object_via_the_authentic_file() { - let data = temp("tool-prune-data"); - let root = temp("tool-prune-proj"); - let manifest = root.join("deploy.toml"); - let body = "name = \"deploy\"\ndescription = \"d\"\ncommand = \"./deploy.sh\"\n"; - std::fs::write(&manifest, body).unwrap(); - std::fs::write(root.join("deploy.sh"), "script").unwrap(); - let script_path = manifest_code(&manifest, &root)[0].path.clone(); - let shas = vec![sha256_hex(body.as_bytes()), sha256_hex(b"script")]; - approve_capability(&data, &root, &manifest, &shas).unwrap(); - std::fs::remove_file(root.join("deploy.sh")).unwrap(); - std::fs::remove_file(project_dir(&data, &root).join("objects").join(&shas[0])).unwrap(); - - let records = history(&data, &root).unwrap(); - assert_eq!( - approval_restore_targets(&data, &root, &records[0]), - vec![(sha256_hex(b"script"), script_path)], - "the on-disk manifest still hashes to the vouched sha, so it names the path" - ); - - std::fs::write(&manifest, "name = \"deploy\"\ndescription = \"d\"\ncommand = \"./other.sh\"\n") - .unwrap(); - assert_eq!( - approval_restore_targets(&data, &root, &records[0]), - Vec::<(String, PathBuf)>::new(), - "an edited manifest authenticates nothing and offers nothing" - ); - let _ = std::fs::remove_dir_all(&data); - let _ = std::fs::remove_dir_all(&root); - } - - /// A bound file the card could not read never enters `also`, so its - /// record's hash and path lists disagree in length. Positional pairing - /// would then print a cp that writes one file's approved bytes over - /// another file's path - the helper refuses instead. - #[test] - fn an_unequal_hash_and_path_list_is_refused_not_guess_paired() { - let data = temp("gap-data"); - let root = temp("gap-proj"); - let manifest = root.join("gate.toml"); - let body = "event = \"pre_tool_use\"\ncommand = \"./wrap.sh\"\nargs = [\"helper.py\"]\n"; - std::fs::write(&manifest, body).unwrap(); - std::fs::write(root.join("wrap.sh"), "wrap").unwrap(); - std::fs::write(root.join("helper.py"), "helper").unwrap(); - // The card skips a file it cannot read; only helper.py's hash rides. - let shas = vec![sha256_hex(body.as_bytes()), sha256_hex(b"helper")]; - approve_capability(&data, &root, &manifest, &shas).unwrap(); - - let records = history(&data, &root).unwrap(); - assert_eq!(records[0].code.len(), 2, "the approved bytes name two files"); - assert_eq!(records[0].also.len(), 1, "only one hash was vouched"); - assert_eq!( - approval_restore_targets(&data, &root, &records[0]), - Vec::<(String, PathBuf)>::new(), - "an ambiguous pairing offers nothing rather than the wrong path" - ); - let _ = std::fs::remove_dir_all(&data); - let _ = std::fs::remove_dir_all(&root); - } - - /// A hook record carries its named paths, so its bound-code restore - /// survives a manifest object that is itself missing - the record is the - /// pairing, no parse needed - while a bound object that is not intact is - /// never offered. - #[test] - fn a_hooks_recorded_paths_pair_without_the_manifest_object() { - let data = temp("hook-restore-data"); - let root = temp("hook-restore-proj"); - let manifest = root.join("gate.toml"); - let body = "event = \"pre_tool_use\"\ncommand = \"./gate.sh\"\n"; - std::fs::write(&manifest, body).unwrap(); - std::fs::write(root.join("gate.sh"), "script").unwrap(); - let shas = vec![sha256_hex(body.as_bytes()), sha256_hex(b"script")]; - approve_capability(&data, &root, &manifest, &shas).unwrap(); - let records = history(&data, &root).unwrap(); - let objects = project_dir(&data, &root).join("objects"); - std::fs::remove_file(objects.join(&shas[0])).unwrap(); - - assert_eq!( - approval_restore_targets(&data, &root, &records[0]), - vec![(sha256_hex(b"script"), PathBuf::from(records[0].code[0].clone()))], - "the record's own path list pairs without the manifest object" - ); - std::fs::remove_file(objects.join(&shas[1])).unwrap(); - assert_eq!( - approval_restore_targets(&data, &root, &records[0]), - Vec::<(String, PathBuf)>::new(), - "a bound object that is not intact is never offered" - ); - let _ = std::fs::remove_dir_all(&data); - let _ = std::fs::remove_dir_all(&root); - } - fn first_line(data: &Path, root: &Path) -> String { let text = std::fs::read_to_string(log_path(&project_dir(data, root))).unwrap(); text.lines().next().unwrap().to_string() @@ -2859,8 +2330,7 @@ mod tests { fn a_forged_approvals_file_approves_nothing() { let data = temp("forge-data"); let root = temp("forge-proj"); - let files = vec![entry(&root, ".openmax/tools/a.toml", "v1")]; - sync(&data, &root, &files, Actor::External, None).unwrap(); + legacy_changes(&data, &root, &[(".openmax/tools/a.toml", Some("v1"))]); let payload = sha256_hex(b"event = \"session_start\"\ncommand = \"/bin/sh\"\n"); let hook = root.join(".openmax/hooks/payload.toml"); @@ -3094,16 +2564,13 @@ mod tests { for (name, tamper) in cases { let data = temp("tamper-data"); let root = temp("tamper-proj"); - for v in ["v1", "v2", "v3"] { - let files = vec![entry(&root, ".openmax/tools/a.toml", v)]; - sync(&data, &root, &files, Actor::External, None).unwrap(); - } + three_legacy_versions(&data, &root); let dir = project_dir(&data, &root); tamper(&dir); let err = history(&data, &root).unwrap_err(); assert!(err.contains("--ledger-repair"), "{name}: no repair path in: {err}"); assert!( - sync(&data, &root, &[], Actor::External, None).is_err(), + approve_hash(&data, &root, &sha256_hex(b"v4")).is_err(), "{name}: an unverifiable ledger must not append" ); let _ = std::fs::remove_dir_all(&data); @@ -3133,8 +2600,7 @@ mod tests { fn an_interrupted_append_reads_as_recoverable_and_heals() { let data = temp("crash-data"); let root = temp("crash-proj"); - let files = vec![entry(&root, ".openmax/tools/a.toml", "v1")]; - sync(&data, &root, &files, Actor::External, None).unwrap(); + legacy_changes(&data, &root, &[(".openmax/tools/a.toml", Some("v1"))]); let dir = project_dir(&data, &root); // Exactly what a SIGKILL after the log write leaves behind: the @@ -3162,16 +2628,17 @@ mod tests { drop(log); std::fs::write(pending_head_path(&dir), &new_head).unwrap(); - let state = read(&data, &root).unwrap(); + let state = read_verified(&dir).unwrap(); assert_eq!(state.records.len(), 2, "nothing was removed, so nothing is hidden"); - assert!(state.interrupted_write, "the pin is behind the log, not the log behind the pin"); - - // The next sync re-pins, even with nothing new to record. - sync(&data, &root, &[entry(&root, ".openmax/tools/a.toml", "v2")], Actor::External, None) - .unwrap(); - assert_eq!(std::fs::read_to_string(chain_head_path(&dir)).unwrap(), new_head); + assert_eq!(state.pin, Pin::Interrupted, "the pin is behind the log, not the log behind the pin"); + assert_eq!(state.head, new_head); + + // The next append chains onto the tail and re-pins past it. + approve_hash(&data, &root, &sha256_hex(b"v2")).unwrap(); + let state = read_verified(&dir).unwrap(); + assert_eq!(state.records.len(), 3); + assert_eq!(state.pin, Pin::Matches); assert!(!pending_head_path(&dir).exists()); - assert!(!read(&data, &root).unwrap().interrupted_write); // The same tail without a pending pin is a forged append, not a // crash: tolerating one must not launder the other. @@ -3196,10 +2663,12 @@ mod tests { fn repair_quarantines_the_damage_and_restores_writes() { let data = temp("repair-data"); let root = temp("repair-proj"); - sync(&data, &root, &[entry(&root, ".openmax/tools/a.toml", "v1")], Actor::External, None) - .unwrap(); + legacy_changes(&data, &root, &[(".openmax/tools/a.toml", Some("v1"))]); approve_hash(&data, &root, &sha256_hex(b"v1")).unwrap(); let dir = project_dir(&data, &root); + // Objects an earlier build stored. + std::fs::create_dir_all(dir.join("objects")).unwrap(); + std::fs::write(dir.join("objects").join(sha256_hex(b"v1")), "v1").unwrap(); truncate_log(&dir, 1); let outcome = repair(&data, &root).unwrap(); @@ -3208,16 +2677,15 @@ mod tests { assert_eq!(outcome.records, 1); assert!(!log_path(&dir).exists()); assert!(!chain_head_path(&dir).exists()); - assert!(dir.join("objects").is_dir(), "rollback bytes survive a repair"); + assert!( + dir.join("objects").join(sha256_hex(b"v1")).is_file(), + "stored objects survive a repair" + ); // Writes work again, and the quarantined approvals really are gone. assert!(!is_approved(&data, &root, &sha256_hex(b"v1"))); - let changes = - sync(&data, &root, &[entry(&root, ".openmax/tools/a.toml", "v1")], Actor::Session, None) - .unwrap(); - assert_eq!(changes.len(), 1); - assert_eq!(changes[0].actor, Actor::Initial, "a repaired ledger starts a new baseline"); - assert!(history(&data, &root).is_ok()); + approve_hash(&data, &root, &sha256_hex(b"v2")).unwrap(); + assert_eq!(history(&data, &root).unwrap().len(), 1, "a repaired ledger starts a new chain"); let _ = std::fs::remove_dir_all(&data); let _ = std::fs::remove_dir_all(&root); } @@ -3258,28 +2726,10 @@ mod tests { } #[test] - fn objects_are_verified_on_read_and_timestamps_are_readable() { - let data = temp("objstate-data"); - let root = temp("objstate-proj"); - let (path, sha, bytes) = entry(&root, ".openmax/tools/a.toml", "authentic"); - sync(&data, &root, &[(path, sha.clone(), bytes)], Actor::External, None).unwrap(); - assert_eq!(object_state(&data, &root, &sha), ObjectState::Intact); - - let object = project_dir(&data, &root).join("objects").join(&sha); - std::fs::write(&object, "### backdoor ###").unwrap(); - assert_eq!( - object_state(&data, &root, &sha), - ObjectState::Corrupt, - "an object that does not hash to its name must never be restored" - ); - std::fs::remove_file(&object).unwrap(); - assert_eq!(object_state(&data, &root, &sha), ObjectState::Missing); - + fn timestamps_are_readable() { assert_eq!(format_ts(0), "1970-01-01 00:00:00Z"); assert_eq!(format_ts(1_785_471_295), "2026-07-31 04:14:55Z"); assert_eq!(format_ts(951_782_400), "2000-02-29 00:00:00Z", "leap day"); - let _ = std::fs::remove_dir_all(&data); - let _ = std::fs::remove_dir_all(&root); } #[test] @@ -3290,7 +2740,7 @@ mod tests { std::fs::create_dir_all(&dir).unwrap(); std::fs::write(log_path(&dir), "not json\n").unwrap(); assert!(history(&data, &root).is_err()); - assert!(sync(&data, &root, &[], Actor::External, None).is_err()); + assert!(approve_hash(&data, &root, &sha256_hex(b"v1")).is_err()); let _ = std::fs::remove_dir_all(&data); let _ = std::fs::remove_dir_all(&root); } @@ -3320,8 +2770,7 @@ mod tests { fn a_pending_tail_grants_no_authority() { let data = temp("tail-data"); let root = temp("tail-proj"); - sync(&data, &root, &[entry(&root, ".openmax/tools/a.toml", "v1")], Actor::External, None) - .unwrap(); + legacy_changes(&data, &root, &[(".openmax/tools/a.toml", Some("v1"))]); approve_hash(&data, &root, &sha256_hex(b"blessed")).unwrap(); let dir = project_dir(&data, &root); @@ -3346,9 +2795,6 @@ mod tests { assert!(!approved.contains(&sha256_hex(b"evil")), "an unpinned approval grants nothing"); // Refused on write: nothing may move the pin past the forgery. - let next = [entry(&root, ".openmax/tools/a.toml", "v2")]; - let err = sync(&data, &root, &next, Actor::External, None).unwrap_err(); - assert!(err.contains("approval-grade"), "{err}"); let err = approve_hash(&data, &root, &sha256_hex(b"more")).unwrap_err(); assert!(err.contains("approval-grade"), "{err}"); @@ -3361,20 +2807,19 @@ mod tests { assert!(approved.contains(&sha256_hex(b"blessed")), "pinned history survives repair"); assert!(!approved.contains(&sha256_hex(b"evil"))); // And the ledger appends again. - sync(&data, &root, &next, Actor::External, None).unwrap(); + approve_hash(&data, &root, &sha256_hex(b"more")).unwrap(); let _ = std::fs::remove_dir_all(&data); let _ = std::fs::remove_dir_all(&root); } - /// A change-only pending tail is a crashed sync, and crash recovery must - /// keep working exactly as before: reads see the records, the next sync - /// re-pins, nothing needs a human. + /// A change-only pending tail is an earlier build's crashed append, and + /// crash recovery must keep working: reads see the records, the next + /// append re-pins past them, nothing needs a human. #[test] fn a_change_only_pending_tail_still_heals_itself() { let data = temp("healtail-data"); let root = temp("healtail-proj"); - sync(&data, &root, &[entry(&root, ".openmax/tools/a.toml", "v1")], Actor::External, None) - .unwrap(); + legacy_changes(&data, &root, &[(".openmax/tools/a.toml", Some("v1"))]); let dir = project_dir(&data, &root); let head = plant_pending_tail(&dir, Record { v: RECORD_VERSION, @@ -3390,11 +2835,13 @@ mod tests { blocking: false, prev: String::new(), }); - assert!(read(&data, &root).unwrap().interrupted_write); - sync(&data, &root, &[entry(&root, ".openmax/tools/a.toml", "v2")], Actor::External, None) - .unwrap(); - assert_eq!(std::fs::read_to_string(chain_head_path(&dir)).unwrap(), head); - assert!(!read(&data, &root).unwrap().interrupted_write); + let state = read_verified(&dir).unwrap(); + assert_eq!((state.pin, state.head.as_str()), (Pin::Interrupted, head.as_str())); + assert!(approved_hashes(&data, &root).unwrap().is_empty(), "observations grant nothing"); + approve_hash(&data, &root, &sha256_hex(b"v2")).unwrap(); + let state = read_verified(&dir).unwrap(); + assert_eq!(state.pin, Pin::Matches); + assert_eq!(state.records.len(), 3, "the healed tail stays in the chain"); let _ = std::fs::remove_dir_all(&data); let _ = std::fs::remove_dir_all(&root); } @@ -3407,8 +2854,7 @@ mod tests { fn a_rewritten_log_with_a_pending_receipt_reads_as_tamper() { let data = temp("rewrite-data"); let root = temp("rewrite-proj"); - sync(&data, &root, &[entry(&root, ".openmax/tools/a.toml", "v1")], Actor::External, None) - .unwrap(); + legacy_changes(&data, &root, &[(".openmax/tools/a.toml", Some("v1"))]); let dir = project_dir(&data, &root); let record = Record { diff --git a/crates/core/src/prompt.rs b/crates/core/src/prompt.rs index 366b1518..1d209860 100644 --- a/crates/core/src/prompt.rs +++ b/crates/core/src/prompt.rs @@ -200,7 +200,7 @@ pub fn system_prompt_with_breakdown(project_root: &Path, registry: &Registry) -> /// resent on every request for the life of the session. const SELF_EXTENSION: &str = "\n\nWhen the user asks for a reusable capability (tool, skill, prompt template, hook, permission rule, provider, or memory), read its contract first with bash: openmax --spec . It gives the file path, format, approval, and activation rules; verify with openmax --check.\n\ Surfaces: tools|skills|prompts|hooks|permissions|providers|memory|stdio|mcp.\n\ -Past sessions and memories: openmax --recall \"\"; capability-file history and restore: openmax --ledger. Isolated or parallel work: a child openmax -p or openmax --stdio process, in tmux when it must outlive the turn."; +Past sessions and memories: openmax --recall \"\". Isolated or parallel work: a child openmax -p or openmax --stdio process, in tmux when it must outlive the turn."; /// One line per skill: name, description, and the SKILL.md path the model /// reads on demand. Project skills show a project-relative path (read_file @@ -414,10 +414,9 @@ mod tests { assert!(prompt.contains("activation rules")); assert!(prompt.contains("openmax --check")); assert!(prompt.contains("openmax --recall"), "preserved history must be findable"); - // The ledger is the only route to a capability file's approved - // versions and their restoration commands; neither --spec nor - // --recall exposes that, so the pointer has to. - assert!(prompt.contains("openmax --ledger"), "capability history must be findable"); + // Capability-file history is no longer recorded, so a pointer to it + // would be bytes every request pays for a command with nothing to show. + assert!(!prompt.contains("openmax --ledger"), "no pointer to history that is not kept"); assert!(prompt.contains("openmax -p or openmax --stdio")); assert!(prompt.contains("tmux")); // The per-surface paths moved into the contracts the pointer names. @@ -765,10 +764,9 @@ mod tests { } /// The path-free base rules, extension pointer, and builtin schemas must - /// fit in 3,500 bytes (the payload measured 3,412 bytes, 770 tokens on a - /// current tokenizer, when the cap was set, and 3,416 bytes, 773 tokens, - /// once the pointer named `mcp`). Grounding sections have - /// separate caps. Measure the payload with + /// fit in 3,450 bytes (the payload measured 3,357 bytes when the cap was + /// set, and 3,361 bytes once the pointer named `mcp`). Grounding sections + /// have separate caps. Measure the payload with /// dump_frozen_prompt_payload_for_tokenizer and a real tokenizer before /// changing this budget; provider framing is not included. #[test] @@ -796,7 +794,7 @@ mod tests { .collect(); let tool_chars = serde_json::to_string(&builtins).expect("serialize").len(); let total = path_free + tool_chars; - const CAP: usize = 3_500; + const CAP: usize = 3_450; assert!( total <= CAP, "frozen prompt budget exceeded by {} bytes: base rules + guide (path-free) \ diff --git a/crates/core/src/recall.rs b/crates/core/src/recall.rs index 19845be2..4952b0ab 100644 --- a/crates/core/src/recall.rs +++ b/crates/core/src/recall.rs @@ -7,7 +7,7 @@ //! "); the memory PR made facts durable. Recall is the searchable form //! of the same commitment: what the harness preserved must be findable //! without hand-grepping home-dir JSONL over bash. It is a read-only -//! standalone operation like `--ledger`: no session, no endpoint, no daemon, +//! standalone operation like `--check`: no session, no endpoint, no daemon, //! and no derived index - the stores on disk stay the single source of truth //! and every scan reads them directly. At harness scale (megabytes, not the //! hundreds of millions of rows a database engine plans for) a bounded diff --git a/crates/core/src/registry.rs b/crates/core/src/registry.rs index 1a40cc55..cc345ce9 100644 --- a/crates/core/src/registry.rs +++ b/crates/core/src/registry.rs @@ -103,17 +103,19 @@ pub struct Registry { /// reason. Receipts and the unknown-tool error name these so a broken /// write is never mistaken for a live capability. pub broken: Vec<(PathBuf, String)>, - /// Every capability file path THIS freeze's capture actually read (tool - /// manifests and SKILL.mds). The refreeze classifier asks whether a path - /// still existed in this generation without a second disk probe, which - /// would race the capture it claims to describe. Empty for a - /// manifest-restored registry, which only ever sits on the outgoing side - /// of that comparison. - pub(crate) read_paths: std::collections::HashSet, + /// Every capability file THIS freeze's capture actually read (tool + /// manifests and SKILL.mds), with a hash of the bytes it read. The + /// refreeze receipt compares two generations file by file from these + /// without a second disk probe, which would race the capture it claims + /// to describe. The session manifest carries them, so a resumed registry + /// compares the same way. None for built-ins only or a manifest written + /// before they were kept, which only ever sits on the outgoing side of + /// that comparison. + pub(crate) read_paths: Option>, /// Broken TOOL manifests with the name each occupies: the declared name, /// or the file stem when the document is too broken to yield one - the - /// same derivation the withhold pass uses. Lets the refreeze classifier - /// see that a broken file already explains an absent name. Empty for a + /// same derivation the withhold pass uses. Lets the unknown-tool error + /// name the broken file that occupies a called name. Empty for a /// manifest-restored registry. pub(crate) broken_tools: Vec<(PathBuf, String)>, /// Same-directory skill name collisions, one record per name (name, @@ -166,16 +168,16 @@ pub(crate) struct ExtensionSnapshot { tools_omitted: usize, /// Skills discovered but dropped by the `MAX_SKILLS` index cap. skills_omitted: usize, - /// Every capability file this generation read: (path, sha256, bytes). - /// The ledger records exactly this generation, so what it attests is what - /// the freeze actually used - never a second read that could differ. - pub(crate) files: Vec<(PathBuf, String, Vec)>, + /// Every capability file this generation read, with a hash of its bytes + /// taken where they were read: the receipt names a changed file from + /// this, so nothing downstream needs a copy of the bytes. + read_paths: Vec<(PathBuf, u64)>, /// Files read but not loaded, with the reason. The bytes are already in /// the fingerprint (a broken write still triggers a refreeze); keeping /// the reason lets that refreeze's receipt say the tool is NOT live. pub(crate) broken: Vec<(PathBuf, String)>, /// The tool-tier subset of `broken` with the name each file occupies - /// (declared, or stem as the fallback), for the refreeze classifier. + /// (declared, or stem as the fallback), for the unknown-tool error. pub(crate) broken_tools: Vec<(PathBuf, String)>, /// Same-directory skill name collisions, one record per name: (name, /// displaced paths, winning path, winner indexed). The receipt names @@ -187,8 +189,7 @@ pub(crate) struct ExtensionSnapshot { /// next refreeze or /reload. It is read at activation, not here: most /// captures match the frozen fingerprint and are discarded, and a memory /// scan in each would read every memory file and the whole access log - /// for nothing. Not ledger files (data, not capability), so not in - /// `files`. + /// for nothing. Data, not capability, so not in `read_paths`. project_root: PathBuf, } @@ -206,7 +207,15 @@ pub(crate) fn capture_extensions(data_dir: &Path, project_root: &Path) -> Extens use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; let mut h = DefaultHasher::new(); - let mut files_read: Vec<(PathBuf, String, Vec)> = Vec::new(); + let mut read_paths: Vec<(PathBuf, u64)> = Vec::new(); + // One file's content identity, for the refreeze receipt. The fingerprint + // still hashes the bytes itself, so the value persisted sessions recorded + // does not move. + let identity = |bytes: &[u8]| { + let mut file = DefaultHasher::new(); + bytes.hash(&mut file); + file.finish() + }; // (path, reason, dir precedence index, declared name if recoverable): // tools are keyed by DECLARED name, not file stem, so a collision between // a broken file and a loaded definition must be judged on the name the @@ -246,7 +255,7 @@ pub(crate) fn capture_extensions(data_dir: &Path, project_root: &Path) -> Extens }; bytes.hash(&mut h); let Some(bytes) = bytes else { continue }; - files_read.push((path.clone(), crate::ledger::sha256_hex(&bytes), bytes.clone())); + read_paths.push((path.clone(), identity(&bytes))); let Ok(text) = std::str::from_utf8(&bytes) else { broken_at.push((path, "not valid UTF-8".into(), dir_index, None)); continue; @@ -271,8 +280,8 @@ pub(crate) fn capture_extensions(data_dir: &Path, project_root: &Path) -> Extens // under a valid project override: the override is legitimately active, // and the reason says so instead of claiming the name is not callable. let mut broken: Vec<(PathBuf, String)> = Vec::new(); - // (path, occupied name) per broken tool file: the refreeze classifier - // tells "removed" from "explained by a broken file" with this, never by + // (path, occupied name) per broken tool file: the unknown-tool error + // names the broken file behind a called name with this, never by // re-probing disk after the capture. let mut broken_tools: Vec<(PathBuf, String)> = Vec::new(); for (path, mut reason, broken_dir, declared) in broken_at { @@ -325,7 +334,7 @@ pub(crate) fn capture_extensions(data_dir: &Path, project_root: &Path) -> Extens let bytes = std::fs::read(&path).ok(); bytes.hash(&mut h); let Some(bytes) = bytes else { continue }; - files_read.push((path.clone(), crate::ledger::sha256_hex(&bytes), bytes.clone())); + read_paths.push((path.clone(), identity(&bytes))); let Ok(text) = std::str::from_utf8(&bytes) else { broken.push((path, "not valid UTF-8".into())); continue; @@ -371,7 +380,7 @@ pub(crate) fn capture_extensions(data_dir: &Path, project_root: &Path) -> Extens // read when the generation activates (`Registry::from_snapshot`). The // directory path is still hashed so a project without memories keeps the // fingerprint its persisted sessions recorded and resumes without a - // refreeze. Never ledgered (data, not capability). + // refreeze. project_root.join(crate::memory::MEMORY_DIR).hash(&mut h); let mut external: Vec = external_by_name.into_values().collect(); // Built-in shadows never load (assemble drops them); excluding them here @@ -409,7 +418,7 @@ pub(crate) fn capture_extensions(data_dir: &Path, project_root: &Path) -> Extens skills: discovered_skills, tools_omitted, skills_omitted, - files: files_read, + read_paths, broken, broken_tools, shadowed_skills, @@ -446,7 +455,7 @@ impl Registry { registry.ext_fingerprint = snapshot.fingerprint; registry.tools_omitted = snapshot.tools_omitted; registry.skills_omitted = snapshot.skills_omitted; - registry.read_paths = snapshot.files.iter().map(|(p, _, _)| p.clone()).collect(); + registry.read_paths = Some(snapshot.read_paths.into_iter().collect()); registry.broken = snapshot.broken; registry.broken_tools = snapshot.broken_tools; registry.shadowed_skills = snapshot.shadowed_skills; @@ -503,7 +512,7 @@ impl Registry { tools_omitted: 0, ext_fingerprint: 0, broken: Vec::new(), - read_paths: std::collections::HashSet::new(), + read_paths: None, broken_tools: Vec::new(), shadowed_skills: Vec::new(), memory_files: None, @@ -678,6 +687,12 @@ pub struct RegistryManifest { /// sections. #[serde(default)] pub memory_rows: Option>, + /// Every capability file the freeze read, with the hash of the bytes it + /// read, so the first refreeze of a resumed session names each file that + /// changed while it was closed, a SKILL.md body edit included. Additive: + /// absent in older manifests, whose first receipt compares index lines. + #[serde(default)] + pub read_files: Option>, } /// Current manifest format. A manifest carrying any other version is treated @@ -753,6 +768,12 @@ impl Registry { // re-suspending must keep the accounting its persisted prompt // still depends on. memory_rows: self.frozen_memory_rows.clone(), + read_files: self.read_paths.as_ref().map(|read| { + let mut files: Vec<(PathBuf, u64)> = + read.iter().map(|(path, hash)| (path.clone(), *hash)).collect(); + files.sort(); + files + }), version: MANIFEST_VERSION, external_tools, skills: self.skills.clone(), @@ -796,6 +817,7 @@ impl Registry { // the persisted prompt, which is precisely what the manifest carries. registry.memory_files = manifest.memory_files.clone(); registry.frozen_memory_rows = manifest.memory_rows.clone(); + registry.read_paths = manifest.read_files.map(|files| files.into_iter().collect()); registry } diff --git a/crates/core/src/sessions.rs b/crates/core/src/sessions.rs index 96e1581b..ba71d652 100644 --- a/crates/core/src/sessions.rs +++ b/crates/core/src/sessions.rs @@ -1884,6 +1884,7 @@ mod tests { ext_fingerprint: 0, memory_files: None, memory_rows: None, + read_files: None, }); assert!(load_manifest(&core, &id).is_none(), "and so does its manifest"); // All five session-scoped files, so this cannot regress one at a time. diff --git a/crates/core/src/spec.rs b/crates/core/src/spec.rs index f4a57d3f..dcfbe1ba 100644 --- a/crates/core/src/spec.rs +++ b/crates/core/src/spec.rs @@ -929,8 +929,8 @@ provider failed with a rate limit, an overload, or a server fault; thinking already streamed for that attempt is void), `diff` (call_id, path, diff, added, removed), `approval_request` (approval_id, name, summary, detail, reason, source_path, source_sha, and an optional `env`), `approval_settled` (approval_id, -outcome), `refrozen` (tools, skills, changes: the refreeze receipt naming -each recorded capability-file change and its actor), `schemas_over_budget` +outcome), `refrozen` (tools, skills, changes: the refreeze receipt, one line +per tool or skill file added, modified, or removed), `schemas_over_budget` (schema_tokens, budget_tokens: the installed tools take most of what the window can spend, so compaction runs early and stops entirely once they reach it; advisory, at most once per session), `compacted` (tokens_before, diff --git a/crates/core/src/state.rs b/crates/core/src/state.rs index ccd66803..bce272af 100644 --- a/crates/core/src/state.rs +++ b/crates/core/src/state.rs @@ -28,10 +28,6 @@ use crate::config::Settings; use crate::registry::Registry; use crate::types::{AgentEvent, AgentEventEnvelope, ChatMessage}; -/// One extension generation exactly as a freeze captured it: for each file, -/// its path, sha256, and bytes. -pub type ExtensionGeneration = Vec<(PathBuf, String, Vec)>; - /// In-memory state of one agent session. #[derive(Default, Clone)] pub struct SessionData { @@ -58,17 +54,6 @@ pub struct SessionData { /// context window. The condition holds on every turn once it holds at all, /// so the advisory is emitted once and not per turn. pub schemas_over_budget_reported: bool, - /// Whether the ledger has reconciled with the extension files this session - /// froze. False until the first turn start: a freeze reads disk directly, - /// so changes made while no session was running would otherwise never be - /// recorded - and the next mid-turn sync would sweep them up as the - /// agent's own work. - pub ledger_synced: bool, - /// A turn-start generation the ledger could not record. Held so a later - /// mid-turn sync lands it as external work first: the delta that sync - /// records would otherwise span a human's pre-session edits and file - /// them as the agent's. In memory only; see `agent::settle_ledger`. - pub unrecorded_external: Option, /// Content hashes of policy notices (inert allow rules, hooks that did /// not load) already narrated to the MODEL this session. The condition /// holds every turn once it holds at all, so the transcript gets one @@ -80,9 +65,11 @@ pub struct SessionData { /// seeded from the chain at build, then advanced as new events are /// narrated. An approval recorded outside the running session (a human /// at another terminal, #199) reaches it through no other channel - the - /// refreeze receipt names file changes, not approvals - so the turn - /// start names any it has not seen. - pub seen_ledger_events: HashSet, + /// refreeze receipt names file changes, not approvals - so a turn start + /// outside auto names any it has not seen. None until the chain was + /// first read: auto never consults it, so a session built in auto is + /// seeded by its first turn in another mode, which narrates nothing. + pub seen_ledger_events: Option>, /// Server-reported `prompt_tokens` over the local estimate of the same /// request, as last observed on a turn's completion. The bytes/4 /// estimator under-counts BPE tokenizers on code, by far more than the @@ -264,10 +251,9 @@ impl Core { /// Settings say how to reach an endpoint and what a turn may spend. A /// history search uses neither, so an unreadable settings file - a key /// from a newer build, a hand edit, a stray comma - must not also make the - /// project's own history unreadable. `--ledger` already reads its store - /// without loading settings at all; this puts `--recall` on the same - /// footing, and leaves the fail-closed rule exactly where it earns its - /// keep: the paths that spend money and run tools. + /// project's own history unreadable. This leaves the fail-closed rule + /// exactly where it earns its keep: the paths that spend money and run + /// tools. /// /// The failure is returned, never swallowed. Degrading silently to /// defaults would hide a real misconfiguration behind a working search; diff --git a/crates/core/src/types.rs b/crates/core/src/types.rs index 88a63b22..11d169eb 100644 --- a/crates/core/src/types.rs +++ b/crates/core/src/types.rs @@ -213,9 +213,10 @@ pub enum AgentEvent { }, /// The session's tools, skills, and system prompt were re-frozen from /// current config (extension files changed, or the user forced /reload). - /// `changes` is the refreeze receipt: one line per capability file the - /// ledger recorded, with who changed it ("tool.toml modified (external)"), - /// so the action space never mutates silently. + /// `changes` is the refreeze receipt: one line per tool or skill file + /// added, modified, or removed (".openmax/tools/deploy.toml modified"), + /// read from the outgoing and incoming registries, so the action space + /// never mutates silently. Refrozen { tools: usize, skills: usize, changes: Vec }, /// A forced compaction (`/compact`) finished. The automatic budget prune /// speaks through the digest note it leaves in the transcript; the forced @@ -398,9 +399,9 @@ mod tests { env(AgentEvent::Refrozen { tools: 7, skills: 2, - changes: vec![".openmax/tools/deploy.toml added (session)".into()], + changes: vec![".openmax/tools/deploy.toml added".into()], }), - r#"{"session_id":"s1","type":"refrozen","tools":7,"skills":2,"changes":[".openmax/tools/deploy.toml added (session)"]}"# + r#"{"session_id":"s1","type":"refrozen","tools":7,"skills":2,"changes":[".openmax/tools/deploy.toml added"]}"# ); assert_eq!( env(AgentEvent::SchemasOverBudget { schema_tokens: 6800, budget_tokens: 2150 }), diff --git a/crates/core/tests/policy_notices.rs b/crates/core/tests/policy_notices.rs index 1af04e2f..85014b79 100644 --- a/crates/core/tests/policy_notices.rs +++ b/crates/core/tests/policy_notices.rs @@ -760,15 +760,19 @@ async fn an_unverifiable_ledger_blocks_the_turn_and_names_the_repair() { "name = \"docsearch\"\ndescription = \"d\"\ncommand = \"/bin/echo\"\n", ) .unwrap(); + // A human approval is what writes the chain. + let manifest = project.join(".openmax/tools/docsearch.toml"); + let sha = open_max_core::ledger::sha256_hex(&std::fs::read(&manifest).unwrap()); + open_max_core::ledger::approve_capability(&data, &project, &manifest, &[sha]).unwrap(); let (base_url, bodies) = recording_endpoint(vec![completion_with_text("turn one")]).await; write_config(&data, &base_url, &project); let (core, mut rx) = Core::new(data.clone()).unwrap(); - // Turn one: healthy ledger (the freeze seeds it), the turn runs. + // Turn one: healthy ledger, the turn runs. drive_turn(&core, &mut rx, "unver", &project, "one").await; // The log gains a tail with no pending pin: tampering, not a crash, so // every verified read now refuses. let log = open_max_core::ledger::project_dir(&data, &project).join("log.jsonl"); - assert!(log.is_file(), "the first turn's sync seeded the ledger"); + assert!(log.is_file(), "the approval wrote the chain"); let mut text = std::fs::read_to_string(&log).unwrap(); text.push_str("{\"forged\": true}\n"); std::fs::write(&log, text).unwrap(); @@ -826,6 +830,10 @@ async fn a_cached_approval_does_not_survive_an_in_place_ledger_rewrite() { "name = \"docsearch\"\ndescription = \"d\"\ncommand = \"/bin/echo\"\n", ) .unwrap(); + // A human approval is what writes the chain. + let manifest = project.join(".openmax/tools/docsearch.toml"); + let sha = open_max_core::ledger::sha256_hex(&std::fs::read(&manifest).unwrap()); + open_max_core::ledger::approve_capability(&data, &project, &manifest, &[sha]).unwrap(); // Two completions on purpose: the second must never be requested. If the // stale cache passes the gate, turn two consumes it and the assertions // below catch the second body. @@ -842,7 +850,7 @@ async fn a_cached_approval_does_not_survive_an_in_place_ledger_rewrite() { // same pin file, broken chain. let log = open_max_core::ledger::project_dir(&data, &project).join("log.jsonl"); let text = std::fs::read_to_string(&log).unwrap(); - let first_line = text.lines().next().expect("the freeze seeded a record"); + let first_line = text.lines().next().expect("the approval wrote a record"); let record: serde_json::Value = serde_json::from_str(first_line).unwrap(); let sha = record["sha256"].as_str().expect("the record carries a hash").to_string(); let mut flipped = sha.clone(); diff --git a/crates/core/tests/refreeze_receipt.rs b/crates/core/tests/refreeze_receipt.rs index 041488ea..b0e5c564 100644 --- a/crates/core/tests/refreeze_receipt.rs +++ b/crates/core/tests/refreeze_receipt.rs @@ -738,8 +738,12 @@ async fn a_rejected_memory_name_is_never_claimed_indexed() { let _ = std::fs::remove_dir_all(dir); } +/// Removing a tool file is named in the receipt as exactly that: the file, +/// and that it was removed. Capability-file history is no longer recorded, +/// so the receipt makes no claim about what an earlier approval would do if +/// the bytes came back. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn removing_an_approved_tool_says_the_approval_outlives_it() { +async fn removing_an_approved_tool_names_the_file_as_removed() { let dir = std::env::temp_dir().join(format!("omx-receipt-{}", uuid::Uuid::new_v4())); let data = dir.join("data"); let project = dir.join("project"); @@ -772,118 +776,12 @@ async fn removing_an_approved_tool_says_the_approval_outlives_it() { .as_str() .unwrap() .to_string(); - assert!(content.contains("Removed approved tools: echoer"), "{content}"); - assert!(content.contains("the approval outlives the file"), "{content}"); - assert!(content.contains("nothing needs forgetting"), "{content}"); - let _ = std::fs::remove_dir_all(dir); -} - -/// Deleting a tool whose SCRIPT was edited (manifest unchanged, but the -/// bound code no longer matches what was approved) must NOT be classified -/// as a removed-approved tool: the runtime gate would ask again, so the -/// receipt cannot claim 'would run without a card'. -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn removing_a_tool_with_edited_bound_code_is_not_called_approved() { - let dir = std::env::temp_dir().join(format!("omx-receipt-{}", uuid::Uuid::new_v4())); - let data = dir.join("data"); - let project = dir.join("project"); - std::fs::create_dir_all(project.join(".openmax/tools")).unwrap(); - let project = project.canonicalize().unwrap(); - let script = project.join("run.sh"); - std::fs::write(&script, "#!/bin/sh\ncat\n").unwrap(); - let manifest = project.join(".openmax/tools/t.toml"); - std::fs::write(&manifest, "name = \"t\"\ndescription = \"d\"\ncommand = \"./run.sh\"\n").unwrap(); - std::fs::create_dir_all(&data).unwrap(); - // Approve manifest + the ORIGINAL script. - let mut shas = vec![open_max_core::ledger::sha256_hex(&std::fs::read(&manifest).unwrap())]; - shas.extend(open_max_core::ledger::manifest_code(&manifest, &project).into_iter().filter_map(|c| c.sha256)); - open_max_core::ledger::approve_capability(&data, &project, &manifest, &shas).unwrap(); - // Edit the script AND delete the manifest in one turn. - let (base_url, bodies) = recording_endpoint(vec![ - completion_with_tool_call("write_file", serde_json::json!({ "path": "run.sh", "content": "#!/bin/sh\n# edited\ncat\n" })), - completion_with_tool_call("bash", serde_json::json!({ "command": "rm .openmax/tools/t.toml" })), - completion_with_text("gone"), - ]) - .await; - write_config(&data, &base_url, &project); - let (core, mut rx) = Core::new(data).unwrap(); - drive_turn(&core, &mut rx, "edit-then-remove", &project, "go").await; - let bodies = bodies.lock().unwrap(); - // The removal receipt (request 3) must not name t as removed-approved. - let third: serde_json::Value = serde_json::from_str(&bodies[2]).unwrap(); - for m in third["messages"].as_array().unwrap() { - if let Some(c) = m["content"].as_str() { - assert!( - !c.contains("Removed approved tools: t"), - "a tool with edited bound code is not approved: {c}" - ); - } - } - let _ = std::fs::remove_dir_all(dir); -} - -/// Deleting BOTH an approved manifest and its bound script in one turn must -/// still report that the approval SURVIVES: the ledger keeps the record and -/// the content-addressed objects, so the bytes are restorable. Recomputing -/// bound_code after the script is gone makes covers_code fail, which used to -/// drop the tool from the receipt entirely, hiding the surviving approval. -/// It is now named in a distinct clause that does not claim the -/// deleted bytes would run without another approval. -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn removing_an_approved_tool_and_its_script_reports_the_surviving_approval() { - let dir = std::env::temp_dir().join(format!("omx-receipt-{}", uuid::Uuid::new_v4())); - let data = dir.join("data"); - let project = dir.join("project"); - std::fs::create_dir_all(project.join(".openmax/tools")).unwrap(); - let project = project.canonicalize().unwrap(); - let script = project.join("run.sh"); - std::fs::write(&script, "#!/bin/sh\ncat\n").unwrap(); - let manifest = project.join(".openmax/tools/t.toml"); - std::fs::write(&manifest, "name = \"t\"\ndescription = \"d\"\ncommand = \"./run.sh\"\n").unwrap(); - std::fs::create_dir_all(&data).unwrap(); - // Approve manifest + the script it runs. - let mut shas = vec![open_max_core::ledger::sha256_hex(&std::fs::read(&manifest).unwrap())]; - shas.extend(open_max_core::ledger::manifest_code(&manifest, &project).into_iter().filter_map(|c| c.sha256)); - open_max_core::ledger::approve_capability(&data, &project, &manifest, &shas).unwrap(); - // Delete BOTH the manifest and the script in one turn. - let (base_url, bodies) = recording_endpoint(vec![ - completion_with_tool_call("bash", serde_json::json!({ "command": "rm .openmax/tools/t.toml run.sh" })), - completion_with_text("gone"), - ]) - .await; - write_config(&data, &base_url, &project); - let (core, mut rx) = Core::new(data).unwrap(); - drive_turn(&core, &mut rx, "remove-both", &project, "go").await; - let bodies = bodies.lock().unwrap(); - let second: serde_json::Value = serde_json::from_str(&bodies[1]).unwrap(); - let content = second["messages"] - .as_array() - .unwrap() - .iter() - .find(|m| m["role"] == "tool") - .unwrap()["content"] - .as_str() - .unwrap() - .to_string(); - assert!( - content.contains("surviving approval: t"), - "the surviving approval must be reported, not hidden: {content}" - ); - // ...and it must NOT claim the deleted bytes run without a card. - assert!( - !content.contains("Removed approved tools: t"), - "deleted code cannot be called cardless-runnable: {content}" - ); - // Restorability is qualified, not promised: a stored object can be missing - // or corrupt, so the claim is "may". - assert!( - content.contains("may run without a card"), - "the receipt must not overstate object restorability: {content}" - ); assert!( - !content.contains("objects remain in the ledger (restorable)"), - "the unconditional restorable claim was removed: {content}" + content.contains("[extension refreeze: .openmax/tools/echoer.toml removed."), + "{content}" ); + assert!(!content.contains("Removed approved"), "{content}"); + assert!(!content.contains("--ledger"), "{content}"); let _ = std::fs::remove_dir_all(dir); } diff --git a/crates/tui/src/app.rs b/crates/tui/src/app.rs index bf2aa6c9..1856d53a 100644 --- a/crates/tui/src/app.rs +++ b/crates/tui/src/app.rs @@ -2932,7 +2932,7 @@ impl App { } } AgentEvent::Refrozen { tools, skills, changes } => { - // The receipt: what changed and who changed it, so the action + // The receipt: which capability files changed, so the action // space never mutates silently (a poisoned skill arriving via // git pull is announced, not slipped in). let detail = if changes.is_empty() { diff --git a/crates/tui/src/headless.rs b/crates/tui/src/headless.rs index 1fb1db75..53d16214 100644 --- a/crates/tui/src/headless.rs +++ b/crates/tui/src/headless.rs @@ -244,9 +244,10 @@ async fn run_turn_events( } AgentEvent::Refrozen { tools, skills, changes } => { if !json { - // `changes` names capability files the ledger recorded; a - // refreeze can record none, and the line must not end in - // a colon with nothing after it. + // `changes` names the capability files that changed; a + // refreeze can name none (a manifest that turned + // unreadable), and the line must not end in a colon with + // nothing after it. let what = match changes.is_empty() { true => String::new(), false => format!(": {}", changes.join(", ")), diff --git a/crates/tui/src/main.rs b/crates/tui/src/main.rs index 24a2e7fe..7dd33faa 100644 --- a/crates/tui/src/main.rs +++ b/crates/tui/src/main.rs @@ -42,7 +42,8 @@ options: budget:, excerpt:; --json for structured output. Full contract: --spec recall - --ledger print the capability-file history for this project + --ledger deprecated: capability-file history is no longer + recorded; prints where earlier records are kept --ledger-repair quarantine an unverifiable ledger log (nothing is deleted) and start a new chain; approvals in the quarantined log must be granted again @@ -487,7 +488,7 @@ async fn main() -> std::io::Result<()> { } if let Some(query) = &cli.recall { - // Read-only introspection, like --ledger: no session, no endpoint, + // Read-only introspection, like --check: no session, no endpoint, // no trust gate. Recall only ever surfaces this project's own history // (the session index is keyed by project), and the index matches // this path, as given or resolved, against the resolved path each @@ -522,215 +523,17 @@ async fn main() -> std::io::Result<()> { } if cli.ledger { - // Read-only history, like --check: no session, no endpoint, no trust. + // Capability-file history is no longer recorded, so there is nothing + // to print. The flag stays accepted so a script that calls it keeps + // working, and says where this project's records and the object + // copies earlier builds made are kept: as they are, never deleted. let project = std::env::current_dir().unwrap_or_else(|_| std::path::PathBuf::from(".")); - let data_dir = default_data_dir(); - match open_max_core::ledger::read(&data_dir, &project) { - Ok(history) if history.records.is_empty() => { - println!("no capability-file history for this project yet"); - std::process::exit(0); - } - Ok(history) => { - use open_max_core::ledger::{Kind, ObjectState}; - let objects = open_max_core::ledger::project_dir(&data_dir, &project).join("objects"); - // Approvals are recorded by content hash; a path is carried - // when the caller knew one, and otherwise resolved from the - // change record that observed the same bytes. - let mut path_of: std::collections::HashMap<&str, &std::path::Path> = - std::collections::HashMap::new(); - for r in &history.records { - if let (Kind::Change, Some(sha)) = (r.kind, &r.sha256) { - path_of.insert(sha.as_str(), r.path.as_path()); - } - } - let mut states: std::collections::HashMap<&str, ObjectState> = - std::collections::HashMap::new(); - let mut damaged = 0usize; - let mut restorable = 0usize; - // Each intact object with the project path it belongs at, so the - // footer can print a `cp` command that actually runs. The - // history lines above abbreviate the sha to 12 chars for - // reading, but an object's filename is the full 64, so the bare - // `cp / ` template was never executable - // with what the history printed (Judge F). - let mut restore: Vec<(String, std::path::PathBuf)> = Vec::new(); - for r in &history.records { - let short = r.sha256.as_deref().map(|s| &s[..12.min(s.len())]); - let where_ = match (r.path.as_os_str().is_empty(), r.sha256.as_deref()) { - (false, _) => r.path.display().to_string(), - (true, Some(sha)) => path_of - .get(sha) - .map(|p| p.display().to_string()) - .unwrap_or_else(|| "(file not in this ledger)".to_string()), - (true, None) => String::new(), - }; - // Objects are the bytes rollback copies, so a rewritten - // one is a backdoor with a documented delivery route: - // verify on read, not only on write. - let note = match (r.kind, r.sha256.as_deref()) { - (Kind::Change, Some(sha)) => { - restorable += 1; - let state = *states.entry(sha).or_insert_with(|| { - open_max_core::ledger::object_state(&data_dir, &project, sha) - }); - match state { - ObjectState::Intact => "", - ObjectState::Missing => { - damaged += 1; - " (object missing: cannot restore)" - } - ObjectState::Corrupt => { - damaged += 1; - " (object CORRUPT: does not hash to its name - do not restore it)" - } - } - } - _ => "", - }; - let session = r - .session_id - .as_deref() - .map(|s| format!(" session {s}")) - .unwrap_or_default(); - // One approval act can bless a manifest and the code it - // runs; the audit has to show that it covered both - and - // whether their bytes are actually stored. A bound object - // that is missing or corrupt has nothing to restore, and - // the footer's recipe must not imply otherwise. - let bound = match r.also.len() { - 0 => String::new(), - n => { - let stored = r - .also - .iter() - .filter(|sha| { - matches!( - *states.entry(sha.as_str()).or_insert_with(|| { - open_max_core::ledger::object_state(&data_dir, &project, sha) - }), - ObjectState::Intact - ) - }) - .count(); - // Each non-intact bound object is damage too, or - // the footer prints an unqualified `restore with cp` - // recipe while this approval cannot be fully - // restored - same accounting the manifest path does. - damaged += n - stored; - let plural = if n == 1 { "" } else { "s" }; - if stored == n { - format!(" (+{n} bound file{plural})") - } else { - format!( - " (+{n} bound file{plural}, {} with no intact object)", - n - stored - ) - } - } - }; - // The manifest object matters as much as the bound ones: - // a damaged store can keep intact bound objects while the - // PRIMARY manifest object is missing or corrupt, so the - // row must not read as fully restorable. - // Checked from the full sha, not the short - // display form. - let manifest_note = - if open_max_core::ledger::approval_manifest_missing(&data_dir, &project, r) { - // Count it as damaged too, or the footer prints an - // unqualified `restore with cp` recipe while this - // very approval cannot be fully restored. - damaged += 1; - " (manifest bytes not stored: cannot restore the manifest)" - } else { - "" - }; - let what = match (r.kind, short) { - (Kind::Change, Some(sha)) => format!("change {sha} {where_}"), - (Kind::Change, None) => format!("removed {:12} {where_}", ""), - (Kind::Approval, Some(sha)) => { - format!("approved {sha} {where_}{bound}{manifest_note}") - } - (Kind::Approval, None) => { - format!("approved {:12} {where_} (path only)", "") - } - (Kind::PathRetired, _) => { - format!("retired {:12} {where_} (path no longer expected)", "") - } - }; - // The row carries the record's stored path, which is a - // capability file's own name. One record is one row, the - // same rule check_row applies to a finding. - println!( - "{}", - open_max_core::text::one_line(&format!( - "{} {:8} {what}{note}{session}", - open_max_core::ledger::format_ts(r.ts), - r.actor.as_str(), - )) - ); - // Record the runnable restore targets for the footer. A - // change record names its own path and hash; an approval - // names its manifest plus, per vouched hash in `also`, the - // file the approved bytes bound it to - the second file - // the deleted-hook recovery needs and that the old footer - // never named. approval_restore_targets pairs them from - // the record's own path list (hooks) or the stored - // manifest object (tools record none), and refuses rather - // than guesses when a card-skipped file leaves the lists - // unequal. Only intact objects are offered; a missing or - // corrupt one is already counted as damage above. - match r.kind { - Kind::Change => { - if let Some(sha) = &r.sha256 { - if states.get(sha.as_str()).copied() == Some(ObjectState::Intact) { - restore.push((sha.clone(), r.path.clone())); - } - } - } - Kind::Approval => { - if let Some(sha) = &r.sha256 { - if !r.path.as_os_str().is_empty() - && !open_max_core::ledger::approval_manifest_missing( - &data_dir, &project, r, - ) - { - restore.push((sha.clone(), r.path.clone())); - } - } - restore.extend(open_max_core::ledger::approval_restore_targets( - &data_dir, &project, r, - )); - } - _ => {} - } - } - if history.interrupted_write { - let authority = history.records[history.pinned..] - .iter() - .filter(|r| r.kind != open_max_core::ledger::Kind::Change) - .count(); - if authority > 0 { - println!( - "\nnote: {authority} record(s) past the chain-head pin grant or retire authority; nobody's pin vouches for them, so they are inert until `openmax --ledger-repair` sets them aside" - ); - } else { - println!( - "\nnote: the last append landed but its chain-head pin did not (an interrupted write); nothing was removed, and the next capability change re-pins it" - ); - } - } - if let Some(summary) = - ledger_objects_summary(objects.is_dir(), &restore, restorable, damaged, &objects) - { - println!("{summary}"); - } - std::process::exit(0); - } - Err(e) => { - eprintln!("openmax: {e}"); - std::process::exit(1); - } - } + let dir = open_max_core::ledger::project_dir(&default_data_dir(), &project); + println!( + "--ledger is deprecated: capability-file history is no longer recorded; this project's existing records and objects are kept, unchanged, in {}", + dir.display() + ); + std::process::exit(0); } if cli.ledger_repair { @@ -786,7 +589,7 @@ async fn main() -> std::io::Result<()> { println!( "the ledger does not verify: {records} record(s), {approvals} of them approval-grade, will be set aside (nothing is deleted)." ); - println!("a new chain starts at the next capability change; every approval must be granted again."); + println!("a new chain starts at the next approval; every approval must be granted again."); Some("quarantine") } }; @@ -814,7 +617,7 @@ async fn main() -> std::io::Result<()> { outcome.records, path.display() ); - println!("a new chain starts at the next capability change; nothing was deleted, and the objects for rollback are untouched"); + println!("a new chain starts at the next approval; nothing was deleted, and stored objects are untouched"); if outcome.approvals > 0 { println!( "{} approval(s) went with it: re-approve each file you still trust with `openmax --approve `", @@ -949,7 +752,7 @@ async fn main() -> std::io::Result<()> { if !std::io::stdin().is_terminal() || !std::io::stdout().is_terminal() { eprintln!( "openmax: --forget retires a human-installed policy, so it only runs at an interactive terminal.\n\ - without one, restore the file instead: `openmax --ledger` names the object holding its approved bytes" + without one, restore the file to its approved bytes instead" ); std::process::exit(3); } @@ -1546,55 +1349,6 @@ async fn tool_example_rows(project: &std::path::Path) -> (Vec /// passed, or the resolved path the prompt printed - and nothing else does: /// the point is that a person read which policy is going away, so "y" is not /// enough. -/// The trailing objects-store summary for `--ledger`. `restore` is every -/// intact object paired with the path it belongs at, rendered as a runnable, -/// shell-quoted `cp` line so the documented recovery ("an ordinary cp from the -/// objects directory") can be copied and run - the old bare template named a -/// 12-char sha prefix the history printed, but object filenames are the full -/// 64, so it never worked (Judge F). `restorable` still counts the change -/// records that named stored bytes: with none, a missing store is the normal -/// state of an approvals-only ledger, not damage, so warning that "no version -/// can be restored" would cry wolf over a ledger holding nothing restorable. -fn ledger_objects_summary( - store_exists: bool, - restore: &[(String, std::path::PathBuf)], - restorable: usize, - damaged: usize, - objects: &std::path::Path, -) -> Option { - if store_exists { - let mut out = format!("\nobjects: {}", objects.display()); - if !restore.is_empty() { - out.push_str( - "\nrestore a file to a stored version by copying its object back into place:", - ); - let mut seen = std::collections::HashSet::new(); - for (sha, path) in restore { - if seen.insert((sha.as_str(), path.as_path())) { - out.push_str(&format!( - "\n cp {} {}", - open_max_core::doctor::shell_quote(&objects.join(sha)), - open_max_core::doctor::shell_quote(path) - )); - } - } - } - if damaged > 0 { - out.push_str(&format!( - "\nwarning: {damaged} record(s) have no trustworthy object; those bytes cannot be restored from this ledger" - )); - } - Some(out) - } else if restorable > 0 { - Some(format!( - "\nobjects: {} is gone, so no version above can be restored from this ledger", - objects.display() - )) - } else { - None - } -} - fn forget_confirmed(answer: &str, given: &str, resolved: &std::path::Path) -> bool { let answer = answer.trim(); !answer.is_empty() && (answer == given.trim() || answer == resolved.display().to_string()) @@ -1724,7 +1478,7 @@ fn print_usage_economics() { ); } println!( - "\nprompt_chars are paid on every request while the extension is installed.\n{} recorded calls total. Delete what you do not use; openmax --ledger keeps the history restorable.", + "\nprompt_chars are paid on every request while the extension is installed.\n{} recorded calls total. Delete what you do not use.", usage.total_calls ); if capped_out > 0 { @@ -2035,46 +1789,6 @@ mod tests { use super::*; use std::sync::{Arc, Mutex}; - /// An approvals-only ledger has no objects store because nothing ever - /// stored bytes; `--ledger` must not warn that "no version can be - /// restored" over records that were never restorable. - #[test] - fn approvals_only_ledger_reports_no_missing_objects() { - let objects = std::path::Path::new("/data/ledger/objects"); - let none: &[(String, std::path::PathBuf)] = &[]; - assert_eq!(ledger_objects_summary(false, none, 0, 0, objects), None); - - // With stored bytes recorded, a missing store is real damage. - let gone = ledger_objects_summary(false, none, 2, 0, objects).unwrap(); - assert!(gone.contains("is gone"), "{gone}"); - - // A present store with intact objects prints a runnable cp per object, - // using the FULL sha (the object filename) and shell-quoting both - // sides, not the bare un-runnable template the history's 12-char prefix - // never satisfied. - let restore = vec![( - "a".repeat(64), - std::path::PathBuf::from("/proj/.openmax/hooks/x's gate.toml"), - )]; - let ok = ledger_objects_summary(true, &restore, 1, 0, objects).unwrap(); - assert!(ok.contains("copying its object back"), "{ok}"); - assert!( - ok.contains(&format!("cp '/data/ledger/objects/{}'", "a".repeat(64))), - "the cp names the full-sha object path: {ok}" - ); - assert!(ok.contains(r"'/proj/.openmax/hooks/x'\''s gate.toml'"), "quoted target: {ok}"); - assert!(!ok.contains(""), "no un-runnable placeholder remains: {ok}"); - assert!(!ok.contains("warning"), "{ok}"); - - // A present store with nothing intact keeps the pointer, no recipe. - let bare = ledger_objects_summary(true, none, 0, 0, objects).unwrap(); - assert!(bare.contains("objects: /data/ledger/objects"), "{bare}"); - assert!(!bare.contains("cp "), "{bare}"); - - let hurt = ledger_objects_summary(true, &restore, 1, 1, objects).unwrap(); - assert!(hurt.contains("1 record(s) have no trustworthy object"), "{hurt}"); - } - #[derive(Clone, Default)] struct Sink(Arc>>); diff --git a/crates/tui/tests/cli.rs b/crates/tui/tests/cli.rs index 1ed94ef1..6e426afc 100644 --- a/crates/tui/tests/cli.rs +++ b/crates/tui/tests/cli.rs @@ -231,6 +231,17 @@ fn trust_in_ask(project: &Path, home: &Path) { assert_eq!(grant.unwrap().1, Some(ApprovalMode::Ask)); } +/// Approval records in this home's ledger logs, read from disk. +fn approval_records(home: &Path) -> usize { + let Ok(dirs) = std::fs::read_dir(home.join(".openmax").join("ledger")) else { + return 0; + }; + dirs.flatten() + .filter_map(|dir| std::fs::read_to_string(dir.path().join("log.jsonl")).ok()) + .map(|log| log.lines().filter(|line| line.contains("\"kind\":\"approval\"")).count()) + .sum() +} + fn cmd(project: &Path, home: &Path) -> Command { let mut c = Command::new(openmax_bin()); c.current_dir(project); @@ -842,14 +853,6 @@ fn approve_names_every_file_it_blesses() { "approving a turn_end observer must say exit status is ignored: {stdout}" ); - // The ledger's restore promise is true for what an approval blessed: - // the bound script's bytes are stored at approval time, so the audit - // row reads as fully restorable, not "(+1 bound file, 1 not stored)". - let ledger = cmd(&project, &home).arg("--ledger").output().unwrap(); - let ledger = String::from_utf8_lossy(&ledger.stdout); - assert!(ledger.contains("(+1 bound file)"), "{ledger}"); - assert!(!ledger.contains("not stored"), "an approval stores its bound bytes: {ledger}"); - // The pair is live, and rewriting the script alone revokes it. let out = cmd(&project, &home).arg("--check").output().unwrap(); assert_eq!(out.status.code(), Some(0), "{}", String::from_utf8_lossy(&out.stdout)); @@ -930,8 +933,8 @@ fn forget_refuses_without_a_human_at_a_terminal() { ); } -/// A ledger nobody can verify refuses to be read as history and names the -/// way back - which is a human at an interactive terminal: agent sessions +/// A ledger nobody can verify refuses to be extended and names the way +/// back - which is a human at an interactive terminal: agent sessions /// and terminal-less runs are both refused, and the fail-closed state /// survives the refusal. The quarantine itself is proven at the unit level, /// where no confirmation prompt stands in the way. @@ -963,7 +966,12 @@ fn an_unverifiable_ledger_is_refused_and_repairable() { // The easiest tamper there is: delete the pin. std::fs::remove_file(dir.join("chain-head")).unwrap(); - let out = cmd(&project, &home).arg("--ledger").output().unwrap(); + // Approving again cannot extend a chain nobody can verify, and the + // refusal names the way back. + let out = cmd(&project, &home) + .args(["--approve", ".openmax/hooks/gate.toml"]) + .output() + .unwrap(); let stderr = String::from_utf8_lossy(&out.stderr); assert_eq!(out.status.code(), Some(1), "{stderr}"); assert!(stderr.contains("--ledger-repair"), "the way back must be named: {stderr}"); @@ -1001,6 +1009,48 @@ fn an_unverifiable_ledger_is_refused_and_repairable() { ); } +/// Capability-file history is no longer recorded, so `--ledger` has nothing +/// to print. It stays accepted so a script that calls it keeps working: one +/// line says so and names where earlier records and objects stay, and +/// nothing there is touched. +#[test] +fn ledger_is_deprecated_and_leaves_existing_records_in_place() { + let (project, home) = fresh_dirs("ledger-deprecated"); + let hooks = project.join(".openmax").join("hooks"); + std::fs::create_dir_all(&hooks).unwrap(); + std::fs::write(hooks.join("gate.toml"), "event = \"pre_tool_use\"\ncommand = \"/bin/echo\"\n") + .unwrap(); + let out = cmd(&project, &home) + .args(["--approve", ".openmax/hooks/gate.toml"]) + .output() + .unwrap(); + assert_eq!(out.status.code(), Some(0), "{}", String::from_utf8_lossy(&out.stderr)); + assert_eq!(approval_records(&home), 1); + let dir = std::fs::read_dir(home.join(".openmax").join("ledger")) + .unwrap() + .flatten() + .map(|e| e.path()) + .find(|p| p.is_dir()) + .expect("a ledger directory"); + // An object an earlier build stored. + std::fs::create_dir_all(dir.join("objects")).unwrap(); + std::fs::write(dir.join("objects").join("a".repeat(64)), "earlier bytes").unwrap(); + let log = std::fs::read(dir.join("log.jsonl")).unwrap(); + + let out = cmd(&project, &home).arg("--ledger").output().unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!(out.status.code(), Some(0), "{}", String::from_utf8_lossy(&out.stderr)); + assert_eq!(stdout.lines().count(), 1, "{stdout}"); + assert!(stdout.contains("no longer recorded"), "{stdout}"); + assert!(stdout.contains(&dir.display().to_string()), "where the records stay: {stdout}"); + assert_eq!( + std::fs::read_to_string(dir.join("objects").join("a".repeat(64))).unwrap(), + "earlier bytes", + "stored objects are kept" + ); + assert_eq!(std::fs::read(dir.join("log.jsonl")).unwrap(), log, "the log is kept as it was"); +} + /// Each early-exit operation runs and exits before the next one is /// considered, so a second operation on the same command line was dropped /// without a word: `--check --ledger` printed history and exited 0 having @@ -1842,11 +1892,7 @@ fn an_attested_parent_does_not_let_a_bash_child_approve() { let out = cmd(&project, &home).args(["--trust-project", "-p", "go"]).output().unwrap(); let stderr = String::from_utf8_lossy(&out.stderr); assert!(stderr.contains("exit=3"), "the child's --approve must refuse: {stderr}"); - let ledger = cmd(&project, &home).arg("--ledger").output().unwrap(); - assert!( - !String::from_utf8_lossy(&ledger.stdout).contains("approved"), - "no approval may land from an agent's bash child" - ); + assert_eq!(approval_records(&home), 0, "no approval may land from an agent's bash child"); } /// A process the harness itself spawned has no human on its client: an agent @@ -1918,8 +1964,7 @@ fn a_nested_stdio_session_cannot_answer_its_own_content_card() { assert!(decline.contains("no human is on this client"), "the refusal says why: {decline}"); assert!(decline.contains("openmax --approve"), "{decline}"); assert!(!marker.exists(), "the unapproved tool must not have run"); - let ledger = cmd(&project, &home).arg("--ledger").output().unwrap(); - assert!(!String::from_utf8_lossy(&ledger.stdout).contains("approved"), "the ledger must record no grant"); + assert_eq!(approval_records(&home), 0, "the ledger must record no grant"); } /// A sandboxed probe cannot prove a tool that needs the network or a write diff --git a/docs/configuration.md b/docs/configuration.md index c27a512b..1fcb70d2 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -21,7 +21,10 @@ optional `~/.openmax/providers.json` for a catalog of named endpoints. `api_key` to a literal or `$ENV_VAR`, or export `OPENMAX_API_KEY`. `max_parallel_tools` bounds concurrent read-only tool calls, defaults to 4, and is clamped to 1 through 32 at runtime. Mutating, approval-gated, and -non-batchable calls remain serial. +non-batchable calls remain serial. In `auto`, read-only external tools batch +whether or not a human approved their content; outside `auto`, an unapproved +one is never batched: it takes the serial path, which asks for approval or, +in `readonly`, refuses the call. A missing settings file means defaults, and the default `base_url` and `model` are empty: endpoint resolution fails with an actionable error until you set @@ -92,7 +95,8 @@ choice and needs its own. - `auto` runs authorized work without confirmation, including newly created or repaired tools, hooks, and project permission `allow` rules. It ignores - permission `ask` requests. It does not create content approval records. + permission `ask` requests. It neither reads nor creates content approval + records. - `ask` prompts for mutating calls unless a permission `allow` applies, and always prompts for unapproved external tool content. Hooks and project permission `allow` rules still require content approval. diff --git a/docs/extending.md b/docs/extending.md index c8c3acf2..a0874956 100644 --- a/docs/extending.md +++ b/docs/extending.md @@ -271,8 +271,8 @@ content, never to a path: any edit produces a new hash and revokes itself. Living in the chain is the point: an approval store beside the log, with none of the log's protection, would be approvable by anything that can append a line of JSON - including the agent's own `bash`. Forging one now means forging -the chain, which reads as tampering, and every real approval shows up in -`openmax --ledger` with its time and actor. +the chain, which reads as tampering, and every real approval is recorded with +its time and actor. What gets hashed is the whole definition, not just the manifest. A tool or hook TOML is a pointer: the file its `command` names (or a path in its `args`) @@ -321,6 +321,9 @@ restores these requirements; running under auto does not bless their hashes: runs makes the next call prompt again. Rewriting a blessed script is the same act as rewriting the manifest, and it is caught on every path a call can take - including the concurrent batch path, where read-only tools run unattended. + In `auto` none of this is consulted: every external tool runs without + content approval, read-only ones batch like any other read-only call, and + no turn reads the ledger. - Deleting an approved hook file fails closed too, and is found by reconciling the approved paths rather than the directory listing: a deleted file leaves nothing to parse and nothing to report against, and `rm gate.toml` is easier @@ -381,15 +384,10 @@ are the gate. ## The capability ledger -Every tool and skill file a freeze reads is recorded in a per-project, -append-only, hash-chained ledger under `~/.openmax/ledger/`, together with a -content-addressed copy of the bytes. Each record carries the actor at the -strength the harness can prove: `session` (changed while an agent turn was -running), `external` (changed while none was: a human, `git pull`, an -installer), or `initial` (present when the ledger was first populated). The -ledger lives outside the project, where the confined file tools never write, -and each record chains the hash of the previous one, so tampering through the -shell is detectable. +Approvals live in a per-project, append-only, hash-chained ledger under +`~/.openmax/ledger/`. The ledger lives outside the project, where the confined +file tools never write, and each record chains the hash of the previous one, +so tampering through the shell is detectable. Turns in `auto` never read it. Beside the log sits `chain-head`, the hash of its final record. The chain alone proves internal order but not completeness - lopping off trailing @@ -398,23 +396,27 @@ detectable, and a log without one cannot be verified at all. Deleting either file therefore reads as tampering, not as a fresh project. An append writes a pending pin first, flushes the records, then moves the pin, so a crash mid-append leaves a state that reads as an interrupted write (nothing was -removed) and re-pins itself on the next change, rather than an accusation. +removed) and re-pins itself on the next append, rather than an accusation. -Every re-freeze announces a receipt - the `refrozen` event lists what changed -and who changed it - so the agent's action space never mutates silently. -`openmax --ledger` prints the history with object paths, human times, the -session that observed each change, and every approval; it verifies each object -against its own hash, so a rewritten one is named instead of silently offered -for restore. Restoring an earlier version is an ordinary `cp` from the objects -directory. There is no rollback command: the core guarantees the history -exists, using it stays file work. +Earlier versions also recorded every change to a tool or skill file here, +with a copy of its bytes under `objects/`. That history is no longer +recorded. Records and objects already on disk are kept unchanged and never +deleted, and their log still verifies, so an older binary can keep using it. +`openmax --ledger` is deprecated: it prints one line saying so and where this +project's records are kept, and exits 0. + +Every re-freeze announces a receipt - the `refrozen` event lists each tool or +skill file added, modified, or removed - so the agent's action space never +mutates silently. The first re-freeze after a session resumes compares against +the session's saved manifest, which carries only the loaded tools and the skill +index, so a skill body edited while the session was closed is not named there. A ledger that cannot be verified stops appending (a chain nobody can trust -must not be extended) and revokes every approval it held, but never blocks a -turn: the failure rides the refreeze receipt. `openmax --ledger-repair` is the -way back - a human action, refused inside agent-spawned processes, that -quarantines the damaged log as evidence, keeps the objects, and starts a new -chain. Approvals in that log go with it and have to be granted again. +must not be extended) and revokes every approval it held, so outside `auto` +hooks fail closed until it is repaired. `openmax --ledger-repair` is the way +back - a human action, refused inside agent-spawned processes, that +quarantines the damaged log as evidence, keeps any stored objects, and starts +a new chain. Approvals in that log go with it and have to be granted again. ## Self-measurement diff --git a/docs/stdio-protocol.md b/docs/stdio-protocol.md index c0f0b91a..3e229c96 100644 --- a/docs/stdio-protocol.md +++ b/docs/stdio-protocol.md @@ -77,7 +77,7 @@ one). | `diff` | `call_id`, `path`, `diff`, `added`, `removed` | | `approval_request` | `approval_id`, `name`, `summary`, `detail`, `reason` (`gate`, or `unapproved_source` which unattended clients must never auto-approve), `source_path`, `source_sha`, and optional `env` (see below) | | `approval_settled` | `approval_id`, `outcome` (`approved`, `declined`, `timed_out`, or `cancelled`) | -| `refrozen` | `tools`, `skills`, `changes` (the refreeze receipt: what changed and who) | +| `refrozen` | `tools`, `skills`, `changes` (the refreeze receipt: one line per tool or skill file added, modified, or removed) | | `schemas_over_budget` | `schema_tokens`, `budget_tokens` (the installed tool schemas take most of what the window can spend, so compaction runs early against what little is left; once `schema_tokens` reaches `budget_tokens` it stops entirely, since pruning cannot pay a fixed per-request cost. Advisory, at most once per session; the turn still runs) | | `compacted` | `tokens_before`, `tokens_after`, `compacted_messages` (the receipt of a forced compaction; `compacted_messages` of 0 means the transcript was already at or under the prune target and nothing changed) | | `hook_failed` | `hook`, `event`, `detail` (a hook did not run: an observe-only hook failed, or a hook file on disk is not loaded; the turn proceeded) |