State machine re-executes a completed tool call when the process dies before its response is persisted
- Langage dominant
- Rust
- Étoiles
- 54.2k
- Forks
- 6.2k
- Merge moyen
- 3 j 4 h
- PR mergées (30 j)
- 240
Description
**Describe the bug**
In the state-machine loop (`GOOSE_STATE_MACHINE=1`, the path #11214 is rolling out to Desktop), a tool's side effect happens inside `StateMachine::step` and its `ToolResponse` is only written to the session store later, in `apply`. Nothing is persisted before dispatch that says "this request was already dispatched". If goose exits between the two (crash, kill, OOM, panic, or `apply` failing on the DB write), the session ends up with an approved `ToolRequest` and no `ToolResponse`, which `pending_tool_requests` classifies as `Execute` again.
That state is exactly what `load_session` auto-resumes: `has_unapplied_tool_confirmation_response` is true (approval request + approval response, no tool response), so reopening the session runs `resume_state_machine_turn` → `ToolExecutionOperation`, and the tool runs a second time without any user action or model call. For a `shell` command, a `git push`, or a "send message" tool, that is a duplicated side effect on every reopen.
Graceful cancellation is already handled: `ToolExecutionOperation::run` writes the `"Tool call was interrupted before completing"` error response when the cancel token fires. The hole is only the process-death / apply-failure window.
Relevant code (main @ 794b04a):
- `crates/goose-agent/src/machine.rs` — `step` returns effects, `apply` persists them, `run` = step → apply.
- `crates/goose/src/agents/state_machine/ops_toolcalling.rs` `ToolExecutionOperation::run` — `dispatch_tool_call` is awaited inside `step`; the response is returned as an `AppendMessage` effect.
- `crates/goose/src/agents/state_machine/ops_toolcalling.rs` `pending_tool_requests` — any unanswered request is `Execute`.
- `crates/goose/src/acp/server/load_session.rs` `should_resume_state_machine` and `crates/goose/src/agents/agent.rs` `resume_state_machine_turn` — the auto-resume path.
---
**To Reproduce**
**Live** (debug build of `main` @ 794b04a with the one-line `load_session` fix from #11807 applied, otherwise the resume never fires; `GOOSE_STATE_MACHINE=1`, `GOOSE_MODE=approve`, `goose acp --with-builtin developer`, ACP over stdio, mock OpenAI endpoint returning one `shell` call):
1. `session/prompt`. The model asks for `shell` with `echo ran-at-$(date +%s.%N) >> marker; kill -9 `. Approve it (`allow_once`).
The command runs (marker has 1 line) and goose is gone before the tool response is persisted.
Persisted rows: user prompt, turn-context, assistant `toolRequest`, assistant `actionRequired/toolConfirmation`, user `actionRequired/toolConfirmationResponse` (`allow_once`). No `toolResponse`.
2. Start `goose acp` again and `session/load` the same session.
goose auto-resumes and runs the shell command again, with no user action and no model request: the marker now has 2 lines (0.7 s apart), goose is killed again, and every further reopen repeats it.
Same steps on the legacy loop (env var unset): nothing runs on load.
**Unit test** on `main` (`crates/goose/src/agents/state_machine/tests/tool_lifecycle.rs`), independent of #11807. It drives the turn step by step the way `run_goose` does, drops the `tool_execution` step result instead of applying it (the process-death window), checks that the store now looks like an approved-but-unexecuted request, then resumes:
```rust
#[tokio::test]
async fn approved_tool_is_not_executed_again_when_its_response_was_never_persisted() -> Result<()> {
let (pipeline, api) = test_pipeline().await?;
let pipeline = pipeline.with_goose_mode(GooseMode::Approve).await;
api.on("add one").calls([("approved", ADD, value(1))]);
api.on("result: 1").reply("added once");
api.on("result: 2").reply("added twice");
pipeline.run(["add one"]).await?;
assert_eq!(pipeline.calculator_total(), 0);
pipeline.confirm("approved", Permission::AllowOnce).await?;
let cancel = tokio_util::sync::CancellationToken::new();
let (tx, _rx) = tokio::sync::mpsc::channel(1024);
let emit = crate::agents::state_machine::Emitter::new(tx, cancel.clone());
let machine = pipeline.machine(cancel.clone());
let executed = loop {
let session = pipeline.session().await?;
let mut result = machine.step(&session, &emit).await?.expect("turn continues");
if result.applied_step == Some("tool_execution") {
break result;
}
machine.apply(pipeline.session_manager.as_ref(), &session, &mut result, &emit).await?;
};
assert_eq!(pipeline.calculator_total(), 1, "the tool ran to completion");
drop(executed); // the response never reaches the session store
drop(machine);
let session = pipeline.session().await?;
let conversation = session.conversation.as_ref().expect("conversation");
assert!(conversation.messages().iter().all(|m| m.get_tool_response_ids().is_empty()));
assert!(crate::agents::state_machine::has_unapplied_tool_confirmation_response(conversation));
let result = pipeline.resume().await?;
assert_eq!(pipeline.calculator_total(), 1, "resuming must not execute the completed tool call a second time");
result.assert_message(-1, Agent, "added once");
Ok(())
}
```
```
cargo test -p goose --lib state_machine::tests::tool_lifecycle::approved_tool_is_not_executed_again
```
```
assertion `left == right` failed: resuming must not execute the completed tool call a second time
left: 2
right: 1
test agents::state_machine::tests::tool_lifecycle::approved_tool_is_not_executed_again_when_its_response_was_never_persisted ... FAILED
```
The other 13 tests in the file pass.
---
**Expected behavior**
A tool request that was already dispatched is never dispatched again on re-entry. After a crash it should be answered the same way a cancelled call is (`"Tool call was interrupted before completing"`), and the model told, rather than the tool re-run.
One way to get there without a new persistence primitive: emit a `PatchToolRequestMeta` effect marking the request as dispatched and apply it *before* `dispatch_tool_call` (the same channel `ToolApprovalOperation` already uses for `executable`). On re-entry, a request that is marked dispatched but unanswered gets the interrupted error response instead of `ToolDisposition::Execute`. Happy to do this once the issue is Ready, or to adjust if you prefer a different shape.
---
**Please provide the following information**
- **OS & Arch:** macOS arm64 (Darwin 25.4)
- **Interface:** ACP (`goose acp`) and the state-machine test harness, `GOOSE_STATE_MACHINE=1`
- **Version:** main @ 794b04a (2026-09-03)
- **Extensions enabled:** developer (any tool with side effects reproduces)
- **Provider & Model:** provider-independent; mock OpenAI-compatible endpoint / in-repo dummy provider
---
**Additional context**
- The legacy `reply` loop does not have this exposure: it persists neither the tool request nor the confirmation before the tool completes, and nothing re-enters it on load. The state-machine design is explicitly "re-entrant over persisted state" (`tests/pipeline.rs::run_reconstructing_each_step` exercises exactly this), so tool execution is the one step that is not safe to re-enter.
- With a fresh user message the kickoff boundary hides the old request, so the duplicate only happens on automatic resume. Today that resume is masked by #11807; fixing #11807 (which #11685 clearly intends) makes this reproduce on every reopen.
Guide de contribution
Ouvrir le guide de contribution
Piste de recherche
The bug is in the state machine's tool execution persistence. Start by reading crates/goose-agent/src/machine.rs to understand the step/apply/run flow, then examine crates/goose/src/agents/state_machine/ops_toolcalling.rs for ToolExecutionOperation::run and pending_tool_requests. The test in crates/goose/src/agents/state_machine/tests/tool_lifecycle.rs shows the exact failure. The fix likely involves marking a ToolRequest as dispatched before the tool runs, using a PatchToolRequestMeta effect. Run the failing test to confirm the duplicate execution, then implement the pre-dispatch persistence.
Rédigé par le modèle d'indexation à partir du texte de l'issue.
Évaluation
- Stack technique
- rust
- Domaine
- backend-api-design
- Type d'issue
- Bug
- Difficulté
- 4/5
- Temps estimé
- 3-5 jours
- Activité
- Active
- Clarté
- Clairement spécifiée
- Accessibilité débutants
- 45/100