diff --git a/crates/xgen-adapter-filesystem/src/apply_patch.rs b/crates/xgen-adapter-filesystem/src/apply_patch.rs index 8b03878..72cc1b2 100644 --- a/crates/xgen-adapter-filesystem/src/apply_patch.rs +++ b/crates/xgen-adapter-filesystem/src/apply_patch.rs @@ -9,14 +9,14 @@ use xgen_runtime::{ AdapterPrepareFailure, AdapterPrepareRequest, AdapterReconcileRequest, AdapterReconciliationInconclusiveReason, AdapterReconciliationObservation, AdapterToolOutput, EffectAdapter, EffectVerifier, PreparedAdapterInvocation, RuleVerificationObservation, - VerificationPortFailure, VerificationReport, VerificationRequest, VerifiedArtifactDescriptor, - VerifierOutputDigest, + StepFailureCode, VerificationPortFailure, VerificationReport, VerificationRequest, + VerifiedArtifactDescriptor, VerifierOutputDigest, }; use xgen_workgraph::EffectClass; use crate::atomic_commit::{AtomicCommitFailure, MAX_ATOMIC_TEXT_BYTES, commit_atomic}; use crate::path::{RelativePath, parse_canonical}; -use crate::read_text::{ReadTextLimits, read_text, sha256_digest}; +use crate::read_text::{ReadFailure, ReadTextLimits, read_text, sha256_digest}; use crate::{APPLY_PATCH_CAPABILITY_ID, APPLY_PATCH_CONTRACT_VERSION, WorkspaceRoot}; /// Maximum total UTF-8 bytes across every `oldText` and `newText` patch field. @@ -214,6 +214,7 @@ impl PreparedAdapterInvocation for PreparedApplyPatch { Err(failure) => AdapterExecutionObservation::Failed { evidence_digest: AdapterEvidenceDigest::new(failure.evidence_digest()) .expect("a SHA-256 digest is canonical"), + failure_code: failure.failure_code(), }, } } @@ -234,7 +235,7 @@ struct InspectedOutput { #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum PatchFailure { - ReadSource, + ReadSource(ReadFailure), PreconditionConflict, ContextMissing, ContextAmbiguous, @@ -246,7 +247,7 @@ enum PatchFailure { impl PatchFailure { fn evidence_digest(self) -> String { let code = match self { - Self::ReadSource => "read-source", + Self::ReadSource(_) => "read-source", Self::PreconditionConflict => "precondition-conflict", Self::ContextMissing => "context-missing", Self::ContextAmbiguous => "context-ambiguous", @@ -256,6 +257,18 @@ impl PatchFailure { }; sha256_digest(format!("xgeny.fs/apply-patch/failure/v1/{code}").as_bytes()) } + + const fn failure_code(self) -> StepFailureCode { + match self { + Self::ReadSource(failure) => failure.failure_code(), + Self::PreconditionConflict => StepFailureCode::PreconditionConflict, + Self::ContextMissing => StepFailureCode::PatchContextMissing, + Self::ContextAmbiguous => StepFailureCode::PatchContextAmbiguous, + Self::OverlappingEdits => StepFailureCode::PatchEditsOverlap, + Self::ResultTooLarge => StepFailureCode::TooLarge, + Self::Atomic(failure) => failure.failure_code(), + } + } } fn verify_contract( @@ -426,7 +439,7 @@ fn apply_patch_to_file( edits: &[PatchEdit], ) -> Result { let observed = read_text(root, relative_path, ReadTextLimits::default()) - .map_err(|_| PatchFailure::ReadSource)?; + .map_err(PatchFailure::ReadSource)?; if observed.digest != expected_digest { return Err(PatchFailure::PreconditionConflict); } @@ -575,29 +588,35 @@ mod tests { sha256_digest(b"stale"), vec![edit("two", "TWO")], PatchFailure::PreconditionConflict, + StepFailureCode::PreconditionConflict, ), ( "one two one", sha256_digest(b"one two one"), vec![edit("missing", "new")], PatchFailure::ContextMissing, + StepFailureCode::PatchContextMissing, ), ( "one two one", sha256_digest(b"one two one"), vec![edit("one", "ONE")], PatchFailure::ContextAmbiguous, + StepFailureCode::PatchContextAmbiguous, ), ( "abcdef", sha256_digest(b"abcdef"), vec![edit("abc", ""), edit("cde", "")], PatchFailure::OverlappingEdits, + StepFailureCode::PatchEditsOverlap, ), ]; - for (source, expected, edits, failure) in cases { + for (source, expected, edits, failure, code) in cases { let fixture = Fixture::new(source); - assert_eq!(fixture.apply(&expected, &edits).unwrap_err(), failure); + let observed = fixture.apply(&expected, &edits).unwrap_err(); + assert_eq!(observed, failure); + assert_eq!(observed.failure_code(), code); assert_eq!(fixture.content(), source); } } diff --git a/crates/xgen-adapter-filesystem/src/atomic_commit.rs b/crates/xgen-adapter-filesystem/src/atomic_commit.rs index 6f091d1..2aad9b0 100644 --- a/crates/xgen-adapter-filesystem/src/atomic_commit.rs +++ b/crates/xgen-adapter-filesystem/src/atomic_commit.rs @@ -6,7 +6,9 @@ use cap_fs_ext::OpenOptionsMaybeDirExt as _; use cap_fs_ext::{DirExt as _, FollowSymlinks, OpenOptionsFollowExt as _, OpenOptionsSyncExt as _}; use cap_std::fs::{Dir, File, OpenOptions, Permissions}; -use crate::path::RelativePath; +use xgen_runtime::StepFailureCode; + +use crate::path::{OpenErrorClass, RelativePath, classify_open_error}; use crate::read_text::sha256_digest; pub(crate) const MAX_ATOMIC_TEXT_BYTES: usize = 64 * 1024; @@ -14,6 +16,7 @@ const TEMP_CREATE_ATTEMPTS: usize = 8; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum AtomicCommitFailure { + ParentNotFound, OpenParent, InspectTarget, Conflict, @@ -27,6 +30,7 @@ pub(crate) enum AtomicCommitFailure { impl AtomicCommitFailure { pub(crate) const fn code(self) -> &'static str { match self { + Self::ParentNotFound => "parent-not-found", Self::OpenParent => "open-parent", Self::InspectTarget => "inspect-target", Self::Conflict => "precondition-conflict", @@ -38,6 +42,21 @@ impl AtomicCommitFailure { } } + /// Classification of a definite failure. Every definite failure happens before the commit + /// rename, so the target is unchanged. + pub(crate) const fn failure_code(self) -> StepFailureCode { + match self { + Self::ParentNotFound => StepFailureCode::NotFound, + Self::OpenParent | Self::InspectTarget => StepFailureCode::Unavailable, + Self::Conflict => StepFailureCode::PreconditionConflict, + Self::CreateTemporary + | Self::WriteTemporary + | Self::CommitRenameUnknown + | Self::CommitSyncUnknown + | Self::CommitVerifyUnknown => StepFailureCode::IoError, + } + } + pub(crate) const fn outcome_is_unknown(self) -> bool { matches!( self, @@ -122,9 +141,14 @@ fn open_parent(root: &Dir, path: &RelativePath) -> Result<(Dir, String), AtomicC .try_clone() .map_err(|_| AtomicCommitFailure::OpenParent)?; for component in parents { - directory = directory - .open_dir_nofollow(component) - .map_err(|_| AtomicCommitFailure::OpenParent)?; + directory = directory.open_dir_nofollow(component).map_err(|error| { + match classify_open_error(&directory, component, &error) { + OpenErrorClass::Missing | OpenErrorClass::NotADirectory => { + AtomicCommitFailure::ParentNotFound + } + OpenErrorClass::Refused => AtomicCommitFailure::OpenParent, + } + })?; if is_windows_reparse_point( &directory .dir_metadata() diff --git a/crates/xgen-adapter-filesystem/src/path.rs b/crates/xgen-adapter-filesystem/src/path.rs index 758cf4d..918bba8 100644 --- a/crates/xgen-adapter-filesystem/src/path.rs +++ b/crates/xgen-adapter-filesystem/src/path.rs @@ -66,6 +66,38 @@ impl std::fmt::Debug for RelativePath { } } +/// Value-free class of an error from opening one path component without following links. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum OpenErrorClass { + /// Nothing exists at the name. + Missing, + /// Something other than a directory exists at the name. + NotADirectory, + /// The name exists but is refused, such as a symbolic link, or the error is not classified. + Refused, +} + +/// Classify an open error for `name` in `parent`. Some platforms report a symbolic link opened +/// without following as "not a directory"; that link exists and is refused, so it is not +/// reported as a wrong kind. +pub(crate) fn classify_open_error( + parent: &cap_std::fs::Dir, + name: &str, + error: &std::io::Error, +) -> OpenErrorClass { + match error.kind() { + std::io::ErrorKind::NotFound => OpenErrorClass::Missing, + std::io::ErrorKind::NotADirectory + if !parent + .symlink_metadata(name) + .is_ok_and(|metadata| metadata.file_type().is_symlink()) => + { + OpenErrorClass::NotADirectory + } + _ => OpenErrorClass::Refused, + } +} + pub(crate) fn canonical_resource( workspace_id: &WorkspaceId, resource: &str, diff --git a/crates/xgen-adapter-filesystem/src/query.rs b/crates/xgen-adapter-filesystem/src/query.rs index 63d7ab3..72b9a52 100644 --- a/crates/xgen-adapter-filesystem/src/query.rs +++ b/crates/xgen-adapter-filesystem/src/query.rs @@ -11,12 +11,15 @@ use xgen_runtime::{ AdapterEvidenceDigest, AdapterExecutionObservation, AdapterPrepareFailure, AdapterPrepareRequest, AdapterReconcileRequest, AdapterReconciliationInconclusiveReason, AdapterReconciliationObservation, AdapterToolOutput, EffectAdapter, EffectVerifier, - PreparedAdapterInvocation, RuleVerificationObservation, VerificationPortFailure, - VerificationReport, VerificationRequest, VerifiedArtifactDescriptor, VerifierOutputDigest, + PreparedAdapterInvocation, RuleVerificationObservation, StepFailureCode, + VerificationPortFailure, VerificationReport, VerificationRequest, VerifiedArtifactDescriptor, + VerifierOutputDigest, }; use xgen_workgraph::EffectClass; -use crate::path::{RelativePath, canonical_resource, parse_canonical}; +use crate::path::{ + OpenErrorClass, RelativePath, canonical_resource, classify_open_error, parse_canonical, +}; use crate::{ LIST_DIRECTORY_CAPABILITY_ID, LIST_DIRECTORY_CONTRACT_VERSION, READ_TEXT_LINES_CAPABILITY_ID, READ_TEXT_LINES_CONTRACT_VERSION, SEARCH_TEXT_CAPABILITY_ID, SEARCH_TEXT_CONTRACT_VERSION, @@ -325,6 +328,7 @@ impl PreparedAdapterInvocation for PreparedFilesystemQuery { failure.evidence_digest(self.operation), ) .expect("SHA-256 failure digest is canonical"), + failure_code: failure.failure_code(), }, } } @@ -441,7 +445,11 @@ struct InspectedOutput { #[derive(Debug, Clone, Copy)] enum QueryFailure { + NotFound, Open, + NotAFile, + NotADirectory, + TooLarge, Read, TooManyEntries, Limit, @@ -452,7 +460,11 @@ enum QueryFailure { impl QueryFailure { fn evidence_digest(self, operation: QueryOperation) -> String { let code = match self { + Self::NotFound => "not-found", Self::Open => "open", + Self::NotAFile => "not-a-file", + Self::NotADirectory => "not-a-directory", + Self::TooLarge => "too-large", Self::Read => "read", Self::TooManyEntries => "too-many-entries", Self::Limit => "limit", @@ -461,6 +473,19 @@ impl QueryFailure { }; sha256_digest(format!("{}/{code}", operation.failure_domain()).as_bytes()) } + + const fn failure_code(self) -> StepFailureCode { + match self { + Self::NotFound => StepFailureCode::NotFound, + Self::Open => StepFailureCode::Unavailable, + Self::NotAFile => StepFailureCode::NotAFile, + Self::NotADirectory => StepFailureCode::NotADirectory, + Self::TooLarge => StepFailureCode::TooLarge, + Self::Read | Self::Encode => StepFailureCode::IoError, + Self::TooManyEntries | Self::Limit => StepFailureCode::LimitExceeded, + Self::Decode => StepFailureCode::NotUtf8, + } + } } fn binding_for(root: &WorkspaceRoot, operation: QueryOperation) -> xgen_domain::InstanceBinding { @@ -704,7 +729,7 @@ fn read_text_lines( let mut file = open_regular_file(root, path)?; let metadata = file.metadata().map_err(|_| QueryFailure::Open)?; if metadata.len() > u64::try_from(MAX_READ_TEXT_LINES_FILE_BYTES).expect("limit fits u64") { - return Err(QueryFailure::Limit); + return Err(QueryFailure::TooLarge); } let mut bytes = Vec::with_capacity( usize::try_from(metadata.len()) @@ -717,7 +742,7 @@ fn read_text_lines( .read_to_end(&mut bytes) .map_err(|_| QueryFailure::Read)?; if bytes.len() > MAX_READ_TEXT_LINES_FILE_BYTES { - return Err(QueryFailure::Limit); + return Err(QueryFailure::TooLarge); } let content = String::from_utf8(bytes).map_err(|_| QueryFailure::Decode)?; line_range_output(&content, range) @@ -976,10 +1001,17 @@ fn bounded_preview(line: &str, match_start: usize, match_end: usize) -> String { fn open_directory(root: &Dir, path: &RelativePath) -> Result { let mut directory = root.try_clone().map_err(|_| QueryFailure::Open)?; - for component in path.components() { - directory = directory - .open_dir_nofollow(component) - .map_err(|_| QueryFailure::Open)?; + let last = path.components().len().saturating_sub(1); + for (index, component) in path.components().iter().enumerate() { + directory = directory.open_dir_nofollow(component).map_err(|error| { + match classify_open_error(&directory, component, &error) { + // A non-directory final component exists but has the wrong kind; an earlier one + // means the requested path does not exist. + OpenErrorClass::NotADirectory if index == last => QueryFailure::NotADirectory, + OpenErrorClass::Missing | OpenErrorClass::NotADirectory => QueryFailure::NotFound, + OpenErrorClass::Refused => QueryFailure::Open, + } + })?; if is_windows_reparse_point(&directory.dir_metadata().map_err(|_| QueryFailure::Open)?) { return Err(QueryFailure::Open); } @@ -988,18 +1020,31 @@ fn open_directory(root: &Dir, path: &RelativePath) -> Result } fn open_regular_file(root: &Dir, path: &RelativePath) -> Result { - let (leaf, parents) = path.components().split_last().ok_or(QueryFailure::Open)?; + // The workspace root itself is a directory. + let (leaf, parents) = path + .components() + .split_last() + .ok_or(QueryFailure::NotAFile)?; let parent = RelativePath::from_components(parents.to_vec()); - let directory = open_directory(root, &parent)?; + let directory = open_directory(root, &parent).map_err(|failure| match failure { + QueryFailure::NotADirectory => QueryFailure::NotFound, + other => other, + })?; let mut options = OpenOptions::new(); options.read(true).follow(FollowSymlinks::No).nonblock(true); - let file = directory - .open_with(leaf, &options) - .map_err(|_| QueryFailure::Open)?; + let file = directory.open_with(leaf, &options).map_err(|error| { + match classify_open_error(&directory, leaf, &error) { + OpenErrorClass::Missing | OpenErrorClass::NotADirectory => QueryFailure::NotFound, + OpenErrorClass::Refused => QueryFailure::Open, + } + })?; let metadata = file.metadata().map_err(|_| QueryFailure::Open)?; - if !metadata.is_file() || is_windows_reparse_point(&metadata) { + if is_windows_reparse_point(&metadata) { return Err(QueryFailure::Open); } + if !metadata.is_file() { + return Err(QueryFailure::NotAFile); + } Ok(file) } @@ -1634,7 +1679,7 @@ mod tests { &fixture.relative("huge.txt"), lines(1, 1) ), - Err(QueryFailure::Limit) + Err(QueryFailure::TooLarge) )); assert!(matches!( read_text_lines( @@ -1646,7 +1691,7 @@ mod tests { )); assert!(matches!( read_text_lines(&fixture.root.directory, &fixture.relative("."), lines(1, 1)), - Err(QueryFailure::Open) + Err(QueryFailure::NotAFile) )); } @@ -1709,6 +1754,51 @@ mod tests { } } + #[test] + fn failures_name_missing_paths_and_wrong_kinds_without_values() { + let fixture = Fixture::new(); + fs::write(fixture.directory.path().join("file.txt"), "text").unwrap(); + fs::create_dir(fixture.directory.path().join("dir")).unwrap(); + let root = &fixture.root.directory; + + for (observed, expected) in [ + ( + list_directory(root, &fixture.relative("missing")).err(), + StepFailureCode::NotFound, + ), + ( + list_directory(root, &fixture.relative("file.txt")).err(), + StepFailureCode::NotADirectory, + ), + ( + list_directory(root, &fixture.relative("file.txt/child")).err(), + StepFailureCode::NotFound, + ), + ( + stat_path(root, &fixture.relative("missing.txt")).err(), + StepFailureCode::NotFound, + ), + ( + read_text_lines(root, &fixture.relative("missing.txt"), lines(1, 1)).err(), + StepFailureCode::NotFound, + ), + ( + read_text_lines(root, &fixture.relative("file.txt/child"), lines(1, 1)).err(), + StepFailureCode::NotFound, + ), + ( + read_text_lines(root, &fixture.relative("dir"), lines(1, 1)).err(), + StepFailureCode::NotAFile, + ), + ] { + assert_eq!( + observed.map(QueryFailure::failure_code), + Some(expected), + "{expected:?}" + ); + } + } + #[cfg(unix)] #[test] fn queries_never_follow_workspace_symlinks() { diff --git a/crates/xgen-adapter-filesystem/src/read_text.rs b/crates/xgen-adapter-filesystem/src/read_text.rs index bbaad33..c065f6f 100644 --- a/crates/xgen-adapter-filesystem/src/read_text.rs +++ b/crates/xgen-adapter-filesystem/src/read_text.rs @@ -13,12 +13,13 @@ use xgen_runtime::{ AdapterEvidenceDigest, AdapterExecutionObservation, AdapterPrepareFailure, AdapterPrepareRequest, AdapterReconcileRequest, AdapterReconciliationInconclusiveReason, AdapterReconciliationObservation, AdapterToolOutput, EffectAdapter, EffectVerifier, - PreparedAdapterInvocation, RuleVerificationObservation, VerificationPortFailure, - VerificationReport, VerificationRequest, VerifiedArtifactDescriptor, VerifierOutputDigest, + PreparedAdapterInvocation, RuleVerificationObservation, StepFailureCode, + VerificationPortFailure, VerificationReport, VerificationRequest, VerifiedArtifactDescriptor, + VerifierOutputDigest, }; use xgen_workgraph::EffectClass; -use crate::path::{RelativePath, parse_canonical}; +use crate::path::{OpenErrorClass, RelativePath, classify_open_error, parse_canonical}; use crate::{READ_TEXT_CAPABILITY_ID, READ_TEXT_CONTRACT_VERSION, WorkspaceRoot}; /// Hard product ceiling chosen so worst-case JSON escaping remains below the durable-output bound @@ -216,6 +217,7 @@ impl PreparedAdapterInvocation for PreparedReadText { Err(failure) => AdapterExecutionObservation::Failed { evidence_digest: AdapterEvidenceDigest::new(failure.evidence_digest()) .expect("a SHA-256 digest is canonical"), + failure_code: failure.failure_code(), }, } } @@ -232,8 +234,9 @@ struct InspectedOutput { postcondition: bool, } -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum ReadFailure { + NotFound, Open, NonRegular, TooLarge, @@ -244,6 +247,7 @@ pub(crate) enum ReadFailure { impl ReadFailure { fn evidence_digest(self) -> String { let code = match self { + Self::NotFound => "not-found", Self::Open => "open", Self::NonRegular => "non-regular", Self::TooLarge => "too-large", @@ -252,6 +256,26 @@ impl ReadFailure { }; sha256_digest(format!("xgeny.fs/read-text/failure/v1/{code}").as_bytes()) } + + pub(crate) const fn failure_code(self) -> StepFailureCode { + match self { + Self::NotFound => StepFailureCode::NotFound, + Self::Open => StepFailureCode::Unavailable, + Self::NonRegular => StepFailureCode::NotAFile, + Self::TooLarge => StepFailureCode::TooLarge, + Self::Read => StepFailureCode::IoError, + Self::InvalidUtf8 => StepFailureCode::NotUtf8, + } + } +} + +/// A missing component, or a parent component that is not a directory, means the path does not +/// exist. Every other open error stays unclassified beyond "unavailable". +fn open_failure(parent: &Dir, name: &str, error: &std::io::Error) -> ReadFailure { + match classify_open_error(parent, name, error) { + OpenErrorClass::Missing | OpenErrorClass::NotADirectory => ReadFailure::NotFound, + OpenErrorClass::Refused => ReadFailure::Open, + } } fn verify_contract( @@ -409,7 +433,7 @@ fn open_file(root: &Dir, relative_path: &RelativePath) -> Result Result { + AdapterExecutionObservation::Failed { + evidence_digest, + failure_code, + } => { + assert_eq!(failure_code, StepFailureCode::NotFound); evidence_digest.as_str().to_owned() } other => panic!("missing read must be a definite failure, got {other:?}"), diff --git a/crates/xgen-adapter-filesystem/src/write_atomic.rs b/crates/xgen-adapter-filesystem/src/write_atomic.rs index 96e48a8..3e705f7 100644 --- a/crates/xgen-adapter-filesystem/src/write_atomic.rs +++ b/crates/xgen-adapter-filesystem/src/write_atomic.rs @@ -210,6 +210,7 @@ impl PreparedAdapterInvocation for PreparedWriteAtomic { Err(failure) => AdapterExecutionObservation::Failed { evidence_digest: AdapterEvidenceDigest::new(write_failure_digest(failure)) .expect("a SHA-256 digest is canonical"), + failure_code: failure.failure_code(), }, } } diff --git a/crates/xgen-cli/examples/planning_boundary_probe/main.rs b/crates/xgen-cli/examples/planning_boundary_probe/main.rs index 4bcf8e2..9101b17 100644 --- a/crates/xgen-cli/examples/planning_boundary_probe/main.rs +++ b/crates/xgen-cli/examples/planning_boundary_probe/main.rs @@ -262,6 +262,7 @@ impl PreparedAdapterInvocation for Invocation { if self.fail_operation.as_deref() == self.output["operation"].as_str() { return AdapterExecutionObservation::Failed { evidence_digest: AdapterEvidenceDigest::new(digest(&self.output)).unwrap(), + failure_code: xgen_runtime::StepFailureCode::IoError, }; } // No UNIQUE constraint or deduplication: an unsafe replay must remain visible. diff --git a/crates/xgen-cli/src/composition.rs b/crates/xgen-cli/src/composition.rs index 39fa87a..2b06627 100644 --- a/crates/xgen-cli/src/composition.rs +++ b/crates/xgen-cli/src/composition.rs @@ -529,7 +529,9 @@ pub enum RejectionReason { MaterialRejected, /// The model call was rejected. The class is the same durable class the journal records. ModelRejected(ModelCallRejectionReason), - FailedWork, + /// A Step with effects failed. The code is the value-free classification of that failure + /// (ADR-0054), when the adapter reported one. + FailedWork(Option), } impl RejectionReason { @@ -551,7 +553,16 @@ impl RejectionReason { Self::ProposalRejected(rejection) => proposal_rejection_code(rejection), Self::MaterialRejected => "material_rejected", Self::ModelRejected(reason) => model_rejection_code(reason), - Self::FailedWork => "failed_work", + Self::FailedWork(_) => "failed_work", + } + } + + /// Classification of the failed Step that stopped the Run. + #[must_use] + pub const fn failure_code(self) -> Option { + match self { + Self::FailedWork(code) => code, + _ => None, } } } @@ -1751,7 +1762,37 @@ fn continue_incomplete( ) else { return classify_durable_boundary_after_driver_error(store, manifest); }; - map_driver_outcome(manifest.run_id(), outcome) + let result = map_driver_outcome(manifest.run_id(), outcome)?; + Ok(with_blocking_failure_code(result, store)) +} + +/// Attach the failure code of the Step that stopped the Run. Read-only failures never stop a Run +/// (ADR-0052), so the blocking Step is a failed Step with any other effect class. +fn with_blocking_failure_code( + result: LocalCommandResult, + store: &SqliteRunStore, +) -> LocalCommandResult { + let LocalCommandResult::Rejected { + run_id, + reason: RejectionReason::FailedWork(None), + } = result + else { + return result; + }; + let code = store.load_current().ok().flatten().and_then(|state| { + state.steps.values().find_map(|step| { + (step.status == StepStatus::Failed + && step.intent.as_ref().is_some_and(|intent| { + intent.effect_class != xgen_workgraph::EffectClass::ReadOnly + })) + .then_some(step.failure_code) + .flatten() + }) + }); + LocalCommandResult::Rejected { + run_id, + reason: RejectionReason::FailedWork(code), + } } fn classify_durable_boundary_after_driver_error( @@ -2335,7 +2376,7 @@ fn map_driver_outcome( DriverOutcome::Quiescent(reason) => match reason { AgentLoopQuiescence::FailedSteps => LocalCommandResult::Rejected { run_id, - reason: RejectionReason::FailedWork, + reason: RejectionReason::FailedWork(None), }, AgentLoopQuiescence::ManualIntervention => LocalCommandResult::RecoveryRequired { run_id, diff --git a/crates/xgen-cli/src/main.rs b/crates/xgen-cli/src/main.rs index 2c000f5..8dddf41 100644 --- a/crates/xgen-cli/src/main.rs +++ b/crates/xgen-cli/src/main.rs @@ -2045,6 +2045,9 @@ fn present(result: Result) -> ExitCode { } Ok(LocalCommandResult::Rejected { run_id, reason }) => { eprintln!("XGEN_REJECTED run_id={run_id} reason={}", reason.code()); + if let Some(code) = reason.failure_code() { + eprintln!("XGEN_STEP_FAILURE run_id={run_id} code={}", code.as_str()); + } if let Some(diagnostic) = reason.invocation_diagnostic() { eprintln!( "XGEN_INVOCATION_DIAGNOSTIC run_id={run_id} version=1 category={} field={}", diff --git a/crates/xgen-cli/src/repl.rs b/crates/xgen-cli/src/repl.rs index c610c38..764c329 100644 --- a/crates/xgen-cli/src/repl.rs +++ b/crates/xgen-cli/src/repl.rs @@ -777,6 +777,9 @@ fn handle_result( session_context.pending_goal = None; } writeln!(output, "rejected: run_id={run_id} reason={}", reason.code())?; + if let Some(code) = reason.failure_code() { + writeln!(output, "A step failed: {}.", code.as_str())?; + } if model_rejected { writeln!( output, diff --git a/crates/xgen-cli/tests/workspace_discovery.rs b/crates/xgen-cli/tests/workspace_discovery.rs index 1bbbcf9..84607c8 100644 --- a/crates/xgen-cli/tests/workspace_discovery.rs +++ b/crates/xgen-cli/tests/workspace_discovery.rs @@ -896,11 +896,13 @@ fn failed_read_is_shown_to_the_planner_and_the_task_can_complete() { let after_failure = planning_context(&requests[1]); assert_eq!(after_failure["steps"][0]["status"], "failed"); + assert_eq!(after_failure["steps"][0]["failureCode"], "too_large"); assert_eq!(after_failure["toolOutputs"], json!([])); let serialized = after_failure.to_string(); assert!(!serialized.contains(path_text(&workspace))); let before_completion = planning_context(&requests[2]); assert_eq!(before_completion["steps"][1]["status"], "completed"); + assert!(before_completion["steps"][1].get("failureCode").is_none()); assert_eq!( chronological_output_capabilities(&before_completion), ["xgeny.fs/read-text"] @@ -1028,6 +1030,12 @@ fn failed_write_still_stops_the_run() { "{}", stderr(&output) ); + assert!( + stderr(&output).contains("XGEN_STEP_FAILURE run_id=") + && stderr(&output).contains(" code=precondition_conflict"), + "{}", + stderr(&output) + ); assert_eq!( fs::read_to_string(workspace.join("src/lib.rs")).unwrap(), source diff --git a/crates/xgen-provider-openai/tests/http_contract.rs b/crates/xgen-provider-openai/tests/http_contract.rs index 8cb3563..91f8955 100644 --- a/crates/xgen-provider-openai/tests/http_contract.rs +++ b/crates/xgen-provider-openai/tests/http_contract.rs @@ -385,6 +385,7 @@ fn completed_output_store() -> (OutputSnapshotStore, AgentLoopBudget, Value) { execution_receipt_digest: Some(receipt_digest), uncertainty_reason: None, reconciliation_evidence_digest: None, + failure_code: None, }; let state = RunState { run_id: RUN_ID.to_owned(), diff --git a/crates/xgen-runtime/src/agent_loop.rs b/crates/xgen-runtime/src/agent_loop.rs index 0cb78e9..e2e5a15 100644 --- a/crates/xgen-runtime/src/agent_loop.rs +++ b/crates/xgen-runtime/src/agent_loop.rs @@ -15,8 +15,8 @@ use xgen_workgraph::{ ModelCallAbandonmentReason, ModelCallBudget, ModelCallRejectionReason, ModelCallReservation, ModelCallSettlement, ModelCallStatus, ModelCallUnknownReason, PlannedExecutionProfile, PlannedInvocationMaterialRecord, PlannedInvocationSpec, PlanningContractError, - ReconstructableMaterialReference, RunEvent, RunEventBody, RunState, StepStatus, - ToolOutputRecord, WorkFrontier, dependency_release_block_reason, derive_frontier, + ReconstructableMaterialReference, RunEvent, RunEventBody, RunState, StepFailureCode, StepState, + StepStatus, ToolOutputRecord, WorkFrontier, dependency_release_block_reason, derive_frontier, receipt_releases_dependency, validate_completion_summary_candidate, }; @@ -194,6 +194,10 @@ pub struct PlanningStepSummary { attempts: u32, capability: Option, dependency_released: bool, + /// Value-free failure classification of a `failed` Step (ADR-0054); absent otherwise and for + /// failures recorded before classification existed. + #[serde(skip_serializing_if = "Option::is_none")] + failure_code: Option<&'static str>, } /// One exact, receipt-completed local tool observation selected for the next planner turn. @@ -312,6 +316,7 @@ impl fmt::Debug for PlanningStepSummary { .field("attempts", &self.attempts) .field("capability", &self.capability) .field("dependency_released", &self.dependency_released) + .field("failure_code", &self.failure_code) .finish() } } @@ -2153,6 +2158,19 @@ fn build_context( ) } +/// Adapter failures carry their own code. A Step can also fail at verification, after a Receipt is +/// recorded; that path has a fixed code. +fn planning_failure_code(step: &StepState) -> Option<&'static str> { + if step.status != StepStatus::Failed { + return None; + } + step.failure_code.map(StepFailureCode::as_str).or_else(|| { + step.execution_receipt_digest + .is_some() + .then_some("verification_failed") + }) +} + #[allow(clippy::too_many_arguments, clippy::too_many_lines)] fn build_context_ordered( state: &RunState, @@ -2291,6 +2309,7 @@ fn build_context_ordered( contract_version: invocation.contract_version().to_owned(), }), dependency_released: receipt_releases_dependency(step), + failure_code: planning_failure_code(step), }; let summary_size = canonical_size(&summary).map_err(|_| ContextBuildError::Canonicalization)?; @@ -3205,6 +3224,7 @@ mod tests { execution_receipt_digest: released.then(|| format!("sha256:{}", "b".repeat(64))), uncertainty_reason: None, reconciliation_evidence_digest: None, + failure_code: None, } } diff --git a/crates/xgen-runtime/src/executor.rs b/crates/xgen-runtime/src/executor.rs index 0dfdfd6..616bf85 100644 --- a/crates/xgen-runtime/src/executor.rs +++ b/crates/xgen-runtime/src/executor.rs @@ -13,7 +13,7 @@ use xgen_protocol::{ }; use xgen_workgraph::{ EffectIntent, InvocationMaterialError, InvocationMaterialRecord, ReceiptPlacement, RunState, - SinkGuarantee, StepStatus, TOOL_OUTPUT_PROFILE_V1, invocation_material_digest, + SinkGuarantee, StepFailureCode, StepStatus, TOOL_OUTPUT_PROFILE_V1, invocation_material_digest, receipt_provenance_digest, validate_tool_output_candidate, }; @@ -206,8 +206,10 @@ pub enum AdapterExecutionObservation { evidence_digest: AdapterEvidenceDigest, output: AdapterToolOutput, }, + /// A definite failure. `failure_code` is the value-free classification the planner sees. Failed { evidence_digest: AdapterEvidenceDigest, + failure_code: StepFailureCode, }, Unknown { reason: AdapterExecutionUnknownReason, @@ -481,11 +483,13 @@ impl EffectSink for DirectSink<'_> { } } } - AdapterExecutionObservation::Failed { evidence_digest } => { - ExecutionObservation::Failed { - evidence_digest: evidence_digest.into_string(), - } - } + AdapterExecutionObservation::Failed { + evidence_digest, + failure_code, + } => ExecutionObservation::Failed { + evidence_digest: evidence_digest.into_string(), + failure_code: Some(failure_code), + }, AdapterExecutionObservation::Unknown { reason } => ExecutionObservation::Unknown { reason: reason.code().to_owned(), }, diff --git a/crates/xgen-runtime/src/lib.rs b/crates/xgen-runtime/src/lib.rs index 18f806f..e43a770 100644 --- a/crates/xgen-runtime/src/lib.rs +++ b/crates/xgen-runtime/src/lib.rs @@ -26,6 +26,7 @@ pub use registry::*; pub use router::*; pub use runtime::*; pub use verification::*; +pub use xgen_workgraph::StepFailureCode; pub(crate) const LOCAL_EXECUTOR_ID: &str = "xgeny-local"; diff --git a/crates/xgen-runtime/src/runtime.rs b/crates/xgen-runtime/src/runtime.rs index cb3716a..0661dba 100644 --- a/crates/xgen-runtime/src/runtime.rs +++ b/crates/xgen-runtime/src/runtime.rs @@ -5,7 +5,8 @@ use time::{OffsetDateTime, format_description::well_known::Rfc3339}; use xgen_local_store::{Commit, ExpectedHead, RunStore, StoreError}; use xgen_workgraph::{ EffectIntent, InvocationMaterialRecord, ReconciliationResolution, RunEvent, RunEventBody, - RunState, SinkGuarantee, StepState, StepStatus, TOOL_OUTPUT_PROFILE_V1, ToolOutputRecord, + RunState, SinkGuarantee, StepFailureCode, StepState, StepStatus, TOOL_OUTPUT_PROFILE_V1, + ToolOutputRecord, }; use crate::RunLease; @@ -159,6 +160,7 @@ pub(crate) enum ExecutionObservation { }, Failed { evidence_digest: String, + failure_code: Option, }, Unknown { reason: String, @@ -583,12 +585,18 @@ impl PendingExecutionCommit { } } - fn failed(step: &StepState, intent: &EffectIntent, evidence_digest: String) -> Self { + fn failed( + step: &StepState, + intent: &EffectIntent, + evidence_digest: String, + failure_code: Option, + ) -> Self { Self { body: RunEventBody::EffectFailed { step_id: step.step_id.clone(), effect_id: intent.effect_id.clone(), evidence_digest, + failure_code, }, action: DriveAction::EffectFailed, output: None, @@ -648,13 +656,17 @@ fn classify_execution_observation( evidence_digest, output, ), - ExecutionObservation::Failed { evidence_digest } => { + ExecutionObservation::Failed { + evidence_digest, + failure_code, + } => { let evidence_digest = require_observation_value("execution_failed", "evidence_digest", evidence_digest)?; Ok(PendingExecutionCommit::failed( step, intent, evidence_digest, + failure_code, )) } ExecutionObservation::Unknown { reason } => { diff --git a/crates/xgen-runtime/src/runtime_tests.rs b/crates/xgen-runtime/src/runtime_tests.rs index fa9fbe5..e1e0919 100644 --- a/crates/xgen-runtime/src/runtime_tests.rs +++ b/crates/xgen-runtime/src/runtime_tests.rs @@ -9,7 +9,7 @@ use std::rc::Rc; use crate::{ DriveAction, DurableEffectRuntime, EffectSink, EventFactory, EventFactoryError, EventMetadata, ExecutionObservation, LeaseError, LocalRunLease, PreparedEffect, PreparedEffectBinding, - ReconciliationObservation, RuntimeError, RuntimePolicy, + ReconciliationObservation, RuntimeError, RuntimePolicy, StepFailureCode, }; use tempfile::{TempDir, tempdir}; use xgen_local_store::{ @@ -467,6 +467,7 @@ fn definite_effect_failure_commits_its_receipt() { let mut sink = ScriptedSink::new(Rc::clone(&trace)); sink.executions.push_back(ExecutionObservation::Failed { evidence_digest: "sha256:failure-receipt-1".to_owned(), + failure_code: Some(StepFailureCode::TooLarge), }); let mut events = DeterministicEvents::default(); let (_directory, lease) = lease(); @@ -484,6 +485,10 @@ fn definite_effect_failure_commits_its_receipt() { .as_deref(), Some("sha256:failure-receipt-1") ); + assert_eq!( + report.state.steps[STEP_ID].failure_code, + Some(StepFailureCode::TooLarge) + ); assert_eq!( trace.borrow().as_slice(), [ diff --git a/crates/xgen-runtime/tests/durable_agent_loop.rs b/crates/xgen-runtime/tests/durable_agent_loop.rs index 97a86a4..e3a2518 100644 --- a/crates/xgen-runtime/tests/durable_agent_loop.rs +++ b/crates/xgen-runtime/tests/durable_agent_loop.rs @@ -1047,6 +1047,7 @@ fn manual_step(step_id: &str, status: StepStatus, attempts: u32) -> StepState { execution_receipt_digest: completed.then(|| format!("sha256:{}", "c".repeat(64))), uncertainty_reason: None, reconciliation_evidence_digest: None, + failure_code: None, } } diff --git a/crates/xgen-workgraph/src/lib.rs b/crates/xgen-workgraph/src/lib.rs index 028b5e9..b630c7e 100644 --- a/crates/xgen-workgraph/src/lib.rs +++ b/crates/xgen-workgraph/src/lib.rs @@ -90,6 +90,8 @@ pub enum RunEventBody { effect_id: String, #[serde(rename = "receiptDigest")] evidence_digest: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + failure_code: Option, }, EffectBecameUnknown { step_id: String, @@ -2950,6 +2952,63 @@ pub struct StepState { pub execution_receipt_digest: Option, pub uncertainty_reason: Option, pub reconciliation_evidence_digest: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub failure_code: Option, +} + +/// Closed, value-free classification of a definite adapter failure (ADR-0054). +/// +/// Codes name the kind of failure only. They never carry paths, sizes, content, or OS error text, +/// so they can be shown to the planner without widening the redaction boundary (ADR-0038). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum StepFailureCode { + /// The target path does not exist. + NotFound, + /// The target exists but cannot be opened inside the confined workspace (for example a + /// symbolic link, a permission error, or a non-portable name). + Unavailable, + /// A regular file was required. + NotAFile, + /// A directory was required. + NotADirectory, + /// The input or target exceeds the capability's size limit. + TooLarge, + /// The file is not valid UTF-8 text. + NotUtf8, + /// A bounded scan or output limit was reached before the observation could complete. + LimitExceeded, + /// The target changed since it was observed; the expected digest no longer matches. + PreconditionConflict, + /// An edit's original text does not occur in the target. + PatchContextMissing, + /// An edit's original text occurs more than once in the target. + PatchContextAmbiguous, + /// Two edits overlap in the target. + PatchEditsOverlap, + /// The operation failed while reading or writing. + IoError, +} + +impl StepFailureCode { + /// Stable `snake_case` wire code. + #[must_use] + pub const fn as_str(self) -> &'static str { + match self { + Self::NotFound => "not_found", + Self::Unavailable => "unavailable", + Self::NotAFile => "not_a_file", + Self::NotADirectory => "not_a_directory", + Self::TooLarge => "too_large", + Self::NotUtf8 => "not_utf8", + Self::LimitExceeded => "limit_exceeded", + Self::PreconditionConflict => "precondition_conflict", + Self::PatchContextMissing => "patch_context_missing", + Self::PatchContextAmbiguous => "patch_context_ambiguous", + Self::PatchEditsOverlap => "patch_edits_overlap", + Self::IoError => "io_error", + } + } } #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] @@ -4105,6 +4164,7 @@ fn build_accepted_plan_candidate( execution_receipt_digest: None, uncertainty_reason: None, reconciliation_evidence_digest: None, + failure_code: None, }, ); } @@ -4272,6 +4332,7 @@ fn plan_step( execution_receipt_digest: None, uncertainty_reason: None, reconciliation_evidence_digest: None, + failure_code: None, }, ); Ok(()) @@ -4320,13 +4381,13 @@ fn apply_effect_lifecycle( step_id, effect_id, evidence_digest, - } => record_effect_observation( + failure_code, + } => record_effect_failure( state, step_id, effect_id, evidence_digest, - None, - StepStatus::Failed, + *failure_code, body, )?, RunEventBody::EffectBecameUnknown { @@ -4532,6 +4593,27 @@ fn record_effect_observation( Ok(()) } +fn record_effect_failure( + state: &mut RunState, + step_id: &str, + effect_id: &str, + evidence_digest: &str, + failure_code: Option, + body: &RunEventBody, +) -> Result<(), TransitionError> { + record_effect_observation( + state, + step_id, + effect_id, + evidence_digest, + None, + StepStatus::Failed, + body, + )?; + matching_step_mut(state, step_id, effect_id)?.failure_code = failure_code; + Ok(()) +} + fn record_verification( state: &mut RunState, step_id: &str, @@ -5441,6 +5523,61 @@ mod tests { } } + #[test] + fn effect_failure_codes_are_optional_on_the_wire_and_kept_in_the_step() { + let legacy = RunEventBody::EffectFailed { + step_id: "step-a".to_owned(), + effect_id: "effect-a".to_owned(), + evidence_digest: "sha256:legacy".to_owned(), + failure_code: None, + }; + let legacy_json = serde_json::to_value(&legacy).unwrap(); + assert!(legacy_json.get("failureCode").is_none()); + assert_eq!( + serde_json::from_value::(legacy_json).unwrap(), + legacy + ); + + let classified = RunEventBody::EffectFailed { + step_id: "step-a".to_owned(), + effect_id: "effect-a".to_owned(), + evidence_digest: "sha256:classified".to_owned(), + failure_code: Some(StepFailureCode::PatchContextMissing), + }; + let classified_json = serde_json::to_value(&classified).unwrap(); + assert_eq!(classified_json["failureCode"], "patch_context_missing"); + assert_eq!( + serde_json::from_value::(classified_json).unwrap(), + classified + ); + + let mut step = serde_json::to_value(StepState { + step_id: "step-a".to_owned(), + objective: "objective".to_owned(), + depends_on: Vec::new(), + planned_invocation: None, + status: StepStatus::Failed, + attempts: 1, + intent: None, + effect_evidence_digest: None, + output_record_digest: None, + execution_receipt_id: None, + execution_receipt_digest: None, + uncertainty_reason: None, + reconciliation_evidence_digest: None, + failure_code: None, + }) + .unwrap(); + assert!(step.get("failureCode").is_none()); + step["failureCode"] = serde_json::Value::from("too_large"); + assert_eq!( + serde_json::from_value::(step) + .unwrap() + .failure_code, + Some(StepFailureCode::TooLarge) + ); + } + #[test] fn invocation_material_record_detects_cross_effect_and_content_tampering() { let mut records = Vec::new(); @@ -6511,6 +6648,7 @@ mod tests { execution_receipt_digest: None, uncertainty_reason: None, reconciliation_evidence_digest: None, + failure_code: None, }; let step_value = serde_json::to_value(&step).expect("Step projection should serialize"); assert!(step_value.pointer("/dependsOn").is_none()); @@ -6545,6 +6683,7 @@ mod tests { execution_receipt_digest: Some(format!("sha256:{}", "a".repeat(64))), uncertainty_reason: None, reconciliation_evidence_digest: None, + failure_code: None, }; if index == step_count - 1 { step.status = StepStatus::Planned; diff --git a/crates/xgen-workgraph/tests/durable_plan.rs b/crates/xgen-workgraph/tests/durable_plan.rs index bcc20c4..5b3dfe1 100644 --- a/crates/xgen-workgraph/tests/durable_plan.rs +++ b/crates/xgen-workgraph/tests/durable_plan.rs @@ -842,6 +842,7 @@ fn transitive_blocked_existing_dependency_cannot_consume_another_plan_turn() { step_id: "step-a".to_owned(), effect_id: "effect-a".to_owned(), evidence_digest: digest('7'), + failure_code: None, }, ); let second_proposal = digest('9'); diff --git a/crates/xgen-workgraph/tests/persistent_frontier.rs b/crates/xgen-workgraph/tests/persistent_frontier.rs index 8314554..3b2fac7 100644 --- a/crates/xgen-workgraph/tests/persistent_frontier.rs +++ b/crates/xgen-workgraph/tests/persistent_frontier.rs @@ -27,6 +27,7 @@ fn step(step_id: &str, depends_on: &[&str], status: StepStatus) -> StepState { execution_receipt_digest: None, uncertainty_reason: None, reconciliation_evidence_digest: None, + failure_code: None, } } diff --git a/docs/adr/0022-capability-confined-filesystem-read-adapter.md b/docs/adr/0022-capability-confined-filesystem-read-adapter.md index 15162cf..a08b3d5 100644 --- a/docs/adr/0022-capability-confined-filesystem-read-adapter.md +++ b/docs/adr/0022-capability-confined-filesystem-read-adapter.md @@ -224,7 +224,7 @@ Linux, macOS와 Windows의 `cargo test --workspace --locked`가 동일 gate를 (UTF-8 줄 범위 읽기는 [ADR-0053](0053-bounded-line-range-text-read.md)가 다룬다) - case/Unicode alias의 stable physical file identity - ReadOnly crash 뒤 자동 retry와 tool failure를 다음 model turn에 전달하는 typed error output - (확정된 read-only 실패 뒤 계획을 계속하는 것은 [ADR-0052](0052-recoverable-read-only-step-failures.md)가 다룬다. 실패 원인 분류 전달은 여전히 범위 밖이다) + (확정된 read-only 실패 뒤 계획을 계속하는 것은 [ADR-0052](0052-recoverable-read-only-step-failures.md)가 다룬다. 값 없는 실패 원인 분류 전달은 [ADR-0054](0054-value-free-step-failure-codes.md)가 다룬다) - remote model egress 승인 ## 결과 diff --git a/docs/adr/0052-recoverable-read-only-step-failures.md b/docs/adr/0052-recoverable-read-only-step-failures.md index 85dce1b..2d8be50 100644 --- a/docs/adr/0052-recoverable-read-only-step-failures.md +++ b/docs/adr/0052-recoverable-read-only-step-failures.md @@ -58,8 +58,8 @@ reducer 조건이 넓어지기만 하므로 기존 journal은 새 binary에서 설명할 수 있다. - 실패한 읽기도 tool-call과 planned-step budget을 소비한다. 반복 실패는 예산 소진으로 끝난다 (ADR-0015). -- 실패 원인 분류(too-large, not-found 등)는 아직 모델에 전달하지 않는다. 지금 adapter는 고정 분류의 - digest만 남긴다. typed failure output은 별도 결정이 필요하다. +- 실패 원인 분류는 이 ADR에서 모델에 전달하지 않는다. 값 없는 분류 code 전달은 + [ADR-0054](0054-value-free-step-failure-codes.md)가 추가한다. - 64 KiB를 넘는 파일을 읽는 방법은 이 ADR에 없다. 줄 범위 읽기는 [ADR-0053](0053-bounded-line-range-text-read.md)이 추가한다. diff --git a/docs/adr/0053-bounded-line-range-text-read.md b/docs/adr/0053-bounded-line-range-text-read.md index 32d0bc6..cb46817 100644 --- a/docs/adr/0053-bounded-line-range-text-read.md +++ b/docs/adr/0053-bounded-line-range-text-read.md @@ -47,7 +47,7 @@ directory의 하위)으로 검사한다. effect class는 `read_only`이고 `--al 경로는 `read-text`·search-text와 같이 symlink를 따르지 않는다. 실패는 기존 조회 adapter와 같은 고정 분류 digest(`open`, `read`, `limit`, `decode`, `encode`)만 남긴다. 실패는 ADR-0052에 따라 recoverable -read failure가 되고, 모델에 원인 분류는 전달되지 않는다. verifier는 digest와 함께 줄 번호 관계 +read failure가 된다. 원인 분류 code는 [ADR-0054](0054-value-free-step-failure-codes.md)에 따라 전달된다. verifier는 digest와 함께 줄 번호 관계 (`endLine + 1 = startLine + 반환 줄 수`, `endLine <= totalLines`)와 content 상한을 다시 검사한다. ## 대안 diff --git a/docs/adr/0054-value-free-step-failure-codes.md b/docs/adr/0054-value-free-step-failure-codes.md new file mode 100644 index 0000000..69f065c --- /dev/null +++ b/docs/adr/0054-value-free-step-failure-codes.md @@ -0,0 +1,102 @@ +# ADR-0054: 실패한 Step의 값 없는 원인 분류를 planner에 전달 + +- 상태: Accepted +- 기준일: 2026-10-25 +- 변경 분류: 범용 runtime 계약. 특정 도구·파일·업무 규칙 없음. +- 기존 경계: ADR-0022, ADR-0038, ADR-0052, ADR-0053 +- journal 변경: `effect_failed` 이벤트와 Step projection에 선택 필드 `failureCode` 추가. SQLite schema는 그대로다. +- planner prompt 변경: 없음. planning context의 실패한 Step 요약에 선택 필드 `failureCode`가 붙는다. + +## 문제 + +ADR-0052 이후 확정된 읽기 실패 뒤에도 계획은 이어진다. 하지만 모델은 그 Step이 `failed`라는 것만 +보고 이유는 모른다. adapter는 고정 분류의 SHA-256 digest만 남기고, ADR-0022·0052는 원인 분류 +전달을 범위 밖으로 남겼다. + +실사용에서 모델은 이유를 추측하느라 같은 실패를 반복했다. 큰 파일 읽기에서는 같은 `read-text`를 +두 번 실패한 뒤 `stat`으로 크기를 확인하고서야 줄 범위 읽기로 바꿨다. 없는 파일을 물은 요청에서는 +같은 경로의 `read-text`·`stat`을 3–17번 반복했고, 한 번은 턴 예산을 다 써서 답하지 못했다. + +## 결정 + +### 1. 닫힌 분류 `StepFailureCode` + +adapter의 확정 실패(`AdapterExecutionObservation::Failed`)는 evidence digest와 함께 `failure_code`를 +반드시 낸다. 분류는 workgraph의 닫힌 enum이다. + +| code | 뜻 | +| --- | --- | +| `not_found` | 경로가 없다. 중간 경로가 directory가 아닌 경우를 포함한다. | +| `unavailable` | 있지만 confined workspace에서 열 수 없다. symlink, 권한, 이식 불가 이름 등 | +| `not_a_file` | regular file이 필요했다. | +| `not_a_directory` | directory가 필요했다. | +| `too_large` | 입력이나 대상이 capability 크기 상한을 넘었다. | +| `not_utf8` | UTF-8 text가 아니다. | +| `limit_exceeded` | bounded scan 또는 output 상한에 닿아 관찰을 끝낼 수 없었다. | +| `precondition_conflict` | 관찰 뒤 대상이 바뀌어 expected digest가 맞지 않는다. | +| `patch_context_missing` | edit의 원문이 대상에 없다. | +| `patch_context_ambiguous` | edit의 원문이 대상에 여러 번 있다. | +| `patch_edits_overlap` | edit끼리 겹친다. | +| `io_error` | 읽기·쓰기 중 실패했다. | + +code는 종류만 나타낸다. 경로, 크기, 내용, OS 오류 문자열을 담지 않는다. 그래서 planner에 보내도 +ADR-0038의 redaction 경계가 넓어지지 않는다. 새 code는 enum에 추가하는 방식으로만 늘린다. 업무 +의미를 가진 code는 넣지 않는다. + +symlink를 따르지 않고 열 때 일부 플랫폼은 "not a directory"를 보고한다. 이 경우 이름의 +`symlink_metadata`를 확인해 `unavailable`로 분류한다. 존재하는 symlink를 `not_found`로 말하지 않는다. + +### 2. journal과 projection + +`effect_failed` 이벤트에 `failureCode`를 선택 필드로 기록한다. reducer는 이를 Step의 `failureCode`에 +복사한다. 값이 없으면 직렬화하지 않는다. 따라서 이 변경 이전 이벤트와 projection은 바이트 단위로 +그대로 replay된다. verification 실패처럼 adapter 실패가 아닌 경로는 이벤트를 바꾸지 않는다. + +### 3. planning context + +`failed` 상태 Step 요약에만 `failureCode`를 붙인다. adapter가 기록한 code를 그대로 쓴다. Receipt가 +기록된 뒤 verification에서 실패한 Step은 `verification_failed`다. 둘 다 없으면, 곧 이 변경 이전에 +기록된 실패라면 필드를 생략한다. 다른 상태에는 붙이지 않는다. + +field가 붙는 것은 실패한 Step이 있을 때뿐이다. 그래서 실패가 없는 Run의 planning context는 바이트 +단위로 그대로다. context profile 이름(`xgeny.planning-context/v2`)과 prompt는 바꾸지 않는다. + +### 4. 사용자 표시 + +read-only가 아닌 Step의 실패로 Run이 멈추면(ADR-0052) 그 Step의 code를 사용자에게도 보인다. + +- `xgen run`은 `XGEN_REJECTED` 다음 줄에 `XGEN_STEP_FAILURE run_id= code=`를 출력한다. +- 대화형 `xgen`은 `A step failed: .`를 출력한다. + +기존 `reason=failed_work`는 그대로다. + +## 결과 + +- 모델은 실패 이유에 맞춰 다음 단계를 고른다. 예를 들어 `too_large`이면 줄 범위 읽기로, + `not_found`이면 목록·검색으로 넘어간다. +- 쓰기 실패는 여전히 Run을 멈춘다(ADR-0052). 실패 이유는 이제 사용자에게 보인다. 확정 실패한 + 쓰기 뒤 계획을 이어 가는 것은 별도 결정이다. +- 이 변경 이전 binary는 `failureCode`가 있는 journal을 읽지 못한다. 기존 journal은 새 binary에서 + 그대로 읽힌다. +- 같은 실패의 failure evidence digest 일부가 바뀐다. 예를 들어 없는 경로는 `open` 대신 `not-found` + 분류다. 이미 기록된 digest는 다시 계산하지 않는다. + +## 검증 + +- adapter 단위: + - read-text·query·apply-patch·write-atomic의 실패별 code + - 중간 경로가 파일이면 `not_found` + - 대상이 파일이면 `not_a_directory` + - symlink 경로는 기존처럼 거절 +- workgraph: `failureCode`가 없는 이벤트·projection의 직렬화가 이전과 같고, 있으면 Step에 남는다. +- runtime: 확정 실패의 code가 journal을 거쳐 Step에 남는다. +- e2e: + - 64 KiB 초과 읽기 실패 뒤 다음 planning context의 Step에 `failureCode: too_large`가 있다. + - stale digest patch로 멈춘 `xgen run`이 `XGEN_STEP_FAILURE ... code=precondition_conflict`를 출력한다. +- 실사용: 대화형 `xgen`, 같은 planner model(default profile), 사례마다 이전 main과 이 변경을 3회씩 + 실행했다. 두 사례 모두 이 ADR의 설계에 쓰지 않은 저장소다. + + | 사례 | 실패한 Step 수, 이전 | 실패한 Step 수, 이후 | 결과 | + | --- | --- | --- | --- | + | 113 KB 파일의 동작 설명 | 2, 2, 2 | 1, 1, 1 | 6회 모두 정답 | + | 없는 `CHANGELOG.md` 요약 요청 | 5, 17, 3 | 1, 2, 1 | 이전 1회는 턴 예산 소진으로 답 없음, 이후 3회 모두 "없다"고 정확히 답함 |