Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion crates/xgen-cli/src/composition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1549,7 +1549,13 @@ impl PlanMaterializer for LocalMaterializer {
reference: &ReconstructableMaterialReference,
material_digest: &str,
) -> Option<serde_json::Value> {
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,
Expand Down
17 changes: 17 additions & 0 deletions crates/xgen-cli/src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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);
}
Expand Down Expand Up @@ -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,
Expand Down
63 changes: 57 additions & 6 deletions crates/xgen-cli/src/material_catalog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Value, MaterialProviderFailure> {
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,
Expand Down Expand Up @@ -534,13 +574,8 @@ impl PlanMaterializer for ProcessMaterializer {
reference: &ReconstructableMaterialReference,
material_digest: &str,
) -> Option<Value> {
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()
}
}
Expand Down Expand Up @@ -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()
);
}
}

Expand Down
90 changes: 86 additions & 4 deletions crates/xgen-cli/tests/interactive_repl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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"])
);
}
}

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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!(
Expand Down
23 changes: 16 additions & 7 deletions crates/xgen-runtime/src/agent_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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<S, M>(
store: &S,
materializer: &M,
registry: &CapabilityRegistry,
state: &RunState,
) -> BTreeMap<String, Value>
where
Expand All @@ -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;
};
Expand All @@ -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,
Expand Down
65 changes: 65 additions & 0 deletions docs/adr/0067-step-arguments-and-repeat-limit.md
Original file line number Diff line number Diff line change
@@ -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 괄호
오류이고, 별도 결정으로 다룬다.
Loading