Skip to content

fix(gateway): preserve prompt pins across synthetic goal/heartbeat/resume turns - #127208

Closed
EdderTalmor wants to merge 2 commits into
NousResearch:mainfrom
EdderTalmor:edder/p0b-channel-pin
Closed

EdderTalmor wants to merge 2 commits into
NousResearch:mainfrom
EdderTalmor:edder/p0b-channel-pin

Conversation

@EdderTalmor

Copy link
Copy Markdown

Fixes #126109.

Root cause

Gateway turns injected from synthetic events (goal continuation, heartbeat due-prompts, /goal resume) are built as non-internal MessageEvents with channel_prompt=None — they must stay non-internal so authorization and the emergency stop still apply. _pinned_channel_inputs only reused the session's pinned channel inputs for internal events, so every synthetic turn recorded (None, parent_chat_id) as the new channel_pin, dropping the channel prompt / channel_overrides prompt for that turn and flipping it back on the next human turn (the same A→B→A pattern as #125763 / #124179: shorter ephemeral system prompt, changed agent-config signature, cached-agent rebuild, prefix-cache miss). The synthetic turn also strips message_id, a session-context change-key input, so the context pin flipped too.

Fix

A dedicated MessageEvent.preserve_prompt_pins flag separates "reuse the pins" from internal (which also gates authorization bypass). _synthetic_prompt_event (the single producer for goal continuation + heartbeat due-prompts) sets it, and /goal resume continuations now route through _synthetic_prompt_event instead of building their own flag-less event. Both pin helpers accept optional preserve_pin (defaulting to internal, so all existing callers are unaffected); the three run_turn.py call sites resolve it via the new _event_preserves_prompt_pins helper. _queue_or_replace_pending_event treats the flag as queue identity so a synthetic continuation never merges with / absorbs a human photo turn.

Prior art

Supersedes two conflicting approaches on the same issue, both credited:

This PR ports the #126167 approach onto current main (it keeps the helper signatures backward-compatible and covers the session-context pin and the queue-merge path, which #126155 does not).

Verification (on current upstream/main tip fd0b98f625)

RED first: a temporary repro driving human → synthetic (via the real _synthetic_prompt_event + _handle_message_with_agent) → human failed on the unfixed tree with ['Channel hint.', None, 'Channel hint.'] (pin stomped to None on the middle turn); removed after confirming.
GREEN after the fix — byte-identical effective ephemeral across all three turns, channel_pin == ('Channel hint.', parent):

  • tests/gateway/test_internal_event_pin_wiring.py (incl. 2 new regression tests)
  • tests/gateway/test_queue_consumption.py, test_heartbeat_poller.py, test_goal_continuation_drain.py, test_goal_resume_restart.py, test_prompt_tail_freeze.py
  • 38 neighboring gateway files exercising the touched paths (_handle_message_with_agent, queued followups, heartbeat/goal/queue/pin wiring): 322 passed, 2 pre-existing skips, 0 failed.
    Note: the canonical scripts/run_tests.sh could not bootstrap here (blocked artifact download), so tests were run as python -m pytest tests/gateway/<file> with the runner's deterministic env (TZ=UTC, LANG=C.UTF-8, PYTHONHASHSEED=0).

@alt-glitch alt-glitch added type/bug Something isn't working P0 Critical — data loss, security, crash loop comp/gateway Gateway runner, session dispatch, delivery area/sessions Session lifecycle, resume, persistence, history sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Sep 29, 2026
teknium1 pushed a commit to EdderTalmor/hermes-agent that referenced this pull request Sep 29, 2026
…ns too

After a restart the first turn a session runs is usually a goal/heartbeat
continuation, and it is non-internal. _rehydrate_prompt_pins was still gated
on event.internal, so that turn found no in-process pins, ran with
channel_prompt=None and re-rendered the session context from the
message_id-less synthetic source; the next human turn flipped it back
(1270 -> 1139 -> 1270 bytes on a Discord thread). Gate the rehydrate on the
same preserve-pins decision the in-process reuse already uses.

Review-fix for NousResearch#127208 (NousResearch#126109).
teknium1 added a commit to EdderTalmor/hermes-agent that referenced this pull request Sep 29, 2026
…ns too

After a restart the first turn a session runs is usually a goal/heartbeat
continuation, and it is non-internal. _rehydrate_prompt_pins was still gated
on event.internal, so that turn found no in-process pins, ran with
channel_prompt=None and re-rendered the session context from the
message_id-less synthetic source; the next human turn flipped it back
(1270 -> 1139 -> 1270 bytes on a Discord thread). Gate the rehydrate on the
same preserve-pins decision the in-process reuse already uses.

Review-fix for NousResearch#127208 (NousResearch#126109).
@teknium1
teknium1 force-pushed the edder/p0b-channel-pin branch from b4da6cb to f016e9a Compare September 29, 2026 02:07

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

PR Review — #127208 (reviewed head 00385d4148f1; review-fix pushed as f016e9aa9bfa)

Verdict: request changes — the code is correct and now complete, but the commit re-authors another contributor's work. Merge decision is Teknium's.

Context. Goal-continuation, heartbeat and /goal resume turns are synthetic but deliberately non-internal (authorization / emergency-stop still apply), and internal was the only proxy for "reuse the prompt pins". Every such turn overwrote channel_pin with (None, parent) and, on Discord, re-keyed the session-context pin because _synthetic_prompt_event strips message_id (#126109). The preserve_prompt_pins flag separates the two meanings.

Premise verified on origin/main @ fd0b98f6: unchanged since the #126167 review — human → synthetic → human gives channel_prompt == ['Channel hint.', None, 'Channel hint.'] and context bytes flip A→B→A.

Attribution (blocking as submitted). The source hunks in this commit are byte-identical to #126167 (d2d80520, @JoaoMarcos44) apart from three docstring lines adapted to _rehydrate_prompt_pins on main; the five test hunks are identical. The PR body says "ports the #126167 approach", but the git author is you and there is no Co-authored-by: trailer. Porting someone else's diff onto a fresh base is a rebase, and the repo lands rebases with the original author's identity (that is exactly what our salvage flow does with git cherry-pick). Please either amend 00385d41 with
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
(or re-author it to him), or leave #126167 as the vehicle — its conflict with main is two docstrings in run_agent_cache.py plus the test file's context, a trivial rebase. Whichever vehicle Teknium picks, the credit goes to @JoaoMarcos44 for the fix and to @chelsealong (#126155) for the first diagnosis.

Gap found and fixed on this head (pushed to your branch, maintainerCanModify): main has landed durable prompt pins since #126167 was written, and run_turn.py::_hmwa_prepare_turn still gated _rehydrate_prompt_pins on event.internal. After a gateway restart the first turn a session runs is typically a goal continuation / heartbeat due-prompt, which is non-internal, so it found no in-process pins and the flip came back for exactly one cycle. Probe on 00385d41 (human turn → new runner → _synthetic_prompt_event → human turn): channel_prompt == ['Channel hint.', None, 'Channel hint.'], ephemeral bytes 1270 → 1139 → 1270. f016e9aa gates the rehydrate on _event_preserves_prompt_pins(event) (1 line) and adds test_first_synthetic_event_after_restart_rehydrates_durable_prompt_pins, red on 00385d41, green on f016e9aa.

Tests (worktree = this head, fake HOME for the .worktrees/manifest.json guard): scripts/run_tests.sh on the 5 touched test files + test_prompt_tail_freeze.py + test_queued_final_ledger.py: 50 passed, 0 failed at f016e9aa (40/40 at 00385d41). ruff, no-tmp-literals, windows-footguns clean. Merge-tree against origin/main is clean.

Suggestions (non-blocking).

  • run_turn.py still calls _persist_prompt_pins for a pin-preserving synthetic turn (if not event.internal: at both sites). The pins are unchanged at that point and the store no-ops an unchanged snapshot, so it is a wasted round-trip rather than a bug; gating both on not self._event_preserves_prompt_pins(event) would make the three decisions read from one predicate.

Looks good. preserve_pin defaulting to internal keeps every other caller byte-identical; the run_busy.py queue-identity guard stops a synthetic head from absorbing a human photo under the wrong prompt identity; the kickoff negative control in test_goal_resume_restart.py pins the #124179 contract.

@teknium1 teknium1 added the needs-decision Awaiting maintainer decision before any implementation label Sep 29, 2026
197609 and others added 2 commits September 28, 2026 22:18
…sume turns

Synthetic turns (goal continuation, heartbeat due-prompts, /goal resume)
are non-internal so authorization still applies, but carry no channel
inputs of their own. _pinned_channel_inputs treated every non-internal
event as a human turn and recorded (None, parent_chat_id) as the
channel_pin, flipping the ephemeral system prompt for that turn and
back on the next human turn (A->B->A: agent rebuild, prefix-cache miss).

Add MessageEvent.preserve_prompt_pins; _synthetic_prompt_event sets it,
and /goal resume continuations now route through _synthetic_prompt_event
so there is a single pin-preserving producer. Both pin helpers take an
optional preserve_pin (defaulting to internal, so existing callers are
unaffected) and the three run_turn.py call sites resolve it via the new
_event_preserves_prompt_pins helper. _queue_or_replace_pending_event
treats the flag as identity so a synthetic continuation never absorbs a
human photo's prompt shape.

Fixes NousResearch#126109.

Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
…ns too

After a restart the first turn a session runs is usually a goal/heartbeat
continuation, and it is non-internal. _rehydrate_prompt_pins was still gated
on event.internal, so that turn found no in-process pins, ran with
channel_prompt=None and re-rendered the session context from the
message_id-less synthetic source; the next human turn flipped it back
(1270 -> 1139 -> 1270 bytes on a Discord thread). Gate the rehydrate on the
same preserve-pins decision the in-process reuse already uses.

Review-fix for NousResearch#127208 (NousResearch#126109).
@EdderTalmor

Copy link
Copy Markdown
Author

Attribution fixed: 00385d4 now carries Co-authored-by: JoaoMarcos44 joaomarcosdias444@gmail.com, with your f016e9a fixup cherry-picked on top (branch force-pushed, same tree). Credit to @JoaoMarcos44 for the fix and @chelsealong for the first diagnosis — thank you for the review and the restart-gap catch.

@ehz0ah ehz0ah left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Request changes

Motivation

This addresses a real prompt-cache stability bug. Goal continuations, heartbeat prompts, and /goal resume turns need to reuse the established session and channel prompt pins while remaining normal, authorized events. The restart follow-up also correctly rehydrates durable pins before the first synthetic turn.

Approach

The new MessageEvent.preserve_prompt_pins marker cleanly separates prompt identity from internal authorization semantics. The canonical synthetic-event builder sets it, both prompt-pin helpers consume one shared decision, and goal kickoff remains the negative control with its current prompt identity.

Concrete behavior

Before this change, a human turn followed by a synthetic continuation and another human turn could produce an A to B to A system-prompt flip. At this head, the normal and post-restart paths keep those prompt bytes stable. The direct FIFO/photo merge guard also recognizes the new identity marker.

Risk and validation

One active queue path still merges across that identity boundary; see the inline blocking comment. I reproduced it with the repository test runner at this exact source tree: a pending synthetic continuation becomes [goal] continue\nreal human follow-up under busy_text_mode: queue. The 46 existing focused gateway tests pass, so the gap is not covered by the current suite. GitHub reports no checks or commit statuses for this head.

Overall verdict

REQUEST_CHANGES. Keep the prompt-pin design and restart coverage, but close every queue merge path over the new identity boundary before this merges.

Comment thread gateway/run_busy.py
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.

@andrexibiza andrexibiza left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

PR Review — #127208 (head 69c12764dfc6, base main)

Summary. The mechanism is sound and the core direction is right: preserve_prompt_pins separates "reuse prompt-pin identity" from internal authorization, _synthetic_prompt_event is now the single pin-preserving producer for goal/heartbeat/resume continuations, and both the restart-adoption path and the FIFO identity guard carry behavior tests. On a head worktree I ran the focused gateway set (9 files, 68 passed, deterministic env) and re-ran the affected CI gates below.

Two P1 blockers remain, both reproduced at this head:

  1. The new identity boundary is enforced at one merge site; the sibling merge paths still fuse a human input into a pending pin-preserving synthetic. The debounce instance is already filed by @ehz0ah inline (id 4128991175, gateway/run_busy.py:390) and I reproduced it exactly; the additional sites below are uncovered by any existing comment and by this PR's tests.
  2. A fused turn is silently destructive downstream: goal bookkeeping identifies continuations by text prefix ([Continuing toward your standing goal]\nGoal:), and a fused human turn still matches. /goal pause|clear deletes it, and the stale-continuation gate discards it without running or replying. Reproduced.

A P3 residual (post-eviction pin flip), the CI/attribution reconciliation, and the prior-review items follow.


P1 — the merge-identity boundary is incomplete (extends the already-filed debounce blocker)

At this head, a human input that lands while a pin-preserving continuation sits in the pending slot is merged into that synthetic event at every merge site except _queue_or_replace_pending_event — the one function this PR guarded (gateway/run_busy.py:389-390). Reproduced at this head, real functions only, head worktree 69c12764:

  • Telegram grace queue (gateway/run_inbound.py:672 — merge_text=True): session busy on a Telegram DM, synthetic continuation pending, human text arrives inside HERMES_TELEGRAM_FOLLOWUP_GRACE_SECONDS → pending text becomes "[Continuing toward your standing goal]\nGoal: …\n<human text>", preserve_prompt_pins stays True.
  • Boot-sentinel path (gateway/run_inbound.py:745 — merge_text=True): same result while _AGENT_PENDING_SENTINEL holds the slot ("picked up after start").
  • 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 appended to the synthetic, no interrupt attempted. This contradicts the PR body's "a synthetic continuation never merges with / absorbs a human photo turn"; that holds for _queue_or_replace_pending_event (and its dedicated test), not for this fast path.
  • Adapter debounce (gateway/platforms/base.py:3858, merge at 3812): reproduced as filed by @ehz0ah — pending "[goal] continue" + human text → "[goal] continue\nreal human follow-up"; their inline stands.

What the fused turn means at consumption: run_turn's _event_preserves_prompt_pins resolves True, so _pinned_channel_inputs/_pinned_session_context_prompt reuse the pinned identity — the human message runs under the continuation's prompt identity (the exact prompt-cache inversion this PR exists to prevent, now for real user content), and the human turn's own inputs are not recorded (the preserve branch returns the established pin without updating it; the following _persist_prompt_pins then persists the unchanged snapshot). Reproduced: merged event resolves preserve=True; the turn uses the pinned channel prompt ("Channel hint.").

Required repair. Enforce the boundary where merges are attempted, not in one caller. Smallest complete options: (a) make merge_pending_message_event / _can_merge_text_debounce_events identity-aware (refuse absorption when preserve_prompt_pins differs, or when the target is pin-preserving and the incoming event is human), or (b) add one shared predicate and call it from the three run_inbound sites plus the debounce pair. The invariant to encode: two turns with different prompt identity never become one turn — a pin-preserving event must never be a merge target.

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

P1 — fused continuations are silently destructive to real user messages

GatewayRunner._is_goal_continuation_event (gateway/run_busy.py:180) matches by text prefix. Because merging appends the human text after the continuation block, a fused turn still matches:

  • /goal pause|clear → _clear_goal_pending_continuations (gateway/run_busy.py:191-197) pops the fused slot / filters the overflow entry as "cleanup": the user's pending message is deleted. Reproduced: removed=1, pending slot emptied, overflow emptied.
  • run_turn.py:3817 first clause is True for the fused text, so when the goal is no longer active (not self._goal_still_active_for_session(...)) the fused turn is return result-discarded — the human content never runs and no reply is sent (stale-goal discard, no evidence trail for the user).

So the same fusion that re-identifies a human turn can also make it disposable as "synthetic machinery". Both reproduced at this head with the real predicate and cleanup function.

Required repair. Primary: prevent the fusion (previous finding). Defense in depth: stop identifying continuations by a text prefix for destructive operations — the producer already sets preserve_prompt_pins=True; use a structural marker for cleanup/discard decisions, and never pop or black-hole a queued event carrying content beyond the continuation block.

Regression contract. After repair, 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.

P3 — post-eviction synthetic turn still flips the session-context pin once (pre-existing, decision requested)

Not introduced here, but it is the same A→B→A class and sits inside this PR's blast radius. _rehydrate_prompt_pins early-returns while either pin exists, and _evict_cached_agent deliberately keeps channel_pin while clearing ephemeral_pin. Sequence at this head (harness with the durable store): human turn pins A → evict → synthetic continuation → context prompt re-renders B (dropping the Triggering message line) and pins B → next human turn re-renders back to A. Reproduced: e0==e1 → False, e0==e2 → True; counterfactual with channel_pin also cleared adopts the durable snapshot verbatim (adopted bytes == turn-1 human pin: True). Either document that eviction deliberately forces re-render for pin-preserving turns too, or treat an incompletely-cleared pin set as adoptable for preserve turns. No action required to merge; flagging for the intent call.


CI state and attribution gate (reconciled)

  • check-attribution fails at this head. The gate range fd0b98f6..69c12764 contains 197609@users.noreply.github.com (commit 512554a6, author name "Edder Talmor"), which is not in contributors/emails/ and not in the legacy map. Reproduced two ways: the workflow's own shell logic replayed locally (MISSING), and the repo's scripts/audit_pr_attribution.py (report-only) — ⚠️ unmapped: 197609@users.noreply.github.com (Edder Talmor). The previous CI run (36511085675) failed on exactly this email. Required: contributors/emails/197609@users.noreply.github.com with the intended login.
  • Do not blind-run audit_pr_attribution.py --fix for this email. The script's bare-noreply resolution maps the local part to login 197609 because that account exists (gh api users/197609 → id 53029273, "Yinan Lv"), and GitHub's own commit API associates 512554a6 with @197609, while the PR author and git author name are @EdderTalmor (id 45949729). Pick the mapping deliberately (author confirmation), since --fix would write 197609 and CI would go green on a likely-wrong credit.
  • The Windows-footguns/Bash-shebang failure in run 36511085675 was base-side. It flagged apps/desktop/electron/remote-lifecycle.test.ts:813 (#!/bin/bash), which came from the merge with main@36dfe585; the branch diff does not touch that file. The #!/bin/bash form has since been replaced on main (3a439b69), and the head tree, the GitHub merge ref (592e5635), and the current main tip all pass scripts/check_bash_shebangs.py (0 violations) — no branch action needed; rerun CI after the mapping lands.
  • This head has zero check-runs (action_required — fork runs await approval), so CI has not certified 69c12764; @ehz0ah noted the same. Mergeable: MERGEABLE; merge state BLOCKED.

Prior-review reconciliation

  • teknium1 (CHANGES_REQUESTED @ f016e9aa) — attribution: 512554a6 now carries Co-authored-by: JoaoMarcos44 (JoaoMarcos44 credited for the fix, chelsealong for the diagnosis); the review-fix commit's tree is identical to this head's (f016e9aa^{tree} == 69c12764^{tree}), so the restart-rehydrate fix is in the reviewed tree. His non-blocking suggestion (gate _persist_prompt_pins on the same predicate at both run_turn.py sites) remains open and still worth doing so the three decisions read from one predicate. Merge decision is his.
  • ehz0ah (COMMENT @ 69c12764) — debounce finding verified and reproduced; extended here to the grace/sentinel/photo surfaces and the downstream destruction paths.

Test evidence (this review)

  • python -m pytest (deterministic env: TZ=UTC, LANG=C.UTF-8, PYTHONHASHSEED=0) on 9 focused files at this head: 68 passed — test_internal_event_pin_wiring.py, test_goal_resume_restart.py, test_goal_continuation_drain.py, test_heartbeat_poller.py, test_queue_consumption.py, test_active_session_text_merge.py, test_busy_session_ack.py, test_prompt_tail_freeze.py, test_queued_final_ledger.py.
  • check_no_tmp_literals clean; check_bash_shebangs clean on head + fresh merge ref (see CI section).
  • Repros for findings 1–3 executed against the real functions at 69c12764 (drivers kept off-tree; exact sequences above).

Review performed at the live head 69c12764dfc6e7d8329a7113aa1ae0de84f51307; published as a COMMENT review — the P1 items above are listed for the author, and the merge decision remains with the maintainers.

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.

Comment thread gateway/run_goals.py
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.

"""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.

@andrexibiza andrexibiza left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Supplemental review: prompt identity survives, receiving-bot identity does not

Reviewed head: 69c12764dfc6e7d8329a7113aa1ae0de84f51307.
Comparison base: fd0b98f6256446a9115c54f1a92ace5ea74c929f.

One additional P1 regression: a busy /goal resume can send the resumed turn's progress through a different bot. This is separate from the merge/cleanup findings in the earlier review and the debounce finding; those are not being filed again here.

P1 - preserve transport provenance when moving resume into the synthetic factory

The new call at slash_commands_goals.py:90 changes more than the prompt-preservation flag. Previously a resume continuation retained event.source. It now calls _synthetic_prompt_event, whose source clone uses plain dataclasses.replace(source, message_id=None).

That copy drops the dynamic _identity, _transport_adapter_ref, and _authorization_profile_home. The serialized source.profile survives, but it names the runtime, not necessarily the receiving bot. The repository already supplies session_identity.replace_source specifically to preserve these fields.

The reachable case is /goal resume during an active turn, with a source message ID and a route whose runtime differs from its transport owner. The command is allowed while busy. Its continuation enters the receiving adapter's FIFO, then the runner drains it in-band. _run_agent_queued_followup passes the copied source into the recursive turn without re-entering the adapter message handler. The delivery resolver consequently falls back to the runtime profile's adapter.

I exercised the enqueue, dequeue, recursive follow-up, profile-scoping wrapper, delivery resolver, and real TurnRunner.send_progress_messages() together as a partially composed routing probe. The entire _run_agent_inner is replaced by a driver that invokes the real progress sender; goal-liveness checking, first-response delivery, inbound preprocessing, cache-count refresh, and prompt-pin persistence are stubbed, and platform send/edit/typing calls use fake I/O. This does not execute the full busy-command dispatch, inner-turn setup, goal lifecycle, or persistence path. The recorded adapter.send calls were:

Receiving bot and route Expected progress sender Actual progress sender
Primary bot, routed to team_b which owns another bot Primary team_b
team_b bot, routed to default team_b Primary

The resumed turn crosses the bot/credential boundary even though its session key and cached prompt can look consistent. This can split progress from the outer response or fail delivery when the other bot cannot reach that chat. The experiment establishes the wrong transport selection, not a live platform delivery or authorization bypass.

Why ordinary coverage misses it: normal adapter ingress can restore the identity; the recursive drain does not run that ingress. Both controls that explicitly invoke the runner canonicalization seam passed; these are not full adapter-handler runs. Same-owner routing and all kickoff controls also passed. The new dataclasses.replace(synthetic, text=...) test protects a dataclass field on the event, but it cannot detect provenance already erased by the earlier copy of the source.

Repair and regression contract: keep the shared synthetic producer, internal=False, the preservation flag, and cleared reply anchor. Use the existing identity-preserving source-copy helper there. Exercise both routed directions through the in-band drain and assert the original bot receives progress sends, the original authorization home remains attached, and both message-ID fields remain empty. Normal kickoff and ordinary ingress should retain their current behavior. The unsafe clone predates this PR for other factory users; the new resume call introduces this regression for /goal resume.

Execution evidence

All runs used scripts/run_tests.sh, native Windows Python 3.11.11, isolated home/state/temp directories, one worker, and zero file retries. The import probe confirmed the reviewed worktree supplied the product modules.

Experiment Base Reviewed head
Enqueue/identity probes, including kickoff, same-owner and identity-aware-copy controls; also independently replaying the existing debounce case 9 passed, 1 failed (debounce) 7 passed, 3 failed (debounce plus both routed resumes)
Composed FIFO-to-progress probe, including identity-copy and ingress controls 6 passed 4 passed, 2 failed (both unmodified routed resumes)
Existing focused tests, six files Not run in this comparison 38 passed, 0 failed

The six existing files were test_internal_event_pin_wiring.py, test_queue_consumption.py, test_heartbeat_poller.py, test_goal_continuation_drain.py, test_prompt_tail_freeze.py, and test_multiplex_transport_matrix.py. This includes the synthetic first-turn-after-restart pin test. test_goal_resume_restart.py was not run here: the local command policy rejected the original selection containing that filename; the reduced selection above excludes it. No whole-suite or live provider-cache claim is made.

Prior work and current CI

  • JoaoMarcos44's co-author trailer is present in 512554a6f56a; chelsealong's earlier diagnosis remains credited in the PR. This observes the repaired co-author attribution, not resolution of the separate contributor-registry question in the earlier review.
  • teknium1's restart rehydration change is present, and the relevant pin-wiring tests pass in this run.
  • The previously reported debounce fusion still reproduces at this head. The other merge, cleanup and eviction observations remain in the earlier review; this supplement does not certify or replace those separate findings.
  • The exact head currently has zero check-runs and three workflow runs marked action_required (CI, Nix, Docker). Those workflows have not certified this head.

This review is submitted as COMMENT, with the new P1 attached to the changed resume call site. No product code or branch history was modified.

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.

@andrexibiza andrexibiza left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Second-pass review of head 69c12764dfc6e7d8329a7113aa1ae0de84f51307.

No additional actionable findings beyond the existing review.

I re-traced the prompt-pin lifecycle through synthetic event construction, ingress/event rewriting, FIFO follow-ups, restart rehydration, and session reset/invalidation ordering. The preservation marker survives the normal event-copy paths, and I did not find a distinct correctness failure that is both reproducible and not already covered by the prior review.

Focused verification: 41 relevant gateway tests passed, including the prompt-pin wiring and continuation paths. I also exercised human → synthetic → human reuse through the composed agent path; the cached agent remained stable across the sequence.

@andrexibiza andrexibiza left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P0 review — prompt identity still breaks at queue recovery boundaries

The normal synthetic-turn repair is real. The canonical producer, shared preservation decision, and cold-runner adoption call preserve the effective prompt across the tested human → synthetic → human sequence. Credit remains with JoaoMarcos44 for the underlying approach, chelsealong for the earlier channel-pin diagnosis, EdderTalmor for this port, and teknium1 for the restart follow-up; the implementation and co-author trailer and follow-up commit establish that lineage.

The P0 acceptance contract remains open. An additional P1 queue-conservation finding and a conditional P2 composition finding are evidenced below: recursion-cap recovery destroys the separation established at admission. It is inherited, not introduced by this PR. The earlier merge/cleanup review, transport-provenance supplement, and eviction inline retain their individual dispositions in the reconciliation ledger; they are not being filed again as duplicate inlines. A passing focused suite is not evidence that those obligations are closed.

Navigation: revision custody · new P1 · composition P2 · RCA and subsystem map · pin lifecycle · prior claims · tests and limits · related work · acceptance.

Revision custody

Object Exact identity and evidence
Reviewed PR #127208, open at retrieval; 11 changed files, 2 commits. All file, commit, discussion, review, and inline pages were collected.
Head 69c12764dfc6e7d8329a7113aa1ae0de84f51307.
True merge base fd0b98f6256446a9115c54f1a92ace5ea74c929f, computed with git merge-base, not substituted with moving main.
Target-main snapshot e85706cba780382ef91ba7a38a20ebe95207a795. The inspected cache/queue/producer/consumer implementation files have no diff from the merge base to this snapshot.
Earlier reviewed tree f016e9aa9bfa25eb65148218bfa3732c0d0385bc and the current head both resolve to tree 52b1c0f551e0f72f5c9bc81cb401c01f87ade232. This verifies tree equivalence, not author identity or acceptance.
Prior publication 5 review objects, 5 inline comments, 1 issue comment before this supplement. Every substantive claim is disposed below in the ledger.
Exact-head hosted checks Check runs: 0. Commit statuses: 0, aggregate pending. CI, Nix, and Docker runs: 3, all action_required. No hosted green result exists for this head.
Local execution Native Windows, Python 3.11.11, pytest 9.1.1, repository canonical runner, one worker, zero file retries, isolated home/state/temp directories. Imports were checked against the pinned worktree. Evidence scope.

F1 — P1: recursion-cap recovery loses or fuses already-admitted turns

Attribution: inherited queue-loss mechanism, still present at the reviewed head. The PR adds a prompt-identity admission guard, but the consumer's recursion-cap branch does not use it. This is an additional lifecycle failure beyond the already-reported ingress/debounce fusion. It matters because successful admission can initially produce separate, correctly marked events and recovery can subsequently destroy them.

Trigger and causal path

A session has two admitted events: older A, newer B. They differ in synthetic/human prompt identity. The FIFO producer stores A in the pending slot and B in overflow. The real drain removes A and promotes B into the slot before invoking the next turn. At _interrupt_depth == _MAX_INTERRUPT_DEPTH, the follow-up consumer tries to restore A with merge_pending_message_event, rather than an order-preserving queue restoration operation.

The shared merge helper either replaces an existing text slot or absorbs media into the existing event. It does not preserve the two admitted events as separate units. Consequently:

Admitted order Observed state after cap recovery User-visible consequence
Synthetic A → human TEXT B Only synthetic A remains. The admitted human message is overwritten before it can run. Replacement branch.
Human TEXT A → synthetic B Only human A remains. The continuation is overwritten; accepted work is not conserved. Cap caller.
Human PHOTO A → synthetic B One synthetic-owned event contains both goal text and the human caption/photo. Prompt identity and human content are fused despite initial FIFO separation. The resulting goal-prefixed event can also meet the existing destructive cleanup predicate.

The same three assertions failed at merge base and head. The empty-slot cap control passed at both pins. The guarded-admission photo control fails at baseline and passes at head, proving that this PR repairs admission while leaving a separate recovery boundary open.

Experiment: the reviewer fixture uses the real synthetic factory, FIFO admission, _run_agent_drain_pending, promotion, and _run_agent_queued_followup, then asserts two distinct event objects in original arrival order and records their text/media and preservation flags. It sets the turn context to the real cap and observes the pending slot plus overflow. The adapter's dequeue is an in-memory slot pop; platform I/O and agent execution are not exercised. It is a deterministic consumer-boundary reproduction, not a claim that a live chat or provider was driven through three preceding turns. Both final cap runs report 3 failed, 1 passed, exit 1. Verification ledger.

Strongest rebuttal: the new admission guard already separates synthetic and human events. That is true and measured. It does not cover the later cap restoration call, which runs after the successor has been promoted. No simultaneous-arrival race or malformed event is necessary once the FIFO and cap state exist.

Repair contract: restore the unprocessed current event at the FIFO front without overwriting or coalescing the staged successor. Preserve the complete order A, B, remaining overflow, source identity, prompt marker, media, and admission state. These events already own queue capacity; recovery must not treat them as fresh unbounded admission or silently discard them at a cap. The existing generic repair PR #78015 owns this broader class; the current #126167 implementation contains a narrower cross-prompt-identity restoration. Those are related work, not fixes present in this head.

Regression: run both event orders plus the human-photo case through the actual drain and cap branch, then drain the restored queue to prove one distinct turn per admitted event in original order. Include an empty-slot control, additional overflow C, and a full-cap restoration control. The experiments here prove the first three failures and the empty-slot control; the remaining assertions are acceptance requirements, not claimed executions.

flowchart TD
  Q["Pending slot A and overflow B"]:::state
  D["Real drain removes A and promotes B"]:::flow
  A["A returned; slot now B"]:::state
  F["Follow-up with A at recursion depth cap"]:::boundary
  M["Shared merge restores A into occupied slot"]:::boundary
  T["Both events are text: replace B with A"]:::failure
  TL["B is no longer queued"]:::failure
  P["A carries a photo: absorb A into B"]:::failure
  PL["Human content inherits synthetic identity"]:::failure
  Q --> D --> A --> F --> M
  M -->|Both text| T --> TL
  M -->|A carries photo| P --> PL
  classDef flow fill:#11243a,stroke:#58a6ff,color:#e6edf3,stroke-width:2px
  classDef state fill:#291b3e,stroke:#bc8cff,color:#f0e6ff,stroke-width:2px
  classDef boundary fill:#3d2c12,stroke:#f5b642,color:#fff2cc,stroke-width:2px
  classDef failure fill:#471e2b,stroke:#fb7185,color:#fff1f2,stroke-width:2px
  classDef control fill:#103c30,stroke:#34d399,color:#ecfdf5,stroke-width:2px
  click Q "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_busy.py#L105-L132"
  click D "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_turn.py#L3622-L3637"
  click A "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_busy.py#L116-L132"
  click F "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_turn.py#L3784-L3796"
  click M "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/platforms/base.py#L1751-L1797"
  click T "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/platforms/base.py#L1791-L1797"
  click TL "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/platforms/base.py#L1797"
  click P "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/platforms/base.py#L1773-L1790"
  click PL "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/platforms/base.py#L1773-L1790"
Loading

Diagram edges: admission → drain → promotion → cap → replacement/media absorption. Back to navigation.

RCA and subsystem structure

The root cause in issue #126109 is the overloaded internal predicate: synthetic inputs carry no fresh prompt identity but remain non-internal, so the baseline pin owners interpret missing metadata as authoritative replacement. This PR separates those meanings at the producer and pin-selection call sites. The unresolved failure class is that prompt continuity, event conservation, and source provenance have different owners and lifetimes. A new field on MessageEvent can solve prompt selection while leaving the source clone, slot mutation, and cleanup classification unchanged.

Boundary Producer / owner / consumer Verified behavior and limit
Synthetic production Goal/heartbeat factory; heartbeat due-prompt caller; resume/kickoff branch. Synthetic turns set preserve_prompt_pins=True without becoming internal. Kickoff retains the current event's inputs. Source provenance is a separate copy contract.
Admission and buffering Adapter text debounce; runner pending-slot/FIFO admission; busy photo/grace/sentinel branches. The new guard protects one admission path. The sibling paths still fuse human content in the executed fixtures. Prior finding.
Execution and recovery Drain and recursive follow-up. A successful enqueue is not execution. Promotion changes slot ownership before the cap fallback. F1.
Goal cleanup Text-prefix classifier and cleanup; inactive-goal early return. Fusion can make human content disposable as a continuation. Executed slot cleanup, overflow cleanup, and actual early return all failed conservation assertions. Prior finding.
In-process pins Conversation-state context pin and channel pin. Existing pins are reused on a preserve turn; the context helper rejects a pin rendered under a different privacy policy. No-pin behavior differs between the two helpers. Lifecycle.
Durable pins Snapshot publisher → SessionStore fenced write/read. The store validates shape, checks expected session ID, persists before publishing the in-memory entry, and no-ops only an equal snapshot. Real-store tests passed on both pins. Store tests.
Final prompt effect TurnRunner combiner → agent resolution and run. The combiner includes context bytes, channel hint, and resolved channel override. Tests measure this combined string; they do not measure remote prefix-cache hits, tokens, billing, or latency.
Transport effect Source copy helper → delivery resolver. Prompt identity does not establish receiving-bot identity. The previous routing supplement remains a separate obligation; this pass statically rechecked its controlling chain without repeating cross-profile delivery experiments.
Eviction / reset Cache eviction versus conversation boundary clear. Eviction deliberately clears the context pin while retaining the channel pin. A conversation boundary is a different operation. The rehydration guard distinguishes neither kind of partial pin state.
flowchart TD
    P[Human or synthetic producer] --> A[Adapter and runner admission]
    A --> Q[Pending slot plus overflow]
    Q --> D[Drain and promote successor]
    D --> C{Recursion cap reached}
    C -->|Yes| R[Restore through shared merge]
    R --> X[Overwrite or identity fusion]
    C -->|No| G[Goal liveness and preprocessing]
    G --> I[Resolve prompt pins]
    I --> S[Publish durable snapshot]
    S --> T[Combine effective prompt]
    T --> E[Agent turn]
    E --> O[Outbound progress and final delivery]
  class P,A,D,G,T,E,O flow
  class Q,I,S state
  class C,R boundary
  class X failure
  classDef flow fill:#11243a,stroke:#58a6ff,color:#e6edf3,stroke-width:2px
  classDef state fill:#291b3e,stroke:#bc8cff,color:#f0e6ff,stroke-width:2px
  classDef boundary fill:#3d2c12,stroke:#f5b642,color:#fff2cc,stroke-width:2px
  classDef failure fill:#471e2b,stroke:#fb7185,color:#fff1f2,stroke-width:2px
  classDef control fill:#103c30,stroke:#34d399,color:#ecfdf5,stroke-width:2px
  click P "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_goals.py#L135-L179" "Pinned state owner or consumer"
  click A "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_inbound.py#L614-L652" "Pinned state owner or consumer"
  click Q "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_busy.py#L105-L132" "Pinned state owner or consumer"
  click D "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_turn.py#L3622-L3637" "Pinned state owner or consumer"
  click C "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_turn.py#L3784-L3796" "Pinned state owner or consumer"
  click R "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/platforms/base.py#L1751-L1797" "Pinned state owner or consumer"
  click X "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/platforms/base.py#L1773-L1797" "Pinned state owner or consumer"
  click G "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_turn.py#L3815-L3836" "Pinned state owner or consumer"
  click I "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_agent_cache.py#L673-L728" "Pinned state owner or consumer"
  click S "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_agent_cache.py#L644-L663" "Pinned state owner or consumer"
  click T "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_turn_runner.py#L1848-L1862" "Pinned state owner or consumer"
  click E "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_turn_runner.py#L1695-L1733" "Pinned state owner or consumer"
  click O "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_turn.py#L1912-L1945" "Pinned state owner or consumer"
Loading

Diagram sources: producer, admission, queue, drain, cap and recovery, goal liveness, pin selection, publication, combiner, agent effect, delivery. The arrows describe inspected control/data dependencies, not a claim that one probe executed every node.

Prompt-pin repair, eviction, and failed recovery

Repaired at head: the same human → synthetic → human fixture fails its combined-prompt equality assertion at baseline and passes at head, both with a warm runner and after constructing a fresh runner over the saved snapshot. This verifies the preservation decision and adoption wiring rather than merely checking the presence of a flag. The harness replaces agent execution and the store with a recording dictionary; separate real-store tests establish the storage boundary, not a real process restart.

Still present, inherited: after eviction, ephemeral_pin is absent but channel_pin remains. Rehydration returns when either exists, so the next synthetic turn rebuilds the context from an anchorless source. The fixture observed A → B → A at both pins. At head the channel hint stays stable, but the context differs. Clearing the remaining channel pin only as a controlled input lets the real adoption helper recover A; this is a positive control, not an implemented remedy.

Prior persistence claim requires qualification: teknium1's earlier review and the main review described synthetic persistence as an unchanged-snapshot no-op. That holds for the warm and fresh-runner controls. In the eviction fixture, the recorded saved snapshot changes on the synthetic turn. The caller still persists non-internal events, and the store's equality check no-ops only if the snapshot is equal. This disproves an unconditional no-op interpretation without asserting a new corruption or privacy incident. The prior P3 classification remains recorded; no silent severity reclassification is made here.

Repair choice: complete the pin-lifetime contract at the state owner, partial-state adoption guard, and publication predicate together. Blindly restoring every old snapshot can erase intentional invalidation; blindly skipping every synthetic write is not proof of coherent recovery. Acceptance requires explicit behavior for warm reuse, process rehydration, eviction, authoritative metadata changes, privacy changes, missing snapshot, and store outage. The outage control here continues to agent entry rather than failing the whole turn. Acceptance ledger.

flowchart TD
    H[Human turn records context A and channel C] --> W[Warm synthetic turn]
    W --> WA[Reuse A and C]
    H --> N[Fresh runner has no pins]
    N --> NR[Adopt saved A and C]
    H --> V[Eviction clears A but keeps C]
    V --> K[Rehydration sees C and returns]
    K --> B[Synthetic source renders context B]
    B --> P[Non-internal caller saves B and C]
    P --> U[Next human renders A again]
  class H,W,N,V state
  class WA,NR control
  class K,B,P boundary
  class U flow
  classDef flow fill:#11243a,stroke:#58a6ff,color:#e6edf3,stroke-width:2px
  classDef state fill:#291b3e,stroke:#bc8cff,color:#f0e6ff,stroke-width:2px
  classDef boundary fill:#3d2c12,stroke:#f5b642,color:#fff2cc,stroke-width:2px
  classDef failure fill:#471e2b,stroke:#fb7185,color:#fff1f2,stroke-width:2px
  classDef control fill:#103c30,stroke:#34d399,color:#ecfdf5,stroke-width:2px
  click H "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_agent_cache.py#L644-L728" "Pinned state owner or consumer"
  click W "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_agent_cache.py#L665-L693" "Pinned state owner or consumer"
  click WA "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_agent_cache.py#L686-L696" "Pinned state owner or consumer"
  click N "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_agent_cache.py#L622-L630" "Pinned state owner or consumer"
  click NR "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_agent_cache.py#L631-L642" "Pinned state owner or consumer"
  click V "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_agent_cache.py#L778-L797" "Pinned state owner or consumer"
  click K "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_agent_cache.py#L622-L630" "Pinned state owner or consumer"
  click B "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_agent_cache.py#L694-L700" "Pinned state owner or consumer"
  click P "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_turn.py#L3841-L3847" "Pinned state owner or consumer"
  click U "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_agent_cache.py#L694-L700" "Pinned state owner or consumer"
Loading

Diagram sources: context/channel owners, fresh-runner adoption, eviction, partial-state guard, render branch, snapshot publication. The saved-snapshot change was measured in the recording store; the real persistence tests were executed separately.

Prior-claim reconciliation

Earlier claim / source Current disposition Deciding evidence / remaining obligation
Issue #126109 describes synthetic channel/context A→B→A. Repaired within tested warm/cold-runner paths. Same-input baseline RED / head GREEN; pin lifecycle. No incident provider requests or cache telemetry were available.
teknium1: restart adoption was absent in the initial PR revision. Repair present; intermediate historical reproduction not rerun. Follow-up commit, current adoption call, identical old/current tree IDs, and current fresh-runner control. Baseline comparison is the true merge base, not the original PR intermediate commit.
PR body: synthetic continuations never absorb a human photo. Only partially repaired. Guarded admission is RED/GREEN, but direct photo handling and cap recovery still absorb photos.
ehz0ah inline: queue-mode debounce fuses a human turn. Still present. Real debounce queue/flush calls append human text to the synthetic event on head and baseline.
Main merge inline: grace, sentinel, photo sibling paths bypass the new boundary. Still present; underlying fusion inherited. Four focused lane fixtures fail conservation; the new marker changes the prompt consequence, not the age of the merge operation. SSA.
Main cleanup inline: fused human content is removed by pause/clear cleanup. Still present, inherited. Real slot/overflow cleanup removes one fused event and leaves no human content on both pins. Command dispatch itself was not exercised.
Main cleanup inline: inactive-goal follow-up is silently discarded. Still present, now checked at the actual consumer. The early-return branch returns the preceding result before preprocessing/agent entry in the focused fixture. The earlier probe's prefix-only observation alone would not establish this execution.
Main review repair suggestion: use the preservation marker as structural cleanup evidence. Requires narrower design. The producer sets the same marker for both goals and heartbeats. It is not by itself a goal-kind identifier. Cleanup needs a dedicated, trustworthy continuation classification plus content-conservation semantics.
Eviction inline: partial pin state causes one context flip. Still present, inherited; prior P3 retained. Head and baseline eviction RED; controlled full-absence adoption GREEN. Lifecycle.
Earlier persistence suggestion: writes are harmless unchanged-snapshot no-ops. Conditional, not universal. Warm/cold-runner controls are unchanged; eviction writes a changed snapshot in the recording store. Publication guard, store equality.
Attribution request and author response: JoaoMarcos44 co-author credit added. Repaired at commit-message level. Implementation commit contains the trailer. This does not resolve the separate author-email registry issue.
Main review: contributor registry lacks the implementation author's email. Still present in a local gate-predicate replay. Workflow predicates applied to the exact commit range find the bare numeric noreply email unmapped. This is not a claim that current hosted CI executed and failed.
Main review: automatic attribution repair can choose the numeric login rather than the PR author. Identity ambiguity still exists; no mapping written. Commit API associates the author with login 197609; live accounts 197609 and EdderTalmor have different IDs. The existing warning stands; provenance must be established rather than inferred from a green registry gate.
Main review: historical Bash-shebang failure was base-side and later scans passed. Historical receipt retained; fresh scan incomplete. This pass's full scans timed out at both pins. No fresh whole-tree shebang-clean claim is made, and no new blame is assigned to this PR.
Routing supplement and inline: busy resume loses receiving-source provenance. Source chain still present; prior runtime receipt retained, not rerun. Changed resume call → plain source clone versus identity-preserving helper → delivery fallback. No live send or authorization-bypass claim.
Second pass: no further actionable findings. Superseded for coverage, not a fabricated historical retraction. F1 demonstrates an additional recovery boundary; the earlier statement describes that pass's examined scope.
All prior test totals and 38-test supplement. Historical selections, not current proof. Fresh named selections and independent assertions appear in the verification ledger; no totals have been blended across runs.
Prior CI descriptions: workflows require approval and there are no exact-head checks. Still present at retrieval. Check-runs endpoint, statuses, workflow runs; local green is not hosted green.
PR prior-art claim: this port supersedes the competing approaches. Historical relationship needs current context. #126167 has advanced with additional queue/provenance changes absent here. Related-work comparison.

Verification and test adequacy

Every Python run below used the repository runner with isolated Windows home/state/temp paths. Failures are intended contract assertions against an unrepaired product, not green tests disguised with xfail.

Experiment Merge base Reviewed head What it establishes
Reviewer queue contracts 2 passed, 11 failed 3 passed, 10 failed Four fusion lanes, three cleanup/discard cases, three cap cases; import-pin and empty-slot controls. The guarded photo admission becomes GREEN only at head. F1, SSA.
Final composed drain→cap subset 1 passed, 3 failed 1 passed, 3 failed Same cap failure identities through the real drain; empty-slot recovery succeeds. It replaces the initial cap probe for the claimed evidence. Causal path.
Reviewer prompt lifecycle contracts 2 passed, 3 failed 4 passed, 1 failed Warm and fresh-runner equality repaired; eviction still RED; partial-state recovery and store-outage controls pass. Lifecycle.
Real store, test_session.py -k prompt_pin 4 passed 4 passed Invalid snapshot rejection, SQLite recovery without the JSON mirror, stale-ID fencing, compression-child recovery, explicit session-boundary clearing. Assertions.
Existing focused gateway selection Not rerun as a group 63 passed across 8 files Pin wiring, goal drain, heartbeat, queue consumption, active text merge, busy acknowledgment, prompt freeze, final delivery ledger.

Harness integrity: the first cap fixture omitted the required pending_event argument to promotion. Its three cap errors were harness errors, not product evidence. That attempt is preserved. The corrected stage fixture failed at the conservation assertion; the final version then exercised the real drain and failed the same three conservation assertions at both pins. No product files were repaired. Logs, probe sources, source pins, commands, exits, and SHA-256 receipts are in the companion evidence packet.

Limits that matter:

  • The recording handler harness mocks agent execution, authorization, session lookup, and storage. Its combined-prompt observations are not live provider-cache measurements. The separate store tests do exercise SQLite and on-disk recovery.
  • The queue probes use in-memory fake adapters and direct real boundary calls. The inactive-goal case supplies a false liveness answer; it proves the consumer response, not a concurrent real DB transition or full slash-command dispatch.
  • The local command policy refused the initial selection containing test_goal_resume_restart.py. That file was omitted rather than the rule bypassed. Its source assertions were reviewed, but this pass claims no execution of that file.
  • No new cross-profile delivery reproduction, production exploitation, credential access, real platform send, provider call, or incident telemetry collection was performed. The prior routing evidence is explicitly historical.
  • Full-tree Bash-shebang scans timed out at both pins. The earlier clean receipts remain historical, not refreshed. Reconciliation.

Related work and composition

Work Live pin / actual relationship Disposition
#126155 10df6fd2b5361e4a8db144c836b6b6950ce76b9f; 6 changed files, channel-pin-focused approach. Overlapping alternative, not a verified combined repair. Its constructor-keyword failure in the PR body is an API-shape RED, not by itself proof of the complete behavioral fix.
#126167 25dfcbbc6775080382b314a02d0793c8069e921a; 12 changed files including debounce, inbound, source-copy, and restoration changes. Live patch inspection confirms broader changes than the historical supersession description. A local merge-tree with this head has 4 content conflicts: cache, goals, turn, and resume-test files. No executable combined tree or union-test claim.
#78015 58a2cd54854a627f9e2c9dd4a1737efe782593fc; generic recursion-cap queue restoration in its older gateway/run.py topology, plus queue tests. Existing ownership of the generic class behind F1. Not copied, implemented, or certified here.
#129126 4592d72e40f9ffbc74cb64b5c2099451c93a740d; base-adapter and busy-runner sender/conservation/order work. Relevant neighboring queue repair; not equivalent to synthetic prompt identity and not tested as a union here.
#127944 dce6efedfca2ae5960e05ed17325938bd6d10322; actual patch changes the Discord context renderer and change key, voice source/adapter, and adds voice tests. Local uncommitted composition created without conflict, tree a4943ea9eec243f58e0312c613939498e585aac6. Composition result appears in the final evidence update below.
Target-main snapshot Local merge-tree resolves to 58f76d0f564fca3b8c772cef82b06d85fef26341. Textually clean, not executed as a full combined suite. The inspected implementation files are unchanged between baseline and that main snapshot.

F2 — P2: the added wording assertion fails with the complementary voice fix

The local composition of this head with #127944 is textually clean, but the six-file canonical run reports 42 passed, 1 failed. The only failure is this PR's added line 354 assertion, which requires the literal substring Triggering message: provided per-turn. The complementary renderer change deliberately uses wording that also describes voice turns without a message ID. The original assertion passes at standalone head; the same test fails in the pinned union, before reaching the next line's byte-equality assertion.

This is a conditional test-composition defect, not evidence that the combined prompt behavior regressed. Independent reviewer lifecycle assertions, which check combined-prompt equality without freezing the sentence, report 5 passed on the same union. That includes the formerly failing eviction fixture: the voice change removes its message-ID-dependent rendering difference. The partial-pin adoption guard itself is still unchanged, so this is a renderer-level resolution for this fixture, not proof that every eviction/invalidation obligation is closed. The union's separate queue contracts remain 3 passed, 10 failed, with the same failure identities as standalone head; F1 and the existing queue/cleanup findings are not repaired by the voice change.

Repair/regression contract: preserve the semantic test—three effective prompts byte-identical and the authoritative channel/parent inputs retained—without requiring obsolete prose. If a fixture-activation assertion is needed, assert the relevant capability/metadata relationship rather than an exact sentence fragment. The exact composition must pass both this pin-contract test and the complementary voice tests. The current #126167 description records the same wording-coupling issue in its own implementation; that adjustment is absent from this PR. No test or product source was changed to make the union green.

The executed union covers test_internal_event_pin_wiring.py, test_goal_continuation_drain.py, test_heartbeat_poller.py, test_queue_consumption.py, test_voice_session_prompt.py, and test_prompt_tail_freeze.py, followed by both reviewer contract files. The policy-refused resume-test file remains excluded. An initial composition worktree allocation failed for lack of disk space; only this task's redundant source copies were removed, and the successful union was rebuilt and pinned before any composition test. No remote merge or commit was made.

Repair and acceptance ledger

Obligation State Evidence needed for closure
Normal and cold-runner prompt continuity Repaired within the measured scope. Retain same-input RED/GREEN and the real-store contracts; incident cache performance remains unmeasured.
All merge surfaces preserve identity Still present / partial repair. Separate accepted human and synthetic events through grace/photo/sentinel, debounce, and cap recovery. Check final effect, not only helper output.
Human content survives goal cleanup Still present, inherited. Distinct human input remains runnable or explicitly visible after pause/clear/inactive transitions; preserve flag alone is not a goal-kind marker.
Admitted FIFO work survives depth limits P1, still present, inherited. Queue-front restoration conserves object identity, contents, order, and capacity through the final consumer.
Receiving-source provenance Source-confirmed outstanding; historical dynamic evidence. Identity-preserving source copy and defensive integration coverage through ordinary ingress and recursive consumption, without borrowing the runtime's transport identity.
Eviction versus durable adoption Still present; prior P3 retained. Define intended invalidation, then prove coherent context/channel lifetime and publication across partial-state recovery.
Contributor registry Unresolved identity/mapping obligation. Evidence-backed intended attribution; no guessed numeric-login mapping.
Related PR composition Separate from standalone head. Resolve actual overlap and assert semantic equality/conservation; textual mergeability is insufficient.
Hosted validation Unexecuted. Exact-head workflow execution and results; pending approval is not green.

Conclusion: this PR repairs the original warm/cold-runner pin discontinuity, but its queue identity boundary is not closed over the lifecycle. F1 adds a concrete recovery failure; the earlier obligations remain individually visible. This is a COMMENT review, not an approval, request-changes verdict, merge action, or implementation.

Comment thread gateway/run_busy.py
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.

[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.


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.

@EdderTalmor

Copy link
Copy Markdown
Author

Thanks @andrexibiza — walking the ledger from the newest review outward.

F1 (recursion-cap restoration) — accepted, and it is the right frame. Restoring the drained event through the unguarded merge helper after promote has already installed the successor is the one place where admitted work can vanish with no concurrent arrival. It is inherited (same 3 failures at fd0b98f6), but "inherited" has not been a useful answer for the two prior rounds on this PR, so I'm taking it as in-scope: if my guard can be undone by the cap branch, the guard is decorative. Repair target is queue-front restoration — retain the staged successor, restore the current event ahead of it, never merge, and don't count the restore as fresh capacity. I'll cover both orders, the photo case, existing overflow, and capacity-full.

P2 (test_internal_event_pin_wiring.py:354) — accepted, trivially fixable, and I'll do it first. You're right that the literal is the wrong oracle. I'll keep byte-equality of the three effective prompts plus the authoritative channel/parent inputs as the contract and drop the sentence fragment; if fixture activation needs asserting, I'll assert the metadata relationship. Green on the composed tree with #127944 is the acceptance bar.

On the acceptance contract as a whole. The honest summary: the normal and cold-runner pin continuity is repaired and reproduced RED/GREEN. The queue identity boundary is not closed — ingress/debounce/grace/photo, goal cleanup, and now cap recovery all still fuse or drop a human turn. The preserve_prompt_pins flag is not a goal-kind marker, so the cleanup/discard sites can't be fixed by widening its use; they need a structural continuation classification plus content conservation. I'm not going to claim those are close.

Two things I want on the record rather than argued:

  1. The concurrency sub-findings (stale-continuation gate, cleanup popping a fused event) are consumed from the same fused object, so they're downstream of the boundary, not separate defects. I'll fix them as defense-in-depth after the boundary, not instead of it.
  2. Eviction (P3) is an intent call, not a mechanical one. I don't think "eviction forces re-render for preserve turns" is acceptable as a documented behavior — eviction is a capacity event, and silently changing prompt identity on a capacity event is exactly the class of surprise this PR exists to remove. But I'm not going to redefine the pin lifetime contract unilaterally; I'd rather take a maintainer decision here than land a half-specified one.

Vehicle. #126167 has advanced and now covers debounce/inbound/source-copy too, with 4 content conflicts against this head. If maintainers would rather consolidate there, say so and I'll rebase onto it rather than maintain two divergent prompt-pin repairs.

CI. This head has zero check-runs and three workflows sitting at action_required. Local green in these reviews is not hosted green, and I'm not treating it as such.

@andrexibiza andrexibiza left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Re-review against the acceptance contract in #127208 (comment), pinned to 69c12764dfc6e7d8329a7113aa1ae0de84f51307.

The response gets the main boundary right, but the head has not moved since that comment, so F1/P2 and the wider queue-lifecycle repair are still proposals, not verified fixes. I found one additional class that needs to be explicit in the repair contract because it exists without any fused event at all.

P1 — goal-continuation identity is currently forgeable by ordinary user text

_is_goal_continuation_event() decides identity from a user-controlled string prefix:

return str(text).startswith("[Continuing toward your standing goal]\nGoal:")

That predicate feeds two destructive sinks:

  1. _clear_goal_pending_continuations() removes matching pending/overflow events on goal pause/clear.
  2. _run_agent_queued_followup() discards a matching event when the goal is no longer active.

So a completely ordinary human message whose text begins with the continuation template can be silently deleted even when it was never synthetic, never merged with a continuation, and never carried preserve_prompt_pins=True. This is independent of the fusion findings. The current text sentinel is presentation, not provenance.

The structural classification you proposed therefore needs to be a first-class event property created at the producer and preserved across every copy/queue/debounce/merge boundary. It cannot be inferred from text and it cannot be aliased to preserve_prompt_pins: prompt identity, authorization/internality, and work-kind are separate axes. It also needs enough specificity to distinguish at least goal continuation from heartbeat/loop/internal synthetic work so goal cleanup cannot consume unrelated synthetic turns.

Queue-front restoration contract

Your F1 repair target is directionally correct, but the acceptance test should assert the complete state transition, not only that the current event reappears. At recursion-cap entry, promotion may already have moved the successor into the single pending slot. Restoring the current event ahead of it therefore has to conserve all three things atomically:

  • current event identity/content,
  • staged successor identity/content,
  • remaining overflow order and total admitted depth.

No merge helper should participate in that restoration path, and restoring already-admitted work must not become a new-cap admission decision.

flowchart TD
  classDef bad fill:#3b0d17,stroke:#ff5470,color:#fff,stroke-width:2px
  classDef good fill:#0b2e2a,stroke:#2dd4bf,color:#fff,stroke-width:2px
  classDef state fill:#171a2b,stroke:#8b5cf6,color:#fff,stroke-width:2px

  A["pending = current<br/>overflow = [successor, rest...]"]:::state
  B["promote()<br/>pending = successor<br/>current returned"]:::state
  C["recursion cap"]:::state
  D["merge_pending_message_event(current)<br/>may absorb/replace successor"]:::bad
  E["restore front without merge<br/>pending = current<br/>overflow = [successor, rest...]"]:::good

  A --> B --> C
  C --> D
  C --> E

  click A "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_busy.py#L116-L132" "Pending and overflow ownership" _blank
  click B "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_busy.py#L116-L132" "Promotion state transition" _blank
  click C "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_turn.py#L3784-L3796" "Current recursion cap" _blank
  click D "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_turn.py#L3784-L3796" "Current cap branch" _blank
  click E "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_busy.py#L116-L132" "Promotion state and restoration acceptance contract" _blank
Loading

Closure conditions I would use for the next head

  • Structural work_kind/equivalent provenance, not text-prefix recognition.
  • Proven preservation through dataclasses.replace, pending slot, overflow FIFO, debounce, photo/grace paths, and recursion-cap restoration.
  • Human text equal to the literal continuation template survives both inactive-goal drain and /goal pause|clear.
  • Goal cleanup removes only events structurally identified as goal continuations.
  • F1 conservation tests cover both slot/overflow orderings, photo, pre-existing overflow, and full-cap state.
  • P2 drops the renderer prose literal and keeps semantic prompt equality on the #127944 composition.
  • Exact-head hosted CI when available; action_required is still not green.

The response correctly rejects “inherited” as a reason to leave the class open. The same standard applies here: the text-sentinel classifier is another hole in that class and should be closed in the same vehicle rather than documented around.

@andrexibiza andrexibiza left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Follow-up evidence: cancellation, FIFO recovery, and pin lifetime

This supplements the structural-classification review with executed command/consumer evidence, a distinct goal-settlement failure, and a correction to the earlier eviction finding. Your response accepts queue-front restoration and a semantic test oracle, but head 69c12764dfc6 has no subsequent implementation commit. A separate human event is deleted without fusion. Independently, a judge returning after /goal pause or /goal clear can restore active despite the command's persisted change and returned acknowledgment. Both fail at the merge base and are inherited. The original warm and fresh-runner prompt-continuity repair remains demonstrated.

I also need to correct my earlier eviction language. Actual capacity eviction retains both conversation pins in this tree. The remaining P3 concerns the explicit rebuild helper reached by /reasoning high, which leaves a partial pair and permits a context flip. The distinction matters to both the repair and the proposed maintainer decision.

Revision and execution boundary

Object Exact revision / meaning
Reviewed PR head 69c12764dfc6e7d8329a7113aa1ae0de84f51307; two commits, eleven changed files
Comparison baseline fd0b98f6256446a9115c54f1a92ace5ea74c929f, the true merge base
Target branch snapshot e85706cba780382ef91ba7a38a20ebe95207a795; distinct from the comparison baseline
Goal-settlement experiment Head plus the exact four-file patch of #124017 at 6d9fe6b9914f07ddd0104d7d5291a582b79f7898; local index tree 985d1dd326dfcda986d8c8551add76048a4ab814
Voice composition experiment Head plus the exact five-file patch of #127944 at dce6efedfca2ae5960e05ed17325938bd6d10322; local index tree 0363886141dafa367dacb95e6e95a2e6c1dbf810

Both exact patches applied cleanly in separate local worktrees; this does not establish mergeability with their full histories or current main. Original head and baseline tracked source stayed unchanged. Runnable reviewer fixtures are included below.

P1 — goal cleanup still misclassifies an independent human message

This supplies runnable evidence for the newer classification concern and extends the existing cleanup finding. Its old fused-content examples remain one class. The case below has no synthetic producer and no merge.

Through the real /queue handler, queue this ordinary request quoting a prior conversation:

[Continuing toward your standing goal]
Goal: prepare the quarterly summary

The two lines above are copied from my earlier conversation. Please explain what the status means.

The resulting event retains the human message ID, internal=False, and preserve_prompt_pins=False. Nevertheless, _is_goal_continuation_event recognizes only the text prefix. The actual busy /goal pause and /goal clear handlers delete that separate event from either the pending slot or overflow. All four combinations fail conservation at both baseline and head. This is loss of an independently accepted human request, not an artifact of mixing it with continuation text.

There is a second consumer of the same incorrect classification. With no goal configured, the real queued-follow-up stale-goal branch discards the quoted human request. In the composed base-adapter lifecycle, the queue ends empty and the recording model sink receives nothing. The otherwise identical ordinary-text control reaches the sink once. This fifth failure remains even if every merge boundary is repaired.

Repair contract: give actual goal continuations a dedicated structural kind at their producer, carry that kind through event rewrites, and use it in both destructive cleanup and stale consumption. The preservation flag set here is shared with heartbeats and cannot be the goal-kind discriminator. Retain separate human events, their text/media and order through pause, clear and inactive-goal processing. Any cleanup of a goal event that already contains human content must also preserve that accepted content while the fusion paths are being closed.

Regression contract: the quoted request survives pause and clear in both slot positions and reaches the consumer when no goal exists; an actual goal continuation is removed when canceled; an ordinary human request remains runnable. The five classification cases in Reproducer A exercise this subset. Heartbeat preservation must not make a heartbeat eligible for goal cleanup; that is a required repair test, not a claimed passing case here.

P1 — an in-flight judge can reactivate an acknowledged paused or cleared goal

This is a separate, inherited state-settlement defect in the unchanged goal evaluator. It is newly demonstrated in this review, with an existing repair owner identified below. A queue-boundary repair cannot prevent it because the lost state belongs to the goal itself.

The gateway loads a manager, awaits its judge in an executor, then enqueues the decision. During that await, the same session's actual /goal pause or /goal clear handler persists the user's changed state and acknowledges it. The evaluator still holds its original active snapshot. When the judge returns continue, evaluate_after_turn unconditionally saves that snapshot through the whole-state writer, restoring active and creating another continuation.

The deterministic interleavings use synchronization events around only the external judge response. Commands, GoalManager, session lookup, SQLite persistence, executor scheduling and continuation production are real:

Interleaving State after acknowledged command State after judge returns Queued continuations Fresh active check
/goal pause while judge waits paused active 1 true
/goal clear while judge waits cleared active 1 true

The fresh read in the stale-continuation gate cannot reject this outcome on inactive-state grounds: the stale writer has made the durable state active again. The measured outcome is a returned pause/clear acknowledgment followed by persisted active state and a newly queued continuation whose fresh goal-active check returns true. The probe did not transport-deliver that acknowledgment or run the generated continuation's downstream admission, model or tools. Serial pause/clear-before-evaluation controls remain inactive and never call the judge.

Existing repair: #124017, by 0xble, implements conditional evaluator settlement. It evaluates an isolated snapshot, stages evaluator writes, and compares the current serialized goal state, including its mutation identity, inside the SQLite write transaction before committing. A conflicting command state is reloaded and continuation is suppressed. Applying its exact four-file patch to this head changes both interleavings to passing: the states remain paused/cleared, the queue stays empty, and the fresh active check is false. The five human-classification failures remain. That separation is executable evidence that these are different controlling mechanisms.

Repair contract: settle the evaluator only against the unchanged goal revision/state, atomically, and discard an obsolete continuation decision. A separate read followed by an unconditional save leaves the race open. Keep command handling responsive while the slow judge runs. Use the existing settlement work as the integration candidate and retain the two command-overlap regressions; the two passing cases do not establish all of #124017's correctness or all later enqueue/cancellation interleavings.

%%{init: {"theme":"base","themeVariables":{"darkMode":true,"primaryTextColor":"#E6EDF3","edgeLabelBackground":"#161b22","lineColor":"#8B949E"}}}%%
flowchart TD
    DB[("SQLite: active goal")]:::owner
    SNAP["Evaluator holds active snapshot"]:::normal
    WAIT["External judge waits"]:::normal
    COMMAND["Concurrent /goal pause or clear"]:::normal
    ACK[("SQLite paused or cleared; command acknowledged")]:::verified
    RESULT["Judge returns continue"]:::normal

    DB --> SNAP --> WAIT
    WAIT -->|"while waiting"| COMMAND
    COMMAND --> ACK
    WAIT -->|"later reply"| RESULT
    ACK -.->|"change commits before reply"| RESULT

    subgraph CURRENT["Reviewed head 69c12764"]
        direction TB
        SAVE["Unconditional stale save"]:::failure
        ACTIVE[("SQLite becomes active again")]:::failure
        ENQUEUE["Enqueue continuation"]:::failure
        CONSUME["Fresh active check returns true"]:::failure
        SAVE --> ACTIVE --> ENQUEUE --> CONSUME
    end

    subgraph REPAIR["Existing #124017: local composition"]
        direction TB
        COMPARE["Atomic comparison with expected snapshot"]:::owner
        REJECT["Changed state rejects settlement"]:::verified
        RETAIN["Pause or clear retained; no continuation"]:::verified
        OPEN["Integration into #127208 remains open"]:::unresolved
        COMPARE --> REJECT --> RETAIN --> OPEN
    end

    RESULT -->|"current head"| SAVE
    RESULT -->|"conditional repair"| COMPARE
    ACK -.->|"current durable state"| COMPARE
    style CURRENT fill:#111820,stroke:#484F58,color:#E6EDF3
    style REPAIR fill:#101D17,stroke:#3FB950,color:#E6EDF3

    classDef normal fill:#102A43,stroke:#58A6FF,color:#E6EDF3,stroke-width:2px;
    classDef failure fill:#3A151C,stroke:#FF7B72,color:#E6EDF3,stroke-width:2px;
    classDef unresolved fill:#392A0E,stroke:#E3B341,color:#E6EDF3,stroke-width:2px;
    classDef verified fill:#0F2D21,stroke:#3FB950,color:#E6EDF3,stroke-width:2px;
    classDef owner fill:#2A1B3D,stroke:#BC8CFF,color:#E6EDF3,stroke-width:2px;
    linkStyle default stroke:#8B949E,stroke-width:2px,color:#C9D1D9
 click DB "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/hermes_cli/goals.py#L634-L644" "Pinned implementation" _blank
 click SNAP "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/hermes_cli/goals.py#L1477-L1533" "Pinned implementation" _blank
 click WAIT "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/hermes_cli/goals.py#L1477-L1533" "Pinned implementation" _blank
 click COMMAND "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/hermes_cli/goals.py#L1163-L1186" "Pinned implementation" _blank
 click ACK "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/hermes_cli/goals.py#L1163-L1186" "Pinned implementation" _blank
 click RESULT "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/hermes_cli/goals.py#L1477-L1533" "Pinned implementation" _blank
 click SAVE "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/hermes_cli/goals.py#L634-L644" "Pinned implementation" _blank
 click ACTIVE "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/hermes_cli/goals.py#L634-L644" "Pinned implementation" _blank
 click ENQUEUE "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_goals.py#L284-L317" "Pinned implementation" _blank
 click CONSUME "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_turn.py#L3815-L3822" "Pinned implementation" _blank
 click COMPARE "https://github.com/NousResearch/hermes-agent/blob/6d9fe6b9914f07ddd0104d7d5291a582b79f7898/hermes_cli/goals_evaluation.py#L48-L102" "Pinned implementation" _blank
 click REJECT "https://github.com/NousResearch/hermes-agent/blob/6d9fe6b9914f07ddd0104d7d5291a582b79f7898/hermes_cli/goals_evaluation.py#L48-L102" "Pinned implementation" _blank
 click RETAIN "https://github.com/NousResearch/hermes-agent/blob/6d9fe6b9914f07ddd0104d7d5291a582b79f7898/hermes_cli/goals_evaluation.py#L48-L102" "Pinned implementation" _blank
 click OPEN "https://github.com/NousResearch/hermes-agent/pull/127208#pullrequestreview-5384343051" "Published observation and acceptance scope" _blank
Loading

Blue denotes the normal path, red the demonstrated failure, green the tested stable outcome, purple the state/transaction owner, and amber an outstanding integration. Sources: gateway snapshot, executor and enqueue, command writes, unconditional settlement, fresh active check, and #124017's transactional comparison. The green branch is the tested local composition; it is absent from this PR head.

A suspected neighboring failure was ruled out: when a genuine stale continuation is dropped while a separate human successor has already been promoted, the real base adapter recovery loop does process that successor. The control paused the goal during final-send suspension, then observed the human turn run, the queue empty, and session ownership release. I am not retaining a claim that this clean successor is stranded.

P1 — accepted FIFO recovery work remains necessary for same-kind messages too

The previous cap finding is still present. The actual drain promotes the successor first, then depth-cap recovery sends the drained event through the generic merge helper. Recovery can therefore replace or absorb the already-staged successor even with no concurrent arrival.

The fresh six-case probe starts from actual _enqueue_fifo, drains/promotes, enters _run_agent_queued_followup at the configured depth cap, and then drains the retained objects. On both baseline and head, the empty-slot control passes and the other five cases fail:

Accepted FIFO Observed violation at the cap
One human event Control: same event retained
Human, human Staged successor lost
Synthetic, human Human successor lost
Human, synthetic Synthetic successor lost
Human photo, synthetic Photo/caption merged into the synthetic-owned event
Human, synthetic, human, human One admitted event lost; the later tail remains

Your accepted repair contract is the correct conservation requirement: restore the current event at the front, retain the successor and overflow order, never merge during recovery, and do not charge restored work as new capacity. The human/human case makes that restoration unconditional. The current #126167 cap branch restores separately only when effective preservation identity differs; its same-identity branch still calls the merge helper. The current #78015 helper and caller provide an existing generic front-restoration implementation to examine. I inspected those current sources; I did not execute a #126167 or #78015 composition in this pass.

Reproducer B asserts object identity, payload/media, count and FIFO order after recovery. It does not run the recovered events' models. Retain the previously requested capacity-full case in the eventual regression suite as well; this new six-case run does not claim to cover full-capacity restoration.

P3 — correction: capacity eviction passes; explicit rebuilding leaves a partial pair

The earlier P3 remains at P3, with a narrower trigger. The LRU, idle-TTL, and memory-pressure paths remove the cached agent directly and preserve both conversation pins. Each actual removal path retained the pair before the following turn; at head, the synthetic and next human turns then preserved the combined ephemeral prompt and durable snapshot. These paths do not call the explicit invalidating helper.

The failure occurs through _evict_cached_agent, including the real /reasoning high handler. It clears ephemeral_pin and leaves channel_pin. Rehydration returns whenever either pin already exists, so the partial pair prevents adoption. The next synthetic event renders context B from its anchorless source, persists the changed pair, and the next human renders context A again. The channel hint remains stable; the observed change is in session-context bytes. The reasoning override itself is applied correctly.

This is a concrete runtime-only caller: the cache-signature code expressly excludes reasoning because it is a per-message setting with no prompt/tool effect. The conversation-cache invariant and conversation-boundary funnel already distinguish a conversation boundary from resource eviction. Capacity release therefore does not need a new lifetime ruling. The remaining implementation question is which explicit callers truly invalidate prompt identity and how that intent is represented.

Repair contract: retain a coherent context/channel pair across resource release and runtime-only rebuilding. For a real prompt invalidation, make the reason and snapshot eligibility explicit at the caller/boundary. Blindly adopting every partial pair could revive an intentionally obsolete snapshot; suppressing every synthetic snapshot write would also be wrong. The current privacy-policy control correctly replaces a snapshot rendered under the prior policy, and authoritative human metadata/channel changes correctly establish a new pair.

Regression contract: real /reasoning high followed by synthetic then human must preserve the effective ephemeral bytes and durable snapshot, while the setting change takes effect. Keep actual LRU/TTL/pressure, both-absent SQLite adoption, authoritative metadata, privacy-policy change, missing snapshot and store-outage controls. Reproducer C covers these paths. Both explicit-helper and real-reasoning cases fail at head; the other ten pass. The baseline is three passing and nine failing. Baseline capacity-case failures occur on the following synthetic turn because the original preservation repair is absent, not because capacity eviction removed the pair.

%%{init: {"theme":"base","themeVariables":{"darkMode":true,"primaryTextColor":"#E6EDF3","edgeLabelBackground":"#161b22","lineColor":"#8B949E"}}}%%
flowchart TD
    PAIR[("Established conversation and SQLite pins A")]:::owner

    subgraph CAPACITY["Capacity removal"]
        direction TB
        CACHE["LRU / TTL / pressure removal"]:::owner
        KEEP["Both conversation pins retained"]:::verified
        CAPSTABLE["Synthetic A → human A"]:::verified
        CACHE --> KEEP --> CAPSTABLE
    end

    subgraph RESTART["Fresh runner"]
        direction TB
        EMPTY["Both in-memory pins absent"]:::normal
        ADOPT["Adopt saved pair A from SQLite"]:::verified
        COLDSTABLE["Synthetic A → human A"]:::verified
        EMPTY --> ADOPT --> COLDSTABLE
    end

    subgraph REBUILD["Explicit /reasoning rebuild"]
        direction TB
        REASON["/reasoning clears context; channel retained"]:::unresolved
        SKIP["Partial pair skips adoption"]:::failure
        WRITE[("Synthetic context B rendered and persisted")]:::failure
        HUMAN["Next human re-renders context A"]:::failure
        CONTRACT["Explicit rebuild repair remains open"]:::unresolved
        REASON --> SKIP --> WRITE --> HUMAN --> CONTRACT
    end

    PAIR --> CACHE
    PAIR --> EMPTY
    PAIR --> REASON
    style CAPACITY fill:#101D17,stroke:#3FB950,color:#E6EDF3
    style RESTART fill:#101D17,stroke:#3FB950,color:#E6EDF3
    style REBUILD fill:#211B10,stroke:#E3B341,color:#E6EDF3

    classDef normal fill:#102A43,stroke:#58A6FF,color:#E6EDF3,stroke-width:2px;
    classDef failure fill:#3A151C,stroke:#FF7B72,color:#E6EDF3,stroke-width:2px;
    classDef unresolved fill:#392A0E,stroke:#E3B341,color:#E6EDF3,stroke-width:2px;
    classDef verified fill:#0F2D21,stroke:#3FB950,color:#E6EDF3,stroke-width:2px;
    classDef owner fill:#2A1B3D,stroke:#BC8CFF,color:#E6EDF3,stroke-width:2px;
    linkStyle default stroke:#8B949E,stroke-width:2px,color:#C9D1D9
 click PAIR "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_agent_cache.py#L622-L642" "Pinned implementation" _blank
 click CACHE "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_agent_cache.py#L1025-L1058" "Pinned implementation" _blank
 click KEEP "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_agent_cache.py#L1025-L1058" "Pinned implementation" _blank
 click CAPSTABLE "https://github.com/NousResearch/hermes-agent/pull/127208#pullrequestreview-5384343051" "Published observation and acceptance scope" _blank
 click EMPTY "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_agent_cache.py#L622-L642" "Pinned implementation" _blank
 click ADOPT "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_agent_cache.py#L622-L642" "Pinned implementation" _blank
 click COLDSTABLE "https://github.com/NousResearch/hermes-agent/pull/127208#pullrequestreview-5384343051" "Published observation and acceptance scope" _blank
 click REASON "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/slash_commands_model.py#L664-L701" "Pinned implementation" _blank
 click SKIP "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_agent_cache.py#L622-L642" "Pinned implementation" _blank
 click WRITE "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_turn.py#L2180-L2189" "Pinned implementation" _blank
 click HUMAN "https://github.com/NousResearch/hermes-agent/blob/69c12764dfc6e7d8329a7113aa1ae0de84f51307/gateway/run_agent_cache.py#L673-L699" "Pinned implementation" _blank
 click CONTRACT "https://github.com/NousResearch/hermes-agent/pull/127208#pullrequestreview-5384343051" "Published observation and acceptance scope" _blank
Loading

A is the established combined ephemeral prompt. B changes its session-context component while retaining the channel pin. Sources: capacity LRU, TTL, pressure, adoption guard, reasoning caller, explicit invalidation, and snapshot publication. This corrects the broad eviction wording in both my earlier comment and the changed rehydration docstring.

P2 — the accepted wording assertion still fails in the voice composition

The previous conditional P2 is freshly reproduced. Standalone head passes the four-file selection, 27 tests. Composing the exact five-file #127944 patch and adding its voice tests yields 33 passed, 1 failed. The sole failure is test_non_internal_synthetic_event_preserves_all_prompt_pins at line 354: the renderer no longer includes Triggering message: provided per-turn. Its next equality assertion is not reached. All seven selected voice tests pass.

Keep the three effective prompts' byte equality and authoritative channel/parent relationships as the oracle, as your response proposes. Remove the unrelated sentence assertion; if fixture activation needs an assertion, test its metadata relationship directly. The acceptance run must then be green on the composed tree. This is a test-compatibility defect introduced by the added assertion under the specified composition, not a standalone runtime failure or proof that the entire voice integration is correct.

Prior findings and related work reconciled

Earlier item Disposition in this pass
Original warm/cold-runner discontinuity and teknium1's restart follow-up Repair freshly measured with real SQLite pin reads/writes and combined-prompt sink. Intermediate-tree equivalence remains a historical receipt.
Debounce and grace/sentinel/direct-photo merge gaps Outstanding prior P1 obligations. Their source paths are unchanged at this same head. Earlier dynamic evidence is retained; those lane fixtures were not rerun in this pass. The narrow guarded admission tests do not close sibling boundaries.
Fused cleanup and inactive-goal discard Outstanding; extended above with five separate human-event cases. The old fused manifestations remain one class.
Depth-cap restoration Inherited P1 freshly reproduced; human/human coverage added above.
Receiving-source provenance Outstanding source chain, historical runtime evidence. The resume factory call and plain source clone remain; the identity-preserving copy helper is the relevant existing mechanism. No new cross-profile or live-delivery reproduction was performed.
Eviction P3 and persistence claim Corrected above: capacity passes; explicit rebuilding fails. Writes are no-ops only for an unchanged pair.
Wording P2 Freshly reproduced under the pinned voice composition.
Co-author credit Repaired at commit-message level: 512554a6 contains JoaoMarcos44's trailer. Credit also remains due to chelsealong's diagnosis and teknium1's restart repair, as recorded in the original review.
Contributor registry / numeric-email identity ambiguity Separate unresolved attribution obligation from the earlier review. The co-author trailer does not settle it. No new mapping or hosted gate result is inferred here.
Historical shebang scans, earlier test totals and “no additional findings” statement Historical receipts only. No fresh whole-tree shebang-clean claim; no aggregate formed from earlier selections. The second-pass statement describes its earlier coverage, now extended by subsequent findings.
Hosted CI Still unexecuted for these commits, as detailed below.

The consolidation choice also needs the current sources, rather than the historical related-head assumptions:

Existing work Current pinned contribution and remaining integration obligation
#124017, 6d9fe6b9914f Existing goal-settlement owner. The exact local composition fixes the two measured judge/control overlaps while leaving all five classification failures.
#126167, 25dfcbbc6775 Broader prompt/ingress/debounce/source-copy work. The factory preserves source extensions, but cap restoration remains conditional. The previously reported four content conflicts were not freshly re-run here; do not treat that historical count as a current composition result.
#78015, 6147b5387ec8 Has advanced into the current split-file layout; front restoration is called unconditionally at the cap. The earlier old-head run.py reference is stale. Source inspected, not composition-tested here.
#127944, dce6efedfca2 Complementary voice-rendering work. Current exact patch composition reproduces the one wording failure above.
#129126, ce85d9d67b06 Has advanced to the shared base_pending predicate. A future combination must carry prompt-preservation identity into that shared predicate. The field is absent from this separate branch; this is an integration obligation, not an independently established defect of isolated #129126.

Verification scope and hosted status

These are separate, named executions; failed reviewer conservation assertions count as failures.

Selection Merge base PR head Exact local composition
Reproducer A: classification, goal-command overlap and six controls 6 passed / 7 failed 6 passed / 7 failed With #124017: 8 passed / 5 failed
Reproducer B: admitted FIFO at the cap 1 passed / 5 failed 1 passed / 5 failed Not run
Reproducer C: prompt lifetime and store/policy controls 3 passed / 9 failed 10 passed / 2 failed Not run
Existing pin wiring, goal drain, heartbeat poller and queue consumption files Not rerun 27 passed / 0 failed With #127944 plus voice tests: 33 passed / 1 failed

The canonical runner was used with an explicit Python 3.12.14 interpreter, pytest 9.1.1, pytest-asyncio 1.4.0, isolated state and zero reviewer-file retries. Full dependency-lock parity was not established; the checkout declares pytest-asyncio 1.3.0. Reviewer probes used one worker; the existing four/five-file selections used two. Each failure result above exited 1; the standalone 27-test selection exited 0.

Reproducer A invokes actual command handlers directly and uses the real session and goal stores, FIFO operations and post-turn continuation production. Its adapter-lifecycle cases compose real drain, promotion, stale consumption and outer recovery through a recording handler, with fake transport, a completed outer result and a recording next-model sink; full gateway inbound admission and agent execution are not exercised. Reproducer B tests real FIFO/cap methods with an in-memory adapter fixture. Reproducer C uses the upstream handler fixture with real SQLite pin reads/writes and TurnRunner._combined_ephemeral_prompt at the sink; admission, transcript routing, leases, client release and model execution remain fixture seams. Memory pressure is an injected metric. Fresh-runner adoption is a new runner reading SQLite after removal of the JSON mirror, not a physical process restart. These are same-user local lifecycle tests; they do not measure a full remote model prefix, cache-hit telemetry, billing, or live platform delivery.

At final retrieval, head check-runs and commit statuses are both empty. The three workflows—CI, Nix, and Docker—are completed/action_required, attempt 1, with zero jobs. The implementation commit 512554a6 likewise has zero checks/statuses and no workflow run. This is unexecuted hosted validation, not an executed test failure and not hosted green. No workflow approval or rerun was performed.

Runnable evidence

Save the three fixtures below at their stated paths in an isolated checkout of the pinned revision. Use the project's configured test interpreter; the version boundary above describes the executions reported here. Run each fixture separately, as done for the recorded results:

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
HERMES_PYTHON=/path/to/test/python scripts/run_tests.sh -j 1 --file-retries 0 tests/gateway/test_review_cap_conservation.py -s --tb=short
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

For the voice composition, the named existing-test selection is:

HERMES_PYTHON=/path/to/test/python scripts/run_tests.sh -j 2 --file-retries 0 tests/gateway/test_internal_event_pin_wiring.py tests/gateway/test_voice_session_prompt.py tests/gateway/test_goal_continuation_drain.py tests/gateway/test_heartbeat_poller.py tests/gateway/test_queue_consumption.py

Patch provenance and composition tree IDs are above. The optional full fixtures support the explained mechanisms and repair contracts.

Reproducer A — test_review_continuation_conservation.py (13 cases; exact tested fixture)
"""Review-only continuation conservation probes for NousResearch/hermes-agent #127208.

No product source is modified.  All messages belong to one synthetic local user,
chat, and default profile.  Goal/session storage, command dispatch, FIFO storage,
and post-turn continuation production use the actual implementation.  The adapter
substitutes platform I/O only.  Race tests replace only the external judge call
with an event-controlled response, without replacing GoalManager or SQLite.
"""

from __future__ import annotations

import asyncio
from pathlib import Path
import threading
from types import SimpleNamespace

import pytest

from gateway.config import GatewayConfig, Platform, PlatformConfig
from gateway.platforms.base import BasePlatformAdapter, SendResult
from gateway.platforms.event import MessageEvent, MessageType
from gateway.run import GatewayRunner
from gateway.session import SessionSource, SessionStore
from gateway.turn_context import TurnContext
from hermes_cli import goals
from hermes_constants import reset_hermes_home_override, set_hermes_home_override


HUMAN_LOG = (
    "[Continuing toward your standing goal]\nGoal: prepare the quarterly summary\n\n"
    "The two lines above are copied from my earlier conversation. "
    "Please explain what the status means."
)


class RecordingAdapter(BasePlatformAdapter):
    """Actual base adapter behavior with synthetic, non-network transport I/O."""

    def __init__(self):
        super().__init__(PlatformConfig(enabled=True, token="local-review-fixture"), Platform.TELEGRAM)
        self.sent = []

    async def connect(self, *, is_reconnect=False):
        return True

    async def disconnect(self):
        self._mark_disconnected()

    async def send(self, chat_id, content, reply_to=None, metadata=None):
        self.sent.append(content)
        return SendResult(success=True, message_id=f"sent-{len(self.sent)}")

    async def send_typing(self, chat_id, metadata=None):
        return None

    async def stop_typing(self, chat_id, metadata=None):
        return None

    async def get_chat_info(self, chat_id):
        return {"id": chat_id, "type": "dm"}


@pytest.fixture
def rig(tmp_path, monkeypatch):
    home = tmp_path / "isolated-hermes"
    home.mkdir()
    monkeypatch.setattr(Path, "home", lambda: tmp_path)
    monkeypatch.setenv("HERMES_HOME", str(home))
    token = set_hermes_home_override(str(home))
    goals._DB_CACHE.clear()
    assert goals._get_session_db() is not None
    config = GatewayConfig(platforms={Platform.TELEGRAM: PlatformConfig(enabled=True, token="local-review-fixture")})
    runner = object.__new__(GatewayRunner)
    runner.config = config
    runner.session_store = SessionStore(home / "sessions", config)
    runner._session_db = None
    runner._queued_events = {}
    adapter = RecordingAdapter()
    runner.adapters = {Platform.TELEGRAM: adapter}
    source = SessionSource(platform=Platform.TELEGRAM, chat_id="4242", chat_type="dm", user_id="local-user")
    entry = runner.session_store.get_or_create_session(source)
    key = runner._session_key_for_source(source)
    assert key == entry.session_key
    assert Path(goals.__file__).resolve().is_relative_to(Path(__file__).resolve().parents[2])
    yield SimpleNamespace(runner=runner, adapter=adapter, source=source, entry=entry, key=key)
    executor = getattr(runner, "_executor", None)
    if executor is not None:
        executor.shutdown(wait=True)
    for db in list(goals._DB_CACHE.values()):
        goals._release_session_db(db)
    goals._DB_CACHE.clear()
    reset_hermes_home_override(token)


def event(rig, text, message_id="human-command"):
    return MessageEvent(text=text, message_type=MessageType.TEXT, source=rig.source, message_id=message_id)


async def queue_human(rig, text, message_id="human-queued"):
    command = event(rig, "/queue " + text, message_id)
    return await rig.runner._busy_queue_command(command, rig.key, rig.source)


async def goal_command(rig, verb):
    command = event(rig, "/goal " + verb, "human-goal-command")
    return await rig.runner._busy_goal_command(command, rig.key, rig.source)


def queued(rig):
    slot = rig.adapter._pending_messages.get(rig.key)
    return ([slot] if slot is not None else []) + list(rig.runner._overflow_queue(rig.key) or [])


@pytest.mark.asyncio
@pytest.mark.parametrize("verb", ["pause", "clear"])
@pytest.mark.parametrize("position", ["slot", "overflow"])
async def test_goal_cleanup_preserves_separate_queued_human_log(rig, verb, position):
    goals.GoalManager(rig.entry.session_id).set("prepare the quarterly summary")
    if position == "overflow":
        await queue_human(rig, "Please keep my first independent request.", "first-human")
    await queue_human(rig, HUMAN_LOG, "quoted-human")
    before = queued(rig)
    quote = before[-1]
    assert quote.text == HUMAN_LOG
    assert quote.internal is False
    assert getattr(quote, "preserve_prompt_pins", False) is False
    assert quote.message_id == "quoted-human"
    assert all(item is not quote for item in before[:-1])
    await goal_command(rig, verb)
    stored = goals.GoalManager(rig.entry.session_id).state
    assert stored is not None and stored.status == ("paused" if verb == "pause" else "cleared")
    after = queued(rig)
    print({"case": "unmerged_human_cleanup", "verb": verb, "position": position,
           "before_ids": [e.message_id for e in before], "after_ids": [e.message_id for e in after],
           "persisted_goal_status": stored.status})
    assert after == before, "Goal cleanup deleted an independently queued human message; no fusion occurred."


@pytest.mark.asyncio
@pytest.mark.parametrize("verb", ["pause", "clear"])
async def test_cleanup_control_removes_real_continuation_and_keeps_ordinary_human(rig, verb):
    mgr = goals.GoalManager(rig.entry.session_id)
    mgr.set("prepare the quarterly summary")
    continuation = rig.runner._synthetic_prompt_event(rig.source, mgr.next_continuation_prompt())
    rig.runner._enqueue_fifo(rig.key, continuation, rig.adapter)
    await queue_human(rig, "Please summarize the recorded status.", "ordinary-human")
    human = queued(rig)[1]
    await goal_command(rig, verb)
    assert queued(rig) == [human]
    assert human.message_id == "ordinary-human"


@pytest.mark.asyncio
@pytest.mark.parametrize("verb", ["pause", "clear"])
async def test_user_goal_control_survives_inflight_post_turn_judge(rig, monkeypatch, verb):
    goals.GoalManager(rig.entry.session_id).set("prepare the quarterly summary", max_turns=20)
    loop = asyncio.get_running_loop()
    judge_entered = asyncio.Event()
    release_judge = threading.Event()

    def controlled_judge(*args, **kwargs):
        loop.call_soon_threadsafe(judge_entered.set)
        assert release_judge.wait(timeout=15), "Harness did not release controlled external judge"
        return ("continue", "One ordinary drafting step remains", False, None, False)

    monkeypatch.setattr(goals, "judge_goal", controlled_judge)
    post_turn = asyncio.create_task(rig.runner._post_turn_goal_continuation(
        session_entry=rig.entry, source=rig.source, final_response="First draft is ready."))
    try:
        await asyncio.wait_for(judge_entered.wait(), timeout=10)
        assert queued(rig) == []
        response = await goal_command(rig, verb)
        after_command = goals.GoalManager(rig.entry.session_id).state
        expected_status = "paused" if verb == "pause" else "cleared"
        assert after_command is not None and after_command.status == expected_status
        assert queued(rig) == []
        release_judge.set()
        await asyncio.wait_for(post_turn, timeout=10)
    finally:
        release_judge.set()
        if not post_turn.done():
            await asyncio.wait_for(post_turn, timeout=10)
    final_state = goals.GoalManager(rig.entry.session_id).state
    pending = queued(rig)
    fresh_active_check = rig.runner._goal_still_active_for_session(rig.entry.session_id)
    print({"case": "judge_control_overlap", "verb": verb, "command_response": response,
           "after_command": after_command.status, "after_judge": final_state.status if final_state else None,
           "pending_count": len(pending), "fresh_active_check": fresh_active_check,
           "pending_prefixes": [e.text.splitlines()[0] for e in pending]})
    assert final_state is not None and final_state.status == expected_status, (
        "The already acknowledged goal control was overwritten by the in-flight judge's stale snapshot.")
    assert pending == [], "A continuation was enqueued after goal cleanup completed."
    assert fresh_active_check is False


@pytest.mark.asyncio
@pytest.mark.parametrize("verb", ["pause", "clear"])
async def test_serial_goal_control_prevents_post_turn_judge(rig, monkeypatch, verb):
    goals.GoalManager(rig.entry.session_id).set("prepare the quarterly summary", max_turns=20)

    def judge_must_not_run(*args, **kwargs):
        pytest.fail("An inactive goal should not call the external judge")

    monkeypatch.setattr(goals, "judge_goal", judge_must_not_run)
    await goal_command(rig, verb)
    await rig.runner._post_turn_goal_continuation(
        session_entry=rig.entry, source=rig.source, final_response="First draft is ready.")
    assert not rig.runner._goal_still_active_for_session(rig.entry.session_id)
    assert queued(rig) == []


def install_composed_drain_handler(rig, monkeypatch):
    """Glue the real runner drain/follow-up consumer into the real base lifecycle.

    The outer completed model result and subsequent model sink are the only
    substituted execution stages.  Admission, base task ownership, queued
    preprocessing, stale-goal check, result delivery, FIFO promotion, and outer
    adapter recovery/drain remain real.  No full agent/model execution is claimed.
    """
    executed = []

    async def model_sink(*, message, **kwargs):
        executed.append(message)
        return {"final_response": "Queued request handled.", "messages": []}

    monkeypatch.setattr(rig.runner, "_run_agent", model_sink)

    async def handler(inbound):
        if inbound.message_id != "outer-human":
            executed.append(inbound.text)
            return "Queued request handled."
        result = {"final_response": "Outer request complete.", "messages": []}
        context = TurnContext(source=rig.source, session_id=rig.entry.session_id,
                              session_key=rig.key, run_generation=1, history=[],
                              context_prompt="", result_holder=[result],
                              event_message_id="outer-human", inbound_message_id="outer-human")
        pending_event, pending = await rig.runner._run_agent_drain_pending(
            result, rig.adapter, rig.source, rig.key)
        if pending_event is not None or pending:
            result = await rig.runner._run_agent_queued_followup(
                context, rig.adapter, pending, pending_event,
                result, result, None)
        return None if result.get("already_sent") else result.get("final_response")

    rig.adapter.set_message_handler(handler)
    return executed


async def settle_adapter(rig):
    # A task can hand off to its next owner; await actual tasks, never sleep to
    # manufacture timing.  The finite fixture contains at most three turns.
    for _ in range(5):
        tasks = list(rig.adapter._background_tasks)
        if not tasks:
            break
        await asyncio.wait_for(asyncio.gather(*tasks), timeout=10)
    assert rig.key not in rig.adapter._active_sessions
    assert not rig.adapter._background_tasks


@pytest.mark.asyncio
@pytest.mark.parametrize("human_text", [HUMAN_LOG, "Please explain the copied goal status."])
async def test_no_goal_queue_consumer_preserves_human_text(rig, monkeypatch, human_text):
    assert goals.GoalManager(rig.entry.session_id).state is None
    await queue_human(rig, human_text)
    admitted = queued(rig)[0]
    assert admitted.internal is False
    assert getattr(admitted, "preserve_prompt_pins", False) is False
    executed = install_composed_drain_handler(rig, monkeypatch)
    await rig.adapter.handle_message(event(rig, "Please finish the outer request.", "outer-human"))
    await settle_adapter(rig)
    print({"case": "human_stale_gate", "has_goal": False, "input": human_text,
           "executed": executed, "remaining_queue_ids": [e.message_id for e in queued(rig)]})
    assert executed == [human_text], "A fresh human /queue request was discarded as a stale goal."
    assert queued(rig) == []


@pytest.mark.asyncio
async def test_stale_continuation_does_not_strand_clean_successor_in_base_lifecycle(rig, monkeypatch):
    manager = goals.GoalManager(rig.entry.session_id)
    manager.set("prepare the quarterly summary")
    continuation = rig.runner._synthetic_prompt_event(rig.source, manager.next_continuation_prompt())
    rig.runner._enqueue_fifo(rig.key, continuation, rig.adapter)
    human_text = "Keep this separately queued request runnable."
    await queue_human(rig, human_text)
    executed = install_composed_drain_handler(rig, monkeypatch)
    first_delivery_entered = asyncio.Event()
    release_first_delivery = asyncio.Event()
    original_send = rig.adapter.send

    async def controlled_send(chat_id, content, reply_to=None, metadata=None):
        if content == "Outer request complete.":
            first_delivery_entered.set()
            await asyncio.wait_for(release_first_delivery.wait(), timeout=10)
        return await original_send(chat_id, content, reply_to=reply_to, metadata=metadata)

    monkeypatch.setattr(rig.adapter, "send", controlled_send)
    await rig.adapter.handle_message(event(rig, "Please finish the outer request.", "outer-human"))
    try:
        await asyncio.wait_for(first_delivery_entered.wait(), timeout=10)
        # The continuation has been drained, and its separate successor promoted.
        assert [e.text for e in queued(rig)] == [human_text]
        await goal_command(rig, "pause")
        assert not rig.runner._goal_still_active_for_session(rig.entry.session_id)
    finally:
        release_first_delivery.set()
    await settle_adapter(rig)
    print({"case": "stale_goal_clean_successor", "executed": executed,
           "remaining_queue_ids": [e.message_id for e in queued(rig)]})
    assert executed == [human_text]
    assert queued(rig) == []
Reproducer B — test_review_cap_conservation.py (6 cases; exact tested fixture)
"""Reviewer contracts for already-admitted FIFO recovery; no external model or transport I/O."""
from __future__ import annotations

from pathlib import Path
from types import MethodType, SimpleNamespace

import pytest

from gateway.config import Platform
from gateway.platforms.base import BasePlatformAdapter
from gateway.platforms.event import MessageEvent, MessageType
from gateway.run import GatewayRunner
from gateway.session import SessionSource, build_session_key
from gateway.turn_context import TurnContext


def _fixture():
    source = SessionSource(platform=Platform.TELEGRAM, chat_id="cap-chat", user_id="cap-user", chat_type="dm")
    key = build_session_key(source)
    runner = GatewayRunner.__new__(GatewayRunner)
    runner._queued_events = {}
    runner._draining = False
    adapter = SimpleNamespace(_pending_messages={}, _active_sessions={})
    adapter.get_pending_message = MethodType(BasePlatformAdapter.get_pending_message, adapter)
    runner._delivery_adapter_for = lambda _: adapter
    return runner, adapter, source, key


def _shape(event):
    return {
        "text": event.text,
        "kind": event.message_type.value,
        "media": list(event.media_urls),
        "preserve": bool(getattr(event, "preserve_prompt_pins", False)),
        "message_id": event.message_id,
    }


def _event(runner, source, kind, label, tmp_path):
    if kind == "synthetic":
        return runner._synthetic_prompt_event(source, f"Continue task {label}.")
    return MessageEvent(
        text=f"User request {label}.",
        message_type=MessageType.PHOTO if kind == "photo" else MessageType.TEXT,
        source=source, message_id=label,
        media_urls=[str(tmp_path / f"{label}.jpg")] if kind == "photo" else [],
        media_types=["image/jpeg"] if kind == "photo" else [],
    )


@pytest.mark.asyncio
@pytest.mark.parametrize("kinds", [
    ("human",),
    ("human", "human"),
    ("synthetic", "human"),
    ("human", "synthetic"),
    ("photo", "synthetic"),
    ("human", "synthetic", "human", "human"),
])
async def test_admitted_fifo_survives_recursion_cap(kinds, tmp_path):
    runner, adapter, source, key = _fixture()
    original = [_event(runner, source, kind, str(i), tmp_path) for i, kind in enumerate(kinds)]
    before = [_shape(event) for event in original]
    for event in original:
        runner._enqueue_fifo(key, event, adapter)
    assert runner._queue_depth(key, adapter=adapter) == len(original)
    result = {"final_response": "previous reply", "messages": []}
    current, text = await runner._run_agent_drain_pending(result, adapter, source, key)
    assert current is original[0]
    ctx = TurnContext(source=source, session_key=key, session_id="cap-session",
                      history=[], _interrupt_depth=runner._MAX_INTERRUPT_DEPTH,
                      result_holder=[result])
    await runner._run_agent_queued_followup(ctx, adapter, text, current, "previous reply", result, None)
    after_events = []
    while runner._queue_depth(key, adapter=adapter):
        item, _ = await runner._run_agent_drain_pending(result, adapter, source, key)
        after_events.append(item)
    actual = [_shape(event) for event in after_events]
    print({"kinds": kinds, "expected": before, "actual": actual})
    assert len(after_events) == len(original), f"accepted={len(original)}, retained={len(after_events)}"
    assert all(a is b for a, b in zip(after_events, original)), "admitted FIFO object identity/order changed"
    assert actual == before, "admitted payload changed during recovery"
Reproducer C — test_reviewer_prompt_lifetime.py (12 cases; exact tested fixture)
"""Reviewer-only same-user prompt-lifetime experiments for PR 127208.

Run with scripts/run_tests.sh. The upstream handler fixture stubs admission,
leases and model execution. Prompt snapshots use a REAL SessionStore and SQLite;
the real TurnRunner combiner runs at the model-entry sink. No network model call.
"""

from __future__ import annotations

from collections import OrderedDict
from dataclasses import replace
import importlib.util
import inspect
import json
from pathlib import Path
from types import SimpleNamespace
import time

import pytest

from gateway.agent_cache_pressure import AgentCacheBounds
from gateway.config import ChannelOverride, GatewayConfig, Platform, PlatformConfig
from gateway.platforms.base import MessageEvent
from gateway.run_agent_cache import GatewayAgentCacheMixin
from gateway.run_turn_runner import TurnRunner
from gateway.session import build_session_context, build_session_context_prompt
from gateway.turn_context import TurnContext

_ROOT = Path(__file__).resolve().parents[2]
_SPEC = importlib.util.spec_from_file_location(
    "reviewer_pin_upstream_fixture", _ROOT / "tests/gateway/test_internal_event_pin_wiring.py"
)
assert _SPEC is not None and _SPEC.loader is not None
_FIXTURE = importlib.util.module_from_spec(_SPEC)
_SPEC.loader.exec_module(_FIXTURE)


@pytest.fixture
def setup_pin(monkeypatch, tmp_path):
    home = tmp_path / "pin-home"
    home.mkdir()
    monkeypatch.setenv("HERMES_HOME", str(home))
    monkeypatch.setenv("HERMES_RUNTIME_DIR", str(tmp_path / "pin-runtime"))
    monkeypatch.setattr(Path, "home", lambda: tmp_path)
    import gateway.run as gr
    policy = {"privacy": {"redact_pii": False}}
    monkeypatch.setattr(gr, "_load_gateway_config", lambda *a, **kw: policy)
    config = GatewayConfig(sessions_dir=home / "sessions", multiplex_profiles=False)
    config.platforms[Platform.DISCORD] = PlatformConfig(
        enabled=True,
        channel_overrides={_FIXTURE.PARENT_ID: ChannelOverride(system_prompt="Review parent persona.")},
    )
    source = _FIXTURE._human_thread_source()
    assert Path(inspect.getfile(GatewayAgentCacheMixin)).resolve() == _ROOT / "gateway/run_agent_cache.py"
    assert Path(inspect.getfile(TurnRunner)).resolve() == _ROOT / "gateway/run_turn_runner.py"

    def make():
        runner = _FIXTURE._make_runner(monkeypatch, config)
        # Keep the upstream routing/lease fixture but wire its pin methods to
        # the real store constructed by GatewayRunner.__init__.
        durable = runner._async_session_store._store
        entry = durable.get_or_create_session(source)
        runner.session_store.get_or_create_session.return_value = entry
        runner.session_store.get_prompt_pin.side_effect = durable.get_prompt_pin
        runner.session_store.set_prompt_pin.side_effect = durable.set_prompt_pin
        runner.session_store._generate_session_key.side_effect = durable._generate_session_key
        calls = []

        async def consume(**kw):
            ctx = TurnContext(
                source=kw["source"], context_prompt=kw["context_prompt"],
                channel_prompt=kw.get("channel_prompt"),
            )
            kw["effective_prompt"] = TurnRunner(runner, ctx)._combined_ephemeral_prompt()
            calls.append(kw)
            return {
                "final_response": "ok", "messages": [], "tools": [],
                "history_offset": 0, "last_prompt_tokens": 0,
            }

        runner._run_agent = consume
        # Resource release itself is outside this experiment. Keep selection,
        # cache removal, pin adoption, persistence and prompt assembly real.
        runner._spawn_release_thread = lambda *a, **kw: None
        runner._release_pressure_batch = lambda plan: None
        return runner, durable, entry, calls

    return SimpleNamespace(make=make, source=source, config=config, policy=policy, home=home)


async def human(runner, source, key, *, hint="Review channel hint."):
    event = MessageEvent(text="Keep working on my example.", source=source,
                         message_id=source.message_id, channel_prompt=hint)
    await runner._handle_message_with_agent(event, source, key, 1)


async def synthetic(runner, source, key):
    event = runner._synthetic_prompt_event(source, "Continue my existing example.")
    await runner._handle_message_with_agent(event, event.source, key, 1)


def cache_agent(runner, key):
    agent = SimpleNamespace(
        _last_activity_ts=time.time() - 100,
        _session_messages=[], _last_flushed_db_idx=0,
    )
    runner._agent_cache = OrderedDict([(key, (agent, "fixture-signature"))])
    return agent


@pytest.mark.asyncio
@pytest.mark.parametrize("transition", ["warm", "cold", "lru", "idle_ttl", "pressure", "explicit_evict", "reasoning_command"])
async def test_prompt_identity_survives_nonboundary_reconstruction(setup_pin, monkeypatch, transition):
    env = setup_pin
    runner, store, entry, calls = env.make()
    key = entry.session_key
    await human(runner, env.source, key)
    assert len(calls) == 1
    first = calls[0]
    pin_before = store.get_prompt_pin(key, expected_session_id=entry.session_id)
    assert pin_before and pin_before["context_prompt"] == first["context_prompt"]
    before_state = runner._peek_session_state(key).conversation
    before_pair = (before_state.ephemeral_pin, before_state.channel_pin)
    cache_agent(runner, key)
    if transition == "cold":
        # Remove the JSON mirror: adoption must read the SQLite-backed record.
        (env.config.sessions_dir / "sessions.json").unlink(missing_ok=True)
        store._db.close()
        runner, store, entry, calls = env.make()
        assert runner._peek_session_state(key) is None
    elif transition == "lru":
        runner._agent_cache_bounds_cache = AgentCacheBounds(max_size=1)
        runner._agent_cache[key + ":same-user-other-conversation"] = (SimpleNamespace(), "other")
        runner._enforce_agent_cache_cap()
        assert key not in runner._agent_cache
    elif transition == "idle_ttl":
        runner._agent_cache_bounds_cache = AgentCacheBounds(idle_ttl_secs=1)
        assert runner._sweep_idle_cached_agents() == 1
        assert key not in runner._agent_cache
    elif transition == "pressure":
        import gateway.agent_cache_pressure as pressure
        monkeypatch.setattr(pressure, "read_anon_rss_mb", lambda: 2)
        runner._agent_cache_bounds_cache = AgentCacheBounds(memory_high_mb=1, protect_recent=0)
        assert runner._sweep_agent_cache_under_pressure() == 1
        assert key not in runner._agent_cache
    elif transition == "explicit_evict":
        runner._evict_cached_agent(key)
    elif transition == "reasoning_command":
        event = MessageEvent(text="/reasoning high", source=env.source)
        await runner._handle_reasoning_command(event)
        assert runner._resolve_session_reasoning_config(session_key=key)["effort"] == "high"

    if transition in {"lru", "idle_ttl", "pressure"}:
        state = runner._peek_session_state(key).conversation
        assert (state.ephemeral_pin, state.channel_pin) == before_pair
    if transition in {"explicit_evict", "reasoning_command"}:
        state = runner._peek_session_state(key).conversation
        assert state.ephemeral_pin is None and state.channel_pin == before_pair[1]
    start = len(calls)
    await synthetic(runner, env.source, key)
    assert len(calls) == start + 1
    continued = calls[-1]
    pin_after = store.get_prompt_pin(key, expected_session_id=entry.session_id)
    await human(runner, env.source, key)
    next_human = calls[-1]
    observed = {
        "transition": transition,
        "context_continuity": first["context_prompt"] == continued["context_prompt"],
        "channel_continuity": first["channel_prompt"] == continued["channel_prompt"],
        "effective_continuity": first["effective_prompt"] == continued["effective_prompt"],
        "next_human_returns_to_first": first["effective_prompt"] == next_human["effective_prompt"],
        "durable_unchanged": pin_after == pin_before,
    }
    print("PIN_LIFETIME " + json.dumps(observed, sort_keys=True))
    assert first["effective_prompt"] == continued["effective_prompt"] == next_human["effective_prompt"], observed
    assert pin_after == pin_before, observed


@pytest.mark.asyncio
async def test_both_absent_after_partial_state_control_adopts_snapshot(setup_pin):
    env = setup_pin
    runner, store, entry, calls = env.make()
    await human(runner, env.source, entry.session_key)
    first = calls[0]["effective_prompt"]
    runner._evict_cached_agent(entry.session_key)
    runner._peek_session_state(entry.session_key).conversation.channel_pin = None
    await synthetic(runner, env.source, entry.session_key)
    assert calls[-1]["effective_prompt"] == first


@pytest.mark.asyncio
async def test_authoritative_human_metadata_and_hint_replace_old_pair(setup_pin):
    env = setup_pin
    runner, store, entry, calls = env.make()
    await human(runner, env.source, entry.session_key)
    first = calls[0]["effective_prompt"]
    renamed = replace(env.source, chat_name="Renamed review thread", chat_topic="A revised same-user topic")
    await human(runner, renamed, entry.session_key, hint="Revised review channel hint.")
    assert calls[-1]["effective_prompt"] != first
    assert calls[-1]["source"].chat_name == renamed.chat_name
    expected = build_session_context_prompt(build_session_context(renamed, env.config, entry), redact_pii=False)
    assert calls[-1]["context_prompt"] == expected
    current = calls[-1]["effective_prompt"]
    await synthetic(runner, renamed, entry.session_key)
    assert calls[-1]["effective_prompt"] == current
    pin = store.get_prompt_pin(entry.session_key)
    assert pin["channel_prompt"] == "Revised review channel hint."


@pytest.mark.asyncio
async def test_current_privacy_policy_wins_over_durable_prior_pin(setup_pin):
    env = setup_pin
    runner, store, entry, calls = env.make()
    await human(runner, env.source, entry.session_key)
    old_context = calls[0]["context_prompt"]
    env.policy["privacy"]["redact_pii"] = True
    runner, store, entry, calls = env.make()
    event = runner._synthetic_prompt_event(env.source, "Continue with my current settings.")
    await runner._handle_message_with_agent(event, event.source, entry.session_key, 1)
    expected = build_session_context_prompt(build_session_context(event.source, env.config, entry), redact_pii=True)
    assert calls[-1]["context_prompt"] == expected != old_context
    assert runner._peek_session_state(entry.session_key).conversation.ephemeral_pin[2] is True
    assert store.get_prompt_pin(entry.session_key)["redact_pii"] is True


@pytest.mark.asyncio
@pytest.mark.parametrize("store_condition", ["missing", "outage"])
async def test_unavailable_snapshot_continues_to_prompt_consumer(setup_pin, store_condition):
    env = setup_pin
    runner, store, entry, calls = env.make()
    if store_condition == "outage":
        runner.session_store.get_prompt_pin.side_effect = OSError("reviewer simulated routing-store outage")
        runner.session_store.set_prompt_pin.side_effect = OSError("reviewer simulated routing-store outage")
    await synthetic(runner, env.source, entry.session_key)
    assert len(calls) == 1 and calls[0]["effective_prompt"]
    assert calls[0]["source"].user_id == env.source.user_id

Comment thread gateway/run_goals.py
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 — 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.

"""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 — 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.

@kvnloo kvnloo left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Current vehicle decision: close this PR as superseded by #126167.

This branch correctly identified the dedicated preserve_prompt_pins shape and added the restart-rehydrate fix, but exact head 69c12764dfc6 still has the reproduced P1s already documented here: human events can merge into a preserving synthetic through sibling debounce/grace/sentinel paths, and /goal resume can lose receiving-bot/authorization provenance through plain dataclasses.replace().

#126167 carries the same original implementation with @JoaoMarcos44's authorship intact and later resolves those blockers with canonical source replacement, complete prompt-identity merge boundaries, bounded FIFO transfer receipts, sender attribution and capacity regressions. Its only remaining blocker is a current-main rebase.

Credit should remain: @JoaoMarcos44 for the implementation, @chelsealong for the first diagnosis, and @EdderTalmor for the current-main port/restart gap that helped shape the canonical branch. No additional code from this head needs promotion.

@andrexibiza

Copy link
Copy Markdown

@greptile review

@greptile-apps

greptile-apps Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 3/5 Tier: plus

[Medium impact] Adds prompt-pin preservation flag to synthetic goal and heartbeat turns.

Fix receiving-bot identity on queued resumes and separate debounced human text from synthetic continuations before merging.

Findings

  1. P1 Resume uses the wrong bot ▶
  2. P1 Human text inherits synthetic pins ▶
Fix with agent prompt
### Issue 1
gateway/slash_commands_goals.py:90
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.

### Issue 2
gateway/run_busy.py:389-390
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.

Summary

Adds preserve_prompt_pins so goal, heartbeat, and resume continuations can reuse prompt inputs without becoming internal events.

  • Synthetic goal and heartbeat turns keep the session’s prompt pins.
  • Synthetic turns and human photos stay in separate queue entries.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    H[Human turn] --> A[Ordinary admission]
    S[Synthetic continuation] --> A
    A --> P{Reuse established pins?}
    P -->|Yes| R[Restore missing pins and reuse prompt inputs]
    P -->|No| U[Refresh prompt inputs from event]
    R --> E[Run agent]
    U --> E
Loading

Reviews (1) · Last reviewed commit: "fix(gateway): rehydrate durable prompt p..." · Reviewed by Greptile

channel_prompt=event.channel_prompt,
)
else:
turn = self._synthetic_prompt_event(event.source, text)

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.

Comment thread gateway/run_busy.py
Comment on lines +389 to +390
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.

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.

@teknium1

Copy link
Copy Markdown
Collaborator

Superseded by #139865, merged as 8eecd1c. It fixes the same synthetic goal/heartbeat/resume pin overwrite (#126109) with a smaller diff, built on #126155 and #131032. Thanks @EdderTalmor for the work here.

@teknium1 teknium1 closed this Oct 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/gateway Gateway runner, session dispatch, delivery needs-decision Awaiting maintainer decision before any implementation P0 Critical — data loss, security, crash loop sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Goal-continuation / heartbeat / resume synthetic events overwrite the channel pin with None → system prompt flip, agent rebuild

7 participants