feat: /rewind v2 — gateway thread-scoped rewind + best-effort delete (#21910) - #25074
SaguaroDev wants to merge 12 commits into
Conversation
…ch#21910) Schema v12 adds: - messages.active (default 1) — soft-delete flag for /rewind - sessions.rewind_count (default 0) — audit counter - idx_messages_session_active deferred index New SessionDB methods: - rewind_to_message(session_id, target_message_id) — soft-deletes rows >= target_id, refuses non-user targets, increments rewind_count - restore_rewound(session_id, since_message_id) — undo for stretch goal - list_recent_user_messages — picker source Existing methods get include_inactive kwarg (default False): - get_messages, get_messages_as_conversation, search_messages. Rewound rows excluded from session_search by default — opt-in for audit. The deferred index pattern (DEFERRED_INDEX_SQL run after _reconcile_columns) avoids 'no such column: active' on legacy pre-v12 databases, since executescript(SCHEMA_SQL) runs before column reconciliation.
Adding rewound as an explicit named kwarg on MemoryManager.on_session_switch broke existing tests that assert exact equality on the extra kwargs dict captured by providers (test_manager_fans_out_to_all_providers, test_manager_reset_flag_preserved). The MemoryProvider base class still documents rewound as a known kwarg, but the manager now passes it through **kwargs like any other forwarded flag. The CLI rewind handler already passes rewound=True keyword-style, so behavior is identical.
…ousResearch#21910) Adds the TUI half of the /rewind feature so the Ink terminal UI gets the same affordance as the prompt_toolkit CLI. Python side (tui_gateway/server.py): - /rewind added to _PENDING_INPUT_COMMANDS so slash.exec rejects it and the TUI falls through to command.dispatch (the only path with access to live session state + memory hooks). - New command.dispatch branch for name == "rewind": v1 auto-picks the most recent user turn (Claude-Code-style single- step undo), calls SessionDB.rewind_to_message, refreshes the in-memory history, fires _memory_manager.on_session_switch with rewound=True, and returns the new "prefill" payload. - A dedicated picker overlay (multi-step rewind) is tracked as a follow-up to NousResearch#21910. TS side (ui-tui/src/): - New "prefill" variant on CommandDispatchResponse + asCommandDispatch validator. Mirrors "send" but does NOT auto-submit; the client drops the message into the composer for editing. - createSlashHandler renders the optional notice via sys() and calls ctx.composer.setInput(d.message), letting the user edit-and-resubmit the rewound turn — the core UX promised by the issue. Tests: - 7 new tui_gateway tests covering prefill payload shape, in-memory history truncation, DB soft-delete, memory-provider notification (rewound=True), busy-session refusal, missing-session error, and registry placement in _PENDING_INPUT_COMMANDS. - Extended asCommandDispatch vitest covering the new prefill variant (with + without notice, and rejection of malformed payloads). Out of scope for v1 (tracked as NousResearch#21910 follow-up): - Dedicated picker overlay in Ink (the multi-step rewind UI). v1 auto- picks the most recent user turn, matching the most common case. - Gateway platforms (Telegram, Discord, etc.) — issue scopes v1 to CLI + TUI only.
…ousResearch#21910) Adds nullable columns to messages for gateway outbound-message tracking so /rewind v2 can best-effort delete the bot's public posts when the in-memory context is truncated: outbound_platform TEXT — e.g. 'discord', 'telegram' outbound_chat_id TEXT — platform chat/channel id outbound_thread_id TEXT — thread/topic id (nullable for flat platforms) outbound_message_id TEXT — the platform message id to delete New SessionDB primitives: set_outbound_ids(row_id, *, platform, chat_id, thread_id, message_id) Stamp coordinates onto a persisted assistant row. Silent no-op when message_id is falsy. get_inactive_outbound_ids(session_id, *, since_message_id, chat_id, thread_id) Lookup helper used by the gateway rewind code path to drive best-effort platform.delete_message calls. Filters scope to caller's (chat, thread) so a single rewind never deletes messages in a sibling thread. rewind_to_message(..., *, require_thread_scope=(chat_id, thread_id)) Optional kwarg that refuses cross-thread rewinds with ValueError. v1 callers (CLI/TUI, no kwarg) stay green — the guard only fires when explicitly passed. Rows missing outbound coordinates are not subject to the guard (NULL bypass). Migration v12 → v13: - Declarative reconcile adds the columns transparently. - Partial index idx_messages_outbound on (session_id, outbound_chat_id) WHERE outbound_message_id IS NOT NULL — created after reconcile so legacy DBs don't trip on the missing column. Tests: - test_rewind_v2_state.py: 12 new (schema, set/get roundtrip, chat/thread filtering, active-row exclusion, thread-scope guard refuse/accept/NULL-bypass/backward-compat, full v12→v13 migration). - test_hermes_state.py: bump 3 version-pin asserts to 13.
…ng seam Two additions to BasePlatform for /rewind v2 (NousResearch#21910): 1. can_delete_messages(chat_id) -> Optional[bool] Best-effort permission probe. Tri-state return: - True adapter has (or always has) deletion in this chat - False known not to have permission / no delete API - None unknown — caller falls back to optimism Default returns None. Adapters that can cheaply probe (Discord channel reachability, Telegram bot connectedness) override. 2. _record_outbound_ids_for_send(event, result) Universal seam wired into _process_message_background's _record_delivery closure. Stamps (platform, chat_id, thread_id, message_id) onto the most recent assistant row in SessionDB after every successful send_with_retry — so when a user runs /rewind in a Discord thread or Telegram topic, the gateway knows which platform messages to attempt to delete. Best-effort and defensively wrapped: any failure (no session store, no SessionDB, no message_id, no recent assistant row) is logged at debug and swallowed. A DB hiccup must never break message delivery. Multi-bubble responses stamp the last-visible id onto the same row — acceptable for v2 since the rewind UI is binary (delete-attempted vs. not); continuation_message_ids tracking is a v3 follow-up. Also extends delete_message docstring with the v2 use case.
Discord (gateway/platforms/discord.py):
- delete_message: discord.Message.delete via REST DELETE. The bot
can delete its own messages without manage_messages perms, which
covers the /rewind v2 use case (a wrong-thread bot post the user
wants retracted). Failures (gone, perms, rate limit) return False
so callers degrade gracefully.
- can_delete_messages: probes channel reachability. Returns True
when the channel resolves, False when it clearly doesn't (bot
kicked, channel deleted), None when the probe itself errors.
Telegram (gateway/platforms/telegram.py):
- can_delete_messages: bot can always delete its own messages
within 48h via Bot API deleteMessage, so returns True whenever
the bot is connected. Aging-out is handled by the existing
delete_message implementation returning False.
Closes the platform-adapter slice of /rewind v2 (NousResearch#21910). Other
platforms (Slack, Matrix, WhatsApp, etc.) inherit the base defaults
(can_delete_messages -> None, delete_message -> False) and can opt
in incrementally with the same pattern.
…lete
Wires /rewind into the gateway slash dispatch next to /branch. The
gateway version mirrors v1 TUI's single-step auto-pick behavior — a
multi-message picker overlay is a v3 follow-up.
Per-turn flow:
1. Resolve target = most-recent user message in active transcript.
2. SessionDB.rewind_to_message(..., require_thread_scope=(chat, thread))
soft-deletes everything at-or-after that turn. Refuses with a
user-visible 'Rewind refused' notice if any in-scope row was
posted into a different (chat_id, thread_id) — keeps the blast
radius predictable per the v2 spec.
3. Probes adapter.can_delete_messages once. When False (adapter
reports no perms), surfaces a 'Skipped N platform deletes' notice
instead of spamming the platform's logs. When True/None, iterates
adapter.delete_message over every recorded outbound id in scope.
4. Aggregates deleted/attempted counts into the reply notice.
5. Evicts cached agent and fires _memory_manager.on_session_switch
(..., rewound=True) per v1's hook contract.
Closes the gateway slice of /rewind v2 (NousResearch#21910). Discord (the
motivating wrong-channel-post case) is fully covered; other platforms
degrade gracefully — they keep the in-memory rewind but skip the
delete step until their adapter overrides delete_message.
Conflicts resolved in hermes_state.py and tests/test_hermes_state.py: - SCHEMA_VERSION: kept 13 (this branch's bump for /rewind v2 outbound tracking); main's PR NousResearch#27660 had bumped to 12 for platform_message_id. - messages table: kept both column sets — outbound_platform/chat_id/ thread_id/message_id (v13, /rewind delete) AND platform_message_id (v12, yuanbao recall exact-id match). Declarative reconciler adds both to legacy DBs. - DEFERRED_INDEX_SQL: added main's partial index idx_messages_platform_msg_id alongside the existing idx_messages_session_active. - get_messages / get_messages_as_conversation: kept this branch's include_inactive=False soft-delete filter; adopted main's ORDER BY id (commit c03acca — AUTOINCREMENT id is the canonical ordering, not timestamp). get_messages_as_conversation SELECT now includes platform_message_id so the row['platform_message_id'] reader added in main works. - search_messages: kept both new kwargs (include_inactive=False AND sort=None) and both docstring sections. Bodies already compose cleanly. - test_hermes_state.py: switched the three '== 13' literals to '== SCHEMA_VERSION' (main's form) so the test moves with future bumps.
# Conflicts: # hermes_state.py # tests/test_hermes_state.py
teknium1
left a comment
There was a problem hiding this comment.
Thanks for carrying the gateway-side cleanup forward. The v2 need remains on current main: gateway /undo soft-deletes state, while gateway/platforms/base.py:2974-2993 only exposes deletion for stream-preview cleanup.
Problems
gateway/platforms/base.py:3818records every successful text delivery against the latest active assistant row. That includes slash-command replies; after/rewindsoft-deletes its target, its own confirmation can be stamped onto an earlier assistant row and later deleted instead of the intended response.gateway/run.py:13647-13660evicts the cached agent, then checks only_running_agentsbefore firingon_session_switch(rewound=True). Idle slash commands have no running agent, so the intended memory notification is skipped.- The only delivery callback is the normal text path (
gateway/platforms/base.py:4003); later media sends are untracked, andcontinuation_message_idsare intentionally collapsed atgateway/platforms/base.py:2160-2163.
Suggested changes
- Persist explicit response-to-outbound associations for every delivered chunk/attachment; do not infer them from the latest assistant row or attach command notices.
- Obtain and notify the cached memory manager before eviction, then add an idle gateway
/undo/rewind regression test. - Add end-to-end gateway delivery tests for command notices, multi-bubble responses, and attachments.
Automated hermes-sweeper review.
| # in this thread can best-effort delete it. Wrapped in a | ||
| # broad try so a SessionDB error never breaks delivery. | ||
| try: | ||
| self._record_outbound_ids_for_send(event, result) |
There was a problem hiding this comment.
This callback runs for every successful text delivery, including slash-command notices, but _record_outbound_ids_for_send selects the latest active assistant row rather than the exact response being delivered. After this handler soft-deletes its target, its own confirmation can be recorded against an earlier assistant row and later deleted by a subsequent rewind. Carry an explicit persisted response-row ID through delivery and skip command/system replies.
| self._evict_cached_agent(session_key) | ||
| except Exception: | ||
| pass | ||
| agent = self._running_agents.get(session_key) |
There was a problem hiding this comment.
_evict_cached_agent() has already removed the between-turn cached agent, while an idle slash command normally has no _running_agents entry. This makes the memory hook below unreachable in the normal case. Capture/notify the cached agent before eviction, then evict it.
Stacked on #23445 (v1) — closes #21910 v2 scope
This PR extends
/rewindto the gateway with the v2 spec from #21910:best-effort message deletion across platforms, per-platform outbound-id
tracking, thread-scoped rewind, and a permission probe.
Base:
feat/rewind-21910(v1). Merge after v1.What ships
Schema v13 (
hermes_state.py)Four nullable columns on
messagesfor outbound tracking:Partial index
idx_messages_outboundon(session_id, outbound_chat_id) WHERE outbound_message_id IS NOT NULLkeeps the rewind-delete lookup cheap.Declarative reconcile adds the columns transparently on existing DBs; v12 → v13 migration block creates the index.
SessionDB primitives
set_outbound_ids(row_id, *, platform, chat_id, thread_id, message_id)— stamp coordinates onto a persisted assistant row. Silent no-op whenmessage_idfalsy.get_inactive_outbound_ids(session_id, *, since_message_id, chat_id, thread_id)— lookup for delete-on-rewind, filterable by chat/thread so a single rewind never touches a sibling thread.rewind_to_message(..., *, require_thread_scope=(chat_id, thread_id))— optional kwarg that refuses cross-thread rewinds withValueError. v1 callers (no kwarg) stay green; rows with NULL outbound coordinates bypass the guard.Platform base (
gateway/platforms/base.py)can_delete_messages(chat_id) -> Optional[bool]— tri-state permission probe (True/False/None-unknown). Default returnsNone; adapters that can cheaply probe override._record_outbound_ids_for_send(event, result)— universal seam wired into_process_message_background's_record_delivery. After every successfulsend_with_retry, looks up the most recent assistant row and stamps(platform, chat_id, thread_id, message_id). Best-effort and defensively wrapped — a DB failure must never break message delivery.Discord override (
gateway/platforms/discord.py)delete_message—discord.Message.deletevia REST DELETE. The bot can delete its own messages withoutmanage_messages, which covers the v2 motivating use case (a wrong-thread bot post the user wants retracted).can_delete_messages— probes channel reachability. ReturnsTruewhen the channel resolves,Falsewhen it clearly doesn't,Noneon probe error.Telegram override (
gateway/platforms/telegram.py)can_delete_messages— Telegram bots can always delete their own messages within 48h via Bot APIdeleteMessage, so returnsTruewhen the bot is connected. Aging-out is handled bydelete_messagereturningFalse.Gateway
/rewindcommand (gateway/run.py)Wired next to
/branch. Auto-picks the most recent user message (single-step undo, mirroring v1 TUI; multi-message picker overlay deferred to v3). Per-turn:rewind_to_message(..., require_thread_scope=(chat, thread))— soft-truncate. SurfacesRewind refused: rewind would cross thread boundaryif any in-scope row was posted elsewhere.adapter.can_delete_messagesonce. WhenFalse, surfaces "Skipped N platform delete(s) — adapter reports no perms" instead of spamming the platform's API. WhenTrue/None, iteratesadapter.delete_messageover every recorded outbound id in scope.deleted/attemptedcounts into the reply notice._memory_manager.on_session_switch(..., rewound=True)per v1's hook contract.Test coverage
tests/test_rewind_v2_state.pytests/test_hermes_state.pyPre-existing gateway suite failures (
test_config,test_tts_media_routing,test_verbose_command,test_update_streaming) reproduce on the v1 base without these changes — unrelated, tracked separately.v2 spec coverage (from #21910)
delete_messagecalled per outbound row; failures swallowed.set_outbound_idswired through_record_delivery.can_delete_messagestri-state with user-visible aggregation.require_thread_scope) so it can't be bypassed from the gateway.Adapter rollout
Discord (the motivating wrong-channel-post case) and Telegram are fully covered. Other platforms (Slack, Matrix, Mattermost, WhatsApp, Signal, Feishu, etc.) inherit the base defaults — they keep the in-memory rewind but skip the delete step until their adapter overrides
delete_message. The pattern is the two-method override shown here; opt-in is incremental and risk-free.CC