From 1535af83b7d4c833cf35ece1d87a4f6eab73dd1b Mon Sep 17 00:00:00 2001 From: Son Seong Jun <166786347+SonAIengine@users.noreply.github.com> Date: Mon, 26 Oct 2026 17:15:29 +0900 Subject: [PATCH] =?UTF-8?q?=EA=B1=B0=EC=A0=88=20=EC=9D=B4=EC=9C=A0?= =?UTF-8?q?=EC=99=80=20=EB=B0=98=EB=B3=B5=20=EC=82=AC=EC=8B=A4=EC=9D=84=20?= =?UTF-8?q?planning=20context=EC=97=90=20=EB=84=A3=EC=97=88=EB=8B=A4=20(AD?= =?UTF-8?q?R-0064)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 바로 앞 model call이 거절되면 다음 context에 previousResponseRejection(reason, Core 거절 code, 고정 문장)을 넣는다. journal의 Rejected settlement에 선택 필드 proposal을 붙여 재시작 뒤에도 같은 context를 만든다. - 끝난 Step이 앞서 끝난 Step과 capability·인자가 같으면 repeatsStep·repeatMeaning을 붙인다. - read-only capability를 방금 끝난 Step과 같은 인자로 다시 제안하면 proposal_rejected.repeated_finished_action으로 거절한다. 쓰기·실행은 거절하지 않는다 (모든 class에 적용하면 T2 patch 재시도가 회귀했다). - 측정(deepseek flash, 작업당 2회): T2 1/2(실패는 materialization_failed), T3 2개 남음 / invalid response 3연속, T4 16/24 8초 / bash 반복 96 turn, T1 통과. 이 변경으로 인한 회귀는 없고, 가장 긴 루프인 프로세스 실행 반복은 아직 막지 못한다. - 테스트 739개 통과, clippy 경고 0. Co-Authored-By: Claude Opus 5.5 --- crates/xgen-cli/src/composition.rs | 1 + crates/xgen-cli/tests/interactive_repl.rs | 24 +- crates/xgen-cli/tests/live_go50902_public.rs | 6 +- crates/xgen-cli/tests/workspace_discovery.rs | 17 +- .../tests/http_contract.rs | 4 +- crates/xgen-runtime/src/agent_loop.rs | 263 +++++++++++++++++- .../xgen-runtime/tests/durable_agent_loop.rs | 73 +++++ crates/xgen-workgraph/src/lib.rs | 69 ++++- .../tests/durable_model_call.rs | 5 + docs/adr/0061-rejected-response-retry.md | 3 +- docs/adr/0064-planning-feedback.md | 60 ++++ docs/getting-started.md | 2 +- 12 files changed, 511 insertions(+), 16 deletions(-) create mode 100644 docs/adr/0064-planning-feedback.md diff --git a/crates/xgen-cli/src/composition.rs b/crates/xgen-cli/src/composition.rs index 3275693..b0a5e68 100644 --- a/crates/xgen-cli/src/composition.rs +++ b/crates/xgen-cli/src/composition.rs @@ -606,6 +606,7 @@ const fn proposal_rejection_code(rejection: ProposalRejection) -> &'static str { "proposal_rejected.invocation_invalid" } ProposalRejection::DuplicateSemanticAction => "proposal_rejected.duplicate_semantic_action", + ProposalRejection::RepeatedFinishedAction => "proposal_rejected.repeated_finished_action", ProposalRejection::PlannedStepBudgetExceeded => { "proposal_rejected.planned_step_budget_exceeded" } diff --git a/crates/xgen-cli/tests/interactive_repl.rs b/crates/xgen-cli/tests/interactive_repl.rs index 986ac49..2210a53 100644 --- a/crates/xgen-cli/tests/interactive_repl.rs +++ b/crates/xgen-cli/tests/interactive_repl.rs @@ -498,10 +498,28 @@ fn one_invalid_response_is_retried_and_the_run_completes_without_resume() { .unwrap(); assert_eq!(output.status.code(), Some(0), "{}", stderr(&output)); // plan, discarded invalid completion, accepted completion - for _ in 0..3 { - server.requests.recv_timeout(TEST_TIMEOUT).unwrap(); - } + let requests: Vec<_> = (0..3) + .map(|_| server.requests.recv_timeout(TEST_TIMEOUT).unwrap()) + .collect(); server.handle.join().unwrap(); + // ADR-0064: only the call after the discarded response learns why it was discarded. + assert!( + planning_context(&requests[1]) + .get("previousResponseRejection") + .is_none() + ); + let rejection = &planning_context(&requests[2])["previousResponseRejection"]; + assert_eq!( + rejection["reason"], "planner_invalid_response", + "{rejection}" + ); + assert!(rejection.get("proposalRejection").is_none()); + assert!( + rejection["meaning"] + .as_str() + .unwrap() + .starts_with("The previous response was discarded") + ); assert!( !stderr(&output).contains("model_rejected"), "{}", diff --git a/crates/xgen-cli/tests/live_go50902_public.rs b/crates/xgen-cli/tests/live_go50902_public.rs index 03ed387..033fa9f 100644 --- a/crates/xgen-cli/tests/live_go50902_public.rs +++ b/crates/xgen-cli/tests/live_go50902_public.rs @@ -1324,7 +1324,11 @@ fn require_workspace_completion(output: &Output, state_root: &Path) { } if let Some(reason) = snapshot.records.iter().rev().find_map(|record| { if let RunEventBody::ModelCallSettled { - settlement: ModelCallSettlement::Rejected { reason }, + settlement: + ModelCallSettlement::Rejected { + reason, + proposal: None, + }, .. } = record.event.body { diff --git a/crates/xgen-cli/tests/workspace_discovery.rs b/crates/xgen-cli/tests/workspace_discovery.rs index d077df9..5b51218 100644 --- a/crates/xgen-cli/tests/workspace_discovery.rs +++ b/crates/xgen-cli/tests/workspace_discovery.rs @@ -1824,12 +1824,17 @@ fn tool_discovery_is_approved_receipted_and_frozen_across_resume() { #[test] #[cfg(unix)] fn tool_discovery_rejects_unknown_collections_and_candidate_execution() { - for (capability_id, input) in [ + for (capability_id, input, code) in [ ( "xgen.tools/search", json!({"collection":"unknown","query":"record","topK":1}), + "invocation_invalid", + ), + ( + "readAsset", + json!({"asset_id":"one"}), + "capability_unavailable", ), - ("readAsset", json!({"asset_id":"one"})), ] { let fixture = tempdir().unwrap(); let state = fixture.path().join("state"); @@ -1873,8 +1878,14 @@ fn tool_discovery_rejects_unknown_collections_and_candidate_execution() { .unwrap(); assert!(store.load_execution_receipts().unwrap().is_empty()); assert!(!state.join("tool-collections/invocations").exists()); - server.requests.recv_timeout(TEST_TIMEOUT).unwrap(); + let requests: Vec<_> = (0..=xgen_cli::MAX_REJECTED_RESPONSE_RETRIES) + .map(|_| server.requests.recv_timeout(TEST_TIMEOUT).unwrap()) + .collect(); server.handle.join().unwrap(); + // ADR-0064: each retry carries the Core class of the rejection before it. + let rejection = &planning_context(&requests[1])["previousResponseRejection"]; + assert_eq!(rejection["reason"], "proposal_rejected", "{rejection}"); + assert_eq!(rejection["proposalRejection"], code, "{rejection}"); } } diff --git a/crates/xgen-provider-openai/tests/http_contract.rs b/crates/xgen-provider-openai/tests/http_contract.rs index 91f8955..f5dc06d 100644 --- a/crates/xgen-provider-openai/tests/http_contract.rs +++ b/crates/xgen-provider-openai/tests/http_contract.rs @@ -406,6 +406,7 @@ fn completed_output_store() -> (OutputSnapshotStore, AgentLoopBudget, Value) { settled_calls: 1, unknown_calls: 0, active_call: None, + last_rejection: None, }), completion_candidate: None, }), @@ -1147,7 +1148,8 @@ fn deterministic_provider_rejection_is_closed_without_raw_error_body() { &last.event.body, RunEventBody::ModelCallSettled { settlement: ModelCallSettlement::Rejected { - reason: ModelCallRejectionReason::ProviderRejected + reason: ModelCallRejectionReason::ProviderRejected, + proposal: None, }, .. } diff --git a/crates/xgen-runtime/src/agent_loop.rs b/crates/xgen-runtime/src/agent_loop.rs index 779abfd..7a714a1 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, + ProposalRejectionCode, ReconstructableMaterialReference, RunEvent, RunEventBody, RunState, + StepStatus, ToolOutputRecord, WorkFrontier, dependency_release_block_reason, derive_frontier, receipt_releases_dependency, validate_completion_summary_candidate, }; @@ -201,8 +201,20 @@ pub struct PlanningStepSummary { /// Fixed, value-free statement of what `failure_code` means (ADR-0055). #[serde(skip_serializing_if = "Option::is_none")] failure_meaning: Option<&'static str>, + /// Earlier finished Step with the same capability and arguments (ADR-0064). + #[serde(skip_serializing_if = "Option::is_none")] + repeats_step: Option, + /// Fixed statement of what `repeats_step` means. + #[serde(skip_serializing_if = "Option::is_none")] + repeat_meaning: Option<&'static str>, } +/// Fixed fact attached to a Step that repeats a finished one (ADR-0064). +const REPEAT_MEANING: &str = "This Step used the same capability with the same arguments as \ + repeatsStep, which had already finished. Its result is already known; repeating it again \ + returns the same result unless something changed the target in between. If the known \ + results already answer the goal, return a completion_candidate instead of planning again."; + /// One exact, receipt-completed local tool observation selected for the next planner turn. /// /// The raw output is serialized only into the provider-bound context. Diagnostics deliberately @@ -306,6 +318,12 @@ impl PlanningStepSummary { pub const fn dependency_released(&self) -> bool { self.dependency_released } + + /// Earlier finished Step this one repeats exactly, if any (ADR-0064). + #[must_use] + pub fn repeats_step(&self) -> Option<&str> { + self.repeats_step.as_deref() + } } impl fmt::Debug for PlanningStepSummary { @@ -321,6 +339,8 @@ impl fmt::Debug for PlanningStepSummary { .field("dependency_released", &self.dependency_released) .field("failure_code", &self.failure_code) .field("failure_meaning", &self.failure_meaning) + .field("repeats_step", &self.repeats_step) + .field("repeat_meaning", &self.repeat_meaning) .finish() } } @@ -421,6 +441,9 @@ struct PlanningContextPayload { tool_outputs: Vec, total_steps: usize, verified_completed_steps: usize, + /// Why the previous response was discarded, when it was (ADR-0064). + #[serde(skip_serializing_if = "Option::is_none")] + previous_response_rejection: Option, run_id: String, authority: String, authority_epoch: u64, @@ -428,6 +451,79 @@ struct PlanningContextPayload { journal_head_digest: String, } +/// Journal-derived fact about the discarded previous response (ADR-0064). +#[derive(Clone, Debug, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +struct PreviousResponseRejection { + reason: ModelCallRejectionReason, + #[serde(skip_serializing_if = "Option::is_none")] + proposal_rejection: Option, + meaning: &'static str, +} + +impl PreviousResponseRejection { + fn from_feedback(feedback: xgen_workgraph::ModelCallRejectionFeedback) -> Self { + Self { + reason: feedback.reason, + proposal_rejection: feedback.proposal, + meaning: rejection_meaning(feedback.reason, feedback.proposal), + } + } +} + +/// Fixed statement of what a rejection means for the next proposal. Never echoes the response. +const fn rejection_meaning( + reason: ModelCallRejectionReason, + proposal: Option, +) -> &'static str { + match (reason, proposal) { + (ModelCallRejectionReason::PlannerInvalidResponse, _) => { + "The previous response was discarded: it did not match the response contract, or its \ + completion failed a host completion check. Nothing was changed." + } + ( + ModelCallRejectionReason::ProposalRejected, + Some( + ProposalRejectionCode::CapabilityUnavailable + | ProposalRejectionCode::CapabilityUnsupported, + ), + ) => { + "The previous proposal named a capability or contract version that is not offered \ + here. Use only capabilityId and contractVersion values listed in capabilities. \ + Nothing was changed." + } + ( + ModelCallRejectionReason::ProposalRejected, + Some(ProposalRejectionCode::InvocationInvalid), + ) => { + "The previous proposal's arguments did not satisfy the capability inputSchema or the \ + allowed scope. Nothing was changed." + } + ( + ModelCallRejectionReason::ProposalRejected, + Some(ProposalRejectionCode::RepeatedFinishedAction), + ) => { + "The previous proposal repeated the read-only capability and arguments of the last \ + finished Step, and nothing ran in between. That result is already in steps and \ + toolOutputs. Use it: return a \ + completion_candidate if it answers the goal, or choose a different action. Nothing \ + was changed." + } + ( + ModelCallRejectionReason::ProposalRejected, + Some(ProposalRejectionCode::DuplicateSemanticAction), + ) => { + "The previous proposal contained two Steps with the same capability and arguments. \ + Nothing was changed." + } + (ModelCallRejectionReason::ProposalRejected, _) => { + "Core rejected the previous proposal for the rule named in proposalRejection. Nothing \ + was changed." + } + _ => "The previous model call was rejected for the reason given. Nothing was changed.", + } +} + /// Deterministic provider-neutral input for one planner call. /// /// It contains no invocation arguments, credentials, policy decisions, selected Instances, or @@ -443,6 +539,17 @@ pub struct PlanningContext { payload: PlanningContextPayload, context_digest: String, canonical_size_bytes: u64, + /// Host-owned: the action of the last finished Step in plan order (ADR-0064). Not part of + /// the provider payload; Core uses it to reject an immediate exact repeat. + last_finished_action: Option, +} + +/// Capability and argument digest of one finished Step. +#[derive(Clone, Debug, PartialEq, Eq)] +struct FinishedAction { + capability_id: String, + contract_version: String, + plan_input_digest: String, } impl PlanningContext { @@ -574,6 +681,7 @@ impl fmt::Debug for PlanningContext { .field("omitted_capabilities", &self.payload.omitted_capabilities) .field("context_digest", &self.context_digest) .field("canonical_size_bytes", &self.canonical_size_bytes) + .field("last_finished_action", &self.last_finished_action) .finish() } } @@ -903,6 +1011,8 @@ pub enum ProposalRejection { InvocationDiagnosed(crate::InvocationDiagnostic), /// Two Steps in the same proposal resolve to one canonical semantic action. DuplicateSemanticAction, + /// A read-only Step repeats the capability and arguments of the last finished Step (ADR-0064). + RepeatedFinishedAction, PlannedStepBudgetExceeded, ToolCallBudgetExhausted, CompletionWithoutReceiptCompletedPlan, @@ -910,6 +1020,44 @@ pub enum ProposalRejection { InvalidCompletionSummary, } +impl ProposalRejection { + /// Journal-safe class of this rejection (ADR-0064). Diagnostics keep only their coarse class. + #[must_use] + pub const fn code(self) -> ProposalRejectionCode { + match self { + Self::EmptyPlan => ProposalRejectionCode::EmptyPlan, + Self::TooManySteps => ProposalRejectionCode::TooManySteps, + Self::TooManyEdges => ProposalRejectionCode::TooManyEdges, + Self::ProposalTooLarge => ProposalRejectionCode::ProposalTooLarge, + Self::InvalidStepKey => ProposalRejectionCode::InvalidStepKey, + Self::DuplicateStepKey => ProposalRejectionCode::DuplicateStepKey, + Self::InvalidObjective => ProposalRejectionCode::InvalidObjective, + Self::InvalidDependency => ProposalRejectionCode::InvalidDependency, + Self::DuplicateDependency => ProposalRejectionCode::DuplicateDependency, + Self::UnknownDependency => ProposalRejectionCode::UnknownDependency, + Self::BlockedExistingDependency => ProposalRejectionCode::BlockedExistingDependency, + Self::SelfDependency => ProposalRejectionCode::SelfDependency, + Self::DependencyCycle => ProposalRejectionCode::DependencyCycle, + Self::CapabilityUnavailable => ProposalRejectionCode::CapabilityUnavailable, + Self::CapabilityUnsupported => ProposalRejectionCode::CapabilityUnsupported, + Self::InvocationInvalid | Self::InvocationDiagnosed(_) => { + ProposalRejectionCode::InvocationInvalid + } + Self::DuplicateSemanticAction => ProposalRejectionCode::DuplicateSemanticAction, + Self::RepeatedFinishedAction => ProposalRejectionCode::RepeatedFinishedAction, + Self::PlannedStepBudgetExceeded => ProposalRejectionCode::PlannedStepBudgetExceeded, + Self::ToolCallBudgetExhausted => ProposalRejectionCode::ToolCallBudgetExhausted, + Self::CompletionWithoutReceiptCompletedPlan => { + ProposalRejectionCode::CompletionWithoutReceiptCompletedPlan + } + Self::ConversationResponseWithPlan => { + ProposalRejectionCode::ConversationResponseWithPlan + } + Self::InvalidCompletionSummary => ProposalRejectionCode::InvalidCompletionSummary, + } + } +} + /// Result of exactly one bounded orchestration tick. #[derive(Debug, Clone, PartialEq, Eq)] pub enum AgentLoopTick { @@ -1527,6 +1675,7 @@ impl AgentLoop { reserved_state, call_id, ModelCallRejectionReason::MaterializationFailed, + None, ) { Ok(commit) => commit, Err(AgentLoopError::Store(StoreError::HeadConflict { .. })) => { @@ -1691,6 +1840,7 @@ impl AgentLoop { reserved_state, call_id, ModelCallRejectionReason::ProposalRejected, + Some(reason.code()), ) { Ok(commit) => commit, Err(AgentLoopError::Store(StoreError::HeadConflict { .. })) => { @@ -1748,6 +1898,7 @@ impl AgentLoop { reserved_state, call_id, ModelCallRejectionReason::PlannerInvalidResponse, + None, ), ModelCallConflictIntent::RejectStale, ), @@ -1770,6 +1921,7 @@ impl AgentLoop { } _ => ModelCallRejectionReason::ProviderLimit, }, + None, ), ModelCallConflictIntent::RejectStale, ), @@ -1780,6 +1932,7 @@ impl AgentLoop { reserved_state, call_id, ModelCallRejectionReason::ProviderRejected, + None, ), ModelCallConflictIntent::RejectStale, ), @@ -1896,6 +2049,7 @@ impl AgentLoop { ¤t, call_id, ModelCallRejectionReason::StaleHead, + None, )?; Ok(AgentLoopTick::ModelCallRejected { reason: ModelCallRejectionReason::StaleHead, @@ -1941,6 +2095,7 @@ fn append_model_call_rejection( state: &RunState, call_id: &str, reason: ModelCallRejectionReason, + proposal: Option, ) -> Result where S: RunStore, @@ -1951,7 +2106,7 @@ where state, RunEventBody::ModelCallSettled { call_id: call_id.to_owned(), - settlement: ModelCallSettlement::Rejected { reason }, + settlement: ModelCallSettlement::Rejected { reason, proposal }, }, )?; Ok(store.append(ExpectedHead::from_state(state), event)?) @@ -2280,12 +2435,37 @@ fn build_context_ordered( }) .map_err(|_| ContextBuildError::Canonicalization)?; let mut step_summaries = Vec::with_capacity(state.steps.len()); + // ADR-0064: first finished Step per (capability, contract, argument digest). + let mut finished_actions: BTreeMap<(&str, &str, &str), &str> = BTreeMap::new(); + let mut last_finished_action = None; for step_id in planning_step_order { let step = state .steps .get(step_id) .ok_or(ContextBuildError::OutputBinding)?; validate_planning_step_shape(step)?; + let action = step.planned_invocation.as_ref().map(|invocation| { + ( + invocation.capability_id(), + invocation.contract_version(), + invocation.plan_input_digest(), + ) + }); + let repeats_step = action + .and_then(|action| finished_actions.get(&action)) + .map(|earlier| (*earlier).to_owned()); + if let Some(action) = action + && matches!(step.status, StepStatus::Completed | StepStatus::Failed) + { + finished_actions + .entry(action) + .or_insert(step.step_id.as_str()); + last_finished_action = Some(FinishedAction { + capability_id: action.0.to_owned(), + contract_version: action.1.to_owned(), + plan_input_digest: action.2.to_owned(), + }); + } let summary = PlanningStepSummary { step_id: step.step_id.clone(), objective: step.objective.clone(), @@ -2302,6 +2482,8 @@ fn build_context_ordered( dependency_released: receipt_releases_dependency(step), failure_code: step.failure_label(), failure_meaning: step.failure_meaning(), + repeat_meaning: repeats_step.is_some().then_some(REPEAT_MEANING), + repeats_step, }; let summary_size = canonical_size(&summary).map_err(|_| ContextBuildError::Canonicalization)?; @@ -2327,6 +2509,12 @@ fn build_context_ordered( planning_constraints: planning_constraints.to_vec(), total_steps: frontier.total_steps, verified_completed_steps: frontier.verified_completed_step_ids.len(), + previous_response_rejection: state + .agent_loop + .as_ref() + .and_then(|loop_state| loop_state.model_calls.as_ref()) + .and_then(|calls| calls.last_rejection) + .map(PreviousResponseRejection::from_feedback), tool_outputs, steps: Vec::new(), omitted_steps: total_steps, @@ -2379,6 +2567,7 @@ fn build_context_ordered( payload, context_digest, canonical_size_bytes, + last_finished_action, }) } @@ -2715,6 +2904,18 @@ fn prepare_plan( if !proposed_semantic_action_digests.insert(facts.semantic_action_digest.clone()) { return Err(ProposalRejection::DuplicateSemanticAction); } + // ADR-0064: a read-only action returns the same result for the same input while nothing + // ran in between, and that result is already in the context. Effects are left alone: a + // repeated write fails with its own diagnosis and a terminal read can return new output. + if definition.spec.effect.class == EffectClass::ReadOnly + && context.last_finished_action.as_ref().is_some_and(|last| { + last.capability_id == step.capability.capability_id + && last.contract_version == step.capability.contract_version + && last.plan_input_digest == facts.material_digest + }) + { + return Err(ProposalRejection::RepeatedFinishedAction); + } validated.push(ValidatedPlanStep { key: step.key, objective: step.objective, @@ -3379,6 +3580,62 @@ mod tests { )); } + fn invoked(mut step: xgen_workgraph::StepState, input: char) -> xgen_workgraph::StepState { + let digest = |fill: char| format!("sha256:{}", fill.to_string().repeat(64)); + step.planned_invocation = Some( + serde_json::from_value(serde_json::json!({ + "formatVersion": 1, + "planId": "plan-test", + "proposalDigest": digest('1'), + "capabilityId": "xgeny.fs/list-directory", + "contractVersion": "1.0.0", + "definitionDigest": digest('2'), + "actionDigest": digest('3'), + "planInputDigest": digest(input), + "executionProfile": "local_sync_read_only_occurrence_v1", + "targetOs": "linux", + "targetArch": "x86_64", + "specDigest": digest('4'), + "planInputRecordDigest": digest('5'), + })) + .expect("binding fixture should deserialize"), + ); + step + } + + #[test] + fn repeated_finished_actions_are_marked_with_the_first_finished_step() { + let state = state(vec![ + invoked(step("first", StepStatus::Failed, Vec::new()), 'a'), + invoked(step("again", StepStatus::Completed, Vec::new()), 'a'), + invoked(step("third", StepStatus::Planned, Vec::new()), 'a'), + invoked(step("other", StepStatus::Completed, Vec::new()), 'b'), + ]); + let frontier = derive_frontier(&state).expect("frontier should derive"); + let order = ["first", "again", "third", "other"].map(str::to_owned); + let context = build_context_ordered( + &state, + &BTreeMap::new(), + &order, + &[], + &frontier, + &CapabilityRegistry::new(), + &[], + &test_budget(), + ) + .expect("context should fit"); + let value = serde_json::to_value(&context).expect("context should serialize"); + let repeats = value["steps"] + .as_array() + .expect("steps should be an array") + .iter() + .map(|step| step["repeatsStep"].as_str()) + .collect::>(); + assert_eq!(repeats, [None, Some("first"), Some("first"), None]); + assert_eq!(value["steps"][1]["repeatMeaning"], REPEAT_MEANING); + assert!(value["steps"][0].get("repeatMeaning").is_none()); + } + #[test] fn planning_context_wire_order_is_stable_first_and_volatile_last() { // ADR-0036: provider prefix caches reuse only a leading run of identical tokens, so the diff --git a/crates/xgen-runtime/tests/durable_agent_loop.rs b/crates/xgen-runtime/tests/durable_agent_loop.rs index e3a2518..6b91ee5 100644 --- a/crates/xgen-runtime/tests/durable_agent_loop.rs +++ b/crates/xgen-runtime/tests/durable_agent_loop.rs @@ -612,6 +612,7 @@ impl RunStore for StalePlanOutcomeStore { call_id, settlement: ModelCallSettlement::Rejected { reason: ModelCallRejectionReason::PlannerInvalidResponse, + proposal: None, }, }, }; @@ -1081,6 +1082,7 @@ fn projected_state( settled_calls: accepted_model_turns, unknown_calls: 0, active_call: None, + last_rejection: None, }), completion_candidate: None, }), @@ -1660,6 +1662,77 @@ fn duplicate_batch_actions_are_rejected_but_a_completed_action_may_recur() { assert_model_call_counters(&after_replan, 2, 2, 0, 2); } +#[test] +fn an_immediate_repeat_of_a_finished_read_only_action_is_rejected() { + // ADR-0064: the same read-only input with nothing in between returns the known result. + let definition = read_only_definition_fixture("xgeny.test/read-marker"); + let cap = capability(&definition); + let registry = registry_with(definition); + let run_id = "run-agent-loop-read-repeat"; + let mut initial_store = MemoryRunStore::new(); + create_run(&mut initial_store, run_id); + let agent_loop = configure(&mut initial_store, run_id, &budget(4, 8, 8, 262_144)); + let proposal = || { + PlanProposal::plan(vec![proposed_step( + "read", + Vec::new(), + cap.clone(), + CANONICAL_PATH, + "same-read", + )]) + }; + let initial = agent_loop + .tick( + &mut initial_store, + &mut DeterministicEvents, + &FixedLease(run_id.to_owned()), + ®istry, + &CanonicalResolver::default(), + &mut ScriptedPlanner::returning(proposal()), + &mut RecordingMaterializer::default(), + ) + .expect("initial read should plan"); + let AgentLoopTick::PlanAccepted { step_ids, .. } = initial else { + panic!("expected initial plan") + }; + let mut finished = initial_store.load_current().unwrap().unwrap(); + let step = finished.steps.get_mut(&step_ids[0]).unwrap(); + step.status = StepStatus::Completed; + step.execution_receipt_id = Some("receipt-read-repeat".to_owned()); + step.execution_receipt_digest = Some(format!("sha256:{}", "d".repeat(64))); + let mut store = ProjectionStore::new(finished.clone()); + let repeat = agent_loop + .tick( + &mut store, + &mut DeterministicEvents, + &FixedLease(run_id.to_owned()), + ®istry, + &CanonicalResolver::default(), + &mut ScriptedPlanner::returning(proposal()), + &mut RecordingMaterializer::default(), + ) + .expect("a repeat should be classified"); + assert!(matches!( + repeat, + AgentLoopTick::ProposalRejected { + reason: ProposalRejection::RepeatedFinishedAction, + .. + } + )); + let after = store.load_current().unwrap().unwrap(); + assert_workgraph_unchanged(&finished, &after); + let feedback = after + .agent_loop + .as_ref() + .and_then(|loop_state| loop_state.model_calls.as_ref()) + .and_then(|calls| calls.last_rejection) + .expect("the rejection should be kept for the next call"); + assert_eq!( + feedback.proposal, + Some(xgen_workgraph::ProposalRejectionCode::RepeatedFinishedAction) + ); +} + #[test] fn sqlite_reopen_restores_atomic_plan_without_calling_planner() { let directory = tempdir().expect("temp directory should exist"); diff --git a/crates/xgen-workgraph/src/lib.rs b/crates/xgen-workgraph/src/lib.rs index 33702c2..7627a29 100644 --- a/crates/xgen-workgraph/src/lib.rs +++ b/crates/xgen-workgraph/src/lib.rs @@ -1570,6 +1570,47 @@ pub enum ModelCallRejectionReason { StaleHead, } +/// Closed, value-free class of a proposal Core rejected (ADR-0064). +/// +/// It names the rule the proposal broke, never its content, so it is safe to journal and to show +/// the planner on its next turn. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ProposalRejectionCode { + EmptyPlan, + TooManySteps, + TooManyEdges, + ProposalTooLarge, + InvalidStepKey, + DuplicateStepKey, + InvalidObjective, + InvalidDependency, + DuplicateDependency, + UnknownDependency, + BlockedExistingDependency, + SelfDependency, + DependencyCycle, + CapabilityUnavailable, + CapabilityUnsupported, + InvocationInvalid, + DuplicateSemanticAction, + RepeatedFinishedAction, + PlannedStepBudgetExceeded, + ToolCallBudgetExhausted, + CompletionWithoutReceiptCompletedPlan, + ConversationResponseWithPlan, + InvalidCompletionSummary, +} + +/// The latest settled model call, kept only while it was a rejection and no call followed it. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct ModelCallRejectionFeedback { + pub reason: ModelCallRejectionReason, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub proposal: Option, +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum ModelCallAbandonmentReason { @@ -1594,8 +1635,16 @@ pub enum ModelCallStatus { rename_all_fields = "camelCase" )] pub enum ModelCallSettlement { - Rejected { reason: ModelCallRejectionReason }, - Abandoned { reason: ModelCallAbandonmentReason }, + Rejected { + reason: ModelCallRejectionReason, + /// Fixed Core class of a `proposal_rejected` settlement (ADR-0064); absent for other + /// reasons and for settlements recorded before it existed. + #[serde(default, skip_serializing_if = "Option::is_none")] + proposal: Option, + }, + Abandoned { + reason: ModelCallAbandonmentReason, + }, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -1614,6 +1663,10 @@ pub struct ModelCallLifecycleState { pub unknown_calls: u32, #[serde(default, skip_serializing_if = "Option::is_none")] pub active_call: Option, + /// Rejection the next planner call should learn about (ADR-0064). Set when a call settles + /// as rejected and cleared when the next call is reserved. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub last_rejection: Option, } /// Durable limits for the model-owned portion of one local Runtime-mode Run. @@ -3862,6 +3915,7 @@ fn configure_model_call_lifecycle( settled_calls: loop_state.accepted_model_turns, unknown_calls: 0, active_call: None, + last_rejection: None, }); validate_model_call_counters(loop_state) } @@ -3917,6 +3971,7 @@ fn reserve_model_call( return Err(TransitionError::ModelCallBaseHeadMismatch); } calls.reserved_calls = expected_call_index; + calls.last_rejection = None; calls.active_call = Some(ModelCallState { reservation: reservation.clone(), status: ModelCallStatus::Reserved, @@ -3978,7 +4033,7 @@ fn settle_model_call( return Err(TransitionError::ModelCallIdMismatch); } match settlement { - ModelCallSettlement::Rejected { reason } => { + ModelCallSettlement::Rejected { reason, .. } => { if active.status != ModelCallStatus::Reserved { return Err(TransitionError::ModelCallNotReserved); } @@ -3994,6 +4049,14 @@ fn settle_model_call( } ModelCallSettlement::Abandoned { .. } => {} } + calls.last_rejection = match settlement { + ModelCallSettlement::Rejected { reason, proposal } + if reason != ModelCallRejectionReason::StaleHead => + { + Some(ModelCallRejectionFeedback { reason, proposal }) + } + _ => None, + }; calls.settled_calls = calls .settled_calls .checked_add(1) diff --git a/crates/xgen-workgraph/tests/durable_model_call.rs b/crates/xgen-workgraph/tests/durable_model_call.rs index 1c94e7b..a3c9f49 100644 --- a/crates/xgen-workgraph/tests/durable_model_call.rs +++ b/crates/xgen-workgraph/tests/durable_model_call.rs @@ -488,6 +488,7 @@ fn rejected_calls_consume_call_budget_but_not_accepted_turn_budget() { call_id: call.call_id().to_owned(), settlement: ModelCallSettlement::Rejected { reason: ModelCallRejectionReason::PlannerInvalidResponse, + proposal: None, }, }, ); @@ -783,6 +784,7 @@ fn unknown_call_blocks_late_response_until_explicit_abandonment() { call_id: call.call_id().to_owned(), settlement: ModelCallSettlement::Rejected { reason: ModelCallRejectionReason::ProposalRejected, + proposal: None, }, }, ), @@ -853,6 +855,7 @@ fn stale_head_requires_stale_rejection_and_other_rejections_require_immediate_he call_id: call.call_id().to_owned(), settlement: ModelCallSettlement::Rejected { reason: ModelCallRejectionReason::ProposalRejected, + proposal: None, }, }, ), @@ -865,6 +868,7 @@ fn stale_head_requires_stale_rejection_and_other_rejections_require_immediate_he call_id: call.call_id().to_owned(), settlement: ModelCallSettlement::Rejected { reason: ModelCallRejectionReason::StaleHead, + proposal: None, }, }, ); @@ -896,6 +900,7 @@ fn stale_head_requires_stale_rejection_and_other_rejections_require_immediate_he call_id: immediate_call.call_id().to_owned(), settlement: ModelCallSettlement::Rejected { reason: ModelCallRejectionReason::StaleHead, + proposal: None, }, }, ), diff --git a/docs/adr/0061-rejected-response-retry.md b/docs/adr/0061-rejected-response-retry.md index dfc724c..2bdab7c 100644 --- a/docs/adr/0061-rejected-response-retry.md +++ b/docs/adr/0061-rejected-response-retry.md @@ -33,7 +33,8 @@ Claude Code와 같은 작업 6개를 비교한 실험(devhub-social 사본, deep provider 한도·4xx·rate limit, 그리고 예산 소진 거절(`tool_call_budget_exhausted`, `planned_step_budget_exceeded`)도 다시 보내도 결과가 같으므로 재시도하지 않는다. 5. 재시도할 때 거절 이유를 모델 context에 넣지 않는다. 지금 `/resume`과 같은 재표본이다. 이유를 넣으려면 - planning context 계약을 바꿔야 하므로 별도 결정으로 미룬다. + planning context 계약을 바꿔야 하므로 별도 결정으로 미룬다. 이후 [ADR-0064](0064-planning-feedback.md)에서 + 거절 이유를 다음 context에 넣도록 바꿨다. `xgen run`(headless)도 같은 driver를 쓰므로 거절 exit code 20은 세 번 연속 거절된 뒤에 나온다. diff --git a/docs/adr/0064-planning-feedback.md b/docs/adr/0064-planning-feedback.md new file mode 100644 index 0000000..d02f02c --- /dev/null +++ b/docs/adr/0064-planning-feedback.md @@ -0,0 +1,60 @@ +# ADR-0064: 거절 이유와 반복 사실을 planning context에 넣기 + +- 상태: Accepted +- 기준일: 2026-10-26 +- 변경 분류: 범용 planning context·Core 규칙. 특정 모델·작업·업무 규칙 없음. +- 기존 경계: ADR-0036(prefix 순서), ADR-0055(`failureMeaning`), ADR-0061(거절 응답 재시도) +- protocol 변경: planning context에 선택 필드 3개(`previousResponseRejection`, Step의 `repeatsStep`·`repeatMeaning`)가 + 붙는다. 값이 없으면 직렬화하지 않으므로 기존 context와 바이트가 같다. profile version은 v3 그대로다. +- journal 변경: `ModelCallSettled`의 `Rejected` settlement에 선택 필드 `proposal`(Core 거절 code)이 붙는다. + 없으면 직렬화하지 않으므로 기존 journal을 그대로 읽는다. model call 상태에 `last_rejection`이 생긴다. + +## 문제 + +ADR-0061은 거절된 응답을 2번까지 다시 묻지만 이유는 알려 주지 않았다. 모델은 같은 실수를 되풀이할 수 있다. + +더 큰 문제는 반복이다. `~/projects/app` 아래 dirty 저장소 찾기(정답 24개, `--trust`, deepseek flash)에서 모델이 +같은 `bash` 명령을 54~72번 다시 실행하며 96 turn을 다 쓰는 경우가 나왔다. rename 작업에서도 이미 읽은 파일을 +같은 인자로 세 번 다시 읽었다. context에는 같은 결과가 이미 있었지만 모델이 그것이 반복이라는 사실을 직접 +비교해야 했다. + +## 결정 + +1. **거절 이유 전달.** 바로 앞 model call이 거절되었으면 다음 context에 `previousResponseRejection` + (`reason`, Core 거절이면 `proposalRejection` code, 고정 영어 문장 `meaning`)을 넣는다. 새 call을 예약하면 + 지우고, 받아들여지면 비운다. stale head 거절은 모델 잘못이 아니므로 넣지 않는다. 문장은 code마다 정해져 + 있고 경로·인자 같은 값을 담지 않는다. +2. **반복 표시.** 끝난(Completed·Failed) Step이 앞서 끝난 Step과 capability·contract·`planInputDigest`가 같으면 + `repeatsStep`에 앞 Step ID를, `repeatMeaning`에 고정 문장을 붙인다. effect class와 상관없이 사실로 표시한다. +3. **읽기 반복 거절.** read-only capability를 마지막으로 끝난 Step과 같은 인자로 바로 다시 제안하면 + `proposal_rejected.repeated_finished_action`으로 거절한다. 사이에 다른 Step이 끝났으면 거절하지 않는다 + (대상이 바뀌었을 수 있다). 쓰기·프로세스 실행은 같은 입력이라도 결과가 달라질 수 있으므로 거절하지 않는다. + 이 거절은 ADR-0061 재시도 대상이며 다음 call은 1번의 이유를 받는다. + +## 대안 + +- **반복 표시만 하고 거절하지 않음:** 측정에서 모델이 `repeatsStep`을 자주 무시했다(같은 명령 54~72회). +- **모든 effect class에 거절 적용:** 반복 루프는 29~42초 만에 끊겼지만(이전 230초), 같은 patch를 다시 적용해 + 실패를 확인하던 작업이 회귀했다. 실패를 보고 다시 시도하는 쓰기 반복은 정상 경로일 수 있다. +- **이유를 넣지 않는 재표본(ADR-0061 그대로):** 같은 거절이 이어져 세 번째에 멈춘다. + +## 검증 + +- 단위: 반복 Step 표시(같은 digest는 처음 끝난 Step을 가리킨다), read-only 즉시 반복 거절과 거절 code 기록. +- e2e: 잘못된 응답 뒤 재시도 요청에 `previousResponseRejection.reason`이 `planner_invalid_response`로 들어간다. + capability 거절은 `capability_unavailable`, 스키마 거절은 `invocation_invalid`가 들어간다. +- 실사용: 아래 측정. + +## 측정 (2026-10-26, deepseek flash, `--trust`, 작업당 2회) + +| 작업 | 1회 | 2회 | 비고 | +| --- | --- | --- | --- | +| T2 dedupe | 통과 (28초) | 실패: `materialization_failed` | 모든 class 거절 때의 회귀(같은 patch 재적용 거절)는 사라졌다. 실패는 이 결정과 무관한 material 오류다. | +| T3 rename | 참조 137개 중 2개 남음 (140초) | `planner_invalid_response` 3연속으로 멈춤 (60개 남음) | 1회에서 읽기 반복 거절 1번 뒤 이어서 진행했다. invalid response는 변경 전 build에서도 같은 작업에서 run당 2~7개 나왔다. | +| T4 dirty 저장소 | 16/24, 8초 | 같은 `bash` 명령 반복으로 96 turn 소진 (217초) | `bash`는 non_idempotent라 거절 대상이 아니다. 1회는 worktree(`.git`이 파일)를 놓쳤다. | +| T1 | 통과 (15초) | | | +| T6 | 스크립트 생성, `PYTHONPATH` 없이 실행하면 import 실패 | | 모델 품질 문제, 반복·거절 없음 | + +해석: 이 결정으로 생긴 회귀는 보이지 않는다. 읽기 반복 거절은 짧은 루프를 끊지만, 측정에서 가장 긴 루프는 +프로세스 실행 반복이었고 이 결정은 그것을 막지 않는다. 같은 명령 반복은 결과를 비교해야 판단할 수 있어 +다음 결정으로 남긴다. 거절 이유가 다음 응답을 고치는지는 표본이 작아 판단하지 않는다. diff --git a/docs/getting-started.md b/docs/getting-started.md index 6903239..7c79a57 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -501,7 +501,7 @@ profile 저장소를 격리하려면 `XGEN_CONFIG_HOME`을 따로 설정한다. | `model_not_advertised` 또는 compatibility 실패 | URL이 bare host가 아니라 provider의 API root인지, exact served model ID와 strict JSON Schema Chat Completions 지원을 확인한다. Redirect나 자동 retry에 의존하지 않는다. | | `model setup`/`model check`의 `timeout` | Inference probe는 production planner와 같은 wall-clock 예산을 쓴다. 로컬 model은 첫 로드가 크기와 disk cache 상태에 따라 수십 초 걸릴 수 있으므로 endpoint에서 model을 한 번 warm-up한 뒤 재시도한다. | | `provider_output_truncated` | Probe나 planner 응답이 출력 token 예산에서 잘렸다. Reasoning을 많이 쓰는 model은 최종 JSON 전에 예산을 소진할 수 있으므로 model의 thinking 설정이나 profile의 출력 예산을 조정한다. Rate limit이 아니므로 재시도로 해결되지 않는다. | -| `proposal_rejected.*` | 뒤의 class가 Core가 제안을 거부한 이유다. `capability_unavailable`/`capability_unsupported`는 허용하지 않은 capability 선택, `invocation_invalid`는 scope 밖 인자나 스키마 위반, `tool_call_budget_exhausted`는 예산 소진이다. 예산 소진이 아닌 거절은 연속 2번까지 자동으로 다시 묻고 세 번째에 멈춘다([ADR-0061](adr/0061-rejected-response-retry.md)). Class는 Core 판정이며 model 출력 원문이 아니다. | +| `proposal_rejected.*` | 뒤의 class가 Core가 제안을 거부한 이유다. `capability_unavailable`/`capability_unsupported`는 허용하지 않은 capability 선택, `invocation_invalid`는 scope 밖 인자나 스키마 위반, `tool_call_budget_exhausted`는 예산 소진, `repeated_finished_action`은 방금 끝난 읽기를 같은 인자로 다시 제안한 경우다. 예산 소진이 아닌 거절은 연속 2번까지 자동으로 다시 묻고 세 번째에 멈추며, 다시 물을 때 거절 이유를 함께 준다([ADR-0061](adr/0061-rejected-response-retry.md), [ADR-0064](adr/0064-planning-feedback.md)). Class는 Core 판정이며 model 출력 원문이 아니다. | | `model_rejected.*` | 뒤의 class는 journal의 model call settlement와 같은 값이다. `planner_invalid_response`는 provider가 strict JSON Schema를 지키지 않은 응답(문법 미지원·미적용), `provider_limit`은 출력 예산·요청 크기 초과, `provider_rejected`는 4xx 거부다. `planner_invalid_response`만 연속 2번까지 자동으로 다시 묻는다(ADR-0061). Class는 Core 판정이며 model 출력 원문이 아니다. | | `configuration_mismatch` | 원래 workspace, file/directory scope, executable와 model profile binding(inference timeout·출력 예산 포함)으로 resume한다. 자동 대체하지 말고 필요하면 새 Run을 시작한다. | | `model_call_unknown.timeout`이 planner 호출마다 반복 | 프로필의 inference timeout이 model·hardware에 비해 짧다. 로컬 27B는 호출당 60초 안팎이 걸리므로 `--inference-timeout`을 올린다. |