State machine re-executes a completed tool call when the process dies before its response is persisted
- Dominant language
- Rust
- Stars
- 54.2k
- Forks
- 6.2k
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 262
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.
Contributor guide
Assessment
This issue has not been assessed yet.