Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions gateway/platforms/event.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ class MessageEvent:
# knows the message was meant for someone else); None means unknown and keeps the visible
# fallback, like True.
reply_expected: Optional[bool] = None
# Synthetic continuations can remain ordinary authorized/activity-touching turns while carrying
# no fresh system-prompt identity. Appended here to preserve MessageEvent positional arguments.
preserve_prompt_pins: bool = False

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1 — merge boundary incomplete] (blocking, extends @ehz0ah's inline 4128991175 on this hunk)

The guard added here protects _queue_or_replace_pending_event, but the same synthetic-vs-human fusion happens at the other merge sites of this pending slot. Reproduced at this head with the real functions (busy Telegram DM session, _synthetic_prompt_event output pending):

  1. Grace queue — gateway/run_inbound.py:672 (merge_text=True): inside HERMES_TELEGRAM_FOLLOWUP_GRACE_SECONDS, pending text becomes "[Continuing toward your standing goal]\nGoal: …\n<human text>", flag stays True.
  2. Boot sentinel — gateway/run_inbound.py:745 (merge_text=True): same while _AGENT_PENDING_SENTINEL holds the slot.
  3. Photo fast-path — gateway/run_inbound.py:651 -> _hm_merge_pending_for_source (merge at 621): a human PHOTO is absorbed into the pending continuation (media_urls extended, no interrupt). The PR body's "never … absorbs a human photo turn" holds for _queue_or_replace_pending_event, not here.
  4. Adapter debounce — gateway/platforms/base.py:3858 (merge at 3812): as filed by @ehz0ah; their inline stands.

At consumption the merged event resolves preserve=True (run_turn _event_preserves_prompt_pins), so the human content runs under the continuation's pinned prompt identity and the human's own inputs are never recorded. Reproduced: merged turn uses the pinned channel prompt ("Channel hint.").

Required repair. Enforce the boundary where merges are attempted, not in one caller — make the merge decision identity-aware (e.g. merge_pending_message_event / _can_merge_text_debounce_events refuse absorption when preserve_prompt_pins differs, or a shared predicate used by the run_inbound sites + debounce pair). Invariant: a pin-preserving event is never a merge target; two turns with different prompt identity never become one turn.

Regression contract. For grace/sentinel/photo/debounce: with a preserve_prompt_pins=True event pending and a human TEXT or PHOTO inbound, assert two queue entries (or a pending owned by the human), and assert the run's prompt identity matches the human turn. The current suite passes because its guard test drives only _queue_or_replace_pending_event.


# Process-local admission receipt, never routing metadata or execution acknowledgement.
_gateway_accepted: bool = field(default=False, init=False, repr=False, compare=False)
Expand Down
44 changes: 28 additions & 16 deletions gateway/run_agent_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -620,8 +620,8 @@ def _voice_channel_sidecar_note(self, event, source: SessionSource, session_key:
return f"[Voice channel now: {vc_now or 'not connected to a voice channel'}]"

async def _rehydrate_prompt_pins(self, session_key: str, expected_session_id: Optional[str]) -> None:
"""Adopt the durable pin snapshot for an internal turn when this process holds no pins for
*session_key* (a restart). Eviction clears only ``ephemeral_pin`` and keeps ``channel_pin``,
"""Adopt the durable pin snapshot for a pin-preserving turn (internal wake or synthetic
goal/heartbeat continuation) when this process holds no pins for *session_key* (a restart). Eviction clears only ``ephemeral_pin`` and keeps ``channel_pin``,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3 — post-eviction pin flip, decision requested] (non-blocking, pre-existing but in this PR's class)

_rehydrate_prompt_pins early-returns while either pin exists, and eviction deliberately keeps channel_pin while clearing ephemeral_pin (as this docstring states). Consequence reproduced at this head with the durable store: human pins A -> _evict_cached_agent -> synthetic continuation re-renders B (drops the Triggering message line) and pins B -> next human re-renders back to A (e0==e1 False, e0==e2 True). Counterfactual: with channel_pin also cleared, rehydration adopts the durable snapshot byte-identically (adopted == turn-1 human pin). Either document that eviction intentionally forces re-render for pin-preserving turns too, or treat an incompletely-cleared pin set as adoptable for preserve turns. Flagged for the intent call; not a merge blocker.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 — narrow the eviction contract: explicit /reasoning rebuilding leaves a partial pin pair.

I need to correct the broad capacity wording in my earlier P3 and in this updated docstring. Actual LRU, TTL, and pressure removal preserve both conversation pins and pass the current continuity controls.

The remaining inherited failure is the real /reasoning high caller. It reaches _evict_cached_agent, clearing context while retaining the channel pin. The either-pin rehydration guard returns, the following anchorless synthetic event renders context B and writes it to SQLite, and the next human renders A again. The channel hint is stable and the reasoning override is correctly applied. The cache signature explicitly treats reasoning as a per-message setting without prompt/tool effects.

Required repair: retain a coherent context/channel pair for resource release and runtime-only rebuilds; express real prompt invalidation and snapshot eligibility explicitly at its caller. Do not adopt every partial snapshot blindly or suppress all synthetic persistence: authoritative human metadata and current privacy policy can legitimately require a replacement pair. Capacity lifetime is already covered by the conversation invariant; this P3 needs a caller-specific repair, not a blanket capacity-eviction exception.

Runnable regression: Reproducer C in this review body is the exact test_reviewer_prompt_lifetime.py fixture. Save it under tests/gateway/ and run:

HERMES_PYTHON=/path/to/test/python scripts/run_tests.sh -j 1 --file-retries 0 tests/gateway/test_reviewer_prompt_lifetime.py -s --tb=short

Head gives 10 passed / 2 failed; direct helper eviction and the real reasoning command are the only failures. Baseline gives 3 passed / 9 failed. For /reasoning high, assert equal combined ephemeral bytes and an unchanged durable snapshot across human → command → synthetic → human, while the reasoning setting takes effect. Retain actual capacity removal, SQLite adoption, authoritative metadata, privacy, missing-snapshot and store-outage controls. Real SQLite pin I/O and the real prompt combiner run at a recording model-entry sink; routing, leases, release and model execution remain fixture seams. No provider cache-hit or billing result is claimed. Priority remains P3.

so an evicted agent still re-renders instead of reviving the snapshot."""
state = self._peek_session_state(session_key)
if state is not None and (
Expand Down Expand Up @@ -662,25 +662,34 @@ async def _persist_prompt_pins(self, session_key: Optional[str], expected_sessio
# Durability protects cache continuity; a store outage must not block the user turn.
logger.debug("Failed to persist prompt pin for %s", session_key, exc_info=True)

@staticmethod
def _event_preserves_prompt_pins(event: Any) -> bool:
"""True when *event* must reuse established prompt-pin identity without becoming internal."""
return bool(
getattr(event, "internal", False)
or getattr(event, "preserve_prompt_pins", False)
)

def _pinned_session_context_prompt(
self, context, redact_pii: bool, session_key: Optional[str], *, internal: bool = False,
self, context, redact_pii: bool, session_key: Optional[str], *,
internal: bool = False, preserve_pin: Optional[bool] = None,
) -> str:
"""Session-context prompt pinned per session: key hit → pinned bytes reused VERBATIM (immune
to renderer nondeterminism); key miss → re-render and re-pin (rename, topic edit, /sethome).

``internal`` events (kanban wakes, delegation completions, watch notifications) carry a
source rebuilt from the persisted origin, without chat_name/user_name/message_id. Rendering
from it re-keyed the pin, and the next human turn re-keyed it back (A→B→A), rewriting
already-sent system bytes each time. An internal event is never a real metadata change, so
it reuses an existing pin verbatim (restored by ``_rehydrate_prompt_pins`` after a restart); with
no pin yet it renders and pins as usual. The pin records the ``privacy.redact_pii`` it was
Pin-preserving events include internal wakes plus synthetic goal/heartbeat continuations
that intentionally carry no fresh channel/session metadata. Re-rendering either shape
re-keys the pin and the next human turn re-keys it back (A→B→A), rewriting already-sent
system bytes each time. With no established pin yet, render and pin as usual. The pin records the ``privacy.redact_pii`` it was
rendered under: bytes from another privacy policy are never reused, even by an internal
event."""
if preserve_pin is None:
preserve_pin = internal
_pin_state = self._peek_session_state(session_key) if session_key else None
_eph_pin = _pin_state.conversation.ephemeral_pin if _pin_state else None
if _eph_pin is not None and _eph_pin[2] != redact_pii:
_eph_pin = None
if internal and _eph_pin is not None:
if preserve_pin and _eph_pin is not None:
return _eph_pin[1]
_eph_key = self._ephemeral_change_key(context, redact_pii)
if _eph_pin is not None and _eph_pin[0] == _eph_key:
Expand All @@ -691,18 +700,21 @@ def _pinned_session_context_prompt(
return text

def _pinned_channel_inputs(
self, session_key: Optional[str], channel_prompt: Optional[str], source: SessionSource, *, internal: bool,
self, session_key: Optional[str], channel_prompt: Optional[str], source: SessionSource, *,
internal: bool = False, preserve_pin: Optional[bool] = None,
):
"""``(channel_prompt, source)`` for this turn's agent run.

The ephemeral system prompt also appends ``channel_prompt`` and the ``channel_overrides``
prompt (looked up by chat/thread/``parent_chat_id``). Internal events carry
``channel_prompt=None`` and a source without ``parent_chat_id``, so they dropped both and
toggled the system prompt like the context pin did. Human turns record their inputs;
internal turns reuse them (``_rehydrate_prompt_pins`` restores both after a restart)."""
prompt (looked up by chat/thread/``parent_chat_id``). Pin-preserving events carry no
authoritative channel inputs of their own, so they reuse the established human-turn pin
without changing authorization semantics. Ordinary turns record their current inputs
(``_rehydrate_prompt_pins`` restores both after a restart)."""
if preserve_pin is None:
preserve_pin = internal
if not session_key:
return channel_prompt, source
if not internal:
if not preserve_pin:
self._session_state(session_key).conversation.channel_pin = (channel_prompt, source.parent_chat_id)
return channel_prompt, source
state = self._peek_session_state(session_key)
Expand Down
2 changes: 2 additions & 0 deletions gateway/run_busy.py
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,8 @@ def _queue_or_replace_pending_event(self, session_key: str, event: MessageEvent)
existing = pending_slot.get(session_key) if isinstance(pending_slot, dict) else None
same_security_context = existing is not None and (
getattr(existing, "internal", False) == getattr(event, "internal", False)
and getattr(existing, "preserve_prompt_pins", False)
== getattr(event, "preserve_prompt_pins", False)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Bug] (blocking) This identity check only protects _queue_or_replace_pending_event, but queue-mode text follow-ups use a different merge path. If a synthetic goal continuation is already pending and the user sends text while the current turn is still active, BasePlatformAdapter._queue_text_debounce() buffers the human event and _flush_text_debounce_now() compares only sender identity before calling merge_pending_message_event(..., merge_text=True). The human text is then appended to the synthetic event, which keeps preserve_prompt_pins=True, so the real user message runs with the old pinned prompt instead of its own prompt identity. I reproduced this at the reviewed source tree: the pending text changed from [goal] continue to [goal] continue\nreal human follow-up. Please make prompt-pin identity a merge boundary for the debounce path as well and cover the synthetic-pending plus human-TEXT case under busy_text_mode: queue.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Preserve this separation when restoring a turn at the recursion cap

The new admission predicate separates a synthetic continuation from human input, but the later recursion-cap branch restores the drained event through the unguarded merge helper. The drain has already promoted the successor into the pending slot. Restoring the older event therefore replaces that successor when both are TEXT, or absorbs the older photo/caption into the successor when media is present.

At 69c12764dfc6e7d8329a7113aa1ae0de84f51307, a deterministic fixture executes the real factory → FIFO admission → drain/promotion → cap follow-up. Synthetic A followed by human TEXT B leaves only A; human TEXT A followed by synthetic B leaves only A; human PHOTO A followed by synthetic B leaves one synthetic-owned photo event. Thus an already-admitted human message can disappear, or lose its independent prompt identity after this guard initially kept it separate. No concurrent arrival is required in the tested transition.

The exact same three conservation assertions fail at merge base fd0b98f6256446a9115c54f1a92ace5ea74c929f; the empty-slot restoration control passes at both pins (3 failed, 1 passed each, canonical runner, native Windows Python 3.11.11). This is inherited, not newly introduced. The fixture sets the real depth-cap value and uses a fake adapter's in-memory dequeue; it does not execute preceding model turns or live platform I/O.

The repair belongs at queue-front restoration: retain the staged successor, restore the unprocessed current event ahead of it, and never merge these already-separated events or count their restoration as fresh capacity. Regression coverage needs both orders, the photo case, existing overflow, and capacity-full restoration, asserting distinct object/content identity and original order through subsequent draining. #78015 already owns the generic restoration class; #126167 has a narrower cross-prompt-identity change. Neither repair exists at this head. This is distinct from the existing ingress/debounce finding, not a duplicate of it.

Comment on lines +389 to +390

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Human text inherits synthetic pins

The new merge guard misses queue-mode text. With busy_text_mode: queue, the runner returns False for ordinary text, so the adapter buffers it separately. _flush_text_debounce_now later merges it into a pending continuation when both sources name the same sender, without checking preserve_prompt_pins.

The merged event keeps the continuation’s source, channel_prompt=None, and preservation flag. The human message’s fresh channel prompt is therefore ignored.

Enforce this boundary in the shared merge/debounce path too, retaining both events, and test through the wired adapter.

Prompt To Fix With AI
This is a comment left during a code review.
Path: gateway/run_busy.py
Line: 389-390

Comment:
**Human text inherits synthetic pins**

The new merge guard misses queue-mode text. With `busy_text_mode: queue`, the runner returns `False` for ordinary text, so the adapter buffers it separately. `_flush_text_debounce_now` later merges it into a pending continuation when both sources name the same sender, without checking `preserve_prompt_pins`.

The merged event keeps the continuation’s source, `channel_prompt=None`, and preservation flag. The human message’s fresh channel prompt is therefore ignored.

Enforce this boundary in the shared merge/debounce path too, retaining both events, and test through the wired adapter.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

and getattr(existing, "allow_gateway_control", True)
== getattr(event, "allow_gateway_control", True)
and all(
Expand Down
5 changes: 4 additions & 1 deletion gateway/run_goals.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,10 @@ def _synthetic_prompt_event(source: Any, text: str, *, internal: bool = False) -
would quote that stale message (Telegram DM topics route anchorless via the topic id).
"""
source = dataclasses.replace(source, message_id=None) if getattr(source, "message_id", None) else source
return MessageEvent(text=text, message_type=MessageType.TEXT, source=source, internal=internal)
return MessageEvent(
text=text, message_type=MessageType.TEXT, source=source, internal=internal,
preserve_prompt_pins=True,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1 — fused continuations are destructively misidentified] (blocking)

This producer's output is identified downstream by text prefix: _is_goal_continuation_event (gateway/run_busy.py:180) matches "[Continuing toward your standing goal]\nGoal:". A merged turn (continuation head + appended human text — see the merge-boundary finding) still matches. Reproduced at this head:

  • /goal pause|clear -> _clear_goal_pending_continuations (run_busy.py:191-197) pops/filters the fused turn as cleanable machinery: the user's pending message is deleted (removed=1, slot and overflow emptied).
  • run_turn.py:3817 first clause is True for the fused text; with the goal inactive, the fused turn is discarded (return result) — the human content never runs and no reply is sent.

Required repair. Primary: prevent the fusion (merge-boundary finding). Defense in depth: use a structural marker (the producer already sets preserve_prompt_pins=True) instead of a text prefix for destructive cleanup/discard decisions, and never pop or black-hole a queued event carrying content beyond the continuation block.

Regression contract. A human turn queued behind a continuation must survive /goal pause and /goal clear, and must run (or surface visibly) when the goal went inactive mid-queue. Today both paths silently lose it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 — give goal work a structural kind at the producer; the current classifier deletes separate human input.

This is fresh execution evidence for the existing classification concern and extends the earlier cleanup finding, rather than another count of its fused-event examples. The new flag at this seam expresses prompt preservation for both goals and heartbeats; it cannot supply a goal-specific identity.

The actual /queue handler accepts a human request quoting the continuation header, retaining its message ID with internal=False and preserve_prompt_pins=False. With no merge or synthetic producer involved, the text-only predicate and pause/clear cleanup delete that event from the pending slot or overflow. Separately, with no goal configured, the actual stale-goal consumer discards the human request and the recording sink receives nothing. Ordinary human text reaches the sink. These five conservation cases fail on both the merge base and this head, so the mechanism is inherited.

Required repair: tag genuine goal continuations structurally at their producer, preserve that identity through copies, and use it in both destructive consumers. Keep heartbeat preservation distinct from goal cancellation. Preserve accepted human content, media and FIFO order at every queue boundary, including cleanup of any previously fused object.

Runnable regression: the exact test_review_continuation_conservation.py fixture is included as Reproducer A in this review body. Save it under tests/gateway/ and run:

HERMES_PYTHON=/path/to/test/python scripts/run_tests.sh -j 1 --file-retries 0 tests/gateway/test_review_continuation_conservation.py -s --tb=short

It checks pause/clear × slot/overflow plus the no-goal consumer, with genuine-continuation cleanup and ordinary-human controls. Head and baseline each give 7 failed / 6 passed overall; the other two failures are the separate judge-settlement race explained in the body. Command handlers are invoked directly; the consumer tests compose real adapter/drain/follow-up behavior with fake transport and a recording model sink. They do not execute full gateway admission or a live model. Closure must retain these human requests and remove only actual canceled goal work; add the corresponding heartbeat-kind control when introducing that field.

)

def _register_heartbeat_watch(self, quick_key: str, source: Any, session_id: str) -> None:
"""Track the canonical route and start the restart-recoverable poller."""
Expand Down
13 changes: 8 additions & 5 deletions gateway/run_turn.py
Original file line number Diff line number Diff line change
Expand Up @@ -2057,10 +2057,11 @@ async def _hmwa_prepare_turn(self, event, source, session_entry, session_key, _q

# The context prompt render is pinned per session, keyed by a hash of the renderer inputs, so
# the system prompt cannot drift turn-over-turn; a miss (thread rename, /sethome) re-renders.
if event.internal and session_key:
if session_key and self._event_preserves_prompt_pins(event):
await self._rehydrate_prompt_pins(session_key, session_entry.session_id)
context_prompt = self._pinned_session_context_prompt(
context, _redact_pii, session_key, internal=event.internal,
context, _redact_pii, session_key,
preserve_pin=self._event_preserves_prompt_pins(event),
)

# Per-turn notes ride the user message via the api_content sidecar, NOT context_prompt
Expand Down Expand Up @@ -2175,9 +2176,10 @@ async def _handle_message_with_agent(self, event, source, _quick_key: str, run_g
# Admission/typing is not execution. All routing, authorization and
# turn preparation gates have passed when the agent runner is entered.
event._heartbeat_execution_started = True
# Internal events reuse the last human turn's channel inputs (see _pinned_channel_inputs).
# Internal wakes and synthetic continuations reuse established prompt-pin inputs.
_turn_channel_prompt, _turn_source = self._pinned_channel_inputs(
session_key, event.channel_prompt, source, internal=event.internal,
session_key, event.channel_prompt, source,
preserve_pin=self._event_preserves_prompt_pins(event),
)
if not event.internal:
# Persist the coherent context+channel pair before execution: a crash during the
Expand Down Expand Up @@ -3837,7 +3839,8 @@ async def _run_agent_queued_followup(
next_message_id = self._reply_anchor_for_event(pending_event)
next_inbound_id = str(pending_event.message_id) if getattr(pending_event, "message_id", None) else None
next_channel_prompt, next_source = self._pinned_channel_inputs(
next_session_key, pending_event.channel_prompt, next_source, internal=pending_event.internal,
next_session_key, pending_event.channel_prompt, next_source,
preserve_pin=self._event_preserves_prompt_pins(pending_event),
)
if not pending_event.internal:
# A drained human turn re-pins its channel inputs; make them durable like a first turn.
Expand Down
17 changes: 10 additions & 7 deletions gateway/slash_commands_goals.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,16 @@ def _enqueue_goal_turn(
try:
adapter, quick_key = self._adapter_and_key_for(event)
if text and adapter and quick_key:
turn = MessageEvent(
text=text,
message_type=MessageType.TEXT,
source=event.source,
message_id=event.message_id if kickoff else None,
channel_prompt=event.channel_prompt if kickoff else None,
)
if kickoff:
turn = MessageEvent(
text=text,
message_type=MessageType.TEXT,
source=event.source,
message_id=event.message_id,
channel_prompt=event.channel_prompt,
)
else:
turn = self._synthetic_prompt_event(event.source, text)

@andrexibiza andrexibiza Sep 29, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Keep the receiving-bot identity when routing resume through this factory

This new call replaces the previous source=event.source with _synthetic_prompt_event(...). When source.message_id is present, that factory calls plain dataclasses.replace(source, message_id=None) (run_goals.py:109). It silently drops _identity, _transport_adapter_ref, and _authorization_profile_home; source.profile survives but identifies the runtime, not the receiving bot.

The failing path is /goal resume while a turn is active, with multiplex_profiles and a route to a different runtime. Resume is permitted by _busy_goal_command. _enqueue_goal_turn places its continuation on the original adapter, but _run_agent_drain_pending -> _run_agent_queued_followup -> recursive _run_agent consumes the copied source without re-entering adapter ingress. _delivery_adapter_for then falls back to the runtime's bot.

Reproduced in a partially composed probe through the real FIFO drain and TurnRunner.send_progress_messages(): the entire _run_agent_inner is replaced with a progress-sender driver; goal-liveness checking, first-response delivery, inbound preprocessing, cache-count refresh, and pin persistence are stubbed, with fake platform I/O. In that probe, primary -> team_b runtime sends progress via TEAM_B, and team_b -> default runtime sends via PRIMARY. The exact same six-case experiment at merge base fd0b98f62564 passes; head 69c12764dfc6 fails those two cases while four identity-copy/canonicalization-seam controls pass. Kickoff and same-owner controls also pass. Normal adapter ingress can restore the lost identity, which is why an idle-only test misses the recursive path. This is a wrong-bot send at the transport boundary, not a claim that a live platform accepted it or that authorization was bypassed.

The common helper's unsafe clone is older code, but moving resume into it introduces this failure for a previously provenance-preserving caller. The existing gateway.session_identity.replace_source preserves precisely these attributes. Using it in _synthetic_prompt_event keeps the prompt fix and anchor clearing without changing owners.

Regression: both routed directions, source message ID present, enqueue a resume during an active turn, drain it in-band, and assert progress is sent through the receiving adapter, authorization-home identity is retained, internal remains false, preserve_prompt_pins remains true, and neither event nor source retains the reply anchor. This also covers the older goal/heartbeat callers of the shared factory. The event-level dataclasses.replace(synthetic, text=...) test does not test source-provenance preservation.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Resume uses the wrong bot

The new /goal resume path loses the receiving bot’s identity when source.message_id is present. _synthetic_prompt_event uses dataclasses.replace, which drops _identity, _transport_adapter_ref, and _authorization_profile_home.

A resume queued during an active turn reaches _run_agent_queued_followup without rebuilding that identity. If the receiving bot routes into a profile with its own bot, progress and streamed replies then use that profile’s bot instead.

The gateway guide requires source copies to use session_identity.replace_source. Use it inside the shared producer and add a test for a routed, mid-run resume.

Context Used: gateway/AGENTS.md (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: gateway/slash_commands_goals.py
Line: 90

Comment:
**Resume uses the wrong bot**

The new `/goal resume` path loses the receiving bot’s identity when `source.message_id` is present. `_synthetic_prompt_event` uses `dataclasses.replace`, which drops `_identity`, `_transport_adapter_ref`, and `_authorization_profile_home`.

A resume queued during an active turn reaches `_run_agent_queued_followup` without rebuilding that identity. If the receiving bot routes into a profile with its own bot, progress and streamed replies then use that profile’s bot instead.

The gateway guide requires source copies to use `session_identity.replace_source`. Use it inside the shared producer and add a test for a routed, mid-run resume.

**Context Used:** gateway/AGENTS.md ([source](https://github.com/nousresearch/hermes-agent/blob/main/gateway/AGENTS.md))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

self._enqueue_fifo(quick_key, turn, adapter)
except Exception as exc:
logger.debug("goal %s failed: %s", label, exc)
Expand Down
1 change: 1 addition & 0 deletions tests/gateway/test_goal_continuation_drain.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,3 +202,4 @@ async def test_runner_goal_hook_enqueues_into_the_key_the_adapter_drains(hermes_
f"drains: pending keys={list(adapter._pending_messages)} "
f"expected={adapter_key}"
)
assert getattr(adapter._pending_messages[adapter_key], "preserve_prompt_pins", False) is True
15 changes: 15 additions & 0 deletions tests/gateway/test_goal_resume_restart.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,18 @@ def _resume_event() -> MessageEvent:


class TestGatewayResumeRestartsWork:
def test_goal_kickoff_keeps_current_prompt_identity(self):
runner, adapter = _make_runner()
event = _resume_event()
event.channel_prompt = "Channel hint."

runner._enqueue_goal_turn(event, "kickoff", label="test kickoff", kickoff=True)

pending = adapter._pending_messages[_GW_KEY]
assert pending.message_id == event.message_id
assert pending.channel_prompt == "Channel hint."
assert pending.preserve_prompt_pins is False

@pytest.mark.asyncio
async def test_resume_after_budget_exhaustion_enqueues_continuation(
self, hermes_home
Expand All @@ -188,6 +200,9 @@ async def test_resume_after_budget_exhaustion_enqueues_continuation(
"— otherwise the goal sits idle until the next real user message"
)
assert pending.text.startswith("[Continuing toward your standing goal]")
assert pending.internal is False
assert pending.channel_prompt is None
assert getattr(pending, "preserve_prompt_pins", False) is True
# The pause/clear stale-work guard must recognize the queued turn as
# a synthetic goal continuation so it can be cleaned up on /goal pause.
assert GatewayRunner._is_goal_continuation_event(pending)
Expand Down
1 change: 1 addition & 0 deletions tests/gateway/test_heartbeat_poller.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ async def handler(event):
await runner._heartbeat_poll_once(watch)
assert len(received) == 1
assert not received[0].internal # authorization and emergency-stop still apply
assert getattr(received[0], "preserve_prompt_pins", False) is True
# Gateway-stamped provenance: the turn may end silently (#113031) and is not a reply to
# the message that registered the watch (#112149).
assert display_kind_for_event(received[0]) == INTERNAL_NOTIFICATION_DISPLAY_KIND
Expand Down
103 changes: 103 additions & 0 deletions tests/gateway/test_internal_event_pin_wiring.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

from __future__ import annotations

import dataclasses
from datetime import datetime
from unittest.mock import AsyncMock, MagicMock

Expand Down Expand Up @@ -320,6 +321,77 @@ async def test_event_backed_followup_overrides_inherited_channel_prompt(monkeypa
assert len(calls) == 1
assert calls[0]["channel_prompt"] == "Queued event prompt.", "event prompt must win over the inherited one"


@pytest.mark.asyncio
async def test_non_internal_synthetic_event_preserves_all_prompt_pins(monkeypatch):
"""#126109: non-internal synthetic turns inherit both session-context and channel pins."""
config = GatewayConfig()
config.platforms[Platform.DISCORD] = PlatformConfig(
enabled=True,
channel_overrides={PARENT_ID: ChannelOverride(system_prompt="Parent persona.")},
)
runner = _make_runner(monkeypatch, config)
calls: list[dict] = []
_capture(runner, calls)

source = _human_thread_source()
await _drive(runner, ((False, source),), channel_prompt="Channel hint.")

synthetic = runner._synthetic_prompt_event(source, "[Continuing toward your standing goal]")
assert synthetic.internal is False
assert synthetic.channel_prompt is None
assert synthetic.source.message_id is None
assert synthetic.preserve_prompt_pins is True
# pre_gateway_dispatch rewrite hooks rebuild events with dataclasses.replace.
synthetic = dataclasses.replace(synthetic, text=synthetic.text + " (rewritten)")
assert synthetic.preserve_prompt_pins is True

await runner._handle_message_with_agent(synthetic, synthetic.source, KEY, 1)
await _drive(runner, ((False, source),), channel_prompt="Channel hint.")

assert [call["channel_prompt"] for call in calls] == ["Channel hint."] * 3
ephemeral = [_effective_ephemeral(runner, call) for call in calls]
assert "Triggering message: provided per-turn" in ephemeral[0]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Assert prompt continuity without freezing this sentence

This literal substring is not the byte-stability contract. In the local uncommitted composition of this head with #127944 at dce6efedfca2ae5960e05ed17325938bd6d10322, the Discord renderer intentionally says that a triggering-message ID is present when the turn comes from a Discord message. This assertion fails before the adjacent three-prompt equality assertion can execute.

The union tree is a4943ea9eec243f58e0312c613939498e585aac6. The canonical six-file run produced 42 passed, 1 failed, exclusively this test at line 354. This file passes standalone at the reviewed head. Five independent lifecycle checks on the same union, including the human → synthetic → human equality check without the sentence literal, all pass. This demonstrates a conditional test-composition failure, not a regression in the combined prompt behavior or a claim of live provider-cache performance.

The repair is to retain semantic equality and the channel/parent inputs as the oracle; if fixture activation needs checking, assert the relevant metadata/capability relationship instead of obsolete wording. Regression acceptance is this pin-wiring file plus the complementary voice tests green on the exact composed tree. The separate queue failures remain unresolved; changing this test does not fix them. No existing product or test source was modified to obtain the observations above.

assert ephemeral[0] == ephemeral[1] == ephemeral[2]
assert runner._peek_session_state(KEY).conversation.channel_pin == ("Channel hint.", PARENT_ID)


@pytest.mark.asyncio
async def test_queued_synthetic_event_preserves_channel_pin(monkeypatch):
"""In-band FIFO continuations consume the same preservation marker."""
runner = _make_runner(monkeypatch)
calls: list[dict] = []
_capture(runner, calls)
runner._run_agent_deliver_first_response = AsyncMock()
runner._refresh_agent_cache_message_count = AsyncMock()
runner._prepare_profile_scoped_inbound_message_text = AsyncMock(return_value="continue")
runner._session_key_for_source = lambda source: KEY

source = _human_thread_source()
await _drive(runner, ((False, source),), channel_prompt="Channel hint.")
first = calls[0]
turn_ctx = TurnContext(
source=first["source"],
context_prompt=first["context_prompt"],
channel_prompt=first["channel_prompt"],
session_key=KEY,
session_id="sess-wiring",
run_generation=1,
history=[],
)
adapter = MagicMock()
adapter._active_sessions = {}
pending = runner._synthetic_prompt_event(source, "[goal] continue")

await runner._run_agent_queued_followup(
turn_ctx, adapter, pending.text, pending, "done",
{"final_response": "done", "messages": []}, None,
)

assert len(calls) == 2
assert calls[1]["channel_prompt"] == "Channel hint."
assert runner._peek_session_state(KEY).conversation.channel_pin == ("Channel hint.", PARENT_ID)

@pytest.mark.asyncio
async def test_first_internal_event_after_restart_rehydrates_durable_prompt_pins(monkeypatch):
config = GatewayConfig()
Expand Down Expand Up @@ -362,6 +434,37 @@ async def test_first_internal_event_after_restart_rehydrates_durable_prompt_pins
assert _effective_ephemeral(after, first_internal) == _effective_ephemeral(before, human)


@pytest.mark.asyncio
async def test_first_synthetic_event_after_restart_rehydrates_durable_prompt_pins(monkeypatch):
"""#126109 across a restart: a goal/heartbeat continuation is usually the first turn a
restarted gateway runs for a session, and it is non-internal, so it must adopt the durable
pins on the same decision the in-process reuse makes (``preserve_prompt_pins``)."""
config = GatewayConfig()
config.platforms[Platform.DISCORD] = PlatformConfig(
enabled=True,
channel_overrides={PARENT_ID: ChannelOverride(system_prompt="Parent persona.")},
)
durable: dict = {}
before = _make_runner(monkeypatch, config, durable_prompt_pin=durable)
calls_before: list[dict] = []
_capture(before, calls_before)
await _drive(before, ((False, _human_thread_source()),), channel_prompt="Channel hint.")
assert durable["value"]["channel_prompt"] == "Channel hint."

after = _make_runner(monkeypatch, config, durable_prompt_pin=durable)
calls_after: list[dict] = []
_capture(after, calls_after)
synthetic = after._synthetic_prompt_event(_human_thread_source(), "[Continuing toward your standing goal]")
assert synthetic.internal is False and synthetic.preserve_prompt_pins is True
await after._handle_message_with_agent(synthetic, synthetic.source, KEY, 1)
await _drive(after, ((False, _human_thread_source()),), channel_prompt="Channel hint.")

human, first_synthetic, next_human = calls_before[0], calls_after[0], calls_after[1]
assert [human["channel_prompt"], first_synthetic["channel_prompt"], next_human["channel_prompt"]] == ["Channel hint."] * 3
assert _effective_ephemeral(after, first_synthetic) == _effective_ephemeral(before, human)
assert _effective_ephemeral(after, next_human) == _effective_ephemeral(before, human)


@pytest.mark.asyncio
async def test_human_first_after_restart_ignores_stale_durable_prompt_pin(monkeypatch):
durable = {
Expand Down
Loading