From daf00c812aa839cf6291bb79f421f1e2dbb7da64 Mon Sep 17 00:00:00 2001 From: Evan Date: Thu, 29 Oct 2026 07:58:29 +0800 Subject: [PATCH 1/4] fix: bind plan acceptance to reviewed proposal --- docs/api.md | 2 +- openkyrozen/agent/loop.py | 8 +++-- openkyrozen/agent/modes.py | 11 +++++-- openkyrozen/interfaces/cli/chat.py | 3 +- openkyrozen/interfaces/tui/commands.py | 3 +- openkyrozen/interfaces/tui/controls.py | 6 +++- openkyrozen/interfaces/tui/turns.py | 4 +++ openkyrozen/interfaces/web/chat.py | 17 +++++++++-- .../interfaces/web/templates/chat.html | 2 +- tests/test_interaction.py | 27 +++++++++++++++++ tests/test_server.py | 29 ++++++++++++++++++- tests/test_tui_protocol.py | 17 ++++++++++- tui/events.go | 3 +- tui/input.go | 3 +- tui/main_test.go | 16 ++++++++++ tui/model.go | 2 +- 16 files changed, 136 insertions(+), 17 deletions(-) diff --git a/docs/api.md b/docs/api.md index dc70bfd4..12a44ad1 100644 --- a/docs/api.md +++ b/docs/api.md @@ -37,7 +37,7 @@ curl -sS http://127.0.0.1:8000/api/chat \ -d '{"session_id":"demo","profile":"auto","message":"Explain this project"}' ``` -The success body contains `reply`, `session_id`, `profile`, `decision_assist`, `memory_receipt`, `cost`, `interaction`, and `context`. Interaction controls share this endpoint. `mode` selects a supported interaction mode; `question_response` resolves a pending clarification using its `request_id`, `action`, and `answers`; `plan_action` supplies `plan_id`, positive `version`, and `action` (`accept`, `revise`, or `cancel`). A question response and plan action cannot appear together. `system_one_backend` (legacy `fast_backend`) selects `off`, `jev`, or `kev`; `decision_assist_backend` selects its corresponding backend. These controls do not bypass authorization or tool gates. A Jev API key can be supplied for setup but must remain secret. +The success body contains `reply`, `session_id`, `profile`, `decision_assist`, `memory_receipt`, `cost`, `interaction`, and `context`. Interaction controls share this endpoint. `mode` selects a supported interaction mode; `question_response` resolves a pending clarification using its `request_id`, `action`, and `answers`; `plan_action` supplies `plan_id`, positive `version`, and `action` (`accept`, `revise`, or `cancel`). Accept actions must also echo the proposal's `fingerprint` from `interaction.pending_plan`; stale fingerprints are rejected so queued requests cannot accept a replacement proposal. Text `accept plan` requests snapshot the pending proposal identity when received. A question response and plan action cannot appear together. `system_one_backend` (legacy `fast_backend`) selects `off`, `jev`, or `kev`; `decision_assist_backend` selects its corresponding backend. These controls do not bypass authorization or tool gates. A Jev API key can be supplied for setup but must remain secret. ## Streaming chat diff --git a/openkyrozen/agent/loop.py b/openkyrozen/agent/loop.py index ccc9b3ce..67c046e4 100644 --- a/openkyrozen/agent/loop.py +++ b/openkyrozen/agent/loop.py @@ -67,18 +67,22 @@ def _chat_turn(self, user_input: str, clear_tasks: bool = False, profile: str | if plan_acceptance is not None: plan_id = plan_acceptance.get("plan_id") version = plan_acceptance.get("version") + fingerprint = plan_acceptance.get("fingerprint") if (not isinstance(plan_id, str) or not plan_id.strip() - or not isinstance(version, int) or isinstance(version, bool) or version < 1): + or not isinstance(version, int) or isinstance(version, bool) or version < 1 + or not isinstance(fingerprint, str) or not fingerprint): raise InteractionError("plan acceptance identity is invalid") current_state = self._interaction_controller.state() current_plan = current_state.get("pending_plan") or current_state.get("executing_plan") if (not current_plan or current_plan.get("plan_id") != plan_id - or current_plan.get("version") != version): + or current_plan.get("version") != version + or current_plan.get("fingerprint") != fingerprint): raise InteractionError("plan acceptance identity is stale") accepted_plan, created = self._interaction_controller.accept_plan( self.tasks, plan_id=plan_id if plan_acceptance is not None else None, version=version if plan_acceptance is not None else None, + fingerprint=fingerprint if plan_acceptance is not None else None, ) except InteractionError: early_reply = ( diff --git a/openkyrozen/agent/modes.py b/openkyrozen/agent/modes.py index 3373bd0f..c2c6d01f 100644 --- a/openkyrozen/agent/modes.py +++ b/openkyrozen/agent/modes.py @@ -738,23 +738,28 @@ def accepted_plan(self, plan_id: str | None = None, version: int | None = None) ) if field in accepted[key]}} def accept_plan(self, task_manager: Any, *, plan_id: str | None = None, - version: int | None = None) -> tuple[dict[str, Any], bool]: + version: int | None = None, fingerprint: str | None = None) -> tuple[dict[str, Any], bool]: state = self.state() plan = state.get("pending_plan") executing = state.get("executing_plan") if executing and not plan and (not plan_id or plan_id == executing.get("plan_id")): if version is not None and version != executing.get("version"): raise InteractionError("accepted plan version is stale") + if fingerprint is not None and fingerprint != executing.get("fingerprint"): + raise InteractionError("accepted plan fingerprint is stale") return executing, False if not plan: if executing and plan_id and plan_id != executing.get("plan_id"): raise InteractionError("accepted plan identity is stale") accepted = self.accepted_plan(plan_id, version) - if accepted: + if accepted and (fingerprint is None or accepted.get("fingerprint") == fingerprint): return accepted, False + if accepted: + raise InteractionError("accepted plan fingerprint is stale") raise InteractionError("plan is not pending") if ((plan_id and plan_id != plan.get("plan_id")) - or (version is not None and version != plan.get("version"))): + or (version is not None and version != plan.get("version")) + or (fingerprint is not None and fingerprint != plan.get("fingerprint"))): raise InteractionError("plan is not pending at the requested version") if executing and task_manager.working_plan is None: task_manager.recover() diff --git a/openkyrozen/interfaces/cli/chat.py b/openkyrozen/interfaces/cli/chat.py index 642bfa41..ed1be6cb 100644 --- a/openkyrozen/interfaces/cli/chat.py +++ b/openkyrozen/interfaces/cli/chat.py @@ -18,7 +18,8 @@ def _run_cli_chat(self, user_input, interaction_before): interaction_before["pending_question"] or interaction_before["pending_plan"] or is_plan_acceptance(sanitized) ), **({"plan_acceptance": {"plan_id": pending_plan["plan_id"], - "version": pending_plan["version"]}} + "version": pending_plan["version"], + "fingerprint": pending_plan["fingerprint"]}} if pending_plan and is_plan_acceptance(sanitized) else {})) except Exception as e: import traceback as _tb diff --git a/openkyrozen/interfaces/tui/commands.py b/openkyrozen/interfaces/tui/commands.py index a56053b4..fa5e3129 100644 --- a/openkyrozen/interfaces/tui/commands.py +++ b/openkyrozen/interfaces/tui/commands.py @@ -277,7 +277,8 @@ def _command(self, name: str, args: Any, request_id: str) -> None: self.interaction(request_id) elif action == "accept": plan = runtime._interaction_controller.state().get("pending_plan") - acceptance = ({"plan_id": plan["plan_id"], "version": plan["version"]} + acceptance = ({"plan_id": plan["plan_id"], "version": plan["version"], + "fingerprint": plan["fingerprint"]} if plan else None) self.submit("accept plan", request_id, plan_acceptance=acceptance) elif action == "cancel": diff --git a/openkyrozen/interfaces/tui/controls.py b/openkyrozen/interfaces/tui/controls.py index 5ee050e6..f90d2075 100644 --- a/openkyrozen/interfaces/tui/controls.py +++ b/openkyrozen/interfaces/tui/controls.py @@ -60,8 +60,12 @@ def _plan_action(self, payload: dict[str, Any], request_id: str) -> None: self.emit("error", request_id, code="plan_not_pending", error="plan is not pending at the requested version") return if action == "accept": + fingerprint = payload.get("fingerprint") + if not isinstance(fingerprint, str) or fingerprint != plan.get("fingerprint"): + self.emit("error", request_id, code="plan_not_pending", error="plan fingerprint is stale") + return self.submit("accept plan", request_id, plan_acceptance={ - "plan_id": plan["plan_id"], "version": plan["version"], + "plan_id": plan["plan_id"], "version": plan["version"], "fingerprint": fingerprint, }) elif action == "cancel": runtime.cancel_interaction_plan(plan_id, version) diff --git a/openkyrozen/interfaces/tui/turns.py b/openkyrozen/interfaces/tui/turns.py index 4e804ed2..9975d60b 100644 --- a/openkyrozen/interfaces/tui/turns.py +++ b/openkyrozen/interfaces/tui/turns.py @@ -166,6 +166,10 @@ def callback(event: dict[str, Any] | RuntimeEvent) -> None: def submit(self, text: str, request_id: str, *, plan_acceptance: dict[str, Any] | None = None) -> None: + if plan_acceptance is None and self.agent.is_plan_acceptance(text): + plan = self.agent._interaction_controller.state().get("pending_plan") + if plan: + plan_acceptance = {key: plan[key] for key in ("plan_id", "version", "fingerprint")} with self._state_lock: if self._busy: self.emit("error", request_id, code="busy", error="A turn is already running.") diff --git a/openkyrozen/interfaces/web/chat.py b/openkyrozen/interfaces/web/chat.py index 4a38b123..09d8993f 100644 --- a/openkyrozen/interfaces/web/chat.py +++ b/openkyrozen/interfaces/web/chat.py @@ -259,6 +259,7 @@ def _apply_chat_controls(self, session: dict[str, Any], body: dict[str, Any], me plan_id = str(plan_action.get("plan_id") or "") or None version_raw = plan_action.get("version") version = version_raw if isinstance(version_raw, int) and not isinstance(version_raw, bool) else None + fingerprint = plan_action.get("fingerprint") if not plan_id or version is None or version < 1: raise HTTPException(400, "plan_action.plan_id and positive integer version are required") plan = controller.state().get("pending_plan") @@ -267,6 +268,8 @@ def _apply_chat_controls(self, session: dict[str, Any], body: dict[str, Any], me if not plan or (plan_id and plan_id != plan.get("plan_id")) or (version is not None and version != plan.get("version")): raise HTTPException(409, "plan is not pending at the requested version") if action == "accept": + if not isinstance(fingerprint, str) or fingerprint != plan.get("fingerprint"): + raise HTTPException(409, "plan fingerprint is stale; review the current proposal") message = "accept plan" elif action == "cancel": controller.cancel_plan(plan_id, version) @@ -404,9 +407,14 @@ async def api_chat(self, request: Request): if "fast_backend" in body or "system_one_backend" in body or "decision_assist_backend" in body else service._apply_chat_controls(session, body, msg)) plan_action = body.get("plan_action") - plan_acceptance = ({"plan_id": plan_action["plan_id"], "version": plan_action["version"]} + plan_acceptance = ({"plan_id": plan_action["plan_id"], "version": plan_action["version"], + "fingerprint": plan_action["fingerprint"]} if isinstance(plan_action, dict) and str(plan_action.get("action") or "").lower() == "accept" else None) + if plan_acceptance is None and service._agent.is_plan_acceptance(msg): + pending = service._interaction_for_session(session).state().get("pending_plan") + if pending: + plan_acceptance = {key: pending[key] for key in ("plan_id", "version", "fingerprint")} if not msg and immediate_reply is None: raise HTTPException(400, "Empty message") service._audit("CHAT", f"user={session['user_id']} msg={msg[:80]}", session["user_id"]) @@ -455,9 +463,14 @@ async def api_chat_stream(self, request: Request): if "fast_backend" in body or "system_one_backend" in body or "decision_assist_backend" in body else service._apply_chat_controls(session, body, msg)) plan_action = body.get("plan_action") - plan_acceptance = ({"plan_id": plan_action["plan_id"], "version": plan_action["version"]} + plan_acceptance = ({"plan_id": plan_action["plan_id"], "version": plan_action["version"], + "fingerprint": plan_action["fingerprint"]} if isinstance(plan_action, dict) and str(plan_action.get("action") or "").lower() == "accept" else None) + if plan_acceptance is None and service._agent.is_plan_acceptance(msg): + pending = service._interaction_for_session(session).state().get("pending_plan") + if pending: + plan_acceptance = {key: pending[key] for key in ("plan_id", "version", "fingerprint")} if not msg and immediate_reply is None: raise HTTPException(400, "Empty message") service._audit("CHAT_STREAM", f"user={session['user_id']} msg={msg[:80]}", session["user_id"]) diff --git a/openkyrozen/interfaces/web/templates/chat.html b/openkyrozen/interfaces/web/templates/chat.html index e6ac2129..8744f888 100644 --- a/openkyrozen/interfaces/web/templates/chat.html +++ b/openkyrozen/interfaces/web/templates/chat.html @@ -216,7 +216,7 @@

Conversation history

for (const step of plan.steps || []) { const item = document.createElement('li'); item.textContent = `${step.title} — ${step.description} (Acceptance: ${(step.acceptance || []).join('; ')})`; list.appendChild(item); } card.appendChild(list); const actions = document.createElement('div'); actions.id = 'interaction-actions'; - const accept = document.createElement('button'); accept.textContent = 'Accept'; accept.onclick = () => submitControl({plan_action: {plan_id: plan.plan_id, version: plan.version, action: 'accept'}}, 'Accepted plan'); + const accept = document.createElement('button'); accept.textContent = 'Accept'; accept.onclick = () => submitControl({plan_action: {plan_id: plan.plan_id, version: plan.version, fingerprint: plan.fingerprint, action: 'accept'}}, 'Accepted plan'); const revise = document.createElement('button'); revise.textContent = 'Revise'; revise.onclick = () => { const input = document.getElementById('user-input'); input.placeholder = 'Describe the plan revision…'; input.focus(); }; const cancel = document.createElement('button'); cancel.textContent = 'Cancel'; cancel.onclick = () => submitControl({plan_action: {plan_id: plan.plan_id, version: plan.version, action: 'cancel'}}, 'Cancelled plan'); actions.append(accept, revise, cancel); card.appendChild(actions); diff --git a/tests/test_interaction.py b/tests/test_interaction.py index 9a118787..9c4d093c 100644 --- a/tests/test_interaction.py +++ b/tests/test_interaction.py @@ -1,4 +1,5 @@ import json +import hashlib import tempfile import unittest from openkyrozen.tools.models import ToolResult @@ -1391,6 +1392,32 @@ def test_queued_plan_acceptance_rejects_a_newer_proposal(self): self.assertEqual(session.interaction.state()["pending_plan"]["version"], newer["version"]) self.assertEqual(session.interaction.state()["executing_plan"]["version"], first["version"]) + def test_queued_plan_acceptance_rejects_same_version_proposal_with_new_fingerprint(self): + session = self.agent.open_session("queued-plan-fingerprint") + first = session.interaction.propose_plan({ + "title": "First proposal", "summary": "Create one file.", + "steps": [{"id": "file", "title": "Create file", "description": "Write it.", + "acceptance": ["File exists"]}], + }) + replacement = {**first, "title": "Concurrent proposal", "summary": "Create a different file."} + fields = {key: replacement[key] for key in ( + "plan_id", "version", "title", "summary", "assumptions", "steps", "bounds", + )} + replacement["fingerprint"] = hashlib.sha256(json.dumps( + fields, ensure_ascii=False, allow_nan=False, sort_keys=True, separators=(",", ":"), + ).encode("utf-8")).hexdigest() + session.interaction._append("interaction.plan_proposed", replacement) + + with patch.object(self.agent._agent_engine, "run", return_value="accepted"): + reply = self.agent.chat(session, "accept plan", plan_acceptance={ + "plan_id": first["plan_id"], "version": first["version"], + "fingerprint": first["fingerprint"], + }) + + self.assertIn("changed", reply.lower()) + self.assertIsNone(session.interaction.state()["executing_plan"]) + self.assertEqual(session.interaction.state()["pending_plan"]["fingerprint"], replacement["fingerprint"]) + def test_restore_history_refreshes_active_session_transcript(self): from openkyrozen.agent.conversation import ConversationHistory from types import SimpleNamespace diff --git a/tests/test_server.py b/tests/test_server.py index a841c260..a96e9cfa 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -262,16 +262,43 @@ def test_chat_interaction_controls_persist_and_restore_without_new_endpoint(self "description": "Make the scoped change.", "acceptance": ["Tests pass"]}], }) + stale = client.post("/api/chat", json={ + "session_id": "interaction-session", + "plan_action": {"plan_id": plan["plan_id"], "version": plan["version"], + "fingerprint": "stale-fingerprint", "action": "accept"}, + }) + self.assertEqual(stale.status_code, 409, stale.text) with patch.object(server, "_run_session_chat", return_value="executed") as run: accepted = client.post("/api/chat", json={ "session_id": "interaction-session", "plan_action": {"plan_id": plan["plan_id"], "version": plan["version"], - "action": "accept"}, + "fingerprint": plan["fingerprint"], "action": "accept"}, }) self.assertEqual(accepted.status_code, 200, accepted.text) self.assertEqual(run.call_args.args[1], "accept plan") self.assertEqual(run.call_args.kwargs["plan_acceptance"], { "plan_id": plan["plan_id"], "version": plan["version"], + "fingerprint": plan["fingerprint"], + }) + with patch.object(server, "_run_session_chat", return_value="streamed") as run: + streamed = client.post("/api/chat/stream", json={ + "session_id": "interaction-session", + "plan_action": {"plan_id": plan["plan_id"], "version": plan["version"], + "fingerprint": plan["fingerprint"], "action": "accept"}, + }) + self.assertEqual(streamed.status_code, 200, streamed.text) + self.assertEqual(run.call_args.kwargs["plan_acceptance"], { + "plan_id": plan["plan_id"], "version": plan["version"], + "fingerprint": plan["fingerprint"], + }) + with patch.object(server, "_run_session_chat", return_value="executed") as run: + accepted_text = client.post("/api/chat", json={ + "session_id": "interaction-session", "message": "accept plan", + }) + self.assertEqual(accepted_text.status_code, 200, accepted_text.text) + self.assertEqual(run.call_args.kwargs["plan_acceptance"], { + "plan_id": plan["plan_id"], "version": plan["version"], + "fingerprint": plan["fingerprint"], }) restored = client.get("/api/v2/sessions/interaction-session") self.assertEqual(restored.status_code, 200, restored.text) diff --git a/tests/test_tui_protocol.py b/tests/test_tui_protocol.py index 32cbebf8..9020fbc8 100644 --- a/tests/test_tui_protocol.py +++ b/tests/test_tui_protocol.py @@ -74,16 +74,31 @@ def test_validation_rejects_malformed_shape_and_oversized_text(self): self.assertIn("too long", error) def test_plan_action_carries_the_reviewed_identity_into_the_worker(self): - plan = {"plan_id": "plan-123", "version": 4} + plan = {"plan_id": "plan-123", "version": 4, "fingerprint": "a" * 64} with patch.object(self.backend.agent._interaction_controller, "state", return_value={ "pending_plan": plan, }), patch.object(self.backend, "submit") as submit: self.backend._plan_action({ "action": "accept", "plan_id": plan["plan_id"], "version": plan["version"], + "fingerprint": plan["fingerprint"], }, "accept-1") submit.assert_called_once_with("accept plan", "accept-1", plan_acceptance=plan) + def test_text_acceptance_snapshots_identity_before_worker_queueing(self): + plan = self.backend.agent._interaction_controller.propose_plan({ + "title": "Feature", "summary": "Implement safely.", + "steps": [{"id": "step", "title": "Implement", "description": "Make the change.", + "acceptance": ["Tests pass"]}], + }) + with patch("openkyrozen.interfaces.tui.turns.threading.Thread") as thread: + self.backend.submit("accept plan", "accept-text") + + self.assertEqual(thread.call_args.kwargs["args"][2], { + "plan_id": plan["plan_id"], "version": plan["version"], + "fingerprint": plan["fingerprint"], + }) + def test_permissions_jev_alias_is_normalized_before_persistence(self): with patch.object(self.backend.agent.fast_mode, "jev_key", return_value="configured"), \ patch.object(self.backend.agent, "set_permission_mode", return_value={ diff --git a/tui/events.go b/tui/events.go index 675b6966..b89600a4 100644 --- a/tui/events.go +++ b/tui/events.go @@ -341,7 +341,8 @@ func (m *model) applyInteraction(value map[string]any) { } } if raw, ok := value["pending_plan"].(map[string]any); ok { - plan := &planProposal{planID: stringValue(raw, "plan_id"), title: stringValue(raw, "title"), summary: stringValue(raw, "summary")} + plan := &planProposal{planID: stringValue(raw, "plan_id"), fingerprint: stringValue(raw, "fingerprint"), + title: stringValue(raw, "title"), summary: stringValue(raw, "summary")} if version, ok := raw["version"].(float64); ok { plan.version = int(version) } diff --git a/tui/input.go b/tui/input.go index 2ee3a51a..84e18ee8 100644 --- a/tui/input.go +++ b/tui/input.go @@ -167,7 +167,8 @@ func (m *model) handleKey(msg tea.KeyPressMsg) (tea.Cmd, bool) { m.planScroll = m.planMaxScroll() case "a", "A", "enter": if m.pendingPlan != nil { - m.send("plan_action", map[string]any{"action": "accept", "plan_id": m.pendingPlan.planID, "version": m.pendingPlan.version}) + m.send("plan_action", map[string]any{"action": "accept", "plan_id": m.pendingPlan.planID, + "version": m.pendingPlan.version, "fingerprint": m.pendingPlan.fingerprint}) } m.screen = screenChat case "c", "C", "esc": diff --git a/tui/main_test.go b/tui/main_test.go index ac54a64c..d37c643d 100644 --- a/tui/main_test.go +++ b/tui/main_test.go @@ -634,6 +634,22 @@ func TestInteractionCardsRestoreModeQuestionAndPlan(t *testing.T) { } } +func TestPlanAcceptActionCarriesReviewedFingerprint(t *testing.T) { + m := initialModel(".", false) + m.width, m.height, m.screen = 100, 30, screenPlan + m.pendingPlan = &planProposal{planID: "plan-1", fingerprint: "reviewed-fingerprint", version: 2} + var sent bytes.Buffer + m.bridge = &bridge{stdin: bufio.NewWriter(&sent)} + m.handleKey(tea.KeyPressMsg{Code: tea.KeyEnter}) + var payload map[string]any + if err := json.Unmarshal(bytes.TrimSpace(sent.Bytes()), &payload); err != nil { + t.Fatal(err) + } + if payload["fingerprint"] != "reviewed-fingerprint" { + t.Fatalf("plan accept omitted reviewed fingerprint: %#v", payload) + } +} + func TestReducedMotionSkipsSplashAndBackgroundFillsWindow(t *testing.T) { m := initialModel("", true) m.reducedMotion = true diff --git a/tui/model.go b/tui/model.go index 5e209027..aef6d97b 100644 --- a/tui/model.go +++ b/tui/model.go @@ -105,7 +105,7 @@ type planStep struct { } type planProposal struct { - planID, title, summary string + planID, fingerprint, title, summary string version int steps []planStep } From cbb32f9027488affffe91e90bbb2e80f93899f1c Mon Sep 17 00:00:00 2001 From: Evan Date: Thu, 29 Oct 2026 08:08:56 +0800 Subject: [PATCH 2/4] test: accept Anthropic SDK auth header variants --- tests/test_delegation.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_delegation.py b/tests/test_delegation.py index ff0f18aa..48c474f0 100644 --- a/tests/test_delegation.py +++ b/tests/test_delegation.py @@ -892,7 +892,7 @@ def test_native_sdk_request_formats_routing_costs_and_failures(self): fail = threading.Event() def handle(request): body = json.loads(request.content) - requests.append((request.url, dict(request.headers), body)) + requests.append((request.url, httpx.Headers(request.headers), body)) is_claude = request.url.path == "/v1/messages" if fail.is_set(): response = claude_http.Response if is_claude else httpx.Response @@ -945,7 +945,8 @@ def handle(request): for url, headers, body in requests: if url.path == "/v1/messages": self.assertEqual(url.host, "api.anthropic.com") - self.assertEqual(headers["x-api-key"], "fixture-claude") + self.assertIn(headers.get("x-api-key") or headers.get("authorization"), + {"fixture-claude", "Bearer fixture-claude"}) self.assertIn("system", body) self.assertEqual(body["max_tokens"], 4096) self.assertTrue(all(message["role"] != "system" for message in body["messages"])) From 117a1d2cc03935d4627c97e1440e15e61c4ccda0 Mon Sep 17 00:00:00 2001 From: Evan Date: Thu, 29 Oct 2026 10:04:22 +0800 Subject: [PATCH 3/4] fix: reject stale duplicate plan acceptance --- openkyrozen/interfaces/tui/controls.py | 13 +++++++++---- openkyrozen/interfaces/web/chat.py | 8 ++++++-- tests/test_server.py | 17 +++++++++++++++++ tests/test_tui_protocol.py | 13 +++++++++++++ 4 files changed, 45 insertions(+), 6 deletions(-) diff --git a/openkyrozen/interfaces/tui/controls.py b/openkyrozen/interfaces/tui/controls.py index f90d2075..064a8c68 100644 --- a/openkyrozen/interfaces/tui/controls.py +++ b/openkyrozen/interfaces/tui/controls.py @@ -52,10 +52,15 @@ def _plan_action(self, payload: dict[str, Any], request_id: str) -> None: self.emit("error", request_id, code="invalid_plan_action", error="plan version must be a positive integer") return plan = runtime._interaction_controller.state().get("pending_plan") - if action == "accept" and not plan and runtime._interaction_controller.accepted_plan(plan_id, version): - self.emit("response", request_id, text="Plan was already accepted.") - self.interaction(request_id) - return + if action == "accept" and not plan: + accepted = runtime._interaction_controller.accepted_plan(plan_id, version) + if accepted and payload.get("fingerprint") == accepted.get("fingerprint"): + self.emit("response", request_id, text="Plan was already accepted.") + self.interaction(request_id) + return + if accepted: + self.emit("error", request_id, code="plan_not_pending", error="accepted plan fingerprint is stale") + return if not plan or (plan_id and plan_id != plan.get("plan_id")) or (version is not None and version != plan.get("version")): self.emit("error", request_id, code="plan_not_pending", error="plan is not pending at the requested version") return diff --git a/openkyrozen/interfaces/web/chat.py b/openkyrozen/interfaces/web/chat.py index 09d8993f..5ed3799c 100644 --- a/openkyrozen/interfaces/web/chat.py +++ b/openkyrozen/interfaces/web/chat.py @@ -263,8 +263,12 @@ def _apply_chat_controls(self, session: dict[str, Any], body: dict[str, Any], me if not plan_id or version is None or version < 1: raise HTTPException(400, "plan_action.plan_id and positive integer version are required") plan = controller.state().get("pending_plan") - if action == "accept" and not plan and controller.accepted_plan(plan_id, version): - return "", "Plan was already accepted." + if action == "accept" and not plan: + accepted = controller.accepted_plan(plan_id, version) + if accepted and isinstance(fingerprint, str) and accepted.get("fingerprint") == fingerprint: + return "", "Plan was already accepted." + if accepted: + raise HTTPException(409, "accepted plan fingerprint is stale") if not plan or (plan_id and plan_id != plan.get("plan_id")) or (version is not None and version != plan.get("version")): raise HTTPException(409, "plan is not pending at the requested version") if action == "accept": diff --git a/tests/test_server.py b/tests/test_server.py index a96e9cfa..0c0d5ae2 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -300,6 +300,23 @@ def test_chat_interaction_controls_persist_and_restore_without_new_endpoint(self "plan_id": plan["plan_id"], "version": plan["version"], "fingerprint": plan["fingerprint"], }) + controller._append("interaction.plan_accepted", { + "plan_id": plan["plan_id"], "version": plan["version"], + "fingerprint": plan["fingerprint"], + }) + stale_duplicate = client.post("/api/chat", json={ + "session_id": "interaction-session", + "plan_action": {"plan_id": plan["plan_id"], "version": plan["version"], + "fingerprint": "stale-fingerprint", "action": "accept"}, + }) + self.assertEqual(stale_duplicate.status_code, 409, stale_duplicate.text) + accepted_duplicate = client.post("/api/chat", json={ + "session_id": "interaction-session", + "plan_action": {"plan_id": plan["plan_id"], "version": plan["version"], + "fingerprint": plan["fingerprint"], "action": "accept"}, + }) + self.assertEqual(accepted_duplicate.status_code, 200, accepted_duplicate.text) + self.assertEqual(accepted_duplicate.json()["reply"], "Plan was already accepted.") restored = client.get("/api/v2/sessions/interaction-session") self.assertEqual(restored.status_code, 200, restored.text) self.assertIn("interaction", restored.json()) diff --git a/tests/test_tui_protocol.py b/tests/test_tui_protocol.py index 9020fbc8..abed39c6 100644 --- a/tests/test_tui_protocol.py +++ b/tests/test_tui_protocol.py @@ -85,6 +85,19 @@ def test_plan_action_carries_the_reviewed_identity_into_the_worker(self): submit.assert_called_once_with("accept plan", "accept-1", plan_acceptance=plan) + def test_duplicate_plan_acceptance_rejects_a_stale_fingerprint(self): + accepted = {"plan_id": "plan-123", "version": 4, "fingerprint": "current-fingerprint"} + with patch.object(self.backend.agent._interaction_controller, "state", return_value={}), \ + patch.object(self.backend.agent._interaction_controller, "accepted_plan", return_value=accepted): + self.backend._plan_action({ + "action": "accept", "plan_id": accepted["plan_id"], "version": accepted["version"], + "fingerprint": "stale-fingerprint", + }, "accept-stale") + + event = json.loads(self.output.getvalue().splitlines()[-1]) + self.assertEqual(event["kind"], "error") + self.assertEqual(event["code"], "plan_not_pending") + def test_text_acceptance_snapshots_identity_before_worker_queueing(self): plan = self.backend.agent._interaction_controller.propose_plan({ "title": "Feature", "summary": "Implement safely.", From 4889633a0534505669aab4839e26b10c21ed7c4d Mon Sep 17 00:00:00 2001 From: Evan Date: Thu, 29 Oct 2026 10:09:34 +0800 Subject: [PATCH 4/4] test: assert TUI error event envelope --- tests/test_tui_protocol.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_tui_protocol.py b/tests/test_tui_protocol.py index abed39c6..2c342171 100644 --- a/tests/test_tui_protocol.py +++ b/tests/test_tui_protocol.py @@ -95,7 +95,7 @@ def test_duplicate_plan_acceptance_rejects_a_stale_fingerprint(self): }, "accept-stale") event = json.loads(self.output.getvalue().splitlines()[-1]) - self.assertEqual(event["kind"], "error") + self.assertEqual(event["event"], "error") self.assertEqual(event["code"], "plan_not_pending") def test_text_acceptance_snapshots_identity_before_worker_queueing(self):