mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
fix: a stale Discord approval click leaves the card to the request's observer
Item 1 of the review is only partly fixed by the stale-click commit
before this one. Clearing `_backend_settled` lets the settle observer
retire the card, but the click still edited the same card with
"Approval expired — command was not run" through its own interaction
response. Nothing serializes that edit with the observer's. If the
click's edit lands last, it hides the real outcome and reattaches the
disabled controls. Discord serializes the interaction payload before
awaiting the response, so a later embed change can't repair it.
When a click on a card with a request id resolves nothing, that request
has already settled through another path. Since the earlier commits, its
observer is always registered (it is registered before the send, and a
request gone before that already retires the card). So the click now
only answers the clicker privately ("no longer pending") and leaves the
card edit to the observer.
A click whose lookup raises recorded nothing, and its request may still
be pending. It used to stamp the card "expired — not run" and disable it
for good, and that edit could race the observer too. Now it re-opens the
card for a retry, tells only the clicker ("Couldn't record your choice.
Click again, or answer with /approve or /deny." — a new string in every
locale), and leaves the final edit to the observer. A card without a
request id has no observer, so its no-op click keeps the old "expired"
edit.
Tests:
- The stale-click regression now asserts that the click never edits the
card. It fails on the previous commit.
- A new test drives a failed lookup on a really pending request with its
observer attached: the card stays live, and a later text answer
retires it with the real outcome.
- A third test pins the expired edit on a card with no request id.
This commit is contained in:
1 parent
e776cdff18
commit
50f57a131f
20 files changed
+93
-3
No files matched your search
@@ -1006,6 +1006,7 @@ platform:
|
||||
already_resolved: "Hierdie goedkeuring is reeds afgehandel~"
|
||||
expired: "⌛ Goedkeuring het verval — die opdrag is nie uitgevoer nie (reeds uitgetel of elders afgehandel)"
|
||||
no_longer_pending: "⌛ Hierdie goedkeuring wag nie meer nie (dit het verval of is elders beantwoord)"
|
||||
click_failed: "⚠️ Kon nie jou keuse aanteken nie. Klik weer, of antwoord met /approve of /deny."
|
||||
by_user: "{label} deur {user}"
|
||||
resolved_once: "Eenmalig goedgekeur"
|
||||
resolved_session: "Vir sessie goedgekeur"
|
||||
|
||||
@@ -1006,6 +1006,7 @@ platform:
|
||||
already_resolved: "تم البتّ في هذه الموافقة مسبقًا~"
|
||||
expired: "⌛ انتهت صلاحية الموافقة — لم يُنفَّذ الأمر (انتهت المهلة أو تم البتّ فيه في مكان آخر)"
|
||||
no_longer_pending: "⌛ لم تعد هذه الموافقة معلّقة (انتهت مهلتها أو تمت الإجابة عنها في مكان آخر)"
|
||||
click_failed: "⚠️ تعذّر تسجيل اختيارك. انقر مرة أخرى، أو أجب باستخدام /approve أو /deny."
|
||||
by_user: "{label} بواسطة {user}"
|
||||
resolved_once: "تمت الموافقة مرة واحدة"
|
||||
resolved_session: "تمت الموافقة للجلسة"
|
||||
|
||||
@@ -1006,6 +1006,7 @@ platform:
|
||||
already_resolved: "Diese Genehmigung wurde bereits entschieden~"
|
||||
expired: "⌛ Genehmigung abgelaufen — Befehl wurde nicht ausgeführt (Zeitüberschreitung oder bereits anderswo entschieden)"
|
||||
no_longer_pending: "⌛ Diese Genehmigung ist nicht mehr offen (abgelaufen oder anderswo beantwortet)"
|
||||
click_failed: "⚠️ Deine Auswahl konnte nicht gespeichert werden. Klicke erneut oder antworte mit /approve oder /deny."
|
||||
by_user: "{label} von {user}"
|
||||
resolved_once: "Einmalig genehmigt"
|
||||
resolved_session: "Für diese Sitzung genehmigt"
|
||||
|
||||
@@ -1052,6 +1052,7 @@ platform:
|
||||
already_resolved: "This approval has already been resolved~"
|
||||
expired: "⌛ Approval expired — command was not run (already timed out or resolved elsewhere)"
|
||||
no_longer_pending: "⌛ This approval is no longer pending (it timed out or was answered elsewhere)"
|
||||
click_failed: "⚠️ Couldn't record your choice. Click again, or answer with /approve or /deny."
|
||||
by_user: "{label} by {user}"
|
||||
resolved_once: "Approved once"
|
||||
resolved_session: "Approved for session"
|
||||
|
||||
@@ -1006,6 +1006,7 @@ platform:
|
||||
already_resolved: "Esta aprobación ya se resolvió~"
|
||||
expired: "⌛ Aprobación expirada — el comando no se ejecutó (ya se agotó el tiempo o se resolvió en otro lugar)"
|
||||
no_longer_pending: "⌛ Esta aprobación ya no está pendiente (expiró o se respondió en otro lugar)"
|
||||
click_failed: "⚠️ No se pudo registrar tu elección. Vuelve a hacer clic o responde con /approve o /deny."
|
||||
by_user: "{label} por {user}"
|
||||
resolved_once: "Aprobado una vez"
|
||||
resolved_session: "Aprobado para la sesión"
|
||||
|
||||
@@ -1202,6 +1202,7 @@ platform:
|
||||
already_resolved: "Cette approbation a déjà été traitée~"
|
||||
expired: "⌛ Approbation expirée — la commande n'a pas été exécutée (délai dépassé ou déjà traitée ailleurs)"
|
||||
no_longer_pending: "⌛ Cette approbation n'est plus en attente (expirée ou traitée ailleurs)"
|
||||
click_failed: "⚠️ Impossible d'enregistrer ton choix. Clique à nouveau ou réponds avec /approve ou /deny."
|
||||
by_user: "{label} par {user}"
|
||||
resolved_once: "Approuvé une fois"
|
||||
resolved_session: "Approuvé pour la session"
|
||||
|
||||
@@ -1006,6 +1006,7 @@ platform:
|
||||
already_resolved: "Tá an cead seo réitithe cheana féin~"
|
||||
expired: "⌛ Tá an cead imithe in éag — níor ritheadh an t-ordú (chuaigh sé thar am cheana nó réitíodh é in áit eile)"
|
||||
no_longer_pending: "⌛ Níl an cead seo ar feitheamh a thuilleadh (chuaigh sé in éag nó freagraíodh in áit eile é)"
|
||||
click_failed: "⚠️ Níorbh fhéidir do rogha a thaifeadadh. Cliceáil arís, nó freagair le /approve nó /deny."
|
||||
by_user: "{label} ag {user}"
|
||||
resolved_once: "Ceadaithe uair amháin"
|
||||
resolved_session: "Ceadaithe don seisiún"
|
||||
|
||||
@@ -1023,6 +1023,7 @@ platform:
|
||||
already_resolved: "Ez a jóváhagyás már eldőlt~"
|
||||
expired: "⌛ A jóváhagyás lejárt — a parancs nem futott le (időtúllépés, vagy máshol már eldőlt)"
|
||||
no_longer_pending: "⌛ Ez a jóváhagyás már nincs függőben (lejárt, vagy máshol válaszoltak rá)"
|
||||
click_failed: "⚠️ Nem sikerült rögzíteni a választásodat. Kattints újra, vagy válaszolj a /approve vagy /deny paranccsal."
|
||||
by_user: "{label} — {user}"
|
||||
resolved_once: "Egyszer jóváhagyva"
|
||||
resolved_session: "Jóváhagyva a munkamenetre"
|
||||
|
||||
@@ -1006,6 +1006,7 @@ platform:
|
||||
already_resolved: "Questa approvazione è già stata risolta~"
|
||||
expired: "⌛ Approvazione scaduta — il comando non è stato eseguito (timeout già scattato o risolto altrove)"
|
||||
no_longer_pending: "⌛ Questa approvazione non è più in attesa (è scaduta o ha ricevuto risposta altrove)"
|
||||
click_failed: "⚠️ Impossibile registrare la tua scelta. Fai di nuovo clic o rispondi con /approve o /deny."
|
||||
by_user: "{label} da {user}"
|
||||
resolved_once: "Approvato una volta"
|
||||
resolved_session: "Approvato per la sessione"
|
||||
|
||||
@@ -1006,6 +1006,7 @@ platform:
|
||||
already_resolved: "この承認はすでに処理済みです~"
|
||||
expired: "⌛ 承認の有効期限が切れました — コマンドは実行されませんでした(タイムアウト済み、または別の場所で処理済み)"
|
||||
no_longer_pending: "⌛ この承認はもう保留中ではありません(期限切れ、または別の場所で回答済み)"
|
||||
click_failed: "⚠️ 選択を記録できませんでした。もう一度クリックするか、/approve または /deny で回答してください。"
|
||||
by_user: "{user} が{label}"
|
||||
resolved_once: "一度だけ承認"
|
||||
resolved_session: "セッション中は承認"
|
||||
|
||||
@@ -1006,6 +1006,7 @@ platform:
|
||||
already_resolved: "이 승인은 이미 처리되었어요~"
|
||||
expired: "⌛ 승인이 만료되어 명령이 실행되지 않았어요 (이미 시간 초과되었거나 다른 곳에서 처리됨)"
|
||||
no_longer_pending: "⌛ 이 승인은 더 이상 대기 중이 아니에요 (만료되었거나 다른 곳에서 응답됨)"
|
||||
click_failed: "⚠️ 선택을 기록하지 못했어요. 다시 클릭하거나 /approve 또는 /deny로 답해 주세요."
|
||||
by_user: "{user}: {label}"
|
||||
resolved_once: "한 번 승인됨"
|
||||
resolved_session: "세션 동안 승인됨"
|
||||
|
||||
@@ -1006,6 +1006,7 @@ platform:
|
||||
already_resolved: "Esta aprovação já foi resolvida~"
|
||||
expired: "⌛ Aprovação expirada — o comando não foi executado (já atingiu o tempo limite ou foi resolvido em outro lugar)"
|
||||
no_longer_pending: "⌛ Esta aprovação já não está pendente (expirou ou foi respondida noutro lugar)"
|
||||
click_failed: "⚠️ Não foi possível registrar sua escolha. Clique novamente ou responda com /approve ou /deny."
|
||||
by_user: "{label} por {user}"
|
||||
resolved_once: "Aprovado uma vez"
|
||||
resolved_session: "Aprovado para a sessão"
|
||||
|
||||
@@ -1006,6 +1006,7 @@ platform:
|
||||
already_resolved: "Решение по этому одобрению уже принято~"
|
||||
expired: "⌛ Срок одобрения истёк — команда не выполнена (уже сработал тайм-аут или решение принято в другом месте)"
|
||||
no_longer_pending: "⌛ Это одобрение больше не ожидает ответа (истекло время или ответ дан в другом месте)"
|
||||
click_failed: "⚠️ Не удалось записать ваш выбор. Нажмите ещё раз или ответьте командой /approve или /deny."
|
||||
by_user: "{label} — {user}"
|
||||
resolved_once: "Одобрено один раз"
|
||||
resolved_session: "Одобрено на сеанс"
|
||||
|
||||
@@ -1006,6 +1006,7 @@ platform:
|
||||
already_resolved: "Bu onay zaten sonuçlandırıldı~"
|
||||
expired: "⌛ Onay süresi doldu — komut çalıştırılmadı (zaman aşımına uğradı veya başka yerde sonuçlandırıldı)"
|
||||
no_longer_pending: "⌛ Bu onay artık beklemede değil (süresi doldu veya başka bir yerde yanıtlandı)"
|
||||
click_failed: "⚠️ Seçimin kaydedilemedi. Tekrar tıkla veya /approve ya da /deny ile yanıtla."
|
||||
by_user: "{label} — {user}"
|
||||
resolved_once: "Bir kez onaylandı"
|
||||
resolved_session: "Oturum için onaylandı"
|
||||
|
||||
@@ -1006,6 +1006,7 @@ platform:
|
||||
already_resolved: "Це схвалення вже опрацьовано~"
|
||||
expired: "⌛ Час схвалення минув — команду не виконано (час вичерпано або її вже опрацьовано в іншому місці)"
|
||||
no_longer_pending: "⌛ Це схвалення більше не очікує відповіді (час минув або відповідь надано деінде)"
|
||||
click_failed: "⚠️ Не вдалося записати ваш вибір. Натисніть ще раз або дайте відповідь командою /approve чи /deny."
|
||||
by_user: "{label} — {user}"
|
||||
resolved_once: "Схвалено один раз"
|
||||
resolved_session: "Схвалено для сеансу"
|
||||
|
||||
@@ -1006,6 +1006,7 @@ platform:
|
||||
already_resolved: "這個批准請求已處理過~"
|
||||
expired: "⌛ 批准已逾期 — 指令未執行(已逾時或已在其他地方處理)"
|
||||
no_longer_pending: "⌛ 這個批准請求已不再等待(已逾時或已在其他地方回應)"
|
||||
click_failed: "⚠️ 無法記錄你的選擇。請再按一次,或用 /approve 或 /deny 回覆。"
|
||||
by_user: "{label}(由 {user} 操作)"
|
||||
resolved_once: "已批准一次"
|
||||
resolved_session: "已批准(本工作階段)"
|
||||
|
||||
@@ -1006,6 +1006,7 @@ platform:
|
||||
already_resolved: "这个审批已经处理过了~"
|
||||
expired: "⌛ 审批已过期 — 命令未运行(已超时或已在别处处理)"
|
||||
no_longer_pending: "⌛ 这个审批已不再等待(已超时或已在其他地方处理)"
|
||||
click_failed: "⚠️ 无法记录你的选择。请再点一次,或用 /approve 或 /deny 回复。"
|
||||
by_user: "{user} {label}"
|
||||
resolved_once: "已批准一次"
|
||||
resolved_session: "已批准(本会话)"
|
||||
|
||||
@@ -6577,9 +6577,22 @@ def _define_discord_view_classes() -> None:
|
||||
)
|
||||
except Exception as exc:
|
||||
logger.error("Failed to resolve gateway approval from button: %s", exc)
|
||||
count = 0
|
||||
# Nothing was recorded and the request may still be pending: tell only the clicker
|
||||
# and keep the card live for a retry. A later click, the request's observer, or
|
||||
# (with no request id) the view timeout does the final edit.
|
||||
self.resolved = False
|
||||
self._backend_settled = False
|
||||
await interaction.response.send_message(
|
||||
t("platform.discord.approval.click_failed"), ephemeral=True)
|
||||
return
|
||||
if not count:
|
||||
self._backend_settled = False
|
||||
if self.request_id:
|
||||
# The request already settled through another path, whose observer owns this
|
||||
# card; editing it here too could land last and hide the real outcome.
|
||||
await interaction.response.send_message(
|
||||
t("platform.discord.approval.no_longer_pending"), ephemeral=True)
|
||||
return
|
||||
color = discord.Color.dark_grey()
|
||||
label = t("platform.discord.approval.expired")
|
||||
await self._finalize_embed(
|
||||
|
||||
@@ -220,3 +220,39 @@ async def test_external_answer_then_stale_click_preserves_resolved_observer(monk
|
||||
footer = message.embeds[0].footer
|
||||
footer_text = footer["text"] if isinstance(footer, dict) else footer.text
|
||||
assert footer_text == t("platform.discord.approval.already_resolved")
|
||||
# The stale click only answers its clicker; the card edit belongs to the observer, so the
|
||||
# two edits can't race and leave "command was not run" on top.
|
||||
interaction.response.edit_message.assert_not_awaited()
|
||||
interaction.response.send_message.assert_awaited_once_with(
|
||||
t("platform.discord.approval.no_longer_pending"), ephemeral=True)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_failed_click_keeps_a_pending_card_live(monkeypatch):
|
||||
"""A click whose lookup raises recorded nothing: the card stays clickable, only the clicker
|
||||
hears about it, and the request's observer still settles the card later."""
|
||||
adapter, view, entry, message = await _pending_card(monkeypatch)
|
||||
session = view.session_key
|
||||
monkeypatch.setattr(view, "_gate", AsyncMock(return_value=True))
|
||||
real_resolve = approval.resolve_gateway_approval
|
||||
monkeypatch.setattr(approval, "resolve_gateway_approval", lambda *a, **k: 1 / 0)
|
||||
interaction = SimpleNamespace(
|
||||
user=SimpleNamespace(display_name="tester"),
|
||||
response=SimpleNamespace(send_message=AsyncMock(), edit_message=AsyncMock()))
|
||||
|
||||
await view._resolve(interaction, "once", 1, "gateway.exec_approval.action_once")
|
||||
|
||||
interaction.response.send_message.assert_awaited_once_with(
|
||||
t("platform.discord.approval.click_failed"), ephemeral=True)
|
||||
interaction.response.edit_message.assert_not_awaited()
|
||||
assert not view.resolved and not view.is_finished()
|
||||
assert approval._gateway_queues[session] == [entry] # still pending, nothing recorded
|
||||
|
||||
assert real_resolve(session, "once", request_id="request-1") == 1 # answered by text later
|
||||
entry.observers[0]("resolved")
|
||||
await asyncio.sleep(0)
|
||||
await asyncio.gather(*adapter._background_tasks)
|
||||
footer = message.embeds[0].footer
|
||||
assert (footer["text"] if isinstance(footer, dict) else footer.text) == t(
|
||||
"platform.discord.approval.already_resolved")
|
||||
assert view.is_finished()
|
||||
@@ -4,6 +4,7 @@ from unittest.mock import AsyncMock
|
||||
|
||||
import pytest
|
||||
|
||||
from agent.i18n import t
|
||||
from gateway.config import PlatformConfig
|
||||
from plugins.platforms.discord.adapter import DiscordAdapter
|
||||
|
||||
@@ -104,12 +105,36 @@ async def test_expired_card_cannot_resolve_a_different_pending_request(monkeypat
|
||||
monkeypatch.setattr(view, "_gate", AsyncMock(return_value=True))
|
||||
monkeypatch.setattr(view, "_finalize_embed", AsyncMock())
|
||||
monkeypatch.setattr(discord_adapter.discord, "Color", SimpleNamespace(dark_grey=lambda: 0))
|
||||
interaction = SimpleNamespace(user=SimpleNamespace(display_name="tester"))
|
||||
interaction = SimpleNamespace(
|
||||
user=SimpleNamespace(display_name="tester"), response=SimpleNamespace(send_message=AsyncMock()))
|
||||
|
||||
await view._resolve(interaction, "once", 1, "gateway.exec_approval.action_once")
|
||||
|
||||
assert not other.event.is_set()
|
||||
assert approval._gateway_queues[session] == [other]
|
||||
# Its own request already settled elsewhere: the clicker hears so, the card is left alone.
|
||||
view._finalize_embed.assert_not_awaited()
|
||||
interaction.response.send_message.assert_awaited_once_with(
|
||||
t("platform.discord.approval.no_longer_pending"), ephemeral=True)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unsettled_click_without_an_observer_marks_the_card_expired(monkeypatch):
|
||||
"""A card with no request id has no observer, so its own no-op click stamps it, as before."""
|
||||
from plugins.platforms.discord import adapter as discord_adapter
|
||||
from plugins.platforms.discord.adapter import ExecApprovalView
|
||||
|
||||
view = ExecApprovalView(session_key="agent:other:discord:555", allowed_user_ids=set())
|
||||
monkeypatch.setattr(view, "_gate", AsyncMock(return_value=True))
|
||||
monkeypatch.setattr(view, "_finalize_embed", AsyncMock())
|
||||
monkeypatch.setattr(discord_adapter.discord, "Color", SimpleNamespace(dark_grey=lambda: 0))
|
||||
interaction = SimpleNamespace(
|
||||
user=SimpleNamespace(display_name="tester"), response=SimpleNamespace(send_message=AsyncMock()))
|
||||
|
||||
await view._resolve(interaction, "once", 1, "gateway.exec_approval.action_once")
|
||||
|
||||
view._finalize_embed.assert_awaited_once_with(interaction, 0, t("platform.discord.approval.expired"))
|
||||
interaction.response.send_message.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -133,7 +158,6 @@ async def test_expired_approval_removes_discord_components():
|
||||
])
|
||||
async def test_card_retires_when_backend_request_ends(monkeypatch, ends, footer_key):
|
||||
"""The card shows how its request ended, even when that happened while it was being sent."""
|
||||
from agent.i18n import t
|
||||
from tools import approval
|
||||
from tools.approval_gateway_wait import _ApprovalEntry
|
||||
|
||||
|
||||
Reference in new issue
Block a user