Repository navigation
fix(gateway): internal events keep the session-context and channel prompt pins - #124179
kshitijk4poor merged 7 commits into
Conversation
…hannel prompt pin Internal events (kanban wakes, delegation completions, watch notifications) carry a source rebuilt from the persisted origin: no chat_name/user_name/message_id/parent_chat_id, and channel_prompt=None. _pinned_session_context_prompt re-rendered from that source, so every internal turn re-keyed the pin and the next human turn re-keyed it back (A->B->A). Each flip rewrote already-sent system bytes and collapsed the prompt cache to the static prefix. The same toggle happened through channel_prompt and parent-keyed channel_overrides in the ephemeral system prompt. - _pinned_session_context_prompt(internal=True) reuses the existing pin verbatim and never re-pins. - Human turns record (channel_prompt, parent_chat_id) in ConversationState.channel_pin; internal turns reuse them (main run and the queued follow-up path). tests/gateway/test_internal_event_pin_wiring.py drives the real _handle_message_with_agent human->internal->human with _run_agent stubbed. Both tests fail on main and pass here; dropping internal= at the call site makes both fail again. (cherry picked from commit 3b38527)
…ompt The internal-event branch returned an unpinned render when no pin existed yet, so sessions that only ever see internal turns (kanban/API-server wakes, startup resumes) re-rendered the session-context prompt every turn and lost the verbatim-reuse immunity base gave them. Reuse an existing pin when present; otherwise fall through to render-and-pin as before. Applies teknium1's inline review suggestion on NousResearch#122048.
_pinned_channel_inputs copied the internal-turn source with a plain dataclasses.replace, which drops the wire-invisible provenance (transport ref, authorization home, identity). Under multiplexing the copied source then reaches _run_agent / _delivery_adapter_for without its identity and falls back to heuristic profile/adapter routing. gateway/AGENTS.md requires source copies to go through session_identity.replace_source; use it (late import, matching the other gateway callers) and drop the now-unused dataclasses import.
The two pin helpers encode one rule (a human turn records, an internal turn reuses) but took it in two shapes: _pinned_session_context_prompt gets `internal` as a keyword while _pinned_channel_inputs took the whole event and read getattr(event, ...) itself. Match the sibling: (session_key, channel_prompt, source, *, internal), with the call sites passing the event's values. MessageEvent.channel_prompt/internal and SessionSource.parent_chat_id are declared dataclass fields, so the getattr guards (including the new context-prompt call site) go too.
…-first pin _effective_ephemeral hand-copied TurnRunner._combined_ephemeral_prompt, so a change to the real ephemeral composition would leave the test checking a stale reimplementation. Build a TurnContext and call the real method instead. The internal-only-session fix (internal turn with no pin renders AND pins) had no test teeth: reverting it kept both tests green. The existing handler test now opens with an internal-first turn on a fresh session and asserts the ephemeral pin is populated before the human -> internal -> human sequence.
_pinned_channel_inputs reads source.parent_chat_id on the queued-followup path. The real SessionSource always has the field; the SimpleNamespace stub in test_queued_final_ledger did not, so two chained-followup tests raised AttributeError on CI. Give the stub the field instead of re-adding a getattr guard for a case production cannot hit.
teknium1
left a comment
There was a problem hiding this comment.
PR Review — #124179 (head a80ae6dcf1 after my push)
Verdict: correct; the CI red on 66b38cb6a9 was a test-stub gap, fixed in place. Merge decision stays with the maintainer; #122048 (@Kyzcreig's original) and this PR must land once.
Premise (origin/main 1c535d9689): confirmed. _pinned_session_context_prompt keys the pin on the event's source, and internal events (kanban wakes, delegation completions, watch ticks) carry a source without chat_name/user_name and channel_prompt=None, so human → internal → human re-renders the ephemeral system prompt twice (A→B→A).
Delta vs #122048: (1) an internal-first session now renders AND pins (the original rendered without pinning) — the inline suggestion on #122048, applied; (2) dataclasses.replace → session_identity.replace_source, which keeps the wire-invisible provenance attrs a plain replace drops; (3) getattr(source, "parent_chat_id", None) → source.parent_chat_id. (3) is what went red: tests/gateway/test_queued_final_ledger.py::_source builds a SimpleNamespace without the field, and two chained-followup tests reach _pinned_channel_inputs through _run_agent_queued_followup. The real SessionSource always has the field, so I gave the stub parent_chat_id=None (a80ae6dcf1) rather than re-adding a guard production cannot hit. Every test that reaches _run_agent_queued_followup / the two pin helpers (39 across 5 files) passes on the new head.
Tests: 2 new invariants pass on head; swapping the three prod files to origin/main turns both red.
Live test: handler-level with the real prompt combiner (the PR's own tests); no gateway process run here.
Suggestions
_evict_agent(gateway/run_agent_cache.py~L735) resetsephemeral_pinandvc_last("prompt-stability state rides the agent-cache lifecycle") but not the newchannel_pin. Harmless today (channel prompt andparent_chat_idare per-chat, not per-agent), but resetting it there keeps the three pins on one lifecycle. One line; non-blocking.- The two "known and unchanged" items in the body (interrupted//steer follow-ups dropping the channel prompt for one turn; the first internal event after a restart pinning a reduced render) are real and pre-existing; a follow-up, not this PR.
Looks Good
- Explicit
internal: bool/channel_promptparameters instead ofgetattron the event; the three call sites pass the event's fields. parent_chat_idis restored only when the internal source lacks one, so a legitimately different destination is never overwritten.
|
Thanks @teknium1 for the stub fix in |
…el_pin Goal continuation, heartbeat due-prompts and /goal resume events are built non-internal (they must still pass authorization) with channel_prompt=None. _pinned_channel_inputs() only reused the pin for internal events, so these synthetic turns recorded (None, parent_chat_id) as the new channel_pin, dropping the channel prompt and channel_overrides for that turn and flipping them back on the next human turn (#125763/#124179-style A->B->A: shorter ephemeral prompt, changed _agent_config_signature, cached-agent rebuild, prefix-cache miss). Add MessageEvent.inherit_channel_pin, independent of `internal` (which also gates authorization bypass and touch_activity), and set it on the three producers that carry no channel inputs of their own. _pinned_channel_inputs now reuses the pin for internal OR pin-inheriting events. Fixes #126109
…el_pin Goal continuation, heartbeat due-prompts and /goal resume events are built non-internal (they must still pass authorization) with channel_prompt=None. _pinned_channel_inputs() only reused the pin for internal events, so these synthetic turns recorded (None, parent_chat_id) as the new channel_pin, dropping the channel prompt and channel_overrides for that turn and flipping them back on the next human turn (#125763/#124179-style A->B->A: shorter ephemeral prompt, changed _agent_config_signature, cached-agent rebuild, prefix-cache miss). Add MessageEvent.inherit_channel_pin, independent of `internal` (which also gates authorization bypass and touch_activity), and set it on the three producers that carry no channel inputs of their own. _pinned_channel_inputs now reuses the pin for internal OR pin-inheriting events. Fixes #126109
…el_pin Goal continuation, heartbeat due-prompts and /goal resume events are built non-internal (they must still pass authorization) with channel_prompt=None. _pinned_channel_inputs() only reused the pin for internal events, so these synthetic turns recorded (None, parent_chat_id) as the new channel_pin, dropping the channel prompt and channel_overrides for that turn and flipping them back on the next human turn (#125763/#124179-style A->B->A: shorter ephemeral prompt, changed _agent_config_signature, cached-agent rebuild, prefix-cache miss). Add MessageEvent.inherit_channel_pin, independent of `internal` (which also gates authorization bypass and touch_activity), and set it on the three producers that carry no channel inputs of their own. _pinned_channel_inputs now reuses the pin for internal OR pin-inheriting events. Fixes #126109
Internal gateway events (background-process notifications, watch/tick events) no longer re-render the session-context and channel prompt pins. The ephemeral prompt stays byte-stable across human → internal → human turns.
Salvage of #122048 by @Kyzcreig (opened 2026-09-25). Their commit is kept as authored, with teknium1's inline review suggestion from that PR applied on top.
What was wrong
The gateway pins the rendered session-context prompt and the channel prompt per session, so the ephemeral part of the prompt stays byte-stable. Internal events carry a synthetic source and no channel prompt. When one arrived, it re-keyed both pins: the context prompt shrank (1001 → 971 bytes in the probe), and the next human turn re-rendered it again. Each flip broke the cached prefix twice.
Change
gateway/run_turn.py,gateway/run_agent_cache.py: an internal turn reuses the pinned session-context render and the pinned channel inputs instead of re-keying them.session_identity.replace_source, so bot identity survives when several bots share a gateway._pinned_channel_inputstakes explicit arguments; the three call sites pass the event's or pending event'schannel_prompt/internal.Verification
replace_sourcefix is covered by the probe only, because the stack is capped at two tests.Known and unchanged (same as base, for a follow-up):
/steerplain-text follow-ups still drop the channel prompt for one turn;Supersedes #122048.