diff --git a/crates/xgen-cli/src/composition.rs b/crates/xgen-cli/src/composition.rs index 1e2f3ac..8e209cb 100644 --- a/crates/xgen-cli/src/composition.rs +++ b/crates/xgen-cli/src/composition.rs @@ -1549,7 +1549,13 @@ impl PlanMaterializer for LocalMaterializer { reference: &ReconstructableMaterialReference, material_digest: &str, ) -> Option { - if capability.capability_id == PROCESS_EXECUTE_CAPABILITY_ID { + if (capability.capability_id == PROCESS_EXECUTE_CAPABILITY_ID + && capability.contract_version == PROCESS_EXECUTE_CONTRACT_VERSION) + || (capability.capability_id == WEB_SEARCH_CAPABILITY_ID + && capability.contract_version == WEB_SEARCH_CONTRACT_VERSION) + || (TerminalOperation::from_capability(&capability.capability_id).is_some() + && capability.contract_version == TERMINAL_VERSION) + { return self.process.as_ref()?.planned_arguments( step_id, capability, diff --git a/crates/xgen-cli/src/driver.rs b/crates/xgen-cli/src/driver.rs index 49fc67a..3881a08 100644 --- a/crates/xgen-cli/src/driver.rs +++ b/crates/xgen-cli/src/driver.rs @@ -146,6 +146,12 @@ pub enum DriverProgressControl { /// Each retry is a new journaled model call and spends the Run's model-call budget. pub const MAX_REJECTED_RESPONSE_RETRIES: u32 = 2; +/// Consecutive `repeated_finished_action` rejections the driver re-asks before it stops (ADR-0067). +/// +/// A repeated read runs nothing and the model usually moves on within a few re-asks; a longer +/// streak in the measured Runs never ended before ten more calls. +pub const MAX_REPEATED_ACTION_RETRIES: u32 = 8; + /// Provider-neutral bounded coordinator used by CLI composition roots and hermetic tests. #[derive(Debug, Clone)] pub struct RunDriver { @@ -318,6 +324,7 @@ impl RunDriver { O: FnMut(DriverProgress) -> DriverProgressControl, { let mut rejected_responses_in_a_row = 0_u32; + let mut repeated_actions_in_a_row = 0_u32; for _ in 0..self.max_ticks.get() { if model_call_may_be_next(store)? { if !model_egress_allowed { @@ -342,6 +349,7 @@ impl RunDriver { | AgentLoopTick::ModelCallAbandoned { .. } => {} AgentLoopTick::PlanAccepted { .. } => { rejected_responses_in_a_row = 0; + repeated_actions_in_a_row = 0; if observer(DriverProgress::PlanCommitted) == DriverProgressControl::Cancel { return Ok(DriverOutcome::Cancelled); } @@ -505,6 +513,15 @@ impl RunDriver { return Ok(DriverOutcome::PlannerUnavailable(failure)); } AgentLoopTick::ProposalRejected { reason, .. } => { + // A repeated read runs nothing and costs one model call; about half the time + // the model moves on once told, so it gets its own, longer limit (ADR-0067). + if reason == ProposalRejection::RepeatedFinishedAction { + if repeated_actions_in_a_row < MAX_REPEATED_ACTION_RETRIES { + repeated_actions_in_a_row += 1; + continue; + } + return Ok(DriverOutcome::ProposalRejected(reason)); + } // A spent budget rejects every later proposal the same way. let budget_spent = matches!( reason, diff --git a/crates/xgen-cli/src/material_catalog.rs b/crates/xgen-cli/src/material_catalog.rs index 10fc290..fe23d86 100644 --- a/crates/xgen-cli/src/material_catalog.rs +++ b/crates/xgen-cli/src/material_catalog.rs @@ -263,6 +263,46 @@ impl RunMaterialCatalog { Ok(record.arguments) } + /// Arguments of a process, terminal, or web search Step, bound to its material digest. + fn process_family_arguments( + &self, + reference: &ReconstructableMaterialReference, + step_id: &str, + material_digest: &str, + capability: &CapabilityRef, + ) -> Result { + let profile = if capability.capability_id == PROCESS_EXECUTE_CAPABILITY_ID + && capability.contract_version == PROCESS_EXECUTE_CONTRACT_VERSION + { + PROCESS_RECIPE_PROFILE + } else if capability.capability_id == WEB_SEARCH_CAPABILITY_ID + && capability.contract_version == WEB_SEARCH_CONTRACT_VERSION + { + WEB_SEARCH_RECIPE_PROFILE + } else if TerminalOperation::from_capability(&capability.capability_id).is_some() + && capability.contract_version == TERMINAL_VERSION + { + TERMINAL_RECIPE_PROFILE + } else { + return Err(MaterialProviderFailure::RevisionChanged); + }; + if reference.provider_id() != profile.provider_id { + return Err(MaterialProviderFailure::RevisionChanged); + } + let record = self.reconstruct_record_with_profile( + reference.reference_id(), + reference.revision(), + profile, + )?; + if record.step_id != step_id + || record.material_digest != material_digest + || record.capability != *capability + { + return Err(MaterialProviderFailure::RevisionChanged); + } + Ok(record.arguments) + } + /// Arguments of a workspace Step exactly as planned and approved, bound to its material digest. pub(crate) fn workspace_arguments( &self, @@ -534,13 +574,8 @@ impl PlanMaterializer for ProcessMaterializer { reference: &ReconstructableMaterialReference, material_digest: &str, ) -> Option { - if capability.capability_id != PROCESS_EXECUTE_CAPABILITY_ID - || capability.contract_version != PROCESS_EXECUTE_CONTRACT_VERSION - { - return None; - } self.catalog - .process_arguments(reference, step_id, material_digest) + .process_family_arguments(reference, step_id, material_digest, capability) .ok() } } @@ -1015,6 +1050,22 @@ mod tests { MaterialProviderFailure::RevisionChanged ); } + // ADR-0067: the planner sees the arguments only under the recorded capability. + assert_eq!( + reopened + .process_family_arguments(&reference, &input.step_id, DIGEST, &input.capability) + .unwrap(), + input.arguments + ); + let process = CapabilityRef { + capability_id: PROCESS_EXECUTE_CAPABILITY_ID.to_owned(), + contract_version: PROCESS_EXECUTE_CONTRACT_VERSION.to_owned(), + }; + assert!( + reopened + .process_family_arguments(&reference, &input.step_id, DIGEST, &process) + .is_err() + ); } } diff --git a/crates/xgen-cli/tests/interactive_repl.rs b/crates/xgen-cli/tests/interactive_repl.rs index de4ce92..bfdddff 100644 --- a/crates/xgen-cli/tests/interactive_repl.rs +++ b/crates/xgen-cli/tests/interactive_repl.rs @@ -208,6 +208,12 @@ fn bare_xgen_streams_durable_progress_prompts_separate_approvals_and_replays_off assert_eq!(request_model(&first_request), MODEL); assert_eq!(request_model(&process_request), MODEL); let context = planning_context(&completion_request); + // ADR-0065/0067: the finished process run shows the command it ran; the read does not. + let steps = context["steps"] + .as_array() + .expect("steps should be present"); + assert!(steps[0].get("arguments").is_none()); + assert_eq!(steps[1]["arguments"]["args"], json!(["--version"])); let output = context["toolOutputs"] .as_array() .expect("tool outputs should be present") @@ -420,6 +426,14 @@ fn rejected_final_response_resumes_after_restart_without_replaying_writes_or_fai .find(|o| o["capability"]["capabilityId"] == "xgeny.process/execute") .unwrap(); assert_eq!(process["output"]["success"], false); + // ADR-0065/0067: after a restart the command still shows its arguments; writes do not. + let steps = context["steps"].as_array().unwrap(); + assert!(steps[0].get("arguments").is_none()); + assert!(steps[1].get("arguments").is_none()); + assert_eq!( + steps[2]["arguments"]["args"], + json!(["--definitely-invalid-option"]) + ); } } @@ -461,6 +475,73 @@ fn headless_run_keeps_the_legacy_contract_and_rejects_conversation_proposals() { ); } +#[test] +fn repeated_reads_have_their_own_longer_retry_limit() { + // ADR-0067: up to MAX_REPEATED_ACTION_RETRIES read repeats in a row are re-asked; one more + // stops the Run with the same rejection. + for repeats in [ + xgen_cli::MAX_REPEATED_ACTION_RETRIES, + xgen_cli::MAX_REPEATED_ACTION_RETRIES + 1, + ] { + let fixture = tempdir().unwrap(); + fs::write(fixture.path().join("README.md"), "repeat fixture\n").unwrap(); + let read = || { + plan_response( + "read", + "Read README", + "xgeny.fs/read-text", + &json!({"path":"README.md"}), + ) + }; + let mut responses = vec![read()]; + responses.extend((0..repeats).map(|_| read())); + let completes = repeats == xgen_cli::MAX_REPEATED_ACTION_RETRIES; + if completes { + responses.push(completion_response()); + } + let calls = responses.len(); + let server = SequentialServer::with_responses(responses); + let output = Command::new(env!("CARGO_BIN_EXE_xgen")) + .args([ + "run", + "Read README", + "--allow-dir", + ".", + "--allow-remote-model-egress", + "--allow-read", + ]) + .current_dir(fixture.path()) + .env("XGEN_STATE_HOME", fixture.path().join("state")) + .env("XGEN_CONFIG_HOME", fixture.path().join("config")) + .env("XGEN_OPENAI_BASE_URL", &server.base_url) + .env("XGEN_OPENAI_MODEL", MODEL) + .env("XGEN_OPENAI_TOKENIZER", TOKENIZER) + .env("XGEN_OPENAI_RESPONSE_FORMAT", "json_object") + .env_remove("XGEN_OPENAI_API_KEY") + .output() + .unwrap(); + let requests: Vec<_> = (0..calls) + .map(|_| server.requests.recv_timeout(TEST_TIMEOUT).unwrap()) + .collect(); + server.handle.join().unwrap(); + if completes { + assert_eq!(output.status.code(), Some(0), "{}", stderr(&output)); + let last = &planning_context(&requests[calls - 1])["previousResponseRejection"]; + assert_eq!( + last["proposalRejection"], "repeated_finished_action", + "{last}" + ); + } else { + assert_eq!(output.status.code(), Some(20), "{}", stderr(&output)); + assert!( + stderr(&output).contains("proposal_rejected.repeated_finished_action"), + "{}", + stderr(&output) + ); + } + } +} + #[test] fn one_invalid_response_is_retried_and_the_run_completes_without_resume() { let fixture = tempdir().unwrap(); @@ -502,10 +583,11 @@ fn one_invalid_response_is_retried_and_the_run_completes_without_resume() { .map(|_| server.requests.recv_timeout(TEST_TIMEOUT).unwrap()) .collect(); server.handle.join().unwrap(); - // ADR-0065: the finished read shows the arguments it ran with. - assert_eq!( - planning_context(&requests[1])["steps"][0]["arguments"]["path"], - "workspace:primary/README.md" + // ADR-0067: only non-idempotent Steps show their arguments. + assert!( + planning_context(&requests[1])["steps"][0] + .get("arguments") + .is_none() ); // ADR-0064: only the call after the discarded response learns why it was discarded. assert!( diff --git a/crates/xgen-runtime/src/agent_loop.rs b/crates/xgen-runtime/src/agent_loop.rs index 49d49d5..4786721 100644 --- a/crates/xgen-runtime/src/agent_loop.rs +++ b/crates/xgen-runtime/src/agent_loop.rs @@ -1403,7 +1403,7 @@ impl AgentLoop { if planning_snapshot.state != state { return Err(AgentLoopError::PlanningSnapshotMismatch); } - let step_arguments = finished_step_arguments(store, materializer, &state); + let step_arguments = finished_step_arguments(store, materializer, capabilities, &state); let context = match build_context_ordered( &state, planning_snapshot.completed_tool_outputs(), @@ -2345,11 +2345,14 @@ fn build_context( /// Arguments of finished Steps, read back through the materializer that persisted them (ADR-0065). /// -/// A Step is skipped when its material is missing, ephemeral, bound to another digest, or larger -/// than `MAX_PLANNING_STEP_ARGUMENT_BYTES`; the context then shows it without arguments. +/// Only non-idempotent Steps (process and terminal runs) show arguments: their outputs do not say +/// which command produced them, while shown read and write arguments made the planner read the +/// same target again (ADR-0067). A Step is also skipped when its material is missing, ephemeral, +/// bound to another digest, or larger than `MAX_PLANNING_STEP_ARGUMENT_BYTES`. fn finished_step_arguments( store: &S, materializer: &M, + registry: &CapabilityRegistry, state: &RunState, ) -> BTreeMap where @@ -2364,6 +2367,16 @@ where let (Some(intent), Some(invocation)) = (&step.intent, &step.planned_invocation) else { continue; }; + let capability = CapabilityRef { + capability_id: invocation.capability_id().to_owned(), + contract_version: invocation.contract_version().to_owned(), + }; + let non_idempotent = registry + .definition(&capability) + .is_some_and(|definition| definition.spec.effect.class == EffectClass::NonIdempotent); + if !non_idempotent { + continue; + } let Ok(Some(material)) = store.load_invocation_material(&intent.effect_id) else { continue; }; @@ -2376,10 +2389,6 @@ where { continue; } - let capability = CapabilityRef { - capability_id: invocation.capability_id().to_owned(), - contract_version: invocation.contract_version().to_owned(), - }; let Some(value) = materializer.planned_arguments( &step.step_id, &capability, diff --git a/docs/adr/0067-step-arguments-and-repeat-limit.md b/docs/adr/0067-step-arguments-and-repeat-limit.md new file mode 100644 index 0000000..4d30b21 --- /dev/null +++ b/docs/adr/0067-step-arguments-and-repeat-limit.md @@ -0,0 +1,65 @@ +# ADR-0067: 인자는 non_idempotent Step에만, 읽기 반복 거절은 따로 8회까지 + +- 상태: Accepted +- 기준일: 2026-10-27 +- 변경 분류: 범용. ADR-0065의 적용 범위와 ADR-0064·0061의 거절 재시도 한도를 고친다. 특정 모델·작업· + capability 규칙 없음. 다만 한도 8은 아래 Run들에서 측정한 값이다. +- protocol·journal·schema 변경: 없음. + +## 문제 + +ADR-0065 뒤 rename 작업 두 개(T3, 검증용 H6)가 같은 방식으로 멈췄다. 모델이 파일을 읽은 직후 같은 파일을 +다시 읽으려 했고, ADR-0064의 읽기 반복 거절이 세 번 이어져 ADR-0061 한도에 걸렸다. + +### 어떤 Step의 인자가 다시 읽기를 부르는가 + +기록된 요청 재생. 마지막 Step이 끝난 읽기인 context 44개(H6·T3 Run 하나씩) x 2회에서 같은 파일을 곧바로 +다시 읽은 횟수다(deepseek flash). + +| context에 보인 인자 | 같은 파일 다시 읽기 | +| --- | --- | +| 모든 Step (ADR-0065) | 26/88 | +| read_only 제외 | 19/88 | +| 없음 | 7/88 | + +읽기 인자와 patch 인자 모두 다시 읽기를 늘렸다. patch 인자가 이미 적용한 patch를 다시 보내는 것을 막는지도 +보았다. 실패한 patch 직후 context 20개 x 2회에서 같은 patch를 다시 보낸 횟수는 인자 없음 22·23, 있음 14·23 +(Run 두 개)이었다. 실제 Run에서도 patch 인자가 보이는 build에서 같은 patch 53회 재전송이 나왔다. 효과가 없다. +반면 process 실행 인자는 같은 명령 반복을 없앴다(ADR-0065의 T4·H5). terminal 실행도 같은 모양이었다. +인자 지원이 없던 `terminal/start`로 같은 명령을 11번 다시 실행한 Run이 있었다. + +### 거절 한도 + +읽기 반복 거절 뒤 모델이 다른 일로 넘어간 비율은 31회 중 15회였다. 연속 거절 길이를 세면 33번 중 28번이 +8회 이내에 끝났다(1회 18, 2회 6, 3회 2, 6·8회 각 1). 나머지(11, 17, 31, 39, 48회)는 사실상 멈춘 상태로 +call을 소모했다. 한도가 없으면 T3 한 Run이 model call 192개를 다 썼다. + +## 결정 + +1. runtime은 effect class가 `non_idempotent`인 Step의 인자만 묻는다(process 실행, terminal, web search). + read_only·idempotent Step과 등록되지 않은 capability에는 인자를 넣지 않는다. +2. CLI는 process·terminal·web search recipe에서 기록된 capability와 같을 때만 인자를 돌려준다. +3. driver는 `repeated_finished_action` 거절을 다른 거절과 따로 센다. 연속 8번(`MAX_REPEATED_ACTION_RETRIES`)까지 + 다시 묻고 9번째에 같은 reason으로 멈춘다. 제안이 받아들여지면 두 counter 모두 0이 된다. 다른 거절은 ADR-0061대로 + 2번이다. + +## 검증 + +- 단위: terminal recipe의 인자는 기록된 capability로만 다시 읽힌다. +- e2e: 읽기와 git 실행을 한 Run에서 git Step만 `arguments`가 있다. 쓰기 두 개와 실패한 명령 뒤 재시작해도 + 명령 인자는 다시 만들어지고 쓰기에는 없다. 읽기 반복 8회 뒤 완료는 exit 0, 9회는 exit 20이다. +- 실사용 (deepseek flash, `--trust`, 이 build): + +| 작업 | 결과 | +| --- | --- | +| H5 `package.json` 디렉터리 (검증 사례) | 2 call 완료 2/2 | +| T4 dirty 저장소 | 3 call 완료 | +| T1, T2, T6 | 통과 (T2 숨은 검사 포함) | +| T3 rename (참조 137개) | 0/2. 읽기 반복 9연속으로 멈춤 1, patch 실패 44개로 turn 소진 1 | +| H6 rename (검증 사례) | 0/2. 읽기 반복 9연속으로 멈춤 1, `material_rejected` 1 | + +긴 rename 작업은 build마다 결과가 크게 흔들렸다. 같은 두 작업에서 거절 한도가 없던 중간 build는 T3 3/4·H6 1/4를 +완료했고 이 build는 0/4다. 표본 2~4개로는 build 차이와 모델 분산을 구분할 수 없어 긴 작업 완료율로 이 결정을 +판단하지 않는다. 결정의 근거는 위의 재생 실험(같은 context에서 조건만 바꾼 비교)이다. 긴 작업의 남은 실패는 +대부분 patch 작성 실패(`patch_context_ambiguous`·`patch_context_missing`, 한 Run에서 최대 44개)와 모델 JSON 괄호 +오류이고, 별도 결정으로 다룬다. diff --git a/docs/getting-started.md b/docs/getting-started.md index 7c79a57..07a7cf9 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`는 예산 소진, `repeated_finished_action`은 방금 끝난 읽기를 같은 인자로 다시 제안한 경우다. 예산 소진이 아닌 거절은 연속 2번까지 자동으로 다시 묻고 세 번째에 멈추며, 다시 물을 때 거절 이유를 함께 준다([ADR-0061](adr/0061-rejected-response-retry.md), [ADR-0064](adr/0064-planning-feedback.md)). Class는 Core 판정이며 model 출력 원문이 아니다. | +| `proposal_rejected.*` | 뒤의 class가 Core가 제안을 거부한 이유다. `capability_unavailable`/`capability_unsupported`는 허용하지 않은 capability 선택, `invocation_invalid`는 scope 밖 인자나 스키마 위반, `tool_call_budget_exhausted`는 예산 소진, `repeated_finished_action`은 방금 끝난 읽기를 같은 인자로 다시 제안한 경우다. 예산 소진이 아닌 거절은 연속 2번까지 자동으로 다시 묻고 세 번째에 멈추며, 다시 물을 때 거절 이유를 함께 준다. `repeated_finished_action`만은 따로 연속 8번까지 다시 묻는다([ADR-0061](adr/0061-rejected-response-retry.md), [ADR-0064](adr/0064-planning-feedback.md), [ADR-0067](adr/0067-step-arguments-and-repeat-limit.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`을 올린다. |