diff --git a/crates/webcodex-workflow-session/src/continuation.rs b/crates/webcodex-workflow-session/src/continuation.rs index 3c0f22947..38e00f50d 100644 --- a/crates/webcodex-workflow-session/src/continuation.rs +++ b/crates/webcodex-workflow-session/src/continuation.rs @@ -28,7 +28,7 @@ use serde::Serialize; use serde_json::{json, Value}; use sha2::{Digest, Sha256}; -use std::collections::{BTreeSet, HashSet}; +use std::collections::{BTreeSet, HashMap, HashSet}; use crate::{ canonical_tool_call_finished_events, closeout_work_projection, current_attempt_event_view, @@ -540,6 +540,94 @@ fn empty_attempt() -> AttemptSummary { } } +/// Whether any mutating tool call in `events` lacks exactly one matching +/// finish after it. Callers pass the full retained history so an unfinished +/// mutation older than a bounded summary window is still detected; the scan +/// is a single reverse pass that allocates only per-identity finish counters. +/// +/// A successful Job acceptance has its own matching terminal tool event; +/// its subsequent running/recovering state belongs to the Job lifecycle, +/// not to this unfinished-call check. Never correlate by wall time or args. +pub(crate) fn has_unclosed_mutating_call<'a>( + events: impl DoubleEndedIterator, + session_id: &str, +) -> bool { + // Exact call identity. `session_id` is fixed for the whole scan, so the + // key carries the remaining match dimensions. Project comes from the + // event's own resolved/plain project, never the Session default. + type CallIdentity<'a> = ( + Option<&'a str>, + &'a str, + &'a str, + Option<&'a str>, + Option<&'a str>, + ); + fn call_identity<'a>(event: &'a SessionEvent) -> Option> { + let call_id = event.call_id.as_deref().filter(|id| !id.is_empty())?; + Some(( + event + .resolved_project + .as_deref() + .or(event.project.as_deref()), + event.tool_name.as_str(), + call_id, + event.logical_invocation_id.as_deref(), + event.logical_invocation_role.as_deref(), + )) + } + // Suffix finish counts, built back-to-front so each start is closed only + // by finishes recorded after it: a duplicated call identity must not let a + // later start inherit an earlier invocation's finish. Duplicated or + // corrupted finishes are not authoritative confirmation — exactly one + // match closes a call. + let mut suffix_finishes: HashMap, usize> = HashMap::new(); + for event in events.rev() { + if event.kind == "tool_call_finished" { + if event.session_id != session_id { + continue; + } + if let Some(identity) = call_identity(event) { + *suffix_finishes.entry(identity).or_insert(0) += 1; + } + continue; + } + if event.kind != "tool_call_started" + || !(event.write_like || event.shell_like) + || event.session_id != session_id + { + continue; + } + let Some(call_id) = event.call_id.as_deref().filter(|id| !id.is_empty()) else { + // Legacy starts cannot be proven to have a matching finish. + return true; + }; + // A Session can record implicitly scoped calls and requests for a + // different Project (including rejected requests). Neither case is + // proof that the recorded start finished. Compare the start's own + // Project identity with the finish, not with the Session default. + let identity: CallIdentity<'_> = ( + event + .resolved_project + .as_deref() + .or(event.project.as_deref()), + event.tool_name.as_str(), + call_id, + event.logical_invocation_id.as_deref(), + event.logical_invocation_role.as_deref(), + ); + let Some(unmatched_finishes) = suffix_finishes.get_mut(&identity) else { + return true; + }; + if *unmatched_finishes != 1 { + return true; + } + // A finish is proof for only one start. Two starts sharing the same + // identity followed by one finish must not both be considered closed. + *unmatched_finishes = 0; + } + false +} + #[allow(clippy::too_many_arguments)] fn build_attempt_summary( attempt_events: &[SessionEvent], @@ -588,6 +676,14 @@ fn build_attempt_summary( }; // --- activity (meaningful tool calls only) --- + // An attempt boundary is not confirmation that a prior mutating request + // finished. Only exact retained call identity can resolve such a start. + // The store computed the flag over the full retained history (see + // `SessionSummary::unclosed_mutating_call`), so an unfinished mutation + // older than this bounded summary window still blocks the attempt. + let unconfirmed_mutating_call = session_summary.unclosed_mutating_call; + let incomplete_call_history = + session_summary.events_truncated || session_summary.retention_truncated; let canonical_finished = canonical_tool_call_finished_events(semantic_attempt_events); let meaningful: Vec<&SessionEvent> = canonical_finished .iter() @@ -665,7 +761,7 @@ fn build_attempt_summary( let guidance_block = build_attempt_guidance(discussion); // --- outcome --- - let (outcome_status, reason_codes) = build_attempt_outcome( + let (mut outcome_status, mut reason_codes) = build_attempt_outcome( &meaningful, actionable_failed_tool_calls, unresolved_failures, @@ -673,6 +769,15 @@ fn build_attempt_summary( &guidance_block, &validation_block, ); + if unconfirmed_mutating_call { + outcome_status = "blocked".to_string(); + push_unique(&mut reason_codes, "unconfirmed_tool_execution"); + } + // The bounded tail cannot prove that no older effect remains unresolved. + // This is uncertainty about evidence coverage, not permission to replay. + if incomplete_call_history { + push_unique(&mut reason_codes, "mutating_call_history_incomplete"); + } let mut suggested_next_actions = build_suggested_next_actions( &outcome_status, @@ -707,6 +812,17 @@ fn build_attempt_summary( suggested_next_actions.insert(0, action.to_string()); suggested_next_actions.truncate(MAX_SUGGESTED_ACTIONS); } + if unconfirmed_mutating_call { + let action = crate::task_context::RECONCILE_ACTION.to_string(); + suggested_next_actions.retain(|suggestion| suggestion != &action); + suggested_next_actions.insert(0, action); + suggested_next_actions.truncate(MAX_SUGGESTED_ACTIONS); + } else if incomplete_call_history { + let action = "inspect missing Session history before inferring prior tool outcomes"; + suggested_next_actions.retain(|suggestion| suggestion != action); + suggested_next_actions.insert(0, action.to_string()); + suggested_next_actions.truncate(MAX_SUGGESTED_ACTIONS); + } AttemptSummary { boundary: AttemptBoundary { diff --git a/crates/webcodex-workflow-session/src/continuation_tests.rs b/crates/webcodex-workflow-session/src/continuation_tests.rs index 32141ff21..584400f48 100644 --- a/crates/webcodex-workflow-session/src/continuation_tests.rs +++ b/crates/webcodex-workflow-session/src/continuation_tests.rs @@ -230,6 +230,512 @@ fn validation_summary(events: Vec) -> Value { json!({"events": events}) } +#[test] +fn unfinished_mutation_survives_ledger_reload_and_new_attempt() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("session.json"); + let initial = SessionStore::with_persistence(&path, 16, 600); + let session = create_session(&initial, "uncertain write"); + add_instruction(&initial, &session, "first attempt"); + let pending = initial.record_tool_call_started( + Some(&session), + SessionTransport::Api, + "synthetic_write", + &json!({"project": PROJECT, "paths": ["src/pending.rs"]}), + synthetic_contract(false, true, SessionPathHint::PathList), + ); + // Keep the unfinished start in the JSON ledger but outside the summary. + for index in 0..220 { + record_read( + &initial, + &session, + &format!("filler-{index}.rs"), + json!({"ok": true}), + ); + } + initial.flush_persistence(); + drop(initial); + + let restored = SessionStore::with_persistence(&path, 16, 600); + let summary = restored.summary(&session, Some(200)).unwrap(); + assert!(summary.events_truncated); + assert!(!summary.retention_truncated); + assert!(summary.unclosed_mutating_call); + assert!(!summary.events.iter().any(|event| event.write_like)); + assert!(serde_json::to_value(&summary) + .unwrap() + .get("unclosed_mutating_call") + .is_none()); + add_instruction(&restored, &session, "continue after disconnect"); + let feedback = feedback_for(&restored, &session); + assert_eq!(feedback["attempt"]["activity"]["meaningful_tool_calls"], 0); + assert_eq!(feedback["attempt"]["outcome"]["status"], "blocked"); + assert!(feedback["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution"))); + assert!(feedback["attempt"]["suggested_next_actions"] + .as_array() + .unwrap() + .iter() + .any(|item| item.as_str().is_some_and(|text| text.contains("reconcile")))); + + restored.record_tool_call_finished(pending, true, &json!({"applied": true}), None, None); + let confirmed = feedback_for(&restored, &session); + assert!(!confirmed["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution"))); +} + +#[test] +fn unfinished_mutation_without_explicit_project_still_blocks_continuation() { + let store = store(200); + let session = create_session(&store, "implicit project"); + add_instruction(&store, &session, "write using session project"); + let started = store.record_tool_call_started( + Some(&session), + SessionTransport::Api, + "synthetic_write", + &json!({"paths": ["src/implicit.rs"]}), + synthetic_contract(false, true, SessionPathHint::PathList), + ); + let unconfirmed = feedback_for(&store, &session); + assert_eq!(unconfirmed["attempt"]["outcome"]["status"], "blocked"); + assert!(unconfirmed["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution"))); + store.record_tool_call_finished(started, true, &json!({"applied": true}), None, None); + assert!( + !feedback_for(&store, &session)["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution")) + ); +} + +#[test] +fn unfinished_mutation_on_different_project_still_blocks_owning_session() { + let store = store(200); + let session = create_session(&store, "other project"); + add_instruction(&store, &session, "record external project invocation"); + let started = store.record_tool_call_started_with_metadata( + Some(&session), + SessionTransport::Api, + "synthetic_write", + &json!({"project": "other-project", "paths": ["src/other.rs"]}), + Some("other-project".into()), + ToolCallRecorderMetadata::default(), + synthetic_contract(false, true, SessionPathHint::PathList), + ); + let unconfirmed = feedback_for(&store, &session); + assert_eq!(unconfirmed["attempt"]["outcome"]["status"], "blocked"); + assert!(unconfirmed["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution"))); + store.record_tool_call_finished(started, true, &json!({"applied": true}), None, None); + assert!( + !feedback_for(&store, &session)["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution")) + ); +} + +/// Reused or corrupted invocation identities cannot let one finish certify +/// multiple write-like starts, even when their exact identity collides. +#[test] +fn duplicate_mutating_starts_with_one_finish_remain_unconfirmed() { + let store = store(20); + let session = create_session(&store, "duplicate mutating identity"); + let started = store.record_tool_call_started( + Some(&session), + SessionTransport::Api, + "synthetic_write", + &json!({"project": PROJECT, "paths": ["src/a.rs"]}), + synthetic_contract(false, true, SessionPathHint::PathList), + ); + store.record_tool_call_finished(started, true, &json!({"applied": true}), None, None); + let summary = store.summary(&session, Some(20)).unwrap(); + let start = summary + .events + .iter() + .find(|e| e.kind == "tool_call_started") + .unwrap() + .clone(); + let finish = summary + .events + .iter() + .find(|e| e.kind == "tool_call_finished") + .unwrap() + .clone(); + let unclosed = |events: &[crate::SessionEvent]| { + crate::continuation::has_unclosed_mutating_call(events.iter(), &session) + }; + assert!(unclosed(&[start.clone(), start.clone(), finish.clone()])); + assert!(unclosed(&[start.clone(), finish.clone(), finish.clone()])); + assert!(unclosed(&[finish.clone(), start.clone()])); + assert!(unclosed(&[start.clone(), finish.clone(), start.clone()])); + assert!(!unclosed(&[ + start.clone(), + finish.clone(), + start.clone(), + finish.clone(), + ])); + let mut missing_call_id = start.clone(); + missing_call_id.call_id = None; + assert!(unclosed(&[missing_call_id, finish.clone()])); + let mut empty_call_id = start.clone(); + empty_call_id.call_id = Some(String::new()); + assert!(unclosed(&[empty_call_id, finish.clone()])); + let mut foreign_finish = finish.clone(); + foreign_finish.session_id.push_str("_other"); + assert!(unclosed(&[start.clone(), foreign_finish])); + let mut wrong_tool = finish.clone(); + wrong_tool.tool_name.push_str("_other"); + assert!(unclosed(&[start.clone(), wrong_tool])); + let mut wrong_logical_id = finish; + wrong_logical_id.logical_invocation_id = Some("other-invocation".into()); + assert!(unclosed(&[start, wrong_logical_id])); +} + +/// The unclosed-call check must run over the full retained history, not only +/// the bounded summary window: an unfinished mutation pushed past the +/// model-facing tail by later completed calls still blocks a resume. +#[test] +fn unfinished_mutation_beyond_summary_window_still_blocks_continuation() { + let store = store(600); + let session = create_session(&store, "old unfinished write"); + add_instruction(&store, &session, "attempt"); + let started = store.record_tool_call_started( + Some(&session), + SessionTransport::Api, + "synthetic_write", + &json!({"project": PROJECT, "paths": ["src/old.rs"]}), + synthetic_contract(false, true, SessionPathHint::PathList), + ); + let started_event_id = started.as_ref().unwrap().event_id.clone(); + // Completed reads push the mutation start past the 200-event summary + // window while the store still retains it. + for index in 0..220 { + record_read( + &store, + &session, + &format!("filler-{index}.rs"), + json!({"ok": true}), + ); + } + let summary = store.summary(&session, Some(200)).unwrap(); + assert!(summary.events_truncated, "window must be exceeded"); + assert!(!summary.retention_truncated, "start must still be retained"); + assert!( + !summary + .events + .iter() + .any(|event| event.event_id == started_event_id), + "start must be outside the returned window" + ); + + let unconfirmed = feedback_for(&store, &session); + assert_eq!(unconfirmed["attempt"]["outcome"]["status"], "blocked"); + assert!(unconfirmed["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution"))); + + // Confirming the old mutation clears the block. + store.record_tool_call_finished(started, true, &json!({"applied": true}), None, None); + assert!( + !feedback_for(&store, &session)["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution")) + ); +} + +/// A finished mutation beyond the window must not keep the attempt blocked; +/// only genuinely unfinished ones do. +#[test] +fn finished_mutation_beyond_summary_window_does_not_block_continuation() { + let store = store(600); + let session = create_session(&store, "old finished write"); + add_instruction(&store, &session, "attempt"); + let started = store.record_tool_call_started( + Some(&session), + SessionTransport::Api, + "synthetic_write", + &json!({"project": PROJECT, "paths": ["src/old.rs"]}), + synthetic_contract(false, true, SessionPathHint::PathList), + ); + store.record_tool_call_finished(started, true, &json!({"applied": true}), None, None); + for index in 0..220 { + record_read( + &store, + &session, + &format!("filler-{index}.rs"), + json!({"ok": true}), + ); + } + let confirmed = feedback_for(&store, &session); + assert!(!confirmed["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution"))); +} + +/// Two mutations beyond the window: finishing one must not confirm the other. +#[test] +fn unfinished_mutation_beyond_window_is_not_confirmed_by_sibling_finish() { + let store = store(600); + let session = create_session(&store, "sibling writes"); + add_instruction(&store, &session, "attempt"); + let first = store.record_tool_call_started( + Some(&session), + SessionTransport::Api, + "synthetic_write", + &json!({"project": PROJECT, "paths": ["src/first.rs"]}), + synthetic_contract(false, true, SessionPathHint::PathList), + ); + let second = store.record_tool_call_started( + Some(&session), + SessionTransport::Api, + "synthetic_write", + &json!({"project": PROJECT, "paths": ["src/second.rs"]}), + synthetic_contract(false, true, SessionPathHint::PathList), + ); + store.record_tool_call_finished(second, true, &json!({"applied": true}), None, None); + for index in 0..220 { + record_read( + &store, + &session, + &format!("filler-{index}.rs"), + json!({"ok": true}), + ); + } + let unconfirmed = feedback_for(&store, &session); + assert_eq!(unconfirmed["attempt"]["outcome"]["status"], "blocked"); + assert!(unconfirmed["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution"))); + store.record_tool_call_finished(first, true, &json!({"applied": true}), None, None); + assert!( + !feedback_for(&store, &session)["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution")) + ); +} + +#[test] +fn unfinished_shell_call_without_explicit_project_is_unconfirmed() { + let store = store(200); + let session = create_session(&store, "implicit shell"); + add_instruction(&store, &session, "run shell in default project"); + let mut contract = synthetic_contract(false, false, SessionPathHint::None); + contract.shell_like = true; + let started = store.record_tool_call_started( + Some(&session), + SessionTransport::Api, + "synthetic_shell", + &json!({"command": "echo test"}), + contract, + ); + assert!( + feedback_for(&store, &session)["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution")) + ); + store.record_tool_call_finished(started, true, &json!({"exit_code": 0}), None, None); + assert!( + !feedback_for(&store, &session)["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution")) + ); +} + +#[test] +fn unfinished_mutation_in_unbound_session_does_not_disappear() { + let store = store(200); + let session = store + .start_session(None, Some("unbound session".to_string())) + .session_id; + let started = store.record_tool_call_started( + Some(&session), + SessionTransport::Api, + "synthetic_write", + &json!({"project": PROJECT, "paths": ["src/unbound.rs"]}), + synthetic_contract(false, true, SessionPathHint::PathList), + ); + assert!( + feedback_for(&store, &session)["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution")) + ); + store.record_tool_call_finished(started, true, &json!({"applied": true}), None, None); + assert!( + !feedback_for(&store, &session)["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution")) + ); +} + +#[test] +fn unfinished_mutation_requires_exact_call_and_logical_invocation_identity() { + let store = store(200); + let session = create_session(&store, "exact invocation"); + add_instruction(&store, &session, "mutate"); + let args = json!({"project": PROJECT, "paths": ["src/identity.rs"]}); + let mut metadata = ToolCallRecorderMetadata::default(); + metadata.assign_logical_invocation(); + let start = store.record_tool_call_started_with_metadata( + Some(&session), + SessionTransport::Api, + "synthetic_write", + &args, + Some(PROJECT.to_string()), + metadata, + synthetic_contract(false, true, SessionPathHint::PathList), + ); + let mut wrong_role = start.clone().unwrap(); + wrong_role.logical_invocation_role = Some("business".to_string()); + store.record_tool_call_finished( + Some(wrong_role), + true, + &json!({"applied": true}), + None, + None, + ); + assert!( + feedback_for(&store, &session)["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution")) + ); + + let mut wrong_call = start.clone().unwrap(); + wrong_call.call_id.push_str("_different"); + store.record_tool_call_finished( + Some(wrong_call), + true, + &json!({"applied": true}), + None, + None, + ); + assert!( + feedback_for(&store, &session)["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution")) + ); + + let mut wrong_project = start.clone().unwrap(); + wrong_project.resolved_project = Some("different-project".to_string()); + store.record_tool_call_finished( + Some(wrong_project), + true, + &json!({"applied": true}), + None, + None, + ); + assert!( + feedback_for(&store, &session)["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution")) + ); + + store.record_tool_call_finished(start, true, &json!({"applied": true}), None, None); + assert!( + !feedback_for(&store, &session)["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution")) + ); +} + +#[test] +fn interrupted_read_only_call_does_not_block_continuation() { + let store = store(200); + let session = create_session(&store, "read-only"); + add_instruction(&store, &session, "inspect"); + store.record_tool_call_started( + Some(&session), + SessionTransport::Api, + "read_files", + &json!({"project": PROJECT, "items": [{"path": "src/lib.rs"}]}), + synthetic_contract(true, false, SessionPathHint::None), + ); + let feedback = feedback_for(&store, &session); + assert!(!feedback["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution"))); + assert_ne!(feedback["attempt"]["outcome"]["status"], "blocked"); +} + +#[test] +fn accepted_job_with_matching_tool_finish_uses_job_lifecycle_not_unknown_effect() { + let store = store(200); + let session = create_session(&store, "accepted job"); + add_instruction(&store, &session, "run a job"); + let start = store.record_tool_call_started( + Some(&session), + SessionTransport::Api, + "synthetic_write", + &json!({"project": PROJECT, "paths": ["src/job.rs"]}), + synthetic_contract(false, true, SessionPathHint::PathList), + ); + store.record_tool_call_finished( + start, + true, + &json!({ + "promoted_to_job": true, "job_id": "wc_job_Accepted123", + "job_status": "running", "execution_state": "running", + }), + None, + None, + ); + let feedback = feedback_with_jobs( + &store, + &session, + &json!({"active_count":1, "running_count":1, "recovering_count":0, + "terminal_pending_count":0, "truncated":false, + "recent":[{"status":"running", "job_id":"wc_job_Accepted123"}]}), + ); + assert!(!feedback["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution"))); + assert_eq!(feedback["attempt"]["jobs"]["recovery_state"], "active"); +} + +#[test] +fn incomplete_event_tail_is_not_evidence_of_no_prior_mutation() { + let store = store(20); + let session = create_session(&store, "historical evidence"); + add_instruction(&store, &session, "work"); + for index in 0..30 { + record_write(&store, &session, &[&format!("src/{index}.rs")]); + } + assert!(store.summary(&session, Some(200)).unwrap().events_truncated); + let feedback = feedback_for(&store, &session); + assert!(feedback["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("mutating_call_history_incomplete"))); + assert!(!feedback["attempt"]["outcome"]["reason_codes"] + .as_array() + .unwrap() + .contains(&json!("unconfirmed_tool_execution"))); +} + #[test] fn attempt_only_counts_events_after_the_last_task_instruction() { let store = store(200); diff --git a/crates/webcodex-workflow-session/src/model.rs b/crates/webcodex-workflow-session/src/model.rs index 28ae38e0b..6766e5725 100644 --- a/crates/webcodex-workflow-session/src/model.rs +++ b/crates/webcodex-workflow-session/src/model.rs @@ -1219,6 +1219,14 @@ pub struct SessionSummary { /// consumers that already interpret it as the returned-window base. #[serde(default)] pub first_retained_sequence: usize, + /// True when the full retained history contains a mutating tool call + /// without exactly one matching finish. The store computes this over all + /// retained events — not just this bounded summary window — so an + /// unfinished mutation older than the window still blocks a resume with + /// `unconfirmed_tool_execution`. Internal projection fact; never + /// model-facing state. + #[serde(skip_serializing)] + pub unclosed_mutating_call: bool, pub messages: SessionMessagesSummary, #[serde(skip_serializing_if = "Option::is_none")] pub project_instructions: Option, diff --git a/crates/webcodex-workflow-session/src/store.rs b/crates/webcodex-workflow-session/src/store.rs index 7b9c07d09..3d4ec857b 100644 --- a/crates/webcodex-workflow-session/src/store.rs +++ b/crates/webcodex-workflow-session/src/store.rs @@ -2719,6 +2719,13 @@ fn summarize_record( events_returned, events_truncated: observed_total > events_returned, first_retained_sequence: observed_total.saturating_sub(events_returned), + // Computed over the full retained history, not the bounded tail + // above: an unfinished mutation older than the summary window must + // still block a resume with `unconfirmed_tool_execution`. + unclosed_mutating_call: super::continuation::has_unclosed_mutating_call( + record.events.iter().map(Arc::as_ref), + &record.session_id, + ), project_instructions, messages: build_messages_summary(record), } diff --git a/crates/webcodex-workflow-session/src/store/borrowed_projection_tests.rs b/crates/webcodex-workflow-session/src/store/borrowed_projection_tests.rs index d1b02cfa7..1865d71c4 100644 --- a/crates/webcodex-workflow-session/src/store/borrowed_projection_tests.rs +++ b/crates/webcodex-workflow-session/src/store/borrowed_projection_tests.rs @@ -85,6 +85,10 @@ fn borrowed_summary_matches_parent_for_every_tail_and_wrapped_deque() { Some(200), Some(usize::MAX), ] { + assert_eq!( + summarize_record(&record, limit, None).unclosed_mutating_call, + reference_summary(&record, limit, None).unclosed_mutating_call, + ); assert_eq!( serde_json::to_value(summarize_record(&record, limit, None)).unwrap(), serde_json::to_value(reference_summary(&record, limit, None)).unwrap() @@ -125,6 +129,10 @@ fn borrowed_hot_cold_summary_and_path_evidence_are_identical_and_non_promoting() store.start_session(None, None); assert!(store.cold_payload_bytes_for_test(&id).is_some()); let before = store.status(); + assert_eq!( + store.summary(&id, Some(8)).unwrap().unclosed_mutating_call, + hot_summary.unclosed_mutating_call, + ); assert_eq!( serde_json::to_value(store.summary(&id, Some(8)).unwrap()).unwrap(), serde_json::to_value(hot_summary).unwrap() diff --git a/crates/webcodex-workflow-session/src/store/borrowed_projection_tests/reference.rs b/crates/webcodex-workflow-session/src/store/borrowed_projection_tests/reference.rs index 0b11bd660..e71c95bfb 100644 --- a/crates/webcodex-workflow-session/src/store/borrowed_projection_tests/reference.rs +++ b/crates/webcodex-workflow-session/src/store/borrowed_projection_tests/reference.rs @@ -85,6 +85,10 @@ pub(super) fn reference_summary( events_returned, events_truncated: observed_total > events_returned, first_retained_sequence: observed_total.saturating_sub(events_returned), + unclosed_mutating_call: crate::continuation::has_unclosed_mutating_call( + retained_events.iter(), + &record.session_id, + ), project_instructions, messages: build_messages_summary(record), } diff --git a/docs/reviews/2026-11-04-unfinished-call-recovery.md b/docs/reviews/2026-11-04-unfinished-call-recovery.md new file mode 100644 index 000000000..04e18a6b4 --- /dev/null +++ b/docs/reviews/2026-11-04-unfinished-call-recovery.md @@ -0,0 +1,78 @@ +# Unfinished-call recovery backport validation + +Date: 2026-11-04 UTC. + +## Scope and provenance + +Backport [upstream PR #1344](https://github.com/yyjeqhc/webcodex/pull/1344), +merged as `1fc31bc2b7fd4e457ec38f7c0167d62566743049`, onto the maintained +fork branch `integrate/reviewed-upstream-20261020` at +`998d85dc8a7f46ef9ca1dbac49933a7638b88ca8`. + +The production patch is unchanged from upstream. It projects unresolved +mutating or shell-like call starts across instruction boundaries, requires an +exact later terminal event, and prioritizes reconciliation without executing +or replaying anything. Incomplete retained history is reported explicitly. +No persistence format, authorization, Job lifecycle, dependency or deployment +change is included. + +The fork still uses JSON Session persistence. The original SQLite-named async +restore test is therefore adapted to a synchronous ledger-named test using +the existing `flush_persistence()` API. It flushes and drops the original +store, reopens the same ledger, resumes the exact Session, verifies the old +unfinished call still blocks continuation, then verifies its matching finish +clears the uncertainty. This is clean persistence/reload coverage, not a +crash or power-loss durability claim. + +## Frozen source identity + +- Production file Git blob: `d0f6fac4cd80b7468dd2ab941bc8dd97903959fd`. +- Regression file Git blob: `a1354140d7d3a05e065bd42a9caac3379e79dddf`. +- Two Rust files: 376 insertions, 1 deletion. +- These source blobs were unchanged throughout final validation. + +## Local validation + +Environment: isolated Linux x86_64 cloud workspace, official Rust +`1.99.1 (58cf8f9c3 2026-10-13)` and Cargo +`1.99.1 (5f94df478 2026-08-27)`. Toolchain paths were isolated from global +shell configuration. Cargo used the checked-in lockfile. + +| Check | Result | +| --- | --- | +| New ledger reload regression on the unchanged base plus candidate tests | Expected red: 0 passed, 1 failed; exit 101. Actual status was `in_progress`, expected `blocked`. | +| `cargo test --locked -p webcodex-workflow-session --lib` on candidate, fresh target directory | 305 passed, 0 failed, 10 existing ignored. | +| `cargo test --locked -p webcodex-workflow-session --lib continuation_tests` on candidate | 42 passed, 0 failed. | +| `cargo fmt --all -- --check` | Passed. | +| Workspace boundary checker | All 22 workspace packages passed. | +| Workspace boundary checker self-tests | 20 passed. | +| Patch forward/reverse checks and `git diff --check` | Passed. | + +An initial candidate test invocation accidentally reused the red-control target +directory without recompiling and returned the baseline's seven failing new +regressions. That run is excluded from acceptance. Rebuilding the candidate in +a separate, empty target directory passed the full package suite without any +source or test-expectation changes. The original test binary lacked the new +helper's symbols; the clean candidate binary contains them. The red-control +and final test binary SHA-256 values differ: + +- Red control: `30bfc6c1a0402a85da2caa44c1db927495c25eb852166e0b1c79952070a4c310`. +- Final candidate: `fc7609cff7e57719520b2b97c270dd3afc7f4016c3e156c0c6d39244494df90f`. + +Use separate Cargo target directories for these two worktrees. + +## Review and acceptance limits + +Independent static review found no blocking defect in the final Rust source. +It checked correlation identity, history truncation, read-only/replay-safe +behavior, older fork API compatibility, and the JSON restore test adaptation. + +The tests cover implicit and different Projects, unbound Sessions, call and +logical-role identity, read-only interruptions, accepted asynchronous Jobs, +history truncation, and restoration across a new instruction. Missing call IDs, +duplicate matching finishes, and wrong logical-invocation IDs are handled in +production but do not have dedicated new test cases in this backport. + +This is not a full Server integration, native Windows/macOS/ARM64, real Host, +or deployed Server/Runner acceptance result. Exact-head fork CI remains a +separate merge gate. No service restart, release, or deployment is included. diff --git a/docs/reviews/2026-11-07-full-history-recovery.md b/docs/reviews/2026-11-07-full-history-recovery.md new file mode 100644 index 000000000..44fbbadd4 --- /dev/null +++ b/docs/reviews/2026-11-07-full-history-recovery.md @@ -0,0 +1,80 @@ +# Full-history recovery backport validation + +Date: 2026-11-07 UTC. + +## Scope and provenance + +This change selects only [upstream PR #1374](https://github.com/yyjeqhc/webcodex/pull/1374), +merge `f855f32fd0741342a567a37e0576a73d341fffcb` and final PR head +`bc8e25d21df5ba7b340b1f14b79dc8ec635bdb8c`, to improve the existing +[fork PR #3](https://github.com/franksong2702/webcodex/pull/3). +The exact patch base is `bda7eb67fc493c501986cb0c344c8d246f672b04`, which +already contains the earlier #1344 backport above maintained branch base +`998d85dc8a7f46ef9ca1dbac49933a7638b88ca8`. +The [earlier validation report](2026-11-04-unfinished-call-recovery.md) is +historical evidence and remains unchanged. + +The comparison snapshot freezes upstream main at +`49604f7b8f5f7dc602eccbe225a71949dd3001eb` and stable `v0.5.0` at +`354541eaa729a1c4bb6f35d4add46a95db4fbca4`. This is not a wholesale main or stable +upgrade. Fork-only patches remain in the ancestry; fork PR #2 is unchanged. + +## Behavior and fork adaptation + +The production patch matches #1374: compute unfinished mutation evidence over +all retained events before slicing the model-facing summary, then consume that +internal fact in continuation projection. A reverse scan requires an exact, +later finish for the same Session, event-resolved Project, tool, call ID, +logical invocation ID, and role. One finish cannot certify two starts; +duplicate finishes or absent/empty call IDs fail closed, and optional identity +fields must match exactly. Read-only interruptions remain +nonblocking, accepted Jobs keep their separate lifecycle, and actual history +eviction still reports incomplete evidence. Nothing executes or replays a call. + +The scan remains bounded by retained Session history. It does not enlarge the +summary window or durable retention, create new authority, or change storage +format. The new summary field is not serialized. The fork retains its existing +JSON ledger, not upstream SQLite persistence. The existing JSON reload test is +strengthened to retain the unfinished call beyond the 200-event summary window, +flush and reopen the ledger, and confirm only the matching finish clears the +block. This proves clean reload, not crash or power-loss durability. + +Additional assertions check nonserialization, hot/cold projection parity, +reference-summary parity, duplicated starts/finishes, reused identities, +finish ordering, absent/empty call IDs, and wrong Session/tool/logical IDs. +Existing tests retain call/Project/role mismatch and Job lifecycle coverage. + +## Cloud validation + +Environment: isolated Linux x86_64 cloud checkout, Rust +`1.99.1 (58cf8f9c3 2026-10-13)` and Cargo +`1.99.1 (5f94df478 2026-08-27)`. Explicit isolated `CARGO_HOME`, +`RUSTUP_HOME`, and `PATH` were used with the checked-in lockfile and +`CARGO_BUILD_JOBS=4`. Red and green worktrees used distinct fresh target +directories; no baseline/candidate binary reuse. + +- Red control: unchanged `bda7eb67` production plus the new window regressions. + `cargo test --locked -p webcodex-workflow-session unfinished_mutation_beyond -- --nocapture` + executed two tests, both failed as expected with exit 101: actual + `in_progress`, expected `blocked` (316 filtered out). +- Final green: `cargo test --locked -p webcodex-workflow-session`: + 309 passed, 0 failed, 10 existing ignored; doc tests: 0. +- Server constructor/integration compile: `cargo check --locked -p webcodex --tests`: + passed. This is compile evidence, not Server test execution. +- Independent reviewer reruns with `--offline --locked`: + `cargo test --offline --locked -p webcodex-workflow-session --lib continuation_tests` + passed 46 tests; the corresponding `--lib borrowed_projection_tests` filter + passed 7 tests with 4 existing ignored. +- `cargo fmt --all -- --check`: passed. +- `bash scripts/workspace_boundary_check.sh --self-test`: 20 passed. +- `bash scripts/workspace_boundary_check.sh`: all 22 workspace packages passed. +- `python3 scripts/check_markdown_links.py`: passed. +- `git diff --check`: passed. + +## Acceptance limits + +The ignored tests remain unexecuted. This change has not received full Server +runtime test execution, real Host/MCP integration, native Windows/macOS/ARM64, +release, or deployed Server/Runner acceptance in this cloud run. Exact-head +remote CI and independent review remain separate gates. No service restart, +release, deployment, or upstream publication is included.