From eddade2d4df8498d72b93d3b747b75bdfef5d452 Mon Sep 17 00:00:00 2001 From: Max Loffgren Date: Mon, 5 Oct 2026 22:20:20 -0500 Subject: [PATCH 1/5] feat: stop recording capability-file history and skip approval bookkeeping in auto Every refreeze synced change records into the hash-chained ledger and copied each tool and skill file into its objects store, and every capture hashed and copied those files only to feed it, yet nothing enforced against that history. In auto, where newly trusted projects now start, each turn also re-read and hashed the whole approval log at turn start and once per external tool after every mutating call, and kept unapproved read-only external tools out of concurrent batches although auto runs them unattended either way. The ledger now records approvals only. Captures keep paths, refreezes write nothing, and approvals no longer copy bytes (a bound file changed since the card is still refused, by its current hash). The refreeze receipt names each tool or skill file added, modified, or removed from the two registries, with no actor attribution, held-generation bookkeeping, or clauses about approvals of removed files. Under auto a turn never reads the ledger: approval events, the per-call approval diff, and the batch content check are skipped. Approval records, their chain verification, and content approval in ask are unchanged. `--ledger` stays accepted, prints one line saying history is no longer recorded and where existing records and objects are kept, and exits 0. The log format is unchanged, so older binaries keep working, and stored objects are never deleted. The frozen prompt drops its `--ledger` pointer, and its budget cap ratchets from 3,500 to 3,450 bytes. --- crates/core/src/agent.rs | 1454 ++++++++----------------- crates/core/src/ledger.rs | 911 ++++------------ crates/core/src/prompt.rs | 16 +- crates/core/src/recall.rs | 2 +- crates/core/src/registry.rs | 35 +- crates/core/src/spec.rs | 4 +- crates/core/src/state.rs | 30 +- crates/core/tests/policy_notices.rs | 14 +- crates/core/tests/refreeze_receipt.rs | 120 +- crates/tui/src/app.rs | 2 +- crates/tui/src/headless.rs | 6 +- crates/tui/src/main.rs | 320 +----- crates/tui/tests/cli.rs | 81 +- docs/configuration.md | 8 +- docs/extending.md | 50 +- docs/stdio-protocol.md | 2 +- 16 files changed, 780 insertions(+), 2275 deletions(-) diff --git a/crates/core/src/agent.rs b/crates/core/src/agent.rs index 95a399b3..de05f117 100644 --- a/crates/core/src/agent.rs +++ b/crates/core/src/agent.rs @@ -111,6 +111,7 @@ fn batchable_call( call: &ToolCall, registry: &Registry, permissions: &TurnPermissions, + mode: ApprovalMode, data_dir: &Path, project_root: &Path, ) -> bool { @@ -130,14 +131,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 +257,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 +469,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 +559,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 +589,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, } }) @@ -1493,11 +1501,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 +1515,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 +1914,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 +1959,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 +1995,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 +2012,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 +2049,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 +2093,56 @@ 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: a tool by its +/// manifest's path and content hash, a skill by its SKILL.md path and the +/// index line the prompt carries 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 or the prompt's cap trailer names it. +fn generation_changes(old: &Registry, new: &Registry, project_root: &Path) -> Vec { + fn files(registry: &Registry) -> std::collections::BTreeMap<&Path, String> { + let tools = registry.tools.iter().filter_map(|spec| match &spec.kind { + crate::registry::ToolKind::External(ext) => { + Some((ext.source_path.as_path(), ext.source_sha256.clone())) + } + crate::registry::ToolKind::Builtin => None, + }); + let skills = registry + .skills + .iter() + .map(|skill| (skill.path.as_path(), format!("{}\n{}", skill.name, skill.description))); + tools.chain(skills).collect() + } + let (before, after) = (files(old), files(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.contains(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 +2153,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 +2162,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 +2179,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 +2212,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 +2231,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 +2240,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 +2250,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 +2270,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 +2389,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 +2561,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 +2602,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 +2934,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, ) @@ -3455,8 +3302,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 +3428,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 +3441,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 +3534,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 +3859,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 +4695,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 +4723,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 +4852,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,9 +5095,11 @@ 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() { let dir = std::env::temp_dir().join(format!("openmax-batch-{}", uuid::Uuid::new_v4())); @@ -5289,24 +5119,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 +5164,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 +5183,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 +5192,7 @@ mod tests { &tool_call("write_file", r#"{"path":"x","content":"y"}"#), ®istry, &empty_perms, + ApprovalMode::Ask, nowhere(), nowhere(), )); @@ -5361,6 +5200,7 @@ mod tests { &tool_call("nope", r#"{}"#), ®istry, &empty_perms, + ApprovalMode::Ask, nowhere(), nowhere(), )); @@ -5442,7 +5282,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 +5303,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 +5748,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 +5757,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 +5783,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 +5815,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,8 +5853,6 @@ 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 note = refreeze_receipt_text(&changes, &added, &broken, &shadowed, Path::new("/p"), ApprovalMode::Ask); @@ -6038,10 +5868,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. + /// already names it under "NOT loaded". Calling it removed on top of that + /// is a second, contradictory line about the same file. 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 +5883,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 +5895,16 @@ 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!(changes.is_empty(), "a broken-by-edit tool is not a removed tool: {changes:?}"); 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 +5917,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,24 +5932,27 @@ 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. + /// 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 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())); @@ -6143,8 +5965,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()); @@ -6160,122 +5980,16 @@ 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(), - "the broken clause speaks alone; no contradictory removal clause: {:?} / {:?}", - added.removed_approved, - added.removed_approval_survives - ); + let changes = generation_changes(&old, &new, &project); + assert!(changes.is_empty(), "the broken clause speaks alone: {changes:?}"); 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. - #[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())); - 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)) - .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" - ); - - // 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") - .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 - ); - - 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 _ = 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. - #[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())); - 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 old = Registry::from_snapshot(crate::registry::capture_extensions(&data, &project)); - assert!(old.get("deploy").is_some()); - - std::fs::remove_file(&manifest).unwrap(); - std::fs::write( - project.join(".openmax/tools/other.toml"), - "name = \"other\"\ndescription = \"broken, no command\"\n", - ) - .unwrap(); - let new = Registry::from_snapshot(crate::registry::capture_extensions(&data, &project)); - - let added = classify_added_tools(&old, &new, &data, &project); - assert!( - added.removed_approved == vec!["deploy".to_string()], - "an unrelated broken file must not swallow the removal: {:?} / {:?}", - added.removed_approved, - added.removed_approval_survives - ); - - 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 +6012,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,12 +6024,11 @@ 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" ); let _ = std::fs::remove_dir_all(dir); @@ -6331,9 +6037,8 @@ mod tests { /// 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 +6053,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 +6074,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 +6086,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,458 +6780,6 @@ 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. - #[tokio::test] - async fn first_turn_records_changes_made_between_sessions() { - 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 @@ -8153,6 +7399,156 @@ 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 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 97f24e5d..2c2b9731 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,18 +22,18 @@ 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, } @@ -55,7 +53,8 @@ impl Actor { #[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 +79,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 +125,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 +165,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 +241,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 +263,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 +272,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 +468,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. @@ -566,7 +530,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() @@ -582,101 +546,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` @@ -686,7 +555,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. @@ -854,6 +723,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() @@ -1251,135 +1097,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 { @@ -1424,8 +1141,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::*; @@ -1938,23 +1634,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 @@ -2007,38 +1686,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: @@ -2053,12 +1736,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 = { @@ -2072,7 +1754,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); @@ -2080,20 +1762,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 @@ -2229,12 +1897,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(); @@ -2245,18 +1912,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); } @@ -2297,8 +1957,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"); @@ -2315,7 +1975,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" @@ -2324,12 +1984,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(); @@ -2339,46 +1998,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); @@ -2388,10 +2020,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)); @@ -2410,10 +2039,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(); @@ -2424,20 +2050,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"); @@ -2666,136 +2278,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() @@ -2861,8 +2343,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"); @@ -3096,16 +2577,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); @@ -3135,8 +2613,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 @@ -3164,16 +2641,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. @@ -3198,10 +2676,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(); @@ -3210,16 +2690,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); } @@ -3260,28 +2739,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] @@ -3292,7 +2753,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); } @@ -3322,8 +2783,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); @@ -3348,9 +2808,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}"); @@ -3363,20 +2820,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, @@ -3392,11 +2848,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); } @@ -3409,8 +2867,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 ca1000f1..b823311d 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.\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. @@ -761,9 +760,8 @@ 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). 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). 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] @@ -791,7 +789,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..7d9852b4 100644 --- a/crates/core/src/registry.rs +++ b/crates/core/src/registry.rs @@ -104,7 +104,7 @@ pub struct Registry { /// 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 + /// manifests and SKILL.mds). The refreeze receipt 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 @@ -112,8 +112,8 @@ pub struct Registry { pub(crate) read_paths: std::collections::HashSet, /// 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 +166,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 path this generation read. Paths only: the + /// bytes were parsed and fingerprinted where they were read, and nothing + /// downstream needs a copy or a second hash of them. + read_paths: Vec, /// 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 +187,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 +205,7 @@ 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 = Vec::new(); // (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 +245,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()); let Ok(text) = std::str::from_utf8(&bytes) else { broken_at.push((path, "not valid UTF-8".into(), dir_index, None)); continue; @@ -271,8 +270,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 +324,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()); let Ok(text) = std::str::from_utf8(&bytes) else { broken.push((path, "not valid UTF-8".into())); continue; @@ -371,7 +370,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 +408,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 +445,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 = snapshot.read_paths.into_iter().collect(); registry.broken = snapshot.broken; registry.broken_tools = snapshot.broken_tools; registry.shadowed_skills = snapshot.shadowed_skills; diff --git a/crates/core/src/spec.rs b/crates/core/src/spec.rs index a50b759c..dad20379 100644 --- a/crates/core/src/spec.rs +++ b/crates/core/src/spec.rs @@ -927,8 +927,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 14f97aaf..db2240e4 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 @@ -262,10 +249,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/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..a4f4db08 100644 --- a/crates/tui/src/headless.rs +++ b/crates/tui/src/headless.rs @@ -244,9 +244,9 @@ 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 skill body edit), 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 a72e6d3a..a46e1b1d 100644 --- a/crates/tui/src/main.rs +++ b/crates/tui/src/main.rs @@ -41,7 +41,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 @@ -405,7 +406,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 project key is the // same raw current_dir form session creation stores. @@ -439,215 +440,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 { @@ -703,7 +506,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") } }; @@ -731,7 +534,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 `", @@ -866,7 +669,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); } @@ -1463,55 +1266,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()) @@ -1641,7 +1395,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 { @@ -1952,46 +1706,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 ccaed20c..935baabb 100644 --- a/crates/tui/tests/cli.rs +++ b/crates/tui/tests/cli.rs @@ -190,6 +190,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); @@ -801,14 +812,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)); @@ -889,8 +892,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. @@ -922,7 +925,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}"); @@ -960,6 +968,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 @@ -1801,11 +1851,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 @@ -1877,8 +1923,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 65951388..ae65518a 100644 --- a/docs/extending.md +++ b/docs/extending.md @@ -245,8 +245,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`) @@ -295,6 +295,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 @@ -355,15 +358,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 @@ -372,23 +370,25 @@ 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. 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) | From 11faa9135d54bacd67df09d74922d5b44f6c0107 Mon Sep 17 00:00:00 2001 From: Max Loffgren Date: Mon, 5 Oct 2026 22:36:02 -0500 Subject: [PATCH 2/5] fix(core): name each changed capability file in the refreeze receipt The refreeze receipt compared skills by their index line (name and description), but the fingerprint hashes every SKILL.md byte. A body-only edit therefore refroze without naming any file, and the receipt fell back to "extension files changed", so a skill rewritten by a pull arrived unannounced. A manifest added past the tool cap was not named either. The capture now keeps a cheap in-memory hash of each file's bytes beside its path, and the receipt compares two captured generations file by file from it. A registry restored from a session manifest read nothing, so that comparison alone falls back to loaded tools and indexed skills. The fingerprint itself is unchanged, so persisted sessions resume without a refreeze. Also drop the unused Actor::as_str, port the one-line-per-entry test to the new receipt, and correct docs and comments that still described ledger-recorded changes with actors. --- crates/core/src/agent.rs | 130 +++++++++++++++++++++++++++++++----- crates/core/src/ledger.rs | 13 +--- crates/core/src/registry.rs | 41 +++++++----- crates/core/src/types.rs | 11 +-- crates/tui/src/headless.rs | 5 +- docs/extending.md | 4 +- 6 files changed, 152 insertions(+), 52 deletions(-) diff --git a/crates/core/src/agent.rs b/crates/core/src/agent.rs index de05f117..e1d72d80 100644 --- a/crates/core/src/agent.rs +++ b/crates/core/src/agent.rs @@ -2096,26 +2096,46 @@ fn added_tool_names(old: &Registry, new: &Registry) -> Vec { /// 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: a tool by its -/// manifest's path and content hash, a skill by its SKILL.md path and the -/// index line the prompt carries 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 or the prompt's cap trailer names it. +/// 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. An outgoing registry restored from +/// a session manifest read nothing, so that one comparison falls back to +/// what the manifest carries: 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 { - fn files(registry: &Registry) -> std::collections::BTreeMap<&Path, String> { + 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(), ext.source_sha256.clone())) + 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(), format!("{}\n{}", skill.name, skill.description))); + .map(|skill| (skill.path.as_path(), Identity::Skill(&skill.name, &skill.description))); tools.chain(skills).collect() } - let (before, after) = (files(old), files(new)); + 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) { @@ -2125,8 +2145,8 @@ fn generation_changes(old: &Registry, new: &Registry, project_root: &Path) -> Ve } } for &path in before.keys() { - let still_on_disk = - new.read_paths.contains(path) || new.broken.iter().any(|(broken, _)| broken == path); + 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")); } @@ -5101,7 +5121,7 @@ mod tests { /// 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"); @@ -5868,9 +5888,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". Calling it removed on top of that - /// is a second, contradictory line about the same file. 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())); @@ -5896,7 +5916,11 @@ mod tests { ); let changes = generation_changes(&old, &new, &project); - assert!(changes.is_empty(), "a broken-by-edit tool is not a removed tool: {changes:?}"); + assert_eq!( + changes, + [".openmax/tools/deploy.toml modified"], + "a broken-by-edit tool is modified, not removed" + ); let _ = std::fs::remove_dir_all(dir); } @@ -5948,6 +5972,68 @@ mod tests { let _ = std::fs::remove_dir_all(dir); } + /// 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_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 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)); + + 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_ne!(old.ext_fingerprint, new.ext_fingerprint, "the body edit refreezes"); + + assert_eq!(generation_changes(&old, &new, &project), [".agents/skills/ship/SKILL.md modified"]); + + 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); + } + /// 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 @@ -6031,6 +6117,18 @@ mod tests { "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); } diff --git a/crates/core/src/ledger.rs b/crates/core/src/ledger.rs index 2c2b9731..64ee18f3 100644 --- a/crates/core/src/ledger.rs +++ b/crates/core/src/ledger.rs @@ -37,16 +37,6 @@ pub enum Actor { 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. @@ -2220,8 +2210,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(); diff --git a/crates/core/src/registry.rs b/crates/core/src/registry.rs index 7d9852b4..080917a8 100644 --- a/crates/core/src/registry.rs +++ b/crates/core/src/registry.rs @@ -103,13 +103,14 @@ 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 receipt 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. None for a registry no capture built (manifest-restored + /// or built-ins only), which only ever sits on the outgoing side of that + /// comparison. In memory only, never persisted. + 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 unknown-tool error @@ -166,10 +167,10 @@ pub(crate) struct ExtensionSnapshot { tools_omitted: usize, /// Skills discovered but dropped by the `MAX_SKILLS` index cap. skills_omitted: usize, - /// Every capability file path this generation read. Paths only: the - /// bytes were parsed and fingerprinted where they were read, and nothing - /// downstream needs a copy or a second hash of them. - read_paths: 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. @@ -205,7 +206,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 read_paths: 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 @@ -245,7 +254,7 @@ pub(crate) fn capture_extensions(data_dir: &Path, project_root: &Path) -> Extens }; bytes.hash(&mut h); let Some(bytes) = bytes else { continue }; - read_paths.push(path.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; @@ -324,7 +333,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 }; - read_paths.push(path.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; @@ -445,7 +454,7 @@ impl Registry { registry.ext_fingerprint = snapshot.fingerprint; registry.tools_omitted = snapshot.tools_omitted; registry.skills_omitted = snapshot.skills_omitted; - registry.read_paths = snapshot.read_paths.into_iter().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; @@ -502,7 +511,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, 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/tui/src/headless.rs b/crates/tui/src/headless.rs index a4f4db08..53d16214 100644 --- a/crates/tui/src/headless.rs +++ b/crates/tui/src/headless.rs @@ -245,8 +245,9 @@ async fn run_turn_events( AgentEvent::Refrozen { tools, skills, changes } => { if !json { // `changes` names the capability files that changed; a - // refreeze can name none (a skill body edit), and the - // line must not end in a colon with nothing after it. + // 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/docs/extending.md b/docs/extending.md index ae65518a..a1a33cf1 100644 --- a/docs/extending.md +++ b/docs/extending.md @@ -381,7 +381,9 @@ 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. +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, so outside `auto` From e58f1d1c522c6300484321886ed2decbbafff6b0 Mon Sep 17 00:00:00 2001 From: Max Loffgren Date: Mon, 5 Oct 2026 22:52:27 -0500 Subject: [PATCH 3/5] fix(core): run an auto-formed batch serially when the mode leaves auto first A reply's tool calls are grouped into concurrent batches once, under the mode of that moment, and auto now lets unapproved read-only external tools into a batch. If the user switched from auto to ask while an earlier call in the same reply was still running, the group still ran as a batch under ask, where the content gate declined every call without raising an approval card. Each segment now runs as a batch only when the live mode still matches the mode it was formed under; otherwise its calls take the serial path, which asks. Two test texts that described the removed receipt clauses are corrected. --- crates/core/src/agent.rs | 97 ++++++++++++++++++++++++++++++++++++++- crates/core/src/ledger.rs | 2 +- 2 files changed, 96 insertions(+), 3 deletions(-) diff --git a/crates/core/src/agent.rs b/crates/core/src/agent.rs index e1d72d80..641706a6 100644 --- a/crates/core/src/agent.rs +++ b/crates/core/src/agent.rs @@ -2978,7 +2978,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, @@ -5874,7 +5878,7 @@ mod tests { modified_unapproved: Vec::new(), memory: None, }; - 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'), @@ -7607,6 +7611,95 @@ mod tests { 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 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 diff --git a/crates/core/src/ledger.rs b/crates/core/src/ledger.rs index 64ee18f3..df6554d5 100644 --- a/crates/core/src/ledger.rs +++ b/crates/core/src/ledger.rs @@ -1914,7 +1914,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"); From 093ecd478d3d688e6514b7070629db33b15c0d25 Mon Sep 17 00:00:00 2001 From: Max Loffgren Date: Mon, 5 Oct 2026 23:16:34 -0500 Subject: [PATCH 4/5] fix(core): keep a queued batch call from starting once the mode leaves auto A concurrent batch admits every call up front, then starts them behind the parallelism cap, so a call can wait in the queue long after auto admitted it. Auto never asks whether a tool's content is approved, so if the user switched to ask while an unapproved external tool waited, it still started, under ask, with no approval card. Each call now re-reads the mode as it starts. A call that auto admitted without the content question, whose tool is unapproved, is refused once the mode has left auto, with a reason that asks the model to request it again; requested again, it takes the serial path, which raises the card. --- crates/core/src/agent.rs | 138 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 132 insertions(+), 6 deletions(-) diff --git a/crates/core/src/agent.rs b/crates/core/src/agent.rs index 641706a6..ff767b0e 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); @@ -707,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 @@ -782,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, @@ -793,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(); @@ -811,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); } - registry.execute(&name, &args, data_dir, &root, caps, cancel).await + // 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, 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 @@ -7700,6 +7727,105 @@ mod tests { 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 From 540db6271481618b9db3d92cf196520dbac7f7d8 Mon Sep 17 00:00:00 2001 From: Max Loffgren Date: Mon, 5 Oct 2026 23:19:57 -0500 Subject: [PATCH 5/5] fix(core): name a capability file edited while its session was closed A resumed session's registry is rebuilt from its manifest, which kept no record of the bytes its freeze read. The refreeze receipt could then only compare index lines, so a SKILL.md body edited while the session was closed went live under a generic "extension files changed" and the file was never named. The manifest now carries the per-file hashes the freeze read, and a resumed registry compares file by file like a live one. The field is additive: a manifest written before it still loads and compares index lines. --- crates/core/src/agent.rs | 45 +++++++++++++++++++++++++++++++++---- crates/core/src/registry.rs | 20 ++++++++++++++--- crates/core/src/sessions.rs | 1 + 3 files changed, 59 insertions(+), 7 deletions(-) diff --git a/crates/core/src/agent.rs b/crates/core/src/agent.rs index ff767b0e..b46a3251 100644 --- a/crates/core/src/agent.rs +++ b/crates/core/src/agent.rs @@ -2126,10 +2126,11 @@ fn added_tool_names(old: &Registry, new: &Registry) -> Vec { /// 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. An outgoing registry restored from -/// a session manifest read nothing, so that one comparison falls back to -/// what the manifest carries: a loaded tool by its content hash and an -/// indexed skill by the index line the prompt shows for it. A file the new +/// 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 { @@ -6030,6 +6031,42 @@ mod tests { let _ = std::fs::remove_dir_all(dir); } + /// 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_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 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 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()); + + 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_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 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 diff --git a/crates/core/src/registry.rs b/crates/core/src/registry.rs index 080917a8..cc345ce9 100644 --- a/crates/core/src/registry.rs +++ b/crates/core/src/registry.rs @@ -107,9 +107,10 @@ pub struct Registry { /// 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. None for a registry no capture built (manifest-restored - /// or built-ins only), which only ever sits on the outgoing side of that - /// comparison. In memory only, never persisted. + /// 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 @@ -686,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 @@ -761,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(), @@ -804,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 debff1e1..f0bb67da 100644 --- a/crates/core/src/sessions.rs +++ b/crates/core/src/sessions.rs @@ -1697,6 +1697,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.