mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
* feat: structured fallback deliverables for failed/stuck jobs (#221) When a job fails or gets stuck, build a FallbackDeliverable that captures partial results, action statistics, cost, timing, and repair attempts. This replaces opaque error strings with structured data users can act on. - Add FallbackDeliverable, LastAction, ActionStats types in context/fallback.rs - Store fallback in JobContext.metadata["fallback_deliverable"] on failure - Surface fallback in job_status tool output and SSE job_result events - Update mark_failed() and mark_stuck() in worker to build fallback - 8 unit tests covering zero/mixed actions, truncation, timing, serialization Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address review comments on fallback deliverables - Fix doc comment: "200 chars" -> "200 bytes (UTF-8 safe)" since truncate_str operates on byte length, not character count. - Add code comment documenting that SSE fallback_deliverable is currently always None (forward-compatible infrastructure). Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor: take Option<&FallbackDeliverable> instead of &Option<…> Addresses Gemini review feedback: idiomatic Rust prefers Option<&T> over &Option<T> for borrowed optional values. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: guard against non-object metadata and add fallback test - store_fallback_in_metadata now resets metadata to {} when it's any non-object type (string, array, number), not just null. Prevents panic on index assignment. - Add test_job_status_includes_fallback_deliverable to verify the fallback field is surfaced in job_status tool output. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: use sanitized output in fallback preview + add integration tests Security fix: FallbackDeliverable::build() now uses output_sanitized instead of output_raw, preventing secrets/PII from leaking through the job_status tool and SSE job_result events. Also adds: - test_fallback_uses_sanitized_output: proves raw secrets don't leak - test_store_fallback_in_metadata_roundtrip: full serialize/deserialize - test_store_fallback_handles_non_object_metadata: edge case coverage - test_store_fallback_none_is_noop: None input is safe Addresses serrrfirat review feedback on PR #236. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: harden fallback deliverables against review findings - Truncate failure_reason to 1000 bytes to prevent metadata bloat - Add tracing::warn on fallback serialization failure (was silently discarded) - Fix module/struct docs to cover stuck jobs, remove stale SSE claim - Fix job.rs test to use real FallbackDeliverable field names - Add tests for failure_reason truncation and completed_at=None elapsed time - Fix pre-existing clippy warning in settings.rs (field_reassign_with_default) Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address Copilot review findings on fallback deliverables - Fix output_raw/output_sanitized field swap in ActionRecord::succeed() so sanitized data actually goes into the sanitized field (security) - Return None instead of empty Memory when get_memory fails in build_fallback, with tracing::warn for observability - Replace manual elapsed calculation with ctx.elapsed() which already clamps negative durations Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: resolve rebase conflicts and update tests for parameter swap - Add fallback field to SseEvent::JobResult in job_monitor - Fix type annotation in fallback deliverable test - Update test_action_record_succeed_sets_fields for new parameter order - Use create_job_for_user in test (API changed on main) Co-Authored-By: Claude Opus 4.6 <[email protected]> * chore: trigger CI re-check after rebase * fix: fall back to error message for failed action output_preview When the last action is a failed tool call, output_sanitized is None, leaving output_preview empty. Now falls back to the action's error message so users see what went wrong. [skip-regression-check] * ci: add safety comments to test code for no-panics check The CI no-panics grep check cannot distinguish test code inside src/ files from production code. Add // safety: test annotations to .unwrap(), .expect(), and assert!() calls in #[cfg(test)] modules. * fix: clarify succeed() doc and avoid clone in output_preview - Fix doc comment: output_raw is stored as pretty-printed JSON string, not a raw JSON value - Borrow string slice directly in fallback preview to avoid cloning potentially large sanitized outputs before truncation * refactor: reuse floor_char_boundary in truncate_str Replace hand-rolled UTF-8 boundary logic with existing crate::util::floor_char_boundary to reduce duplication. * fix: rename SSE fallback field to fallback_deliverable for consistency The SSE JobResult field was named `fallback` while everywhere else (metadata key, job_status tool) uses `fallback_deliverable`. Align the SSE wire format to avoid forcing clients to handle two names. --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
320 lines
12 KiB
Rust
320 lines
12 KiB
Rust
//! Structured fallback deliverables for failed or stuck jobs.
|
|
//!
|
|
//! When a job fails or is detected as stuck, a [`FallbackDeliverable`] captures
|
|
//! what was accomplished before the failure: partial results, action statistics,
|
|
//! cost, and timing. This gives users visibility into terminal jobs instead of
|
|
//! just an error string.
|
|
//!
|
|
//! Fallback deliverables are stored in `JobContext.metadata["fallback_deliverable"]`
|
|
//! and surfaced through the `job_status` tool.
|
|
|
|
use serde::{Deserialize, Serialize};
|
|
|
|
use crate::context::memory::Memory;
|
|
use crate::context::state::JobContext;
|
|
|
|
/// Structured summary of a failed or stuck job.
|
|
///
|
|
/// Stored in `JobContext.metadata["fallback_deliverable"]` when a job fails
|
|
/// or is marked stuck. Surfaced through the `job_status` tool.
|
|
#[derive(Debug, Clone, Serialize, Deserialize)]
|
|
pub struct FallbackDeliverable {
|
|
/// True if at least one action succeeded before failure.
|
|
pub partial: bool,
|
|
/// Why the job failed.
|
|
pub failure_reason: String,
|
|
/// Last action taken before failure.
|
|
pub last_action: Option<LastAction>,
|
|
/// Aggregate action statistics.
|
|
pub action_stats: ActionStats,
|
|
/// Total tokens consumed.
|
|
pub tokens_used: u64,
|
|
/// Total cost incurred (decimal as string for JSON safety).
|
|
pub cost: String,
|
|
/// Wall-clock elapsed time in seconds.
|
|
pub elapsed_secs: f64,
|
|
/// Number of self-repair attempts.
|
|
pub repair_attempts: u32,
|
|
}
|
|
|
|
/// Summary of the last action taken before failure.
|
|
#[derive(Debug, Clone, Serialize, Deserialize)]
|
|
pub struct LastAction {
|
|
pub tool_name: String,
|
|
/// Truncated to 200 bytes (UTF-8 safe).
|
|
pub output_preview: String,
|
|
pub success: bool,
|
|
}
|
|
|
|
/// Aggregate action counts.
|
|
#[derive(Debug, Clone, Serialize, Deserialize)]
|
|
pub struct ActionStats {
|
|
pub total: u32,
|
|
pub successful: u32,
|
|
pub failed: u32,
|
|
}
|
|
|
|
impl FallbackDeliverable {
|
|
/// Build a fallback deliverable from a job context and its memory.
|
|
pub fn build(ctx: &JobContext, memory: &Memory, reason: &str) -> Self {
|
|
let successful = memory.successful_actions() as u32;
|
|
let failed = memory.failed_actions() as u32;
|
|
let total = memory.actions.len() as u32;
|
|
|
|
let last_action = memory.last_action().map(|a| {
|
|
// Use sanitized output to avoid leaking secrets through the fallback API surface.
|
|
// For failed actions (no sanitized output), fall back to the error message.
|
|
// Borrow the string slice directly when possible to avoid cloning
|
|
// potentially large outputs just for truncation.
|
|
let owned_fallback;
|
|
let preview_str: &str = if let Some(v) = a.output_sanitized.as_ref() {
|
|
match v {
|
|
serde_json::Value::String(s) => s.as_str(),
|
|
other => {
|
|
owned_fallback = serde_json::to_string(other).unwrap_or_default();
|
|
&owned_fallback
|
|
}
|
|
}
|
|
} else if let Some(ref err) = a.error {
|
|
err.as_str()
|
|
} else {
|
|
""
|
|
};
|
|
let preview = truncate_str(preview_str, 200);
|
|
LastAction {
|
|
tool_name: a.tool_name.clone(),
|
|
output_preview: preview.to_string(),
|
|
success: a.success,
|
|
}
|
|
});
|
|
|
|
let elapsed_secs = ctx.elapsed().map_or(0.0, |d| d.as_secs_f64());
|
|
|
|
Self {
|
|
partial: successful > 0,
|
|
failure_reason: truncate_str(reason, 1000).to_string(),
|
|
last_action,
|
|
action_stats: ActionStats {
|
|
total,
|
|
successful,
|
|
failed,
|
|
},
|
|
tokens_used: ctx.total_tokens_used,
|
|
cost: ctx.actual_cost.to_string(),
|
|
elapsed_secs,
|
|
repair_attempts: ctx.repair_attempts,
|
|
}
|
|
}
|
|
}
|
|
|
|
/// Truncate a string to at most `max_len` bytes on a char boundary.
|
|
fn truncate_str(s: &str, max_len: usize) -> &str {
|
|
&s[..crate::util::floor_char_boundary(s, max_len)]
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::*;
|
|
use crate::context::memory::Memory;
|
|
use crate::context::state::JobContext;
|
|
use chrono::{Duration, Utc};
|
|
use rust_decimal::Decimal;
|
|
use std::time::Duration as StdDuration;
|
|
|
|
#[test]
|
|
fn test_fallback_zero_actions() {
|
|
let ctx = JobContext::new("Test", "Empty job");
|
|
let memory = Memory::new(ctx.job_id);
|
|
|
|
let fb = FallbackDeliverable::build(&ctx, &memory, "timed out");
|
|
|
|
assert!(!fb.partial); // safety: test
|
|
assert_eq!(fb.failure_reason, "timed out"); // safety: test
|
|
assert!(fb.last_action.is_none()); // safety: test
|
|
assert_eq!(fb.action_stats.total, 0); // safety: test
|
|
assert_eq!(fb.action_stats.successful, 0); // safety: test
|
|
assert_eq!(fb.action_stats.failed, 0); // safety: test
|
|
assert_eq!(fb.tokens_used, 0); // safety: test
|
|
assert_eq!(fb.cost, "0"); // safety: test
|
|
assert_eq!(fb.repair_attempts, 0); // safety: test
|
|
}
|
|
|
|
#[test]
|
|
fn test_fallback_mixed_actions() {
|
|
let mut ctx = JobContext::new("Test", "Mixed job");
|
|
ctx.total_tokens_used = 5000;
|
|
ctx.actual_cost = Decimal::new(42, 2); // 0.42
|
|
ctx.repair_attempts = 1;
|
|
|
|
let mut memory = Memory::new(ctx.job_id);
|
|
|
|
// 3 successes
|
|
for _ in 0..3 {
|
|
let action = memory
|
|
.create_action("tool_a", serde_json::json!({}))
|
|
.succeed(
|
|
Some("output".to_string()),
|
|
serde_json::json!({}),
|
|
StdDuration::from_secs(1),
|
|
);
|
|
memory.record_action(action);
|
|
}
|
|
// 2 failures
|
|
for _ in 0..2 {
|
|
let action = memory
|
|
.create_action("tool_b", serde_json::json!({}))
|
|
.fail("broke", StdDuration::from_secs(1));
|
|
memory.record_action(action);
|
|
}
|
|
|
|
let fb = FallbackDeliverable::build(&ctx, &memory, "max iterations");
|
|
|
|
assert!(fb.partial); // safety: test
|
|
assert_eq!(fb.action_stats.total, 5); // safety: test
|
|
assert_eq!(fb.action_stats.successful, 3); // safety: test
|
|
assert_eq!(fb.action_stats.failed, 2); // safety: test
|
|
assert_eq!(fb.tokens_used, 5000); // safety: test
|
|
assert_eq!(fb.cost, "0.42"); // safety: test
|
|
assert_eq!(fb.repair_attempts, 1); // safety: test
|
|
assert!(fb.last_action.is_some()); // safety: test
|
|
let la = fb.last_action.unwrap(); // safety: test
|
|
assert_eq!(la.tool_name, "tool_b"); // safety: test
|
|
assert!(!la.success); // safety: test
|
|
// Failed actions should surface the error message as the output preview
|
|
assert_eq!(la.output_preview, "broke"); // safety: test
|
|
}
|
|
|
|
#[test]
|
|
fn test_fallback_failed_action_shows_error() {
|
|
let ctx = JobContext::new("Test", "Error preview");
|
|
let mut memory = Memory::new(ctx.job_id);
|
|
|
|
let action = memory
|
|
.create_action("broken_tool", serde_json::json!({}))
|
|
.fail("connection timed out after 30s", StdDuration::from_secs(30));
|
|
memory.record_action(action);
|
|
|
|
let fb = FallbackDeliverable::build(&ctx, &memory, "tool failure");
|
|
let la = fb.last_action.unwrap(); // safety: test
|
|
assert!(!la.success); // safety: test
|
|
assert_eq!(la.output_preview, "connection timed out after 30s"); // safety: test
|
|
}
|
|
|
|
#[test]
|
|
fn test_fallback_last_action_truncation() {
|
|
let ctx = JobContext::new("Test", "Truncation");
|
|
let mut memory = Memory::new(ctx.job_id);
|
|
|
|
let long_output = "x".repeat(500);
|
|
let action = memory
|
|
.create_action("tool_c", serde_json::json!({}))
|
|
.succeed(
|
|
Some(long_output.clone()),
|
|
serde_json::Value::String(long_output),
|
|
StdDuration::from_secs(1),
|
|
);
|
|
memory.record_action(action);
|
|
|
|
let fb = FallbackDeliverable::build(&ctx, &memory, "failed");
|
|
let la = fb.last_action.unwrap(); // safety: test
|
|
assert!(la.output_preview.len() <= 200); // safety: test
|
|
assert!(!la.output_preview.is_empty()); // safety: test
|
|
}
|
|
|
|
#[test]
|
|
fn test_fallback_uses_sanitized_output() {
|
|
let ctx = JobContext::new("Test", "Sanitized");
|
|
let mut memory = Memory::new(ctx.job_id);
|
|
|
|
let action = memory
|
|
.create_action("tool_d", serde_json::json!({}))
|
|
.succeed(
|
|
Some("[REDACTED]".to_string()),
|
|
serde_json::json!({"api_key": "sk-secret-key-12345"}),
|
|
StdDuration::from_secs(1),
|
|
);
|
|
memory.record_action(action);
|
|
|
|
let fb = FallbackDeliverable::build(&ctx, &memory, "failed");
|
|
let la = fb.last_action.unwrap(); // safety: test
|
|
// Must use sanitized output, not raw
|
|
assert!(!la.output_preview.contains("sk-secret")); // safety: test
|
|
assert!(la.output_preview.contains("REDACTED")); // safety: test
|
|
}
|
|
|
|
#[test]
|
|
fn test_fallback_elapsed_time() {
|
|
let mut ctx = JobContext::new("Test", "Timing");
|
|
let now = Utc::now();
|
|
ctx.started_at = Some(now - Duration::seconds(10));
|
|
ctx.completed_at = Some(now);
|
|
|
|
let memory = Memory::new(ctx.job_id);
|
|
let fb = FallbackDeliverable::build(&ctx, &memory, "failed");
|
|
|
|
// Should be approximately 10 seconds
|
|
assert!((fb.elapsed_secs - 10.0).abs() < 0.1); // safety: test
|
|
}
|
|
|
|
#[test]
|
|
fn test_fallback_no_started_at() {
|
|
let ctx = JobContext::new("Test", "Never started");
|
|
let memory = Memory::new(ctx.job_id);
|
|
|
|
let fb = FallbackDeliverable::build(&ctx, &memory, "failed");
|
|
assert!((fb.elapsed_secs - 0.0).abs() < 0.001); // safety: test
|
|
}
|
|
|
|
#[test]
|
|
fn test_fallback_elapsed_time_no_completed_at() {
|
|
let mut ctx = JobContext::new("Test", "Still running");
|
|
ctx.started_at = Some(Utc::now() - Duration::seconds(5));
|
|
// completed_at is None — should use Utc::now() as fallback
|
|
|
|
let memory = Memory::new(ctx.job_id);
|
|
let fb = FallbackDeliverable::build(&ctx, &memory, "stuck");
|
|
|
|
// Should be approximately 5 seconds (using now as end time)
|
|
assert!(fb.elapsed_secs >= 4.0 && fb.elapsed_secs <= 7.0); // safety: test
|
|
}
|
|
|
|
#[test]
|
|
fn test_fallback_failure_reason_truncation() {
|
|
let ctx = JobContext::new("Test", "Long reason");
|
|
let memory = Memory::new(ctx.job_id);
|
|
|
|
let long_reason = "x".repeat(5000);
|
|
let fb = FallbackDeliverable::build(&ctx, &memory, &long_reason);
|
|
|
|
assert!(fb.failure_reason.len() <= 1000); // safety: test
|
|
assert!(!fb.failure_reason.is_empty()); // safety: test
|
|
}
|
|
|
|
#[test]
|
|
fn test_truncate_str_ascii() {
|
|
assert_eq!(truncate_str("hello", 10), "hello"); // safety: test
|
|
assert_eq!(truncate_str("hello world", 5), "hello"); // safety: test
|
|
}
|
|
|
|
#[test]
|
|
fn test_truncate_str_unicode() {
|
|
// "é" is 2 bytes in UTF-8
|
|
let s = "café";
|
|
assert_eq!(truncate_str(s, 10), "café"); // safety: test
|
|
// Truncating at 4 would split "é", should back up to 3
|
|
assert_eq!(truncate_str(s, 4), "caf"); // safety: test
|
|
}
|
|
|
|
#[test]
|
|
fn test_fallback_serialization() {
|
|
let ctx = JobContext::new("Test", "Serialize");
|
|
let memory = Memory::new(ctx.job_id);
|
|
let fb = FallbackDeliverable::build(&ctx, &memory, "test error");
|
|
|
|
// Should serialize to JSON and back without error
|
|
let json = serde_json::to_value(&fb).unwrap(); // safety: test
|
|
let deserialized: FallbackDeliverable = serde_json::from_value(json).unwrap(); // safety: test
|
|
assert_eq!(deserialized.failure_reason, "test error"); // safety: test
|
|
}
|
|
}
|