From f11710a5783ad2cf0efdaf344d9394cc0c09a0a1 Mon Sep 17 00:00:00 2001 From: chengyongru <61816729+chengyongru@users.noreply.github.com> Date: Mon, 3 Aug 2026 16:43:01 +0800 Subject: [PATCH] fix(webui): show actual local trigger messages (#5228) --- .../tests/test_websocket_http_routes.py | 3 ++ nanobot/triggers/local_store.py | 15 +++++++- nanobot/triggers/local_types.py | 3 ++ nanobot/webui/session_automations.py | 2 +- tests/triggers/test_local_triggers.py | 33 +++++++++++++++++ webui/src/tests/session-info-popover.test.tsx | 37 +++++++++++++++++++ 6 files changed, 90 insertions(+), 3 deletions(-) diff --git a/nanobot/channels/websocket/tests/test_websocket_http_routes.py b/nanobot/channels/websocket/tests/test_websocket_http_routes.py index bcba8a7a9..f294a84b1 100644 --- a/nanobot/channels/websocket/tests/test_websocket_http_routes.py +++ b/nanobot/channels/websocket/tests/test_websocket_http_routes.py @@ -428,6 +428,7 @@ async def test_session_automations_route_lists_local_triggers( chat_id="abc", session_key="websocket:abc", ) + trigger_store.enqueue(trigger.id, "Review PR #4591") channel = _ch( bus, session_manager=_seed_session(tmp_path, key="websocket:abc"), @@ -454,6 +455,7 @@ async def test_session_automations_route_lists_local_triggers( assert job["kind"] == "local_trigger" assert job["schedule"]["kind"] == "local" assert job["payload"]["kind"] == "local_trigger" + assert job["payload"]["message"] == "Review PR #4591" assert job["payload"]["command"] == f'nanobot trigger {trigger.id} "message"' assert job["state"]["pending"] is True finally: @@ -2615,6 +2617,7 @@ async def test_webui_automations_route_manages_local_triggers( by_id = {job["id"]: job for job in listed.json()["jobs"]} assert by_id[trigger.id]["kind"] == "local_trigger" assert by_id[trigger.id]["state"]["pending"] is True + assert by_id[trigger.id]["payload"]["message"] == "Review queued PR" assert by_id[trigger.id]["trigger"]["command"] == f'nanobot trigger {trigger.id} "message"' disabled = await _http_get( diff --git a/nanobot/triggers/local_store.py b/nanobot/triggers/local_store.py index 83e1094ff..3e5e93d8b 100644 --- a/nanobot/triggers/local_store.py +++ b/nanobot/triggers/local_store.py @@ -166,7 +166,8 @@ class LocalTriggerStore: raise ValueError("trigger message is required") self._ensure_dirs() with self._lock: - trigger = self._find_unlocked(self._load_triggers_unlocked(), trigger_id) + triggers = self._load_triggers_unlocked() + trigger = self._find_unlocked(triggers, trigger_id) if trigger is None: raise TriggerNotFoundError(f"trigger not found: {trigger_id}") if not trigger.enabled: @@ -180,10 +181,20 @@ class LocalTriggerStore: path = self.inbox_dir / f"{delivery.created_at_ms}-{delivery.id}.json" self._atomic_write(path, json.dumps(_delivery_payload(delivery), ensure_ascii=False)) delivery.path = path + run_record_path: Path | None = None try: - self.write_delivery_run_record(delivery, trigger=trigger, status="queued") + run_record_path = self.write_delivery_run_record( + delivery, + trigger=trigger, + status="queued", + ) + trigger.last_message = _run_record_text(content) + trigger.updated_at_ms = delivery.created_at_ms + self._save_triggers_unlocked(triggers) except BaseException: path.unlink(missing_ok=True) + if run_record_path is not None: + run_record_path.unlink(missing_ok=True) delivery.path = None raise return delivery diff --git a/nanobot/triggers/local_types.py b/nanobot/triggers/local_types.py index 4dda5e9cb..0801823ba 100644 --- a/nanobot/triggers/local_types.py +++ b/nanobot/triggers/local_types.py @@ -61,6 +61,7 @@ class LocalTrigger: origin_metadata: dict[str, Any] = field(default_factory=dict) created_at_ms: int = 0 updated_at_ms: int = 0 + last_message: str = "" last_run_at_ms: int | None = None last_status: TriggerStatus | None = None last_error: str | None = None @@ -90,6 +91,7 @@ class LocalTrigger: origin_metadata=dict(_get(data, "originMetadata", "origin_metadata", {}) or {}), created_at_ms=_int_or_zero(_get(data, "createdAtMs", "created_at_ms", 0)), updated_at_ms=_int_or_zero(_get(data, "updatedAtMs", "updated_at_ms", 0)), + last_message=str(_get(data, "lastMessage", "last_message", "") or ""), last_run_at_ms=_optional_int(_get(data, "lastRunAtMs", "last_run_at_ms")), last_status=_get(data, "lastStatus", "last_status"), # type: ignore[arg-type] last_error=_get(data, "lastError", "last_error"), @@ -108,6 +110,7 @@ class LocalTrigger: "originMetadata": self.origin_metadata, "createdAtMs": self.created_at_ms, "updatedAtMs": self.updated_at_ms, + "lastMessage": self.last_message, "lastRunAtMs": self.last_run_at_ms, "lastStatus": self.last_status, "lastError": self.last_error, diff --git a/nanobot/webui/session_automations.py b/nanobot/webui/session_automations.py index be54c330f..93bdb8c21 100644 --- a/nanobot/webui/session_automations.py +++ b/nanobot/webui/session_automations.py @@ -209,7 +209,7 @@ def _serialize_trigger( }, "payload": { "kind": "local_trigger", - "message": command, + "message": trigger.last_message or command, "command": command, }, "state": { diff --git a/tests/triggers/test_local_triggers.py b/tests/triggers/test_local_triggers.py index b4d6304c9..890c1b78c 100644 --- a/tests/triggers/test_local_triggers.py +++ b/tests/triggers/test_local_triggers.py @@ -150,6 +150,36 @@ def test_enqueue_writes_trigger_run_record(tmp_path: Path) -> None: assert record["content"] == "Review PR #4591" assert record["origin_metadata"] == {"webui": True} assert record["updated_at_ms"] > 0 + stored = store.get(trigger.id) + assert stored is not None + assert stored.last_message == "Review PR #4591" + + +def test_enqueue_rolls_back_delivery_and_audit_when_trigger_save_fails( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + store = LocalTriggerStore(tmp_path) + trigger = store.create( + name="PR review", + channel="websocket", + chat_id="chat-1", + session_key="websocket:chat-1", + ) + + def fail_save(_triggers: list[LocalTrigger]) -> None: + raise OSError("store write failed") + + monkeypatch.setattr(store, "_save_triggers_unlocked", fail_save) + + with pytest.raises(OSError, match="store write failed"): + store.enqueue(trigger.id, "Review PR #4591") + + assert list(store.inbox_dir.glob("*.json")) == [] + assert list(store.runs_dir.glob("*.json")) == [] + stored = LocalTriggerStore(tmp_path).get(trigger.id) + assert stored is not None + assert stored.last_message == "" def test_delivery_run_record_truncates_large_content_and_response(tmp_path: Path) -> None: @@ -168,6 +198,9 @@ def test_delivery_run_record_truncates_large_content_and_response(tmp_path: Path assert queued_record["content"].startswith("content-") assert queued_record["content"].endswith("\n... (truncated)") assert len(queued_record["content"]) < len(large_content) + stored = store.get(trigger.id) + assert stored is not None + assert stored.last_message == queued_record["content"] store.write_delivery_run_record( delivery, diff --git a/webui/src/tests/session-info-popover.test.tsx b/webui/src/tests/session-info-popover.test.tsx index be8d07710..7bb059545 100644 --- a/webui/src/tests/session-info-popover.test.tsx +++ b/webui/src/tests/session-info-popover.test.tsx @@ -113,6 +113,43 @@ describe("SessionInfoPopover", () => { expect(screen.queryByText(/ago/i)).not.toBeInTheDocument(); }); + it("shows the actual message received by a local trigger", async () => { + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue( + automationsResponse([ + { + id: "trg_123", + name: "PR monitor", + enabled: true, + kind: "local_trigger", + schedule: { kind: "local" }, + payload: { + kind: "local_trigger", + message: "Review PR #4591", + command: 'nanobot trigger trg_123 "message"', + }, + state: { pending: false }, + }, + ]), + ), + ); + const user = userEvent.setup(); + + render( + , + ); + + await user.click(screen.getByRole("button", { name: "Session details" })); + + expect(await screen.findByText("Review PR #4591")).toBeInTheDocument(); + expect(screen.queryByText('nanobot trigger trg_123 "message"')).not.toBeInTheDocument(); + }); + it("refreshes while open so completed one-shot automations disappear", async () => { vi.stubGlobal( "fetch",