From a089af4bd27d0c601b13c40bed24bdec6e19f9fe Mon Sep 17 00:00:00 2001 From: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> Date: Sat, 17 Oct 2026 22:08:23 +0800 Subject: [PATCH] fix(acceptance): look up the canonical idempotency key on prepare retry The accepted-retry recovery compared the persisted canonical idempotency key with the caller's raw operation id, but ChatActionStore strips surrounding whitespace before it binds and persists that key. A request accepted as " recover-original-intent " was stored as "recover-original-intent", so retrying the identical command after a harmless note missed the saved proposal, built a new preview against a newer basis, and was then rejected by the idempotency binding. Normalize the operation id once at the prepare/recovery boundary through the store's own canonical form and reuse it for the saved lookup, the competing recovery and the preview. Genuinely different operations, Agents and documents stay rejected. Cover padded-to-padded and padded-to-canonical retries on real File and SQLite authority stores. Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> --- loopx/chat_action_store.py | 7 ++++++- .../control_plane/goals/acceptance_actions.py | 10 ++++++++- .../test_goal_acceptance_prepared_action.py | 21 +++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/loopx/chat_action_store.py b/loopx/chat_action_store.py index ccf22e793c..3601c7a6f3 100644 --- a/loopx/chat_action_store.py +++ b/loopx/chat_action_store.py @@ -101,6 +101,11 @@ def _opaque_id(value: Any, *, field: str) -> str: return token +def normalize_idempotency_key(value: Any) -> str: + """Return the canonical token this store binds and persists.""" + return _opaque_id(value, field="idempotency_key") + + def _bounded_text(value: Any, *, field: str, limit: int = 4000) -> str: text = str(value or "").strip() if not text or len(text) > limit or any(ord(character) < 32 for character in text): @@ -231,7 +236,7 @@ def create_preview( }, path="preview", ) - key = _opaque_id(idempotency_key, field="idempotency_key") + key = normalize_idempotency_key(idempotency_key) request_digest = _canonical_digest(request) with exclusive_file_lock( self.path, diff --git a/loopx/control_plane/goals/acceptance_actions.py b/loopx/control_plane/goals/acceptance_actions.py index 8bdeebd3fc..b56729e9a0 100644 --- a/loopx/control_plane/goals/acceptance_actions.py +++ b/loopx/control_plane/goals/acceptance_actions.py @@ -12,7 +12,11 @@ from typing import Any, TypedDict from uuid import uuid4 -from ...chat_action_store import ActionConflictError, ChatActionStore +from ...chat_action_store import ( + ActionConflictError, + ChatActionStore, + normalize_idempotency_key, +) from ...file_lock import exclusive_file_lock from .acceptance import ( configure_goal_acceptance, inspect_goal_acceptance, public_goal_acceptance, @@ -71,6 +75,10 @@ def prepare_acceptance_check(*, registry_path: Path, runtime_root: str | None, return {**public_goal_acceptance(preview), "prepared": False, "executed": False} service = ChatActionService(store=ChatActionStore(root / "chat" / "actions"), registry_path=registry_path) + # The store persists and binds the canonical token. Look up that same token + # so a re-sent request with surrounding whitespace is recovered as the + # accepted intent instead of becoming a conflicting new preview. + operation_id = normalize_idempotency_key(operation_id) parameters = service._normalize("goal.acceptance.check", { "goal_id": goal_id, "document": document, "prepared_by": agent_id}) proposal = _saved_check(service.store, parameters, operation_id) diff --git a/tests/control_plane/test_goal_acceptance_prepared_action.py b/tests/control_plane/test_goal_acceptance_prepared_action.py index d3b4a4540a..f619a0b4df 100644 --- a/tests/control_plane/test_goal_acceptance_prepared_action.py +++ b/tests/control_plane/test_goal_acceptance_prepared_action.py @@ -198,6 +198,27 @@ def test_preparation_retry_does_not_return_another_preparers_plan(conversation_c assert not (project / "runs.txt").exists() +@pytest.mark.parametrize("retry_spelling", ["padded", "canonical"]) +def test_preparation_retry_uses_the_canonical_operation_id(conversation_check, retry_spelling): + project, cli, controller, _, _, call, _ = conversation_check + padded = " recover-original-intent " + # The store strips and persists the canonical token; the first tool reply may + # be lost after that write, so the retry must find the same accepted intent. + original = call(operation_id=padded)["proposal"] + assert original["idempotency_key"] == "recover-original-intent" + store = ChatActionStore(project.parent / "runtime/chat/actions") + code, updated = run_json_cli_result("todo", "update", "--goal-id", "goal-acceptance", + "--todo-id", "todo_export", "--agent-id", "agent-a", "--note", "Work is progressing", + registry_path=controller.registry_path, runtime_root=project.parent / "runtime") + assert code == 0, updated + authority, journal = cli("inspect")[1], store.path.read_bytes() + recovered = call(operation_id=padded if retry_spelling == "padded" else "recover-original-intent") + assert recovered["ok"] is True and recovered["executed"] is False, recovered + assert recovered["proposal"] == original + assert cli("inspect")[1] == authority and store.path.read_bytes() == journal + assert not (project / "runs.txt").exists() + + def test_preparation_retry_does_not_replace_unavailable_authority(conversation_check, monkeypatch): project, cli, _, _, _, call, _ = conversation_check call()