mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-13 23:14:17 +00:00
fabro(01KQT1V8JM80VEWG4ZQYJC885G): strengthen notice handling
Add typed run notice codes for internal emitters, type local pipe drain stream names, and only warn about skipped non-git worktrees when worktree mode was explicitly set to always.
This commit is contained in:
parent
dd63112b09
commit
bebbcf3829
19 changed files with 214 additions and 202 deletions
|
|
@ -14,7 +14,7 @@ use std::time::Duration;
|
|||
use anyhow::{Context, Result, bail};
|
||||
use chrono::{DateTime, Utc};
|
||||
use fabro_redact::redact_jsonl_line;
|
||||
use fabro_types::run_event::is_metadata_snapshot_compat_notice_code;
|
||||
use fabro_types::RunNoticeCode;
|
||||
use fabro_util::json::normalize_json_value;
|
||||
use fabro_util::terminal::Styles;
|
||||
use tokio::time;
|
||||
|
|
@ -801,7 +801,9 @@ fn format_event_pretty_value(envelope: &serde_json::Value, styles: &Styles) -> O
|
|||
}
|
||||
|
||||
fn is_metadata_snapshot_compat_notice(envelope: &serde_json::Value) -> bool {
|
||||
prop_str_field(envelope, "code").is_some_and(is_metadata_snapshot_compat_notice_code)
|
||||
prop_str_field(envelope, "code")
|
||||
.and_then(|code| code.parse::<RunNoticeCode>().ok())
|
||||
.is_some_and(RunNoticeCode::is_metadata_snapshot_compat)
|
||||
}
|
||||
|
||||
fn str_field<'a>(value: &'a serde_json::Value, key: &str) -> Option<&'a str> {
|
||||
|
|
@ -1150,14 +1152,27 @@ mod tests {
|
|||
#[test]
|
||||
fn pretty_run_notice_warn() {
|
||||
let styles = no_color_styles();
|
||||
let line = r#"{"ts":"2026-01-01T14:25:00Z","event":"run.notice","properties":{"level":"warn","code":"sandbox_cleanup_failed","message":"sandbox cleanup failed: boom"}}"#;
|
||||
let result = format_event_pretty(line, &styles).unwrap();
|
||||
let code = RunNoticeCode::SandboxCleanupFailed.to_string();
|
||||
let line = serde_json::json!({
|
||||
"ts": "2026-01-01T14:25:00Z",
|
||||
"event": "run.notice",
|
||||
"properties": {
|
||||
"level": "warn",
|
||||
"code": code,
|
||||
"message": "sandbox cleanup failed: boom",
|
||||
},
|
||||
})
|
||||
.to_string();
|
||||
let result = format_event_pretty(&line, &styles).unwrap();
|
||||
assert!(result.contains("Warning:"), "got: {result}");
|
||||
assert!(
|
||||
result.contains("sandbox cleanup failed: boom"),
|
||||
"got: {result}"
|
||||
);
|
||||
assert!(result.contains("[sandbox_cleanup_failed]"), "got: {result}");
|
||||
assert!(
|
||||
result.contains(&format!("[{}]", RunNoticeCode::SandboxCleanupFailed)),
|
||||
"got: {result}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -1235,13 +1250,31 @@ mod tests {
|
|||
fn pretty_stream_suppresses_metadata_compat_notice_only() {
|
||||
let styles = no_color_styles();
|
||||
let failed = r#"{"ts":"2026-01-01T14:25:00Z","event":"metadata.snapshot.failed","properties":{"phase":"checkpoint","branch":"fabro/meta","duration_ms":900,"failure_kind":"write","error":"write failed"}}"#;
|
||||
let compat_notice = r#"{"ts":"2026-01-01T14:25:01Z","event":"run.notice","properties":{"level":"warn","code":"checkpoint_metadata_write_failed","message":"legacy metadata warning"}}"#;
|
||||
let degraded_notice = r#"{"ts":"2026-01-01T14:25:02Z","event":"run.notice","properties":{"level":"warn","code":"checkpoint_metadata_degraded","message":"metadata snapshots disabled"}}"#;
|
||||
let compat_notice = serde_json::json!({
|
||||
"ts": "2026-01-01T14:25:01Z",
|
||||
"event": "run.notice",
|
||||
"properties": {
|
||||
"level": "warn",
|
||||
"code": RunNoticeCode::CheckpointMetadataWriteFailed,
|
||||
"message": "legacy metadata warning",
|
||||
},
|
||||
})
|
||||
.to_string();
|
||||
let degraded_notice = serde_json::json!({
|
||||
"ts": "2026-01-01T14:25:02Z",
|
||||
"event": "run.notice",
|
||||
"properties": {
|
||||
"level": "warn",
|
||||
"code": RunNoticeCode::CheckpointMetadataDegraded,
|
||||
"message": "metadata snapshots disabled",
|
||||
},
|
||||
})
|
||||
.to_string();
|
||||
let mut state = PrettyEventState::default();
|
||||
|
||||
assert!(format_event_pretty_streamed(failed, &styles, &mut state).is_some());
|
||||
assert!(format_event_pretty_streamed(compat_notice, &styles, &mut state).is_none());
|
||||
let degraded = format_event_pretty_streamed(degraded_notice, &styles, &mut state).unwrap();
|
||||
assert!(format_event_pretty_streamed(&compat_notice, &styles, &mut state).is_none());
|
||||
let degraded = format_event_pretty_streamed(°raded_notice, &styles, &mut state).unwrap();
|
||||
assert!(
|
||||
degraded.contains("metadata snapshots disabled"),
|
||||
"got: {degraded}"
|
||||
|
|
|
|||
|
|
@ -527,7 +527,7 @@ fn display_value(value: &Value) -> Option<String> {
|
|||
mod tests {
|
||||
use fabro_agent::AgentEvent;
|
||||
use fabro_types::{MetadataSnapshotFailureKind, MetadataSnapshotPhase, fixtures};
|
||||
use fabro_workflow::event::{Event, to_run_event};
|
||||
use fabro_workflow::event::{Event, RunNoticeCode, to_run_event};
|
||||
|
||||
use super::*;
|
||||
|
||||
|
|
@ -804,10 +804,11 @@ mod tests {
|
|||
fn round_trip_run_notice() {
|
||||
let event = Event::RunNotice {
|
||||
level: RunNoticeLevel::Warn,
|
||||
code: "sandbox_cleanup_failed".into(),
|
||||
code: RunNoticeCode::SandboxCleanupFailed.to_string(),
|
||||
message: "sandbox cleanup failed".into(),
|
||||
exec_output_tail: None,
|
||||
};
|
||||
let expected_code = RunNoticeCode::SandboxCleanupFailed.to_string();
|
||||
|
||||
let stored = to_run_event(&fixtures::RUN_1, &event);
|
||||
let parsed = from_run_event(&stored).unwrap();
|
||||
|
|
@ -817,7 +818,7 @@ mod tests {
|
|||
level: RunNoticeLevel::Warn,
|
||||
code,
|
||||
message,
|
||||
} if code == "sandbox_cleanup_failed" && message == "sandbox cleanup failed"
|
||||
} if code == expected_code && message == "sandbox cleanup failed"
|
||||
));
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -3,8 +3,7 @@
|
|||
reason = "sync CLI run-progress renderer: writes to std::io::stderr directly"
|
||||
)]
|
||||
|
||||
use fabro_types::RunEvent;
|
||||
use fabro_types::run_event::is_metadata_snapshot_compat_notice_code;
|
||||
use fabro_types::{RunEvent, RunNoticeCode};
|
||||
|
||||
mod event;
|
||||
mod info_display;
|
||||
|
|
@ -444,7 +443,10 @@ impl ProgressUI {
|
|||
message,
|
||||
} => {
|
||||
if self.saw_metadata_snapshot_failure
|
||||
&& is_metadata_snapshot_compat_notice_code(&code)
|
||||
&& code
|
||||
.parse::<RunNoticeCode>()
|
||||
.ok()
|
||||
.is_some_and(RunNoticeCode::is_metadata_snapshot_compat)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
|
@ -1208,7 +1210,7 @@ mod tests {
|
|||
|
||||
emit(&mut ui, Event::RunNotice {
|
||||
level: RunNoticeLevel::Warn,
|
||||
code: "sandbox_cleanup_failed".into(),
|
||||
code: RunNoticeCode::SandboxCleanupFailed.to_string(),
|
||||
message: "sandbox cleanup failed".into(),
|
||||
exec_output_tail: None,
|
||||
});
|
||||
|
|
@ -1279,13 +1281,13 @@ mod tests {
|
|||
});
|
||||
emit(&mut ui, Event::RunNotice {
|
||||
level: RunNoticeLevel::Warn,
|
||||
code: "checkpoint_metadata_write_failed".into(),
|
||||
code: RunNoticeCode::CheckpointMetadataWriteFailed.to_string(),
|
||||
message: "legacy metadata warning".into(),
|
||||
exec_output_tail: None,
|
||||
});
|
||||
emit(&mut ui, Event::RunNotice {
|
||||
level: RunNoticeLevel::Warn,
|
||||
code: "checkpoint_metadata_degraded".into(),
|
||||
code: RunNoticeCode::CheckpointMetadataDegraded.to_string(),
|
||||
message: "metadata snapshots are disabled for this run".into(),
|
||||
exec_output_tail: None,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -160,7 +160,7 @@ fn attach_replays_completed_detached_run() {
|
|||
----- stdout -----
|
||||
----- stderr -----
|
||||
Web UI: http://localhost:3000/runs/[ULID]
|
||||
Warning: Worktree mode requested but no Git repository was found; running without a worktree. [worktree_skipped_no_git]
|
||||
Warning: Worktree mode `always` requested but no Git repository was found; running without a worktree. [worktree_skipped_no_git]
|
||||
Sandbox: local (ready in [TIME])
|
||||
✓ Start [TIME]
|
||||
✓ Run Tests [TIME]
|
||||
|
|
@ -268,7 +268,7 @@ fn attach_before_completion_streams_to_finished_state() {
|
|||
----- stdout -----
|
||||
----- stderr -----
|
||||
Web UI: http://localhost:3000/runs/[ULID]
|
||||
Warning: Worktree mode requested but no Git repository was found; running without a worktree. [worktree_skipped_no_git]
|
||||
Warning: Worktree mode `always` requested but no Git repository was found; running without a worktree. [worktree_skipped_no_git]
|
||||
Sandbox: local (ready in [TIME])
|
||||
✓ start [DURATION]
|
||||
✓ wait [DURATION]
|
||||
|
|
@ -711,7 +711,7 @@ fn attach_json_errors_without_prompting_for_human_input() {
|
|||
"properties": {
|
||||
"code": "worktree_skipped_no_git",
|
||||
"level": "warn",
|
||||
"message": "Worktree mode requested but no Git repository was found; running without a worktree."
|
||||
"message": "Worktree mode `always` requested but no Git repository was found; running without a worktree."
|
||||
},
|
||||
"run_id": "[ULID]",
|
||||
"ts": "[TIMESTAMP]"
|
||||
|
|
|
|||
|
|
@ -684,7 +684,7 @@ fn dry_run_simple() {
|
|||
|
||||
Run: [ULID]
|
||||
Web UI: http://localhost:3000/runs/[ULID]
|
||||
Warning: Worktree mode requested but no Git repository was found; running without a worktree. [worktree_skipped_no_git]
|
||||
Warning: Worktree mode `always` requested but no Git repository was found; running without a worktree. [worktree_skipped_no_git]
|
||||
Sandbox: local (ready in [TIME])
|
||||
✓ Start [TIME]
|
||||
✓ Run Tests [TIME]
|
||||
|
|
|
|||
|
|
@ -21,7 +21,7 @@ fn dry_run_branching() {
|
|||
warning [node: implement]: Node 'implement' has goal_gate=true but no retry_target or fallback_retry_target (goal_gate_has_retry)
|
||||
Run: [ULID]
|
||||
Web UI: http://localhost:3000/runs/[ULID]
|
||||
Warning: Worktree mode requested but no Git repository was found; running without a worktree. [worktree_skipped_no_git]
|
||||
Warning: Worktree mode `always` requested but no Git repository was found; running without a worktree. [worktree_skipped_no_git]
|
||||
Sandbox: local (ready in [TIME])
|
||||
✓ Start [TIME]
|
||||
✓ Plan [TIME]
|
||||
|
|
@ -58,7 +58,7 @@ fn dry_run_conditions() {
|
|||
|
||||
Run: [ULID]
|
||||
Web UI: http://localhost:3000/runs/[ULID]
|
||||
Warning: Worktree mode requested but no Git repository was found; running without a worktree. [worktree_skipped_no_git]
|
||||
Warning: Worktree mode `always` requested but no Git repository was found; running without a worktree. [worktree_skipped_no_git]
|
||||
Sandbox: local (ready in [TIME])
|
||||
✓ start [TIME]
|
||||
✓ Decide [TIME]
|
||||
|
|
@ -93,7 +93,7 @@ fn dry_run_parallel() {
|
|||
|
||||
Run: [ULID]
|
||||
Web UI: http://localhost:3000/runs/[ULID]
|
||||
Warning: Worktree mode requested but no Git repository was found; running without a worktree. [worktree_skipped_no_git]
|
||||
Warning: Worktree mode `always` requested but no Git repository was found; running without a worktree. [worktree_skipped_no_git]
|
||||
Sandbox: local (ready in [TIME])
|
||||
✓ start [TIME]
|
||||
✓ Fork Work [TIME]
|
||||
|
|
@ -129,7 +129,7 @@ fn dry_run_styled() {
|
|||
|
||||
Run: [ULID]
|
||||
Web UI: http://localhost:3000/runs/[ULID]
|
||||
Warning: Worktree mode requested but no Git repository was found; running without a worktree. [worktree_skipped_no_git]
|
||||
Warning: Worktree mode `always` requested but no Git repository was found; running without a worktree. [worktree_skipped_no_git]
|
||||
Sandbox: local (ready in [TIME])
|
||||
✓ start [TIME]
|
||||
✓ Plan [TIME]
|
||||
|
|
@ -165,7 +165,7 @@ fn dry_run_legacy_tool() {
|
|||
|
||||
Run: [ULID]
|
||||
Web UI: http://localhost:3000/runs/[ULID]
|
||||
Warning: Worktree mode requested but no Git repository was found; running without a worktree. [worktree_skipped_no_git]
|
||||
Warning: Worktree mode `always` requested but no Git repository was found; running without a worktree. [worktree_skipped_no_git]
|
||||
Sandbox: local (ready in [TIME])
|
||||
✓ Start [TIME]
|
||||
✓ Echo [TIME]
|
||||
|
|
|
|||
|
|
@ -126,24 +126,14 @@ fn process_env_vars() -> Vec<(String, String)> {
|
|||
std::env::vars().collect()
|
||||
}
|
||||
|
||||
async fn drain_pipe<R>(mut pipe: Option<R>, stream: &'static str) -> String
|
||||
async fn drain_pipe<R>(mut pipe: Option<R>, stream: CommandOutputStream) -> String
|
||||
where
|
||||
R: AsyncRead + Unpin,
|
||||
{
|
||||
let mut buf = String::new();
|
||||
if let Some(ref mut reader) = pipe {
|
||||
if let Err(err) = reader.read_to_string(&mut buf).await {
|
||||
match stream {
|
||||
"stdout" => {
|
||||
tracing::warn!(error = %err, stream, "Failed to drain child stdout");
|
||||
}
|
||||
"stderr" => {
|
||||
tracing::warn!(error = %err, stream, "Failed to drain child stderr");
|
||||
}
|
||||
_ => {
|
||||
tracing::warn!(error = %err, stream, "Failed to drain child output");
|
||||
}
|
||||
}
|
||||
tracing::warn!(error = %err, ?stream, "Failed to drain child output");
|
||||
}
|
||||
}
|
||||
buf
|
||||
|
|
@ -302,8 +292,10 @@ impl Sandbox for LocalSandbox {
|
|||
// on child.wait().
|
||||
let stdout_pipe = child.stdout.take();
|
||||
let stderr_pipe = child.stderr.take();
|
||||
let stdout_task = tokio::spawn(async move { drain_pipe(stdout_pipe, "stdout").await });
|
||||
let stderr_task = tokio::spawn(async move { drain_pipe(stderr_pipe, "stderr").await });
|
||||
let stdout_task =
|
||||
tokio::spawn(async move { drain_pipe(stdout_pipe, CommandOutputStream::Stdout).await });
|
||||
let stderr_task =
|
||||
tokio::spawn(async move { drain_pipe(stderr_pipe, CommandOutputStream::Stderr).await });
|
||||
|
||||
let (termination, exit_code) = tokio::select! {
|
||||
status_result = child.wait() => {
|
||||
|
|
@ -752,7 +744,7 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
let output = drain_pipe(Some(FailingReader), "stdout").await;
|
||||
let output = drain_pipe(Some(FailingReader), CommandOutputStream::Stdout).await;
|
||||
|
||||
assert!(output.is_empty());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -72,7 +72,7 @@ pub use run::{
|
|||
pub use run_blob_id::RunBlobId;
|
||||
pub use run_event::{
|
||||
EventBody, ExecOutputTail, InterviewOption, MetadataSnapshotFailureKind, MetadataSnapshotPhase,
|
||||
RunEvent, RunNoticeLevel,
|
||||
RunEvent, RunNoticeCode, RunNoticeLevel,
|
||||
};
|
||||
pub use run_id::{RunId, fixtures};
|
||||
pub use run_projection::{PendingInterviewRecord, RunProjection, StageProjection, first_event_seq};
|
||||
|
|
|
|||
|
|
@ -1,17 +1,48 @@
|
|||
use serde::{Deserialize, Serialize};
|
||||
|
||||
/// Legacy `run.notice` codes paired with the new `metadata.snapshot.failed`
|
||||
/// event for backward compatibility. Display layers suppress these so the
|
||||
/// typed event renders without a duplicate raw warning.
|
||||
pub const NOTICE_CODE_CHECKPOINT_METADATA_WRITE_FAILED: &str = "checkpoint_metadata_write_failed";
|
||||
pub const NOTICE_CODE_CHECKPOINT_METADATA_PUSH_FAILED: &str = "checkpoint_metadata_push_failed";
|
||||
#[derive(
|
||||
Debug,
|
||||
Clone,
|
||||
Copy,
|
||||
PartialEq,
|
||||
Eq,
|
||||
Hash,
|
||||
Serialize,
|
||||
Deserialize,
|
||||
strum::Display,
|
||||
strum::EnumString,
|
||||
strum::IntoStaticStr,
|
||||
)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
#[strum(serialize_all = "snake_case")]
|
||||
pub enum RunNoticeCode {
|
||||
ArtifactCollectionFailed,
|
||||
ArtifactOffloadFailed,
|
||||
ArtifactSyncFailed,
|
||||
ArtifactUploadFailed,
|
||||
CheckpointMetadataDegraded,
|
||||
CheckpointMetadataPushFailed,
|
||||
CheckpointMetadataWriteFailed,
|
||||
DirtyWorktree,
|
||||
GitDiffFailed,
|
||||
GitPushFailed,
|
||||
GithubTokenFailed,
|
||||
ParallelBaseCheckpointFailed,
|
||||
PullRequestFailed,
|
||||
SandboxCleanupFailed,
|
||||
SandboxGitUnavailable,
|
||||
SandboxPreserved,
|
||||
WorktreeSkippedNoGit,
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn is_metadata_snapshot_compat_notice_code(code: &str) -> bool {
|
||||
matches!(
|
||||
code,
|
||||
NOTICE_CODE_CHECKPOINT_METADATA_WRITE_FAILED | NOTICE_CODE_CHECKPOINT_METADATA_PUSH_FAILED
|
||||
)
|
||||
impl RunNoticeCode {
|
||||
#[must_use]
|
||||
pub fn is_metadata_snapshot_compat(self) -> bool {
|
||||
matches!(
|
||||
self,
|
||||
Self::CheckpointMetadataWriteFailed | Self::CheckpointMetadataPushFailed
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(
|
||||
|
|
|
|||
|
|
@ -1366,7 +1366,7 @@ mod tests {
|
|||
for body in [
|
||||
EventBody::RunNotice(RunNoticeProps {
|
||||
level: RunNoticeLevel::Warn,
|
||||
code: "git_diff_failed".to_string(),
|
||||
code: RunNoticeCode::GitDiffFailed.to_string(),
|
||||
message: "git diff failed".to_string(),
|
||||
exec_output_tail: Some(tail.clone()),
|
||||
}),
|
||||
|
|
@ -1402,7 +1402,7 @@ mod tests {
|
|||
for body in [
|
||||
EventBody::RunNotice(RunNoticeProps {
|
||||
level: RunNoticeLevel::Warn,
|
||||
code: "git_diff_failed".to_string(),
|
||||
code: RunNoticeCode::GitDiffFailed.to_string(),
|
||||
message: "git diff failed".to_string(),
|
||||
exec_output_tail: None,
|
||||
}),
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ mod stored_fields;
|
|||
#[cfg(test)]
|
||||
mod test_support;
|
||||
|
||||
pub use fabro_types::{EventBody, RunNoticeLevel};
|
||||
pub use fabro_types::{EventBody, RunNoticeCode, RunNoticeLevel};
|
||||
|
||||
pub use self::convert::{to_run_event, to_run_event_at};
|
||||
pub use self::emitter::Emitter;
|
||||
|
|
|
|||
|
|
@ -1167,8 +1167,8 @@ mod tests {
|
|||
use std::collections::BTreeMap;
|
||||
|
||||
use ::fabro_types::{
|
||||
EventBody, FailureReason, ParallelBranchId, Principal, RunNoticeLevel, RunProvenance,
|
||||
StageId, SystemActorKind, fixtures, run_event as fabro_types,
|
||||
EventBody, FailureReason, ParallelBranchId, Principal, RunNoticeCode, RunNoticeLevel,
|
||||
RunProvenance, StageId, SystemActorKind, fixtures, run_event as fabro_types,
|
||||
};
|
||||
use chrono::Utc;
|
||||
use fabro_agent::{AgentEvent, SandboxEvent};
|
||||
|
|
@ -1616,7 +1616,7 @@ mod tests {
|
|||
fn run_notice_maps_exec_output_tail_to_props() {
|
||||
let stored = to_run_event(&fixtures::RUN_1, &Event::RunNotice {
|
||||
level: RunNoticeLevel::Warn,
|
||||
code: "git_diff_failed".to_string(),
|
||||
code: RunNoticeCode::GitDiffFailed.to_string(),
|
||||
message: "git diff failed".to_string(),
|
||||
exec_output_tail: Some(exec_tail()),
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
use std::sync::Arc;
|
||||
use std::sync::atomic::{AtomicI64, Ordering};
|
||||
|
||||
use ::fabro_types::{ExecOutputTail, RunEvent, RunId, RunNoticeLevel};
|
||||
use ::fabro_types::{ExecOutputTail, RunEvent, RunId, RunNoticeCode, RunNoticeLevel};
|
||||
use chrono::Utc;
|
||||
use fabro_agent::{WorktreeEvent, WorktreeEventCallback};
|
||||
|
||||
|
|
@ -76,15 +76,10 @@ impl Emitter {
|
|||
self.emit_with_scope(event, Some(scope));
|
||||
}
|
||||
|
||||
pub fn notice(
|
||||
&self,
|
||||
level: RunNoticeLevel,
|
||||
code: impl Into<String>,
|
||||
message: impl Into<String>,
|
||||
) {
|
||||
pub fn notice(&self, level: RunNoticeLevel, code: RunNoticeCode, message: impl Into<String>) {
|
||||
self.emit(&Event::RunNotice {
|
||||
level,
|
||||
code: code.into(),
|
||||
code: code.to_string(),
|
||||
message: message.into(),
|
||||
exec_output_tail: None,
|
||||
});
|
||||
|
|
@ -93,13 +88,13 @@ impl Emitter {
|
|||
pub fn notice_with_tail(
|
||||
&self,
|
||||
level: RunNoticeLevel,
|
||||
code: impl Into<String>,
|
||||
code: RunNoticeCode,
|
||||
message: impl Into<String>,
|
||||
exec_output_tail: Option<ExecOutputTail>,
|
||||
) {
|
||||
self.emit(&Event::RunNotice {
|
||||
level,
|
||||
code: code.into(),
|
||||
code: code.to_string(),
|
||||
message: message.into(),
|
||||
exec_output_tail,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@ use tokio::sync::Semaphore;
|
|||
use super::{EngineServices, Handler};
|
||||
use crate::context::{Context, WorkflowContext, keys};
|
||||
use crate::error::Error;
|
||||
use crate::event::{Event, RunNoticeLevel, StageScope};
|
||||
use crate::event::{Event, RunNoticeCode, RunNoticeLevel, StageScope};
|
||||
use crate::git::sanitize_ref_component;
|
||||
use crate::hook_context::set_hook_node;
|
||||
use crate::millis_u64;
|
||||
|
|
@ -209,7 +209,7 @@ impl Handler for ParallelHandler {
|
|||
);
|
||||
services.run.emitter.notice_with_tail(
|
||||
RunNoticeLevel::Warn,
|
||||
"parallel_base_checkpoint_failed",
|
||||
RunNoticeCode::ParallelBaseCheckpointFailed,
|
||||
format!("Could not checkpoint base state before parallel branches: {e}"),
|
||||
fabro_sandbox::default_redacted_output_tail(&e),
|
||||
);
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ use tokio::time::sleep;
|
|||
use crate::artifact::{normalize_durable_updates, offload_large_values, sync_artifacts_to_env};
|
||||
use crate::artifact_snapshot::collect_artifacts;
|
||||
use crate::artifact_upload::ArtifactSink;
|
||||
use crate::event::{Emitter, Event, RunNoticeLevel};
|
||||
use crate::event::{Emitter, Event, RunNoticeCode, RunNoticeLevel};
|
||||
use crate::graph::{WorkflowGraph, WorkflowNode};
|
||||
use crate::lifecycle::event::{stage_scope_for, stage_visit};
|
||||
use crate::outcome::BilledModelUsage;
|
||||
|
|
@ -125,7 +125,7 @@ impl RunLifecycle<WorkflowGraph> for ArtifactLifecycle {
|
|||
{
|
||||
self.emitter.notice(
|
||||
RunNoticeLevel::Warn,
|
||||
"artifact_upload_failed",
|
||||
RunNoticeCode::ArtifactUploadFailed,
|
||||
format!("[node: {node_id}] artifact upload failed: {err}"),
|
||||
);
|
||||
return Ok(());
|
||||
|
|
@ -151,7 +151,7 @@ impl RunLifecycle<WorkflowGraph> for ArtifactLifecycle {
|
|||
Err(e) => {
|
||||
self.emitter.notice(
|
||||
RunNoticeLevel::Warn,
|
||||
"artifact_collection_failed",
|
||||
RunNoticeCode::ArtifactCollectionFailed,
|
||||
format!("[node: {node_id}] artifact collection failed: {e}"),
|
||||
);
|
||||
}
|
||||
|
|
@ -174,7 +174,7 @@ impl RunLifecycle<WorkflowGraph> for ArtifactLifecycle {
|
|||
{
|
||||
self.emitter.notice(
|
||||
RunNoticeLevel::Warn,
|
||||
"artifact_offload_failed",
|
||||
RunNoticeCode::ArtifactOffloadFailed,
|
||||
format!("[node: {node_id}] artifact offload failed: {e}"),
|
||||
);
|
||||
}
|
||||
|
|
@ -187,7 +187,7 @@ impl RunLifecycle<WorkflowGraph> for ArtifactLifecycle {
|
|||
{
|
||||
self.emitter.notice(
|
||||
RunNoticeLevel::Warn,
|
||||
"artifact_sync_failed",
|
||||
RunNoticeCode::ArtifactSyncFailed,
|
||||
format!("[node: {node_id}] artifact sync failed: {e}"),
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ use fabro_util::error::collect_causes;
|
|||
use fabro_util::time::elapsed_ms;
|
||||
|
||||
use crate::artifact;
|
||||
use crate::event::{Emitter, Event, RunNoticeLevel, StageScope};
|
||||
use crate::event::{Emitter, Event, RunNoticeCode, RunNoticeLevel, StageScope};
|
||||
use crate::graph::{WorkflowGraph, WorkflowNode};
|
||||
use crate::lifecycle::event::stage_scope_for;
|
||||
use crate::outcome::BilledModelUsage;
|
||||
|
|
@ -128,7 +128,10 @@ impl RunLifecycle<WorkflowGraph> for GitLifecycle {
|
|||
None,
|
||||
None,
|
||||
);
|
||||
self.emit_metadata_warning("checkpoint_metadata_write_failed", message);
|
||||
self.emit_metadata_warning(
|
||||
RunNoticeCode::CheckpointMetadataWriteFailed,
|
||||
message,
|
||||
);
|
||||
}
|
||||
},
|
||||
Err(err) => {
|
||||
|
|
@ -145,7 +148,10 @@ impl RunLifecycle<WorkflowGraph> for GitLifecycle {
|
|||
None,
|
||||
None,
|
||||
);
|
||||
self.emit_metadata_warning("checkpoint_metadata_write_failed", message);
|
||||
self.emit_metadata_warning(
|
||||
RunNoticeCode::CheckpointMetadataWriteFailed,
|
||||
message,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -217,7 +223,7 @@ impl RunLifecycle<WorkflowGraph> for GitLifecycle {
|
|||
Some(&scope),
|
||||
);
|
||||
self.emit_metadata_warning(
|
||||
"checkpoint_metadata_write_failed",
|
||||
RunNoticeCode::CheckpointMetadataWriteFailed,
|
||||
message,
|
||||
);
|
||||
None
|
||||
|
|
@ -239,7 +245,10 @@ impl RunLifecycle<WorkflowGraph> for GitLifecycle {
|
|||
None,
|
||||
Some(&scope),
|
||||
);
|
||||
self.emit_metadata_warning("checkpoint_metadata_write_failed", message);
|
||||
self.emit_metadata_warning(
|
||||
RunNoticeCode::CheckpointMetadataWriteFailed,
|
||||
message,
|
||||
);
|
||||
None
|
||||
}
|
||||
}
|
||||
|
|
@ -294,7 +303,7 @@ impl RunLifecycle<WorkflowGraph> for GitLifecycle {
|
|||
);
|
||||
self.emitter.notice_with_tail(
|
||||
RunNoticeLevel::Warn,
|
||||
"git_push_failed",
|
||||
RunNoticeCode::GitPushFailed,
|
||||
format!("Failed to push run branch {branch}: {err}"),
|
||||
exec_output_tail.clone(),
|
||||
);
|
||||
|
|
@ -327,7 +336,7 @@ impl RunLifecycle<WorkflowGraph> for GitLifecycle {
|
|||
fabro_sandbox::default_redacted_output_tail(&err);
|
||||
self.emitter.notice_with_tail(
|
||||
RunNoticeLevel::Warn,
|
||||
"git_diff_failed",
|
||||
RunNoticeCode::GitDiffFailed,
|
||||
format!("[node: {node_id}] git diff failed: {err}"),
|
||||
exec_output_tail,
|
||||
);
|
||||
|
|
@ -401,7 +410,10 @@ impl GitLifecycle {
|
|||
Some(snapshot.bytes),
|
||||
scope,
|
||||
);
|
||||
self.emit_metadata_warning("checkpoint_metadata_push_failed", message);
|
||||
self.emit_metadata_warning(
|
||||
RunNoticeCode::CheckpointMetadataPushFailed,
|
||||
message,
|
||||
);
|
||||
} else {
|
||||
self.emit_metadata_snapshot_completed(
|
||||
phase,
|
||||
|
|
@ -427,7 +439,7 @@ impl GitLifecycle {
|
|||
None,
|
||||
scope,
|
||||
);
|
||||
self.emit_metadata_warning("checkpoint_metadata_write_failed", message);
|
||||
self.emit_metadata_warning(RunNoticeCode::CheckpointMetadataWriteFailed, message);
|
||||
None
|
||||
}
|
||||
}
|
||||
|
|
@ -512,14 +524,9 @@ impl GitLifecycle {
|
|||
}
|
||||
}
|
||||
|
||||
fn emit_metadata_warning(&self, code: &str, message: String) {
|
||||
fn emit_metadata_warning(&self, code: RunNoticeCode, message: String) {
|
||||
if self.metadata_runtime.mark_metadata_degraded() {
|
||||
self.emitter.emit(&Event::RunNotice {
|
||||
level: RunNoticeLevel::Warn,
|
||||
code: code.to_string(),
|
||||
message,
|
||||
exec_output_tail: None,
|
||||
});
|
||||
self.emitter.notice(RunNoticeLevel::Warn, code, message);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ use fabro_util::time::elapsed_ms;
|
|||
|
||||
use super::types::{Concluded, FinalizeOptions, Retroed};
|
||||
use crate::error::Error;
|
||||
use crate::event::{Event, RunNoticeLevel};
|
||||
use crate::event::{Event, RunNoticeCode, RunNoticeLevel};
|
||||
use crate::outcome::{Outcome, OutcomeExt, StageOutcome};
|
||||
use crate::records::{Checkpoint, Conclusion, StageSummary};
|
||||
use crate::run_metadata::MetadataSnapshot;
|
||||
|
|
@ -235,7 +235,11 @@ pub async fn write_finalize_commit(
|
|||
None,
|
||||
None,
|
||||
);
|
||||
emit_metadata_warning(services, "checkpoint_metadata_write_failed", message);
|
||||
emit_metadata_warning(
|
||||
services,
|
||||
RunNoticeCode::CheckpointMetadataWriteFailed,
|
||||
message,
|
||||
);
|
||||
return;
|
||||
}
|
||||
};
|
||||
|
|
@ -256,7 +260,11 @@ pub async fn write_finalize_commit(
|
|||
None,
|
||||
None,
|
||||
);
|
||||
emit_metadata_warning(services, "checkpoint_metadata_write_failed", message);
|
||||
emit_metadata_warning(
|
||||
services,
|
||||
RunNoticeCode::CheckpointMetadataWriteFailed,
|
||||
message,
|
||||
);
|
||||
return;
|
||||
}
|
||||
};
|
||||
|
|
@ -277,7 +285,11 @@ pub async fn write_finalize_commit(
|
|||
Some(snapshot.entry_count),
|
||||
Some(snapshot.bytes),
|
||||
);
|
||||
emit_metadata_warning(services, "checkpoint_metadata_push_failed", message);
|
||||
emit_metadata_warning(
|
||||
services,
|
||||
RunNoticeCode::CheckpointMetadataPushFailed,
|
||||
message,
|
||||
);
|
||||
} else {
|
||||
emit_metadata_snapshot_completed(services, phase, meta_branch, started, &snapshot);
|
||||
}
|
||||
|
|
@ -296,7 +308,11 @@ pub async fn write_finalize_commit(
|
|||
None,
|
||||
None,
|
||||
);
|
||||
emit_metadata_warning(services, "checkpoint_metadata_write_failed", message);
|
||||
emit_metadata_warning(
|
||||
services,
|
||||
RunNoticeCode::CheckpointMetadataWriteFailed,
|
||||
message,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -359,7 +375,7 @@ fn emit_metadata_snapshot_failed(
|
|||
});
|
||||
}
|
||||
|
||||
fn emit_metadata_warning(services: &RunServices, code: &str, message: String) {
|
||||
fn emit_metadata_warning(services: &RunServices, code: RunNoticeCode, message: String) {
|
||||
if services.metadata_runtime.mark_metadata_degraded() {
|
||||
services.emitter.notice(RunNoticeLevel::Warn, code, message);
|
||||
}
|
||||
|
|
@ -383,7 +399,7 @@ async fn compute_final_patch(
|
|||
Err(err) => {
|
||||
services.emitter.notice(
|
||||
RunNoticeLevel::Warn,
|
||||
"git_diff_failed",
|
||||
RunNoticeCode::GitDiffFailed,
|
||||
format!("final diff failed: {err}"),
|
||||
);
|
||||
None
|
||||
|
|
@ -534,7 +550,7 @@ pub async fn finalize(retroed: Retroed, options: &FinalizeOptions) -> Result<Con
|
|||
if services.metadata_runtime.metadata_degraded() {
|
||||
services.emitter.notice(
|
||||
RunNoticeLevel::Warn,
|
||||
"checkpoint_metadata_degraded",
|
||||
RunNoticeCode::CheckpointMetadataDegraded,
|
||||
"checkpoint metadata archive writes were degraded for this run".to_string(),
|
||||
);
|
||||
}
|
||||
|
|
@ -556,9 +572,11 @@ pub async fn finalize(retroed: Retroed, options: &FinalizeOptions) -> Result<Con
|
|||
} else {
|
||||
format!("sandbox preserved: {info}")
|
||||
};
|
||||
services
|
||||
.emitter
|
||||
.notice(RunNoticeLevel::Info, "sandbox_preserved", message);
|
||||
services.emitter.notice(
|
||||
RunNoticeLevel::Info,
|
||||
RunNoticeCode::SandboxPreserved,
|
||||
message,
|
||||
);
|
||||
}
|
||||
if let Err(e) = cleanup_sandbox(
|
||||
&services,
|
||||
|
|
@ -572,7 +590,7 @@ pub async fn finalize(retroed: Retroed, options: &FinalizeOptions) -> Result<Con
|
|||
let exec_output_tail = fabro_sandbox::default_redacted_output_tail(&e);
|
||||
services.emitter.notice_with_tail(
|
||||
RunNoticeLevel::Warn,
|
||||
"sandbox_cleanup_failed",
|
||||
RunNoticeCode::SandboxCleanupFailed,
|
||||
format!("sandbox cleanup failed: {}", e.display_with_causes()),
|
||||
exec_output_tail,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ use tokio::time::timeout as tokio_timeout;
|
|||
use super::types::{InitOptions, Initialized, LlmSpec, Persisted, SandboxEnvSpec};
|
||||
use crate::devcontainer_bridge::{devcontainer_to_snapshot_config, run_devcontainer_lifecycle};
|
||||
use crate::error::Error;
|
||||
use crate::event::{Emitter, Event, RunNoticeLevel};
|
||||
use crate::event::{Emitter, Event, RunNoticeCode, RunNoticeLevel};
|
||||
use crate::git::RUN_BRANCH_PREFIX;
|
||||
use crate::handler::llm::{AgentApiBackend, AgentCliBackend, BackendRouter};
|
||||
use crate::handler::{HandlerRegistry, default_registry, sandbox_cancel_token};
|
||||
|
|
@ -155,7 +155,7 @@ fn resolve_worktree_plan(options: &mut InitOptions) -> Option<WorktreePlan> {
|
|||
if let Some(env_name) = env_name {
|
||||
options.emitter.notice(
|
||||
RunNoticeLevel::Warn,
|
||||
"dirty_worktree",
|
||||
RunNoticeCode::DirtyWorktree,
|
||||
format!("Uncommitted changes will not be included in the {env_name}."),
|
||||
);
|
||||
}
|
||||
|
|
@ -200,6 +200,14 @@ fn resolve_worktree_plan(options: &mut InitOptions) -> Option<WorktreePlan> {
|
|||
})
|
||||
}
|
||||
|
||||
fn worktree_skipped_notice(mode: Option<WorktreeMode>) -> Option<(RunNoticeCode, &'static str)> {
|
||||
matches!(mode, Some(WorktreeMode::Always)).then_some((
|
||||
RunNoticeCode::WorktreeSkippedNoGit,
|
||||
"Worktree mode `always` requested but no Git repository was found; running without a \
|
||||
worktree.",
|
||||
))
|
||||
}
|
||||
|
||||
fn git_setup_intent(run_options: &RunOptions) -> GitSetupIntent {
|
||||
if let Some(source) = run_options.fork_source_ref.as_ref() {
|
||||
GitSetupIntent::ForkFromCheckpoint {
|
||||
|
|
@ -243,7 +251,7 @@ async fn build_sandbox_env(
|
|||
tracing::warn!(error = %e, "Failed to mint GitHub token");
|
||||
emitter.notice(
|
||||
RunNoticeLevel::Warn,
|
||||
"github_token_failed",
|
||||
RunNoticeCode::GithubTokenFailed,
|
||||
format!("Failed to mint GitHub token: {e}"),
|
||||
);
|
||||
}
|
||||
|
|
@ -519,16 +527,13 @@ pub async fn initialize(
|
|||
))
|
||||
};
|
||||
if worktree_plan.is_some() && !worktree_created {
|
||||
tracing::warn!(
|
||||
worktree_mode = ?options.worktree_mode,
|
||||
"worktree requested but cwd is not a git repository; running without a worktree"
|
||||
);
|
||||
options.emitter.notice(
|
||||
RunNoticeLevel::Warn,
|
||||
"worktree_skipped_no_git",
|
||||
"Worktree mode requested but no Git repository was found; running without a \
|
||||
worktree.",
|
||||
);
|
||||
if let Some((code, message)) = worktree_skipped_notice(options.worktree_mode) {
|
||||
tracing::warn!(
|
||||
worktree_mode = ?options.worktree_mode,
|
||||
"worktree skipped: cwd is not a git repository"
|
||||
);
|
||||
options.emitter.notice(RunNoticeLevel::Warn, code, message);
|
||||
}
|
||||
options.run_options.git = None;
|
||||
}
|
||||
let cleanup_guard = scopeguard::guard(Arc::clone(&sandbox), |sandbox| {
|
||||
|
|
@ -637,7 +642,7 @@ pub async fn initialize(
|
|||
if sandbox_has_origin {
|
||||
options.emitter.notice(
|
||||
RunNoticeLevel::Warn,
|
||||
"sandbox_git_unavailable",
|
||||
RunNoticeCode::SandboxGitUnavailable,
|
||||
"Sandbox could not set up Git despite a configured origin; running \
|
||||
without checkpointing or PR support.",
|
||||
);
|
||||
|
|
@ -725,7 +730,7 @@ pub async fn initialize(
|
|||
if metadata_runtime.mark_metadata_degraded() {
|
||||
options.emitter.notice(
|
||||
RunNoticeLevel::Warn,
|
||||
"checkpoint_metadata_write_failed",
|
||||
RunNoticeCode::CheckpointMetadataWriteFailed,
|
||||
message,
|
||||
);
|
||||
}
|
||||
|
|
@ -1034,87 +1039,15 @@ mod tests {
|
|||
assert!(options.run_options.git.is_none());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn initialize_emits_worktree_skipped_no_git_in_non_git_cwd() {
|
||||
let temp = tempfile::tempdir().unwrap();
|
||||
let run_dir = temp.path().join("run");
|
||||
std::fs::create_dir_all(&run_dir).unwrap();
|
||||
// Non-git working directory: a tmpdir without a `.git` parent.
|
||||
let cwd = temp.path().join("cwd");
|
||||
std::fs::create_dir_all(&cwd).unwrap();
|
||||
#[test]
|
||||
fn worktree_skipped_notice_only_warns_for_always() {
|
||||
assert!(worktree_skipped_notice(None).is_none());
|
||||
assert!(worktree_skipped_notice(Some(WorktreeMode::Clean)).is_none());
|
||||
assert!(worktree_skipped_notice(Some(WorktreeMode::Dirty)).is_none());
|
||||
assert!(worktree_skipped_notice(Some(WorktreeMode::Never)).is_none());
|
||||
|
||||
let (graph, source) = simple_graph();
|
||||
let persisted = test_persisted(graph, source, &run_dir);
|
||||
let emitter = Arc::new(crate::event::Emitter::new(test_run_id()));
|
||||
let seen = Arc::new(std::sync::Mutex::new(Vec::new()));
|
||||
emitter.on_event({
|
||||
let seen = Arc::clone(&seen);
|
||||
move |event| seen.lock().unwrap().push(event.clone())
|
||||
});
|
||||
|
||||
// The notice we assert below is emitted before any later sandbox/setup work
|
||||
// that could fail in this minimal fixture (no real
|
||||
// git/devcontainer/lifecycle), so the overall result is intentionally
|
||||
// ignored — the assertion runs against the captured event stream
|
||||
// regardless of how `initialize` ultimately resolves.
|
||||
let _ = initialize(persisted, InitOptions {
|
||||
run_id: test_run_id(),
|
||||
run_store: {
|
||||
let store = memory_store();
|
||||
let inner = store.create_run(&test_run_id()).await.unwrap();
|
||||
inner.into()
|
||||
},
|
||||
dry_run: false,
|
||||
emitter,
|
||||
sandbox: SandboxSpec::Local {
|
||||
working_directory: cwd,
|
||||
},
|
||||
llm: LlmSpec {
|
||||
model: "test-model".to_string(),
|
||||
provider: fabro_llm::Provider::Anthropic,
|
||||
fallback_chain: Vec::new(),
|
||||
mcp_servers: Vec::new(),
|
||||
dry_run: true,
|
||||
},
|
||||
interviewer: Arc::new(AutoApproveInterviewer::engine()),
|
||||
lifecycle: crate::run_options::LifecycleOptions {
|
||||
setup_commands: vec![],
|
||||
setup_command_timeout_ms: 1_000,
|
||||
devcontainer_phases: vec![],
|
||||
},
|
||||
run_options: test_settings(&run_dir),
|
||||
workflow_path: None,
|
||||
workflow_bundle: None,
|
||||
hooks: fabro_hooks::HookSettings { hooks: vec![] },
|
||||
sandbox_env: SandboxEnvSpec {
|
||||
devcontainer_env: HashMap::new(),
|
||||
toml_env: HashMap::new(),
|
||||
github_permissions: None,
|
||||
origin_url: None,
|
||||
},
|
||||
vault: None,
|
||||
devcontainer: None,
|
||||
git: None,
|
||||
worktree_mode: Some(WorktreeMode::Always),
|
||||
run_control: None,
|
||||
registry_override: None,
|
||||
artifact_sink: None,
|
||||
checkpoint: None,
|
||||
seed_context: None,
|
||||
})
|
||||
.await;
|
||||
|
||||
let events = seen.lock().unwrap().clone();
|
||||
let notice = events
|
||||
.iter()
|
||||
.find_map(|event| match &event.body {
|
||||
EventBody::RunNotice(props) if props.code == "worktree_skipped_no_git" => {
|
||||
Some(props.clone())
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.expect("worktree_skipped_no_git notice");
|
||||
assert!(matches!(notice.level, fabro_types::RunNoticeLevel::Warn));
|
||||
let (code, _) = worktree_skipped_notice(Some(WorktreeMode::Always)).unwrap();
|
||||
assert_eq!(code, RunNoticeCode::WorktreeSkippedNoGit);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
|
|
|||
|
|
@ -13,7 +13,7 @@ use fabro_util::text::strip_goal_decoration;
|
|||
use tracing::{debug, info};
|
||||
|
||||
use super::types::{Concluded, Finalized, PullRequestOptions};
|
||||
use crate::event::{Event, RunNoticeLevel};
|
||||
use crate::event::{Event, RunNoticeCode, RunNoticeLevel};
|
||||
use crate::outcome::{StageOutcome, format_cost as outcome_format_cost};
|
||||
use crate::records::{Conclusion, RunSpec};
|
||||
use crate::runtime_store::RunStoreHandle;
|
||||
|
|
@ -603,7 +603,7 @@ pub async fn pull_request(concluded: Concluded, options: &PullRequestOptions) ->
|
|||
.emit(&Event::PullRequestFailed { error: e.clone() });
|
||||
services.emitter.notice(
|
||||
RunNoticeLevel::Warn,
|
||||
"pull_request_failed",
|
||||
RunNoticeCode::PullRequestFailed,
|
||||
format!("PR creation failed: {e}"),
|
||||
);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue