diff --git a/docs/api.md b/docs/api.md index a7815884f..456820acb 100644 --- a/docs/api.md +++ b/docs/api.md @@ -874,6 +874,29 @@ and approvals among them. While it is more than 0, the tab title starts with "!" (see [web-ui.md](web-ui.md#browser-alerts)). The filters and the limit do not apply to the two counts. +#### `POST /api/notifications/{id}/dismiss` + +Dismisses a pending notification of any type and records the signed-in +person in `dismissed_by_actor_id`. Answers `{notification_id, dismissed: true}`, +or `409` when the row is not pending or an answer to an approval is already +running. Every dismissal sends a `notification_dismissed` event. + +A question or an approval ends without an answer, as at expiry: the Telegram +and Slack cards get "🔕 Dismissed without an answer" and lose their buttons. +The session that asked a question gets the internal turn +`[Question dismissed without an answer: ]`, written by the person who +dismissed it. An external, archived or missing session gets no turn. While +the engine drains, the dismissal of a question that would start a turn is +refused (`503`) and the question stays pending. An approval gets an +`approval-dismissed` record in the mechanical-actions audit log. A review loop +that waits on its card proposes a new card at its next reconcile tick, as it +does after an expiry. + +#### `POST /api/notifications/dismiss-all` + +Dismisses the pending `notify` rows only, and sends no event. Answers +`{dismissed: <count>}`. + ### Web Push The installable panel and its push notifications (see @@ -2304,6 +2327,8 @@ those for another session. Events marked *global* go to every socket. { type: "notification_answered", notification_id: "…", session_id: "…", answer: "…", answered_by: "…", answered_at: "…" | null, actor_id: "…" } // answered_at: the stored answer time (ISO 8601). A snoozed approval has approval_status: "snoozed" and snooze_until instead. { type: "notification_expired", notification_id: "…", session_id: "…", notification_type: "question", title: "…" } +// One row was dismissed. actor_id: the person who dismissed it +{ type: "notification_dismissed", notification_id: "…", session_id: "…", notification_type: "question", title: "…", actor_id: "…" } // An answer was relayed into the session as a message { type: "answer_injected", session_id: "…", notification_id: "…", title: "…", answer: "…", answered_by: "…", content: "…", actor_id: "…" } diff --git a/docs/architecture.md b/docs/architecture.md index 95809f3ae..fbaf1f64b 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -123,7 +123,8 @@ Async notification system for agent→user communication: - **Multi-channel delivery** — web UI via `__global__` WebSocket broadcast channel, Telegram via direct bot API with inline keyboard buttons, Slack via Block Kit action buttons - **Answer routing**: answers from any channel (web UI, Telegram inline button or a reply to the card, Slack button, `/reply` command) are persisted and either unblock a waiting tool or injected as a user message into the originating session. A web answer records the person in `answered_by_actor_id`, and the relayed message is written by that person; the agent's work in that turn has the session's cause. A channel answer keeps the provider user ID in metadata. A hosted channel answer in external mode records the person that the channel gateway resolved, as a web answer does; another channel answer records no actor. An approval dispatcher (`nerve/notifications/handlers.py`) receives the person who answered as its `actor` keyword argument, `None` for a channel answer with no actor A dispatcher that asks for a snooze can return an `after_snooze` callback; the service awaits it with the result of its snooze write (False when the card was no longer pending) -- **Web UI**: `/notifications` page with status/type filters, inline answer options, dismiss, dismiss-all; real-time toast overlay for new notifications; NavRail badge for pending count. The page, the toast and the `ask_user` card in the chat show the same option list (see [web-ui.md](web-ui.md#tool-call-rendering)) +- **Dismissal**: a dismissed question or approval ends without an answer, as at expiry: a `notification_dismissed` broadcast, edited Telegram and Slack cards, an internal note to the session that asked a question, and an `approval-dismissed` audit record for an approval +- **Web UI**: `/notifications` page with status/type filters, inline answer options, dismiss (also for questions and approvals), dismiss-all (`notify` rows only); real-time toast overlay for new notifications; NavRail badge for pending count. The page, the toast and the `ask_user` card in the chat show the same option list (see [web-ui.md](web-ui.md#tool-call-rendering)) ### Cron Service (`nerve/cron/`) APScheduler-based job scheduler: diff --git a/docs/web-ui.md b/docs/web-ui.md index 7e779ddd6..6661e5d7f 100644 --- a/docs/web-ui.md +++ b/docs/web-ui.md @@ -111,7 +111,7 @@ Tool calls are collapsed by default with specialized renderers: - **Background continuation** — An autonomous turn (the runtime resumed after background work settled) starts with a divider naming what settled ("Background · build-chart finished"). - **AskUserQuestion** — Interactive question card with clickable options (radio for single-select, checkboxes for multi-select). Each question also offers **Other…**, which opens a "Your answer" field: the typed text (required once chosen) is that question's answer — it replaces the option in a single-select question and is appended to the chosen labels in a multi-select one. A card with one single-select question still answers on one click of an option. Multiple questions grouped in one card with a shared Submit button. Markdown previews on hover. When the agent is paused mid-turn (via `can_use_tool`), answers are sent through the interaction protocol (`answer_interaction` WebSocket message), injecting them into the SDK's `answers` field so the agent continues seamlessly. Once the question is no longer pending (answered here or in another tab, after a reload, or closed by a timeout), the card is read-only and shows the chosen answers; it never posts an answer as a chat message. - **ask_form** (Nerve's form tool, `mcp__nerve__ask_form`) — One card for up to six questions while the agent's tool call waits for the answers (up to an hour). The call's optional intro (Markdown, the agent's explanation of why it asks) shows above the card as chat text, and stays there after the card is answered and folds. In the card: an optional title, then each question with its label chip (`*` when required, or "Optional"), the question and the agent's reason in muted text. By kind: `choice` is option rows to pick one and `multi` option rows to pick any, each with **Other…** for the person's own words unless the agent turned it off (in `multi` the Other… text is one more answer); `text` is a two-row box that grows (Enter is a new line); `url` is checked to be http(s) with a host and no spaces; `list` is chips (a comma or Enter adds one, Backspace in the empty field removes the last, a paste splits at commas and line breaks, and leaving the field keeps the typed text); `repos` is chips like `list`, each `owner/name` (a pasted GitHub URL becomes `owner/name`; an invalid one is red with a hint and blocks Submit). Defaults are prefilled. **Submit** (or Cmd/Ctrl+Enter anywhere in the card) is enabled once every required question has an answer and nothing is invalid; Escape inside the card never stops the waiting turn. The answers go back through `answer_interaction` (`result: { answers }`), and the agent gets one line per question plus the same answers as JSON. The card answers only while its own form waits; after an answer here or in another tab, a timeout or a reload it is read-only. Answered cards fold, `ask_form` and AskUserQuestion alike: an answered or closed card shows one line per question, `LABEL → answer` (chips for a list, "—" for no answer); **Show questions** opens the full card, read-only, and **Hide** folds it again. Cards start folded, also after a reload, where the answers are read from the tool result; a card that still waits is shown in full. "Closed" means it ended without an answer (a timeout, the form was closed, or the session stopped). -- **ask_user** (Nerve's question tool, `mcp__nerve__ask_user`): the question and its options, answerable in the chat while it waits. The card reads the question's notification, so the chat, the Notifications page and the toast show the same question with the same option list: one full-width row per option with a letter key (A, B, C). While focus is in the list, a letter or a digit 1 to 9 chooses that row. An option that ends in "(recommended)" shows first with a "Recommended" tag. The last row, **Other…**, opens a field for an answer in your own words (Enter sends, Escape closes the field). An approval (`propose_action`, on the Notifications page and in the toast) has no Other… row and says "Nothing happens until you choose."; a question does not, because the agent can continue without the answer. In an approval, Approve (green, ✓), Decline (red, ✕) and Snooze 24h (moon) show their icon in place of the letter, and the answer line shows that icon in place of `✓`. The letter keys also choose these rows. After an answer from any place (the chat, the Notifications page, the toast, Telegram, Slack), each place shows `✓ <answer>` with who answered, when, and through which channel; **Show options** opens the list, read-only. When the server refuses an answer, the card reads the question again and shows the answer from a different place, if there is one; if not, "Could not send the answer. Try again." shows under the options. A question that expired shows "Expired without an answer". The toast shows all options and stays open until you answer or dismiss it; when the question is answered in another place, the toast shows the answer and closes after 5 seconds. A `notify` call shows as a "Notify" row. +- **ask_user** (Nerve's question tool, `mcp__nerve__ask_user`): the question and its options, answerable in the chat while it waits. The card reads the question's notification, so the chat, the Notifications page and the toast show the same question with the same option list: one full-width row per option with a letter key (A, B, C). While focus is in the list, a letter or a digit 1 to 9 chooses that row. An option that ends in "(recommended)" shows first with a "Recommended" tag. The last row, **Other…**, opens a field for an answer in your own words (Enter sends, Escape closes the field). An approval (`propose_action`, on the Notifications page and in the toast) has no Other… row and says "Nothing happens until you choose."; a question does not, because the agent can continue without the answer. In an approval, Approve (green, ✓), Decline (red, ✕) and Snooze 24h (moon) show their icon in place of the letter, and the answer line shows that icon in place of `✓`. The letter keys also choose these rows. After an answer from any place (the chat, the Notifications page, the toast, Telegram, Slack), each place shows `✓ <answer>` with who answered, when, and through which channel; **Show options** opens the list, read-only. When the server refuses an answer, the card reads the question again and shows the answer from a different place, if there is one; if not, "Could not send the answer. Try again." shows under the options. While a question or an approval waits, **Dismiss** under the options in the chat and on the Notifications page ends it without an answer: the agent that asked a question gets a note, and the Telegram and Slack cards lose their buttons (see [api.md](api.md#post-apinotificationsiddismiss)). A question that expired shows "Expired without an answer"; one that was dismissed in any place shows "Dismissed without an answer". The toast shows all options and stays open until you answer or dismiss it; when the question is answered in another place, the toast shows the answer and closes after 5 seconds. A `notify` call shows as a "Notify" row. - **send_file** (`mcp__nerve__send_file`): an image file (PNG, JPEG, GIF, WebP, AVIF or SVG) shows in the chat, at most 320 px high, with its name and a download link below it. A click on the image opens it at full size, with a download card. Another file, or an image that does not load (for example, a file that was deleted later), shows as a download card. A call that sent nothing shows "Could not send `<name>`" and the reason, for example a file outside the workspace. The image comes from `GET /api/files/download`, the same as the download card. - **EnterPlanMode** — A status line, "Entering plan mode…" while it runs and "Entered plan mode" when complete ("Plan mode declined" on an error), with no buttons: entering plan mode only restricts the agent, so no approval is involved (the Claude CLI switches the mode itself; if a CLI asks, the server allows it at once). - **ExitPlanMode** — The decision card. The agent pauses mid-turn until the person decides. The card shows an autogenerated summary, not the plan itself: up to six short bullets that the server writes with `agent.title_model` when the pause starts (the `plan_summary` event), with "Summarizing…" until they arrive and the plan's first heading and two lines when no summary can be made; the full plan (the interaction's `tool_input.plan`, which the CLI fills with the plan file's text) is in the side panel, which **Read the plan** opens. **Approve** lets the agent go on (it starts on the plan in the same turn) and closes the plan tab. **Request changes…** opens a required "What should change?" field; **Send** refuses the plan with that text, which reaches the agent verbatim: it stays in plan mode, revises the plan and asks again with a new card. **Edit**, offered when the pause carries the plan text, opens the side panel with the plan as a text field ("Edit the plan"); there **Apply edited plan** (enabled once the text changed) approves the person's text instead (`answer_interaction` with `result: { plan }`): the CLI saves it to the plan file and tells the agent the plan was approved with the person's edits. **Cancel** drops the edits. A decided card keeps its outcome line and a **Read the plan** link that opens that plan read-only (the edited text, for an edited approval). In the onboarding conversation the card is titled "Setup proposal" with **Apply setup**, **Change something…** and "Edit the setup proposal". After an approval, while that turn still runs, the card reads "Approved — applying…" ("Approved with your edits — applying…"; "Applying the setup…" in the onboarding conversation), then "Approved" or "Approved with your edits"; a change request reads "Changes requested" with the first line of the feedback, and an error that is not the person's feedback (a timeout, a stopped session, a CLI validation error) reads "Closed" with its reason. @@ -420,7 +420,7 @@ above the list ("Browser alerts are on" or "off") opens the setting. Otherwise it starts with "(N)", the number of pending notifications that the Notifs badge shows. The favicon gets a red dot for "!" and a blue dot for "(N)". A page that sets its own title gets the prefix again. The server - sends no event for a dismissal, so a tab that dismisses notifications tells + sends no event for Dismiss All, so a tab that dismisses notifications tells the other tabs on the same `BroadcastChannel`, and they read the count again. This needs no permission and no setting. - **Limits:** browsers show alerts only for HTTPS addresses (and diff --git a/nerve/channels/slack.py b/nerve/channels/slack.py index 0dc7713c4..b69990f60 100644 --- a/nerve/channels/slack.py +++ b/nerve/channels/slack.py @@ -1632,7 +1632,7 @@ async def expire_notification( message_id: str, text: str, ) -> None: - """Replace a notification card with its expired state.""" + """Replace a notification card with *text* and no buttons.""" if not self.is_available: return channel_id, _ = parse_target(target) @@ -1645,7 +1645,7 @@ async def expire_notification( ) except Exception as exc: logger.debug( - "Slack expiry edit failed for %s: %s", message_id, exc, + "Slack card edit failed for %s: %s", message_id, exc, ) async def send(self, message: OutboundMessage) -> None: diff --git a/nerve/db/notifications.py b/nerve/db/notifications.py index b9e326733..3d34463d3 100644 --- a/nerve/db/notifications.py +++ b/nerve/db/notifications.py @@ -286,10 +286,15 @@ async def find_notification_for_delivery( async def dismiss_notification( self, notification_id: str, *, attribution: Attribution = UNATTRIBUTED, ) -> bool: - """Dismiss a pending row. ``attribution.actor_id`` is who dismissed it.""" + """Dismiss a pending row. ``attribution.actor_id`` is who dismissed it. + + An approval that a press claimed (see :meth:`claim_pending_approval`) + is not dismissed: its dispatcher runs the decision. + """ async with self._atomic(): async with self.db.execute( - "SELECT id FROM notifications WHERE id = ? AND status = 'pending'", + """SELECT id FROM notifications + WHERE id = ? AND status = 'pending' AND answered_at IS NULL""", (notification_id,), ) as cursor: if not await cursor.fetchone(): diff --git a/nerve/notifications/service.py b/nerve/notifications/service.py index ab91d8e46..836e3f790 100644 --- a/nerve/notifications/service.py +++ b/nerve/notifications/service.py @@ -46,6 +46,11 @@ # Telegram caps an inline button's callback_data at 64 bytes. _CALLBACK_DATA_MAX_BYTES = 64 +# The line that a Telegram or Slack card gets when its question or approval +# closes without an answer. +_EXPIRED_LINE = "⏰ Expired unanswered" +_DISMISSED_LINE = "🔕 Dismissed without an answer" + def _fit_callback_value(notification_id: str, value: str) -> str: """Cut ``value`` so ``notif:<id>:<value>`` fits, dropping a split UTF-8 tail.""" @@ -939,18 +944,80 @@ def _on_answer_task_done(self, task: asyncio.Task) -> None: async def handle_dismiss( self, notification_id: str, *, actor: Actor | None = None, ) -> bool: - """Dismiss a notification (no answer routing needed). + """Dismiss a pending notification. ``actor`` is the person who dismissed it, recorded in - ``dismissed_by_actor_id``. + ``dismissed_by_actor_id``. Every dismissal sends a + ``notification_dismissed`` web broadcast. A question or an approval + then ends without an answer, as at expiry: the Telegram and Slack + cards say so and lose their buttons, an approval gets an + ``approval-dismissed`` audit event, and the asking session of a + question gets an internal note, written by ``actor``. + + Raises: + NotAcceptingTurnsError: the dismissal of a question while the + engine drains, when its session takes the note. The + question stays pending. """ notif = await self.db.get_notification(notification_id) if not notif or notif["status"] != "pending": return False - await self.db.dismiss_notification( - notification_id, attribution=attribution_for(actor), + kind = notif.get("type") + session_id = notif["session_id"] + title = notif.get("title") or "" + takes_note = kind == "question" and await self._takes_note( + session_id, "dismissal note", ) + # During a drain the turn with the note is refused. Keep the + # question pending, so the person can dismiss it after the restart. + if takes_note and not self.engine.accepting_turns: + from nerve.agent.engine import NotAcceptingTurnsError + + raise NotAcceptingTurnsError() + + if not await self.db.dismiss_notification( + notification_id, attribution=attribution_for(actor), + ): + return False + + from nerve.agent.streaming import broadcaster + try: + await broadcaster.broadcast("__global__", { + "type": "notification_dismissed", + "notification_id": notification_id, + "session_id": session_id, + "notification_type": kind, + "title": title, + "actor_id": actor_id_of(actor), + }) + except Exception as exc: # pragma: no cover - defensive + logger.warning( + "dismissal broadcast failed for %s: %s", notification_id, exc, + ) + + if kind in ("question", "approval"): + await self._edit_telegram_closed(notif, _DISMISSED_LINE) + await self._edit_slack_closed(notif, _DISMISSED_LINE) + + if kind == "approval": + await self._append_approval_audit({ + "event": "approval-dismissed", + "notification_id": notification_id, + "target_kind": notif.get("target_kind") or "", + "target_id": notif.get("target_id") or "", + "redelivery_count": notif.get("redelivery_count") or 0, + "dismissed_by_actor_id": actor_id_of(actor), + }) + + if takes_note: + self._dispatch_into_session( + session_id, + f"[Question dismissed without an answer: {title}]", + source="notification:dismiss", + actor=actor, + internal=True, + ) return True # ------------------------------------------------------------------ # @@ -1446,10 +1513,12 @@ async def _deliver_slack( ) return message_id - async def _edit_slack_expired(self, notif: dict[str, Any]) -> None: - """Best-effort edit of the Slack card to show it expired. + async def _edit_slack_closed( + self, notif: dict[str, Any], status_line: str, + ) -> None: + """Best-effort edit of the Slack card of a row that closed unanswered. - Rebuilds the original text from the row and appends the status line, + Rebuilds the original text from the row and appends ``status_line``, dropping the now-dead buttons. All failures are swallowed by design. """ channel = self._get_slack_channel() @@ -1467,7 +1536,7 @@ async def _edit_slack_expired(self, notif: dict[str, Any]) -> None: notif.get("body") or "", notif.get("priority") or "normal", ) - text += "\n\n⏰ Expired unanswered" + text += f"\n\n{status_line}" await channel.expire_notification( delivery["target"], str(delivery["message_id"]), @@ -1625,8 +1694,8 @@ async def _report_expired(self, rows: list[dict[str, Any]]) -> None: ) # Chat channels: mark the card expired, drop dead buttons. - await self._edit_telegram_expired(notif) - await self._edit_slack_expired(notif) + await self._edit_telegram_closed(notif, _EXPIRED_LINE) + await self._edit_slack_closed(notif, _EXPIRED_LINE) # Approvals: the proposer is the mechanical pipeline, not a # conversation — record the expiry in its audit log. @@ -1657,36 +1726,11 @@ async def _inject_expiry_note( Dispatched unconditionally — ``engine.run`` serializes per session, so a mid-turn session just processes the note when its current turn finishes (same pattern as the wakeup dispatcher - and ``handle_answer``). Skipped for external (satellite) - sessions, whose conversation loop Nerve doesn't own, and - archived/missing sessions — those got the web broadcast and + and ``handle_answer``). Skipped for the sessions that + :meth:`_takes_note` refuses; those got the web broadcast and nothing more. """ - try: - session = await self.db.get_session(session_id) - except Exception as exc: # pragma: no cover - defensive - logger.warning( - "expiry injection: session lookup failed for %s: %s", - session_id, exc, - ) - return - if not session: - logger.info( - "expiry injection: session %s missing — broadcast only", - session_id, - ) - return - if session.get("status") == "archived": - logger.info( - "expiry injection: session %s archived — broadcast only", - session_id, - ) - return - if session.get("source") == "external": - logger.info( - "expiry injection: session %s is external — broadcast only", - session_id, - ) + if not await self._takes_note(session_id, "expiry note"): return if len(rows) == 1: @@ -1707,11 +1751,41 @@ async def _inject_expiry_note( internal=True, ) - async def _edit_telegram_expired(self, notif: dict[str, Any]) -> None: - """Best-effort edit of the Telegram card to show it expired. + async def _takes_note(self, session_id: str, purpose: str) -> bool: + """Whether a session gets a note about its closed question. + + False for an external (satellite) session, whose conversation + loop Nerve does not own, and for an archived or missing session. + ``purpose`` names the note in the log. + """ + try: + session = await self.db.get_session(session_id) + except Exception as exc: # pragma: no cover - defensive + logger.warning( + "%s: session lookup failed for %s: %s", + purpose, session_id, exc, + ) + return False + if not session: + reason = "missing" + elif session.get("status") == "archived": + reason = "archived" + elif session.get("source") == "external": + reason = "external" + else: + return True + logger.info( + "%s: session %s is %s, no note", purpose, session_id, reason, + ) + return False + + async def _edit_telegram_closed( + self, notif: dict[str, Any], status_line: str, + ) -> None: + """Best-effort edit of the Telegram card of a row that closed unanswered. Rebuilds the original message text from the row (same builder - the delivery used) and appends the status line, dropping the + the delivery used) and appends ``status_line``, dropping the now-dead inline keyboard. Telegram refuses edits on old messages (>48h) — all failures are swallowed by design. """ @@ -1742,7 +1816,7 @@ async def _edit_telegram_expired(self, notif: dict[str, Any]) -> None: notif.get("body") or "", notif.get("priority") or "normal", ) - text += "\n\n⏰ Expired unanswered" + text += f"\n\n{status_line}" from nerve.channels.telegram import _md_to_tg_html from telegram.constants import ParseMode @@ -1760,6 +1834,6 @@ async def _edit_telegram_expired(self, notif: dict[str, Any]) -> None: ) except Exception as exc: logger.debug( - "telegram expiry edit failed for %s: %s", + "telegram card edit failed for %s: %s", notif["id"], exc, ) diff --git a/nerve/workflows/review_loop.py b/nerve/workflows/review_loop.py index aaf84f86f..53f37b017 100644 --- a/nerve/workflows/review_loop.py +++ b/nerve/workflows/review_loop.py @@ -1865,8 +1865,8 @@ async def _reconcile_tick(self) -> None: await self._milestone( fresh, f"❌ **Review loop {loop['id']} closed** — its " - "decision card expired " - f"{count + 1}× unanswered. Last state: " + "decision card closed " + f"{count + 1}× without an answer. Last state: " f"{loop.get('failure_reason')}.", notify=True, ) @@ -1876,7 +1876,8 @@ async def _reconcile_tick(self) -> None: ) await self._propose_card( loop, - f"(re-proposed — previous card expired) Parked on: " + "(re-proposed: the previous card closed without an " + "answer) Parked on: " f"{loop.get('failure_reason')}", ) elif loop["status"] in ("implementing", "verifying"): diff --git a/tests/test_notification_lifecycle.py b/tests/test_notification_lifecycle.py index d79d2f5f1..204218261 100644 --- a/tests/test_notification_lifecycle.py +++ b/tests/test_notification_lifecycle.py @@ -317,7 +317,9 @@ async def test_expiry_uses_the_latest_recorded_target( fake_engine.router.get_channel.return_value = channel service = NotificationService(fake_config, db, fake_engine) - await service._edit_slack_expired(await db.get_notification("n1")) + await service._edit_slack_closed( + await db.get_notification("n1"), "⏰ Expired unanswered", + ) channel.expire_notification.assert_awaited_once() args = channel.expire_notification.await_args.args @@ -722,6 +724,231 @@ async def test_telegram_edit_failure_is_swallowed( assert bot.edit_message_text.await_count == 2 +# ---------------------------------------------------------------------- +# Dismissal +# ---------------------------------------------------------------------- + + +def _telegram_bot(fake_engine: MagicMock) -> MagicMock: + """A Telegram bot behind the engine's router, for card edits.""" + bot = MagicMock() + bot.edit_message_text = AsyncMock() + channel = MagicMock() + channel._app.bot = bot + fake_engine.router.get_channel.return_value = channel + return bot + + +def _dismissed_events(captured: list[tuple[str, dict]]) -> list[dict]: + return [ + m for ch, m in captured + if ch == "__global__" and m.get("type") == "notification_dismissed" + ] + + +@pytest.mark.asyncio +class TestDismissal: + """A dismissed question or approval ends without an answer, as at expiry.""" + + async def test_a_dismissed_question_tells_the_web_and_the_session( + self, db: Database, fake_config, fake_engine, patch_broadcaster, + ): + await db.create_session("s1", actor=None) + svc = NotificationService(fake_config, db, fake_engine) + asked = await svc.ask_question( + session_id="s1", title="pick one", options=["yes", "no"], + ) + nid = asked["notification_id"] + + assert await svc.handle_dismiss(nid) is True + await asyncio.sleep(0) + + assert (await db.get_notification(nid))["status"] == "dismissed" + events = _dismissed_events(patch_broadcaster) + assert len(events) == 1 + assert events[0]["notification_id"] == nid + assert events[0]["session_id"] == "s1" + assert events[0]["notification_type"] == "question" + assert events[0]["title"] == "pick one" + fake_engine.run.assert_called_once() + kwargs = fake_engine.run.call_args.kwargs + assert kwargs["session_id"] == "s1" + assert kwargs["source"] == "notification:dismiss" + assert kwargs["internal"] is True + assert kwargs["user_message"] == ( + "[Question dismissed without an answer: pick one]" + ) + + async def test_a_dismissed_question_edits_the_telegram_and_slack_cards( + self, db: Database, fake_config, fake_engine, patch_broadcaster, + ): + bot = _telegram_bot(fake_engine) + slack = MagicMock(is_available=True) + slack.expire_notification = AsyncMock() + svc = NotificationService(fake_config, db, fake_engine) + svc._get_slack_channel = lambda: slack + await db.create_session("s1", actor=None) + asked = await svc.ask_question(session_id="s1", title="tg question") + nid = asked["notification_id"] + await db.update_notification( + nid, telegram_message_id="4242", telegram_chat_id="1001", + ) + await db.record_notification_delivery( + nid, "slack", target="C0123ABC", message_id="1.0", + ) + + assert await svc.handle_dismiss(nid) is True + + bot.edit_message_text.assert_awaited_once() + kwargs = bot.edit_message_text.await_args.kwargs + assert (kwargs["chat_id"], kwargs["message_id"]) == (1001, 4242) + assert "Dismissed without an answer" in kwargs["text"] + slack.expire_notification.assert_awaited_once() + args = slack.expire_notification.await_args.args + assert args[:2] == ("C0123ABC", "1.0") + assert args[2].endswith("🔕 Dismissed without an answer") + + async def test_a_dismissed_notify_row_is_only_broadcast( + self, db: Database, fake_config, fake_engine, patch_broadcaster, + ): + bot = _telegram_bot(fake_engine) + await db.create_session("s1", actor=None) + await db.create_notification( + notification_id="fyi-1", session_id="s1", type="notify", + title="fyi", + ) + await db.update_notification( + "fyi-1", telegram_message_id="4242", telegram_chat_id="1001", + ) + svc = NotificationService(fake_config, db, fake_engine) + + assert await svc.handle_dismiss("fyi-1") is True + await asyncio.sleep(0) + + assert [e["notification_id"] for e in _dismissed_events(patch_broadcaster)] == ["fyi-1"] + fake_engine.run.assert_not_called() + bot.edit_message_text.assert_not_awaited() + + async def test_a_dismissed_approval_is_audited_and_not_injected( + self, db: Database, fake_config, fake_engine, patch_broadcaster, + audit_workspace: Path, + ): + bot = _telegram_bot(fake_engine) + await db.create_session("s1", actor=None) + svc = NotificationService(fake_config, db, fake_engine) + nid = await _make_approval(svc, db) + await db.update_notification( + nid, telegram_message_id="4242", telegram_chat_id="1001", + ) + + assert await svc.handle_dismiss(nid) is True + await asyncio.sleep(0) + + assert (await db.get_notification(nid))["status"] == "dismissed" + fake_engine.run.assert_not_called() + assert "Dismissed without an answer" in ( + bot.edit_message_text.await_args.kwargs["text"] + ) + events = read_audit_jsonl( + audit_workspace / ".nerve" / "mechanical-actions", + ) + dismissed = [e for e in events if e.get("event") == "approval-dismissed"] + assert len(dismissed) == 1 + assert dismissed[0]["notification_id"] == nid + assert dismissed[0]["target_kind"] == "lifecycle-test" + assert dismissed[0]["target_id"] == "prop-1" + assert [e["notification_type"] for e in _dismissed_events(patch_broadcaster)] == ["approval"] + + async def test_a_claimed_approval_is_not_dismissed( + self, db: Database, fake_config, fake_engine, patch_broadcaster, + ): + await db.create_session("s1", actor=None) + svc = NotificationService(fake_config, db, fake_engine) + nid = await _make_approval(svc, db) + # A press took the row; its dispatcher runs the decision. + assert await db.claim_pending_approval(nid) + + assert await svc.handle_dismiss(nid) is False + + assert (await db.get_notification(nid))["status"] == "pending" + assert not _dismissed_events(patch_broadcaster) + + async def test_a_snoozed_approval_can_be_dismissed( + self, db: Database, fake_config, fake_engine, patch_broadcaster, + ): + _handlers.register("lifecycle-test", _snooze_dispatcher(_iso(24))) + await db.create_session("s1", actor=None) + svc = NotificationService(fake_config, db, fake_engine) + nid = await _make_approval(svc, db) + assert await svc.handle_answer(nid, "snooze_24h", "web") + assert (await db.get_notification(nid))["status"] == "pending" + + assert await svc.handle_dismiss(nid) is True + + assert (await db.get_notification(nid))["status"] == "dismissed" + + async def test_a_row_that_is_not_pending_is_left_alone( + self, db: Database, fake_config, fake_engine, patch_broadcaster, + ): + await db.create_session("s1", actor=None) + svc = NotificationService(fake_config, db, fake_engine) + asked = await svc.ask_question(session_id="s1", title="pick one") + nid = asked["notification_id"] + assert await svc.handle_answer(nid, "yes", "web") + await asyncio.sleep(0) + fake_engine.run.reset_mock() + + assert await svc.handle_dismiss(nid) is False + assert await svc.handle_dismiss("no-such-row") is False + await asyncio.sleep(0) + + assert (await db.get_notification(nid))["status"] == "answered" + assert not _dismissed_events(patch_broadcaster) + fake_engine.run.assert_not_called() + + async def test_external_and_archived_sessions_get_no_note( + self, db: Database, fake_config, fake_engine, patch_broadcaster, + ): + await db.create_session("sat-1", source="external", actor=None) + await db.create_session("old", status="archived", actor=None) + svc = NotificationService(fake_config, db, fake_engine) + external = await svc.ask_question(session_id="sat-1", title="q1") + archived = await svc.ask_question(session_id="old", title="q2") + + assert await svc.handle_dismiss(external["notification_id"]) is True + assert await svc.handle_dismiss(archived["notification_id"]) is True + await asyncio.sleep(0) + + fake_engine.run.assert_not_called() + assert len(_dismissed_events(patch_broadcaster)) == 2 + + async def test_a_question_stays_pending_while_the_engine_drains( + self, db: Database, fake_config, fake_engine, patch_broadcaster, + ): + from nerve.agent.engine import NotAcceptingTurnsError + + await db.create_session("s1", actor=None) + await db.create_session("sat-1", source="external", actor=None) + svc = NotificationService(fake_config, db, fake_engine) + asked = await svc.ask_question(session_id="s1", title="pick one") + external = await svc.ask_question(session_id="sat-1", title="q1") + await db.create_notification( + notification_id="fyi-1", session_id="s1", type="notify", + title="fyi", + ) + fake_engine.accepting_turns = False + + with pytest.raises(NotAcceptingTurnsError): + await svc.handle_dismiss(asked["notification_id"]) + + status = (await db.get_notification(asked["notification_id"]))["status"] + assert status == "pending" + assert not _dismissed_events(patch_broadcaster) + # No note to deliver: these rows are dismissed during the drain. + assert await svc.handle_dismiss(external["notification_id"]) is True + assert await svc.handle_dismiss("fyi-1") is True + + # ---------------------------------------------------------------------- # Concurrent presses # ---------------------------------------------------------------------- diff --git a/tests/test_notification_plan_attribution.py b/tests/test_notification_plan_attribution.py index 7ef2be702..70f486189 100644 --- a/tests/test_notification_plan_attribution.py +++ b/tests/test_notification_plan_attribution.py @@ -663,6 +663,40 @@ async def test_dismiss_and_dismiss_all_record_each_person(install): assert by == {first: alice, second: bob, third: bob} +@pytest.mark.asyncio +async def test_a_web_dismissal_of_a_question_tells_the_session_as_the_person(install): + alice, bob = install.actor_id("alice"), install.actor_id("bob") + # Bob started the session that asks; Alice dismisses the question. + await install.db.create_session("asker", source="web", actor=_actor(install, "bob")) + asked = await install.notifications.ask_question("asker", title="Pick one") + nid = asked["notification_id"] + + async with _client(install.app()) as client: + response = await client.post( + f"/api/notifications/{nid}/dismiss", headers=install.headers("alice"), + ) + again = await client.post( + f"/api/notifications/{nid}/dismiss", headers=install.headers("alice"), + ) + assert response.status_code == 200 + assert again.status_code == 409 + await install.turn_done("asker") + + row = await install.db.get_notification(nid) + assert (row["status"], row["dismissed_by_actor_id"]) == ("dismissed", alice) + # The note is an internal turn of Alice's: the agent reads it, the + # transcript does not store it, and the session keeps Bob as its cause. + run = install.runs[-1] + assert (run["session_id"], run["source"], run["internal"]) == ( + "asker", "notification:dismiss", True, + ) + assert run["user_message"] == "[Question dismissed without an answer: Pick one]" + assert run["actor"].actor_id == alice + assert await install.user_rows("asker") == [] + assert await install.session_cause_of("asker") == bob + assert install.events_of("notification_dismissed")[-1]["actor_id"] == alice + + # --------------------------------------------------------------------------- # # Silences # # --------------------------------------------------------------------------- # diff --git a/web/src/api/websocket.ts b/web/src/api/websocket.ts index 662538f8d..ef900ca92 100644 --- a/web/src/api/websocket.ts +++ b/web/src/api/websocket.ts @@ -41,6 +41,8 @@ export type WSMessage = // `actor_id` is the person who answered; null for a chat-channel answer. | { type: 'notification_answered'; notification_id: string; session_id: string; answer: string; answered_by: string; answered_at?: string | null; actor_id?: string | null; approval_status?: 'answered' | 'snoozed'; dispatch_ok?: boolean; snooze_until?: string } | { type: 'notification_expired'; notification_id: string; session_id: string; notification_type: string; title: string } + // `actor_id` is the person who dismissed the row. + | { type: 'notification_dismissed'; notification_id: string; session_id: string; notification_type: string; title: string; actor_id?: string | null } // The answer becomes a user message written by the person who answered; // `actor_id` is null for a chat-channel answer. | { type: 'answer_injected'; session_id: string; notification_id: string; title: string; answer: string; answered_by: string; content: string; actor_id?: string | null } diff --git a/web/src/components/Chat/tools/AskUserBlock.test.tsx b/web/src/components/Chat/tools/AskUserBlock.test.tsx index 84ed87689..a8b9592d4 100644 --- a/web/src/components/Chat/tools/AskUserBlock.test.tsx +++ b/web/src/components/Chat/tools/AskUserBlock.test.tsx @@ -9,6 +9,7 @@ vi.mock('../../../api/client', () => ({ api: { getNotification: vi.fn(), answerNotification: vi.fn(), + dismissNotification: vi.fn(), listNotifications: vi.fn(async () => ({ notifications: [], pending_count: 0 })), listActors: vi.fn(async () => ({ actors: [{ id: 'actor-alice', kind: 'human', display_name: 'Alice', username: null }] })), }, @@ -25,6 +26,7 @@ const { formatMessageTime } = await import('../../../utils/messageTime'); const getNotification = vi.mocked(api.getNotification); const answerNotification = vi.mocked(api.answerNotification); +const dismissNotification = vi.mocked(api.dismissNotification); const ID = 'ask-1a2b3c4d'; /** The server's answer time, from the answer event. */ @@ -151,6 +153,33 @@ describe('AskUserBlock', () => { expect(screen.queryByRole('group')).not.toBeInTheDocument(); }); + it('dismisses the question from the chat', async () => { + getNotification.mockResolvedValue(row()); + dismissNotification.mockResolvedValue({ notification_id: ID, dismissed: true }); + renderCall(); + await options(); + + await userEvent.click(screen.getByRole('button', { name: 'Dismiss' })); + + expect(dismissNotification).toHaveBeenCalledWith(ID); + expect(await screen.findByText('Dismissed without an answer')).toBeInTheDocument(); + expect(screen.queryByRole('group')).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Dismiss' })).not.toBeInTheDocument(); + }); + + it('shows a question that was dismissed in another place', async () => { + getNotification.mockResolvedValue(row()); + renderCall(); + await options(); + act(() => { + useNotificationStore.getState().handleWSNotificationDismissed({ + notification_id: ID, notification_type: 'question', actor_id: 'actor-alice', + }); + }); + expect(screen.getByText('Dismissed without an answer')).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Dismiss' })).not.toBeInTheDocument(); + }); + it('shows the options read-only when the question cannot be read', async () => { getNotification.mockRejectedValue(new Error('404')); vi.spyOn(console, 'error').mockImplementation(() => {}); diff --git a/web/src/components/Chat/tools/AskUserBlock.tsx b/web/src/components/Chat/tools/AskUserBlock.tsx index 7a8127886..c696d8a47 100644 --- a/web/src/components/Chat/tools/AskUserBlock.tsx +++ b/web/src/components/Chat/tools/AskUserBlock.tsx @@ -6,6 +6,7 @@ import { useNotificationStore } from '../../../stores/notificationStore'; import { extractResultText } from '../../../utils/extractResultText'; import { NotificationBody } from '../../Notifications/NotificationParts'; import { NotificationOptions } from '../../Notifications/NotificationOptions'; +import { DismissButton } from '../../Notifications/DismissButton'; import { OptionList } from '../../Notifications/OptionList'; import { parseAskOptions, questionIdFromResult, questionOptions } from '../../Notifications/askOptions'; @@ -65,6 +66,7 @@ export function AskUserBlock({ block }: { block: ToolCallBlockData }) { <NotificationBody body={body} className="mt-1 text-sm text-text-muted" /> )} {content} + {waiting && <DismissButton notif={row} className="mt-2 w-full justify-end" />} </div> ); } diff --git a/web/src/components/Notifications/BrowserAlerts.test.tsx b/web/src/components/Notifications/BrowserAlerts.test.tsx index 243b4066e..e4dd35bec 100644 --- a/web/src/components/Notifications/BrowserAlerts.test.tsx +++ b/web/src/components/Notifications/BrowserAlerts.test.tsx @@ -22,6 +22,7 @@ vi.mock('../../api/client', async (orig) => ({ api: { listNotifications: vi.fn(async () => ({ notifications: [], pending_count: 0, pending_answer_count: 0 })), dismissNotification: vi.fn(async () => ({})), + getNotification: vi.fn(async () => { throw new Error('offline'); }), dismissAllNotifications: vi.fn(async () => ({})), }, })); @@ -274,6 +275,12 @@ describe('BrowserAlerts', () => { expect(FakeNotification.shown[1].closed).toBe(false); dispatch({ type: 'notification_answered', notification_id: 'ask-2', session_id: 's1', answer: 'Yes', answered_by: 'telegram' }); expect(FakeNotification.shown[1].closed).toBe(true); + + send({ notification_id: 'ask-3', title: 'Ship it?' }); + act(() => { vi.advanceTimersByTime(ALERT_DELAY_MS); }); + expect(FakeNotification.shown[2]).toMatchObject({ title: 'Ship it?', closed: false }); + dispatch({ type: 'notification_dismissed', notification_id: 'ask-3', session_id: 's1', notification_type: 'question', title: 'Ship it?' }); + expect(FakeNotification.shown[2].closed).toBe(true); }); it('shows no alert while alerts are off', () => { diff --git a/web/src/components/Notifications/BrowserAlerts.tsx b/web/src/components/Notifications/BrowserAlerts.tsx index a63dc56e0..f44f544a3 100644 --- a/web/src/components/Notifications/BrowserAlerts.tsx +++ b/web/src/components/Notifications/BrowserAlerts.tsx @@ -40,7 +40,11 @@ export function BrowserAlerts() { if (msg.type === 'notification' && !msg.silenced) { const alerts = useBrowserAlertStore.getState(); alerter.receive(msg, alertsOn(alerts) && levelWants(alerts.level, msg.notification_type, msg.priority)); - } else if (msg.type === 'notification_answered' || msg.type === 'notification_expired') { + } else if ( + msg.type === 'notification_answered' + || msg.type === 'notification_expired' + || msg.type === 'notification_dismissed' + ) { alerter.close(msg.notification_id); } }); diff --git a/web/src/components/Notifications/DismissButton.tsx b/web/src/components/Notifications/DismissButton.tsx new file mode 100644 index 000000000..6f7dc6905 --- /dev/null +++ b/web/src/components/Notifications/DismissButton.tsx @@ -0,0 +1,35 @@ +import { useState } from 'react'; +import { Button, cx } from '../ui'; +import { EyeOff } from '../ui/icons'; +import { useNotificationStore, type Notification } from '../../stores/notificationStore'; + +/** + * Dismiss for a pending notification, on the Notifications page and in the + * chat. A question or an approval then ends without an answer, and the agent + * that asked a question gets a note. When the server refuses, the row is read + * again: an answer from a different place replaces the button. + */ +export function DismissButton({ notif, className }: { notif: Notification; className?: string }) { + const dismissNotification = useNotificationStore(s => s.dismissNotification); + const [busy, setBusy] = useState(false); + const [failed, setFailed] = useState(false); + if (notif.status !== 'pending') return null; + + const dismiss = async () => { + setBusy(true); + setFailed(false); + const ok = await dismissNotification(notif.id); + setBusy(false); + setFailed(!ok); + }; + + return ( + <span className={cx('inline-flex flex-wrap items-center gap-2', className)}> + <Button variant="ghost" size="xs" onClick={dismiss} disabled={busy}> + <EyeOff size={11} aria-hidden="true" /> + <span>Dismiss</span> + </Button> + {failed && <span role="alert" className="text-xs text-error">Could not dismiss. Try again.</span>} + </span> + ); +} diff --git a/web/src/pages/NotificationsPage.test.tsx b/web/src/pages/NotificationsPage.test.tsx index 369f5cdb6..55ffb2076 100644 --- a/web/src/pages/NotificationsPage.test.tsx +++ b/web/src/pages/NotificationsPage.test.tsx @@ -14,6 +14,7 @@ vi.mock('../api/client', () => ({ answerNotification: vi.fn(), getNotification: vi.fn(), dismissAllNotifications: vi.fn(), + dismissNotification: vi.fn(), }, // ActorLabel reads the viewer through the auth store, which wires these. setToken: vi.fn(), clearToken: vi.fn(), getToken: vi.fn(), @@ -34,6 +35,7 @@ const listActors = vi.mocked(api.listActors); const answerNotification = vi.mocked(api.answerNotification); const getNotification = vi.mocked(api.getNotification); const dismissAllNotifications = vi.mocked(api.dismissAllNotifications); +const dismissNotification = vi.mocked(api.dismissNotification); const ALICE = 'actor-alice'; const BOB = 'actor-bob'; @@ -347,6 +349,73 @@ describe('NotificationsPage options', () => { }); }); +describe('NotificationsPage dismiss', () => { + const PENDING = { status: 'pending', answer: null, answered_by: null, answered_at: null, options: ['yes', 'no'] }; + + it('dismisses a pending question and folds it to "Dismissed without an answer"', async () => { + const asked = notification({ id: 'ask-1', ...PENDING }); + listNotifications + .mockResolvedValueOnce({ notifications: [asked], pending_count: 1, pending_answer_count: 1 }) + .mockResolvedValue({ + notifications: [{ ...asked, status: 'dismissed' }], pending_count: 0, pending_answer_count: 0, + }); + dismissNotification.mockResolvedValue({ notification_id: 'ask-1', dismissed: true }); + renderPage(); + + await userEvent.click(await screen.findByRole('button', { name: 'Dismiss' })); + + expect(dismissNotification).toHaveBeenCalledWith('ask-1'); + expect(await screen.findByText('Dismissed without an answer')).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Dismiss' })).not.toBeInTheDocument(); + expect(screen.queryByRole('group', { name: 'Pick one' })).not.toBeInTheDocument(); + }); + + it('offers Dismiss on a pending approval and a notify row, not on an answered question', async () => { + listNotifications.mockResolvedValue({ + notifications: [ + notification({ id: 'ok-1', type: 'approval', ...PENDING, options: ['approve', 'decline'] }), + notification({ id: 'note-1', type: 'notify', ...PENDING, options: null }), + notification({ id: 'ask-old' }), + ], + pending_count: 2, pending_answer_count: 1, + }); + renderPage(); + await screen.findByText('Answer:'); + expect(screen.getAllByRole('button', { name: 'Dismiss' })).toHaveLength(2); + }); + + it('shows the answer from another place when the server refuses', async () => { + const answered = notification({ id: 'ask-1', answer: 'yes', answered_by: 'telegram' }); + listNotifications + .mockResolvedValueOnce({ + notifications: [notification({ id: 'ask-1', ...PENDING })], pending_count: 1, pending_answer_count: 1, + }) + .mockResolvedValue({ notifications: [answered], pending_count: 0, pending_answer_count: 0 }); + dismissNotification.mockRejectedValue(new Error('409')); + getNotification.mockResolvedValue(answered); + renderPage(); + + await userEvent.click(await screen.findByRole('button', { name: 'Dismiss' })); + + expect(getNotification).toHaveBeenCalledWith('ask-1'); + expect((await screen.findByText('Answer:')).parentElement).toHaveTextContent('Answer: yes'); + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + }); + + it('says so when the server refuses and the question still waits', async () => { + const asked = notification({ id: 'ask-1', ...PENDING }); + listNotifications.mockResolvedValue({ notifications: [asked], pending_count: 1, pending_answer_count: 1 }); + dismissNotification.mockRejectedValue(new Error('503')); + getNotification.mockResolvedValue(asked); + renderPage(); + + await userEvent.click(await screen.findByRole('button', { name: 'Dismiss' })); + + expect(await screen.findByRole('alert')).toHaveTextContent('Could not dismiss. Try again.'); + expect(screen.getByRole('button', { name: 'Dismiss' })).toBeEnabled(); + }); +}); + describe('NotificationToast options', () => { const ASK = { notification_id: 'ask-live', notification_type: 'question', session_id: 's1', @@ -417,6 +486,25 @@ describe('NotificationToast options', () => { expect(screen.getByText('Which region?')).toBeInTheDocument(); }); + it('folds to "Dismissed without an answer" from another place, then closes', () => { + vi.useFakeTimers(); + try { + renderAt(<NotificationToast />); + push(ASK); + act(() => { + useNotificationStore.getState().handleWSNotificationDismissed({ + notification_id: 'ask-live', notification_type: 'question', actor_id: ALICE, + }); + }); + expect(screen.queryByRole('group')).not.toBeInTheDocument(); + expect(screen.getByText('Dismissed without an answer')).toBeInTheDocument(); + act(() => { vi.advanceTimersByTime(6000); }); + expect(screen.queryByText('Which region?')).not.toBeInTheDocument(); + } finally { + vi.useRealTimers(); + } + }); + it('folds to the answer from another place, then closes', () => { vi.useFakeTimers(); try { diff --git a/web/src/pages/NotificationsPage.tsx b/web/src/pages/NotificationsPage.tsx index ffd7a6ff1..9d897486b 100644 --- a/web/src/pages/NotificationsPage.tsx +++ b/web/src/pages/NotificationsPage.tsx @@ -1,12 +1,13 @@ import { useEffect, useState } from 'react'; import { useNavigate, useSearchParams } from 'react-router-dom'; -import { Bell, CheckCheck, EyeOff, BellOff, Trash2, Plus, RotateCw, Clock } from '../components/ui/icons'; +import { Bell, CheckCheck, BellOff, Trash2, Plus, RotateCw, Clock } from '../components/ui/icons'; import { Badge, Button, IconButton, TextField, type BadgeTone } from '../components/ui'; import { useNotificationStore, type Notification, type Silence } from '../stores/notificationStore'; import { PageHeader } from '../components/ui/PageHeader'; import { ActorAttribution } from '../components/Chat/ActorLabel'; import { NotificationBody, NotificationLinks } from '../components/Notifications/NotificationParts'; import { NotificationOptions } from '../components/Notifications/NotificationOptions'; +import { DismissButton } from '../components/Notifications/DismissButton'; import { BrowserAlertsLink } from '../components/Notifications/BrowserAlertOffer'; /** Lifecycle state — a status, so it takes a `Badge` tone. */ @@ -86,7 +87,6 @@ function formatLocalTime(iso: string): string { function NotificationCard({ notif }: { notif: Notification }) { const navigate = useNavigate(); - const dismissNotification = useNotificationStore(s => s.dismissNotification); const priorityDot = PRIORITY_DOTS[notif.priority]; const isApproval = notif.type === 'approval'; @@ -148,12 +148,7 @@ function NotificationCard({ notif }: { notif: Notification }) { Session: {notif.session_title || notif.session_id} </Button> <span className="text-text-faint">{notif.created_at?.slice(0, 16).replace('T', ' ')}</span> - {notif.status === 'pending' && notif.type === 'notify' && ( - <Button variant="ghost" size="xs" onClick={() => dismissNotification(notif.id)}> - <EyeOff size={11} /> - <span>Dismiss</span> - </Button> - )} + <DismissButton notif={notif} /> </div> {/* The options while it waits, then the answer */} diff --git a/web/src/stores/chatStore.test.ts b/web/src/stores/chatStore.test.ts index 70b511eec..61e81413e 100644 --- a/web/src/stores/chatStore.test.ts +++ b/web/src/stores/chatStore.test.ts @@ -187,12 +187,31 @@ describe('the sessions that need you', () => { dispatch({ ...base, type: 'notification', notification_type: 'question', body: '', priority: 'normal', options: null }); dispatch({ ...base, type: 'notification_answered', answer: 'yes', answered_by: 'web' }); dispatch({ ...base, type: 'notification_expired', notification_type: 'question' }); + dispatch({ ...base, type: 'notification_dismissed', notification_type: 'question' }); expect(api.listSessions).not.toHaveBeenCalled(); await vi.advanceTimersByTimeAsync(500); expect(api.listSessions).toHaveBeenCalledTimes(1); }); + it('a dismissal in another place reloads the list and folds the question', async () => { + vi.useFakeTimers(); + useChatStore.setState({ sessionsNextOffset: 0, sessionsHasMore: false, archivedSessions: null, systemSessions: null }); + const { useNotificationStore } = await import('./notificationStore'); + const asked = { + id: 'n1', session_id: 's1', session_title: null, type: 'question' as const, title: 't', body: '', + priority: 'normal', status: 'pending', options: null, answer: null, answered_by: null, answered_at: null, + created_at: '2026-11-07T10:00:00Z', expires_at: null, + }; + useNotificationStore.setState({ byId: { n1: asked } }); + + dispatch({ type: 'notification_dismissed', notification_id: 'n1', session_id: 's1', notification_type: 'question', title: 't' }); + await vi.advanceTimersByTimeAsync(500); + + expect(api.listSessions).toHaveBeenCalledTimes(1); + expect(useNotificationStore.getState().byId.n1?.status).toBe('dismissed'); + }); + it('a task event reloads the list: a plan decided in another place clears "Plan to review"', async () => { vi.useFakeTimers(); useChatStore.setState({ sessionsNextOffset: 0, sessionsHasMore: false, archivedSessions: null, systemSessions: null }); diff --git a/web/src/stores/chatStore.ts b/web/src/stores/chatStore.ts index 79eeda437..e081a1b6b 100644 --- a/web/src/stores/chatStore.ts +++ b/web/src/stores/chatStore.ts @@ -24,7 +24,7 @@ import { registerSessionsLoader, reloadSessionsSoon } from './helpers/sessionsRe import { discardStreamDeltas, flushStreamDeltas, handleThinking, handleToken, handleToolUse, handleToolResult, handleToolOutput, handleDone, handleStopped, handleError, handleWakeup, handleAutoTurn, handleModelChanged } from './handlers/streamingHandlers'; import { handleSessionUpdated, handleSessionStatus, handleSessionSwitched, handleSessionForked, handleSessionResumed, handleSessionArchived, handleSessionRunning, handleSessionAwaitingInput, handleAnswerInjected, handleUserMessage, handleReviewLoopUpdate, exitStreaming } from './handlers/sessionHandlers'; import { handlePlanUpdate, handleWorkflowProgress } from './handlers/panelHandlers'; -import { handleInteraction, handleInteractionResolved, handlePlanSummary, handleFileChanged, handleNotification, handleNotificationAnswered, handleNotificationExpired } from './handlers/auxiliaryHandlers'; +import { handleInteraction, handleInteractionResolved, handlePlanSummary, handleFileChanged, handleNotification, handleNotificationAnswered, handleNotificationExpired, handleNotificationDismissed } from './handlers/auxiliaryHandlers'; export interface TodoItem { content: string; @@ -1737,6 +1737,7 @@ export const useChatStore = create<ChatState>((set, get) => ({ case 'notification': return handleNotification(msg, get, set); case 'notification_answered': return handleNotificationAnswered(msg, get, set); case 'notification_expired': return handleNotificationExpired(msg, get, set); + case 'notification_dismissed': return handleNotificationDismissed(msg, get, set); } }, })); diff --git a/web/src/stores/handlers/auxiliaryHandlers.ts b/web/src/stores/handlers/auxiliaryHandlers.ts index 7b3a24ca1..a8d50c856 100644 --- a/web/src/stores/handlers/auxiliaryHandlers.ts +++ b/web/src/stores/handlers/auxiliaryHandlers.ts @@ -80,7 +80,7 @@ export function handleFileChanged( } // A notification event can change the pending counts of the session list -// ("Needs you"), so each of the three reloads it. +// ("Needs you"), so each of them reloads it. export function handleNotification( msg: Extract<WSMessage, { type: 'notification' }>, _get: Get, @@ -113,3 +113,14 @@ export function handleNotificationExpired( ); reloadSessionsSoon(); } + +export function handleNotificationDismissed( + msg: Extract<WSMessage, { type: 'notification_dismissed' }>, + _get: Get, + _set: Set, +): void { + import('../notificationStore').then(({ useNotificationStore }) => + useNotificationStore.getState().handleWSNotificationDismissed(msg) + ); + reloadSessionsSoon(); +} diff --git a/web/src/stores/notificationStore.test.ts b/web/src/stores/notificationStore.test.ts index c0ead4391..76040c706 100644 --- a/web/src/stores/notificationStore.test.ts +++ b/web/src/stores/notificationStore.test.ts @@ -89,4 +89,44 @@ describe('the count of pending questions and approvals', () => { expect(counts()).toEqual({ pendingCount: 1, pendingAnswerCount: 0 }); expect(prefix()).toBe('(1) '); }); + + it('goes down for a dismissal, and only a question or an approval lowers the answer count', () => { + useNotificationStore.setState({ + filter: 'answered', + notifications: [row('ask-old', 'question', 'answered')], + byId: { 'ask-chat': row('ask-chat', 'question', 'dismissed') }, + pendingCount: 4, pendingAnswerCount: 2, + }); + const { handleWSNotificationDismissed } = useNotificationStore.getState(); + + handleWSNotificationDismissed({ notification_id: 'note-1', notification_type: 'notify', actor_id: 'actor-alice' }); + expect(counts()).toEqual({ pendingCount: 3, pendingAnswerCount: 2 }); + + handleWSNotificationDismissed({ notification_id: 'ask-1', notification_type: 'question', actor_id: 'actor-alice' }); + expect(counts()).toEqual({ pendingCount: 2, pendingAnswerCount: 1 }); + + // Rows that are not pending here: in the list, and in the chat only. + handleWSNotificationDismissed({ notification_id: 'ask-old', notification_type: 'question' }); + handleWSNotificationDismissed({ notification_id: 'ask-chat', notification_type: 'question' }); + expect(counts()).toEqual({ pendingCount: 2, pendingAnswerCount: 1 }); + + handleWSNotificationDismissed({ notification_id: 'ok-1', notification_type: 'approval' }); + expect(counts()).toEqual({ pendingCount: 1, pendingAnswerCount: 0 }); + }); + + it('marks every copy of a dismissed row with the person who dismissed it', () => { + const pending = row('ask-1', 'question', 'pending'); + useNotificationStore.setState({ + notifications: [pending], toastQueue: [pending], byId: { 'ask-1': pending }, pendingCount: 1, pendingAnswerCount: 1, + }); + + useNotificationStore.getState().handleWSNotificationDismissed({ + notification_id: 'ask-1', notification_type: 'question', actor_id: 'actor-alice', + }); + + const { notifications, toastQueue, byId } = useNotificationStore.getState(); + for (const copy of [notifications[0], toastQueue[0], byId['ask-1']]) { + expect(copy).toMatchObject({ status: 'dismissed', dismissed_by_actor_id: 'actor-alice' }); + } + }); }); diff --git a/web/src/stores/notificationStore.ts b/web/src/stores/notificationStore.ts index 900996ce3..9e0644ecf 100644 --- a/web/src/stores/notificationStore.ts +++ b/web/src/stores/notificationStore.ts @@ -105,11 +105,16 @@ interface NotificationState { fetchNotification: (id: string) => Promise<void>; /** Resolves true when the server took the answer. */ answerNotification: (id: string, answer: string) => Promise<boolean>; - dismissNotification: (id: string) => Promise<void>; + /** + * Resolves true when the server dismissed the row. A question or an + * approval then ends without an answer. + */ + dismissNotification: (id: string) => Promise<boolean>; dismissAll: () => Promise<void>; handleWSNotification: (data: any) => void; handleWSNotificationAnswered: (data: any) => void; handleWSNotificationExpired: (data: any) => void; + handleWSNotificationDismissed: (data: any) => void; dismissToast: (id: string) => void; loadSilences: () => Promise<void>; addSilence: (pattern: string, reason: string, ttlHours: number) => Promise<void>; @@ -208,14 +213,21 @@ export const useNotificationStore = create<NotificationState>((set, get) => ({ }, dismissNotification: async (id: string) => { + let ok = true; try { await api.dismissNotification(id); set(s => patchCopies(s, id, { status: 'dismissed' })); - get().loadNotifications(); tellNotificationsChanged(); } catch (e) { console.error('Failed to dismiss notification:', e); + ok = false; + // The row can have an answer from a different place: read it again. + await api.getNotification(id) + .then(row => set(s => patchCopies(s, id, row))) + .catch(err => console.error('Failed to reload notification:', err)); } + get().loadNotifications(); + return ok; }, dismissAll: async () => { @@ -334,6 +346,24 @@ export const useNotificationStore = create<NotificationState>((set, get) => ({ }); }, + handleWSNotificationDismissed: (data: any) => { + set(s => { + // The server sends this event for a row that it took from pending. A + // filter or the limit can leave the row out of the list; a question in + // the chat can then be in `byId` only. + const id = data.notification_id; + const row = s.notifications.find(n => n.id === id) ?? s.byId[id]; + const removed = !row || row.status === 'pending' ? 1 : 0; + return { + ...patchCopies(s, id, { status: 'dismissed', dismissed_by_actor_id: data.actor_id ?? null }), + pendingCount: Math.max(0, s.pendingCount - removed), + pendingAnswerCount: waitsForAnswer(data.notification_type) + ? Math.max(0, s.pendingAnswerCount - removed) + : s.pendingAnswerCount, + }; + }); + }, + dismissToast: (id: string) => { set(s => ({ toastQueue: s.toastQueue.filter(n => n.id !== id) })); }, diff --git a/web/src/utils/browserAlerts.ts b/web/src/utils/browserAlerts.ts index eb0cf307f..d8021f097 100644 --- a/web/src/utils/browserAlerts.ts +++ b/web/src/utils/browserAlerts.ts @@ -32,7 +32,7 @@ export function tabInFront(): boolean { /** * `seen`: a tab in front shows the item. `shown`: a tab showed the alert. - * `changed`: a tab dismissed items. The server sends no event for a dismissal. + * `changed`: a tab dismissed items. The server sends no event for Dismiss All. */ type TabMessage = { kind: 'seen' | 'shown'; key: string } | { kind: 'changed' }; @@ -47,7 +47,7 @@ export function tellNotificationsChanged(): void { export interface Alerter { /** A new item. `wanted`: the choices of this browser want an alert for it. */ receive: (event: NotificationEvent, wanted: boolean) => void; - /** The item was answered or expired: its alert goes away. */ + /** The item was answered, expired or dismissed: its alert goes away. */ close: (id: string) => void; dispose: () => void; }