scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-08-28 05:27:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

refactor: unify metadata snapshot failure helpers and drop floor_char_boundary copies

Browse source 
Groups the MetadataSnapshotFailed event payload into a MetadataSnapshotFailure
struct and replaces the two near-identical 11-arg emit_metadata_snapshot_failed
helpers in lifecycle/git.rs and pipeline/finalize.rs with one shared helper
in sandbox_metadata.rs. Both #[allow(too_many_arguments)] blocks are removed.

Also deletes two hand-written floor_char_boundary copies (fabro-agent and
fabro-sandbox) in favor of the stable str::floor_char_boundary, matching how
most existing call sites already use it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-05-01 09:17:47 -04:00
parent 96d7ba4aa0
commit 5437257521
No known key found for this signature in database
9 changed files with 137 additions and 181 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
lib/crates/fabro-agent/src/cli.rs
View file

@ -30,7 +30,7 @@ use crate::subagent::{SessionFactory, SubAgentManager};
use crate::tools::WebFetchSummarizer;
use crate::{
AgentEvent, AgentProfile, AnthropicProfile, GeminiProfile, LocalSandbox, OpenAiProfile,
Sandbox, Session, SessionOptions, Turn, truncation,
Sandbox, Session, SessionOptions, Turn,
};
/// Public arguments for the agent command, usable from an external CLI.
@ -288,7 +288,7 @@ fn format_tool_args(args: &serde_json::Value, cwd: &str) -> String {
serde_json::Value::String(s) => {
let s = s.strip_prefix(&cwd_prefix).unwrap_or(s);
let display = if s.len() > 80 {
format!("{}...", &s[..truncation::floor_char_boundary(s, 77)])
format!("{}...", &s[..s.floor_char_boundary(77)])
} else {
s.to_string()
};
@ -671,7 +671,7 @@ pub async fn run_with_args_and_client(
..
} => {
let task_preview = if task.len() > 60 {
&task[..truncation::floor_char_boundary(task, 60)]
&task[..task.floor_char_boundary(60)]
} else {
task
};

8
lib/crates/fabro-agent/src/compaction.rs
View file

@ -9,7 +9,6 @@ use crate::error::Error;
use crate::event::Emitter;
use crate::file_tracker::FileTracker;
use crate::history::History;
use crate::truncation;
use crate::types::{AgentEvent, Turn};
/// Check whether the context window usage exceeds the configured threshold.
@ -210,10 +209,7 @@ pub fn render_turns_for_summary(turns: &[Turn]) -> String {
for tc in tool_calls {
let args_str = tc.arguments.to_string();
let truncated = if args_str.len() > 500 {
format!(
"{}...",
&args_str[..truncation::floor_char_boundary(&args_str, 500)]
)
format!("{}...", &args_str[..args_str.floor_char_boundary(500)])
} else {
args_str
};
@ -226,7 +222,7 @@ pub fn render_turns_for_summary(turns: &[Turn]) -> String {
let truncated = if content_str.len() > 500 {
format!(
"{}...",
&content_str[..truncation::floor_char_boundary(&content_str, 500)]
&content_str[..content_str.floor_char_boundary(500)]
)
} else {
content_str

4
lib/crates/fabro-agent/src/lib.rs
View file

@ -54,9 +54,7 @@ pub use tools::{
WebFetchSummarizer, make_edit_file_tool, make_glob_tool, make_grep_tool, make_read_file_tool,
make_shell_tool, make_shell_tool_with_config, make_write_file_tool, register_core_tools,
};
pub use truncation::{
TruncationMode, floor_char_boundary, truncate_lines, truncate_output, truncate_tool_output,
};
pub use truncation::{TruncationMode, truncate_lines, truncate_output, truncate_tool_output};
pub use types::{AgentEvent, SessionEvent, SessionState, Turn};
#[cfg(test)]

37
lib/crates/fabro-agent/src/truncation.rs
View file

@ -1,19 +1,5 @@
use crate::config::SessionOptions;
/// Round a byte index down to the nearest UTF-8 char boundary.
/// Stable equivalent of `str::floor_char_boundary` (nightly-only).
#[must_use]
pub fn floor_char_boundary(s: &str, index: usize) -> usize {
if index >= s.len() {
return s.len();
}
let mut i = index;
while i > 0 && !s.is_char_boundary(i) {
i -= 1;
}
i
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum TruncationMode {
HeadTail,
@ -58,8 +44,8 @@ pub fn truncate_output(output: &str, max_chars: usize, mode: TruncationMode) ->
match mode {
TruncationMode::HeadTail => {
let half = max_chars / 2;
let head_end = floor_char_boundary(output, half);
let tail_start = floor_char_boundary(output, output.len() - half);
let head_end = output.floor_char_boundary(half);
let tail_start = output.floor_char_boundary(output.len() - half);
let head = &output[..head_end];
let tail = &output[tail_start..];
format!(
@ -69,7 +55,7 @@ pub fn truncate_output(output: &str, max_chars: usize, mode: TruncationMode) ->
)
}
TruncationMode::Tail => {
let tail_start = floor_char_boundary(output, output.len() - max_chars);
let tail_start = output.floor_char_boundary(output.len() - max_chars);
let tail = &output[tail_start..];
format!(
"[WARNING: Tool output was truncated. First {removed} characters were removed. \
@ -250,23 +236,6 @@ mod tests {
assert_eq!(result, output);
}
#[test]
fn floor_char_boundary_ascii() {
assert_eq!(floor_char_boundary("hello", 3), 3);
assert_eq!(floor_char_boundary("hello", 10), 5);
assert_eq!(floor_char_boundary("hello", 0), 0);
}
#[test]
fn floor_char_boundary_multibyte() {
// ✅ is 3 bytes (E2 9C 85)
let s = "a✅b";
assert_eq!(floor_char_boundary(s, 1), 1); // just past 'a'
assert_eq!(floor_char_boundary(s, 2), 1); // inside ✅, rounds down to 'a'
assert_eq!(floor_char_boundary(s, 3), 1); // still inside ✅
assert_eq!(floor_char_boundary(s, 4), 4); // at 'b'
}
#[test]
fn truncate_output_multibyte_no_panic() {
let output = "✅".repeat(100); // 300 bytes

13
lib/crates/fabro-sandbox/src/sandbox.rs
View file

@ -497,7 +497,7 @@ fn redacted_tail(text: &str, max_bytes: usize) -> (Option<String>, bool) {
let sanitized = sanitize_exec_output(&redacted);
let truncated = sanitized.len() > max_bytes;
let start = if truncated {
floor_char_boundary(&sanitized, sanitized.len() - max_bytes)
sanitized.floor_char_boundary(sanitized.len() - max_bytes)
} else {
0
};
@ -547,17 +547,6 @@ fn sanitize_exec_output(text: &str) -> String {
sanitized
}
fn floor_char_boundary(text: &str, index: usize) -> usize {
if index >= text.len() {
return text.len();
}
let mut boundary = index;
while boundary > 0 && !text.is_char_boundary(boundary) {
boundary -= 1;
}
boundary
}
#[derive(Debug, Clone)]
pub struct ExecStreamingResult {
pub result: ExecResult,

2
lib/crates/fabro-workflow/src/handler/agent.rs
View file

@ -194,7 +194,7 @@ pub(crate) fn truncate(s: &str, max_chars: usize) -> &str {
if s.len() <= max_chars {
s
} else {
&s[..fabro_agent::floor_char_boundary(s, max_chars)]
&s[..s.floor_char_boundary(max_chars)]
}
}

116
lib/crates/fabro-workflow/src/lifecycle/git.rs
View file

@ -21,7 +21,10 @@ use crate::run_dump::RunDump;
use crate::run_options::RunOptions;
use crate::runtime_store::RunStoreHandle;
use crate::sandbox_git::{checked_git_checkpoint, git_diff};
use crate::sandbox_metadata::{MetadataSnapshot, SandboxGitRuntime, SandboxMetadataWriter};
use crate::sandbox_metadata::{
MetadataSnapshot, MetadataSnapshotFailure, SandboxGitRuntime, SandboxMetadataWriter,
emit_metadata_snapshot_failed,
};
type WfRunState = ExecutionState<Option<BilledModelUsage>>;
type WfNodeResult = NodeResult<Option<BilledModelUsage>>;
@ -106,17 +109,20 @@ impl RunLifecycle<WorkflowGraph> for GitLifecycle {
}
Err(err) => {
let message = format!("failed to load run state for metadata init: {err}");
self.emit_metadata_snapshot_failed(
emit_metadata_snapshot_failed(
&self.emitter,
phase,
&meta_branch,
started,
MetadataSnapshotFailureKind::LoadState,
message.clone(),
collect_causes(err.as_ref()),
None,
None,
None,
None,
MetadataSnapshotFailure {
kind: MetadataSnapshotFailureKind::LoadState,
error: message.clone(),
causes: collect_causes(err.as_ref()),
commit_sha: None,
entry_count: None,
bytes: None,
exec_output_tail: None,
},
None,
);
self.emit_metadata_warning("checkpoint_metadata_write_failed", message);
@ -176,17 +182,20 @@ impl RunLifecycle<WorkflowGraph> for GitLifecycle {
Err(err) => {
let message =
format!("failed to load run state for metadata checkpoint: {err}");
self.emit_metadata_snapshot_failed(
emit_metadata_snapshot_failed(
&self.emitter,
phase,
&meta_branch,
started,
MetadataSnapshotFailureKind::LoadState,
message.clone(),
collect_causes(err.as_ref()),
None,
None,
None,
None,
MetadataSnapshotFailure {
kind: MetadataSnapshotFailureKind::LoadState,
error: message.clone(),
causes: collect_causes(err.as_ref()),
commit_sha: None,
entry_count: None,
bytes: None,
exec_output_tail: None,
},
Some(&scope),
);
self.emit_metadata_warning("checkpoint_metadata_write_failed", message);
@ -328,17 +337,20 @@ impl GitLifecycle {
let message = format!(
"failed to push metadata ref refs/heads/{meta_branch}: {push_error}"
);
self.emit_metadata_snapshot_failed(
emit_metadata_snapshot_failed(
&self.emitter,
phase,
meta_branch,
started,
MetadataSnapshotFailureKind::Push,
message.clone(),
Vec::new(),
Some(snapshot.commit_sha.clone()),
Some(snapshot.entry_count),
Some(snapshot.bytes),
push_error.exec_output_tail(),
MetadataSnapshotFailure {
kind: MetadataSnapshotFailureKind::Push,
error: message.clone(),
causes: Vec::new(),
commit_sha: Some(snapshot.commit_sha.clone()),
entry_count: Some(snapshot.entry_count),
bytes: Some(snapshot.bytes),
exec_output_tail: push_error.exec_output_tail(),
},
scope,
);
self.emit_metadata_warning("checkpoint_metadata_push_failed", message);
@ -355,17 +367,20 @@ impl GitLifecycle {
}
Err(err) => {
let message = format!("failed to write checkpoint metadata: {err}");
self.emit_metadata_snapshot_failed(
emit_metadata_snapshot_failed(
&self.emitter,
phase,
meta_branch,
started,
MetadataSnapshotFailureKind::Write,
message.clone(),
collect_causes(&err),
None,
None,
None,
err.exec_output_tail(),
MetadataSnapshotFailure {
kind: MetadataSnapshotFailureKind::Write,
error: message.clone(),
causes: collect_causes(&err),
commit_sha: None,
entry_count: None,
bytes: None,
exec_output_tail: err.exec_output_tail(),
},
scope,
);
self.emit_metadata_warning("checkpoint_metadata_write_failed", message);
@ -410,41 +425,6 @@ impl GitLifecycle {
);
}
#[allow(
clippy::too_many_arguments,
reason = "Metadata failure event carries the full event contract explicitly."
)]
fn emit_metadata_snapshot_failed(
&self,
phase: MetadataSnapshotPhase,
branch: &str,
started: Instant,
failure_kind: MetadataSnapshotFailureKind,
error: String,
causes: Vec<String>,
commit_sha: Option<String>,
entry_count: Option<usize>,
bytes: Option<u64>,
exec_output_tail: Option<fabro_types::ExecOutputTail>,
scope: Option<&StageScope>,
) {
self.emit_metadata_snapshot_event(
&Event::MetadataSnapshotFailed {
phase,
branch: branch.to_string(),
duration_ms: elapsed_ms(started),
failure_kind,
error,
causes,
commit_sha,
entry_count,
bytes,
exec_output_tail,
},
scope,
);
}
fn emit_metadata_snapshot_event(&self, event: &Event, scope: Option<&StageScope>) {
if let Some(scope) = scope {
self.emitter.emit_scoped(event, scope);

88
lib/crates/fabro-workflow/src/pipeline/finalize.rs
View file

@ -16,7 +16,9 @@ use crate::run_options::RunOptions;
use crate::run_status::{FailureReason, RunStatus, SuccessReason};
use crate::runtime_store::RunStoreHandle;
use crate::sandbox_git::git_diff_with_timeout;
use crate::sandbox_metadata::{MetadataSnapshot, SandboxMetadataWriter};
use crate::sandbox_metadata::{
MetadataSnapshot, MetadataSnapshotFailure, SandboxMetadataWriter, emit_metadata_snapshot_failed,
};
use crate::services::RunServices;
pub fn classify_engine_result(
@ -171,16 +173,19 @@ pub async fn write_finalize_commit(
Err(err) => {
let message = format!("failed to load run state for final metadata snapshot: {err}");
emit_metadata_snapshot_failed(
services,
&services.emitter,
phase,
meta_branch,
started,
MetadataSnapshotFailureKind::LoadState,
message.clone(),
collect_causes(err.as_ref()),
None,
None,
None,
MetadataSnapshotFailure {
kind: MetadataSnapshotFailureKind::LoadState,
error: message.clone(),
causes: collect_causes(err.as_ref()),
commit_sha: None,
entry_count: None,
bytes: None,
exec_output_tail: None,
},
None,
);
emit_metadata_warning(services, "checkpoint_metadata_write_failed", message);
@ -203,17 +208,20 @@ pub async fn write_finalize_commit(
let message =
format!("failed to push metadata ref refs/heads/{meta_branch}: {push_error}");
emit_metadata_snapshot_failed(
services,
&services.emitter,
phase,
meta_branch,
started,
MetadataSnapshotFailureKind::Push,
message.clone(),
Vec::new(),
Some(snapshot.commit_sha.clone()),
Some(snapshot.entry_count),
Some(snapshot.bytes),
push_error.exec_output_tail(),
MetadataSnapshotFailure {
kind: MetadataSnapshotFailureKind::Push,
error: message.clone(),
causes: Vec::new(),
commit_sha: Some(snapshot.commit_sha.clone()),
entry_count: Some(snapshot.entry_count),
bytes: Some(snapshot.bytes),
exec_output_tail: push_error.exec_output_tail(),
},
None,
);
emit_metadata_warning(services, "checkpoint_metadata_push_failed", message);
} else {
@ -223,17 +231,20 @@ pub async fn write_finalize_commit(
Err(err) => {
let message = format!("failed to write final checkpoint metadata: {err}");
emit_metadata_snapshot_failed(
services,
&services.emitter,
phase,
meta_branch,
started,
MetadataSnapshotFailureKind::Write,
message.clone(),
collect_causes(&err),
MetadataSnapshotFailure {
kind: MetadataSnapshotFailureKind::Write,
error: message.clone(),
causes: collect_causes(&err),
commit_sha: None,
entry_count: None,
bytes: None,
exec_output_tail: err.exec_output_tail(),
},
None,
None,
None,
err.exec_output_tail(),
);
emit_metadata_warning(services, "checkpoint_metadata_write_failed", message);
}
@ -268,37 +279,6 @@ fn emit_metadata_snapshot_completed(
});
}
#[allow(
clippy::too_many_arguments,
reason = "Metadata failure event carries the full event contract explicitly."
)]
fn emit_metadata_snapshot_failed(
services: &RunServices,
phase: MetadataSnapshotPhase,
branch: &str,
started: Instant,
failure_kind: MetadataSnapshotFailureKind,
error: String,
causes: Vec<String>,
commit_sha: Option<String>,
entry_count: Option<usize>,
bytes: Option<u64>,
exec_output_tail: Option<fabro_types::ExecOutputTail>,
) {
services.emitter.emit(&Event::MetadataSnapshotFailed {
phase,
branch: branch.to_string(),
duration_ms: elapsed_ms(started),
failure_kind,
error,
causes,
commit_sha,
entry_count,
bytes,
exec_output_tail,
});
}
fn emit_metadata_warning(services: &RunServices, code: &str, message: String) {
if services.metadata_runtime.mark_metadata_degraded() {
services.emitter.notice(RunNoticeLevel::Warn, code, message);

44
lib/crates/fabro-workflow/src/sandbox_metadata.rs
View file

@ -1,12 +1,16 @@
use std::collections::HashMap;
use std::fmt::Write as _;
use std::sync::atomic::{AtomicBool, Ordering};
use std::time::Instant;
use fabro_agent::Sandbox;
use fabro_sandbox::shell_quote;
use fabro_types::run_event::{MetadataSnapshotFailureKind, MetadataSnapshotPhase};
use fabro_util::time::elapsed_ms;
use tokio::fs;
use tokio::sync::OnceCell;
use crate::event::{Emitter, Event, StageScope};
use crate::git::{GitAuthor, META_BRANCH_PREFIX};
use crate::run_dump::RunDump;
use crate::sandbox_git::GIT_REMOTE;
@ -94,6 +98,46 @@ pub(crate) struct MetadataSnapshot {
pub bytes: u64,
}
/// Full payload for a `MetadataSnapshotFailed` event, grouped so the emit
/// helper stays under clippy's argument threshold.
#[derive(Debug)]
pub(crate) struct MetadataSnapshotFailure {
pub kind: MetadataSnapshotFailureKind,
pub error: String,
pub causes: Vec<String>,
pub commit_sha: Option<String>,
pub entry_count: Option<usize>,
pub bytes: Option<u64>,
pub exec_output_tail: Option<fabro_types::ExecOutputTail>,
}
pub(crate) fn emit_metadata_snapshot_failed(
emitter: &Emitter,
phase: MetadataSnapshotPhase,
branch: &str,
started: Instant,
failure: MetadataSnapshotFailure,
scope: Option<&StageScope>,
) {
let event = Event::MetadataSnapshotFailed {
phase,
branch: branch.to_string(),
duration_ms: elapsed_ms(started),
failure_kind: failure.kind,
error: failure.error,
causes: failure.causes,
commit_sha: failure.commit_sha,
entry_count: failure.entry_count,
bytes: failure.bytes,
exec_output_tail: failure.exec_output_tail,
};
if let Some(scope) = scope {
emitter.emit_scoped(&event, scope);
} else {
emitter.emit(&event);
}
}
impl<'a> SandboxMetadataWriter<'a> {
pub(crate) fn new(
sandbox: &'a dyn Sandbox,